From 81b76cbb599d5ec1b2f85eea8719b01c67ffab45 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 12 Apr 2023 20:45:00 +0530 Subject: [PATCH 01/14] wip: move ValidateBasic() logic tomsg_server --- x/staking/client/cli/tx.go | 3 - x/staking/keeper/msg_server.go | 35 +++ x/staking/keeper/msg_server_test.go | 250 ++++++++++++++++++- x/staking/testutil/expected_keepers_mocks.go | 28 +++ x/staking/types/expected_keepers.go | 2 + x/staking/types/msg.go | 50 ---- x/staking/types/msg_test.go | 181 +------------- 7 files changed, 315 insertions(+), 234 deletions(-) diff --git a/x/staking/client/cli/tx.go b/x/staking/client/cli/tx.go index 9453c10aef1a..6b26f60a3bda 100644 --- a/x/staking/client/cli/tx.go +++ b/x/staking/client/cli/tx.go @@ -379,9 +379,6 @@ func newBuildCreateValidatorMsg(clientCtx client.Context, txf tx.Factory, fs *fl if err != nil { return txf, nil, err } - if err := msg.ValidateBasic(); err != nil { - return txf, nil, err - } genOnly, _ := fs.GetBool(flags.FlagGenerateOnly) if genOnly { diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 8f97e1d0ac7a..20ee0be37dcb 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -37,9 +37,40 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) + } + + if msg.Pubkey == nil { + return nil, types.ErrEmptyValidatorPubKey + } + + if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount") + } + + if msg.Description == (types.Description{}) { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description") + } + + if msg.Commission == (types.CommissionRates{}) { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission") + } + + if err := msg.Commission.Validate(); err != nil { return nil, err } + if !msg.MinSelfDelegation.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "minimum self delegation must be a positive integer", + ) + } + + if msg.Value.Amount.LT(msg.MinSelfDelegation) { + return nil, types.ErrSelfDelegationBelowMinimum + } + if msg.Commission.Rate.LT(k.MinCommissionRate(ctx)) { return nil, errorsmod.Wrapf(types.ErrCommissionLTMinRate, "cannot set validator commission to less than minimum rate of %s", k.MinCommissionRate(ctx)) } @@ -482,6 +513,10 @@ func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParam return nil, errorsmod.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority, msg.Authority) } + if err := msg.Params.Validate(); err != nil { + return nil, err + } + // store params if err := k.SetParams(ctx, msg.Params); err != nil { return nil, err diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 87a7dff90936..4b14ee82689d 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -2,11 +2,243 @@ package keeper_test import ( "testing" + "time" "cosmossdk.io/math" + + "github.com/golang/mock/gomock" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) +var PKS = simtestutil.CreateTestPubKeys(3) + +func (s *KeeperTestSuite) TestMsgCreateValidator() { + ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer + require := s.Require() + + delAddr := sdk.AccAddress(PKS[0].Address()) + pk1 := ed25519.GenPrivKey().PubKey() + require.NotNil(pk1) + + authority := authtypes.NewModuleAddress(govtypes.ModuleName) + valAddr := sdk.ValAddress(authority) + + s.bankKeeper.EXPECT().MintCoins(gomock.Any(), stakingtypes.ModuleName, gomock.Any()).AnyTimes() + amt := keeper.TokensFromConsensusPower(s.ctx, int64(100)) + s.bankKeeper.MintCoins(s.ctx, stakingtypes.ModuleName, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, amt))) + + s.bankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), stakingtypes.ModuleName, delAddr, gomock.Any()).AnyTimes() + s.bankKeeper.SendCoinsFromModuleToAccount(s.ctx, stakingtypes.ModuleName, delAddr, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, amt))) + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), authority, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + pubkey, err := codectypes.NewAnyWithValue(pk1) + require.NoError(err) + + testCases := []struct { + name string + input *stakingtypes.MsgCreateValidator + expErr bool + expErrMsg string + }{ + { + name: "empty description", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{}, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 10000), + }, + expErr: true, + expErrMsg: "empty description", + }, + { + name: "invalid validator address", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 10000), + }, + expErr: true, + expErrMsg: "invalid validator address", + }, + { + name: "empty validator pubkey", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: nil, + Value: sdk.NewInt64Coin("stake", 10000), + }, + expErr: true, + expErrMsg: "empty validator public key", + }, + { + name: "empty delegation amount", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 0), + }, + expErr: true, + expErrMsg: "invalid delegation amount", + }, + { + name: "nil delegation amount", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.Coin{}, + }, + expErr: true, + expErrMsg: "invalid delegation amount", + }, + { + name: "zero minimum self delegation", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(0), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 10000), + }, + expErr: true, + expErrMsg: "minimum self delegation must be a positive integer", + }, + { + name: "negative minimum self delegation", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(-1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 10000), + }, + expErr: true, + expErrMsg: "minimum self delegation must be a positive integer", + }, + { + name: "delegation less than minimum self delegation", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(100), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 10), + }, + expErr: true, + expErrMsg: "validator's self delegation must be greater than their minimum self delegation", + }, + { + name: "valid msg", + input: &stakingtypes.MsgCreateValidator{ + Description: stakingtypes.Description{ + Moniker: "NewValidator", + }, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(5, 1), + MaxRate: sdk.NewDecWithPrec(5, 1), + MaxChangeRate: math.LegacyNewDec(0), + }, + MinSelfDelegation: math.NewInt(1), + DelegatorAddress: delAddr.String(), + ValidatorAddress: valAddr.String(), + Pubkey: pubkey, + Value: sdk.NewInt64Coin("stake", 10000), + }, + expErr: false, + }, + } + for _, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + _, err := msgServer.CreateValidator(ctx, tc.input) + if tc.expErr { + require.Error(err) + require.Contains(err.Error(), tc.expErrMsg) + } else { + require.NoError(err) + } + }) + } +} + func (s *KeeperTestSuite) TestMsgUpdateParams() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() @@ -83,7 +315,7 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() { expErrMsg: "bond denom cannot be blank", }, { - name: "max validators most be positive", + name: "max validators must be positive", input: &stakingtypes.MsgUpdateParams{ Authority: keeper.GetAuthority(), Params: stakingtypes.Params{ @@ -114,6 +346,22 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() { expErr: true, expErrMsg: "max entries must be positive", }, + { + name: "negative unbounding time", + input: &stakingtypes.MsgUpdateParams{ + Authority: keeper.GetAuthority(), + Params: stakingtypes.Params{ + UnbondingTime: time.Hour * 24 * 7 * 3 * -1, + MaxEntries: stakingtypes.DefaultMaxEntries, + MaxValidators: stakingtypes.DefaultMaxValidators, + HistoricalEntries: stakingtypes.DefaultHistoricalEntries, + MinCommissionRate: stakingtypes.DefaultMinCommissionRate, + BondDenom: "denom", + }, + }, + expErr: true, + expErrMsg: "unbonding time must be positive", + }, } for _, tc := range testCases { diff --git a/x/staking/testutil/expected_keepers_mocks.go b/x/staking/testutil/expected_keepers_mocks.go index ba3036da7561..fa94ee7d1253 100644 --- a/x/staking/testutil/expected_keepers_mocks.go +++ b/x/staking/testutil/expected_keepers_mocks.go @@ -291,6 +291,34 @@ func (mr *MockBankKeeperMockRecorder) LockedCoins(ctx, addr interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockedCoins", reflect.TypeOf((*MockBankKeeper)(nil).LockedCoins), ctx, addr) } +// MintCoins mocks base method. +func (m *MockBankKeeper) MintCoins(ctx types.Context, moduleName string, amt types.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MintCoins", ctx, moduleName, amt) + ret0, _ := ret[0].(error) + return ret0 +} + +// MintCoins indicates an expected call of MintCoins. +func (mr *MockBankKeeperMockRecorder) MintCoins(ctx, moduleName, amt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MintCoins", reflect.TypeOf((*MockBankKeeper)(nil).MintCoins), ctx, moduleName, amt) +} + +// SendCoinsFromModuleToAccount mocks base method. +func (m *MockBankKeeper) SendCoinsFromModuleToAccount(ctx types.Context, senderModule string, recipientAddr types.AccAddress, amt types.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SendCoinsFromModuleToAccount", ctx, senderModule, recipientAddr, amt) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendCoinsFromModuleToAccount indicates an expected call of SendCoinsFromModuleToAccount. +func (mr *MockBankKeeperMockRecorder) SendCoinsFromModuleToAccount(ctx, senderModule, recipientAddr, amt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromModuleToAccount", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromModuleToAccount), ctx, senderModule, recipientAddr, amt) +} + // SendCoinsFromModuleToModule mocks base method. func (m *MockBankKeeper) SendCoinsFromModuleToModule(ctx types.Context, senderPool, recipientPool string, amt types.Coins) error { m.ctrl.T.Helper() diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go index b70505d0ec30..a4e8c80e7ff4 100644 --- a/x/staking/types/expected_keepers.go +++ b/x/staking/types/expected_keepers.go @@ -36,8 +36,10 @@ type BankKeeper interface { LockedCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins + MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error GetSupply(ctx sdk.Context, denom string) sdk.Coin + SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error 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 diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 5a8a765020e3..1a6e75688f49 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -71,48 +71,6 @@ func (msg MsgCreateValidator) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgCreateValidator) ValidateBasic() error { - // note that unmarshaling from bech32 ensures both non-empty and valid - _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) - if err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) - } - - if msg.Pubkey == nil { - return ErrEmptyValidatorPubKey - } - - if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount") - } - - if msg.Description == (Description{}) { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description") - } - - if msg.Commission == (CommissionRates{}) { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission") - } - - if err := msg.Commission.Validate(); err != nil { - return err - } - - if !msg.MinSelfDelegation.IsPositive() { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "minimum self delegation must be a positive integer", - ) - } - - if msg.Value.Amount.LT(msg.MinSelfDelegation) { - return ErrSelfDelegationBelowMinimum - } - - return nil -} - // UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces func (msg MsgCreateValidator) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { var pubKey cryptotypes.PubKey @@ -347,14 +305,6 @@ func (m MsgUpdateParams) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic executes sanity validation on the provided data -func (m MsgUpdateParams) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(m.Authority); err != nil { - return errorsmod.Wrap(err, "invalid authority address") - } - return m.Params.Validate() -} - // GetSigners returns the expected signers for a MsgUpdateParams message func (m MsgUpdateParams) GetSigners() []sdk.AccAddress { addr, _ := sdk.AccAddressFromBech32(m.Authority) diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index 428e25c95594..640f262008ce 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -2,9 +2,9 @@ package types_test import ( "testing" - "time" "cosmossdk.io/math" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -56,44 +56,6 @@ func TestMsgDecode(t *testing.T) { require.True(t, msg.Pubkey.Equal(msg2.Pubkey)) } -// test ValidateBasic for MsgCreateValidator -func TestMsgCreateValidator(t *testing.T) { - commission1 := types.NewCommissionRates(math.LegacyZeroDec(), math.LegacyZeroDec(), math.LegacyZeroDec()) - commission2 := types.NewCommissionRates(math.LegacyNewDec(5), math.LegacyNewDec(5), math.LegacyNewDec(5)) - - tests := []struct { - name, moniker, identity, website, securityContact, details string - CommissionRates types.CommissionRates - minSelfDelegation math.Int - validatorAddr sdk.ValAddress - pubkey cryptotypes.PubKey - bond sdk.Coin - expectPass bool - }{ - {"basic good", "a", "b", "c", "d", "e", commission1, math.OneInt(), valAddr1, pk1, coinPos, true}, - {"partial description", "", "", "c", "", "", commission1, math.OneInt(), valAddr1, pk1, coinPos, true}, - {"empty description", "", "", "", "", "", commission2, math.OneInt(), valAddr1, pk1, coinPos, false}, - {"empty address", "a", "b", "c", "d", "e", commission2, math.OneInt(), emptyAddr, pk1, coinPos, false}, - {"empty pubkey", "a", "b", "c", "d", "e", commission1, math.OneInt(), valAddr1, emptyPubkey, coinPos, false}, - {"empty bond", "a", "b", "c", "d", "e", commission2, math.OneInt(), valAddr1, pk1, coinZero, false}, - {"nil bond", "a", "b", "c", "d", "e", commission2, math.OneInt(), valAddr1, pk1, sdk.Coin{}, false}, - {"zero min self delegation", "a", "b", "c", "d", "e", commission1, math.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(math.OneInt()), valAddr1, pk1, coinPos, false}, - } - - for _, tc := range tests { - description := types.NewDescription(tc.moniker, tc.identity, tc.website, tc.securityContact, tc.details) - msg, err := types.NewMsgCreateValidator(tc.validatorAddr, tc.pubkey, tc.bond, description, tc.CommissionRates, tc.minSelfDelegation) - require.NoError(t, err) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", tc.name) - } - } -} - // test ValidateBasic for MsgEditValidator func TestMsgEditValidator(t *testing.T) { tests := []struct { @@ -211,144 +173,3 @@ func TestMsgUpdateParams(t *testing.T) { require.Equal(t, []sdk.AccAddress{authtypes.NewModuleAddress(govtypes.ModuleName)}, msg.GetSigners()) } - -func TestMsgUpdateParamsValidateBasic(t *testing.T) { - tests := []struct { - name string - msgUpdateParams types.MsgUpdateParams - expFail bool - expError string - }{ - { - "valid msg", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.DefaultParams(), - }, - false, - "", - }, - { - "negative unbounding time", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3 * -1, - MaxEntries: types.DefaultMaxEntries, - MaxValidators: types.DefaultMaxValidators, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: types.DefaultMinCommissionRate, - BondDenom: "denom", - }, - }, - true, - "unbonding time must be positive:", - }, - { - "cero value max validator", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3, - MaxEntries: types.DefaultMaxEntries, - MaxValidators: 0, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: types.DefaultMinCommissionRate, - BondDenom: "denom", - }, - }, - true, - "max validators must be positive:", - }, - { - "cero value max validator", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3, - MaxEntries: 0, - MaxValidators: types.DefaultMaxValidators, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: types.DefaultMinCommissionRate, - BondDenom: "denom", - }, - }, - true, - "max entries must be positive:", - }, - { - "negative commission rate", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3, - MaxEntries: types.DefaultMaxEntries, - MaxValidators: types.DefaultMaxValidators, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: math.LegacyNewDec(-1), - BondDenom: "denom", - }, - }, - true, - "minimum commission rate cannot be negative:", - }, - { - "negative commission rate", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3, - MaxEntries: types.DefaultMaxEntries, - MaxValidators: types.DefaultMaxValidators, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: math.LegacyNewDec(2), - BondDenom: "denom", - }, - }, - true, - "minimum commission rate cannot be greater than 100", - }, - { - "blank bonddenom", - types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3, - MaxEntries: types.DefaultMaxEntries, - MaxValidators: types.DefaultMaxValidators, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: types.DefaultMinCommissionRate, - BondDenom: "", - }, - }, - true, - "bond denom cannot be blank", - }, - { - "Invalid authority", - types.MsgUpdateParams{ - Authority: "invalid", - Params: types.Params{ - UnbondingTime: time.Hour * 24 * 7 * 3, - MaxEntries: types.DefaultMaxEntries, - MaxValidators: types.DefaultMaxValidators, - HistoricalEntries: types.DefaultHistoricalEntries, - MinCommissionRate: types.DefaultMinCommissionRate, - BondDenom: "denom", - }, - }, - true, - "invalid authority address", - }, - } - - for _, tc := range tests { - err := tc.msgUpdateParams.ValidateBasic() - if tc.expFail { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expError) - } else { - require.NoError(t, err) - } - } -} From ff32a6f7361c31467a762b405adc6d369963e4c7 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 10:47:40 +0530 Subject: [PATCH 02/14] fix tests --- testutil/testnet/genesis.go | 33 ++++++++++++++++++++++- x/distribution/testutil/staking_helper.go | 3 ++- x/staking/testutil/helpers.go | 2 +- x/staking/types/data_test.go | 4 +-- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/testutil/testnet/genesis.go b/testutil/testnet/genesis.go index b5b985a98fb3..2a2510ae825e 100644 --- a/testutil/testnet/genesis.go +++ b/testutil/testnet/genesis.go @@ -6,12 +6,15 @@ import ( "strconv" "time" + errorsmod "cosmossdk.io/errors" cmttypes "github.com/cometbft/cometbft/types" + "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/cosmos/cosmos-sdk/x/auth/tx" @@ -23,6 +26,7 @@ import ( genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) @@ -117,10 +121,37 @@ func (b *GenesisBuilder) GenTx(privVal secp256k1.PrivKey, val cmttypes.GenesisVa msg.DelegatorAddress = sdk.AccAddress(valAddr).String() - if err := msg.ValidateBasic(); err != nil { + if msg.Pubkey == nil { + panic(types.ErrEmptyValidatorPubKey) + } + + if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { + panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount")) + } + + if msg.Description == (types.Description{}) { + panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description")) + } + + if msg.Commission == (types.CommissionRates{}) { + panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission")) + } + + if err := msg.Commission.Validate(); err != nil { panic(err) } + if !msg.MinSelfDelegation.IsPositive() { + panic(errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "minimum self delegation must be a positive integer", + )) + } + + if msg.Value.Amount.LT(msg.MinSelfDelegation) { + panic(types.ErrSelfDelegationBelowMinimum) + } + txConf := authtx.NewTxConfig(b.codec, tx.DefaultSignModes) txb := txConf.NewTxBuilder() diff --git a/x/distribution/testutil/staking_helper.go b/x/distribution/testutil/staking_helper.go index 9e1b259a069c..a568898aabf3 100644 --- a/x/distribution/testutil/staking_helper.go +++ b/x/distribution/testutil/staking_helper.go @@ -4,6 +4,7 @@ import ( "fmt" "cosmossdk.io/math" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/keeper" @@ -12,7 +13,7 @@ import ( func CreateValidator(pk cryptotypes.PubKey, stake math.Int) (stakingtypes.Validator, error) { valConsAddr := sdk.GetConsAddress(pk) - val, err := stakingtypes.NewValidator(sdk.ValAddress(valConsAddr), pk, stakingtypes.Description{}) + val, err := stakingtypes.NewValidator(sdk.ValAddress(valConsAddr), pk, stakingtypes.Description{Moniker: "TestValidator"}) val.Tokens = stake val.DelegatorShares = math.LegacyNewDecFromInt(val.Tokens) return val, err diff --git a/x/staking/testutil/helpers.go b/x/staking/testutil/helpers.go index 3dae28b09a06..532779bead54 100644 --- a/x/staking/testutil/helpers.go +++ b/x/staking/testutil/helpers.go @@ -62,7 +62,7 @@ func (sh *Helper) CreateValidatorWithMsg(ctx context.Context, msg *stakingtypes. } func (sh *Helper) createValidator(addr sdk.ValAddress, pk cryptotypes.PubKey, coin sdk.Coin, ok bool) { - msg, err := stakingtypes.NewMsgCreateValidator(addr, pk, coin, stakingtypes.Description{}, sh.Commission, math.OneInt()) + msg, err := stakingtypes.NewMsgCreateValidator(addr, pk, coin, stakingtypes.Description{Moniker: "TestValidator"}, sh.Commission, math.OneInt()) require.NoError(sh.t, err) res, err := sh.msgSrvr.CreateValidator(sh.Ctx, msg) if ok { diff --git a/x/staking/types/data_test.go b/x/staking/types/data_test.go index d875b088fdbb..ba010f47cbe7 100644 --- a/x/staking/types/data_test.go +++ b/x/staking/types/data_test.go @@ -5,7 +5,6 @@ import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -18,8 +17,7 @@ var ( valAddr2 = sdk.ValAddress(pk2.Address()) valAddr3 = sdk.ValAddress(pk3.Address()) - emptyAddr sdk.ValAddress - emptyPubkey cryptotypes.PubKey + emptyAddr sdk.ValAddress ) func init() { From f557f5f07effaf13bcc83ad8ccfa18cf54ac0c27 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 12:44:00 +0530 Subject: [PATCH 03/14] wip: updates --- .../slashing/keeper/keeper_test.go | 1 + testutil/testnet/genesis.go | 9 +- x/staking/keeper/msg_server.go | 21 +- x/staking/keeper/msg_server_test.go | 186 +++++++++++++++++- x/staking/types/msg.go | 26 --- x/staking/types/msg_test.go | 28 --- 6 files changed, 210 insertions(+), 61 deletions(-) diff --git a/tests/integration/slashing/keeper/keeper_test.go b/tests/integration/slashing/keeper/keeper_test.go index 0fb2cb66c293..c876662af2f7 100644 --- a/tests/integration/slashing/keeper/keeper_test.go +++ b/tests/integration/slashing/keeper/keeper_test.go @@ -87,6 +87,7 @@ func TestUnJailNotBonded(t *testing.T) { amt := f.stakingKeeper.TokensFromConsensusPower(f.ctx, 50) msg := tstaking.CreateValidatorMsg(addr, val, amt) msg.MinSelfDelegation = amt + msg.Description = stakingtypes.Description{Moniker: "TestValidator"} res, err := tstaking.CreateValidatorWithMsg(f.ctx, msg) assert.NilError(t, err) assert.Assert(t, res != nil) diff --git a/testutil/testnet/genesis.go b/testutil/testnet/genesis.go index 07d27a5f8ddb..7eaa2a75b14a 100644 --- a/testutil/testnet/genesis.go +++ b/testutil/testnet/genesis.go @@ -26,7 +26,6 @@ import ( genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" - "github.com/cosmos/cosmos-sdk/x/staking/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) @@ -120,18 +119,18 @@ func (b *GenesisBuilder) GenTx(privVal secp256k1.PrivKey, val cmttypes.GenesisVa } if msg.Pubkey == nil { - panic(types.ErrEmptyValidatorPubKey) + panic(stakingtypes.ErrEmptyValidatorPubKey) } if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount")) } - if msg.Description == (types.Description{}) { + if msg.Description == (stakingtypes.Description{}) { panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description")) } - if msg.Commission == (types.CommissionRates{}) { + if msg.Commission == (stakingtypes.CommissionRates{}) { panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission")) } @@ -147,7 +146,7 @@ func (b *GenesisBuilder) GenTx(privVal secp256k1.PrivKey, val cmttypes.GenesisVa } if msg.Value.Amount.LT(msg.MinSelfDelegation) { - panic(types.ErrSelfDelegationBelowMinimum) + panic(stakingtypes.ErrSelfDelegationBelowMinimum) } txConf := authtx.NewTxConfig(b.codec, authtx.DefaultSignModes) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 20ee0be37dcb..45cf6cca0ec4 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -10,6 +10,7 @@ import ( "google.golang.org/grpc/status" errorsmod "cosmossdk.io/errors" + "cosmossdk.io/math" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/telemetry" @@ -168,8 +169,26 @@ func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValida ctx := sdk.UnwrapSDKContext(goCtx) valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) + } + + if msg.Description == (types.Description{}) { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description") + } + + if msg.MinSelfDelegation != nil && !msg.MinSelfDelegation.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "minimum self delegation must be a positive integer", + ) + } + + if msg.CommissionRate != nil { + if msg.CommissionRate.GT(math.LegacyOneDec()) || msg.CommissionRate.IsNegative() { + return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "commission rate must be between 0 and 1 (inclusive)") + } } + // validator must already be registered validator, found := k.GetValidator(ctx, valAddr) if !found { diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 4b14ee82689d..7f93f0ed9edf 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -209,7 +209,11 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { name: "valid msg", input: &stakingtypes.MsgCreateValidator{ Description: stakingtypes.Description{ - Moniker: "NewValidator", + Moniker: "NewValidator", + Identity: "xyz", + Website: "xyz.com", + SecurityContact: "xyz@gmail.com", + Details: "details", }, Commission: stakingtypes.CommissionRates{ Rate: sdk.NewDecWithPrec(5, 1), @@ -239,6 +243,186 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { } } +func (s *KeeperTestSuite) TestMsgEditValidator() { + ctx, msgServer := s.ctx, s.msgServer + require := s.Require() + + // create new context with updated block time + newCtx := ctx.WithBlockTime(ctx.BlockTime().AddDate(0, 0, 1)) + + authority := authtypes.NewModuleAddress(govtypes.ModuleName) + valAddr := sdk.ValAddress(authority) + + pk := ed25519.GenPrivKey().PubKey() + require.NotNil(pk) + + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), authority, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) + msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, sdk.NewCoin("stake", sdk.NewInt(10)), stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + require.NoError(err) + + res, err := msgServer.CreateValidator(ctx, msg) + require.NoError(err) + require.NotNil(res) + + newRate := math.LegacyZeroDec() + invalidRate := sdk.NewDec(2) + + lowSelfDel := math.OneInt() + highSelfDel := math.NewInt(100) + negSelfDel := math.NewInt(-1) + newSelfDel := math.NewInt(3) + + testCases := []struct { + name string + ctx sdk.Context + input *stakingtypes.MsgEditValidator + expErr bool + expErrMsg string + }{ + { + name: "invalid validator", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), + CommissionRate: &newRate, + MinSelfDelegation: &newSelfDel, + }, + expErr: true, + expErrMsg: "invalid validator address", + }, + { + name: "empty description", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{}, + ValidatorAddress: valAddr.String(), + CommissionRate: &newRate, + MinSelfDelegation: &newSelfDel, + }, + expErr: true, + expErrMsg: "empty description", + }, + { + name: "negative self delegation", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: valAddr.String(), + CommissionRate: &newRate, + MinSelfDelegation: &negSelfDel, + }, + expErr: true, + expErrMsg: "minimum self delegation must be a positive integer", + }, + { + name: "invalid commission rate", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: valAddr.String(), + CommissionRate: &invalidRate, + MinSelfDelegation: &newSelfDel, + }, + expErr: true, + expErrMsg: "commission rate must be between 0 and 1 (inclusive)", + }, + { + name: "validator does not exist", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: sdk.ValAddress([]byte("val")).String(), + CommissionRate: &newRate, + MinSelfDelegation: &newSelfDel, + }, + expErr: true, + expErrMsg: "validator does not exist", + }, + { + name: "change commmission rate in <24hrs", + ctx: ctx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: valAddr.String(), + CommissionRate: &newRate, + MinSelfDelegation: &newSelfDel, + }, + expErr: true, + expErrMsg: "commission cannot be changed more than once in 24h", + }, + { + name: "minimum self delegation cannot decrease", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: valAddr.String(), + CommissionRate: &newRate, + MinSelfDelegation: &lowSelfDel, + }, + expErr: true, + expErrMsg: "minimum self delegation cannot be decrease", + }, + { + name: "validator self-delegation must be greater than min self delegation", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + }, + ValidatorAddress: valAddr.String(), + CommissionRate: &newRate, + MinSelfDelegation: &highSelfDel, + }, + expErr: true, + expErrMsg: "validator's self delegation must be greater than their minimum self delegation", + }, + { + name: "valid msg", + ctx: newCtx, + input: &stakingtypes.MsgEditValidator{ + Description: stakingtypes.Description{ + Moniker: "TestValidator", + Identity: "abc", + Website: "abc.com", + SecurityContact: "abc@gmail.com", + Details: "newDetails", + }, + ValidatorAddress: valAddr.String(), + CommissionRate: &newRate, + MinSelfDelegation: &newSelfDel, + }, + expErr: false, + }, + } + for _, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + _, err := msgServer.EditValidator(tc.ctx, tc.input) + if tc.expErr { + require.Error(err) + require.Contains(err.Error(), tc.expErrMsg) + } else { + require.NoError(err) + } + }) + } +} + func (s *KeeperTestSuite) TestMsgUpdateParams() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 1a6e75688f49..c8714fb840bc 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -99,32 +99,6 @@ func (msg MsgEditValidator) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgEditValidator) ValidateBasic() error { - if _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) - } - - if msg.Description == (Description{}) { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description") - } - - if msg.MinSelfDelegation != nil && !msg.MinSelfDelegation.IsPositive() { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "minimum self delegation must be a positive integer", - ) - } - - if msg.CommissionRate != nil { - if msg.CommissionRate.GT(math.LegacyOneDec()) || msg.CommissionRate.IsNegative() { - return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "commission rate must be between 0 and 1 (inclusive)") - } - } - - return nil -} - // NewMsgDelegate creates a new MsgDelegate instance. func NewMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, amount sdk.Coin) *MsgDelegate { return &MsgDelegate{ diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index 640f262008ce..f904418cef0a 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -56,34 +56,6 @@ func TestMsgDecode(t *testing.T) { require.True(t, msg.Pubkey.Equal(msg2.Pubkey)) } -// test ValidateBasic for MsgEditValidator -func TestMsgEditValidator(t *testing.T) { - tests := []struct { - name, moniker, identity, website, securityContact, details string - validatorAddr sdk.ValAddress - expectPass bool - minSelfDelegation math.Int - }{ - {"basic good", "a", "b", "c", "d", "e", valAddr1, true, math.OneInt()}, - {"partial description", "", "", "c", "", "", valAddr1, true, math.OneInt()}, - {"empty description", "", "", "", "", "", valAddr1, false, math.OneInt()}, - {"empty address", "a", "b", "c", "d", "e", emptyAddr, false, math.OneInt()}, - {"nil int", "a", "b", "c", "d", "e", emptyAddr, false, math.Int{}}, - } - - for _, tc := range tests { - description := types.NewDescription(tc.moniker, tc.identity, tc.website, tc.securityContact, tc.details) - newRate := math.LegacyZeroDec() - - msg := types.NewMsgEditValidator(tc.validatorAddr, description, &newRate, &tc.minSelfDelegation) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", tc.name) - } - } -} - // test ValidateBasic for MsgDelegate func TestMsgDelegate(t *testing.T) { tests := []struct { From 2f3520b497bc3fcdf2fdad23f34d3ebe83b8954c Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 15:29:15 +0530 Subject: [PATCH 04/14] wip: updates --- x/staking/keeper/msg_server.go | 17 +++-- x/staking/keeper/msg_server_test.go | 101 ++++++++++++++++++++++++++++ x/staking/types/msg.go | 19 ------ x/staking/types/msg_test.go | 27 -------- 4 files changed, 113 insertions(+), 51 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 45cf6cca0ec4..c93ddbe8f5c1 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -5,13 +5,13 @@ import ( "strconv" "time" + errorsmod "cosmossdk.io/errors" + "cosmossdk.io/math" + "github.com/armon/go-metrics" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - errorsmod "cosmossdk.io/errors" - "cosmossdk.io/math" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" @@ -178,7 +178,7 @@ func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValida if msg.MinSelfDelegation != nil && !msg.MinSelfDelegation.IsPositive() { return nil, errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, + sdkerrors.ErrInvalidRequest, "minimum self delegation must be a positive integer", ) } @@ -247,7 +247,7 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ ctx := sdk.UnwrapSDKContext(goCtx) valAddr, valErr := sdk.ValAddressFromBech32(msg.ValidatorAddress) if valErr != nil { - return nil, valErr + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", valErr) } validator, found := k.GetValidator(ctx, valAddr) @@ -260,6 +260,13 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ return nil, err } + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid delegation amount", + ) + } + bondDenom := k.BondDenom(ctx) if msg.Amount.Denom != bondDenom { return nil, errorsmod.Wrapf( diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 7f93f0ed9edf..5c8670451888 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -423,6 +423,107 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { } } +func (s *KeeperTestSuite) TestMsgDelegate() { + ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer + require := s.Require() + + authority := authtypes.NewModuleAddress(govtypes.ModuleName) + valAddr := sdk.ValAddress(authority) + + pk := ed25519.GenPrivKey().PubKey() + require.NotNil(pk) + + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), authority, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) + msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, sdk.NewCoin("stake", sdk.NewInt(10)), stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + require.NoError(err) + + res, err := msgServer.CreateValidator(ctx, msg) + require.NoError(err) + require.NotNil(res) + + testCases := []struct { + name string + input *stakingtypes.MsgDelegate + expErr bool + expErrMsg string + }{ + { + name: "invalid validator", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: keeper.GetAuthority(), + ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "invalid validator address", + }, + { + name: "validator does not exist", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: keeper.GetAuthority(), + ValidatorAddress: sdk.ValAddress([]byte("val")).String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "validator does not exist", + }, + { + name: "zero amount", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: keeper.GetAuthority(), + ValidatorAddress: valAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(0))}, + }, + expErr: true, + expErrMsg: "invalid delegation amount", + }, + { + name: "negative amount", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: keeper.GetAuthority(), + ValidatorAddress: valAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(-1))}, + }, + expErr: true, + expErrMsg: "invalid delegation amount", + }, + { + name: "invalid BondDenom", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: keeper.GetAuthority(), + ValidatorAddress: valAddr.String(), + Amount: sdk.Coin{Denom: "test", Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "invalid coin denomination", + }, + { + name: "valid msg", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: keeper.GetAuthority(), + ValidatorAddress: valAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: false, + }, + } + + for _, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + _, err := msgServer.Delegate(ctx, tc.input) + if tc.expErr { + require.Error(err) + require.Contains(err.Error(), tc.expErrMsg) + } else { + require.NoError(err) + } + }) + } +} + func (s *KeeperTestSuite) TestMsgUpdateParams() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index c8714fb840bc..9488be499531 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -120,25 +120,6 @@ func (msg MsgDelegate) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgDelegate) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.DelegatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) - } - if _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) - } - - if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid delegation amount", - ) - } - - return nil -} - // NewMsgBeginRedelegate creates a new MsgBeginRedelegate instance. func NewMsgBeginRedelegate( delAddr sdk.AccAddress, valSrcAddr, valDstAddr sdk.ValAddress, amount sdk.Coin, diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index f904418cef0a..7c3c39add501 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -56,33 +56,6 @@ func TestMsgDecode(t *testing.T) { require.True(t, msg.Pubkey.Equal(msg2.Pubkey)) } -// test ValidateBasic for MsgDelegate -func TestMsgDelegate(t *testing.T) { - tests := []struct { - name string - delegatorAddr sdk.AccAddress - validatorAddr sdk.ValAddress - bond sdk.Coin - expectPass bool - }{ - {"basic good", sdk.AccAddress(valAddr1), valAddr2, coinPos, true}, - {"self bond", sdk.AccAddress(valAddr1), valAddr1, coinPos, true}, - {"empty delegator", sdk.AccAddress(emptyAddr), valAddr1, coinPos, false}, - {"empty validator", sdk.AccAddress(valAddr1), emptyAddr, coinPos, false}, - {"empty bond", sdk.AccAddress(valAddr1), valAddr2, coinZero, false}, - {"nil bold", sdk.AccAddress(valAddr1), valAddr2, sdk.Coin{}, false}, - } - - for _, tc := range tests { - msg := types.NewMsgDelegate(tc.delegatorAddr, tc.validatorAddr, tc.bond) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", tc.name) - } - } -} - // test ValidateBasic for MsgUnbond func TestMsgBeginRedelegate(t *testing.T) { tests := []struct { From 39ee12378e1c09a9da867dfd3a81926584613b61 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 16:16:43 +0530 Subject: [PATCH 05/14] wip: more updates --- x/staking/keeper/msg_server.go | 11 +- x/staking/keeper/msg_server_test.go | 155 ++++++++++++++++++++++++++++ x/staking/types/msg.go | 22 ---- x/staking/types/msg_test.go | 28 ----- 4 files changed, 164 insertions(+), 52 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index c93ddbe8f5c1..69264d6cf87c 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -308,7 +308,7 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed ctx := sdk.UnwrapSDKContext(goCtx) valSrcAddr, err := sdk.ValAddressFromBech32(msg.ValidatorSrcAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid source validator address: %s", err) } delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) if err != nil { @@ -330,7 +330,14 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed valDstAddr, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid destination validator address: %s", err) + } + + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid shares amount", + ) } completionTime, err := k.BeginRedelegation( diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 5c8670451888..0e8c677e73bc 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -524,6 +524,161 @@ func (s *KeeperTestSuite) TestMsgDelegate() { } } +func (s *KeeperTestSuite) TestMsgBeginRedelegate() { + ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer + require := s.Require() + + addr := sdk.AccAddress(PKS[0].Address()) + srcValAddr := sdk.ValAddress(addr) + addr2 := sdk.AccAddress(PKS[1].Address()) + dstValAddr := sdk.ValAddress(addr2) + + pk := ed25519.GenPrivKey().PubKey() + require.NotNil(pk) + dstPk := ed25519.GenPrivKey().PubKey() + require.NotNil(dstPk) + + comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) + amt := sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))} + + s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() + s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes() + + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + msg, err := stakingtypes.NewMsgCreateValidator(srcValAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + require.NoError(err) + res, err := msgServer.CreateValidator(ctx, msg) + require.NoError(err) + require.NotNil(res) + + s.accountKeeper.EXPECT().StringToBytes(addr2.String()).Return(addr2, nil).AnyTimes() + s.accountKeeper.EXPECT().BytesToString(addr2).Return(addr2.String(), nil).AnyTimes() + + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr2, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + msg, err = stakingtypes.NewMsgCreateValidator(dstValAddr, dstPk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + require.NoError(err) + + res, err = msgServer.CreateValidator(ctx, msg) + require.NoError(err) + require.NotNil(res) + + shares := math.LegacyNewDec(100) + del := stakingtypes.NewDelegation(addr, srcValAddr, shares) + keeper.SetDelegation(ctx, del) + _, found := keeper.GetDelegation(ctx, addr, srcValAddr) + require.True(found) + + testCases := []struct { + name string + input *stakingtypes.MsgBeginRedelegate + expErr bool + expErrMsg string + }{ + { + name: "invalid source validator", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: sdk.AccAddress([]byte("invalid")).String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: true, + expErrMsg: "invalid source validator address", + }, + { + name: "invalid destination validator", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: sdk.AccAddress([]byte("invalid")).String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: true, + expErrMsg: "invalid destination validator address", + }, + { + name: "validator does not exist", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: sdk.ValAddress([]byte("invalid")).String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: true, + expErrMsg: "validator does not exist", + }, + { + name: "self redelegation", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: srcValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: true, + expErrMsg: "cannot redelegate to the same validator", + }, + { + name: "amount greater than delegated shares amount", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(101)), + }, + expErr: true, + expErrMsg: "invalid shares amount", + }, + { + name: "zero amount", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(0)), + }, + expErr: true, + expErrMsg: "invalid shares amount", + }, + { + name: "invalid coin denom", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.NewCoin("test", shares.RoundInt()), + }, + expErr: true, + expErrMsg: "invalid coin denomination", + }, + { + name: "valid msg", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: addr.String(), + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: false, + }, + } + + for _, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + _, err := msgServer.BeginRedelegate(ctx, tc.input) + if tc.expErr { + require.Error(err) + require.Contains(err.Error(), tc.expErrMsg) + } else { + require.NoError(err) + } + }) + } +} + func (s *KeeperTestSuite) TestMsgUpdateParams() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 9488be499531..21dc532e0c17 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -144,28 +144,6 @@ func (msg MsgBeginRedelegate) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgBeginRedelegate) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.DelegatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) - } - if _, err := sdk.ValAddressFromBech32(msg.ValidatorSrcAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid source validator address: %s", err) - } - if _, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid destination validator address: %s", err) - } - - if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid shares amount", - ) - } - - return nil -} - // NewMsgUndelegate creates a new MsgUndelegate instance. func NewMsgUndelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, amount sdk.Coin) *MsgUndelegate { return &MsgUndelegate{ diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index 7c3c39add501..6ad07307c3fe 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -56,34 +56,6 @@ func TestMsgDecode(t *testing.T) { require.True(t, msg.Pubkey.Equal(msg2.Pubkey)) } -// test ValidateBasic for MsgUnbond -func TestMsgBeginRedelegate(t *testing.T) { - tests := []struct { - name string - delegatorAddr sdk.AccAddress - validatorSrcAddr sdk.ValAddress - validatorDstAddr sdk.ValAddress - amount sdk.Coin - expectPass bool - }{ - {"regular", sdk.AccAddress(valAddr1), valAddr2, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), true}, - {"zero amount", sdk.AccAddress(valAddr1), valAddr2, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 0), false}, - {"nil amount", sdk.AccAddress(valAddr1), valAddr2, valAddr3, sdk.Coin{}, false}, - {"empty delegator", sdk.AccAddress(emptyAddr), valAddr1, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, - {"empty source validator", sdk.AccAddress(valAddr1), emptyAddr, valAddr3, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, - {"empty destination validator", sdk.AccAddress(valAddr1), valAddr2, emptyAddr, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, - } - - for _, tc := range tests { - msg := types.NewMsgBeginRedelegate(tc.delegatorAddr, tc.validatorSrcAddr, tc.validatorDstAddr, tc.amount) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", tc.name) - } - } -} - // test ValidateBasic for MsgUnbond func TestMsgUndelegate(t *testing.T) { tests := []struct { From 6f277699825e20196447bf05045b206da3e17304 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 17:40:05 +0530 Subject: [PATCH 06/14] wip: add more msg server tests --- x/staking/keeper/msg_server.go | 25 ++- x/staking/keeper/msg_server_test.go | 255 ++++++++++++++++++++++++++++ x/staking/types/msg.go | 47 ----- x/staking/types/msg_test.go | 38 ----- 4 files changed, 278 insertions(+), 87 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 69264d6cf87c..90b43b7ecbfa 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -379,7 +379,7 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) ( addr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) } delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) if err != nil { @@ -399,6 +399,13 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) ( ) } + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid shares amount", + ) + } + completionTime, undelegatedAmt, err := k.Keeper.Undelegate(ctx, delegatorAddress, addr, shares) if err != nil { return nil, err @@ -439,7 +446,7 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) } delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) @@ -454,6 +461,20 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M ) } + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid amount", + ) + } + + if msg.CreationHeight <= 0 { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid height", + ) + } + validator, found := k.GetValidator(ctx, valAddr) if !found { return nil, types.ErrNoValidatorFound diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index 0e8c677e73bc..b4ba78b23fc0 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -679,6 +679,261 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { } } +func (s *KeeperTestSuite) TestMsgUndelegate() { + ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer + require := s.Require() + + addr := sdk.AccAddress(PKS[0].Address()) + valAddr := sdk.ValAddress(addr) + pk := ed25519.GenPrivKey().PubKey() + require.NotNil(pk) + + comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) + amt := sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))} + + s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() + s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes() + + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + require.NoError(err) + res, err := msgServer.CreateValidator(ctx, msg) + require.NoError(err) + require.NotNil(res) + + shares := math.LegacyNewDec(100) + del := stakingtypes.NewDelegation(addr, valAddr, shares) + keeper.SetDelegation(ctx, del) + _, found := keeper.GetDelegation(ctx, addr, valAddr) + require.True(found) + + testCases := []struct { + name string + input *stakingtypes.MsgUndelegate + expErr bool + expErrMsg string + }{ + { + name: "invalid validator", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: addr.String(), + ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: true, + expErrMsg: "invalid validator address", + }, + { + name: "validator does not exist", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: addr.String(), + ValidatorAddress: sdk.ValAddress([]byte("invalid")).String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: true, + expErrMsg: "validator does not exist", + }, + { + name: "amount greater than delegated shares amount", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(101)), + }, + expErr: true, + expErrMsg: "invalid shares amount", + }, + { + name: "zero amount", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(0)), + }, + expErr: true, + expErrMsg: "invalid shares amount", + }, + { + name: "invalid coin denom", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin("test", shares.RoundInt()), + }, + expErr: true, + expErrMsg: "invalid coin denomination", + }, + { + name: "valid msg", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + }, + expErr: false, + }, + } + + for _, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + _, err := msgServer.Undelegate(ctx, tc.input) + if tc.expErr { + require.Error(err) + require.Contains(err.Error(), tc.expErrMsg) + } else { + require.NoError(err) + } + }) + } +} + +func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { + ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer + require := s.Require() + + addr := sdk.AccAddress(PKS[0].Address()) + valAddr := sdk.ValAddress(addr) + pk := ed25519.GenPrivKey().PubKey() + require.NotNil(pk) + + comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) + amt := sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))} + + s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() + s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes() + + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + + msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + require.NoError(err) + res, err := msgServer.CreateValidator(ctx, msg) + require.NoError(err) + require.NotNil(res) + + shares := math.LegacyNewDec(100) + del := stakingtypes.NewDelegation(addr, valAddr, shares) + keeper.SetDelegation(ctx, del) + resDel, found := keeper.GetDelegation(ctx, addr, valAddr) + require.True(found) + require.Equal(del, resDel) + + ubd := stakingtypes.NewUnbondingDelegation(addr, valAddr, 10, ctx.BlockTime().Add(time.Minute*10), shares.RoundInt(), 0) + keeper.SetUnbondingDelegation(ctx, ubd) + resUnbond, found := keeper.GetUnbondingDelegation(ctx, addr, valAddr) + require.True(found) + require.Equal(ubd, resUnbond) + + testCases := []struct { + name string + input *stakingtypes.MsgCancelUnbondingDelegation + expErr bool + expErrMsg string + }{ + { + name: "invalid validator", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "invalid validator address", + }, + { + name: "entry not found at height", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: 11, + }, + expErr: true, + expErrMsg: "unbonding delegation entry is not found at block height", + }, + { + name: "invalid height", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: -1, + }, + expErr: true, + expErrMsg: "invalid height", + }, + { + name: "invalid coin", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin("test", shares.RoundInt()), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "invalid coin denomination", + }, + { + name: "validator does not exist", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: sdk.ValAddress([]byte("invalid")).String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "validator does not exist", + }, + { + name: "amount is greater than balance", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(101)), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "amount is greater than the unbonding delegation entry balance", + }, + { + name: "zero amount", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0)), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "invalid amount", + }, + { + name: "valid msg", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: addr.String(), + ValidatorAddress: valAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: 10, + }, + expErr: false, + }, + } + + for _, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + _, err := msgServer.CancelUnbondingDelegation(ctx, tc.input) + if tc.expErr { + require.Error(err) + require.Contains(err.Error(), tc.expErrMsg) + } else { + require.NoError(err) + } + }) + } +} + func (s *KeeperTestSuite) TestMsgUpdateParams() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 21dc532e0c17..b87c69f6d6e8 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -1,13 +1,11 @@ package types import ( - errorsmod "cosmossdk.io/errors" "cosmossdk.io/math" codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" ) @@ -165,25 +163,6 @@ func (msg MsgUndelegate) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgUndelegate) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.DelegatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) - } - if _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) - } - - if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid shares amount", - ) - } - - return nil -} - // NewMsgCancelUnbondingDelegation creates a new MsgCancelUnbondingDelegation instance. func NewMsgCancelUnbondingDelegation(delAddr sdk.AccAddress, valAddr sdk.ValAddress, creationHeight int64, amount sdk.Coin) *MsgCancelUnbondingDelegation { return &MsgCancelUnbondingDelegation{ @@ -205,32 +184,6 @@ func (msg MsgCancelUnbondingDelegation) GetSignBytes() []byte { return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg)) } -// ValidateBasic implements the sdk.Msg interface. -func (msg MsgCancelUnbondingDelegation) ValidateBasic() error { - if _, err := sdk.AccAddressFromBech32(msg.DelegatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) - } - if _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress); err != nil { - return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) - } - - if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid amount", - ) - } - - if msg.CreationHeight <= 0 { - return errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid height", - ) - } - - return nil -} - // GetSignBytes returns the raw bytes for a MsgUpdateParams message that // the expected signer needs to sign. func (m MsgUpdateParams) GetSignBytes() []byte { diff --git a/x/staking/types/msg_test.go b/x/staking/types/msg_test.go index 37e2daea94cb..a59a897bc42a 100644 --- a/x/staking/types/msg_test.go +++ b/x/staking/types/msg_test.go @@ -5,9 +5,6 @@ import ( "cosmossdk.io/math" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/codec" @@ -52,38 +49,3 @@ func TestMsgDecode(t *testing.T) { require.True(t, msg.Value.IsEqual(msg2.Value)) require.True(t, msg.Pubkey.Equal(msg2.Pubkey)) } - -// test ValidateBasic for MsgUnbond -func TestMsgUndelegate(t *testing.T) { - tests := []struct { - name string - delegatorAddr sdk.AccAddress - validatorAddr sdk.ValAddress - amount sdk.Coin - expectPass bool - }{ - {"regular", sdk.AccAddress(valAddr1), valAddr2, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), true}, - {"zero amount", sdk.AccAddress(valAddr1), valAddr2, sdk.NewInt64Coin(sdk.DefaultBondDenom, 0), false}, - {"nil amount", sdk.AccAddress(valAddr1), valAddr2, sdk.Coin{}, false}, - {"empty delegator", sdk.AccAddress(emptyAddr), valAddr1, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, - {"empty validator", sdk.AccAddress(valAddr1), emptyAddr, sdk.NewInt64Coin(sdk.DefaultBondDenom, 1), false}, - } - - for _, tc := range tests { - msg := types.NewMsgUndelegate(tc.delegatorAddr, tc.validatorAddr, tc.amount) - if tc.expectPass { - require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) - } else { - require.NotNil(t, msg.ValidateBasic(), "test: %v", tc.name) - } - } -} - -func TestMsgUpdateParams(t *testing.T) { - msg := types.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: types.DefaultParams(), - } - - require.Equal(t, []sdk.AccAddress{authtypes.NewModuleAddress(govtypes.ModuleName)}, msg.GetSigners()) -} From d9462ca36807615c365b1f4b74969a55d6e37103 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 17:46:35 +0530 Subject: [PATCH 07/14] fix integration tests --- .../integration/staking/keeper/msg_server_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index 7ab5c27e4898..5721bdaf94ad 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -81,6 +81,17 @@ func TestCancelUnbondingDelegation(t *testing.T) { req types.MsgCancelUnbondingDelegation expErrMsg string }{ + { + Name: "entry not found at height", + ExceptErr: true, + req: types.MsgCancelUnbondingDelegation{ + DelegatorAddress: resUnbond.DelegatorAddress, + ValidatorAddress: resUnbond.ValidatorAddress, + Amount: sdk.NewCoin(stakingKeeper.BondDenom(ctx), sdk.NewInt(4)), + CreationHeight: 11, + }, + expErrMsg: "unbonding delegation entry is not found at block height", + }, { Name: "invalid height", ExceptErr: true, @@ -90,7 +101,7 @@ func TestCancelUnbondingDelegation(t *testing.T) { Amount: sdk.NewCoin(stakingKeeper.BondDenom(ctx), sdk.NewInt(4)), CreationHeight: 0, }, - expErrMsg: "unbonding delegation entry is not found at block height", + expErrMsg: "invalid height", }, { Name: "invalid coin", @@ -110,7 +121,7 @@ func TestCancelUnbondingDelegation(t *testing.T) { DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: sdk.ValAddress(sdk.AccAddress("asdsad")).String(), Amount: unbondingAmount, - CreationHeight: 0, + CreationHeight: 10, }, expErrMsg: "validator does not exist", }, From 95bd0fd12a25272749f3bdb07c529110478953e2 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 13 Apr 2023 18:18:19 +0530 Subject: [PATCH 08/14] fix lint --- x/staking/types/data_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/staking/types/data_test.go b/x/staking/types/data_test.go index ba010f47cbe7..f8b927ef1e6f 100644 --- a/x/staking/types/data_test.go +++ b/x/staking/types/data_test.go @@ -16,8 +16,6 @@ var ( valAddr1 = sdk.ValAddress(pk1.Address()) valAddr2 = sdk.ValAddress(pk2.Address()) valAddr3 = sdk.ValAddress(pk3.Address()) - - emptyAddr sdk.ValAddress ) func init() { From c527d992c57000b0d1f1e55a4f648e728cdc4824 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 14 Apr 2023 10:55:22 +0530 Subject: [PATCH 09/14] wip: address review comments --- .../staking/keeper/msg_server_test.go | 40 ++++++++--------- testutil/testnet/genesis.go | 35 +-------------- x/staking/client/cli/tx.go | 3 ++ x/staking/keeper/msg_server.go | 30 +------------ x/staking/types/msg.go | 44 +++++++++++++++++++ 5 files changed, 70 insertions(+), 82 deletions(-) diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index 5721bdaf94ad..be8fa6ccfb1d 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -76,14 +76,14 @@ func TestCancelUnbondingDelegation(t *testing.T) { assert.DeepEqual(t, ubd, resUnbond) testCases := []struct { - Name string - ExceptErr bool + name string + exceptErr bool req types.MsgCancelUnbondingDelegation expErrMsg string }{ { - Name: "entry not found at height", - ExceptErr: true, + name: "entry not found at height", + exceptErr: true, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, @@ -93,8 +93,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { expErrMsg: "unbonding delegation entry is not found at block height", }, { - Name: "invalid height", - ExceptErr: true, + name: "invalid height", + exceptErr: true, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, @@ -104,8 +104,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { expErrMsg: "invalid height", }, { - Name: "invalid coin", - ExceptErr: true, + name: "invalid coin", + exceptErr: true, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, @@ -115,8 +115,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { expErrMsg: "invalid coin denomination", }, { - Name: "validator not exists", - ExceptErr: true, + name: "validator not exists", + exceptErr: true, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: sdk.ValAddress(sdk.AccAddress("asdsad")).String(), @@ -126,8 +126,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { expErrMsg: "validator does not exist", }, { - Name: "invalid delegator address", - ExceptErr: true, + name: "invalid delegator address", + exceptErr: true, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: "invalid_delegator_addrtess", ValidatorAddress: resUnbond.ValidatorAddress, @@ -137,8 +137,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { expErrMsg: "decoding bech32 failed", }, { - Name: "invalid amount", - ExceptErr: true, + name: "invalid amount", + exceptErr: true, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, @@ -148,8 +148,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { expErrMsg: "amount is greater than the unbonding delegation entry balance", }, { - Name: "success", - ExceptErr: false, + name: "success", + exceptErr: false, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, @@ -158,8 +158,8 @@ func TestCancelUnbondingDelegation(t *testing.T) { }, }, { - Name: "success", - ExceptErr: false, + name: "success", + exceptErr: false, req: types.MsgCancelUnbondingDelegation{ DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, @@ -170,9 +170,9 @@ func TestCancelUnbondingDelegation(t *testing.T) { } for _, testCase := range testCases { - t.Run(testCase.Name, func(t *testing.T) { + t.Run(testCase.name, func(t *testing.T) { _, err := msgServer.CancelUnbondingDelegation(ctx, &testCase.req) - if testCase.ExceptErr { + if testCase.exceptErr { assert.ErrorContains(t, err, testCase.expErrMsg) } else { assert.NilError(t, err) diff --git a/testutil/testnet/genesis.go b/testutil/testnet/genesis.go index 7eaa2a75b14a..7b867dee7360 100644 --- a/testutil/testnet/genesis.go +++ b/testutil/testnet/genesis.go @@ -6,7 +6,6 @@ import ( "strconv" "time" - errorsmod "cosmossdk.io/errors" cmttypes "github.com/cometbft/cometbft/types" "github.com/cosmos/cosmos-sdk/codec" @@ -14,7 +13,6 @@ import ( cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" @@ -113,42 +111,11 @@ func (b *GenesisBuilder) GenTx(privVal secp256k1.PrivKey, val cmttypes.GenesisVa if err != nil { panic(err) } - _, err = sdk.ValAddressFromBech32(msg.ValidatorAddress) - if err != nil { - panic(err) - } - - if msg.Pubkey == nil { - panic(stakingtypes.ErrEmptyValidatorPubKey) - } - - if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { - panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount")) - } - if msg.Description == (stakingtypes.Description{}) { - panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description")) - } - - if msg.Commission == (stakingtypes.CommissionRates{}) { - panic(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission")) - } - - if err := msg.Commission.Validate(); err != nil { + if err := msg.Validate(); err != nil { panic(err) } - if !msg.MinSelfDelegation.IsPositive() { - panic(errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "minimum self delegation must be a positive integer", - )) - } - - if msg.Value.Amount.LT(msg.MinSelfDelegation) { - panic(stakingtypes.ErrSelfDelegationBelowMinimum) - } - txConf := authtx.NewTxConfig(b.codec, authtx.DefaultSignModes) txb := txConf.NewTxBuilder() diff --git a/x/staking/client/cli/tx.go b/x/staking/client/cli/tx.go index 6b26f60a3bda..8c8191cdd256 100644 --- a/x/staking/client/cli/tx.go +++ b/x/staking/client/cli/tx.go @@ -379,6 +379,9 @@ func newBuildCreateValidatorMsg(clientCtx client.Context, txf tx.Factory, fs *fl if err != nil { return txf, nil, err } + if err := msg.Validate(); err != nil { + return txf, nil, err + } genOnly, _ := fs.GetBool(flags.FlagGenerateOnly) if genOnly { diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 90b43b7ecbfa..9ce600acd4e4 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -36,42 +36,16 @@ var _ types.MsgServer = msgServer{} func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateValidator) (*types.MsgCreateValidatorResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + // note that unmarshaling from bech32 ensures both non-empty and valid valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) } - if msg.Pubkey == nil { - return nil, types.ErrEmptyValidatorPubKey - } - - if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { - return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount") - } - - if msg.Description == (types.Description{}) { - return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description") - } - - if msg.Commission == (types.CommissionRates{}) { - return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission") - } - - if err := msg.Commission.Validate(); err != nil { + if err := msg.Validate(); err != nil { return nil, err } - if !msg.MinSelfDelegation.IsPositive() { - return nil, errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "minimum self delegation must be a positive integer", - ) - } - - if msg.Value.Amount.LT(msg.MinSelfDelegation) { - return nil, types.ErrSelfDelegationBelowMinimum - } - if msg.Commission.Rate.LT(k.MinCommissionRate(ctx)) { return nil, errorsmod.Wrapf(types.ErrCommissionLTMinRate, "cannot set validator commission to less than minimum rate of %s", k.MinCommissionRate(ctx)) } diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index b87c69f6d6e8..3b19e57b67af 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -1,11 +1,13 @@ package types import ( + errorsmod "cosmossdk.io/errors" "cosmossdk.io/math" codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" ) @@ -69,6 +71,48 @@ func (msg MsgCreateValidator) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } +// ValidateBasic implements the sdk.Msg interface. +func (msg MsgCreateValidator) Validate() error { + // note that unmarshaling from bech32 ensures both non-empty and valid + _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) + if err != nil { + return sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) + } + + if msg.Pubkey == nil { + return ErrEmptyValidatorPubKey + } + + if !msg.Value.IsValid() || !msg.Value.Amount.IsPositive() { + return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid delegation amount") + } + + if msg.Description == (Description{}) { + return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty description") + } + + if msg.Commission == (CommissionRates{}) { + return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty commission") + } + + if err := msg.Commission.Validate(); err != nil { + return err + } + + if !msg.MinSelfDelegation.IsPositive() { + return errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "minimum self delegation must be a positive integer", + ) + } + + if msg.Value.Amount.LT(msg.MinSelfDelegation) { + return ErrSelfDelegationBelowMinimum + } + + return nil +} + // UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces func (msg MsgCreateValidator) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { var pubKey cryptotypes.PubKey From eef62e8fc325a297f094832b1bce7bbbd4a4842a Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 14 Apr 2023 14:07:31 +0530 Subject: [PATCH 10/14] address more comments --- x/staking/keeper/msg_server.go | 28 ++++++++++++++++------------ x/staking/types/msg.go | 2 +- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 9ce600acd4e4..8e0d4d67b1bc 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -34,9 +34,6 @@ var _ types.MsgServer = msgServer{} // CreateValidator defines a method for creating a new validator func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateValidator) (*types.MsgCreateValidatorResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - - // note that unmarshaling from bech32 ensures both non-empty and valid valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) @@ -46,6 +43,8 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa return nil, err } + ctx := sdk.UnwrapSDKContext(goCtx) + if msg.Commission.Rate.LT(k.MinCommissionRate(ctx)) { return nil, errorsmod.Wrapf(types.ErrCommissionLTMinRate, "cannot set validator commission to less than minimum rate of %s", k.MinCommissionRate(ctx)) } @@ -140,7 +139,6 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa // EditValidator defines a method for editing an existing validator func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValidator) (*types.MsgEditValidatorResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) @@ -163,6 +161,8 @@ func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValida } } + ctx := sdk.UnwrapSDKContext(goCtx) + // validator must already be registered validator, found := k.GetValidator(ctx, valAddr) if !found { @@ -218,12 +218,13 @@ func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValida // Delegate defines a method for performing a delegation of coins from a delegator to a validator func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*types.MsgDelegateResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) valAddr, valErr := sdk.ValAddressFromBech32(msg.ValidatorAddress) if valErr != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", valErr) } + ctx := sdk.UnwrapSDKContext(goCtx) + validator, found := k.GetValidator(ctx, valAddr) if !found { return nil, types.ErrNoValidatorFound @@ -279,7 +280,6 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ // BeginRedelegate defines a method for performing a redelegation of coins from a delegator and source validator to a destination validator func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRedelegate) (*types.MsgBeginRedelegateResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) valSrcAddr, err := sdk.ValAddressFromBech32(msg.ValidatorSrcAddress) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid source validator address: %s", err) @@ -288,6 +288,9 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed if err != nil { return nil, err } + + ctx := sdk.UnwrapSDKContext(goCtx) + shares, err := k.ValidateUnbondAmount( ctx, delegatorAddress, valSrcAddr, msg.Amount.Amount, ) @@ -349,8 +352,6 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed // Undelegate defines a method for performing an undelegation from a delegate and a validator func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (*types.MsgUndelegateResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - addr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) @@ -359,6 +360,9 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) ( if err != nil { return nil, err } + + ctx := sdk.UnwrapSDKContext(goCtx) + shares, err := k.ValidateUnbondAmount( ctx, delegatorAddress, addr, msg.Amount.Amount, ) @@ -416,8 +420,6 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) ( // CancelUnbondingDelegation defines a method for canceling the unbonding delegation // and delegate back to the validator. func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.MsgCancelUnbondingDelegation) (*types.MsgCancelUnbondingDelegationResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) @@ -428,6 +430,8 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M return nil, err } + ctx := sdk.UnwrapSDKContext(goCtx) + bondDenom := k.BondDenom(ctx) if msg.Amount.Denom != bondDenom { return nil, errorsmod.Wrapf( @@ -535,8 +539,6 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M } func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - if k.authority != msg.Authority { return nil, errorsmod.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority, msg.Authority) } @@ -545,6 +547,8 @@ func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParam return nil, err } + ctx := sdk.UnwrapSDKContext(goCtx) + // store params if err := k.SetParams(ctx, msg.Params); err != nil { return nil, err diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 3b19e57b67af..6899e5e35f95 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -71,7 +71,7 @@ func (msg MsgCreateValidator) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic implements the sdk.Msg interface. +// Validate validates the MsgCreateValidator sdk msg. func (msg MsgCreateValidator) Validate() error { // note that unmarshaling from bech32 ensures both non-empty and valid _, err := sdk.ValAddressFromBech32(msg.ValidatorAddress) From 723a37609a7d441c84c3544ccd5284139a97bec1 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 14 Apr 2023 16:43:29 +0530 Subject: [PATCH 11/14] address review comments and refactor tests --- x/staking/keeper/keeper_test.go | 4 + x/staking/keeper/msg_server.go | 80 ++++---- x/staking/keeper/msg_server_test.go | 308 +++++++++++++++++----------- 3 files changed, 229 insertions(+), 163 deletions(-) diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index cf7bf5f15303..4379a7e3448c 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "errors" "testing" "cosmossdk.io/math" @@ -52,6 +53,9 @@ func (s *KeeperTestSuite) SetupTest() { accountKeeper.EXPECT().GetModuleAddress(stakingtypes.NotBondedPoolName).Return(notBondedAcc.GetAddress()) accountKeeper.EXPECT().StringToBytes(authtypes.NewModuleAddress(govtypes.ModuleName).String()).Return(authtypes.NewModuleAddress(govtypes.ModuleName), nil).AnyTimes() accountKeeper.EXPECT().BytesToString(authtypes.NewModuleAddress(govtypes.ModuleName)).Return(authtypes.NewModuleAddress(govtypes.ModuleName).String(), nil).AnyTimes() + accountKeeper.EXPECT().StringToBytes("").Return(nil, errors.New("empty address string is not allowed")).AnyTimes() + accountKeeper.EXPECT().StringToBytes("invalid").Return(nil, errors.New("invalid bech32 string")).AnyTimes() + bankKeeper := stakingtestutil.NewMockBankKeeper(ctrl) keeper := stakingkeeper.NewKeeper( diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 8e0d4d67b1bc..83bdd0149edd 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -223,16 +223,9 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", valErr) } - ctx := sdk.UnwrapSDKContext(goCtx) - - validator, found := k.GetValidator(ctx, valAddr) - if !found { - return nil, types.ErrNoValidatorFound - } - delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) } if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { @@ -242,6 +235,13 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ ) } + ctx := sdk.UnwrapSDKContext(goCtx) + + validator, found := k.GetValidator(ctx, valAddr) + if !found { + return nil, types.ErrNoValidatorFound + } + bondDenom := k.BondDenom(ctx) if msg.Amount.Denom != bondDenom { return nil, errorsmod.Wrapf( @@ -284,9 +284,22 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid source validator address: %s", err) } + + valDstAddr, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddress) + if err != nil { + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid destination validator address: %s", err) + } + delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) + } + + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid shares amount", + ) } ctx := sdk.UnwrapSDKContext(goCtx) @@ -305,18 +318,6 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed ) } - valDstAddr, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddress) - if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid destination validator address: %s", err) - } - - if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { - return nil, errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid shares amount", - ) - } - completionTime, err := k.BeginRedelegation( ctx, delegatorAddress, valSrcAddr, valDstAddr, shares, ) @@ -356,9 +357,17 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) ( if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err) } + delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) if err != nil { - return nil, err + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) + } + + if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { + return nil, errorsmod.Wrap( + sdkerrors.ErrInvalidRequest, + "invalid shares amount", + ) } ctx := sdk.UnwrapSDKContext(goCtx) @@ -377,13 +386,6 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) ( ) } - if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { - return nil, errorsmod.Wrap( - sdkerrors.ErrInvalidRequest, - "invalid shares amount", - ) - } - completionTime, undelegatedAmt, err := k.Keeper.Undelegate(ctx, delegatorAddress, addr, shares) if err != nil { return nil, err @@ -427,16 +429,7 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress) if err != nil { - return nil, err - } - - ctx := sdk.UnwrapSDKContext(goCtx) - - bondDenom := k.BondDenom(ctx) - if msg.Amount.Denom != bondDenom { - return nil, errorsmod.Wrapf( - sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Amount.Denom, bondDenom, - ) + return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err) } if !msg.Amount.IsValid() || !msg.Amount.Amount.IsPositive() { @@ -453,6 +446,15 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M ) } + ctx := sdk.UnwrapSDKContext(goCtx) + + bondDenom := k.BondDenom(ctx) + if msg.Amount.Denom != bondDenom { + return nil, errorsmod.Wrapf( + sdkerrors.ErrInvalidRequest, "invalid coin denomination: got %s, expected %s", msg.Amount.Denom, bondDenom, + ) + } + validator, found := k.GetValidator(ctx, valAddr) if !found { return nil, types.ErrNoValidatorFound diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go index b4ba78b23fc0..30cf6e6b963c 100644 --- a/x/staking/keeper/msg_server_test.go +++ b/x/staking/keeper/msg_server_test.go @@ -12,32 +12,29 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) -var PKS = simtestutil.CreateTestPubKeys(3) +var ( + PKS = simtestutil.CreateTestPubKeys(3) + Addr = sdk.AccAddress(PKS[0].Address()) + ValAddr = sdk.ValAddress(Addr) +) + +func (s *KeeperTestSuite) execExpectCalls() { + s.accountKeeper.EXPECT().StringToBytes(Addr.String()).Return(Addr, nil).AnyTimes() + s.accountKeeper.EXPECT().BytesToString(Addr).Return(Addr.String(), nil).AnyTimes() + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), Addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() +} func (s *KeeperTestSuite) TestMsgCreateValidator() { - ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer + ctx, msgServer := s.ctx, s.msgServer require := s.Require() + s.execExpectCalls() - delAddr := sdk.AccAddress(PKS[0].Address()) pk1 := ed25519.GenPrivKey().PubKey() require.NotNil(pk1) - authority := authtypes.NewModuleAddress(govtypes.ModuleName) - valAddr := sdk.ValAddress(authority) - - s.bankKeeper.EXPECT().MintCoins(gomock.Any(), stakingtypes.ModuleName, gomock.Any()).AnyTimes() - amt := keeper.TokensFromConsensusPower(s.ctx, int64(100)) - s.bankKeeper.MintCoins(s.ctx, stakingtypes.ModuleName, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, amt))) - - s.bankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), stakingtypes.ModuleName, delAddr, gomock.Any()).AnyTimes() - s.bankKeeper.SendCoinsFromModuleToAccount(s.ctx, stakingtypes.ModuleName, delAddr, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, amt))) - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), authority, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() - pubkey, err := codectypes.NewAnyWithValue(pk1) require.NoError(err) @@ -57,8 +54,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(1), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 10000), }, @@ -77,7 +74,7 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(1), - DelegatorAddress: delAddr.String(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 10000), @@ -97,8 +94,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(1), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: nil, Value: sdk.NewInt64Coin("stake", 10000), }, @@ -117,8 +114,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(1), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 0), }, @@ -137,8 +134,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(1), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.Coin{}, }, @@ -157,8 +154,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(0), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 10000), }, @@ -177,8 +174,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(-1), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 10000), }, @@ -197,8 +194,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(100), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 10), }, @@ -221,8 +218,8 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { MaxChangeRate: math.LegacyNewDec(0), }, MinSelfDelegation: math.NewInt(1), - DelegatorAddress: delAddr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Pubkey: pubkey, Value: sdk.NewInt64Coin("stake", 10000), }, @@ -246,20 +243,16 @@ func (s *KeeperTestSuite) TestMsgCreateValidator() { func (s *KeeperTestSuite) TestMsgEditValidator() { ctx, msgServer := s.ctx, s.msgServer require := s.Require() + s.execExpectCalls() // create new context with updated block time newCtx := ctx.WithBlockTime(ctx.BlockTime().AddDate(0, 0, 1)) - authority := authtypes.NewModuleAddress(govtypes.ModuleName) - valAddr := sdk.ValAddress(authority) - pk := ed25519.GenPrivKey().PubKey() require.NotNil(pk) - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), authority, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() - comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) - msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, sdk.NewCoin("stake", sdk.NewInt(10)), stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + msg, err := stakingtypes.NewMsgCreateValidator(ValAddr, pk, sdk.NewCoin("stake", sdk.NewInt(10)), stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) require.NoError(err) res, err := msgServer.CreateValidator(ctx, msg) @@ -300,7 +293,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { ctx: newCtx, input: &stakingtypes.MsgEditValidator{ Description: stakingtypes.Description{}, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &newRate, MinSelfDelegation: &newSelfDel, }, @@ -314,7 +307,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { Description: stakingtypes.Description{ Moniker: "TestValidator", }, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &newRate, MinSelfDelegation: &negSelfDel, }, @@ -328,7 +321,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { Description: stakingtypes.Description{ Moniker: "TestValidator", }, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &invalidRate, MinSelfDelegation: &newSelfDel, }, @@ -356,7 +349,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { Description: stakingtypes.Description{ Moniker: "TestValidator", }, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &newRate, MinSelfDelegation: &newSelfDel, }, @@ -370,7 +363,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { Description: stakingtypes.Description{ Moniker: "TestValidator", }, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &newRate, MinSelfDelegation: &lowSelfDel, }, @@ -384,7 +377,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { Description: stakingtypes.Description{ Moniker: "TestValidator", }, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &newRate, MinSelfDelegation: &highSelfDel, }, @@ -402,7 +395,7 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { SecurityContact: "abc@gmail.com", Details: "newDetails", }, - ValidatorAddress: valAddr.String(), + ValidatorAddress: ValAddr.String(), CommissionRate: &newRate, MinSelfDelegation: &newSelfDel, }, @@ -426,17 +419,14 @@ func (s *KeeperTestSuite) TestMsgEditValidator() { func (s *KeeperTestSuite) TestMsgDelegate() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() - - authority := authtypes.NewModuleAddress(govtypes.ModuleName) - valAddr := sdk.ValAddress(authority) + s.execExpectCalls() pk := ed25519.GenPrivKey().PubKey() require.NotNil(pk) - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), authority, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() - comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) - msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, sdk.NewCoin("stake", sdk.NewInt(10)), stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + + msg, err := stakingtypes.NewMsgCreateValidator(ValAddr, pk, sdk.NewCoin("stake", sdk.NewInt(10)), stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) require.NoError(err) res, err := msgServer.CreateValidator(ctx, msg) @@ -452,17 +442,37 @@ func (s *KeeperTestSuite) TestMsgDelegate() { { name: "invalid validator", input: &stakingtypes.MsgDelegate{ - DelegatorAddress: keeper.GetAuthority(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, }, expErr: true, expErrMsg: "invalid validator address", }, + { + name: "empty delegator", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: "", + ValidatorAddress: ValAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "invalid delegator address: empty address string is not allowed", + }, + { + name: "invalid delegator", + input: &stakingtypes.MsgDelegate{ + DelegatorAddress: "invalid", + ValidatorAddress: ValAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "invalid delegator address: invalid bech32 string", + }, { name: "validator does not exist", input: &stakingtypes.MsgDelegate{ - DelegatorAddress: keeper.GetAuthority(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.ValAddress([]byte("val")).String(), Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, }, @@ -472,8 +482,8 @@ func (s *KeeperTestSuite) TestMsgDelegate() { { name: "zero amount", input: &stakingtypes.MsgDelegate{ - DelegatorAddress: keeper.GetAuthority(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(0))}, }, expErr: true, @@ -482,8 +492,8 @@ func (s *KeeperTestSuite) TestMsgDelegate() { { name: "negative amount", input: &stakingtypes.MsgDelegate{ - DelegatorAddress: keeper.GetAuthority(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(-1))}, }, expErr: true, @@ -492,8 +502,8 @@ func (s *KeeperTestSuite) TestMsgDelegate() { { name: "invalid BondDenom", input: &stakingtypes.MsgDelegate{ - DelegatorAddress: keeper.GetAuthority(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.Coin{Denom: "test", Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, }, expErr: true, @@ -502,8 +512,8 @@ func (s *KeeperTestSuite) TestMsgDelegate() { { name: "valid msg", input: &stakingtypes.MsgDelegate{ - DelegatorAddress: keeper.GetAuthority(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, }, expErr: false, @@ -527,9 +537,9 @@ func (s *KeeperTestSuite) TestMsgDelegate() { func (s *KeeperTestSuite) TestMsgBeginRedelegate() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() + s.execExpectCalls() - addr := sdk.AccAddress(PKS[0].Address()) - srcValAddr := sdk.ValAddress(addr) + srcValAddr := ValAddr addr2 := sdk.AccAddress(PKS[1].Address()) dstValAddr := sdk.ValAddress(addr2) @@ -541,11 +551,6 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) amt := sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))} - s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() - s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes() - - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() - msg, err := stakingtypes.NewMsgCreateValidator(srcValAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) require.NoError(err) res, err := msgServer.CreateValidator(ctx, msg) @@ -554,7 +559,6 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { s.accountKeeper.EXPECT().StringToBytes(addr2.String()).Return(addr2, nil).AnyTimes() s.accountKeeper.EXPECT().BytesToString(addr2).Return(addr2.String(), nil).AnyTimes() - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr2, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() msg, err = stakingtypes.NewMsgCreateValidator(dstValAddr, dstPk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) @@ -565,9 +569,9 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { require.NotNil(res) shares := math.LegacyNewDec(100) - del := stakingtypes.NewDelegation(addr, srcValAddr, shares) + del := stakingtypes.NewDelegation(Addr, srcValAddr, shares) keeper.SetDelegation(ctx, del) - _, found := keeper.GetDelegation(ctx, addr, srcValAddr) + _, found := keeper.GetDelegation(ctx, Addr, srcValAddr) require.True(found) testCases := []struct { @@ -579,7 +583,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "invalid source validator", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: sdk.AccAddress([]byte("invalid")).String(), ValidatorDstAddress: dstValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), @@ -587,10 +591,32 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { expErr: true, expErrMsg: "invalid source validator address", }, + { + name: "empty delegator", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: "", + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "invalid delegator address: empty address string is not allowed", + }, + { + name: "invalid delegator", + input: &stakingtypes.MsgBeginRedelegate{ + DelegatorAddress: "invalid", + ValidatorSrcAddress: srcValAddr.String(), + ValidatorDstAddress: dstValAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))}, + }, + expErr: true, + expErrMsg: "invalid delegator address: invalid bech32 string", + }, { name: "invalid destination validator", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: srcValAddr.String(), ValidatorDstAddress: sdk.AccAddress([]byte("invalid")).String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), @@ -601,7 +627,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "validator does not exist", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: sdk.ValAddress([]byte("invalid")).String(), ValidatorDstAddress: dstValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), @@ -612,7 +638,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "self redelegation", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: srcValAddr.String(), ValidatorDstAddress: srcValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), @@ -623,7 +649,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "amount greater than delegated shares amount", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: srcValAddr.String(), ValidatorDstAddress: dstValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(101)), @@ -634,7 +660,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "zero amount", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: srcValAddr.String(), ValidatorDstAddress: dstValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(0)), @@ -645,7 +671,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "invalid coin denom", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: srcValAddr.String(), ValidatorDstAddress: dstValAddr.String(), Amount: sdk.NewCoin("test", shares.RoundInt()), @@ -656,7 +682,7 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { { name: "valid msg", input: &stakingtypes.MsgBeginRedelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorSrcAddress: srcValAddr.String(), ValidatorDstAddress: dstValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), @@ -682,30 +708,24 @@ func (s *KeeperTestSuite) TestMsgBeginRedelegate() { func (s *KeeperTestSuite) TestMsgUndelegate() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() + s.execExpectCalls() - addr := sdk.AccAddress(PKS[0].Address()) - valAddr := sdk.ValAddress(addr) pk := ed25519.GenPrivKey().PubKey() require.NotNil(pk) comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) amt := sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))} - s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() - s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes() - - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() - - msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + msg, err := stakingtypes.NewMsgCreateValidator(ValAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) require.NoError(err) res, err := msgServer.CreateValidator(ctx, msg) require.NoError(err) require.NotNil(res) shares := math.LegacyNewDec(100) - del := stakingtypes.NewDelegation(addr, valAddr, shares) + del := stakingtypes.NewDelegation(Addr, ValAddr, shares) keeper.SetDelegation(ctx, del) - _, found := keeper.GetDelegation(ctx, addr, valAddr) + _, found := keeper.GetDelegation(ctx, Addr, ValAddr) require.True(found) testCases := []struct { @@ -717,17 +737,37 @@ func (s *KeeperTestSuite) TestMsgUndelegate() { { name: "invalid validator", input: &stakingtypes.MsgUndelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), }, expErr: true, expErrMsg: "invalid validator address", }, + { + name: "empty delegator", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: "", + ValidatorAddress: ValAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: shares.RoundInt()}, + }, + expErr: true, + expErrMsg: "invalid delegator address: empty address string is not allowed", + }, + { + name: "invalid delegator", + input: &stakingtypes.MsgUndelegate{ + DelegatorAddress: "invalid", + ValidatorAddress: ValAddr.String(), + Amount: sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: shares.RoundInt()}, + }, + expErr: true, + expErrMsg: "invalid delegator address: invalid bech32 string", + }, { name: "validator does not exist", input: &stakingtypes.MsgUndelegate{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.ValAddress([]byte("invalid")).String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), }, @@ -737,8 +777,8 @@ func (s *KeeperTestSuite) TestMsgUndelegate() { { name: "amount greater than delegated shares amount", input: &stakingtypes.MsgUndelegate{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(101)), }, expErr: true, @@ -747,8 +787,8 @@ func (s *KeeperTestSuite) TestMsgUndelegate() { { name: "zero amount", input: &stakingtypes.MsgUndelegate{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(0)), }, expErr: true, @@ -757,8 +797,8 @@ func (s *KeeperTestSuite) TestMsgUndelegate() { { name: "invalid coin denom", input: &stakingtypes.MsgUndelegate{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin("test", shares.RoundInt()), }, expErr: true, @@ -767,8 +807,8 @@ func (s *KeeperTestSuite) TestMsgUndelegate() { { name: "valid msg", input: &stakingtypes.MsgUndelegate{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), }, expErr: false, @@ -793,35 +833,33 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer require := s.Require() - addr := sdk.AccAddress(PKS[0].Address()) - valAddr := sdk.ValAddress(addr) pk := ed25519.GenPrivKey().PubKey() require.NotNil(pk) comm := stakingtypes.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0)) amt := sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: keeper.TokensFromConsensusPower(s.ctx, int64(100))} - s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() - s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes() + s.accountKeeper.EXPECT().StringToBytes(Addr.String()).Return(Addr, nil).AnyTimes() + s.accountKeeper.EXPECT().BytesToString(Addr).Return(Addr.String(), nil).AnyTimes() - s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() + s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), Addr, stakingtypes.NotBondedPoolName, gomock.Any()).AnyTimes() - msg, err := stakingtypes.NewMsgCreateValidator(valAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) + msg, err := stakingtypes.NewMsgCreateValidator(ValAddr, pk, amt, stakingtypes.Description{Moniker: "NewVal"}, comm, sdk.OneInt()) require.NoError(err) res, err := msgServer.CreateValidator(ctx, msg) require.NoError(err) require.NotNil(res) shares := math.LegacyNewDec(100) - del := stakingtypes.NewDelegation(addr, valAddr, shares) + del := stakingtypes.NewDelegation(Addr, ValAddr, shares) keeper.SetDelegation(ctx, del) - resDel, found := keeper.GetDelegation(ctx, addr, valAddr) + resDel, found := keeper.GetDelegation(ctx, Addr, ValAddr) require.True(found) require.Equal(del, resDel) - ubd := stakingtypes.NewUnbondingDelegation(addr, valAddr, 10, ctx.BlockTime().Add(time.Minute*10), shares.RoundInt(), 0) + ubd := stakingtypes.NewUnbondingDelegation(Addr, ValAddr, 10, ctx.BlockTime().Add(time.Minute*10), shares.RoundInt(), 0) keeper.SetUnbondingDelegation(ctx, ubd) - resUnbond, found := keeper.GetUnbondingDelegation(ctx, addr, valAddr) + resUnbond, found := keeper.GetUnbondingDelegation(ctx, Addr, ValAddr) require.True(found) require.Equal(ubd, resUnbond) @@ -834,7 +872,7 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "invalid validator", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.AccAddress([]byte("invalid")).String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), CreationHeight: 10, @@ -842,11 +880,33 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { expErr: true, expErrMsg: "invalid validator address", }, + { + name: "empty delegator", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: "", + ValidatorAddress: ValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "invalid delegator address: empty address string is not allowed", + }, + { + name: "invalid delegator", + input: &stakingtypes.MsgCancelUnbondingDelegation{ + DelegatorAddress: "invalid", + ValidatorAddress: ValAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), + CreationHeight: 10, + }, + expErr: true, + expErrMsg: "invalid delegator address: invalid bech32 string", + }, { name: "entry not found at height", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), CreationHeight: 11, }, @@ -856,8 +916,8 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "invalid height", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), CreationHeight: -1, }, @@ -867,8 +927,8 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "invalid coin", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin("test", shares.RoundInt()), CreationHeight: 10, }, @@ -878,7 +938,7 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "validator does not exist", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), + DelegatorAddress: Addr.String(), ValidatorAddress: sdk.ValAddress([]byte("invalid")).String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), CreationHeight: 10, @@ -889,8 +949,8 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "amount is greater than balance", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(101)), CreationHeight: 10, }, @@ -900,8 +960,8 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "zero amount", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0)), CreationHeight: 10, }, @@ -911,8 +971,8 @@ func (s *KeeperTestSuite) TestMsgCancelUnbondingDelegation() { { name: "valid msg", input: &stakingtypes.MsgCancelUnbondingDelegation{ - DelegatorAddress: addr.String(), - ValidatorAddress: valAddr.String(), + DelegatorAddress: Addr.String(), + ValidatorAddress: ValAddr.String(), Amount: sdk.NewCoin(sdk.DefaultBondDenom, shares.RoundInt()), CreationHeight: 10, }, From ca18381f067769cd7a1abd9ddce6492462624527 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 14 Apr 2023 16:49:19 +0530 Subject: [PATCH 12/14] remove unncessary expected keepers --- x/staking/testutil/expected_keepers_mocks.go | 28 -------------------- x/staking/types/expected_keepers.go | 2 -- 2 files changed, 30 deletions(-) diff --git a/x/staking/testutil/expected_keepers_mocks.go b/x/staking/testutil/expected_keepers_mocks.go index fa94ee7d1253..ba3036da7561 100644 --- a/x/staking/testutil/expected_keepers_mocks.go +++ b/x/staking/testutil/expected_keepers_mocks.go @@ -291,34 +291,6 @@ func (mr *MockBankKeeperMockRecorder) LockedCoins(ctx, addr interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockedCoins", reflect.TypeOf((*MockBankKeeper)(nil).LockedCoins), ctx, addr) } -// MintCoins mocks base method. -func (m *MockBankKeeper) MintCoins(ctx types.Context, moduleName string, amt types.Coins) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MintCoins", ctx, moduleName, amt) - ret0, _ := ret[0].(error) - return ret0 -} - -// MintCoins indicates an expected call of MintCoins. -func (mr *MockBankKeeperMockRecorder) MintCoins(ctx, moduleName, amt interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MintCoins", reflect.TypeOf((*MockBankKeeper)(nil).MintCoins), ctx, moduleName, amt) -} - -// SendCoinsFromModuleToAccount mocks base method. -func (m *MockBankKeeper) SendCoinsFromModuleToAccount(ctx types.Context, senderModule string, recipientAddr types.AccAddress, amt types.Coins) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SendCoinsFromModuleToAccount", ctx, senderModule, recipientAddr, amt) - ret0, _ := ret[0].(error) - return ret0 -} - -// SendCoinsFromModuleToAccount indicates an expected call of SendCoinsFromModuleToAccount. -func (mr *MockBankKeeperMockRecorder) SendCoinsFromModuleToAccount(ctx, senderModule, recipientAddr, amt interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoinsFromModuleToAccount", reflect.TypeOf((*MockBankKeeper)(nil).SendCoinsFromModuleToAccount), ctx, senderModule, recipientAddr, amt) -} - // SendCoinsFromModuleToModule mocks base method. func (m *MockBankKeeper) SendCoinsFromModuleToModule(ctx types.Context, senderPool, recipientPool string, amt types.Coins) error { m.ctrl.T.Helper() diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go index a4e8c80e7ff4..b70505d0ec30 100644 --- a/x/staking/types/expected_keepers.go +++ b/x/staking/types/expected_keepers.go @@ -36,10 +36,8 @@ type BankKeeper interface { LockedCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins - MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error GetSupply(ctx sdk.Context, denom string) sdk.Coin - SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error 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 From 7ed484235b6b5e1dc210ffc44d56b50c6d8e80d4 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 14 Apr 2023 17:30:08 +0530 Subject: [PATCH 13/14] fix failing test --- tests/integration/staking/keeper/msg_server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/staking/keeper/msg_server_test.go b/tests/integration/staking/keeper/msg_server_test.go index be8fa6ccfb1d..50b3e9d3c47e 100644 --- a/tests/integration/staking/keeper/msg_server_test.go +++ b/tests/integration/staking/keeper/msg_server_test.go @@ -110,7 +110,7 @@ func TestCancelUnbondingDelegation(t *testing.T) { DelegatorAddress: resUnbond.DelegatorAddress, ValidatorAddress: resUnbond.ValidatorAddress, Amount: sdk.NewCoin("dump_coin", sdk.NewInt(4)), - CreationHeight: 0, + CreationHeight: 10, }, expErrMsg: "invalid coin denomination", }, From 024c2b1db64a8ef3899d1b7a0243c7ba6f985522 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 14 Apr 2023 17:49:07 +0530 Subject: [PATCH 14/14] add nit --- x/staking/keeper/params.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x/staking/keeper/params.go b/x/staking/keeper/params.go index 3e1bb083916e..74316694d9bf 100644 --- a/x/staking/keeper/params.go +++ b/x/staking/keeper/params.go @@ -50,11 +50,8 @@ func (k Keeper) MinCommissionRate(ctx sdk.Context) math.LegacyDec { } // SetParams sets the x/staking module parameters. +// CONTRACT: This method performs no validation of the parameters. func (k Keeper) SetParams(ctx sdk.Context, params types.Params) error { - if err := params.Validate(); err != nil { - return err - } - store := ctx.KVStore(k.storeKey) bz, err := k.cdc.Marshal(¶ms) if err != nil {