From a77b0c38e3eb96adcf398dac275e91538db378b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 12 Mar 2021 17:32:17 +0100 Subject: [PATCH 01/37] write back account changes after tracking delegation/undelegation --- x/bank/keeper/keeper.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 305665ee506f..f661f376a02b 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -430,6 +430,7 @@ func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, balanc if ok { // TODO: return error on account.TrackDelegation vacc.TrackDelegation(ctx.BlockHeader().Time, balance, amt) + k.ak.SetAccount(ctx, acc) } return nil @@ -445,6 +446,7 @@ func (k BaseKeeper) trackUndelegation(ctx sdk.Context, addr sdk.AccAddress, amt if ok { // TODO: return error on account.TrackUndelegation vacc.TrackUndelegation(amt) + k.ak.SetAccount(ctx, acc) } return nil From af5b275bfb9e5e8e8c20bff726c61db836ea74b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 12 Mar 2021 18:12:06 +0100 Subject: [PATCH 02/37] add test case for delegated vesting amount --- x/bank/keeper/keeper_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index ebc529984bcb..6644676b799f 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" "github.com/stretchr/testify/suite" @@ -860,6 +862,12 @@ func (suite *IntegrationTestSuite) TestDelegateCoins() { // require the ability for a vesting account to delegate suite.Require().NoError(app.BankKeeper.DelegateCoins(ctx, addr1, addrModule, delCoins)) suite.Require().Equal(delCoins, app.BankKeeper.GetAllBalances(ctx, addr1)) + + // require that delegated vesting amount is equal to what was delegated with DelegateCoins + acc = app.AccountKeeper.GetAccount(ctx, addr1) + vestingAcc, ok := acc.(exported.VestingAccount) + suite.Require().True(ok) + suite.Require().Equal(delCoins, vestingAcc.GetDelegatedVesting()) } func (suite *IntegrationTestSuite) TestDelegateCoins_Invalid() { From 58c1b725e5165e59eeb5e7b94dcaaaa8fa402b0b Mon Sep 17 00:00:00 2001 From: Gianguido Sora Date: Fri, 12 Mar 2021 18:40:17 +0100 Subject: [PATCH 03/37] remove whitespace Co-authored-by: Alessio Treglia --- x/bank/keeper/keeper_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 6644676b799f..1ee48345b8a2 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -5,7 +5,6 @@ import ( "time" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" - minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" "github.com/stretchr/testify/suite" From a58c50069f0694f18401c7ac974522ba8eff21a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Mon, 15 Mar 2021 15:04:48 +0100 Subject: [PATCH 04/37] check that undelegate updates the delegatedvesting amount --- x/bank/keeper/keeper_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 1ee48345b8a2..7a957b77bcab 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -941,6 +941,12 @@ func (suite *IntegrationTestSuite) TestUndelegateCoins() { suite.Require().Equal(origCoins, app.BankKeeper.GetAllBalances(ctx, addr1)) suite.Require().True(app.BankKeeper.GetAllBalances(ctx, addrModule).Empty()) + + // require that delegated vesting amount is completely empty, since they were completely undelegated + acc = app.AccountKeeper.GetAccount(ctx, addr1) + vestingAcc, ok := acc.(exported.VestingAccount) + suite.Require().True(ok) + suite.Require().Empty(vestingAcc.GetDelegatedVesting()) } func (suite *IntegrationTestSuite) TestUndelegateCoins_Invalid() { From fcb93b30bbf990deca99050d06695a40213c71ea Mon Sep 17 00:00:00 2001 From: Jonathan Gimeno Date: Wed, 17 Mar 2021 15:45:16 +0100 Subject: [PATCH 05/37] temp commit --- x/auth/keeper/migrations.go | 19 ++++++++ x/auth/keeper/migrations_test.go | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 x/auth/keeper/migrations.go create mode 100644 x/auth/keeper/migrations_test.go diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go new file mode 100644 index 000000000000..ee5178a9a82c --- /dev/null +++ b/x/auth/keeper/migrations.go @@ -0,0 +1,19 @@ +package keeper + +import sdk "github.com/cosmos/cosmos-sdk/types" + +// Migrator is a struct for handling in-place store migrations. +type Migrator struct { + keeper AccountKeeper +} + +// NewMigrator returns a new Migrator. +func NewMigrator(keeper AccountKeeper) Migrator { + return Migrator{keeper: keeper} +} + +// Migrate1to2 migrates from version 1 to 2. +func (m Migrator) Migrate1to2(ctx sdk.Context) error { + + return nil +} diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go new file mode 100644 index 000000000000..098cd70d39d0 --- /dev/null +++ b/x/auth/keeper/migrations_test.go @@ -0,0 +1,84 @@ +package keeper_test + +import ( + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" + + "github.com/cosmos/cosmos-sdk/x/auth/keeper" + + "github.com/cosmos/cosmos-sdk/x/staking" + + "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + + "github.com/cosmos/cosmos-sdk/simapp" + sdk "github.com/cosmos/cosmos-sdk/types" + types3 "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" + stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +func TestMigrateVestingAccounts(t *testing.T) { + app := simapp.Setup(false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(310)) + delegatorAddr := addrs[0] + + _, valAddr := createValidator(t, ctx, app, 300) + validator, found := app.StakingKeeper.GetValidator(ctx, valAddr) + require.True(t, found) + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, time.Now().Unix()) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + + // We introduce the bug + delayedAccount.DelegatedFree = nil + delayedAccount.DelegatedVesting = nil + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + migrator := keeper.NewMigrator(app.AccountKeeper) + err = migrator.Migrate1to2(ctx) + require.NoError(t, err) + + savedDelayedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) + vDA, ok := savedDelayedAccount.(exported.VestingAccount) + require.True(t, ok) + require.Equal(t, vestedCoins, vDA.GetDelegatedVesting()) +} + +func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers int64) (sdk.AccAddress, sdk.ValAddress) { + addrs := simapp.AddTestAddrsIncremental(app, ctx, 1, sdk.NewInt(30000000)) + valAddrs := simapp.ConvertAddrsToValAddrs(addrs) + pks := simapp.CreateTestPubKeys(1) + cdc := simapp.MakeTestEncodingConfig().Marshaler + + app.StakingKeeper = stakingkeeper.NewKeeper( + cdc, + app.GetKey(stakingtypes.StoreKey), + app.AccountKeeper, + app.BankKeeper, + app.GetSubspace(stakingtypes.ModuleName), + ) + + val1, err := stakingtypes.NewValidator(valAddrs[0], pks[0], stakingtypes.Description{}) + require.NoError(t, err) + + app.StakingKeeper.SetValidator(ctx, val1) + app.StakingKeeper.SetValidatorByConsAddr(ctx, val1) + app.StakingKeeper.SetNewValidatorByPowerIndex(ctx, val1) + + _, _ = app.StakingKeeper.Delegate(ctx, addrs[0], sdk.TokensFromConsensusPower(powers), stakingtypes.Unbonded, val1, true) + + _ = staking.EndBlocker(ctx, app.StakingKeeper) + + return addrs[0], valAddrs[0] +} From 3f979a80355e97a7b7a04c4587f42320760eeb53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 17 Mar 2021 16:29:54 +0100 Subject: [PATCH 06/37] add migrator code to auth module --- x/auth/keeper/migrations.go | 17 ++++++++++++----- x/auth/module.go | 2 ++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index ee5178a9a82c..32cc2ad92260 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -1,19 +1,26 @@ package keeper -import sdk "github.com/cosmos/cosmos-sdk/types" +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/gogo/protobuf/grpc" +) // Migrator is a struct for handling in-place store migrations. type Migrator struct { - keeper AccountKeeper + keeper AccountKeeper + queryServer grpc.Server } // NewMigrator returns a new Migrator. -func NewMigrator(keeper AccountKeeper) Migrator { - return Migrator{keeper: keeper} +func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator { + return Migrator{keeper: keeper, queryServer: queryServer} } // Migrate1to2 migrates from version 1 to 2. func (m Migrator) Migrate1to2(ctx sdk.Context) error { - + m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { + return false + }) return nil } diff --git a/x/auth/module.go b/x/auth/module.go index ff053d8c7c76..32573568a2a0 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -129,6 +129,8 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd // module-specific GRPC queries. func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterQueryServer(cfg.QueryServer(), am.accountKeeper) + m := keeper.NewMigrator(am.accountKeeper, cfg.QueryServer()) + cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2) } // InitGenesis performs genesis initialization for the auth module. It returns From bcc5198941ee1a55480cb9fbcf11f6227c212d1a Mon Sep 17 00:00:00 2001 From: Frojdi Dymylja Date: Wed, 17 Mar 2021 17:01:04 +0100 Subject: [PATCH 07/37] add: query way --- x/auth/keeper/migrations.go | 37 +++++++++++++++++++++++++++++++- x/auth/keeper/migrations_test.go | 22 +++++++++---------- x/auth/module.go | 7 ++++-- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 32cc2ad92260..96e979779608 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -1,9 +1,17 @@ package keeper import ( + "fmt" + "log" + + "github.com/gogo/protobuf/grpc" + "github.com/gogo/protobuf/proto" + abci "github.com/tendermint/tendermint/abci/types" + + "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/gogo/protobuf/grpc" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) // Migrator is a struct for handling in-place store migrations. @@ -19,8 +27,35 @@ func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator { // Migrate1to2 migrates from version 1 to 2. func (m Migrator) Migrate1to2(ctx sdk.Context) error { + const fqPath = "/cosmos.bank.v1beta1.Query/AllBalances" + querier, ok := m.queryServer.(*baseapp.GRPCQueryRouter) + if !ok { + panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", m.queryServer)) + } + log.Printf("%#v", querier) + queryFn := querier.Route(fqPath) m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { + q := &banktypes.QueryAllBalancesRequest{ + Address: account.GetAddress().String(), + Pagination: nil, + } + b, err := proto.Marshal(q) + if err != nil { + panic(err) + } + req := abci.RequestQuery{ + Data: b, + Path: fqPath, + } + resp, err := queryFn(ctx, req) + if err != nil { + panic(err) + } + balance := new(banktypes.QueryAllBalancesResponse) + err = proto.Unmarshal(resp.Value, balance) + log.Printf("%s", balance.String()) return false }) + return nil } diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index 098cd70d39d0..c977d999a051 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -4,9 +4,7 @@ import ( "testing" "time" - "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" - - "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/staking" @@ -44,15 +42,17 @@ func TestMigrateVestingAccounts(t *testing.T) { delayedAccount.DelegatedFree = nil delayedAccount.DelegatedVesting = nil app.AccountKeeper.SetAccount(ctx, delayedAccount) - - migrator := keeper.NewMigrator(app.AccountKeeper) - err = migrator.Migrate1to2(ctx) + err = app.RunMigrations(ctx, module.MigrationMap{ + types3.ModuleName: 1, + // "bank": 1, + }) require.NoError(t, err) - - savedDelayedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) - vDA, ok := savedDelayedAccount.(exported.VestingAccount) - require.True(t, ok) - require.Equal(t, vestedCoins, vDA.GetDelegatedVesting()) + /* + savedDelayedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) + vDA, ok := savedDelayedAccount.(exported.VestingAccount) + require.True(t, ok) + require.Equal(t, vestedCoins, vDA.GetDelegatedVesting()) + */ } func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers int64) (sdk.AccAddress, sdk.ValAddress) { diff --git a/x/auth/module.go b/x/auth/module.go index 32573568a2a0..f0c26cd97e59 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -130,7 +130,10 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterQueryServer(cfg.QueryServer(), am.accountKeeper) m := keeper.NewMigrator(am.accountKeeper, cfg.QueryServer()) - cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2) + err := cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2) + if err != nil { + panic(err) + } } // InitGenesis performs genesis initialization for the auth module. It returns @@ -150,7 +153,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONMarshaler) json } // ConsensusVersion implements AppModule/ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 1 } +func (AppModule) ConsensusVersion() uint64 { return 2 } // BeginBlock returns the begin blocker for the auth module. func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} From c7d5798939d5705c00fabb4ace88cf73ea807a76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Thu, 18 Mar 2021 10:50:01 +0100 Subject: [PATCH 08/37] implement first iteration of migration handling for vesting accounts tracking --- x/auth/keeper/migrations.go | 129 ++++++++++++++++++++++++------- x/auth/keeper/migrations_test.go | 68 ++++++++++++---- 2 files changed, 155 insertions(+), 42 deletions(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 96e979779608..865fdb41c6a9 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -2,7 +2,10 @@ package keeper import ( "fmt" - "log" + + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/gogo/protobuf/grpc" "github.com/gogo/protobuf/proto" @@ -14,6 +17,11 @@ import ( banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) +const ( + delegatorDelegationPath = "/cosmos.staking.v1beta1.Query/DelegatorDelegations" + balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances" +) + // Migrator is a struct for handling in-place store migrations. type Migrator struct { keeper AccountKeeper @@ -27,35 +35,104 @@ func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator { // Migrate1to2 migrates from version 1 to 2. func (m Migrator) Migrate1to2(ctx sdk.Context) error { - const fqPath = "/cosmos.bank.v1beta1.Query/AllBalances" - querier, ok := m.queryServer.(*baseapp.GRPCQueryRouter) - if !ok { - panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", m.queryServer)) - } - log.Printf("%#v", querier) - queryFn := querier.Route(fqPath) m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { - q := &banktypes.QueryAllBalancesRequest{ - Address: account.GetAddress().String(), - Pagination: nil, + asVesting := vesting(account) + if asVesting == nil { + return false } - b, err := proto.Marshal(q) - if err != nil { - panic(err) - } - req := abci.RequestQuery{ - Data: b, - Path: fqPath, - } - resp, err := queryFn(ctx, req) - if err != nil { - panic(err) - } - balance := new(banktypes.QueryAllBalancesResponse) - err = proto.Unmarshal(resp.Value, balance) - log.Printf("%s", balance.String()) + + addr := account.GetAddress().String() + balance := getBalance( + ctx, + addr, + m.queryServer, + ) + + delegations := getDelegatorDelegations( + ctx, + addr, + m.queryServer, + ) + + asVesting.TrackDelegation(ctx.BlockTime(), balance.Add(delegations...), delegations) + + m.keeper.SetAccount(ctx, account) + return false }) return nil } + +func vesting(account types.AccountI) exported.VestingAccount { + v, ok := account.(exported.VestingAccount) + if !ok { + return nil + } + + return v +} + +func getDelegatorDelegations(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { + querier, ok := queryServer.(*baseapp.GRPCQueryRouter) + if !ok { + panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)) + } + + queryFn := querier.Route(delegatorDelegationPath) + + q := &stakingtypes.QueryDelegatorDelegationsRequest{ + DelegatorAddr: address, + } + + b, err := proto.Marshal(q) + if err != nil { + panic(err) + } + req := abci.RequestQuery{ + Data: b, + Path: delegatorDelegationPath, + } + resp, err := queryFn(ctx, req) + if err != nil { + panic(err) + } + balance := new(stakingtypes.QueryDelegatorDelegationsResponse) + err = proto.Unmarshal(resp.Value, balance) + + res := sdk.NewCoins() + for _, i := range balance.DelegationResponses { + res = res.Add(i.Balance) + } + + return res +} + +func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { + querier, ok := queryServer.(*baseapp.GRPCQueryRouter) + if !ok { + panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)) + } + + queryFn := querier.Route(balancesPath) + + q := &banktypes.QueryAllBalancesRequest{ + Address: address, + Pagination: nil, + } + b, err := proto.Marshal(q) + if err != nil { + panic(err) + } + req := abci.RequestQuery{ + Data: b, + Path: balancesPath, + } + resp, err := queryFn(ctx, req) + if err != nil { + panic(err) + } + balance := new(banktypes.QueryAllBalancesResponse) + err = proto.Unmarshal(resp.Value, balance) + return balance.Balances +} diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index c977d999a051..078967c0393c 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -1,10 +1,11 @@ package keeper_test import ( + "fmt" "testing" "time" - "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/cosmos/cosmos-sdk/x/staking" @@ -13,7 +14,9 @@ import ( "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" + authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" types3 "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -35,24 +38,57 @@ func TestMigrateVestingAccounts(t *testing.T) { delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, time.Now().Unix()) - _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + + migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) // We introduce the bug - delayedAccount.DelegatedFree = nil - delayedAccount.DelegatedVesting = nil - app.AccountKeeper.SetAccount(ctx, delayedAccount) - err = app.RunMigrations(ctx, module.MigrationMap{ - types3.ModuleName: 1, - // "bank": 1, - }) - require.NoError(t, err) - /* - savedDelayedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) - vDA, ok := savedDelayedAccount.(exported.VestingAccount) - require.True(t, ok) - require.Equal(t, vestedCoins, vDA.GetDelegatedVesting()) - */ + require.NoError(t, introduceTrackingBug(ctx, delayedAccount, app)) + + require.NoError(t, migrator.Migrate1to2(ctx)) + + trackingCorrected( + ctx, + t, + app.AccountKeeper, + baseAccount.GetAddress(), + vestedCoins, + sdk.Coins{}, + ) +} + +func trackingCorrected(ctx sdk.Context, t *testing.T, ak authkeeper.AccountKeeper, addr sdk.AccAddress, expDelVesting sdk.Coins, expDelFree sdk.Coins) { + baseAccount := ak.GetAccount(ctx, addr) + vDA, ok := baseAccount.(exported.VestingAccount) + require.True(t, ok) + require.True(t, expDelVesting.IsEqual(vDA.GetDelegatedVesting())) + require.True(t, expDelFree.IsEqual(vDA.GetDelegatedFree())) +} + +func introduceTrackingBug(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error { + switch t := vesting.(type) { + case *types.DelayedVestingAccount: + t.DelegatedFree = nil + t.DelegatedVesting = nil + app.AccountKeeper.SetAccount(ctx, t) + case *types.ContinuousVestingAccount: + t.DelegatedFree = nil + t.DelegatedVesting = nil + app.AccountKeeper.SetAccount(ctx, t) + case *types.PeriodicVestingAccount: + t.DelegatedFree = nil + t.DelegatedVesting = nil + app.AccountKeeper.SetAccount(ctx, t) + default: + return fmt.Errorf("expected vesting account, found %t", t) + } + + return nil } func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers int64) (sdk.AccAddress, sdk.ValAddress) { From 0bbbc0a6221656b7f2b526e8ee28c629fe71965b Mon Sep 17 00:00:00 2001 From: Jonathan Gimeno Date: Thu, 18 Mar 2021 11:07:49 +0100 Subject: [PATCH 09/37] add test --- x/auth/keeper/migrations_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index 078967c0393c..08c468c4b778 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -35,8 +35,8 @@ func TestMigrateVestingAccounts(t *testing.T) { baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, time.Now().Unix()) + app.AccountKeeper.SetAccount(ctx, delayedAccount) _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) @@ -45,11 +45,13 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) - migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) - // We introduce the bug - require.NoError(t, introduceTrackingBug(ctx, delayedAccount, app)) + savedAccount := app.AccountKeeper.GetAccount(ctx, delayedAccount.GetAddress()) + vestingAccount, ok := savedAccount.(exported.VestingAccount) + require.True(t, ok) + require.NoError(t, introduceTrackingBug(ctx, vestingAccount, app)) + migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) require.NoError(t, migrator.Migrate1to2(ctx)) trackingCorrected( From 520a94a8a0aacd7c9fea4fd4c8cd2fa8235af292 Mon Sep 17 00:00:00 2001 From: Jonathan Gimeno Date: Thu, 18 Mar 2021 11:18:54 +0100 Subject: [PATCH 10/37] next test iteration --- x/auth/keeper/migrations_test.go | 87 ++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index 08c468c4b778..b17d70bec926 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -23,45 +23,68 @@ import ( ) func TestMigrateVestingAccounts(t *testing.T) { - app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + testCases := []struct { + name string + prepareFunc func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) + expectedFunc func(app *simapp.SimApp, ctx sdk.Context, address sdk.AccAddress) + }{ + { + "not end time", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + func(app *simapp.SimApp, ctx sdk.Context, address sdk.AccAddress) { + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + trackingCorrected( + ctx, + t, + app.AccountKeeper, + address, + vestedCoins, + sdk.Coins{}, + ) + }, + }, + } - addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(310)) - delegatorAddr := addrs[0] + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + app := simapp.Setup(false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - _, valAddr := createValidator(t, ctx, app, 300) - validator, found := app.StakingKeeper.GetValidator(ctx, valAddr) - require.True(t, found) + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(310)) + delegatorAddr := addrs[0] - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) - vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, time.Now().Unix()) - app.AccountKeeper.SetAccount(ctx, delayedAccount) + _, valAddr := createValidator(t, ctx, app, 300) + validator, found := app.StakingKeeper.GetValidator(ctx, valAddr) + require.True(t, found) - _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) - require.NoError(t, err) - _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) - require.NoError(t, err) - _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) - require.NoError(t, err) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, time.Now().Unix()) + app.AccountKeeper.SetAccount(ctx, delayedAccount) - // We introduce the bug - savedAccount := app.AccountKeeper.GetAccount(ctx, delayedAccount.GetAddress()) - vestingAccount, ok := savedAccount.(exported.VestingAccount) - require.True(t, ok) - require.NoError(t, introduceTrackingBug(ctx, vestingAccount, app)) + tc.prepareFunc(app, ctx, validator, delegatorAddr) - migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) - require.NoError(t, migrator.Migrate1to2(ctx)) + // We introduce the bug + savedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) + vestingAccount, ok := savedAccount.(exported.VestingAccount) + require.True(t, ok) + require.NoError(t, introduceTrackingBug(ctx, vestingAccount, app)) + + migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) + require.NoError(t, migrator.Migrate1to2(ctx)) + + tc.expectedFunc(app, ctx, savedAccount.GetAddress()) + }) + } - trackingCorrected( - ctx, - t, - app.AccountKeeper, - baseAccount.GetAddress(), - vestedCoins, - sdk.Coins{}, - ) } func trackingCorrected(ctx sdk.Context, t *testing.T, ak authkeeper.AccountKeeper, addr sdk.AccAddress, expDelVesting sdk.Coins, expDelFree sdk.Coins) { From a9501d3958896c609b2ff1a68ec797f6d9d6d171 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Thu, 18 Mar 2021 12:27:30 +0100 Subject: [PATCH 11/37] account for the case where delegations are greater than the original vesting amount --- x/auth/keeper/migrations.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 865fdb41c6a9..794bcb334da2 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -54,7 +54,15 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { m.queryServer, ) - asVesting.TrackDelegation(ctx.BlockTime(), balance.Add(delegations...), delegations) + if delegations.IsAllGTE(asVesting.GetOriginalVesting()) { + delegations = asVesting.GetOriginalVesting() + } + + if balance.IsAllLT(delegations) { + balance = balance.Add(delegations...) + } + + asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations) m.keeper.SetAccount(ctx, account) From aaa7a9c15379196954341dd86d1a21a56f1b6256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Thu, 18 Mar 2021 12:27:50 +0100 Subject: [PATCH 12/37] delayed vesting test cases, first test case for continuous vesting --- x/auth/keeper/migrations_test.go | 209 +++++++++++++++++++++++++++---- 1 file changed, 188 insertions(+), 21 deletions(-) diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index b17d70bec926..519e702b9caf 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -24,13 +24,118 @@ import ( func TestMigrateVestingAccounts(t *testing.T) { testCases := []struct { - name string - prepareFunc func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) - expectedFunc func(app *simapp.SimApp, ctx sdk.Context, address sdk.AccAddress) + name string + prepareFunc func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) + expVested int64 + expFree int64 }{ + { + "delayed vesting has vested, multiple delegations less than the total account balance", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 0, + 200, + }, + { + "delayed vesting has vested, single delegations which exceed the vested amount", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 0, + 200, + }, + { + "delayed vesting has vested, multiple delegations which exceed the vested amount", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 0, + 200, + }, + { + "delayed vesting has not vested, single delegations which exceed the vested amount", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 200, + 0, + }, + { + "delayed vesting has not vested, multiple delegations which exceed the vested amount", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 200, + 0, + }, { "not end time", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) @@ -38,17 +143,61 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, - func(app *simapp.SimApp, ctx sdk.Context, address sdk.AccAddress) { + 300, + 0, + }, + { + "delayed vesting has not vested, single delegation greater than the total account balance", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 300, + 0, + }, + { + "delayed vesting has vested, single delegation greater than the total account balance", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) - trackingCorrected( - ctx, - t, - app.AccountKeeper, - address, - vestedCoins, - sdk.Coins{}, - ) + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) }, + 0, + 300, + }, + { + "continuous vesting has vested, single delegation greater than the total account balance", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + startTime := ctx.BlockTime().AddDate(-1, 0, 0).Unix() + endTime := ctx.BlockTime().AddDate(1, 0, 0).Unix() + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 150, + 150, }, } @@ -56,7 +205,9 @@ func TestMigrateVestingAccounts(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + ctx := app.BaseApp.NewContext(false, tmproto.Header{ + Time: time.Now(), + }) addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(310)) delegatorAddr := addrs[0] @@ -65,11 +216,6 @@ func TestMigrateVestingAccounts(t *testing.T) { validator, found := app.StakingKeeper.GetValidator(ctx, valAddr) require.True(t, found) - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) - vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) - delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, time.Now().Unix()) - app.AccountKeeper.SetAccount(ctx, delayedAccount) - tc.prepareFunc(app, ctx, validator, delegatorAddr) // We introduce the bug @@ -81,7 +227,25 @@ func TestMigrateVestingAccounts(t *testing.T) { migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) require.NoError(t, migrator.Migrate1to2(ctx)) - tc.expectedFunc(app, ctx, savedAccount.GetAddress()) + var expVested sdk.Coins + var expFree sdk.Coins + + if tc.expVested != 0 { + expVested = sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(tc.expVested))) + } + + if tc.expFree != 0 { + expFree = sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(tc.expFree))) + } + + trackingCorrected( + ctx, + t, + app.AccountKeeper, + savedAccount.GetAddress(), + expVested, + expFree, + ) }) } @@ -91,8 +255,11 @@ func trackingCorrected(ctx sdk.Context, t *testing.T, ak authkeeper.AccountKeepe baseAccount := ak.GetAccount(ctx, addr) vDA, ok := baseAccount.(exported.VestingAccount) require.True(t, ok) - require.True(t, expDelVesting.IsEqual(vDA.GetDelegatedVesting())) - require.True(t, expDelFree.IsEqual(vDA.GetDelegatedFree())) + + vestedOk := expDelVesting.IsEqual(vDA.GetDelegatedVesting()) + freeOk := expDelFree.IsEqual(vDA.GetDelegatedFree()) + require.True(t, vestedOk, vDA.GetDelegatedVesting().String()) + require.True(t, freeOk, vDA.GetDelegatedFree().String()) } func introduceTrackingBug(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error { From c3fdb8a68ad602db20dc72f1127948e9a30e3ca1 Mon Sep 17 00:00:00 2001 From: Jonathan Gimeno Date: Thu, 18 Mar 2021 16:08:21 +0100 Subject: [PATCH 13/37] add more tests --- x/auth/keeper/migrations_test.go | 48 +++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index 519e702b9caf..bfaa5de3c316 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -180,11 +180,31 @@ func TestMigrateVestingAccounts(t *testing.T) { 300, }, { - "continuous vesting has vested, single delegation greater than the total account balance", + "continuous vesting, start time after blocktime", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + startTime := ctx.BlockTime().AddDate(1, 0, 0).Unix() + endTime := ctx.BlockTime().AddDate(2, 0, 0).Unix() + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 300, + 0, + }, + { + "continuous vesting, start time passed but not ended", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { startTime := ctx.BlockTime().AddDate(-1, 0, 0).Unix() - endTime := ctx.BlockTime().AddDate(1, 0, 0).Unix() + endTime := ctx.BlockTime().AddDate(2, 0, 0).Unix() baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) @@ -196,8 +216,28 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, - 150, - 150, + 200, + 100, + }, + { + "continuous vesting, start time and endtime passed", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + startTime := ctx.BlockTime().AddDate(-2, 0, 0).Unix() + endTime := ctx.BlockTime().AddDate(-1, 0, 0).Unix() + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 0, + 300, }, } From 17e5c5a3d6524f024b0c805ddfd98183898d0fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Thu, 18 Mar 2021 19:41:47 +0100 Subject: [PATCH 14/37] prototype of periodic vesting account test --- x/auth/keeper/migrations_test.go | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index bfaa5de3c316..ebac35598484 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -239,6 +239,31 @@ func TestMigrateVestingAccounts(t *testing.T) { 0, 300, }, + { + "periodic vesting, start time and endtime passed", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + startTime := ctx.BlockTime().AddDate(-1, 0, 0).Unix() + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + + periods := []types.Period{ + { + Length: 1, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 0, + 300, + }, } for _, tc := range testCases { @@ -298,8 +323,8 @@ func trackingCorrected(ctx sdk.Context, t *testing.T, ak authkeeper.AccountKeepe vestedOk := expDelVesting.IsEqual(vDA.GetDelegatedVesting()) freeOk := expDelFree.IsEqual(vDA.GetDelegatedFree()) - require.True(t, vestedOk, vDA.GetDelegatedVesting().String()) - require.True(t, freeOk, vDA.GetDelegatedFree().String()) + require.True(t, vestedOk, fmt.Sprint("delegated vesting ", vDA.GetDelegatedVesting().String())) + require.True(t, freeOk, fmt.Sprint("delegated free ", vDA.GetDelegatedFree().String())) } func introduceTrackingBug(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error { From 61ea15a97d278fb795688d2692fa8048fd7e08e8 Mon Sep 17 00:00:00 2001 From: Gianguido Sora Date: Fri, 19 Mar 2021 22:44:13 +0100 Subject: [PATCH 15/37] better function naming Co-authored-by: Aleksandr Bezobchuk --- x/auth/keeper/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 794bcb334da2..128ff79e0dbf 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -81,7 +81,7 @@ func vesting(account types.AccountI) exported.VestingAccount { return v } -func getDelegatorDelegations(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { +func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)) From 6920bb97d275ecc1fa12b48c2a6813796961bb77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 19 Mar 2021 23:03:00 +0100 Subject: [PATCH 16/37] correct func name --- x/auth/keeper/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 128ff79e0dbf..1c5131909a31 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -48,7 +48,7 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { m.queryServer, ) - delegations := getDelegatorDelegations( + delegations := getDelegatorDelegationsSum( ctx, addr, m.queryServer, From e5339515946ca0f4abd8afb74d870afd33c8d53c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 19 Mar 2021 23:04:20 +0100 Subject: [PATCH 17/37] handle error --- x/auth/keeper/migrations.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 1c5131909a31..b96d9df17fca 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -106,7 +106,9 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp panic(err) } balance := new(stakingtypes.QueryDelegatorDelegationsResponse) - err = proto.Unmarshal(resp.Value, balance) + if err := proto.Unmarshal(resp.Value, balance); err != nil { + panic(err) + } res := sdk.NewCoins() for _, i := range balance.DelegationResponses { From 0f03346b301408a3478e4f09616ba35972065dd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 19 Mar 2021 23:04:47 +0100 Subject: [PATCH 18/37] added remaining test for periodic vesting account --- x/auth/keeper/migrations_test.go | 178 ++++++++++++++++++++++++++++++- 1 file changed, 175 insertions(+), 3 deletions(-) diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index ebac35598484..0ea4e9525d26 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -26,8 +26,10 @@ func TestMigrateVestingAccounts(t *testing.T) { testCases := []struct { name string prepareFunc func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) + tokenAmount int64 expVested int64 expFree int64 + blockTime int64 }{ { "delayed vesting has vested, multiple delegations less than the total account balance", @@ -48,8 +50,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 200, + 0, }, { "delayed vesting has vested, single delegations which exceed the vested amount", @@ -66,8 +70,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 200, + 0, }, { "delayed vesting has vested, multiple delegations which exceed the vested amount", @@ -88,8 +94,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 200, + 0, }, { "delayed vesting has not vested, single delegations which exceed the vested amount", @@ -104,8 +112,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 200, 0, + 0, }, { "delayed vesting has not vested, multiple delegations which exceed the vested amount", @@ -124,8 +134,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 200, 0, + 0, }, { "not end time", @@ -144,6 +156,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 300, + 300, + 0, 0, }, { @@ -159,6 +173,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 300, + 300, + 0, 0, }, { @@ -176,8 +192,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "continuous vesting, start time after blocktime", @@ -197,6 +215,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 300, + 300, + 0, 0, }, { @@ -216,8 +236,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 200, 100, + 0, }, { "continuous vesting, start time and endtime passed", @@ -236,8 +258,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "periodic vesting, start time and endtime passed", @@ -261,8 +285,152 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, + }, + { + "periodic vesting account, nothing has vested yet", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 3666666670000, + 0, + 0, + }, + { + "periodic vesting account, all has vested", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+15897600+15897600+1, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 0, + 3666666670000, + 1601042400 + 31536000 + 15897600 + 15897600 + 1, + }, + { + "periodic vesting account, first period has vested", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+1, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 3666666670000 - 1833333335000, + 1833333335000, + 1601042400 + 31536000 + 1, + }, + { + "periodic vesting account, first 2 period has vested", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+15638400+1, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 3666666670000 - 1833333335000 - 916666667500, + 1833333335000 + 916666667500, + 1601042400 + 31536000 + 15638400 + 1, }, } @@ -274,15 +442,19 @@ func TestMigrateVestingAccounts(t *testing.T) { Time: time.Now(), }) - addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(310)) + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(tc.tokenAmount+10)) delegatorAddr := addrs[0] - _, valAddr := createValidator(t, ctx, app, 300) + _, valAddr := createValidator(t, ctx, app, tc.tokenAmount*2) validator, found := app.StakingKeeper.GetValidator(ctx, valAddr) require.True(t, found) tc.prepareFunc(app, ctx, validator, delegatorAddr) + if tc.blockTime != 0 { + ctx = ctx.WithBlockTime(time.Unix(tc.blockTime, 0)) + } + // We introduce the bug savedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) vestingAccount, ok := savedAccount.(exported.VestingAccount) @@ -366,7 +538,7 @@ func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers i require.NoError(t, err) app.StakingKeeper.SetValidator(ctx, val1) - app.StakingKeeper.SetValidatorByConsAddr(ctx, val1) + require.NoError(t, app.StakingKeeper.SetValidatorByConsAddr(ctx, val1)) app.StakingKeeper.SetNewValidatorByPowerIndex(ctx, val1) _, _ = app.StakingKeeper.Delegate(ctx, addrs[0], sdk.TokensFromConsensusPower(powers), stakingtypes.Unbonded, val1, true) From 8d2cc6cc36d7ffc66d3f1d5efc1d65345ad4655e Mon Sep 17 00:00:00 2001 From: Adam Bozanich Date: Mon, 22 Mar 2021 13:50:12 +0800 Subject: [PATCH 19/37] x/auth/keeper: add test for periodic vesting --- x/auth/keeper/migrations.go | 2 +- x/auth/keeper/migrations_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 128ff79e0dbf..1c5131909a31 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -48,7 +48,7 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { m.queryServer, ) - delegations := getDelegatorDelegations( + delegations := getDelegatorDelegationsSum( ctx, addr, m.queryServer, diff --git a/x/auth/keeper/migrations_test.go b/x/auth/keeper/migrations_test.go index bfaa5de3c316..10b39934c066 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/keeper/migrations_test.go @@ -239,6 +239,32 @@ func TestMigrateVestingAccounts(t *testing.T) { 0, 300, }, + { + "periodic vesting account, yet to be vested, some rewards delegated", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100))) + + start := ctx.BlockTime().Unix() + int64(time.Hour/time.Second) + + periods := []types.Period{ + { + Length: int64((24 * time.Hour) / time.Second), + Amount: vestedCoins, + }, + } + + account := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, start, periods) + + app.AccountKeeper.SetAccount(ctx, account) + + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(150), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 100, + 50, + }, } for _, tc := range testCases { From 80335ad7e50fb99fe99e608bdd5898733d37999a Mon Sep 17 00:00:00 2001 From: Adam Bozanich Date: Mon, 22 Mar 2021 14:36:05 +0800 Subject: [PATCH 20/37] x/auth: add migrations module * during migration, reset corrupted values to zero --- x/auth/{keeper => migrations}/migrations.go | 47 +++++++++++++++---- .../{keeper => migrations}/migrations_test.go | 2 +- x/auth/module.go | 3 +- 3 files changed, 41 insertions(+), 11 deletions(-) rename x/auth/{keeper => migrations}/migrations.go (71%) rename x/auth/{keeper => migrations}/migrations_test.go (99%) diff --git a/x/auth/keeper/migrations.go b/x/auth/migrations/migrations.go similarity index 71% rename from x/auth/keeper/migrations.go rename to x/auth/migrations/migrations.go index 1c5131909a31..3ac1cc37c785 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/migrations/migrations.go @@ -1,11 +1,13 @@ -package keeper +package migrations import ( "fmt" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" + vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" "github.com/gogo/protobuf/grpc" "github.com/gogo/protobuf/proto" @@ -24,12 +26,12 @@ const ( // Migrator is a struct for handling in-place store migrations. type Migrator struct { - keeper AccountKeeper + keeper keeper.AccountKeeper queryServer grpc.Server } // NewMigrator returns a new Migrator. -func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator { +func NewMigrator(keeper keeper.AccountKeeper, queryServer grpc.Server) Migrator { return Migrator{keeper: keeper, queryServer: queryServer} } @@ -54,12 +56,9 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { m.queryServer, ) - if delegations.IsAllGTE(asVesting.GetOriginalVesting()) { - delegations = asVesting.GetOriginalVesting() - } - - if balance.IsAllLT(delegations) { - balance = balance.Add(delegations...) + asVesting, ok := resetVestingDelegatedBalances(asVesting) + if !ok { + return false } asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations) @@ -81,6 +80,36 @@ func vesting(account types.AccountI) exported.VestingAccount { return v } +func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.VestingAccount, bool) { + // reset `DelegatedVesting` and `DelegatedFree` to zero + df := sdk.NewCoins() + dv := sdk.NewCoins() + + for _, coin := range evacct.GetDelegatedFree() { + df = df.Add(sdk.NewCoin(coin.Denom, sdk.NewInt(0))) + } + for _, coin := range evacct.GetDelegatedVesting() { + dv = dv.Add(sdk.NewCoin(coin.Denom, sdk.NewInt(0))) + } + + switch vacct := evacct.(type) { + case *vestingtypes.ContinuousVestingAccount: + vacct.DelegatedVesting = dv + vacct.DelegatedFree = df + return vacct, true + case *vestingtypes.DelayedVestingAccount: + vacct.DelegatedVesting = dv + vacct.DelegatedFree = df + return vacct, true + case *vestingtypes.PeriodicVestingAccount: + vacct.DelegatedVesting = dv + vacct.DelegatedFree = df + return vacct, true + default: + return nil, false + } +} + func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { diff --git a/x/auth/keeper/migrations_test.go b/x/auth/migrations/migrations_test.go similarity index 99% rename from x/auth/keeper/migrations_test.go rename to x/auth/migrations/migrations_test.go index 10b39934c066..ee34c79a0324 100644 --- a/x/auth/keeper/migrations_test.go +++ b/x/auth/migrations/migrations_test.go @@ -1,4 +1,4 @@ -package keeper_test +package migrations_test import ( "fmt" diff --git a/x/auth/module.go b/x/auth/module.go index f0c26cd97e59..7fdb9d3cb361 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -21,6 +21,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/x/auth/client/rest" "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/x/auth/migrations" "github.com/cosmos/cosmos-sdk/x/auth/simulation" "github.com/cosmos/cosmos-sdk/x/auth/types" ) @@ -129,7 +130,7 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd // module-specific GRPC queries. func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterQueryServer(cfg.QueryServer(), am.accountKeeper) - m := keeper.NewMigrator(am.accountKeeper, cfg.QueryServer()) + m := migrations.NewMigrator(am.accountKeeper, cfg.QueryServer()) err := cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2) if err != nil { panic(err) From 9a1c74a0479bbe4c87db201dd1ddc6839d344fd6 Mon Sep 17 00:00:00 2001 From: Adam Bozanich Date: Mon, 22 Mar 2021 15:30:44 +0800 Subject: [PATCH 21/37] x/auth/migrations: fix lint --- x/auth/migrations/migrations.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/auth/migrations/migrations.go b/x/auth/migrations/migrations.go index 3ac1cc37c785..a53729762aed 100644 --- a/x/auth/migrations/migrations.go +++ b/x/auth/migrations/migrations.go @@ -135,7 +135,9 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp panic(err) } balance := new(stakingtypes.QueryDelegatorDelegationsResponse) - err = proto.Unmarshal(resp.Value, balance) + if err := proto.Unmarshal(resp.Value, balance); err != nil { + panic(fmt.Errorf("unable to unmarshal delegator query delegations: %w", err)) + } res := sdk.NewCoins() for _, i := range balance.DelegationResponses { @@ -170,6 +172,8 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Co panic(err) } balance := new(banktypes.QueryAllBalancesResponse) - err = proto.Unmarshal(resp.Value, balance) + if err := proto.Unmarshal(resp.Value, balance); err != nil { + panic(fmt.Errorf("unable to unmarshal bank balance response: %w", err)) + } return balance.Balances } From 93b2ec960a68ebcd1d97bd1bc135b2b0bb675d9f Mon Sep 17 00:00:00 2001 From: Adam Bozanich Date: Mon, 22 Mar 2021 17:31:17 +0800 Subject: [PATCH 22/37] x/auth/migrations: fixes --- x/auth/migrations/migrations.go | 12 +++++------- x/auth/migrations/migrations_test.go | 3 ++- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/x/auth/migrations/migrations.go b/x/auth/migrations/migrations.go index a53729762aed..a8a6b9b4d8f5 100644 --- a/x/auth/migrations/migrations.go +++ b/x/auth/migrations/migrations.go @@ -61,6 +61,11 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { return false } + // balance before any delegation includes balance of delegation + for _, coin := range delegations { + balance = balance.Add(coin) + } + asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations) m.keeper.SetAccount(ctx, account) @@ -85,13 +90,6 @@ func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.Ves df := sdk.NewCoins() dv := sdk.NewCoins() - for _, coin := range evacct.GetDelegatedFree() { - df = df.Add(sdk.NewCoin(coin.Denom, sdk.NewInt(0))) - } - for _, coin := range evacct.GetDelegatedVesting() { - dv = dv.Add(sdk.NewCoin(coin.Denom, sdk.NewInt(0))) - } - switch vacct := evacct.(type) { case *vestingtypes.ContinuousVestingAccount: vacct.DelegatedVesting = dv diff --git a/x/auth/migrations/migrations_test.go b/x/auth/migrations/migrations_test.go index ee34c79a0324..72f27ff7b032 100644 --- a/x/auth/migrations/migrations_test.go +++ b/x/auth/migrations/migrations_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/cosmos/cosmos-sdk/x/auth/migrations" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/cosmos/cosmos-sdk/x/staking" @@ -290,7 +291,7 @@ func TestMigrateVestingAccounts(t *testing.T) { require.True(t, ok) require.NoError(t, introduceTrackingBug(ctx, vestingAccount, app)) - migrator := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) + migrator := migrations.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) require.NoError(t, migrator.Migrate1to2(ctx)) var expVested sdk.Coins From c10b189b0638a34cf12e27eaf3572a9ee9da98f8 Mon Sep 17 00:00:00 2001 From: Adam Bozanich Date: Mon, 22 Mar 2021 19:27:14 +0800 Subject: [PATCH 23/37] x/auth/migrations: fix tests --- x/auth/migrations/migrations_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/x/auth/migrations/migrations_test.go b/x/auth/migrations/migrations_test.go index 72f27ff7b032..620b834b7729 100644 --- a/x/auth/migrations/migrations_test.go +++ b/x/auth/migrations/migrations_test.go @@ -50,7 +50,7 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 0, - 200, + 300, }, { "delayed vesting has vested, single delegations which exceed the vested amount", @@ -68,7 +68,7 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 0, - 200, + 300, }, { "delayed vesting has vested, multiple delegations which exceed the vested amount", @@ -90,7 +90,7 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 0, - 200, + 300, }, { "delayed vesting has not vested, single delegations which exceed the vested amount", @@ -106,7 +106,7 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 200, - 0, + 100, }, { "delayed vesting has not vested, multiple delegations which exceed the vested amount", @@ -126,7 +126,7 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 200, - 0, + 100, }, { "not end time", @@ -319,6 +319,7 @@ func TestMigrateVestingAccounts(t *testing.T) { } func trackingCorrected(ctx sdk.Context, t *testing.T, ak authkeeper.AccountKeeper, addr sdk.AccAddress, expDelVesting sdk.Coins, expDelFree sdk.Coins) { + t.Helper() baseAccount := ak.GetAccount(ctx, addr) vDA, ok := baseAccount.(exported.VestingAccount) require.True(t, ok) From d99c22e677267c0b27d131cbf1467b3aa288d49d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Tue, 23 Mar 2021 11:54:10 +0100 Subject: [PATCH 24/37] meaningful comment for Migrate1to2 function --- x/auth/migrations/migrations.go | 48 ++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/x/auth/migrations/migrations.go b/x/auth/migrations/migrations.go index a8a6b9b4d8f5..8f84311e554c 100644 --- a/x/auth/migrations/migrations.go +++ b/x/auth/migrations/migrations.go @@ -35,8 +35,12 @@ func NewMigrator(keeper keeper.AccountKeeper, queryServer grpc.Server) Migrator return Migrator{keeper: keeper, queryServer: queryServer} } -// Migrate1to2 migrates from version 1 to 2. +// Migrate1to2 migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly +// track delegations. +// References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 func (m Migrator) Migrate1to2(ctx sdk.Context) error { + var iterationErr error + m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { asVesting := vesting(account) if asVesting == nil { @@ -44,18 +48,28 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { } addr := account.GetAddress().String() - balance := getBalance( + balance, err := getBalance( ctx, addr, m.queryServer, ) - delegations := getDelegatorDelegationsSum( + if err != nil { + iterationErr = err + return true + } + + delegations, err := getDelegatorDelegationsSum( ctx, addr, m.queryServer, ) + if err != nil { + iterationErr = err + return true + } + asVesting, ok := resetVestingDelegatedBalances(asVesting) if !ok { return false @@ -73,7 +87,7 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { return false }) - return nil + return iterationErr } func vesting(account types.AccountI) exported.VestingAccount { @@ -108,10 +122,10 @@ func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.Ves } } -func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { +func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { - panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)) + return nil, fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer) } queryFn := querier.Route(delegatorDelegationPath) @@ -122,7 +136,7 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp b, err := proto.Marshal(q) if err != nil { - panic(err) + return nil, fmt.Errorf("cannot marshal staking type query request, %w", err) } req := abci.RequestQuery{ Data: b, @@ -130,11 +144,12 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp } resp, err := queryFn(ctx, req) if err != nil { - panic(err) + return nil, fmt.Errorf("staking query error, %w", err) } + balance := new(stakingtypes.QueryDelegatorDelegationsResponse) if err := proto.Unmarshal(resp.Value, balance); err != nil { - panic(fmt.Errorf("unable to unmarshal delegator query delegations: %w", err)) + return nil, fmt.Errorf("unable to unmarshal delegator query delegations: %w", err) } res := sdk.NewCoins() @@ -142,13 +157,13 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp res = res.Add(i.Balance) } - return res + return res, nil } -func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Coins { +func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { - panic(fmt.Sprintf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)) + return nil, fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer) } queryFn := querier.Route(balancesPath) @@ -159,19 +174,20 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) sdk.Co } b, err := proto.Marshal(q) if err != nil { - panic(err) + return nil, fmt.Errorf("cannot marshal bank type query request, %w", err) } + req := abci.RequestQuery{ Data: b, Path: balancesPath, } resp, err := queryFn(ctx, req) if err != nil { - panic(err) + return nil, fmt.Errorf("bank query error, %w", err) } balance := new(banktypes.QueryAllBalancesResponse) if err := proto.Unmarshal(resp.Value, balance); err != nil { - panic(fmt.Errorf("unable to unmarshal bank balance response: %w", err)) + return nil, fmt.Errorf("unable to unmarshal bank balance response: %w", err) } - return balance.Balances + return balance.Balances, nil } From 327277db677c37d703650a9ea7b7062db60502e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Tue, 23 Mar 2021 12:27:33 +0100 Subject: [PATCH 25/37] re-introduced periodic vesting tests derped a lil with git, this commit was done late in the day after a long day of work :-) added comments on intricate test cases to make sure everybody who reads 'em doesn't go crazy --- x/auth/migrations/migrations_test.go | 214 ++++++++++++++++++++++++++- 1 file changed, 211 insertions(+), 3 deletions(-) diff --git a/x/auth/migrations/migrations_test.go b/x/auth/migrations/migrations_test.go index 620b834b7729..f05838ace757 100644 --- a/x/auth/migrations/migrations_test.go +++ b/x/auth/migrations/migrations_test.go @@ -27,8 +27,10 @@ func TestMigrateVestingAccounts(t *testing.T) { testCases := []struct { name string prepareFunc func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) + tokenAmount int64 expVested int64 expFree int64 + blockTime int64 }{ { "delayed vesting has vested, multiple delegations less than the total account balance", @@ -49,8 +51,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "delayed vesting has vested, single delegations which exceed the vested amount", @@ -67,8 +71,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "delayed vesting has vested, multiple delegations which exceed the vested amount", @@ -89,8 +95,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "delayed vesting has not vested, single delegations which exceed the vested amount", @@ -105,8 +113,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 200, 100, + 0, }, { "delayed vesting has not vested, multiple delegations which exceed the vested amount", @@ -125,8 +135,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 200, 100, + 0, }, { "not end time", @@ -145,6 +157,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 300, + 300, + 0, 0, }, { @@ -160,6 +174,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 300, + 300, + 0, 0, }, { @@ -177,8 +193,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "continuous vesting, start time after blocktime", @@ -198,6 +216,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.NoError(t, err) }, 300, + 300, + 0, 0, }, { @@ -217,8 +237,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 200, 100, + 0, }, { "continuous vesting, start time and endtime passed", @@ -237,8 +259,10 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 0, 300, + 0, }, { "periodic vesting account, yet to be vested, some rewards delegated", @@ -263,8 +287,188 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(150), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + 300, 100, 50, + 0, + }, + { + "periodic vesting account, nothing has vested yet", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + /* + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - nothing has vested, we put the block time slightly after start time + - expected vested: original vesting amount + - expected free: zero + - we're delegating the full original vesting + */ + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 3666666670000, + 0, + 1601042400 + 1, + }, + { + "periodic vesting account, all has vested", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + /* + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - all has vested, so we set the block time at initial time + sum of all periods times + 1 => 1601042400 + 31536000 + 15897600 + 15897600 + 1 + - expected vested: zero + - expected free: original vesting amount + - we're delegating the full original vesting + */ + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+15897600+15897600+1, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 0, + 3666666670000, + 1601042400 + 31536000 + 15897600 + 15897600 + 1, + }, + { + "periodic vesting account, first period has vested", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + /* + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - first period have vested, so we set the block time at initial time + time of the first periods + 1 => 1601042400 + 31536000 + 1 + - expected vested: original vesting - first period amount + - expected free: first period amount + - we're delegating the full original vesting + */ + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+1, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 3666666670000 - 1833333335000, + 1833333335000, + 1601042400 + 31536000 + 1, + }, + { + "periodic vesting account, first 2 period has vested", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + /* + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - first 2 periods have vested, so we set the block time at initial time + time of the two periods + 1 => 1601042400 + 31536000 + 15638400 + 1 + - expected vested: original vesting - (sum of the first two periods amounts) + - expected free: sum of the first two periods + - we're delegating the full original vesting + */ + startTime := int64(1601042400) + baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) + periods := []types.Period{ + { + Length: 31536000, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1833333335000))), + }, + { + Length: 15638400, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + { + Length: 15897600, + Amount: sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(916666667500))), + }, + } + + delayedAccount := types.NewPeriodicVestingAccount(baseAccount, vestedCoins, startTime, periods) + + ctx = ctx.WithBlockTime(time.Unix(1601042400+31536000+15638400+1, 0)) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + }, + 3666666670000, + 3666666670000 - 1833333335000 - 916666667500, + 1833333335000 + 916666667500, + 1601042400 + 31536000 + 15638400 + 1, }, } @@ -276,15 +480,19 @@ func TestMigrateVestingAccounts(t *testing.T) { Time: time.Now(), }) - addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(310)) + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(tc.tokenAmount+10)) delegatorAddr := addrs[0] - _, valAddr := createValidator(t, ctx, app, 300) + _, valAddr := createValidator(t, ctx, app, tc.tokenAmount*2) validator, found := app.StakingKeeper.GetValidator(ctx, valAddr) require.True(t, found) tc.prepareFunc(app, ctx, validator, delegatorAddr) + if tc.blockTime != 0 { + ctx = ctx.WithBlockTime(time.Unix(tc.blockTime, 0)) + } + // We introduce the bug savedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) vestingAccount, ok := savedAccount.(exported.VestingAccount) @@ -369,7 +577,7 @@ func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers i require.NoError(t, err) app.StakingKeeper.SetValidator(ctx, val1) - app.StakingKeeper.SetValidatorByConsAddr(ctx, val1) + require.NoError(t, app.StakingKeeper.SetValidatorByConsAddr(ctx, val1)) app.StakingKeeper.SetNewValidatorByPowerIndex(ctx, val1) _, _ = app.StakingKeeper.Delegate(ctx, addrs[0], sdk.TokensFromConsensusPower(powers), stakingtypes.Unbonded, val1, true) From e8f906b3bcb7195a6f71b8daec2438225024e39c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Tue, 23 Mar 2021 13:57:14 +0100 Subject: [PATCH 26/37] refactor auth migration to use the same directory tree/call style as other in-store migrations --- x/auth/keeper/migrations.go | 33 +++++++++ .../migrations.go => legacy/v043/store.go} | 69 ++++++++----------- .../v043/store_test.go} | 13 ++-- x/auth/module.go | 3 +- 4 files changed, 70 insertions(+), 48 deletions(-) create mode 100644 x/auth/keeper/migrations.go rename x/auth/{migrations/migrations.go => legacy/v043/store.go} (81%) rename x/auth/{migrations/migrations_test.go => legacy/v043/store_test.go} (98%) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go new file mode 100644 index 000000000000..60f2c9323cd5 --- /dev/null +++ b/x/auth/keeper/migrations.go @@ -0,0 +1,33 @@ +package keeper + +import ( + v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043" + "github.com/gogo/protobuf/grpc" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// Migrator is a struct for handling in-place store migrations. +type Migrator struct { + keeper AccountKeeper + queryServer grpc.Server +} + +// NewMigrator returns a new Migrator. +func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator { + return Migrator{keeper: keeper, queryServer: queryServer} +} + +// Migrate1to2 migrates from version 1 to 2. +func (m Migrator) Migrate1to2(ctx sdk.Context) error { + wb, err := v043.MigrateStore(ctx, m.keeper.GetAllAccounts(ctx), m.queryServer) + if err != nil { + return err + } + + for _, a := range wb { + m.keeper.SetAccount(ctx, a) + } + + return nil +} diff --git a/x/auth/migrations/migrations.go b/x/auth/legacy/v043/store.go similarity index 81% rename from x/auth/migrations/migrations.go rename to x/auth/legacy/v043/store.go index 8f84311e554c..55da62c081d2 100644 --- a/x/auth/migrations/migrations.go +++ b/x/auth/legacy/v043/store.go @@ -1,22 +1,18 @@ -package migrations +package v043 import ( "fmt" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - - "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/baseapp" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" - + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/gogo/protobuf/grpc" "github.com/gogo/protobuf/proto" abci "github.com/tendermint/tendermint/abci/types" - - "github.com/cosmos/cosmos-sdk/baseapp" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth/types" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) const ( @@ -24,55 +20,39 @@ const ( balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances" ) -// Migrator is a struct for handling in-place store migrations. -type Migrator struct { - keeper keeper.AccountKeeper - queryServer grpc.Server -} - -// NewMigrator returns a new Migrator. -func NewMigrator(keeper keeper.AccountKeeper, queryServer grpc.Server) Migrator { - return Migrator{keeper: keeper, queryServer: queryServer} -} - -// Migrate1to2 migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly -// track delegations. -// References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 -func (m Migrator) Migrate1to2(ctx sdk.Context) error { - var iterationErr error - - m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { - asVesting := vesting(account) +func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, queryServer grpc.Server) ([]types.AccountI, error) { + for i := 0; i < len(accounts); i++ { + asVesting := vesting(accounts[i]) if asVesting == nil { - return false + continue } + account := accounts[i] + addr := account.GetAddress().String() balance, err := getBalance( ctx, addr, - m.queryServer, + queryServer, ) if err != nil { - iterationErr = err - return true + return nil, err } delegations, err := getDelegatorDelegationsSum( ctx, addr, - m.queryServer, + queryServer, ) if err != nil { - iterationErr = err - return true + return nil, err } asVesting, ok := resetVestingDelegatedBalances(asVesting) if !ok { - return false + continue } // balance before any delegation includes balance of delegation @@ -82,12 +62,10 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations) - m.keeper.SetAccount(ctx, account) - - return false - }) + accounts[i] = asVesting.(types.AccountI) + } - return iterationErr + return accounts, nil } func vesting(account types.AccountI) exported.VestingAccount { @@ -191,3 +169,10 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.C } return balance.Balances, nil } + +// MigrateStore migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly +// track delegations. +// References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 +func MigrateStore(ctx sdk.Context, accounts []types.AccountI, queryServer grpc.Server) ([]types.AccountI, error) { + return migrateVestingAccounts(ctx, accounts, queryServer) +} diff --git a/x/auth/migrations/migrations_test.go b/x/auth/legacy/v043/store_test.go similarity index 98% rename from x/auth/migrations/migrations_test.go rename to x/auth/legacy/v043/store_test.go index f05838ace757..f432c16a58b4 100644 --- a/x/auth/migrations/migrations_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -1,11 +1,12 @@ -package migrations_test +package v043_test import ( "fmt" "testing" "time" - "github.com/cosmos/cosmos-sdk/x/auth/migrations" + v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043" + "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/cosmos/cosmos-sdk/x/staking" @@ -499,8 +500,12 @@ func TestMigrateVestingAccounts(t *testing.T) { require.True(t, ok) require.NoError(t, introduceTrackingBug(ctx, vestingAccount, app)) - migrator := migrations.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) - require.NoError(t, migrator.Migrate1to2(ctx)) + newData, err := v043.MigrateStore(ctx, app.AccountKeeper.GetAllAccounts(ctx), app.GRPCQueryRouter()) + require.NoError(t, err) + + for _, a := range newData { + app.AccountKeeper.SetAccount(ctx, a) + } var expVested sdk.Coins var expFree sdk.Coins diff --git a/x/auth/module.go b/x/auth/module.go index 7fdb9d3cb361..f0c26cd97e59 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -21,7 +21,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/x/auth/client/rest" "github.com/cosmos/cosmos-sdk/x/auth/keeper" - "github.com/cosmos/cosmos-sdk/x/auth/migrations" "github.com/cosmos/cosmos-sdk/x/auth/simulation" "github.com/cosmos/cosmos-sdk/x/auth/types" ) @@ -130,7 +129,7 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd // module-specific GRPC queries. func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterQueryServer(cfg.QueryServer(), am.accountKeeper) - m := migrations.NewMigrator(am.accountKeeper, cfg.QueryServer()) + m := keeper.NewMigrator(am.accountKeeper, cfg.QueryServer()) err := cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2) if err != nil { panic(err) From b40333f636dc5991514f0166af2a2851d94d1053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Mar 2021 10:35:08 +0100 Subject: [PATCH 27/37] check test Delegate() error --- x/auth/legacy/v043/store_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go index f432c16a58b4..4294a96be6ca 100644 --- a/x/auth/legacy/v043/store_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -585,7 +585,8 @@ func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers i require.NoError(t, app.StakingKeeper.SetValidatorByConsAddr(ctx, val1)) app.StakingKeeper.SetNewValidatorByPowerIndex(ctx, val1) - _, _ = app.StakingKeeper.Delegate(ctx, addrs[0], sdk.TokensFromConsensusPower(powers), stakingtypes.Unbonded, val1, true) + _, err = app.StakingKeeper.Delegate(ctx, addrs[0], sdk.TokensFromConsensusPower(powers), stakingtypes.Unbonded, val1, true) + require.NoError(t, err) _ = staking.EndBlocker(ctx, app.StakingKeeper) From 3c6489deac56ebf789292a9dbc234ecca0b94d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Mar 2021 11:08:25 +0100 Subject: [PATCH 28/37] init test validator properly --- x/auth/legacy/v043/store_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go index 4294a96be6ca..f6e3ca84e32d 100644 --- a/x/auth/legacy/v043/store_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -565,7 +565,8 @@ func introduceTrackingBug(ctx sdk.Context, vesting exported.VestingAccount, app } func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers int64) (sdk.AccAddress, sdk.ValAddress) { - addrs := simapp.AddTestAddrsIncremental(app, ctx, 1, sdk.NewInt(30000000)) + valTokens := sdk.TokensFromConsensusPower(powers) + addrs := simapp.AddTestAddrsIncremental(app, ctx, 1, valTokens) valAddrs := simapp.ConvertAddrsToValAddrs(addrs) pks := simapp.CreateTestPubKeys(1) cdc := simapp.MakeTestEncodingConfig().Marshaler @@ -585,7 +586,7 @@ func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers i require.NoError(t, app.StakingKeeper.SetValidatorByConsAddr(ctx, val1)) app.StakingKeeper.SetNewValidatorByPowerIndex(ctx, val1) - _, err = app.StakingKeeper.Delegate(ctx, addrs[0], sdk.TokensFromConsensusPower(powers), stakingtypes.Unbonded, val1, true) + _, err = app.StakingKeeper.Delegate(ctx, addrs[0], valTokens, stakingtypes.Unbonded, val1, true) require.NoError(t, err) _ = staking.EndBlocker(ctx, app.StakingKeeper) From 60c41cc47d8147da256abacb65b65b410ed90262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Mar 2021 11:11:26 +0100 Subject: [PATCH 29/37] remove vesting() function since it's just a type assertion, to it directly --- x/auth/legacy/v043/store.go | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/x/auth/legacy/v043/store.go b/x/auth/legacy/v043/store.go index 55da62c081d2..b1b78017dc11 100644 --- a/x/auth/legacy/v043/store.go +++ b/x/auth/legacy/v043/store.go @@ -22,8 +22,8 @@ const ( func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, queryServer grpc.Server) ([]types.AccountI, error) { for i := 0; i < len(accounts); i++ { - asVesting := vesting(accounts[i]) - if asVesting == nil { + asVesting, ok := accounts[i].(exported.VestingAccount) + if !ok { continue } @@ -50,7 +50,7 @@ func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, querySer return nil, err } - asVesting, ok := resetVestingDelegatedBalances(asVesting) + asVesting, ok = resetVestingDelegatedBalances(asVesting) if !ok { continue } @@ -68,15 +68,6 @@ func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, querySer return accounts, nil } -func vesting(account types.AccountI) exported.VestingAccount { - v, ok := account.(exported.VestingAccount) - if !ok { - return nil - } - - return v -} - func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.VestingAccount, bool) { // reset `DelegatedVesting` and `DelegatedFree` to zero df := sdk.NewCoins() From d9f55cd25c1edd9758ecbc828b075443fcf3f29c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Mar 2021 11:20:02 +0100 Subject: [PATCH 30/37] import sorting renamed auth types import to a more recognizable name --- x/auth/legacy/v043/store_test.go | 100 +++++++++++++++---------------- 1 file changed, 48 insertions(+), 52 deletions(-) diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go index f6e3ca84e32d..cdff0584322d 100644 --- a/x/auth/legacy/v043/store_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -5,21 +5,17 @@ import ( "testing" "time" - v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043" - - "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" - - "github.com/cosmos/cosmos-sdk/x/staking" - "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" - types3 "github.com/cosmos/cosmos-sdk/x/auth/types" - + v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" + "github.com/cosmos/cosmos-sdk/x/staking" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) @@ -37,7 +33,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "delayed vesting has vested, multiple delegations less than the total account balance", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) @@ -61,7 +57,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "delayed vesting has vested, single delegations which exceed the vested amount", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) @@ -81,7 +77,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "delayed vesting has vested, multiple delegations which exceed the vested amount", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) @@ -105,7 +101,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "delayed vesting has not vested, single delegations which exceed the vested amount", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) @@ -123,7 +119,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "delayed vesting has not vested, multiple delegations which exceed the vested amount", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(200))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) @@ -144,7 +140,7 @@ func TestMigrateVestingAccounts(t *testing.T) { { "not end time", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) @@ -165,7 +161,7 @@ func TestMigrateVestingAccounts(t *testing.T) { { "delayed vesting has not vested, single delegation greater than the total account balance", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(1, 0, 0).Unix()) @@ -183,7 +179,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "delayed vesting has vested, single delegation greater than the total account balance", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().Unix()) @@ -205,7 +201,7 @@ func TestMigrateVestingAccounts(t *testing.T) { startTime := ctx.BlockTime().AddDate(1, 0, 0).Unix() endTime := ctx.BlockTime().AddDate(2, 0, 0).Unix() - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) @@ -227,7 +223,7 @@ func TestMigrateVestingAccounts(t *testing.T) { startTime := ctx.BlockTime().AddDate(-1, 0, 0).Unix() endTime := ctx.BlockTime().AddDate(2, 0, 0).Unix() - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) @@ -249,7 +245,7 @@ func TestMigrateVestingAccounts(t *testing.T) { startTime := ctx.BlockTime().AddDate(-2, 0, 0).Unix() endTime := ctx.BlockTime().AddDate(-1, 0, 0).Unix() - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) delayedAccount := types.NewContinuousVestingAccount(baseAccount, vestedCoins, startTime, endTime) @@ -269,7 +265,7 @@ func TestMigrateVestingAccounts(t *testing.T) { "periodic vesting account, yet to be vested, some rewards delegated", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100))) start := ctx.BlockTime().Unix() + int64(time.Hour/time.Second) @@ -297,16 +293,16 @@ func TestMigrateVestingAccounts(t *testing.T) { "periodic vesting account, nothing has vested yet", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { /* - Test case: - - periodic vesting account starts at time 1601042400 - - account balance and original vesting: 3666666670000 - - nothing has vested, we put the block time slightly after start time - - expected vested: original vesting amount - - expected free: zero - - we're delegating the full original vesting + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - nothing has vested, we put the block time slightly after start time + - expected vested: original vesting amount + - expected free: zero + - we're delegating the full original vesting */ startTime := int64(1601042400) - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) periods := []types.Period{ { @@ -340,16 +336,16 @@ func TestMigrateVestingAccounts(t *testing.T) { "periodic vesting account, all has vested", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { /* - Test case: - - periodic vesting account starts at time 1601042400 - - account balance and original vesting: 3666666670000 - - all has vested, so we set the block time at initial time + sum of all periods times + 1 => 1601042400 + 31536000 + 15897600 + 15897600 + 1 - - expected vested: zero - - expected free: original vesting amount - - we're delegating the full original vesting + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - all has vested, so we set the block time at initial time + sum of all periods times + 1 => 1601042400 + 31536000 + 15897600 + 15897600 + 1 + - expected vested: zero + - expected free: original vesting amount + - we're delegating the full original vesting */ startTime := int64(1601042400) - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) periods := []types.Period{ { @@ -385,16 +381,16 @@ func TestMigrateVestingAccounts(t *testing.T) { "periodic vesting account, first period has vested", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { /* - Test case: - - periodic vesting account starts at time 1601042400 - - account balance and original vesting: 3666666670000 - - first period have vested, so we set the block time at initial time + time of the first periods + 1 => 1601042400 + 31536000 + 1 - - expected vested: original vesting - first period amount - - expected free: first period amount - - we're delegating the full original vesting + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - first period have vested, so we set the block time at initial time + time of the first periods + 1 => 1601042400 + 31536000 + 1 + - expected vested: original vesting - first period amount + - expected free: first period amount + - we're delegating the full original vesting */ startTime := int64(1601042400) - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) periods := []types.Period{ { @@ -430,16 +426,16 @@ func TestMigrateVestingAccounts(t *testing.T) { "periodic vesting account, first 2 period has vested", func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { /* - Test case: - - periodic vesting account starts at time 1601042400 - - account balance and original vesting: 3666666670000 - - first 2 periods have vested, so we set the block time at initial time + time of the two periods + 1 => 1601042400 + 31536000 + 15638400 + 1 - - expected vested: original vesting - (sum of the first two periods amounts) - - expected free: sum of the first two periods - - we're delegating the full original vesting + Test case: + - periodic vesting account starts at time 1601042400 + - account balance and original vesting: 3666666670000 + - first 2 periods have vested, so we set the block time at initial time + time of the two periods + 1 => 1601042400 + 31536000 + 15638400 + 1 + - expected vested: original vesting - (sum of the first two periods amounts) + - expected free: sum of the first two periods + - we're delegating the full original vesting */ startTime := int64(1601042400) - baseAccount := types3.NewBaseAccountWithAddress(delegatorAddr) + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(3666666670000))) periods := []types.Period{ { From fef2d0075b801f3e448e5ae175b3ab2e70f469c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Mar 2021 15:02:12 +0100 Subject: [PATCH 31/37] properly support unbonding delegations --- x/auth/legacy/v043/store.go | 107 ++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 5 deletions(-) diff --git a/x/auth/legacy/v043/store.go b/x/auth/legacy/v043/store.go index b1b78017dc11..d9c4bea2a569 100644 --- a/x/auth/legacy/v043/store.go +++ b/x/auth/legacy/v043/store.go @@ -1,10 +1,12 @@ package v043 import ( + "errors" "fmt" "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" @@ -13,14 +15,24 @@ import ( "github.com/gogo/protobuf/grpc" "github.com/gogo/protobuf/proto" abci "github.com/tendermint/tendermint/abci/types" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) const ( - delegatorDelegationPath = "/cosmos.staking.v1beta1.Query/DelegatorDelegations" - balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances" + delegatorDelegationPath = "/cosmos.staking.v1beta1.Query/DelegatorDelegations" + stakingParamsPath = "/cosmos.staking.v1beta1.Query/Params" + delegatorUnbondingDelegationsPath = "/cosmos.staking.v1beta1.Query/DelegatorUnbondingDelegations" + balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances" ) func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, queryServer grpc.Server) ([]types.AccountI, error) { + bondDenom, err := getBondDenom(ctx, queryServer) + + if err != nil { + return nil, err + } + for i := 0; i < len(accounts); i++ { asVesting, ok := accounts[i].(exported.VestingAccount) if !ok { @@ -46,9 +58,14 @@ func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, querySer queryServer, ) - if err != nil { - return nil, err - } + unbondingDelegations, err := getDelegatorUnbondingDelegationsSum( + ctx, + addr, + bondDenom, + queryServer, + ) + + delegations = delegations.Add(unbondingDelegations...) asVesting, ok = resetVestingDelegatedBalances(asVesting) if !ok { @@ -113,6 +130,10 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp } resp, err := queryFn(ctx, req) if err != nil { + e, ok := status.FromError(err) + if ok && e.Code() == codes.NotFound { + return nil, nil + } return nil, fmt.Errorf("staking query error, %w", err) } @@ -129,6 +150,50 @@ func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grp return res, nil } +func getDelegatorUnbondingDelegationsSum(ctx sdk.Context, address, bondDenom string, queryServer grpc.Server) (sdk.Coins, error) { + querier, ok := queryServer.(*baseapp.GRPCQueryRouter) + if !ok { + return nil, fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer) + } + + queryFn := querier.Route(delegatorUnbondingDelegationsPath) + + q := &stakingtypes.QueryDelegatorUnbondingDelegationsRequest{ + DelegatorAddr: address, + } + + b, err := proto.Marshal(q) + if err != nil { + return nil, fmt.Errorf("cannot marshal staking type query request, %w", err) + } + req := abci.RequestQuery{ + Data: b, + Path: delegatorUnbondingDelegationsPath, + } + resp, err := queryFn(ctx, req) + if err != nil && !errors.Is(err, sdkerrors.ErrNotFound) { + e, ok := status.FromError(err) + if ok && e.Code() == codes.NotFound { + return nil, nil + } + return nil, fmt.Errorf("staking query error, %w", err) + } + + balance := new(stakingtypes.QueryDelegatorUnbondingDelegationsResponse) + if err := proto.Unmarshal(resp.Value, balance); err != nil { + return nil, fmt.Errorf("unable to unmarshal delegator query delegations: %w", err) + } + + res := sdk.NewCoins() + for _, i := range balance.UnbondingResponses { + for _, r := range i.Entries { + res = res.Add(sdk.NewCoin(bondDenom, r.Balance)) + } + } + + return res, nil +} + func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) { querier, ok := queryServer.(*baseapp.GRPCQueryRouter) if !ok { @@ -161,6 +226,38 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.C return balance.Balances, nil } +func getBondDenom(ctx sdk.Context, queryServer grpc.Server) (string, error) { + querier, ok := queryServer.(*baseapp.GRPCQueryRouter) + if !ok { + return "", fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer) + } + + queryFn := querier.Route(stakingParamsPath) + + q := &stakingtypes.QueryParamsRequest{} + + b, err := proto.Marshal(q) + if err != nil { + return "", fmt.Errorf("cannot marshal staking params query request, %w", err) + } + req := abci.RequestQuery{ + Data: b, + Path: stakingParamsPath, + } + + resp, err := queryFn(ctx, req) + if err != nil { + return "", fmt.Errorf("staking query error, %w", err) + } + + params := new(stakingtypes.QueryParamsResponse) + if err := proto.Unmarshal(resp.Value, params); err != nil { + return "", fmt.Errorf("unable to unmarshal delegator query delegations: %w", err) + } + + return params.Params.BondDenom, nil +} + // MigrateStore migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly // track delegations. // References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 From bfc1860a6d1df295f29cc15e2df41d458e101743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Mar 2021 15:10:07 +0100 Subject: [PATCH 32/37] more tests! - verify that an account that has unbonding delegations are accounted for - vesting account that have no delegations --- x/auth/legacy/v043/store_test.go | 107 ++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 3 deletions(-) diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go index cdff0584322d..04277264a2a2 100644 --- a/x/auth/legacy/v043/store_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -24,6 +24,7 @@ func TestMigrateVestingAccounts(t *testing.T) { testCases := []struct { name string prepareFunc func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) + garbageFunc func(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error tokenAmount int64 expVested int64 expFree int64 @@ -48,6 +49,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 0, 300, @@ -68,6 +70,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 0, 300, @@ -92,6 +95,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 0, 300, @@ -110,6 +114,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 200, 100, @@ -132,6 +137,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 200, 100, @@ -153,6 +159,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 300, 0, @@ -170,6 +177,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 300, 0, @@ -190,6 +198,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 0, 300, @@ -212,6 +221,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 300, 0, @@ -234,6 +244,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 200, 100, @@ -256,6 +267,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 0, 300, @@ -284,6 +296,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(150), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 300, 100, 50, @@ -327,6 +340,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 3666666670000, 3666666670000, 0, @@ -372,6 +386,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 3666666670000, 0, 3666666670000, @@ -417,6 +432,7 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 3666666670000, 3666666670000 - 1833333335000, 1833333335000, @@ -462,11 +478,73 @@ func TestMigrateVestingAccounts(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true) require.NoError(t, err) }, + cleartTrackingFields, 3666666670000, 3666666670000 - 1833333335000 - 916666667500, 1833333335000 + 916666667500, 1601042400 + 31536000 + 15638400 + 1, }, + { + "vesting account has unbonding delegations in place", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + + // delegation of the original vesting + _, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true) + require.NoError(t, err) + + ctx = ctx.WithBlockTime(ctx.BlockTime().AddDate(1, 0, 0)) + + valAddr, err := sdk.ValAddressFromBech32(validator.OperatorAddress) + require.NoError(t, err) + + // un-delegation of the original vesting + _, err = app.StakingKeeper.Undelegate(ctx, delegatorAddr, valAddr, sdk.NewDecFromInt(sdk.NewInt(300))) + require.NoError(t, err) + }, + cleartTrackingFields, + 450, + 300, + 0, + 0, + }, + { + "vesting account has never delegated anything", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + }, + cleartTrackingFields, + 450, + 0, + 0, + 0, + }, + { + "vesting account has no delegation but dirty DelegatedFree and DelegatedVesting fields", + func(app *simapp.SimApp, ctx sdk.Context, validator stakingtypes.Validator, delegatorAddr sdk.AccAddress) { + baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr) + vestedCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(300))) + + delayedAccount := types.NewDelayedVestingAccount(baseAccount, vestedCoins, ctx.BlockTime().AddDate(10, 0, 0).Unix()) + + app.AccountKeeper.SetAccount(ctx, delayedAccount) + }, + dirtyTrackingFields, + 450, + 0, + 0, + 0, + }, } for _, tc := range testCases { @@ -477,7 +555,7 @@ func TestMigrateVestingAccounts(t *testing.T) { Time: time.Now(), }) - addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(tc.tokenAmount+10)) + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(tc.tokenAmount)) delegatorAddr := addrs[0] _, valAddr := createValidator(t, ctx, app, tc.tokenAmount*2) @@ -494,7 +572,7 @@ func TestMigrateVestingAccounts(t *testing.T) { savedAccount := app.AccountKeeper.GetAccount(ctx, delegatorAddr) vestingAccount, ok := savedAccount.(exported.VestingAccount) require.True(t, ok) - require.NoError(t, introduceTrackingBug(ctx, vestingAccount, app)) + require.NoError(t, tc.garbageFunc(ctx, vestingAccount, app)) newData, err := v043.MigrateStore(ctx, app.AccountKeeper.GetAllAccounts(ctx), app.GRPCQueryRouter()) require.NoError(t, err) @@ -539,7 +617,7 @@ func trackingCorrected(ctx sdk.Context, t *testing.T, ak authkeeper.AccountKeepe require.True(t, freeOk, vDA.GetDelegatedFree().String()) } -func introduceTrackingBug(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error { +func cleartTrackingFields(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error { switch t := vesting.(type) { case *types.DelayedVestingAccount: t.DelegatedFree = nil @@ -560,6 +638,29 @@ func introduceTrackingBug(ctx sdk.Context, vesting exported.VestingAccount, app return nil } +func dirtyTrackingFields(ctx sdk.Context, vesting exported.VestingAccount, app *simapp.SimApp) error { + dirt := sdk.NewCoins(sdk.NewInt64Coin("stake", 42)) + + switch t := vesting.(type) { + case *types.DelayedVestingAccount: + t.DelegatedFree = dirt + t.DelegatedVesting = dirt + app.AccountKeeper.SetAccount(ctx, t) + case *types.ContinuousVestingAccount: + t.DelegatedFree = dirt + t.DelegatedVesting = dirt + app.AccountKeeper.SetAccount(ctx, t) + case *types.PeriodicVestingAccount: + t.DelegatedFree = dirt + t.DelegatedVesting = dirt + app.AccountKeeper.SetAccount(ctx, t) + default: + return fmt.Errorf("expected vesting account, found %t", t) + } + + return nil +} + func createValidator(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers int64) (sdk.AccAddress, sdk.ValAddress) { valTokens := sdk.TokensFromConsensusPower(powers) addrs := simapp.AddTestAddrsIncremental(app, ctx, 1, valTokens) From c44fe3832ba36ea0adc39674df6b8d40c78aa3ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 26 Mar 2021 13:06:33 +0100 Subject: [PATCH 33/37] use account iterator instead of a big slice of accounts perf/memory-wise, using an iterator is far better --- x/auth/keeper/migrations.go | 25 +++++++---- x/auth/legacy/v043/store.go | 82 +++++++++++++++++-------------------- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 60f2c9323cd5..a9a749023517 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -2,6 +2,7 @@ package keeper import ( v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043" + "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/gogo/protobuf/grpc" sdk "github.com/cosmos/cosmos-sdk/types" @@ -20,14 +21,22 @@ func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator { // Migrate1to2 migrates from version 1 to 2. func (m Migrator) Migrate1to2(ctx sdk.Context) error { - wb, err := v043.MigrateStore(ctx, m.keeper.GetAllAccounts(ctx), m.queryServer) - if err != nil { - return err - } + var iterErr error - for _, a := range wb { - m.keeper.SetAccount(ctx, a) - } + m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { + wb, err := v043.MigrateStore(ctx, account, m.queryServer) + if err != nil { + iterErr = err + return true + } - return nil + if wb == nil { + return false + } + + m.keeper.SetAccount(ctx, wb) + return false + }) + + return iterErr } diff --git a/x/auth/legacy/v043/store.go b/x/auth/legacy/v043/store.go index d9c4bea2a569..5fe344337114 100644 --- a/x/auth/legacy/v043/store.go +++ b/x/auth/legacy/v043/store.go @@ -26,63 +26,57 @@ const ( balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances" ) -func migrateVestingAccounts(ctx sdk.Context, accounts []types.AccountI, queryServer grpc.Server) ([]types.AccountI, error) { +func migrateVestingAccounts(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) { bondDenom, err := getBondDenom(ctx, queryServer) if err != nil { return nil, err } - for i := 0; i < len(accounts); i++ { - asVesting, ok := accounts[i].(exported.VestingAccount) - if !ok { - continue - } - - account := accounts[i] - - addr := account.GetAddress().String() - balance, err := getBalance( - ctx, - addr, - queryServer, - ) - - if err != nil { - return nil, err - } + asVesting, ok := account.(exported.VestingAccount) + if !ok { + return nil, nil + } - delegations, err := getDelegatorDelegationsSum( - ctx, - addr, - queryServer, - ) + addr := account.GetAddress().String() + balance, err := getBalance( + ctx, + addr, + queryServer, + ) - unbondingDelegations, err := getDelegatorUnbondingDelegationsSum( - ctx, - addr, - bondDenom, - queryServer, - ) + if err != nil { + return nil, err + } - delegations = delegations.Add(unbondingDelegations...) + delegations, err := getDelegatorDelegationsSum( + ctx, + addr, + queryServer, + ) - asVesting, ok = resetVestingDelegatedBalances(asVesting) - if !ok { - continue - } + unbondingDelegations, err := getDelegatorUnbondingDelegationsSum( + ctx, + addr, + bondDenom, + queryServer, + ) - // balance before any delegation includes balance of delegation - for _, coin := range delegations { - balance = balance.Add(coin) - } + delegations = delegations.Add(unbondingDelegations...) - asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations) + asVesting, ok = resetVestingDelegatedBalances(asVesting) + if !ok { + return nil, nil + } - accounts[i] = asVesting.(types.AccountI) + // balance before any delegation includes balance of delegation + for _, coin := range delegations { + balance = balance.Add(coin) } - return accounts, nil + asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations) + + return asVesting.(types.AccountI), nil } func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.VestingAccount, bool) { @@ -261,6 +255,6 @@ func getBondDenom(ctx sdk.Context, queryServer grpc.Server) (string, error) { // MigrateStore migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly // track delegations. // References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 -func MigrateStore(ctx sdk.Context, accounts []types.AccountI, queryServer grpc.Server) ([]types.AccountI, error) { - return migrateVestingAccounts(ctx, accounts, queryServer) +func MigrateStore(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) { + return migrateVestingAccounts(ctx, account, queryServer) } From 5fdda3d3f7ac9c9a1cdd97f4b2a69feee190e1c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 26 Mar 2021 13:07:03 +0100 Subject: [PATCH 34/37] call auth keeper migrations in tests --- x/auth/legacy/v043/store_test.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go index 04277264a2a2..acd06fe8783c 100644 --- a/x/auth/legacy/v043/store_test.go +++ b/x/auth/legacy/v043/store_test.go @@ -11,7 +11,6 @@ import ( "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" - v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" @@ -574,12 +573,8 @@ func TestMigrateVestingAccounts(t *testing.T) { require.True(t, ok) require.NoError(t, tc.garbageFunc(ctx, vestingAccount, app)) - newData, err := v043.MigrateStore(ctx, app.AccountKeeper.GetAllAccounts(ctx), app.GRPCQueryRouter()) - require.NoError(t, err) - - for _, a := range newData { - app.AccountKeeper.SetAccount(ctx, a) - } + m := authkeeper.NewMigrator(app.AccountKeeper, app.GRPCQueryRouter()) + require.NoError(t, m.Migrate1to2(ctx)) var expVested sdk.Coins var expFree sdk.Coins From f6a602bd15aa09d7d59bccbcb9e4580a8a132659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Fri, 26 Mar 2021 13:08:08 +0100 Subject: [PATCH 35/37] check delegations queries errors --- x/auth/legacy/v043/store.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x/auth/legacy/v043/store.go b/x/auth/legacy/v043/store.go index 5fe344337114..524b7de353ea 100644 --- a/x/auth/legacy/v043/store.go +++ b/x/auth/legacy/v043/store.go @@ -55,6 +55,10 @@ func migrateVestingAccounts(ctx sdk.Context, account types.AccountI, queryServer queryServer, ) + if err != nil { + return nil, err + } + unbondingDelegations, err := getDelegatorUnbondingDelegationsSum( ctx, addr, @@ -62,6 +66,10 @@ func migrateVestingAccounts(ctx sdk.Context, account types.AccountI, queryServer queryServer, ) + if err != nil { + return nil, err + } + delegations = delegations.Add(unbondingDelegations...) asVesting, ok = resetVestingDelegatedBalances(asVesting) From 5d9d4f9370ec540469510219484c12b07f8381df Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Sun, 4 Apr 2021 20:52:21 +0200 Subject: [PATCH 36/37] fix linter warnings The interfacer linter has been deprecated. --- x/auth/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 3ead1674e742..62a10e26ba52 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -206,7 +206,7 @@ func (ak AccountKeeper) GetModuleAccount(ctx sdk.Context, moduleName string) typ } // SetModuleAccount sets the module account to the auth account store -func (ak AccountKeeper) SetModuleAccount(ctx sdk.Context, macc types.ModuleAccountI) { //nolint:interfacer +func (ak AccountKeeper) SetModuleAccount(ctx sdk.Context, macc types.ModuleAccountI) { ak.SetAccount(ctx, macc) } From f342589ac24f3612cce51963fb2c310f0d16a776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Tue, 6 Apr 2021 21:01:48 +0200 Subject: [PATCH 37/37] rename migrate function to MigrateAccount --- x/auth/keeper/migrations.go | 2 +- x/auth/legacy/v043/store.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index a9a749023517..20f60f6abb32 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -24,7 +24,7 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { var iterErr error m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) { - wb, err := v043.MigrateStore(ctx, account, m.queryServer) + wb, err := v043.MigrateAccount(ctx, account, m.queryServer) if err != nil { iterErr = err return true diff --git a/x/auth/legacy/v043/store.go b/x/auth/legacy/v043/store.go index 524b7de353ea..7abb2917cab5 100644 --- a/x/auth/legacy/v043/store.go +++ b/x/auth/legacy/v043/store.go @@ -260,9 +260,9 @@ func getBondDenom(ctx sdk.Context, queryServer grpc.Server) (string, error) { return params.Params.BondDenom, nil } -// MigrateStore migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly +// MigrateAccount migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly // track delegations. // References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812 -func MigrateStore(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) { +func MigrateAccount(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) { return migrateVestingAccounts(ctx, account, queryServer) }