From f560e440008b4b6a0c950e7d49461e898f3d6c4f Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Fri, 19 Aug 2022 12:52:14 +0530 Subject: [PATCH 01/12] fix: fix the cancel-unbond issue --- go.work.sum | 8 +++++ test.go | 30 ++++++++++++++++ x/staking/keeper/migrations.go | 6 ++++ x/staking/migrations/v5/store.go | 59 ++++++++++++++++++++++++++++++++ x/staking/module.go | 5 ++- x/staking/types/delegation.go | 23 +++++++++++-- 6 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 test.go create mode 100644 x/staking/migrations/v5/store.go diff --git a/go.work.sum b/go.work.sum index 97292c8f8b16..936fb3de9010 100644 --- a/go.work.sum +++ b/go.work.sum @@ -11,6 +11,7 @@ github.com/coinbase/kryptology v1.8.0/go.mod h1:RYXOAPdzOGUe3qlSFkMGn58i3xUA8hmx github.com/consensys/bavard v0.1.8-0.20210915155054-088da2f7f54a/go.mod h1:9ItSMtA/dXMAiL7BG6bqW2m3NdSEObYWoH223nGHukI= github.com/consensys/gnark-crypto v0.5.3/go.mod h1:hOdPlWQV1gDLp7faZVeg8Y0iEPFaOUnCc4XeCCk96p0= github.com/cosmos/cosmos-sdk/db v1.0.0-beta.1/go.mod h1:JUMM2MxF9wuwzRWZJjb8BjXsn1BmPmdBd3a75pIct4I= +github.com/denisenkom/go-mssqldb v0.12.0 h1:VtrkII767ttSPNRfFekePK3sctr+joXgO58stqQbtUA= github.com/dgraph-io/ristretto v0.0.3/go.mod h1:KPxhHT9ZxKefz+PCeOGsrHpl1qZ7i70dGTu2u+Ahh6E= github.com/ethereum/go-ethereum v1.10.19/go.mod h1:IJBNMtzKcNHPtllYihy6BL2IgK1u+32JriaTbdt4v+w= github.com/facebookgo/ensure v0.0.0-20200202191622-63f1cf65ac4c/go.mod h1:Yg+htXGokKKdzcwhuNDwVvN+uBxDGXJ7G/VN1d8fa64= @@ -18,12 +19,19 @@ github.com/facebookgo/subset v0.0.0-20200203212716-c811ad88dec4/go.mod h1:5tD+ne github.com/go-kit/log v0.2.1 h1:MRVx0/zhvdseW+Gza6N9rVzU/IVzaeE1SFI4raAhmBU= github.com/go-logfmt/logfmt v0.5.1 h1:otpy5pqBCBZ1ng9RQ0dPu4PN7ba75Y/aA+UpowDyNVA= github.com/go-playground/assert/v2 v2.0.1 h1:MsBgLAaY856+nPRTKrp3/OZK38U/wa0CcBYNjji3q3A= +github.com/go-sql-driver/mysql v1.6.0 h1:BCTh4TKNUYmOmMUcQ3IipzF5prigylS7XXjEkfCHuOE= +github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe h1:lXe2qZdvpiX5WZkZR4hgp4KJVfY3nMkvmwbVkpv1rVY= +github.com/golang-sql/sqlexp v0.0.0-20170517235910-f1bb20e5a188 h1:+eHOFJl1BaXrQxKX+T06f78590z4qA2ZzBTqahsKSE4= +github.com/gotestyourself/gotestyourself v2.2.0+incompatible h1:AQwinXlbQR2HvPjQZOmDhRqsv5mZf+Jb1RnSLxcqZcI= github.com/gtank/merlin v0.1.1 h1:eQ90iG7K9pOhtereWsmyRJ6RAwcP4tHTDBHXNg+u5is= github.com/gtank/ristretto255 v0.1.2 h1:JEqUCPA1NvLq5DwYtuzigd7ss8fwbYay9fi4/5uMzcc= github.com/leanovate/gopter v0.2.9/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/lucasjones/reggen v0.0.0-20180717132126-cdb49ff09d77/go.mod h1:5ELEyG+X8f+meRWHuqUOewBOhvHkl7M76pdGEansxW4= +github.com/mattn/go-sqlite3 v1.14.9 h1:10HX2Td0ocZpYEjhilsuo6WWtUqttj2Kb0KtD86/KYA= github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643 h1:hLDRPB66XQT/8+wG9WsDpiCvZf1yKO7sz7scAjSlBa0= +github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 h1:dcztxKSvZ4Id8iPpHERQBbIJfabdt4wUm5qy3wOL2Zc= github.com/neilotoole/errgroup v0.1.6/go.mod h1:Q2nLGf+594h0CLBs/Mbg6qOr7GtqDK7C2S41udRnToE= +github.com/ory/dockertest/v3 v3.9.1 h1:v4dkG+dlu76goxMiTT2j8zV7s4oPPEppKT8K8p2f1kY= github.com/segmentio/fasthash v1.0.3/go.mod h1:waKX8l2N8yckOgmSsXJi7x1ZfdKZ4x7KRMzBtS3oedY= github.com/tendermint/tendermint v0.34.20-rc1/go.mod h1:u2xI6q3IeLQQ2NdIpRKLUKBNog0o7EzBpvDCE0/OTmg= github.com/tidwall/gjson v1.12.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= diff --git a/test.go b/test.go new file mode 100644 index 000000000000..fb910c6e6a8c --- /dev/null +++ b/test.go @@ -0,0 +1,30 @@ +package main + +import "fmt" + +type sai struct { + name string + age int +} + +func main() { + var My_map = make(map[float64][]string) + fmt.Println(My_map) + + // As we already know that make() function + // always returns a map which is initialized + // So, we can add values in it + My_map[1.3] = append(My_map[1.3], "Rohit") + My_map[1.5] = append(My_map[1.3], "Sumit") + fmt.Println(My_map) + + for k, val := range My_map { + fmt.Println("Key ", k) + fmt.Println("Val ", val) + } + + var a sai + a.age = 10 + a.name = "Sai" + fmt.Println(a) +} diff --git a/x/staking/keeper/migrations.go b/x/staking/keeper/migrations.go index b34a7993bfc7..8782f5978316 100644 --- a/x/staking/keeper/migrations.go +++ b/x/staking/keeper/migrations.go @@ -6,6 +6,7 @@ import ( v2 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v2" v3 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v3" v4 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v4" + v5 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v5" ) // Migrator is a struct for handling in-place store migrations. @@ -36,3 +37,8 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error { func (m Migrator) Migrate3to4(ctx sdk.Context) error { return v4.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.legacySubspace) } + +// Migrate4to5 migrates x/staking state from consensus version 4 to 5. +func (m Migrator) Migrate4to5(ctx sdk.Context) error { + return v5.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) +} diff --git a/x/staking/migrations/v5/store.go b/x/staking/migrations/v5/store.go new file mode 100644 index 000000000000..a7dd873a3799 --- /dev/null +++ b/x/staking/migrations/v5/store.go @@ -0,0 +1,59 @@ +package v5 + +import ( + "github.com/cosmos/cosmos-sdk/codec" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +// MigrateStore performs in-place store migrations from v3 to v4. +/* + this migrateStore will remove the ubdEntries with same creation_height + and create a new ubdEntry with updated balance and initial_balance +*/ +func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { + store := ctx.KVStore(storeKey) + iterator := sdk.KVStorePrefixIterator(store, types.UnbondingDelegationKey) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + ubd := types.MustUnmarshalUBD(cdc, iterator.Value()) + entriesAtSameCreationHeight := make(map[int64][]types.UnbondingDelegationEntry) + for _, ubdEntry := range ubd.Entries { + entriesAtSameCreationHeight[ubdEntry.CreationHeight] = append(entriesAtSameCreationHeight[ubdEntry.CreationHeight], ubdEntry) + } + // clear the old ubdEntries + ubd.Entries = nil + + for _, entries := range entriesAtSameCreationHeight { + var ubdEntry types.UnbondingDelegationEntry + for _, entry := range entries { + ubdEntry.Balance = ubdEntry.Balance.Add(entry.Balance) + ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(entry.InitialBalance) + ubdEntry.CreationHeight = entry.CreationHeight + ubdEntry.CompletionTime = entry.CompletionTime + } + ubd.Entries = append(ubd.Entries, ubdEntry) + } + + // set the new ubd to the store + setUBDToStore(ctx, storeKey, cdc, ubd) + } + + return nil +} + +func setUBDToStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec, ubd types.UnbondingDelegation) { + delegatorAddress := sdk.MustAccAddressFromBech32(ubd.DelegatorAddress) + + store := ctx.KVStore(storeKey) + bz := types.MustMarshalUBD(cdc, ubd) + addr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) + if err != nil { + panic(err) + } + key := types.GetUBDKey(delegatorAddress, addr) + store.Set(key, bz) + store.Set(types.GetUBDByValIndexKey(delegatorAddress, addr), []byte{}) // index, store empty bytes +} diff --git a/x/staking/module.go b/x/staking/module.go index fda2ab1a34dd..18dbfc462457 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -33,7 +33,7 @@ import ( ) const ( - consensusVersion uint64 = 4 + consensusVersion uint64 = 5 ) var ( @@ -152,6 +152,9 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4); err != nil { panic(fmt.Sprintf("failed to migrate x/%s from version 3 to 4: %v", types.ModuleName, err)) } + if err := cfg.RegisterMigration(types.ModuleName, 4, m.Migrate4to5); err != nil { + panic(fmt.Sprintf("failed to migrate x/%s from version 4 to 5: %v", types.ModuleName, err)) + } } // InitGenesis performs genesis initialization for the staking module. diff --git a/x/staking/types/delegation.go b/x/staking/types/delegation.go index cad8f5f0d497..364da68b04cd 100644 --- a/x/staking/types/delegation.go +++ b/x/staking/types/delegation.go @@ -130,8 +130,27 @@ func NewUnbondingDelegation( // AddEntry - append entry to the unbonding delegation func (ubd *UnbondingDelegation) AddEntry(creationHeight int64, minTime time.Time, balance math.Int) { - entry := NewUnbondingDelegationEntry(creationHeight, minTime, balance) - ubd.Entries = append(ubd.Entries, entry) + // Check the entries exists with creation_height and complete_time + var entryIndex int = -1 + for index, ubdEntry := range ubd.Entries { + if ubdEntry.CreationHeight == creationHeight && ubdEntry.CompletionTime.Equal(minTime) { + entryIndex = index + break + } + } + // entryIndex exists + if entryIndex != -1 { + ubdEntry := ubd.Entries[entryIndex] + ubdEntry.Balance = ubdEntry.Balance.Add(balance) + ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(balance) + + // update the entry + ubd.Entries[entryIndex] = ubdEntry + } else { + // append the new unbond delegation entry + entry := NewUnbondingDelegationEntry(creationHeight, minTime, balance) + ubd.Entries = append(ubd.Entries, entry) + } } // RemoveEntry - remove entry at index i to the unbonding delegation From 9d2f1224b05d8cd1ad766166b31b27508895ff6b Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Fri, 19 Aug 2022 13:13:20 +0530 Subject: [PATCH 02/12] updated the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33bb459967cb..fa7db00ac1f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) Migrate `x/staking` to self-managed parameters and deprecate it's usage of `x/params`. * (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly. * (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time. +* (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple ubd msgs broadcast. ### API Breaking Changes From fd7affcb8c45709b0ca969746a0a27f964c463cf Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Fri, 19 Aug 2022 13:21:41 +0530 Subject: [PATCH 03/12] chroe: removed un wanted code --- test.go | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 test.go diff --git a/test.go b/test.go deleted file mode 100644 index fb910c6e6a8c..000000000000 --- a/test.go +++ /dev/null @@ -1,30 +0,0 @@ -package main - -import "fmt" - -type sai struct { - name string - age int -} - -func main() { - var My_map = make(map[float64][]string) - fmt.Println(My_map) - - // As we already know that make() function - // always returns a map which is initialized - // So, we can add values in it - My_map[1.3] = append(My_map[1.3], "Rohit") - My_map[1.5] = append(My_map[1.3], "Sumit") - fmt.Println(My_map) - - for k, val := range My_map { - fmt.Println("Key ", k) - fmt.Println("Val ", val) - } - - var a sai - a.age = 10 - a.name = "Sai" - fmt.Println(a) -} From 9393857c28d7da17e31ac3c48d8b112ec8012baa Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Fri, 19 Aug 2022 13:41:40 +0530 Subject: [PATCH 04/12] chore: removed max ubd entries check --- x/staking/keeper/delegation.go | 14 ----- x/staking/keeper/delegation_test.go | 80 ----------------------------- x/staking/simulation/operations.go | 4 -- 3 files changed, 98 deletions(-) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index c63286e2769e..a35812faf159 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -259,16 +259,6 @@ func (k Keeper) IterateDelegatorRedelegations(ctx sdk.Context, delegator sdk.Acc } } -// HasMaxUnbondingDelegationEntries - check if unbonding delegation has maximum number of entries. -func (k Keeper) HasMaxUnbondingDelegationEntries(ctx sdk.Context, delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress) bool { - ubd, found := k.GetUnbondingDelegation(ctx, delegatorAddr, validatorAddr) - if !found { - return false - } - - return len(ubd.Entries) >= int(k.MaxEntries(ctx)) -} - // SetUnbondingDelegation sets the unbonding delegation and associated index. func (k Keeper) SetUnbondingDelegation(ctx sdk.Context, ubd types.UnbondingDelegation) { delegatorAddress := sdk.MustAccAddressFromBech32(ubd.DelegatorAddress) @@ -804,10 +794,6 @@ func (k Keeper) Undelegate( return time.Time{}, types.ErrNoDelegatorForAddress } - if k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr) { - return time.Time{}, types.ErrMaxUnbondingDelegationEntries - } - returnAmount, err := k.Unbond(ctx, delAddr, valAddr, sharesAmount) if err != nil { return time.Time{}, err diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index a74c14978954..d417fa02960d 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -237,86 +237,6 @@ func TestUnbondDelegation(t *testing.T) { require.Equal(t, remainingTokens, validator.BondedTokens()) } -func TestUnbondingDelegationsMaxEntries(t *testing.T) { - _, app, ctx := createTestInput(t) - - addrDels := simapp.AddTestAddrsIncremental(app, ctx, 1, sdk.NewInt(10000)) - addrVals := simtestutil.ConvertAddrsToValAddrs(addrDels) - - startTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, 10) - - bondDenom := app.StakingKeeper.BondDenom(ctx) - notBondedPool := app.StakingKeeper.GetNotBondedPool(ctx) - - require.NoError(t, testutil.FundModuleAccount(app.BankKeeper, ctx, notBondedPool.GetName(), sdk.NewCoins(sdk.NewCoin(bondDenom, startTokens)))) - app.AccountKeeper.SetModuleAccount(ctx, notBondedPool) - - // create a validator and a delegator to that validator - validator := teststaking.NewValidator(t, addrVals[0], PKs[0]) - - validator, issuedShares := validator.AddTokensFromDel(startTokens) - require.Equal(t, startTokens, issuedShares.RoundInt()) - - validator = keeper.TestingUpdateValidator(app.StakingKeeper, ctx, validator, true) - require.True(math.IntEq(t, startTokens, validator.BondedTokens())) - require.True(t, validator.IsBonded()) - - delegation := types.NewDelegation(addrDels[0], addrVals[0], issuedShares) - app.StakingKeeper.SetDelegation(ctx, delegation) - - maxEntries := app.StakingKeeper.MaxEntries(ctx) - - oldBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount - oldNotBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - - // should all pass - var completionTime time.Time - for i := uint32(0); i < maxEntries; i++ { - var err error - completionTime, err = app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) - require.NoError(t, err) - } - - newBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount - newNotBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - require.True(math.IntEq(t, newBonded, oldBonded.SubRaw(int64(maxEntries)))) - require.True(math.IntEq(t, newNotBonded, oldNotBonded.AddRaw(int64(maxEntries)))) - - oldBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount - oldNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - - // an additional unbond should fail due to max entries - _, err := app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) - require.Error(t, err) - - newBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount - newNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - - require.True(math.IntEq(t, newBonded, oldBonded)) - require.True(math.IntEq(t, newNotBonded, oldNotBonded)) - - // mature unbonding delegations - ctx = ctx.WithBlockTime(completionTime) - _, err = app.StakingKeeper.CompleteUnbonding(ctx, addrDels[0], addrVals[0]) - require.NoError(t, err) - - newBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount - newNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - require.True(math.IntEq(t, newBonded, oldBonded)) - require.True(math.IntEq(t, newNotBonded, oldNotBonded.SubRaw(int64(maxEntries)))) - - oldNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - - // unbonding should work again - _, err = app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) - require.NoError(t, err) - - newBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount - newNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount - require.True(math.IntEq(t, newBonded, oldBonded.SubRaw(1))) - require.True(math.IntEq(t, newNotBonded, oldNotBonded.AddRaw(1))) -} - // // test undelegating self delegation from a validator pushing it below MinSelfDelegation // // shift it from the bonded to unbonding state and jailed func TestUndelegateSelfDelegationBelowMinSelfDelegation(t *testing.T) { diff --git a/x/staking/simulation/operations.go b/x/staking/simulation/operations.go index c4dbd7ecd631..dc8677cd2a90 100644 --- a/x/staking/simulation/operations.go +++ b/x/staking/simulation/operations.go @@ -329,10 +329,6 @@ func SimulateMsgUndelegate(ak types.AccountKeeper, bk types.BankKeeper, k *keepe delegation := delegations[r.Intn(len(delegations))] delAddr := delegation.GetDelegatorAddr() - if k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr) { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "keeper does have a max unbonding delegation entries"), nil, nil - } - totalBond := validator.TokensFromShares(delegation.GetShares()).TruncateInt() if !totalBond.IsPositive() { return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "total bond is negative"), nil, nil From 0c3abf92a8d3543aca4ba0e62318a39fa3806102 Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Fri, 19 Aug 2022 15:33:45 +0530 Subject: [PATCH 05/12] chore: add max entries check for ubd --- x/staking/keeper/delegation.go | 14 +++++ x/staking/keeper/delegation_test.go | 81 +++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index a35812faf159..c63286e2769e 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -259,6 +259,16 @@ func (k Keeper) IterateDelegatorRedelegations(ctx sdk.Context, delegator sdk.Acc } } +// HasMaxUnbondingDelegationEntries - check if unbonding delegation has maximum number of entries. +func (k Keeper) HasMaxUnbondingDelegationEntries(ctx sdk.Context, delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress) bool { + ubd, found := k.GetUnbondingDelegation(ctx, delegatorAddr, validatorAddr) + if !found { + return false + } + + return len(ubd.Entries) >= int(k.MaxEntries(ctx)) +} + // SetUnbondingDelegation sets the unbonding delegation and associated index. func (k Keeper) SetUnbondingDelegation(ctx sdk.Context, ubd types.UnbondingDelegation) { delegatorAddress := sdk.MustAccAddressFromBech32(ubd.DelegatorAddress) @@ -794,6 +804,10 @@ func (k Keeper) Undelegate( return time.Time{}, types.ErrNoDelegatorForAddress } + if k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr) { + return time.Time{}, types.ErrMaxUnbondingDelegationEntries + } + returnAmount, err := k.Unbond(ctx, delAddr, valAddr, sharesAmount) if err != nil { return time.Time{}, err diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index d417fa02960d..79456679d006 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -237,6 +237,87 @@ func TestUnbondDelegation(t *testing.T) { require.Equal(t, remainingTokens, validator.BondedTokens()) } +func TestUnbondingDelegationsMaxEntries(t *testing.T) { + _, app, ctx := createTestInput(t) + + addrDels := simapp.AddTestAddrsIncremental(app, ctx, 1, sdk.NewInt(10000)) + addrVals := simtestutil.ConvertAddrsToValAddrs(addrDels) + + startTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, 10) + + bondDenom := app.StakingKeeper.BondDenom(ctx) + notBondedPool := app.StakingKeeper.GetNotBondedPool(ctx) + + require.NoError(t, testutil.FundModuleAccount(app.BankKeeper, ctx, notBondedPool.GetName(), sdk.NewCoins(sdk.NewCoin(bondDenom, startTokens)))) + app.AccountKeeper.SetModuleAccount(ctx, notBondedPool) + + // create a validator and a delegator to that validator + validator := teststaking.NewValidator(t, addrVals[0], PKs[0]) + + validator, issuedShares := validator.AddTokensFromDel(startTokens) + require.Equal(t, startTokens, issuedShares.RoundInt()) + + validator = keeper.TestingUpdateValidator(app.StakingKeeper, ctx, validator, true) + require.True(math.IntEq(t, startTokens, validator.BondedTokens())) + require.True(t, validator.IsBonded()) + + delegation := types.NewDelegation(addrDels[0], addrVals[0], issuedShares) + app.StakingKeeper.SetDelegation(ctx, delegation) + + maxEntries := app.StakingKeeper.MaxEntries(ctx) + + oldBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + oldNotBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + + // should all pass + var completionTime time.Time + for i := uint32(0); i < maxEntries; i++ { + var err error + ctx = ctx.WithBlockHeight(int64(i)) + completionTime, err = app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) + require.NoError(t, err) + } + + newBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + newNotBonded := app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + require.True(math.IntEq(t, newBonded, oldBonded.SubRaw(int64(maxEntries)))) + require.True(math.IntEq(t, newNotBonded, oldNotBonded.AddRaw(int64(maxEntries)))) + + oldBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + oldNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + + // an additional unbond should fail due to max entries + _, err := app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) + require.Error(t, err) + + newBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + newNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + + require.True(math.IntEq(t, newBonded, oldBonded)) + require.True(math.IntEq(t, newNotBonded, oldNotBonded)) + + // mature unbonding delegations + ctx = ctx.WithBlockTime(completionTime) + _, err = app.StakingKeeper.CompleteUnbonding(ctx, addrDels[0], addrVals[0]) + require.NoError(t, err) + + newBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + newNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + require.True(math.IntEq(t, newBonded, oldBonded)) + require.True(math.IntEq(t, newNotBonded, oldNotBonded.SubRaw(int64(maxEntries)))) + + oldNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + + // unbonding should work again + _, err = app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) + require.NoError(t, err) + + newBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetBondedPool(ctx).GetAddress(), bondDenom).Amount + newNotBonded = app.BankKeeper.GetBalance(ctx, app.StakingKeeper.GetNotBondedPool(ctx).GetAddress(), bondDenom).Amount + require.True(math.IntEq(t, newBonded, oldBonded.SubRaw(1))) + require.True(math.IntEq(t, newNotBonded, oldNotBonded.AddRaw(1))) +} + // // test undelegating self delegation from a validator pushing it below MinSelfDelegation // // shift it from the bonded to unbonding state and jailed func TestUndelegateSelfDelegationBelowMinSelfDelegation(t *testing.T) { From fbbf1ef3fe9f7284f4cf4636974752f48ea1bcec Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Fri, 19 Aug 2022 17:58:01 +0530 Subject: [PATCH 06/12] chore: address the pr comments --- CHANGELOG.md | 2 +- x/staking/simulation/operations.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7db00ac1f8..18da9e87d794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +78,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) Migrate `x/staking` to self-managed parameters and deprecate it's usage of `x/params`. * (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly. * (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time. -* (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple ubd msgs broadcast. +* (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple ubd msgs broadcast at a single height. ### API Breaking Changes diff --git a/x/staking/simulation/operations.go b/x/staking/simulation/operations.go index dc8677cd2a90..c4dbd7ecd631 100644 --- a/x/staking/simulation/operations.go +++ b/x/staking/simulation/operations.go @@ -329,6 +329,10 @@ func SimulateMsgUndelegate(ak types.AccountKeeper, bk types.BankKeeper, k *keepe delegation := delegations[r.Intn(len(delegations))] delAddr := delegation.GetDelegatorAddr() + if k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr) { + return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "keeper does have a max unbonding delegation entries"), nil, nil + } + totalBond := validator.TokensFromShares(delegation.GetShares()).TruncateInt() if !totalBond.IsPositive() { return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgUndelegate, "total bond is negative"), nil, nil From eef238baf9bc2fe625ee91d4fbdfb8081e906497 Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Sat, 20 Aug 2022 10:38:30 +0530 Subject: [PATCH 07/12] chore: address the pr comments --- CHANGELOG.md | 2 +- x/staking/migrations/v5/store.go | 29 +++++++++++++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ce613d8e1ff..acdf465e4f9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,7 +79,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) Migrate `x/staking` to self-managed parameters and deprecate it's usage of `x/params`. * (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly. * (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time. -* (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple ubd msgs broadcast at a single height. +* (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple unbondings exist at a single height (e.g. through multiple messages in a transaction). ### API Breaking Changes diff --git a/x/staking/migrations/v5/store.go b/x/staking/migrations/v5/store.go index a7dd873a3799..83b55bf71944 100644 --- a/x/staking/migrations/v5/store.go +++ b/x/staking/migrations/v5/store.go @@ -1,6 +1,8 @@ package v5 import ( + "sort" + "github.com/cosmos/cosmos-sdk/codec" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -8,10 +10,8 @@ import ( ) // MigrateStore performs in-place store migrations from v3 to v4. -/* - this migrateStore will remove the ubdEntries with same creation_height - and create a new ubdEntry with updated balance and initial_balance -*/ +// this migrateStore will remove the ubdEntries with same creation_height +// and create a new ubdEntry with updated balance and initial_balance func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { store := ctx.KVStore(storeKey) iterator := sdk.KVStorePrefixIterator(store, types.UnbondingDelegationKey) @@ -19,16 +19,24 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar for ; iterator.Valid(); iterator.Next() { ubd := types.MustUnmarshalUBD(cdc, iterator.Value()) + entriesAtSameCreationHeight := make(map[int64][]types.UnbondingDelegationEntry) + + var creationHeights []int64 + for _, ubdEntry := range ubd.Entries { entriesAtSameCreationHeight[ubdEntry.CreationHeight] = append(entriesAtSameCreationHeight[ubdEntry.CreationHeight], ubdEntry) + creationHeights = append(creationHeights, ubdEntry.CreationHeight) } + // clear the old ubdEntries ubd.Entries = nil - for _, entries := range entriesAtSameCreationHeight { + sort.Slice(creationHeights, func(i, j int) bool { return creationHeights[i] < creationHeights[j] }) + + for index := 0; index < len(creationHeights); index++ { var ubdEntry types.UnbondingDelegationEntry - for _, entry := range entries { + for _, entry := range entriesAtSameCreationHeight[creationHeights[index]] { ubdEntry.Balance = ubdEntry.Balance.Add(entry.Balance) ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(entry.InitialBalance) ubdEntry.CreationHeight = entry.CreationHeight @@ -38,22 +46,23 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar } // set the new ubd to the store - setUBDToStore(ctx, storeKey, cdc, ubd) + setUBDToStore(ctx, store, cdc, ubd) } return nil } -func setUBDToStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec, ubd types.UnbondingDelegation) { +func setUBDToStore(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, ubd types.UnbondingDelegation) { delegatorAddress := sdk.MustAccAddressFromBech32(ubd.DelegatorAddress) - store := ctx.KVStore(storeKey) bz := types.MustMarshalUBD(cdc, ubd) + addr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) if err != nil { panic(err) } + key := types.GetUBDKey(delegatorAddress, addr) + store.Set(key, bz) - store.Set(types.GetUBDByValIndexKey(delegatorAddress, addr), []byte{}) // index, store empty bytes } From d9ff33292ae16e10d1a12910987621475abf8950 Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Mon, 22 Aug 2022 09:57:15 +0530 Subject: [PATCH 08/12] chore: address the pr comments++ --- x/staking/migrations/v5/store.go | 4 ++-- x/staking/types/delegation.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/staking/migrations/v5/store.go b/x/staking/migrations/v5/store.go index 83b55bf71944..36b9672c2c79 100644 --- a/x/staking/migrations/v5/store.go +++ b/x/staking/migrations/v5/store.go @@ -34,9 +34,9 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar sort.Slice(creationHeights, func(i, j int) bool { return creationHeights[i] < creationHeights[j] }) - for index := 0; index < len(creationHeights); index++ { + for _, h := range creationHeights { var ubdEntry types.UnbondingDelegationEntry - for _, entry := range entriesAtSameCreationHeight[creationHeights[index]] { + for _, entry := range entriesAtSameCreationHeight[h] { ubdEntry.Balance = ubdEntry.Balance.Add(entry.Balance) ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(entry.InitialBalance) ubdEntry.CreationHeight = entry.CreationHeight diff --git a/x/staking/types/delegation.go b/x/staking/types/delegation.go index 364da68b04cd..51df1eb78b68 100644 --- a/x/staking/types/delegation.go +++ b/x/staking/types/delegation.go @@ -131,7 +131,7 @@ func NewUnbondingDelegation( // AddEntry - append entry to the unbonding delegation func (ubd *UnbondingDelegation) AddEntry(creationHeight int64, minTime time.Time, balance math.Int) { // Check the entries exists with creation_height and complete_time - var entryIndex int = -1 + entryIndex := -1 for index, ubdEntry := range ubd.Entries { if ubdEntry.CreationHeight == creationHeight && ubdEntry.CompletionTime.Equal(minTime) { entryIndex = index From 184bfd878b684ef9a0f9f81e44777dd08b5ebd16 Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Tue, 23 Aug 2022 11:11:04 +0530 Subject: [PATCH 09/12] chore: address the pr comments ++ --- x/staking/keeper/migrations.go | 6 --- x/staking/migrations/v4/store.go | 55 ++++++++++++++++++++++++++ x/staking/migrations/v5/store.go | 68 -------------------------------- x/staking/module.go | 5 +-- 4 files changed, 56 insertions(+), 78 deletions(-) delete mode 100644 x/staking/migrations/v5/store.go diff --git a/x/staking/keeper/migrations.go b/x/staking/keeper/migrations.go index 8782f5978316..b34a7993bfc7 100644 --- a/x/staking/keeper/migrations.go +++ b/x/staking/keeper/migrations.go @@ -6,7 +6,6 @@ import ( v2 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v2" v3 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v3" v4 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v4" - v5 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v5" ) // Migrator is a struct for handling in-place store migrations. @@ -37,8 +36,3 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error { func (m Migrator) Migrate3to4(ctx sdk.Context) error { return v4.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.legacySubspace) } - -// Migrate4to5 migrates x/staking state from consensus version 4 to 5. -func (m Migrator) Migrate4to5(ctx sdk.Context) error { - return v5.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) -} diff --git a/x/staking/migrations/v4/store.go b/x/staking/migrations/v4/store.go index 9a2215fc9b28..1ae295737d18 100644 --- a/x/staking/migrations/v4/store.go +++ b/x/staking/migrations/v4/store.go @@ -1,6 +1,8 @@ package v4 import ( + "sort" + "github.com/cosmos/cosmos-sdk/codec" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -21,5 +23,58 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar bz := cdc.MustMarshal(&legacyParams) store.Set(types.ParamsKey, bz) + // this will remove the ubdEntries with same creation_height + // and create a new ubdEntry with updated balance and initial_balance + + iterator := sdk.KVStorePrefixIterator(store, types.UnbondingDelegationKey) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + ubd := types.MustUnmarshalUBD(cdc, iterator.Value()) + + entriesAtSameCreationHeight := make(map[int64][]types.UnbondingDelegationEntry) + for _, ubdEntry := range ubd.Entries { + entriesAtSameCreationHeight[ubdEntry.CreationHeight] = append(entriesAtSameCreationHeight[ubdEntry.CreationHeight], ubdEntry) + } + + creationHeights := make([]int64, 0, len(entriesAtSameCreationHeight)) + for k := range entriesAtSameCreationHeight { + creationHeights = append(creationHeights, k) + } + + sort.Slice(creationHeights, func(i, j int) bool { return creationHeights[i] < creationHeights[j] }) + + ubd.Entries = make([]types.UnbondingDelegationEntry, 0, len(creationHeights)) + + for _, h := range creationHeights { + var ubdEntry types.UnbondingDelegationEntry + for _, entry := range entriesAtSameCreationHeight[h] { + ubdEntry.Balance = ubdEntry.Balance.Add(entry.Balance) + ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(entry.InitialBalance) + ubdEntry.CreationHeight = entry.CreationHeight + ubdEntry.CompletionTime = entry.CompletionTime + } + ubd.Entries = append(ubd.Entries, ubdEntry) + } + + // set the new ubd to the store + setUBDToStore(ctx, store, cdc, ubd) + } + return nil } + +func setUBDToStore(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, ubd types.UnbondingDelegation) { + delegatorAddress := sdk.MustAccAddressFromBech32(ubd.DelegatorAddress) + + bz := types.MustMarshalUBD(cdc, ubd) + + addr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) + if err != nil { + panic(err) + } + + key := types.GetUBDKey(delegatorAddress, addr) + + store.Set(key, bz) +} diff --git a/x/staking/migrations/v5/store.go b/x/staking/migrations/v5/store.go deleted file mode 100644 index 36b9672c2c79..000000000000 --- a/x/staking/migrations/v5/store.go +++ /dev/null @@ -1,68 +0,0 @@ -package v5 - -import ( - "sort" - - "github.com/cosmos/cosmos-sdk/codec" - storetypes "github.com/cosmos/cosmos-sdk/store/types" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/staking/types" -) - -// MigrateStore performs in-place store migrations from v3 to v4. -// this migrateStore will remove the ubdEntries with same creation_height -// and create a new ubdEntry with updated balance and initial_balance -func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { - store := ctx.KVStore(storeKey) - iterator := sdk.KVStorePrefixIterator(store, types.UnbondingDelegationKey) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - ubd := types.MustUnmarshalUBD(cdc, iterator.Value()) - - entriesAtSameCreationHeight := make(map[int64][]types.UnbondingDelegationEntry) - - var creationHeights []int64 - - for _, ubdEntry := range ubd.Entries { - entriesAtSameCreationHeight[ubdEntry.CreationHeight] = append(entriesAtSameCreationHeight[ubdEntry.CreationHeight], ubdEntry) - creationHeights = append(creationHeights, ubdEntry.CreationHeight) - } - - // clear the old ubdEntries - ubd.Entries = nil - - sort.Slice(creationHeights, func(i, j int) bool { return creationHeights[i] < creationHeights[j] }) - - for _, h := range creationHeights { - var ubdEntry types.UnbondingDelegationEntry - for _, entry := range entriesAtSameCreationHeight[h] { - ubdEntry.Balance = ubdEntry.Balance.Add(entry.Balance) - ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(entry.InitialBalance) - ubdEntry.CreationHeight = entry.CreationHeight - ubdEntry.CompletionTime = entry.CompletionTime - } - ubd.Entries = append(ubd.Entries, ubdEntry) - } - - // set the new ubd to the store - setUBDToStore(ctx, store, cdc, ubd) - } - - return nil -} - -func setUBDToStore(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, ubd types.UnbondingDelegation) { - delegatorAddress := sdk.MustAccAddressFromBech32(ubd.DelegatorAddress) - - bz := types.MustMarshalUBD(cdc, ubd) - - addr, err := sdk.ValAddressFromBech32(ubd.ValidatorAddress) - if err != nil { - panic(err) - } - - key := types.GetUBDKey(delegatorAddress, addr) - - store.Set(key, bz) -} diff --git a/x/staking/module.go b/x/staking/module.go index 18dbfc462457..fda2ab1a34dd 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -33,7 +33,7 @@ import ( ) const ( - consensusVersion uint64 = 5 + consensusVersion uint64 = 4 ) var ( @@ -152,9 +152,6 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4); err != nil { panic(fmt.Sprintf("failed to migrate x/%s from version 3 to 4: %v", types.ModuleName, err)) } - if err := cfg.RegisterMigration(types.ModuleName, 4, m.Migrate4to5); err != nil { - panic(fmt.Sprintf("failed to migrate x/%s from version 4 to 5: %v", types.ModuleName, err)) - } } // InitGenesis performs genesis initialization for the staking module. From 1cc48065522f387124d7fb31283e9eac0a3a7487 Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Wed, 24 Aug 2022 12:14:53 +0530 Subject: [PATCH 10/12] test: add store migrations --- x/staking/migrations/v4/migrations_test.go | 112 +++++++++++++++++++-- x/staking/migrations/v4/store.go | 30 +++++- 2 files changed, 127 insertions(+), 15 deletions(-) diff --git a/x/staking/migrations/v4/migrations_test.go b/x/staking/migrations/v4/migrations_test.go index da0bc977c22d..553b14eb4380 100644 --- a/x/staking/migrations/v4/migrations_test.go +++ b/x/staking/migrations/v4/migrations_test.go @@ -2,16 +2,19 @@ package v4_test import ( "testing" + "time" - "github.com/stretchr/testify/require" - + "github.com/cosmos/cosmos-sdk/codec" + storetypes "github.com/cosmos/cosmos-sdk/store/types" "github.com/cosmos/cosmos-sdk/testutil" + "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" - "github.com/cosmos/cosmos-sdk/x/distribution" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - "github.com/cosmos/cosmos-sdk/x/staking/migrations/v4" + "github.com/cosmos/cosmos-sdk/x/staking" + v4 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v4" "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/stretchr/testify/require" ) type mockSubspace struct { @@ -27,19 +30,108 @@ func (ms mockSubspace) GetParamSet(ctx sdk.Context, ps paramtypes.ParamSet) { } func TestMigrate(t *testing.T) { - encCfg := moduletestutil.MakeTestEncodingConfig(distribution.AppModuleBasic{}) + encCfg := moduletestutil.MakeTestEncodingConfig(staking.AppModuleBasic{}) cdc := encCfg.Codec storeKey := sdk.NewKVStoreKey(v4.ModuleName) tKey := sdk.NewTransientStoreKey("transient_test") ctx := testutil.DefaultContext(storeKey, tKey) store := ctx.KVStore(storeKey) + duplicateCreationHeight := int64(1) + + accAddrs := sims.CreateIncrementalAccounts(1) + accAddr := accAddrs[0] + + valAddrs := sims.ConvertAddrsToValAddrs(accAddrs) + valAddr := valAddrs[0] + + err := createOldStateUnbondind(t, duplicateCreationHeight, valAddr, accAddr, cdc, store) + require.NoError(t, err) legacySubspace := newMockSubspace(types.DefaultParams()) - require.NoError(t, v4.MigrateStore(ctx, storeKey, cdc, legacySubspace)) - var res types.Params - bz := store.Get(v4.ParamsKey) - require.NoError(t, cdc.Unmarshal(bz, &res)) - require.Equal(t, legacySubspace.ps, res) + testCases := []struct { + name string + doMigration bool + }{ + { + name: "without state migration", + doMigration: false, + }, + { + name: "with state migration", + doMigration: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if tc.doMigration { + require.NoError(t, v4.MigrateStore(ctx, storeKey, cdc, legacySubspace)) + } + + ubd := getUBD(t, accAddr, valAddr, store, cdc) + if tc.doMigration { + var res types.Params + bz := store.Get(v4.ParamsKey) + require.NoError(t, cdc.Unmarshal(bz, &res)) + require.Equal(t, legacySubspace.ps, res) + + // checking the updated balnace for duplicateCreationHeight + for _, ubdEntry := range ubd.Entries { + if ubdEntry.CreationHeight == duplicateCreationHeight { + require.Equal(t, sdk.NewInt(100*10), ubdEntry.Balance) + break + } + } + require.Equal(t, 11, len(ubd.Entries)) + } else { + require.Equal(t, true, true) + require.Equal(t, 20, len(ubd.Entries)) + } + }) + } +} + +// createOldStateUnbondind will create the ubd entries with duplicate heights +// 10 duplicate heights and 10 unique ubd creation height +func createOldStateUnbondind(t *testing.T, creationHeight int64, valAddr sdk.ValAddress, accAddr sdk.AccAddress, cdc codec.BinaryCodec, store storetypes.KVStore) error { + unbondBalance := sdk.NewInt(100) + udbEntries := make([]types.UnbondingDelegationEntry, 0, 10) + for i := int64(0); i < 10; i++ { + udbEntry := types.UnbondingDelegationEntry{ + CreationHeight: creationHeight, + Balance: unbondBalance, + InitialBalance: unbondBalance, + CompletionTime: time.Now().Add(time.Second * 10), + } + udbEntries = append(udbEntries, udbEntry) + // creating more entries for testing the creation_heights + udbEntry.CreationHeight = i + 2 + udbEntries = append(udbEntries, udbEntry) + } + + ubd := types.UnbondingDelegation{ + ValidatorAddress: valAddr.String(), + DelegatorAddress: accAddr.String(), + Entries: udbEntries, + } + + // set the unbond delegation with validator and delegator + bz := types.MustMarshalUBD(cdc, ubd) + key := getUBDKey(accAddr, valAddr) + store.Set(key, bz) + return nil +} + +func getUBD(t *testing.T, accAddr sdk.AccAddress, valAddr sdk.ValAddress, store storetypes.KVStore, cdc codec.BinaryCodec) types.UnbondingDelegation { + // get the unbondind delegations + var udbRes types.UnbondingDelegation + ubdbz := store.Get(getUBDKey(accAddr, valAddr)) + require.NoError(t, cdc.Unmarshal(ubdbz, &udbRes)) + return udbRes +} + +func getUBDKey(accAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { + return types.GetUBDKey(accAddr, valAddr) } diff --git a/x/staking/migrations/v4/store.go b/x/staking/migrations/v4/store.go index 1ae295737d18..43251e46310a 100644 --- a/x/staking/migrations/v4/store.go +++ b/x/staking/migrations/v4/store.go @@ -13,6 +13,22 @@ import ( // MigrateStore performs in-place store migrations from v3 to v4. func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec, legacySubspace exported.Subspace) error { store := ctx.KVStore(storeKey) + + // migrate params + if err := migrateParams(ctx, store, cdc, legacySubspace); err != nil { + return err + } + + // migrate unbondig delegations + if err := migrateUBD(ctx, store, cdc, legacySubspace); err != nil { + return err + } + + return nil +} + +// migrateParams will set the params to store from legacySubspace +func migrateParams(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, legacySubspace exported.Subspace) error { var legacyParams types.Params legacySubspace.GetParamSet(ctx, &legacyParams) @@ -22,10 +38,12 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar bz := cdc.MustMarshal(&legacyParams) store.Set(types.ParamsKey, bz) + return nil +} - // this will remove the ubdEntries with same creation_height - // and create a new ubdEntry with updated balance and initial_balance - +// migrateUBD will remove the ubdEntries with same creation_height +// and create a new ubdEntry with updated balance and initial_balance +func migrateUBD(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, legacySubspace exported.Subspace) error { iterator := sdk.KVStorePrefixIterator(store, types.UnbondingDelegationKey) defer iterator.Close() @@ -47,7 +65,10 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar ubd.Entries = make([]types.UnbondingDelegationEntry, 0, len(creationHeights)) for _, h := range creationHeights { - var ubdEntry types.UnbondingDelegationEntry + ubdEntry := types.UnbondingDelegationEntry{ + Balance: sdk.ZeroInt(), + InitialBalance: sdk.ZeroInt(), + } for _, entry := range entriesAtSameCreationHeight[h] { ubdEntry.Balance = ubdEntry.Balance.Add(entry.Balance) ubdEntry.InitialBalance = ubdEntry.InitialBalance.Add(entry.InitialBalance) @@ -60,7 +81,6 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar // set the new ubd to the store setUBDToStore(ctx, store, cdc, ubd) } - return nil } From 62498a33edd57ef137672f9245c89b75405c637d Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Wed, 24 Aug 2022 12:18:07 +0530 Subject: [PATCH 11/12] chore: small fix --- x/staking/migrations/v4/migrations_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/x/staking/migrations/v4/migrations_test.go b/x/staking/migrations/v4/migrations_test.go index 553b14eb4380..99a6bc2dbcdf 100644 --- a/x/staking/migrations/v4/migrations_test.go +++ b/x/staking/migrations/v4/migrations_test.go @@ -97,18 +97,21 @@ func TestMigrate(t *testing.T) { // 10 duplicate heights and 10 unique ubd creation height func createOldStateUnbondind(t *testing.T, creationHeight int64, valAddr sdk.ValAddress, accAddr sdk.AccAddress, cdc codec.BinaryCodec, store storetypes.KVStore) error { unbondBalance := sdk.NewInt(100) + completionTime := time.Now() udbEntries := make([]types.UnbondingDelegationEntry, 0, 10) + for i := int64(0); i < 10; i++ { - udbEntry := types.UnbondingDelegationEntry{ + ubdEntry := types.UnbondingDelegationEntry{ CreationHeight: creationHeight, Balance: unbondBalance, InitialBalance: unbondBalance, - CompletionTime: time.Now().Add(time.Second * 10), + CompletionTime: completionTime, } - udbEntries = append(udbEntries, udbEntry) + udbEntries = append(udbEntries, ubdEntry) // creating more entries for testing the creation_heights - udbEntry.CreationHeight = i + 2 - udbEntries = append(udbEntries, udbEntry) + ubdEntry.CreationHeight = i + 2 + ubdEntry.CompletionTime = completionTime.Add(time.Minute * 10) + udbEntries = append(udbEntries, ubdEntry) } ubd := types.UnbondingDelegation{ From c6654a8705e0431c7d251560dc0e1350bc030579 Mon Sep 17 00:00:00 2001 From: Sai Kumar Date: Wed, 24 Aug 2022 18:23:25 +0530 Subject: [PATCH 12/12] chore: address the review comments++ --- x/staking/keeper/delegation_test.go | 4 ++-- x/staking/migrations/v4/migrations_test.go | 27 +++++++++++----------- x/staking/migrations/v4/store.go | 8 +++---- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 79456679d006..f93617a3bf2f 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -271,9 +271,9 @@ func TestUnbondingDelegationsMaxEntries(t *testing.T) { // should all pass var completionTime time.Time - for i := uint32(0); i < maxEntries; i++ { + for i := int64(0); i < int64(maxEntries); i++ { var err error - ctx = ctx.WithBlockHeight(int64(i)) + ctx = ctx.WithBlockHeight(i) completionTime, err = app.StakingKeeper.Undelegate(ctx, addrDels[0], addrVals[0], math.LegacyNewDec(1)) require.NoError(t, err) } diff --git a/x/staking/migrations/v4/migrations_test.go b/x/staking/migrations/v4/migrations_test.go index 99a6bc2dbcdf..eee755078571 100644 --- a/x/staking/migrations/v4/migrations_test.go +++ b/x/staking/migrations/v4/migrations_test.go @@ -45,7 +45,8 @@ func TestMigrate(t *testing.T) { valAddrs := sims.ConvertAddrsToValAddrs(accAddrs) valAddr := valAddrs[0] - err := createOldStateUnbondind(t, duplicateCreationHeight, valAddr, accAddr, cdc, store) + // creating 10 ubdEntries with same height and 10 ubdEntries with different creation height + err := createOldStateUnbonding(t, duplicateCreationHeight, valAddr, accAddr, cdc, store) require.NoError(t, err) legacySubspace := newMockSubspace(types.DefaultParams()) @@ -77,7 +78,7 @@ func TestMigrate(t *testing.T) { require.NoError(t, cdc.Unmarshal(bz, &res)) require.Equal(t, legacySubspace.ps, res) - // checking the updated balnace for duplicateCreationHeight + // checking the updated balance for duplicateCreationHeight for _, ubdEntry := range ubd.Entries { if ubdEntry.CreationHeight == duplicateCreationHeight { require.Equal(t, sdk.NewInt(100*10), ubdEntry.Balance) @@ -93,12 +94,12 @@ func TestMigrate(t *testing.T) { } } -// createOldStateUnbondind will create the ubd entries with duplicate heights -// 10 duplicate heights and 10 unique ubd creation height -func createOldStateUnbondind(t *testing.T, creationHeight int64, valAddr sdk.ValAddress, accAddr sdk.AccAddress, cdc codec.BinaryCodec, store storetypes.KVStore) error { +// createOldStateUnbonding will create the ubd entries with duplicate heights +// 10 duplicate heights and 10 unique ubd with creation height +func createOldStateUnbonding(t *testing.T, creationHeight int64, valAddr sdk.ValAddress, accAddr sdk.AccAddress, cdc codec.BinaryCodec, store storetypes.KVStore) error { unbondBalance := sdk.NewInt(100) completionTime := time.Now() - udbEntries := make([]types.UnbondingDelegationEntry, 0, 10) + ubdEntries := make([]types.UnbondingDelegationEntry, 0, 10) for i := int64(0); i < 10; i++ { ubdEntry := types.UnbondingDelegationEntry{ @@ -107,17 +108,17 @@ func createOldStateUnbondind(t *testing.T, creationHeight int64, valAddr sdk.Val InitialBalance: unbondBalance, CompletionTime: completionTime, } - udbEntries = append(udbEntries, ubdEntry) + ubdEntries = append(ubdEntries, ubdEntry) // creating more entries for testing the creation_heights ubdEntry.CreationHeight = i + 2 ubdEntry.CompletionTime = completionTime.Add(time.Minute * 10) - udbEntries = append(udbEntries, ubdEntry) + ubdEntries = append(ubdEntries, ubdEntry) } ubd := types.UnbondingDelegation{ ValidatorAddress: valAddr.String(), DelegatorAddress: accAddr.String(), - Entries: udbEntries, + Entries: ubdEntries, } // set the unbond delegation with validator and delegator @@ -128,11 +129,11 @@ func createOldStateUnbondind(t *testing.T, creationHeight int64, valAddr sdk.Val } func getUBD(t *testing.T, accAddr sdk.AccAddress, valAddr sdk.ValAddress, store storetypes.KVStore, cdc codec.BinaryCodec) types.UnbondingDelegation { - // get the unbondind delegations - var udbRes types.UnbondingDelegation + // get the unbonding delegations + var ubdRes types.UnbondingDelegation ubdbz := store.Get(getUBDKey(accAddr, valAddr)) - require.NoError(t, cdc.Unmarshal(ubdbz, &udbRes)) - return udbRes + require.NoError(t, cdc.Unmarshal(ubdbz, &ubdRes)) + return ubdRes } func getUBDKey(accAddr sdk.AccAddress, valAddr sdk.ValAddress) []byte { diff --git a/x/staking/migrations/v4/store.go b/x/staking/migrations/v4/store.go index 43251e46310a..9c09ddf60361 100644 --- a/x/staking/migrations/v4/store.go +++ b/x/staking/migrations/v4/store.go @@ -19,8 +19,8 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar return err } - // migrate unbondig delegations - if err := migrateUBD(ctx, store, cdc, legacySubspace); err != nil { + // migrate unbonding delegations + if err := migrateUBDEntries(ctx, store, cdc, legacySubspace); err != nil { return err } @@ -41,9 +41,9 @@ func migrateParams(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCo return nil } -// migrateUBD will remove the ubdEntries with same creation_height +// migrateUBDEntries will remove the ubdEntries with same creation_height // and create a new ubdEntry with updated balance and initial_balance -func migrateUBD(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, legacySubspace exported.Subspace) error { +func migrateUBDEntries(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, legacySubspace exported.Subspace) error { iterator := sdk.KVStorePrefixIterator(store, types.UnbondingDelegationKey) defer iterator.Close()