LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix(x/consensus): harden consensus params proposal (#20381)

Browse Source 
Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: sontrinh16 <trinhleson2000@gmail.com>
This commit is contained in:
Marko 2024-05-28 12:25:56 +02:00 committed by GitHub
parent 443f153559
commit d180df817e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 125 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd.
* (runtime) [#20264](https://github.com/cosmos/cosmos-sdk/pull/20264) Expose grpc query router via depinject.
* (x/consensus) [#20381](https://github.com/cosmos/cosmos-sdk/pull/20381) Use Comet utility for consensus module consensus param updates.
* (client) [#20356](https://github.com/cosmos/cosmos-sdk/pull/20356) Overwrite client context when available in `SetCmdClientContext`.
### Bug Fixes

20
x/consensus/keeper/keeper.go
View File

@ -14,6 +14,7 @@ import (
"cosmossdk.io/errors"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/consensus/exported"
"github.com/cosmos/cosmos-sdk/x/consensus/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
@ -72,11 +73,26 @@ func (k Keeper) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) (*
if err != nil {
return nil, err
}
if err := cmttypes.ConsensusParamsFromProto(consensusParams).ValidateBasic(); err != nil {
paramsProto, err := k.ParamsStore.Get(ctx)
if err != nil {
return nil, err
}
params := cmttypes.ConsensusParamsFromProto(paramsProto)
nextParams := params.Update(&consensusParams)
if err := nextParams.ValidateBasic(); err != nil {
return nil, err
}
if err := k.ParamsStore.Set(ctx, consensusParams); err != nil {
sdkCtx := sdk.UnwrapSDKContext(ctx)
if err := params.ValidateUpdate(&consensusParams, sdkCtx.BlockHeader().Height); err != nil {
return nil, err
}
if err := k.ParamsStore.Set(ctx, nextParams.ToProto()); err != nil {
return nil, err
}

125
x/consensus/keeper/keeper_test.go
View File

@ -30,7 +30,8 @@ type KeeperTestSuite struct {
func (s *KeeperTestSuite) SetupTest() {
key := storetypes.NewKVStoreKey(consensusparamkeeper.StoreKey)
testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{})
header := cmtproto.Header{Height: 5}
ctx := testCtx.Ctx.WithBlockHeader(header)
encCfg := moduletestutil.MakeTestEncodingConfig()
storeService := runtime.NewKVStoreService(key)
@ -43,6 +44,8 @@ func (s *KeeperTestSuite) SetupTest() {
queryHelper := baseapp.NewQueryServerTestHelper(ctx, encCfg.InterfaceRegistry)
types.RegisterQueryServer(queryHelper, keeper)
s.queryClient = types.NewQueryClient(queryHelper)
err := s.consensusParamsKeeper.ParamsStore.Set(ctx, cmttypes.DefaultConsensusParams().ToProto())
s.Require().NoError(err)
}
func TestKeeperTestSuite(t *testing.T) {
@ -50,7 +53,14 @@ func TestKeeperTestSuite(t *testing.T) {
}
func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() {
defaultConsensusParams := cmttypes.DefaultConsensusParams().ToProto()
// Create ConsensusParams with modified fields
modifiedConsensusParams := cmttypes.DefaultConsensusParams().ToProto()
modifiedConsensusParams.Block.MaxBytes++
modifiedConsensusParams.Block.MaxGas = 100
modifiedConsensusParams.Evidence.MaxAgeDuration++
modifiedConsensusParams.Evidence.MaxAgeNumBlocks++
modifiedConsensusParams.Evidence.MaxBytes++
modifiedConsensusParams.Validator.PubKeyTypes = []string{cmttypes.ABCIPubKeyTypeSecp256k1}
testCases := []struct {
msg string
@ -65,18 +75,22 @@ func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() {
func() {
input := &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Block: modifiedConsensusParams.Block,
Validator: modifiedConsensusParams.Validator,
Evidence: modifiedConsensusParams.Evidence,
}
s.consensusParamsKeeper.UpdateParams(s.ctx, input)
_, err := s.consensusParamsKeeper.UpdateParams(s.ctx, input)
s.Require().NoError(err)
},
types.QueryParamsResponse{
Params: &cmtproto.ConsensusParams{
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Version: defaultConsensusParams.Version,
Block: modifiedConsensusParams.Block,
Validator: modifiedConsensusParams.Validator,
Evidence: modifiedConsensusParams.Evidence,
Version: modifiedConsensusParams.Version,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 0,
},
},
},
true,
@ -87,21 +101,22 @@ func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() {
func() {
input := &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Block: modifiedConsensusParams.Block,
Validator: modifiedConsensusParams.Validator,
Evidence: modifiedConsensusParams.Evidence,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 1234,
},
}
s.consensusParamsKeeper.UpdateParams(s.ctx, input)
_, err := s.consensusParamsKeeper.UpdateParams(s.ctx, input)
s.Require().NoError(err)
},
types.QueryParamsResponse{
Params: &cmtproto.ConsensusParams{
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Version: defaultConsensusParams.Version,
Block: modifiedConsensusParams.Block,
Validator: modifiedConsensusParams.Validator,
Evidence: modifiedConsensusParams.Evidence,
Version: modifiedConsensusParams.Version,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 1234,
},
@ -204,6 +219,76 @@ func (s *KeeperTestSuite) TestUpdateParams() {
expErr: true,
expErrMsg: "all parameters must be present",
},
{
name: "valid ABCI update",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 1235,
},
},
expErr: false,
expErrMsg: "",
},
{
name: "noop ABCI update",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 1235,
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid ABCI clear",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 0,
},
},
expErr: false,
expErrMsg: "",
},
{
name: "invalid ABCI update - current height",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 5,
},
},
expErr: true,
expErrMsg: "vote extensions cannot be updated to a past or current height",
},
{
name: "invalid ABCI update - past height",
input: &types.MsgUpdateParams{
Authority: s.consensusParamsKeeper.GetAuthority(),
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 4,
},
},
expErr: true,
expErrMsg: "vote extensions cannot be updated to a past or current height",
},
}
for _, tc := range testCases {
@ -220,7 +305,9 @@ func (s *KeeperTestSuite) TestUpdateParams() {
res, err := s.consensusParamsKeeper.Params(s.ctx, &types.QueryParamsRequest{})
s.Require().NoError(err)
s.Require().Equal(tc.input.Abci, res.Params.Abci)
if tc.input.Abci != nil {
s.Require().Equal(tc.input.Abci, res.Params.Abci)
}
s.Require().Equal(tc.input.Block, res.Params.Block)
s.Require().Equal(tc.input.Evidence, res.Params.Evidence)
s.Require().Equal(tc.input.Validator, res.Params.Validator)