fix: fix the cancel unbond

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
 

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

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) {

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)
+}

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
+}

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.

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

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