From 8a37ba04a4824507b11772f3799b22a68f26827d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 22 Mar 2021 13:41:27 +0100 Subject: [PATCH] CreateValidator self delegation must be at least one consensus power (#8909) * update MsgCreateValidator ValidateBasic to enforce self delegation minimum MsgCreateValidator ValidateBasic requires a self delegation of at least one consensus power, this prevents a common, but hard to debug error * add changelog * Update CHANGELOG.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * fix simulation tests Construct successfuly MsgCreateValidator to use a self delegation which is greater than the PowerReduction * fix cli tests * fix cli tests Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: Alessio Treglia Co-authored-by: Marko Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- CHANGELOG.md | 1 + x/staking/client/cli/cli_test.go | 8 ++++---- x/staking/simulation/operations.go | 13 ++++++++++++- x/staking/types/msg.go | 5 +++++ x/staking/types/msg_test.go | 3 ++- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 628ee735fb..dda608c263 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (x/staking) [\#8909](https://github.com/cosmos/cosmos-sdk/pull/8909) Require self delegation in `MsgCreateValidator` to be at least one consensus power. * (x/bank) [\#8614](https://github.com/cosmos/cosmos-sdk/issues/8614) Add `Name` and `Symbol` fields to denom metadata * (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts * (crypto/types) [\#8600](https://github.com/cosmos/cosmos-sdk/pull/8600) `CompactBitArray`: optimize the `NumTrueBitsBefore` method and add an `Equal` method. diff --git a/x/staking/client/cli/cli_test.go b/x/staking/client/cli/cli_test.go index adaa825f92..fc270f430d 100644 --- a/x/staking/client/cli/cli_test.go +++ b/x/staking/client/cli/cli_test.go @@ -97,7 +97,7 @@ func (s *IntegrationTestSuite) TestNewCreateValidatorCmd() { val.ClientCtx, val.Address, newAddr, - sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(200))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10000000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), ) @@ -131,7 +131,7 @@ func (s *IntegrationTestSuite) TestNewCreateValidatorCmd() { { "invalid transaction (missing pubkey)", []string{ - fmt.Sprintf("--%s=100stake", cli.FlagAmount), + fmt.Sprintf("--%s=%dstake", cli.FlagAmount, sdk.PowerReduction.Int64()), fmt.Sprintf("--%s=AFAF00C4", cli.FlagIdentity), fmt.Sprintf("--%s=https://newvalidator.io", cli.FlagWebsite), fmt.Sprintf("--%s=contact@newvalidator.io", cli.FlagSecurityContact), @@ -151,7 +151,7 @@ func (s *IntegrationTestSuite) TestNewCreateValidatorCmd() { "invalid transaction (missing moniker)", []string{ fmt.Sprintf("--%s=%s", cli.FlagPubKey, consPubKey), - fmt.Sprintf("--%s=100stake", cli.FlagAmount), + fmt.Sprintf("--%s=%dstake", cli.FlagAmount, sdk.PowerReduction.Int64()), fmt.Sprintf("--%s=AFAF00C4", cli.FlagIdentity), fmt.Sprintf("--%s=https://newvalidator.io", cli.FlagWebsite), fmt.Sprintf("--%s=contact@newvalidator.io", cli.FlagSecurityContact), @@ -171,7 +171,7 @@ func (s *IntegrationTestSuite) TestNewCreateValidatorCmd() { "valid transaction", []string{ fmt.Sprintf("--%s=%s", cli.FlagPubKey, consPubKey), - fmt.Sprintf("--%s=100stake", cli.FlagAmount), + fmt.Sprintf("--%s=%dstake", cli.FlagAmount, sdk.PowerReduction.Int64()), fmt.Sprintf("--%s=NewValidator", cli.FlagMoniker), fmt.Sprintf("--%s=AFAF00C4", cli.FlagIdentity), fmt.Sprintf("--%s=https://newvalidator.io", cli.FlagWebsite), diff --git a/x/staking/simulation/operations.go b/x/staking/simulation/operations.go index cf99446b9f..558a63b4e7 100644 --- a/x/staking/simulation/operations.go +++ b/x/staking/simulation/operations.go @@ -112,11 +112,22 @@ func SimulateMsgCreateValidator(ak types.AccountKeeper, bk types.BankKeeper, k k return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCreateValidator, "balance is negative"), nil, nil } - amount, err := simtypes.RandPositiveInt(r, balance) + // A validator can only be created if it has at least one consensus power worth of tokens. + // Construct a maximum random value that is reduced by the PowerReduction, allowing + // for a minimum self delegation of the PowerReduction. + max := balance.Sub(sdk.PowerReduction) + if !max.IsPositive() { + return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCreateValidator, "balance doesn't have enough delegation amount"), nil, nil + } + + amount, err := simtypes.RandPositiveInt(r, max) if err != nil { return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgCreateValidator, "unable to generate positive amount"), nil, err } + // ensure self delegation meets minimum requirement + amount = amount.Add(sdk.PowerReduction) + selfDelegation := sdk.NewCoin(denom, amount) account := ak.GetAccount(ctx, simAccount.Address) diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index e397046e57..6cbb6e8254 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -140,6 +140,11 @@ func (msg MsgCreateValidator) ValidateBasic() error { return ErrMinSelfDelegationInvalid } + // ensure delegation is at least one consensus power + if sdk.TokensToConsensusPower(msg.Value.Amount) <= 0 { + return sdkerrors.Wrapf(ErrBadDelegationAmount, "self delegation amount (%s) must be at least one consensus power (%s)", msg.Value.Amount, sdk.PowerReduction) + } + if msg.Value.Amount.LT(msg.MinSelfDelegation) { return ErrSelfDelegationBelowMinimum } diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index 493434527c..7b11d96a74 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -15,7 +15,7 @@ import ( ) var ( - coinPos = sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000) + coinPos = sdk.NewInt64Coin(sdk.DefaultBondDenom, sdk.PowerReduction.Int64()) coinZero = sdk.NewInt64Coin(sdk.DefaultBondDenom, 0) ) @@ -75,6 +75,7 @@ func TestMsgCreateValidator(t *testing.T) { {"zero min self delegation", "a", "b", "c", "d", "e", commission1, sdk.ZeroInt(), valAddr1, pk1, coinPos, false}, {"negative min self delegation", "a", "b", "c", "d", "e", commission1, sdk.NewInt(-1), valAddr1, pk1, coinPos, false}, {"delegation less than min self delegation", "a", "b", "c", "d", "e", commission1, coinPos.Amount.Add(sdk.OneInt()), valAddr1, pk1, coinPos, false}, + {"delegation less than one consensus power", "a", "b", "c", "d", "e", commission1, sdk.OneInt(), valAddr1, pk1, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000), false}, } for _, tc := range tests {