Merge pull request #117 from persistenceOne/max/fix-ica-delegation

fix: ICA account delegation wasn't accounted. Added a test case.

x/staking/keeper/liquid_stake.go

@@ -124,36 +124,36 @@ func (k Keeper) DecreaseTotalLiquidStakedTokens(ctx sdk.Context, amount sdk.Int)
 
 // SafelyIncreaseValidatorTotalLiquidShares increments the total liquid shares on a validator, if:
 // the validator bond factor and validator liquid staking cap will not be exceeded by this delegation
-func (k Keeper) SafelyIncreaseValidatorTotalLiquidShares(ctx sdk.Context, validator types.Validator, shares sdk.Dec) error {
+func (k Keeper) SafelyIncreaseValidatorTotalLiquidShares(ctx sdk.Context, validator *types.Validator, shares sdk.Dec) error {
 	// Confirm the validator bond factor and validator liquid staking cap will be not exceeded
-	if k.CheckExceedsValidatorBondCap(ctx, validator, shares) {
+	if k.CheckExceedsValidatorBondCap(ctx, *validator, shares) {
 		return types.ErrInsufficientValidatorBondShares
 	}
-	if k.CheckExceedsValidatorLiquidStakingCap(ctx, validator, shares) {
+	if k.CheckExceedsValidatorLiquidStakingCap(ctx, *validator, shares) {
 		return types.ErrValidatorLiquidStakingCapExceeded
 	}
 
 	// Increment the validator's total liquid shares
 	validator.TotalLiquidShares = validator.TotalLiquidShares.Add(shares)
-	k.SetValidator(ctx, validator)
+	k.SetValidator(ctx, *validator)
 
 	return nil
 }
 
 // DecreaseValidatorTotalLiquidShares decrements the total liquid shares on a validator
-func (k Keeper) DecreaseValidatorTotalLiquidShares(ctx sdk.Context, validator types.Validator, shares sdk.Dec) error {
+func (k Keeper) DecreaseValidatorTotalLiquidShares(ctx sdk.Context, validator *types.Validator, shares sdk.Dec) error {
 	if shares.GT(validator.TotalLiquidShares) {
 		return types.ErrValidatorLiquidSharesUnderflow
 	}
 	validator.TotalLiquidShares = validator.TotalLiquidShares.Sub(shares)
-	k.SetValidator(ctx, validator)
+	k.SetValidator(ctx, *validator)
 	return nil
 }
 
 // SafelyDecreaseValidatorBond decrements the total validator's self bond
 // so long as it will not cause the current delegations to exceed the threshold
 // set by validator bond factor
-func (k Keeper) SafelyDecreaseValidatorBond(ctx sdk.Context, validator types.Validator, shares sdk.Dec) error {
+func (k Keeper) SafelyDecreaseValidatorBond(ctx sdk.Context, validator *types.Validator, shares sdk.Dec) error {
 	// Check if the decreased self bond will cause the validator bond threshold to be exceeded
 	validatorBondFactor := k.ValidatorBondFactor(ctx)
 	maxValTotalShare := validator.TotalValidatorBondShares.Sub(shares).Mul(validatorBondFactor)
@@ -163,7 +163,7 @@ func (k Keeper) SafelyDecreaseValidatorBond(ctx sdk.Context, validator types.Val
 
 	// Decrement the validator's total self bond
 	validator.TotalValidatorBondShares = validator.TotalValidatorBondShares.Sub(shares)
-	k.SetValidator(ctx, validator)
+	k.SetValidator(ctx, *validator)
 
 	return nil
 }

x/staking/keeper/liquid_stake_test.go

@@ -662,7 +662,8 @@ func TestSafelyIncreaseValidatorTotalLiquidShares(t *testing.T) {
 
 	// Attempt to increase the validator liquid shares, it should throw an
 	// error that the validator bond cap was exceeded
-	err := app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, initialValidator, firstIncreaseAmount)
+	validator := initialValidator
+	err := app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, firstIncreaseAmount)
 	require.ErrorIs(t, err, types.ErrInsufficientValidatorBondShares)
 	checkValidatorLiquidShares(initialLiquidShares, "shares after low bond factor")
 
@@ -671,13 +672,15 @@ func TestSafelyIncreaseValidatorTotalLiquidShares(t *testing.T) {
 	app.StakingKeeper.SetParams(ctx, params)
 
 	// Try the increase again and check that it succeeded
+	validator = initialValidator
 	expectedLiquidSharesAfterFirstStake := initialLiquidShares.Add(firstIncreaseAmount)
-	err = app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, initialValidator, firstIncreaseAmount)
+	err = app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, firstIncreaseAmount)
 	require.NoError(t, err)
 	checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares with cap loose bond cap")
 
 	// Attempt another increase, it should fail from the liquid staking cap
-	err = app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, initialValidator, secondIncreaseAmount)
+	validator = initialValidator
+	err = app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, secondIncreaseAmount)
 	require.ErrorIs(t, err, types.ErrValidatorLiquidStakingCapExceeded)
 	checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares after liquid staking cap hit")
 
@@ -686,8 +689,9 @@ func TestSafelyIncreaseValidatorTotalLiquidShares(t *testing.T) {
 	app.StakingKeeper.SetParams(ctx, params)
 
 	// Finally confirm that the increase succeeded this time
+	validator = initialValidator
 	expectedLiquidSharesAfterSecondStake := initialLiquidShares.Add(secondIncreaseAmount)
-	err = app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, initialValidator, secondIncreaseAmount)
+	err = app.StakingKeeper.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, secondIncreaseAmount)
 	require.NoError(t, err, "no error expected after increasing liquid staking cap")
 	checkValidatorLiquidShares(expectedLiquidSharesAfterSecondStake, "shares after loose liquid stake cap")
 }
