fix!: x/staking - remove delegation with amount is zero (#10254)

CHANGELOG.md

@@ -150,6 +150,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### State Machine Breaking
 
+* (x/staking) [#10254](https://github.com/cosmos/cosmos-sdk/pull/10254) Instead of using the shares to determine if a delegation should be removed, use the truncated (token) amount.
 * (store) [#10247](https://github.com/cosmos/cosmos-sdk/pull/10247) Charge gas for the key length in gas meter.
 * (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.
 * (x/auth)[\#9596](https://github.com/cosmos/cosmos-sdk/pull/9596) Enable creating periodic vesting accounts with a transactions instead of requiring them to be created in genesis.

x/staking/genesis.go

@@ -77,6 +77,7 @@ func InitGenesis(
 		}
 
 		keeper.SetDelegation(ctx, delegation)
+
 		// Call the after-modification hook if not exported
 		if !data.Exported {
 			if err := keeper.AfterDelegationModified(ctx, delegatorAddress, delegation.GetValidatorAddr()); err != nil {
@@ -110,15 +111,18 @@ func InitGenesis(
 	if bondedPool == nil {
 		panic(fmt.Sprintf("%s module account has not been set", types.BondedPoolName))
 	}
-	// TODO remove with genesis 2-phases refactor https://github.com/cosmos/cosmos-sdk/issues/2862
+
+	// TODO: remove with genesis 2-phases refactor https://github.com/cosmos/cosmos-sdk/issues/2862
 	bondedBalance := bankKeeper.GetAllBalances(ctx, bondedPool.GetAddress())
 	if bondedBalance.IsZero() {
 		accountKeeper.SetModuleAccount(ctx, bondedPool)
 	}
+
 	// if balance is different from bonded coins panic because genesis is most likely malformed
 	if !bondedBalance.IsEqual(bondedCoins) {
 		panic(fmt.Sprintf("bonded pool balance is different from bonded coins: %s <-> %s", bondedBalance, bondedCoins))
 	}
+
 	notBondedPool := keeper.GetNotBondedPool(ctx)
 	if notBondedPool == nil {
 		panic(fmt.Sprintf("%s module account has not been set", types.NotBondedPoolName))
@@ -128,10 +132,13 @@ func InitGenesis(
 	if notBondedBalance.IsZero() {
 		accountKeeper.SetModuleAccount(ctx, notBondedPool)
 	}
-	// if balance is different from non bonded coins panic because genesis is most likely malformed
+
+	// If balance is different from non bonded coins panic because genesis is most
+	// likely malformed.
 	if !notBondedBalance.IsEqual(notBondedCoins) {
 		panic(fmt.Sprintf("not bonded pool balance is different from not bonded coins: %s <-> %s", notBondedBalance, notBondedCoins))
 	}
+
 	// don't need to run Tendermint updates if we exported
 	if data.Exported {
 		for _, lv := range data.LastValidatorPowers {
@@ -139,6 +146,7 @@ func InitGenesis(
 			if err != nil {
 				panic(err)
 			}
+
 			keeper.SetLastValidatorPower(ctx, valAddr, lv.Power)
 			validator, found := keeper.GetValidator(ctx, valAddr)
 
@@ -152,6 +160,7 @@ func InitGenesis(
 		}
 	} else {
 		var err error
+
 		res, err = keeper.ApplyAndReturnValidatorSetUpdates(ctx)
 		if err != nil {
 			panic(err)

x/staking/keeper/delegation.go

@@ -680,8 +680,11 @@ func (k Keeper) Unbond(
 		validator = k.mustGetValidator(ctx, validator.GetOperator())
 	}
 
-	// remove the delegation
-	if delegation.Shares.IsZero() {
+	// Remove the delegation if the resulting shares yield a truncated zero amount
+	// of tokens in the delegation. Note, in this this case, the shares themselves
+	// may not be zero, but rather a small fractional amount. Otherwise, we update
+	// the delegation object.
+	if validator.TokensFromShares(delegation.Shares).TruncateInt().IsZero() || delegation.Shares.IsZero() {
 		err = k.RemoveDelegation(ctx, delegation)
 	} else {
 		k.SetDelegation(ctx, delegation)
@@ -696,7 +699,6 @@ func (k Keeper) Unbond(
 	// remove the shares and coins from the validator
 	// NOTE that the amount is later (in keeper.Delegation) moved between staking module pools
 	validator, amount = k.RemoveValidatorTokensAndShares(ctx, validator, shares)
-
 	if validator.DelegatorShares.IsZero() && validator.IsUnbonded() {
 		// if not unbonded, we must instead remove validator in EndBlocker once it finishes its unbonding period
 		k.RemoveValidator(ctx, validator.GetOperator())

x/staking/keeper/invariants.go

@@ -137,13 +137,11 @@ func PositiveDelegationInvariant(k Keeper) sdk.Invariant {
 		for _, delegation := range delegations {
 			if delegation.Shares.IsNegative() {
 				count++
-
 				msg += fmt.Sprintf("\tdelegation with negative shares: %+v\n", delegation)
 			}
 
 			if delegation.Shares.IsZero() {
 				count++
-
 				msg += fmt.Sprintf("\tdelegation with zero shares: %+v\n", delegation)
 			}
 		}

x/staking/keeper/migrations.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	v043 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v043"
+	v045 "github.com/cosmos/cosmos-sdk/x/staking/migrations/v045"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -12,10 +13,17 @@ type Migrator struct {
 
 // NewMigrator returns a new Migrator.
 func NewMigrator(keeper Keeper) Migrator {
-	return Migrator{keeper: keeper}
+	return Migrator{
+		keeper: keeper,
+	}
 }
 
 // Migrate1to2 migrates from version 1 to 2.
 func (m Migrator) Migrate1to2(ctx sdk.Context) error {
 	return v043.MigrateStore(ctx, m.keeper.storeKey)
 }
+
+// Migrate2to3 migrates x/staking state from consensus version 2 to 3.
+func (m Migrator) Migrate2to3(ctx sdk.Context) error {
+	return v045.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
+}

x/staking/migrations/v040/types.go

@@ -183,6 +183,10 @@ func (v Validator) String() string {
 	return string(out)
 }
 
+func (v Validator) TokensFromShares(shares sdk.Dec) sdk.Dec {
+	return (shares.MulInt(v.Tokens)).Quo(v.DelegatorShares)
+}
+
 // Validators is a collection of Validator
 type Validators []Validator
 

x/staking/migrations/v045/keys.go

@@ -0,0 +1,11 @@
+package v045
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/address"
+	v040staking "github.com/cosmos/cosmos-sdk/x/staking/migrations/v040"
+)
+
+func getValidatorKey(operatorAddr sdk.ValAddress) []byte {
+	return append(v040staking.ValidatorsKey, address.MustLengthPrefix(operatorAddr)...)
+}

x/staking/migrations/v045/store.go

@@ -0,0 +1,57 @@
+package v045
+
+import (
+	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/prefix"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	v040staking "github.com/cosmos/cosmos-sdk/x/staking/migrations/v040"
+)
+
+// MigrateStore performs in-place store migrations from v0.43/v0.44 to v0.45.
+// The migration includes:
+//
+// - Removing delegations that have a zero share or token amount.
+func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
+	store := ctx.KVStore(storeKey)
+
+	return purgeDelegations(store, cdc)
+}
+
+func purgeDelegations(store sdk.KVStore, cdc codec.BinaryCodec) error {
+	prefixDelStore := prefix.NewStore(store, v040staking.DelegationKey)
+
+	delStoreIter := prefixDelStore.Iterator(nil, nil)
+	defer delStoreIter.Close()
+
+	valCache := make(map[string]v040staking.Validator)
+
+	for ; delStoreIter.Valid(); delStoreIter.Next() {
+		var delegation v040staking.Delegation
+		if err := cdc.Unmarshal(delStoreIter.Value(), &delegation); err != nil {
+			return err
+		}
+
+		validator, ok := valCache[delegation.ValidatorAddress]
+		if !ok {
+			valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
+			if err != nil {
+				return err
+			}
+
+			if err := cdc.Unmarshal(store.Get(getValidatorKey(valAddr)), &validator); err != nil {
+				return err
+			}
+
+			valCache[delegation.ValidatorAddress] = validator
+		}
+
+		// TODO: On-chain, we call BeforeDelegationRemoved prior to removing the
+		// object from state. Do we need to do the same here?
+		if validator.TokensFromShares(delegation.Shares).TruncateInt().IsZero() || delegation.Shares.IsZero() {
+			prefixDelStore.Delete(delStoreIter.Key())
+		}
+	}
+
+	return nil
+}

x/staking/module.go

@@ -24,6 +24,10 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
+const (
+	consensusVersion uint64 = 3
+)
+
 var (
 	_ module.AppModule           = AppModule{}
 	_ module.AppModuleBasic      = AppModuleBasic{}
@@ -140,6 +144,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
 
 	m := keeper.NewMigrator(am.keeper)
 	cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2)
+	cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3)
 }
 
 // InitGenesis performs genesis initialization for the staking module. It returns
@@ -160,7 +165,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw
 }
 
 // ConsensusVersion implements AppModule/ConsensusVersion.
-func (AppModule) ConsensusVersion() uint64 { return 2 }
+func (AppModule) ConsensusVersion() uint64 { return consensusVersion }
 
 // BeginBlock returns the begin blocker for the staking module.
 func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) {