From d24d469a34b7ce5193a2240acde6a64e26a78c74 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 11 Oct 2023 21:38:11 +0200 Subject: [PATCH] chore(x/consensus): check for nil params (backport #18041) (#18069) Co-authored-by: Marko --- x/consensus/keeper/msg_server_test.go | 53 ++++++++++++++++++++++++--- x/consensus/types/msgs.go | 6 +++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/x/consensus/keeper/msg_server_test.go b/x/consensus/keeper/msg_server_test.go index 2c2745ba962a..28c5307aba5c 100644 --- a/x/consensus/keeper/msg_server_test.go +++ b/x/consensus/keeper/msg_server_test.go @@ -12,6 +12,7 @@ func (s *KeeperTestSuite) TestUpdateParams() { name string input *types.MsgUpdateParams expErr bool + expPanic bool expErrMsg string }{ { @@ -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) + } } }) } diff --git a/x/consensus/types/msgs.go b/x/consensus/types/msgs.go index 167a7b76cc27..5fdc34baf6b9 100644 --- a/x/consensus/types/msgs.go +++ b/x/consensus/types/msgs.go @@ -1,6 +1,8 @@ package types import ( + "errors" + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" tmtypes "github.com/cometbft/cometbft/types" @@ -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,