From 7ded32163bb39b366789032e73e96f52a072964f Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Fri, 9 Dec 2022 15:30:44 +0530 Subject: [PATCH] chore: x/slashing audit changes (#14211) ## Description ref: #14158 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- x/slashing/keeper/grpc_query.go | 3 + x/slashing/keeper/keeper_test.go | 23 ++++ x/slashing/keeper/msg_server.go | 2 + x/slashing/keeper/msg_server_test.go | 184 +++++++++++++++++++++++++-- x/slashing/types/codec.go | 1 + x/slashing/types/msg.go | 9 +- x/slashing/types/params.go | 2 +- x/slashing/types/signing_info.go | 6 +- 8 files changed, 215 insertions(+), 15 deletions(-) diff --git a/x/slashing/keeper/grpc_query.go b/x/slashing/keeper/grpc_query.go index 324fcd1cd12c..5f49a7b724bf 100644 --- a/x/slashing/keeper/grpc_query.go +++ b/x/slashing/keeper/grpc_query.go @@ -14,6 +14,7 @@ import ( var _ types.QueryServer = Keeper{} +// Params returns parameters of x/slashing module func (k Keeper) Params(c context.Context, req *types.QueryParamsRequest) (*types.QueryParamsResponse, error) { if req == nil { return nil, status.Errorf(codes.InvalidArgument, "empty request") @@ -25,6 +26,7 @@ func (k Keeper) Params(c context.Context, req *types.QueryParamsRequest) (*types return &types.QueryParamsResponse{Params: params}, nil } +// SigningInfo returns signing-info of a specific validator. func (k Keeper) SigningInfo(c context.Context, req *types.QuerySigningInfoRequest) (*types.QuerySigningInfoResponse, error) { if req == nil { return nil, status.Errorf(codes.InvalidArgument, "empty request") @@ -48,6 +50,7 @@ func (k Keeper) SigningInfo(c context.Context, req *types.QuerySigningInfoReques return &types.QuerySigningInfoResponse{ValSigningInfo: signingInfo}, nil } +// SigningInfos returns signing-infos of all validators. func (k Keeper) SigningInfos(c context.Context, req *types.QuerySigningInfosRequest) (*types.QuerySigningInfosResponse, error) { if req == nil { return nil, status.Errorf(codes.InvalidArgument, "empty request") diff --git a/x/slashing/keeper/keeper_test.go b/x/slashing/keeper/keeper_test.go index 583d157e2b49..e2d338d6dddd 100644 --- a/x/slashing/keeper/keeper_test.go +++ b/x/slashing/keeper/keeper_test.go @@ -19,6 +19,7 @@ import ( slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper" slashingtestutil "github.com/cosmos/cosmos-sdk/x/slashing/testutil" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" ) var consAddr = sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________"))) @@ -74,6 +75,28 @@ func (s *KeeperTestSuite) TestPubkey() { require.Equal(pubKey, expectedPubKey) } +func (s *KeeperTestSuite) TestJailAndSlash() { + s.stakingKeeper.EXPECT().Slash(s.ctx, + consAddr, + s.ctx.BlockHeight(), + sdk.TokensToConsensusPower(sdk.NewInt(1), sdk.DefaultPowerReduction), + s.slashingKeeper.SlashFractionDoubleSign(s.ctx), + types.Infraction_INFRACTION_DOUBLE_SIGN, + ).Return(sdk.NewInt(0)) + + s.slashingKeeper.Slash( + s.ctx, + consAddr, + s.slashingKeeper.SlashFractionDoubleSign(s.ctx), + sdk.TokensToConsensusPower(sdk.NewInt(1), sdk.DefaultPowerReduction), + s.ctx.BlockHeight(), + types.Infraction_INFRACTION_DOUBLE_SIGN, + ) + + s.stakingKeeper.EXPECT().Jail(s.ctx, consAddr).Return() + s.slashingKeeper.Jail(s.ctx, consAddr) +} + func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } diff --git a/x/slashing/keeper/msg_server.go b/x/slashing/keeper/msg_server.go index 31ec48f0c7a5..d3852d30b680 100644 --- a/x/slashing/keeper/msg_server.go +++ b/x/slashing/keeper/msg_server.go @@ -21,6 +21,8 @@ func NewMsgServerImpl(keeper Keeper) types.MsgServer { return &msgServer{Keeper: keeper} } +// UpdateParams implements MsgServer.UpdateParams method. +// It defines a method to update the x/slashing module parameters. func (k msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) { if k.authority != req.Authority { return nil, errors.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority, req.Authority) diff --git a/x/slashing/keeper/msg_server_test.go b/x/slashing/keeper/msg_server_test.go index 025504b922f8..5db479c20976 100644 --- a/x/slashing/keeper/msg_server_test.go +++ b/x/slashing/keeper/msg_server_test.go @@ -3,10 +3,10 @@ package keeper_test import ( "time" - "github.com/golang/mock/gomock" - + "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" ) func (s *KeeperTestSuite) TestUpdateParams() { @@ -144,13 +144,179 @@ func (s *KeeperTestSuite) TestUpdateParams() { } func (s *KeeperTestSuite) TestUnjail() { - addr := sdk.AccAddress([]byte("val1_______________")) - request := &slashingtypes.MsgUnjail{ - ValidatorAddr: sdk.ValAddress(addr).String(), + testCases := []struct { + name string + malleate func() *slashingtypes.MsgUnjail + expErr bool + expErrMsg string + }{ + { + name: "no self delegation: invalid request", + malleate: func() *slashingtypes.MsgUnjail { + _, pubKey, addr := testdata.KeyTestPubAddr() + valAddr := sdk.ValAddress(addr) + val, err := types.NewValidator(valAddr, pubKey, types.Description{Moniker: "test"}) + s.Require().NoError(err) + + s.stakingKeeper.EXPECT().Validator(s.ctx, valAddr).Return(val) + s.stakingKeeper.EXPECT().Delegation(s.ctx, addr, valAddr).Return(nil) + + return &slashingtypes.MsgUnjail{ + ValidatorAddr: sdk.ValAddress(addr).String(), + } + }, + expErr: true, + expErrMsg: "validator has no self-delegation", + }, + { + name: "validator not in the state: invalid request", + malleate: func() *slashingtypes.MsgUnjail { + _, _, addr := testdata.KeyTestPubAddr() + valAddr := sdk.ValAddress(addr) + + s.stakingKeeper.EXPECT().Validator(s.ctx, valAddr).Return(nil) + + return &slashingtypes.MsgUnjail{ + ValidatorAddr: valAddr.String(), + } + }, + expErr: true, + expErrMsg: "address is not associated with any known validator", + }, + { + name: "validator not jailed: invalid request", + malleate: func() *slashingtypes.MsgUnjail { + _, pubKey, addr := testdata.KeyTestPubAddr() + valAddr := sdk.ValAddress(addr) + + val, err := types.NewValidator(valAddr, pubKey, types.Description{Moniker: "test"}) + val.Tokens = sdk.NewInt(1000) + val.DelegatorShares = sdk.NewDec(1) + val.Jailed = false + + s.Require().NoError(err) + + info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3), + time.Unix(2, 0), false, int64(10)) + + s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info) + + s.stakingKeeper.EXPECT().Validator(s.ctx, valAddr).Return(val) + del := types.NewDelegation(addr, valAddr, sdk.NewDec(100)) + + s.stakingKeeper.EXPECT().Delegation(s.ctx, addr, valAddr).Return(del) + + return &slashingtypes.MsgUnjail{ + ValidatorAddr: sdk.ValAddress(addr).String(), + } + }, + expErr: true, + expErrMsg: "validator not jailed", + }, + { + name: "validator tombstoned: invalid request", + malleate: func() *slashingtypes.MsgUnjail { + _, pubKey, addr := testdata.KeyTestPubAddr() + valAddr := sdk.ValAddress(addr) + + val, err := types.NewValidator(valAddr, pubKey, types.Description{Moniker: "test"}) + val.Tokens = sdk.NewInt(1000) + val.DelegatorShares = sdk.NewDec(1) + val.Jailed = true + + s.Require().NoError(err) + + info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3), + time.Unix(2, 0), true, int64(10)) + + s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info) + + s.stakingKeeper.EXPECT().Validator(s.ctx, valAddr).Return(val) + del := types.NewDelegation(addr, valAddr, sdk.NewDec(100)) + + s.stakingKeeper.EXPECT().Delegation(s.ctx, addr, valAddr).Return(del) + + return &slashingtypes.MsgUnjail{ + ValidatorAddr: sdk.ValAddress(addr).String(), + } + }, + expErr: true, + expErrMsg: "validator still jailed; cannot be unjailed", + }, + { + name: "unjailing before wait period: invalid request", + malleate: func() *slashingtypes.MsgUnjail { + _, pubKey, addr := testdata.KeyTestPubAddr() + valAddr := sdk.ValAddress(addr) + + val, err := types.NewValidator(valAddr, pubKey, types.Description{Moniker: "test"}) + val.Tokens = sdk.NewInt(1000) + val.DelegatorShares = sdk.NewDec(1) + val.Jailed = true + + s.Require().NoError(err) + + info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3), + s.ctx.BlockTime().AddDate(0, 0, 1), false, int64(10)) + + s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info) + + s.stakingKeeper.EXPECT().Validator(s.ctx, valAddr).Return(val) + del := types.NewDelegation(addr, valAddr, sdk.NewDec(10000)) + + s.stakingKeeper.EXPECT().Delegation(s.ctx, addr, valAddr).Return(del) + + return &slashingtypes.MsgUnjail{ + ValidatorAddr: sdk.ValAddress(addr).String(), + } + }, + expErr: true, + expErrMsg: "validator still jailed; cannot be unjailed", + }, + { + name: "valid request", + malleate: func() *slashingtypes.MsgUnjail { + _, pubKey, addr := testdata.KeyTestPubAddr() + valAddr := sdk.ValAddress(addr) + + val, err := types.NewValidator(valAddr, pubKey, types.Description{Moniker: "test"}) + val.Tokens = sdk.NewInt(1000) + val.DelegatorShares = sdk.NewDec(1) + + val.Jailed = true + s.Require().NoError(err) + + info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3), + time.Unix(2, 0), false, int64(10)) + + s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info) + + s.stakingKeeper.EXPECT().Validator(s.ctx, valAddr).Return(val) + del := types.NewDelegation(addr, valAddr, sdk.NewDec(100)) + + s.stakingKeeper.EXPECT().Delegation(s.ctx, addr, valAddr).Return(del) + s.stakingKeeper.EXPECT().Unjail(s.ctx, sdk.ConsAddress(addr)).Return() + + return &slashingtypes.MsgUnjail{ + ValidatorAddr: sdk.ValAddress(addr).String(), + } + }, + expErr: false, + }, } - s.stakingKeeper.EXPECT().Validator(gomock.Any(), gomock.Any()).Return(nil) - _, err := s.msgServer.Unjail(s.ctx, request) - s.Require().Error(err) - s.Require().Equal(err, slashingtypes.ErrNoValidatorForAddress) + for _, tc := range testCases { + tc := tc + s.Run(tc.name, func() { + req := tc.malleate() + _, err := s.msgServer.Unjail(s.ctx, req) + + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.expErrMsg) + } else { + s.Require().NoError(err) + } + }) + } } diff --git a/x/slashing/types/codec.go b/x/slashing/types/codec.go index f2e5efa65d9f..ce9fb7e48970 100644 --- a/x/slashing/types/codec.go +++ b/x/slashing/types/codec.go @@ -19,6 +19,7 @@ func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { legacy.RegisterAminoMsg(cdc, &MsgUpdateParams{}, "cosmos-sdk/x/slashing/MsgUpdateParams") } +// RegisterInterfaces registers the interfaces types with the Interface Registry. func RegisterInterfaces(registry types.InterfaceRegistry) { registry.RegisterImplementations((*sdk.Msg)(nil), &MsgUnjail{}, diff --git a/x/slashing/types/msg.go b/x/slashing/types/msg.go index c3756eb0580f..4f958ec1a456 100644 --- a/x/slashing/types/msg.go +++ b/x/slashing/types/msg.go @@ -25,8 +25,13 @@ func NewMsgUnjail(validatorAddr sdk.ValAddress) *MsgUnjail { } } +// Route implements the sdk.Msg interface. func (msg MsgUnjail) Route() string { return RouterKey } -func (msg MsgUnjail) Type() string { return TypeMsgUnjail } + +// Type implements the sdk.Msg interface. +func (msg MsgUnjail) Type() string { return TypeMsgUnjail } + +// GetSigners returns the expected signers for MsgUnjail. func (msg MsgUnjail) GetSigners() []sdk.AccAddress { valAddr, _ := sdk.ValAddressFromBech32(msg.ValidatorAddr) return []sdk.AccAddress{sdk.AccAddress(valAddr)} @@ -38,7 +43,7 @@ func (msg MsgUnjail) GetSignBytes() []byte { return sdk.MustSortJSON(bz) } -// ValidateBasic does a sanity check on the provided message +// ValidateBasic does a sanity check on the provided message. func (msg MsgUnjail) ValidateBasic() error { if _, err := sdk.ValAddressFromBech32(msg.ValidatorAddr); err != nil { return sdkerrors.ErrInvalidAddress.Wrapf("validator input address: %s", err) diff --git a/x/slashing/types/params.go b/x/slashing/types/params.go index 934489a48681..ec7ef191b6dc 100644 --- a/x/slashing/types/params.go +++ b/x/slashing/types/params.go @@ -46,7 +46,7 @@ func DefaultParams() Params { ) } -// validate params +// Validate validates the params func (p Params) Validate() error { if err := validateSignedBlocksWindow(p.SignedBlocksWindow); err != nil { return err diff --git a/x/slashing/types/signing_info.go b/x/slashing/types/signing_info.go index f3a06f767b16..27ed54e0b819 100644 --- a/x/slashing/types/signing_info.go +++ b/x/slashing/types/signing_info.go @@ -11,11 +11,11 @@ import ( // //nolint:interfacer func NewValidatorSigningInfo( - condAddr sdk.ConsAddress, startHeight, indexOffset int64, + consAddr sdk.ConsAddress, startHeight, indexOffset int64, jailedUntil time.Time, tombstoned bool, missedBlocksCounter int64, ) ValidatorSigningInfo { return ValidatorSigningInfo{ - Address: condAddr.String(), + Address: consAddr.String(), StartHeight: startHeight, IndexOffset: indexOffset, JailedUntil: jailedUntil, @@ -24,7 +24,7 @@ func NewValidatorSigningInfo( } } -// unmarshal a validator signing info from a store value +// UnmarshalValSigningInfo unmarshals a validator signing info from a store value func UnmarshalValSigningInfo(cdc codec.Codec, value []byte) (signingInfo ValidatorSigningInfo, err error) { err = cdc.Unmarshal(value, &signingInfo) return signingInfo, err