From f4ce1ebbc5c833c823c7d84b5fffaf7dd10080f2 Mon Sep 17 00:00:00 2001 From: sampocs Date: Tue, 1 Aug 2023 14:31:35 -0500 Subject: [PATCH 1/3] increased validator bond shares during delegation --- x/staking/keeper/liquid_stake.go | 7 ++ x/staking/keeper/msg_server.go | 42 +++++++----- x/staking/keeper/msg_server_test.go | 100 ++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 15 deletions(-) diff --git a/x/staking/keeper/liquid_stake.go b/x/staking/keeper/liquid_stake.go index acde931f50c2..a0e9df1c974b 100644 --- a/x/staking/keeper/liquid_stake.go +++ b/x/staking/keeper/liquid_stake.go @@ -164,6 +164,13 @@ func (k Keeper) DecreaseValidatorLiquidShares(ctx sdk.Context, validator types.V return nil } +// Increase validator bond shares increments the validator's self bond +// in the event that the delegation amount on a validator bond delegation is increased +func (k Keeper) IncreaseValidatorBondShares(ctx sdk.Context, validator types.Validator, shares sdk.Dec) { + validator.ValidatorBondShares = validator.ValidatorBondShares.Add(shares) + k.SetValidator(ctx, validator) +} + // SafelyDecreaseValidatorBond decrements the validator's self bond // so long as it will not cause the current delegations to exceed the threshold // set by validator bond factor diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 8e9e4279a634..5cb088601dda 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -205,6 +205,16 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ tokens := msg.Amount.Amount + // If the delegation exists already and is a validator bond, increment the validator bond shares + delegation, found := k.Keeper.GetDelegation(ctx, delegatorAddress, valAddr) + if found && delegation.ValidatorBond { + shares, err := validator.SharesFromTokens(tokens) + if err != nil { + return nil, err + } + k.IncreaseValidatorBondShares(ctx, validator, shares) + } + // if this delegation is from a liquid staking provider (identified if the delegator // is an ICA account), it cannot exceed the global or validator bond cap if k.DelegatorIsLiquidStaker(delegatorAddress) { @@ -218,12 +228,13 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ if err := k.SafelyIncreaseValidatorLiquidShares(ctx, validator, shares); err != nil { return nil, err } - // Note: this is required for downstream uses of the validator variable - // since the validator's liquid shares were updated above - validator, found = k.GetValidator(ctx, valAddr) - if !found { - return nil, types.ErrNoValidatorFound - } + } + + // Note: this is required for downstream uses of the validator variable + // since the validator's liquid shares were updated above + validator, found = k.GetValidator(ctx, valAddr) + if !found { + return nil, types.ErrNoValidatorFound } // NOTE: source funds are always unbonded @@ -300,12 +311,9 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed if err != nil { return nil, err } - dstShares, err := dstValidator.SharesFromTokensTruncated(msg.Amount.Amount) - if err != nil { - return nil, err - } - // if this is a validator self-bond, the new liquid delegation cannot fall below the self-bond * bond factor + // If this is a validator self-bond, the new liquid delegation cannot fall below the self-bond * bond factor + // The delegation on the new validator will not a validator bond if delegation.ValidatorBond { if err := k.SafelyDecreaseValidatorBond(ctx, srcValidator, srcShares); err != nil { return nil, err @@ -322,6 +330,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.DelegatorIsLiquidStaker(delegatorAddress) { + dstShares, err := dstValidator.SharesFromTokensTruncated(msg.Amount.Amount) + if err != nil { + return nil, err + } if err := k.SafelyIncreaseValidatorLiquidShares(ctx, dstValidator, dstShares); err != nil { return nil, err } @@ -547,11 +559,11 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M // if this undelegation was from a liquid staking provider (identified if the delegator // is an ICA account), the global and validator liquid totals should be incremented tokens := msg.Amount.Amount - shares, err := validator.SharesFromTokens(tokens) - if err != nil { - return nil, err - } if k.DelegatorIsLiquidStaker(delegatorAddress) { + shares, err := validator.SharesFromTokens(tokens) + if err != nil { + return nil, err + } if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil { return nil, err } diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 545365ee5cff..c838871dcf3e 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -993,6 +993,106 @@ func TestValidatorBond(t *testing.T) { } } +func TestChangeValidatorBond(t *testing.T) { + _, app, ctx := createTestInput() + msgServer := keeper.NewMsgServerImpl(app.StakingKeeper) + + checkValidatorBondShares := func(validatorAddress sdk.ValAddress, expectedShares sdk.Int) { + validator, found := app.StakingKeeper.GetValidator(ctx, validatorAddress) + require.True(t, found, "validator should have been found") + require.Equal(t, expectedShares.Int64(), validator.ValidatorBondShares.TruncateInt64(), "validator bond shares") + } + + // Create a delegator and 2 validators + addresses := simapp.AddTestAddrs(app, ctx, 3, sdk.NewInt(1_000_000)) + pubKeys := simapp.CreateTestPubKeys(3) + + validatorAPubKey := pubKeys[1] + validatorBPubKey := pubKeys[2] + + delegatorAddress := addresses[0] + validatorAAddress := sdk.ValAddress(validatorAPubKey.Address()) + validatorBAddress := sdk.ValAddress(validatorBPubKey.Address()) + + validatorA := teststaking.NewValidator(t, validatorAAddress, validatorAPubKey) + validatorB := teststaking.NewValidator(t, validatorBAddress, validatorBPubKey) + + validatorA.Tokens = sdk.NewInt(1_000_000) + validatorB.Tokens = sdk.NewInt(1_000_000) + validatorA.DelegatorShares = sdk.NewDec(1_000_000) + validatorB.DelegatorShares = sdk.NewDec(1_000_000) + + app.StakingKeeper.SetValidator(ctx, validatorA) + app.StakingKeeper.SetValidator(ctx, validatorB) + + // The test will go through Delegate/Redelegate/Undelegate messages with the following + delegation1Amount := sdk.NewInt(1000) + delegation2Amount := sdk.NewInt(1000) + redelegateAmount := sdk.NewInt(500) + undelegateAmount := sdk.NewInt(500) + + delegate1Coin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegation1Amount) + delegate2Coin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), delegation2Amount) + redelegateCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), redelegateAmount) + undelegateCoin := sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), undelegateAmount) + + // Delegate to validator A - validator bond shares should not change + _, err := msgServer.Delegate(sdk.WrapSDKContext(ctx), &types.MsgDelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAAddress.String(), + Amount: delegate1Coin, + }) + require.NoError(t, err, "no error expected during first delegation") + + checkValidatorBondShares(validatorAAddress, sdk.ZeroInt()) + checkValidatorBondShares(validatorBAddress, sdk.ZeroInt()) + + // Flag the delegation as a validator bond + _, err = msgServer.ValidatorBond(sdk.WrapSDKContext(ctx), &types.MsgValidatorBond{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAAddress.String(), + }) + require.NoError(t, err, "no error expected during validator bond") + + checkValidatorBondShares(validatorAAddress, delegation1Amount) + checkValidatorBondShares(validatorBAddress, sdk.ZeroInt()) + + // Delegate more - it should increase the validator bond shares + _, err = msgServer.Delegate(sdk.WrapSDKContext(ctx), &types.MsgDelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAAddress.String(), + Amount: delegate2Coin, + }) + require.NoError(t, err, "no error expected during second delegation") + + checkValidatorBondShares(validatorAAddress, delegation1Amount.Add(delegation2Amount)) + checkValidatorBondShares(validatorBAddress, sdk.ZeroInt()) + + // Redelegate partially from A to B - it should remove the bond shares from the source validator + _, err = msgServer.BeginRedelegate(sdk.WrapSDKContext(ctx), &types.MsgBeginRedelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorSrcAddress: validatorAAddress.String(), + ValidatorDstAddress: validatorBAddress.String(), + Amount: redelegateCoin, + }) + require.NoError(t, err, "no error expected during redelegation") + + checkValidatorBondShares(validatorAAddress, delegation1Amount.Add(delegation2Amount).Sub(redelegateAmount)) + checkValidatorBondShares(validatorBAddress, sdk.ZeroInt()) + + // Undelegate from validator A - it should have removed the shares + _, err = msgServer.Undelegate(sdk.WrapSDKContext(ctx), &types.MsgUndelegate{ + DelegatorAddress: delegatorAddress.String(), + ValidatorAddress: validatorAAddress.String(), + Amount: undelegateCoin, + }) + require.NoError(t, err, "no error expected during undelegation") + + expectedBondShares := delegation1Amount.Add(delegation2Amount).Sub(redelegateAmount).Sub(undelegateAmount) + checkValidatorBondShares(validatorAAddress, expectedBondShares) + checkValidatorBondShares(validatorBAddress, sdk.ZeroInt()) +} + func TestEnableDisableTokenizeShares(t *testing.T) { _, app, ctx := createTestInput() msgServer := keeper.NewMsgServerImpl(app.StakingKeeper) From 321f888b2500830b2916503224e45b8b12f53687 Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 2 Aug 2023 21:02:27 -0500 Subject: [PATCH 2/3] moved validator bond check after keeper.Delegate --- x/staking/keeper/msg_server.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 5cb088601dda..ada8e5ea09a5 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -205,16 +205,6 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ tokens := msg.Amount.Amount - // If the delegation exists already and is a validator bond, increment the validator bond shares - delegation, found := k.Keeper.GetDelegation(ctx, delegatorAddress, valAddr) - if found && delegation.ValidatorBond { - shares, err := validator.SharesFromTokens(tokens) - if err != nil { - return nil, err - } - k.IncreaseValidatorBondShares(ctx, validator, shares) - } - // if this delegation is from a liquid staking provider (identified if the delegator // is an ICA account), it cannot exceed the global or validator bond cap if k.DelegatorIsLiquidStaker(delegatorAddress) { @@ -243,6 +233,15 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ return nil, err } + // If the delegation is a validator bond, increment the validator bond shares + delegation, found := k.Keeper.GetDelegation(ctx, delegatorAddress, valAddr) + if !found { + return nil, types.ErrNoDelegation + } + if delegation.ValidatorBond { + k.IncreaseValidatorBondShares(ctx, validator, newShares) + } + if tokens.IsInt64() { defer func() { telemetry.IncrCounter(1, types.ModuleName, "delegate") From 160611ecf817810ac382adb65b84d0ba1c00c152 Mon Sep 17 00:00:00 2001 From: sampocs Date: Wed, 2 Aug 2023 21:21:31 -0500 Subject: [PATCH 3/3] moved validator lookup back under if statement --- x/staking/keeper/msg_server.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index ada8e5ea09a5..ffdf1bcf6019 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -218,13 +218,12 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ if err := k.SafelyIncreaseValidatorLiquidShares(ctx, validator, shares); err != nil { return nil, err } - } - - // Note: this is required for downstream uses of the validator variable - // since the validator's liquid shares were updated above - validator, found = k.GetValidator(ctx, valAddr) - if !found { - return nil, types.ErrNoValidatorFound + // Note: this is required for downstream uses of the validator variable + // since the validator's liquid shares were updated above + validator, found = k.GetValidator(ctx, valAddr) + if !found { + return nil, types.ErrNoValidatorFound + } } // NOTE: source funds are always unbonded