@@ -711,15 +715,15 @@ func TestDecreaseValidatorTotalLiquidShares(t *testing.T) {
 	app.StakingKeeper.SetValidator(ctx, initialValidator)
 
 	// Decrease the validator liquid shares, and confirm the new share amount has been updated
-	err := app.StakingKeeper.DecreaseValidatorTotalLiquidShares(ctx, initialValidator, decreaseAmount)
+	err := app.StakingKeeper.DecreaseValidatorTotalLiquidShares(ctx, &initialValidator, decreaseAmount)
 	require.NoError(t, err, "no error expected when decreasing validator total liquid shares")
 
 	actualValidator, found := app.StakingKeeper.GetLiquidValidator(ctx, valAddress)
 	require.True(t, found)
 	require.Equal(t, initialLiquidShares.Sub(decreaseAmount), actualValidator.TotalLiquidShares, "total liquid shares")
 
 	// Attempt to decrease by a larger amount than it has, it should fail
-	err = app.StakingKeeper.DecreaseValidatorTotalLiquidShares(ctx, actualValidator, initialLiquidShares)
+	err = app.StakingKeeper.DecreaseValidatorTotalLiquidShares(ctx, &actualValidator, initialLiquidShares)
 	require.ErrorIs(t, err, types.ErrValidatorLiquidSharesUnderflow)
 }
 

x/staking/keeper/msg_server.go

@@ -236,7 +236,7 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ
 		if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil {
 			return nil, err
 		}
-		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, validator, shares); err != nil {
+		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, shares); err != nil {
 			return nil, err
 		}
 	}
@@ -322,7 +322,7 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
 
 	// if this is a validator self-bond, the new liquid delegation cannot fall below the self-bond * bond factor
 	if delegation.ValidatorBond {
-		if err := k.SafelyDecreaseValidatorBond(ctx, srcValidator, srcShares); err != nil {
+		if err := k.SafelyDecreaseValidatorBond(ctx, &srcValidator, srcShares); err != nil {
 			return nil, err
 		}
 	}
@@ -331,10 +331,10 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
 	// cannot exceed that validator's self-bond cap
 	// The liquid shares from the source validator should get moved to the destination validator
 	if k.AccountIsLiquidStakingProvider(delegatorAddress) {
-		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, dstValidator, dstShares); err != nil {
+		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, &dstValidator, dstShares); err != nil {
 			return nil, err
 		}
-		if err := k.DecreaseValidatorTotalLiquidShares(ctx, srcValidator, srcShares); err != nil {
+		if err := k.DecreaseValidatorTotalLiquidShares(ctx, &srcValidator, srcShares); err != nil {
 			return nil, err
 		}
 	}
@@ -421,7 +421,7 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (
 
 	// if this is a validator self-bond, the new liquid delegation cannot fall below the self-bond * bond factor
 	if delegation.ValidatorBond {
-		if err := k.SafelyDecreaseValidatorBond(ctx, validator, shares); err != nil {
+		if err := k.SafelyDecreaseValidatorBond(ctx, &validator, shares); err != nil {
 			return nil, err
 		}
 	}
@@ -432,7 +432,7 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (
 		if err := k.DecreaseTotalLiquidStakedTokens(ctx, tokens); err != nil {
 			return nil, err
 		}
-		if err := k.DecreaseValidatorTotalLiquidShares(ctx, validator, shares); err != nil {
+		if err := k.DecreaseValidatorTotalLiquidShares(ctx, &validator, shares); err != nil {
 			return nil, err
 		}
 	}
@@ -528,7 +528,7 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M
 		if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil {
 			return nil, err
 		}
-		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, validator, shares); err != nil {
+		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, shares); err != nil {
 			return nil, err
 		}
 	}
@@ -695,7 +695,7 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS
 		if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, msg.Amount.Amount, true); err != nil {
 			return nil, err
 		}
-		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, validator, shares); err != nil {
+		if err := k.SafelyIncreaseValidatorTotalLiquidShares(ctx, &validator, shares); err != nil {
 			return nil, err
 		}
 	}
