Skip to content

Commit

Permalink
chore(x/consensus): check for nil params (backport #18041) (#18069)
Browse files Browse the repository at this point in the history
Co-authored-by: Marko <[email protected]>
  • Loading branch information
mergify[bot] and tac0turtle authored Oct 11, 2023
1 parent 02c403f commit d24d469
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 5 deletions.
53 changes: 48 additions & 5 deletions x/consensus/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ func (s *KeeperTestSuite) TestUpdateParams() {
name string
input *types.MsgUpdateParams
expErr bool
expPanic bool
expErrMsg string
}{
{
Expand Down Expand Up @@ -47,18 +48,60 @@ func (s *KeeperTestSuite) TestUpdateParams() {
expErr: true,
expErrMsg: "invalid authority",
},
{
name: "nil evidence params",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: nil,
},
expErr: false,
expPanic: true,
expErrMsg: "all parameters must be present",
},
{
name: "nil block params",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: nil,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
},
expErr: false,
expPanic: true,
expErrMsg: "all parameters must be present",
},
{
name: "nil validator params",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: nil,
Evidence: defaultConsensusParams.Evidence,
},
expErr: false,
expPanic: true,
expErrMsg: "all parameters must be present",
},
}

for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
s.SetupTest()
_, err := s.msgServer.UpdateParams(s.ctx, tc.input)
if tc.expErr {
s.Require().Error(err)
s.Require().Contains(err.Error(), tc.expErrMsg)
if tc.expPanic {
s.Require().Panics(func() {
s.msgServer.UpdateParams(s.ctx, tc.input)
})
} else {
s.Require().NoError(err)
_, err := s.msgServer.UpdateParams(s.ctx, tc.input)
if tc.expErr {
s.Require().Error(err)
s.Require().Contains(err.Error(), tc.expErrMsg)
} else {
s.Require().NoError(err)
}
}
})
}
Expand Down
6 changes: 6 additions & 0 deletions x/consensus/types/msgs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package types

import (
"errors"

tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
tmtypes "github.com/cometbft/cometbft/types"

Expand Down Expand Up @@ -42,6 +44,10 @@ func (msg MsgUpdateParams) ValidateBasic() error {
}

func (msg MsgUpdateParams) ToProtoConsensusParams() tmproto.ConsensusParams {
if msg.Evidence == nil || msg.Block == nil || msg.Validator == nil {
panic(errors.New("all parameters must be present"))
}

return tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxBytes: msg.Block.MaxBytes,
Expand Down

0 comments on commit d24d469

Please sign in to comment.