fix: allow delegate only spendable coins

(cherry picked from commit 47933d0) (cherry picked from commit e28b9d8)
cosmosquad-labs · Apr 22, 2022 · d3297ed · d3297ed
1 parent c2fd51b
commit d3297ed
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 45 deletions.
diff --git a/x/auth/legacy/v043/store_test.go b/x/auth/legacy/v043/store_test.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting/exported"
@@ -110,14 +111,17 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
+				// failed because spendable balances is 100
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
+				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
 				require.NoError(t, err)
 			},
 			cleartTrackingFields,
 			300,
-			200,
 			100,
 			0,
+			0,
 		},
 		{
 			"delayed vesting has not vested, multiple delegations which exceed the vested amount",
@@ -131,16 +135,17 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
 				require.NoError(t, err)
+				// failed because of no more spendable balances
 				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 			},
 			cleartTrackingFields,
 			300,
-			200,
 			100,
 			0,
+			0,
 		},
 		{
 			"not end time",
@@ -151,16 +156,17 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
+				// failed because of no spendable balances
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 			},
 			cleartTrackingFields,
 			300,
-			300,
+			0,
 			0,
 			0,
 		},
@@ -173,12 +179,13 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
+				// failed because of no spendable balances
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 			},
 			cleartTrackingFields,
 			300,
-			300,
+			0,
 			0,
 			0,
 		},
@@ -218,11 +225,11 @@ func TestMigrateVestingAccounts(t *testing.T) {
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 			},
 			cleartTrackingFields,
 			300,
-			300,
+			0,
 			0,
 			0,
 		},
@@ -240,14 +247,17 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
+				// failed because spendable balances is 100
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
+				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(100), stakingtypes.Unbonded, validator, true)
 				require.NoError(t, err)
 			},
 			cleartTrackingFields,
 			300,
-			200,
 			100,
 			0,
+			0,
 		},
 		{
 			"continuous vesting, start time and endtime passed",
@@ -309,9 +319,8 @@ func TestMigrateVestingAccounts(t *testing.T) {
 					 - 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 vested: zero
 					 - expected free: zero
-					 - we're delegating the full original vesting
 				*/
 				startTime := int64(1601042400)
 				baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr)
@@ -335,13 +344,13 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
-				// delegation of the original vesting
+				// delegation of the original vesting, failed because of no spendable balances
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 			},
 			cleartTrackingFields,
 			3666666670000,
-			3666666670000,
+			0,
 			0,
 			1601042400 + 1,
 		},
@@ -399,9 +408,9 @@ func TestMigrateVestingAccounts(t *testing.T) {
 					 - 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
+					 - expected vested: first period amount
+					 - expected free: zero
+					 - we're delegating the vested amount
 				*/
 				startTime := int64(1601042400)
 				baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr)
@@ -427,14 +436,17 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
-				// delegation of the original vesting
+				// delegation of the original vesting, failed because of no spendable balances
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
+				// delegation of vested spendable amount
+				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(1833333335000), stakingtypes.Unbonded, validator, true)
 				require.NoError(t, err)
 			},
 			cleartTrackingFields,
 			3666666670000,
-			3666666670000 - 1833333335000,
 			1833333335000,
+			0,
 			1601042400 + 31536000 + 1,
 		},
 		{
@@ -445,9 +457,9 @@ func TestMigrateVestingAccounts(t *testing.T) {
 					 - 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
+					 - expected vested: amount of the second periods
+					 - expected free: amount of the second periods
+					 - we're delegating the first period have vested
 				*/
 				startTime := int64(1601042400)
 				baseAccount := authtypes.NewBaseAccountWithAddress(delegatorAddr)
@@ -473,14 +485,17 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
-				// delegation of the original vesting
+				// delegation of the original vesting, failed because of no spendable balances
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(3666666670000), stakingtypes.Unbonded, validator, true)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
+				// delegation of the first period have vested
+				_, err = app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(1833333335000), stakingtypes.Unbonded, validator, true)
 				require.NoError(t, err)
 			},
 			cleartTrackingFields,
 			3666666670000,
-			3666666670000 - 1833333335000 - 916666667500,
-			1833333335000 + 916666667500,
+			916666667500,
+			916666667500,
 			1601042400 + 31536000 + 15638400 + 1,
 		},
 		{
@@ -493,22 +508,15 @@ func TestMigrateVestingAccounts(t *testing.T) {
 
 				app.AccountKeeper.SetAccount(ctx, delayedAccount)
 
-				// delegation of the original vesting
+				// delegation of the original vesting, failed because of no spendable balances
 				_, err := app.StakingKeeper.Delegate(ctx, delegatorAddr, sdk.NewInt(300), stakingtypes.Unbonded, validator, true)
-				require.NoError(t, err)
+				require.ErrorIs(t, err, sdkerrors.ErrInsufficientFunds)
 
 				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,
 			0,
 		},

diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go
@@ -593,7 +593,7 @@ func (k Keeper) Delegate(
 		}
 
 		coins := sdk.NewCoins(sdk.NewCoin(k.BondDenom(ctx), bondAmt))
-		if err := k.bankKeeper.DelegateCoinsFromAccountToModule(ctx, delegatorAddress, sendName, coins); err != nil {
+		if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, delegatorAddress, sendName, coins); err != nil {
 			return sdk.Dec{}, err
 		}
 	} else {
@@ -779,7 +779,7 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delAddr sdk.AccAddress, valAd
 			// track undelegation only when remaining or truncated shares are non-zero
 			if !entry.Balance.IsZero() {
 				amt := sdk.NewCoin(bondDenom, entry.Balance)
-				if err := k.bankKeeper.UndelegateCoinsFromModuleToAccount(
+				if err := k.bankKeeper.SendCoinsFromModuleToAccount(
 					ctx, types.NotBondedPoolName, delegatorAddress, sdk.NewCoins(amt),
 				); err != nil {
 					return nil, err

diff --git a/x/staking/simulation/operations.go b/x/staking/simulation/operations.go
@@ -106,7 +106,7 @@ func SimulateMsgCreateValidator(ak types.AccountKeeper, bk types.BankKeeper, k k
 
 		denom := k.GetParams(ctx).BondDenom
 
-		balance := bk.GetBalance(ctx, simAccount.Address, denom).Amount
+		balance := bk.SpendableCoins(ctx, simAccount.Address).AmountOf(denom)
 		if !balance.IsPositive() {
 			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCreateValidator, "balance is negative"), nil, nil
 		}
@@ -248,7 +248,7 @@ func SimulateMsgDelegate(ak types.AccountKeeper, bk types.BankKeeper, k keeper.K
 			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgDelegate, "validator's invalid echange rate"), nil, nil
 		}
 
-		amount := bk.GetBalance(ctx, simAccount.Address, denom).Amount
+		amount := bk.SpendableCoins(ctx, simAccount.Address).AmountOf(denom)
 		if !amount.IsPositive() {
 			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgDelegate, "balance is negative"), nil, nil
 		}

diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go
@@ -33,8 +33,8 @@ type BankKeeper interface {
 	GetSupply(ctx sdk.Context, denom string) sdk.Coin
 
 	SendCoinsFromModuleToModule(ctx sdk.Context, senderPool, recipientPool string, amt sdk.Coins) error
-	UndelegateCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
-	DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
+	SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
+	SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
 
 	BurnCoins(ctx sdk.Context, name string, amt sdk.Coins) error
 }