@@ -821,7 +821,7 @@ func (k msgServer) RedeemTokens(goCtx context.Context, msg *types.MsgRedeemToken
 		if err := k.DecreaseTotalLiquidStakedTokens(ctx, tokens); err != nil {
 			return nil, err
 		}
-		if err := k.DecreaseValidatorTotalLiquidShares(ctx, validator, shares); err != nil {
+		if err := k.DecreaseValidatorTotalLiquidShares(ctx, &validator, shares); err != nil {
 			return nil, err
 		}
 	}

x/staking/keeper/msg_server_test.go

@@ -923,3 +923,76 @@ func TestUnbondValidator(t *testing.T) {
 	require.True(t, found)
 	require.True(t, validator.Jailed)
 }
+
+// TestICADelegateUndelegate tests that an ICA account can undelegate
+// sequentially right after delegating.
+func TestICADelegateUndelegate(t *testing.T) {
+	_, app, ctx := createTestInput(t)
+	msgServer := keeper.NewMsgServerImpl(app.StakingKeeper)
+
+	// Create a delegator and validator (the delegator will be an ICA account)
+	delegateAmount := sdk.NewInt(1000)
+	delegateCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegateAmount)
+	icaAccountAddress := createICAAccount(app, ctx, "ica-module-account")
+
+	// Fund ICA account
+	err := app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(delegateCoin))
+	require.NoError(t, err)
+	err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, icaAccountAddress, sdk.NewCoins(delegateCoin))
+	require.NoError(t, err)
+
+	addresses := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(0))
+	pubKeys := simapp.CreateTestPubKeys(1)
+	validatorAddress := sdk.ValAddress(addresses[0])
+	validator := teststaking.NewValidator(t, validatorAddress, pubKeys[0])
+
+	validator.DelegatorShares = sdk.NewDec(1_000_000)
+	validator.Tokens = sdk.NewInt(1_000_000)
+	validator.TotalLiquidShares = sdk.NewDec(0)
+	app.StakingKeeper.SetValidator(ctx, validator)
+
+	delegateMsg := types.MsgDelegate{
+		DelegatorAddress: icaAccountAddress.String(),
+		ValidatorAddress: validatorAddress.String(),
+		Amount:           delegateCoin,
+	}
+
+	undelegateMsg := types.MsgUndelegate{
+		DelegatorAddress: icaAccountAddress.String(),
+		ValidatorAddress: validatorAddress.String(),
+		Amount:           delegateCoin,
+	}
+
+	// Delegate normally
+	_, err = msgServer.Delegate(sdk.WrapSDKContext(ctx), &delegateMsg)
+	require.NoError(t, err, "no error expected when delegating")
+
+	// Confirm delegation record
+	_, found := app.StakingKeeper.GetDelegation(ctx, icaAccountAddress, validatorAddress)
+	require.True(t, found, "delegation should have been found")
+
+	// Confirm liquid staking totals were incremented
+	expectedTotalLiquidStaked := delegateAmount.Int64()
+	actualTotalLiquidStaked := app.StakingKeeper.GetTotalLiquidStakedTokens(ctx).Int64()
+	require.Equal(t, expectedTotalLiquidStaked, actualTotalLiquidStaked, "total liquid staked tokens after delegation")
+
+	validator, found = app.StakingKeeper.GetValidator(ctx, validatorAddress)
+	require.True(t, found, "validator should have been found")
+	require.Equal(t, delegateAmount.ToDec(), validator.TotalLiquidShares, "validator total liquid shares after delegation")
+
+	// Try to undelegate
+	_, err = msgServer.Undelegate(sdk.WrapSDKContext(ctx), &undelegateMsg)
+	require.NoError(t, err, "no error expected when sequentially undelegating")
+
+	// Confirm delegation record was removed
+	_, found = app.StakingKeeper.GetDelegation(ctx, icaAccountAddress, validatorAddress)
+	require.False(t, found, "delegation not have been found")
+
+	// Confirm liquid staking totals were decremented
+	actualTotalLiquidStaked = app.StakingKeeper.GetTotalLiquidStakedTokens(ctx).Int64()
+	require.Zero(t, actualTotalLiquidStaked, "total liquid staked tokens after undelegation")
+
+	validator, found = app.StakingKeeper.GetValidator(ctx, validatorAddress)
+	require.True(t, found, "validator should have been found")
+	require.Equal(t, sdk.ZeroDec(), validator.TotalLiquidShares, "validator total liquid shares after undelegation")
+}