diff --git a/testutil/integration/router.go b/testutil/integration/router.go index e921c40133..c3f0c95401 100644 --- a/testutil/integration/router.go +++ b/testutil/integration/router.go @@ -6,6 +6,7 @@ import ( cmtabcitypes "github.com/cometbft/cometbft/abci/types" cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" + cmttypes "github.com/cometbft/cometbft/types" dbm "github.com/cosmos/cosmos-db" "cosmossdk.io/core/address" @@ -93,13 +94,8 @@ func NewIntegrationApp( bApp.SetParamStore(consensusParamsKeeper.ParamsStore) consensusparamtypes.RegisterQueryServer(grpcRouter, consensusParamsKeeper) - _, err := consensusParamsKeeper.SetParams(sdkCtx, &consensusparamtypes.ConsensusMsgParams{ - Version: simtestutil.DefaultConsensusParams.Version, - Block: simtestutil.DefaultConsensusParams.Block, - Evidence: simtestutil.DefaultConsensusParams.Evidence, - Validator: simtestutil.DefaultConsensusParams.Validator, - Abci: simtestutil.DefaultConsensusParams.Abci, - }) + params := cmttypes.ConsensusParamsFromProto(*simtestutil.DefaultConsensusParams) // This fills up missing param sections + err := consensusParamsKeeper.ParamsStore.Set(sdkCtx, params.ToProto()) if err != nil { panic(fmt.Errorf("failed to set consensus params: %w", err)) } diff --git a/x/consensus/keeper/keeper.go b/x/consensus/keeper/keeper.go index 7fa30482a9..ebcf2dab53 100644 --- a/x/consensus/keeper/keeper.go +++ b/x/consensus/keeper/keeper.go @@ -69,11 +69,24 @@ 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 { + if err := params.ValidateUpdate(&consensusParams, k.HeaderService.HeaderInfo(ctx).Height); err != nil { + return nil, err + } + + if err := k.ParamsStore.Set(ctx, nextParams.ToProto()); err != nil { return nil, err } @@ -86,21 +99,3 @@ func (k Keeper) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) (* return &types.MsgUpdateParamsResponse{}, nil } - -// SetParams sets the consensus parameters on init of a chain. This is a consensus message. It can only be called by the consensus server -// This is used in the consensus message handler set in module.go. -func (k Keeper) SetParams(ctx context.Context, req *types.ConsensusMsgParams) (*types.ConsensusMsgParamsResponse, error) { - consensusParams, err := req.ToProtoConsensusParams() - if err != nil { - return nil, err - } - if err := cmttypes.ConsensusParamsFromProto(consensusParams).ValidateBasic(); err != nil { - return nil, err - } - - if err := k.ParamsStore.Set(ctx, consensusParams); err != nil { - return nil, err - } - - return &types.ConsensusMsgParamsResponse{}, nil -} diff --git a/x/consensus/keeper/keeper_test.go b/x/consensus/keeper/keeper_test.go index c286a9bd01..e71e727f15 100644 --- a/x/consensus/keeper/keeper_test.go +++ b/x/consensus/keeper/keeper_test.go @@ -2,12 +2,14 @@ package keeper_test import ( "testing" + "time" cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" cmttypes "github.com/cometbft/cometbft/types" gogotypes "github.com/cosmos/gogoproto/types" "github.com/stretchr/testify/suite" + "cosmossdk.io/core/header" "cosmossdk.io/log" storetypes "cosmossdk.io/store/types" authtypes "cosmossdk.io/x/auth/types" @@ -30,10 +32,15 @@ type KeeperTestSuite struct { queryClient types.QueryClient } -func (s *KeeperTestSuite) SetupTest() { +func getDuration(d time.Duration) *time.Duration { + dur := d + return &dur +} + +func (s *KeeperTestSuite) SetupTest(enabledFeatures bool) { key := storetypes.NewKVStoreKey(consensusparamkeeper.StoreKey) testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) - ctx := testCtx.Ctx + ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Height: 5}) encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}) env := runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()) @@ -49,6 +56,13 @@ func (s *KeeperTestSuite) SetupTest() { queryHelper := baseapp.NewQueryServerTestHelper(ctx, encCfg.InterfaceRegistry) types.RegisterQueryServer(queryHelper, keeper) s.queryClient = types.NewQueryClient(queryHelper) + params := cmttypes.DefaultConsensusParams() + if enabledFeatures { + params.Feature.VoteExtensionsEnableHeight = 5 + params.Feature.PbtsEnableHeight = 5 + } + err = s.consensusParamsKeeper.ParamsStore.Set(ctx, params.ToProto()) + s.Require().NoError(err) } func TestKeeperTestSuite(t *testing.T) { @@ -56,7 +70,18 @@ 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} + *modifiedConsensusParams.Synchrony.MessageDelay += time.Second + *modifiedConsensusParams.Synchrony.Precision += 100 * time.Millisecond + modifiedConsensusParams.Feature.VoteExtensionsEnableHeight.Value = 200 + modifiedConsensusParams.Feature.PbtsEnableHeight.Value = 100 testCases := []struct { msg string @@ -71,58 +96,51 @@ func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() { func() { input := &types.MsgUpdateParams{ Authority: s.consensusParamsKeeper.GetAuthority(), - Block: defaultConsensusParams.Block, - Validator: defaultConsensusParams.Validator, - Evidence: defaultConsensusParams.Evidence, - Abci: defaultConsensusParams.Abci, - Synchrony: defaultConsensusParams.Synchrony, - Feature: defaultConsensusParams.Feature, + Block: modifiedConsensusParams.Block, + Validator: modifiedConsensusParams.Validator, + Evidence: modifiedConsensusParams.Evidence, + Synchrony: modifiedConsensusParams.Synchrony, + Feature: modifiedConsensusParams.Feature, } _, 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, - Abci: defaultConsensusParams.Abci, - Synchrony: defaultConsensusParams.Synchrony, - Feature: defaultConsensusParams.Feature, + Block: modifiedConsensusParams.Block, + Validator: modifiedConsensusParams.Validator, + Evidence: modifiedConsensusParams.Evidence, + Version: modifiedConsensusParams.Version, + Synchrony: modifiedConsensusParams.Synchrony, + Feature: modifiedConsensusParams.Feature, }, }, true, }, { - "success with abci", + "success with (deprecated) ABCI", types.QueryParamsRequest{}, func() { input := &types.MsgUpdateParams{ Authority: s.consensusParamsKeeper.GetAuthority(), - Block: defaultConsensusParams.Block, - Validator: defaultConsensusParams.Validator, - Evidence: defaultConsensusParams.Evidence, - Abci: &cmtproto.ABCIParams{ //nolint: staticcheck // needs update in a follow up pr + Block: modifiedConsensusParams.Block, + Validator: modifiedConsensusParams.Validator, + Evidence: modifiedConsensusParams.Evidence, + Synchrony: modifiedConsensusParams.Synchrony, + Abci: &cmtproto.ABCIParams{ //nolint: staticcheck // testing backwards compatibility VoteExtensionsEnableHeight: 1234, }, - Synchrony: defaultConsensusParams.Synchrony, - Feature: &cmtproto.FeatureParams{ - VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 1234}, - PbtsEnableHeight: &gogotypes.Int64Value{Value: 0}, - }, } _, 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, - Abci: defaultConsensusParams.Abci, - Synchrony: defaultConsensusParams.Synchrony, + Block: modifiedConsensusParams.Block, + Validator: modifiedConsensusParams.Validator, + Evidence: modifiedConsensusParams.Evidence, + Version: modifiedConsensusParams.Version, + Synchrony: modifiedConsensusParams.Synchrony, Feature: &cmtproto.FeatureParams{ VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 1234}, PbtsEnableHeight: &gogotypes.Int64Value{Value: 0}, @@ -137,7 +155,7 @@ func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() { tc := tc s.Run(tc.msg, func() { - s.SetupTest() // reset + s.SetupTest(false) // reset tc.malleate() res, err := s.consensusParamsKeeper.Params(s.ctx, &tc.req) @@ -157,10 +175,11 @@ func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() { func (s *KeeperTestSuite) TestUpdateParams() { defaultConsensusParams := cmttypes.DefaultConsensusParams().ToProto() testCases := []struct { - name string - input *types.MsgUpdateParams - expErr bool - expErrMsg string + name string + enabledFeatures bool + input *types.MsgUpdateParams + expErr bool + expErrMsg string }{ { name: "valid params", @@ -169,7 +188,6 @@ func (s *KeeperTestSuite) TestUpdateParams() { Block: defaultConsensusParams.Block, Validator: defaultConsensusParams.Validator, Evidence: defaultConsensusParams.Evidence, - Abci: defaultConsensusParams.Abci, }, expErr: false, expErrMsg: "", @@ -181,7 +199,6 @@ func (s *KeeperTestSuite) TestUpdateParams() { Block: &cmtproto.BlockParams{MaxGas: -10, MaxBytes: -10}, Validator: defaultConsensusParams.Validator, Evidence: defaultConsensusParams.Evidence, - Abci: defaultConsensusParams.Abci, }, expErr: true, expErrMsg: "block.MaxBytes must be -1 or greater than 0. Got -10", @@ -193,7 +210,6 @@ func (s *KeeperTestSuite) TestUpdateParams() { Block: defaultConsensusParams.Block, Validator: defaultConsensusParams.Validator, Evidence: defaultConsensusParams.Evidence, - Abci: defaultConsensusParams.Abci, }, expErr: true, expErrMsg: "invalid authority", @@ -205,7 +221,6 @@ func (s *KeeperTestSuite) TestUpdateParams() { Block: defaultConsensusParams.Block, Validator: defaultConsensusParams.Validator, Evidence: nil, - Abci: defaultConsensusParams.Abci, }, expErr: true, expErrMsg: "all parameters must be present", @@ -217,7 +232,6 @@ func (s *KeeperTestSuite) TestUpdateParams() { Block: nil, Validator: defaultConsensusParams.Validator, Evidence: defaultConsensusParams.Evidence, - Abci: defaultConsensusParams.Abci, }, expErr: true, expErrMsg: "all parameters must be present", @@ -229,17 +243,325 @@ func (s *KeeperTestSuite) TestUpdateParams() { Block: defaultConsensusParams.Block, Validator: nil, Evidence: defaultConsensusParams.Evidence, - Abci: defaultConsensusParams.Abci, }, expErr: true, expErrMsg: "all parameters must be present", }, + { + name: "valid Feature update - vote extensions", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 300}, + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid Feature update - pbts", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: 150}, + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid Feature update - vote extensions + pbts", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 120}, + PbtsEnableHeight: &gogotypes.Int64Value{Value: 110}, + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid noop Feature update - vote extensions + pbts (enabled feature)", + enabledFeatures: true, + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 5}, + PbtsEnableHeight: &gogotypes.Int64Value{Value: 5}, + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid (deprecated) ABCI update", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Abci: &cmtproto.ABCIParams{ //nolint: staticcheck // testing backwards compatibility + VoteExtensionsEnableHeight: 90, + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "invalid Feature + (deprecated) ABCI vote extensions update", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Abci: &cmtproto.ABCIParams{ //nolint: staticcheck // testing backwards compatibility + VoteExtensionsEnableHeight: 3000, + }, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 3000}, + }, + }, + expErr: true, + expErrMsg: "abci in sections Feature and (deprecated) ABCI cannot be used simultaneously", + }, + { + name: "invalid vote extensions update - current height", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 5}, + }, + }, + expErr: true, + expErrMsg: "xtensions cannot be updated to a past or current height", + }, + { + name: "invalid pbts update - current height", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: 5}, + }, + }, + expErr: true, + expErrMsg: "PBTS cannot be updated to a past or current height", + }, + { + name: "invalid vote extensions update - past height", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 4}, + }, + }, + expErr: true, + expErrMsg: "xtensions cannot be updated to a past or current height", + }, + { + name: "invalid pbts update - past height", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: 5}, + }, + }, + expErr: true, + expErrMsg: "PBTS cannot be updated to a past or current height", + }, + { + name: "invalid vote extensions update - negative height", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: -1}, + }, + }, + expErr: true, + expErrMsg: "Feature.VoteExtensionsEnabledHeight cannot be negative", + }, + { + name: "invalid pbts update - negative height", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: -1}, + }, + }, + expErr: true, + expErrMsg: "Feature.PbtsEnableHeight cannot be negative", + }, + { + name: "invalid vote extensions update - enabled feature", + enabledFeatures: true, + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 25}, + }, + }, + expErr: true, + expErrMsg: "xtensions cannot be modified once enabledenabled", + }, + { + name: "invalid pbts update - enabled feature", + enabledFeatures: true, + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: 35}, + }, + }, + expErr: true, + expErrMsg: "PBTS cannot be modified once enabled", + }, + { + name: "valid Synchrony update - precision", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Synchrony: &cmtproto.SynchronyParams{ + Precision: getDuration(3 * time.Second), + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid Synchrony update - delay", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Synchrony: &cmtproto.SynchronyParams{ + MessageDelay: getDuration(10 * time.Second), + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid Synchrony update - precision + delay", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Synchrony: &cmtproto.SynchronyParams{ + Precision: getDuration(4 * time.Second), + MessageDelay: getDuration(11 * time.Second), + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid Synchrony update - 0 precision", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Synchrony: &cmtproto.SynchronyParams{ + Precision: getDuration(0), + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "valid Synchrony update - 0 delay", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Synchrony: &cmtproto.SynchronyParams{ + MessageDelay: getDuration(0), + }, + }, + expErr: false, + expErrMsg: "", + }, + { + name: "invalid Synchrony update - 0 precision with PBTS set", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: 20}, + }, + Synchrony: &cmtproto.SynchronyParams{ + Precision: getDuration(0), + }, + }, + expErr: true, + expErrMsg: "synchrony.Precision must be greater than 0", + }, + { + name: "invalid Synchrony update - 0 delay with PBTS set", + input: &types.MsgUpdateParams{ + Authority: s.consensusParamsKeeper.GetAuthority(), + Block: defaultConsensusParams.Block, + Validator: defaultConsensusParams.Validator, + Evidence: defaultConsensusParams.Evidence, + Feature: &cmtproto.FeatureParams{ + PbtsEnableHeight: &gogotypes.Int64Value{Value: 20}, + }, + Synchrony: &cmtproto.SynchronyParams{ + MessageDelay: getDuration(0), + }, + }, + expErr: true, + expErrMsg: "synchrony.MessageDelay must be greater than 0", + }, } for _, tc := range testCases { tc := tc s.Run(tc.name, func() { - s.SetupTest() + s.SetupTest(tc.enabledFeatures) _, err := s.consensusParamsKeeper.UpdateParams(s.ctx, tc.input) if tc.expErr { s.Require().Error(err) @@ -250,116 +572,33 @@ 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) 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) - } - }) - } -} - -func (s *KeeperTestSuite) TestSetParams() { - defaultConsensusParams := cmttypes.DefaultConsensusParams().ToProto() - testCases := []struct { - name string - input *types.ConsensusMsgParams - expErr bool - expErrMsg string - }{ - { - name: "valid params", - input: &types.ConsensusMsgParams{ - Abci: defaultConsensusParams.Abci, - Version: &cmtproto.VersionParams{App: 1}, - Block: defaultConsensusParams.Block, - Validator: defaultConsensusParams.Validator, - Evidence: defaultConsensusParams.Evidence, - }, - expErr: false, - expErrMsg: "", - }, - { - name: "invalid params", - input: &types.ConsensusMsgParams{ - Abci: defaultConsensusParams.Abci, - Version: &cmtproto.VersionParams{App: 1}, - Block: &cmtproto.BlockParams{MaxGas: -10, MaxBytes: -10}, - Validator: defaultConsensusParams.Validator, - Evidence: defaultConsensusParams.Evidence, - }, - expErr: true, - expErrMsg: "block.MaxBytes must be -1 or greater than 0. Got -10", - }, - { - name: "nil version params", - input: &types.ConsensusMsgParams{ - Abci: defaultConsensusParams.Abci, - Version: nil, - Block: defaultConsensusParams.Block, - Validator: defaultConsensusParams.Validator, - Evidence: defaultConsensusParams.Evidence, - }, - expErr: true, - expErrMsg: "all parameters must be present", - }, - { - name: "nil evidence params", - input: &types.ConsensusMsgParams{ - Abci: defaultConsensusParams.Abci, - Version: &cmtproto.VersionParams{App: 1}, - Block: defaultConsensusParams.Block, - Validator: defaultConsensusParams.Validator, - Evidence: nil, - }, - expErr: true, - expErrMsg: "all parameters must be present", - }, - { - name: "nil block params", - input: &types.ConsensusMsgParams{ - Abci: defaultConsensusParams.Abci, - Version: &cmtproto.VersionParams{App: 1}, - Block: nil, - Validator: defaultConsensusParams.Validator, - Evidence: defaultConsensusParams.Evidence, - }, - expErr: true, - expErrMsg: "all parameters must be present", - }, - { - name: "nil validator params", - input: &types.ConsensusMsgParams{ - Abci: defaultConsensusParams.Abci, - Version: &cmtproto.VersionParams{App: 1}, - Block: defaultConsensusParams.Block, - Validator: nil, - Evidence: defaultConsensusParams.Evidence, - }, - expErr: true, - expErrMsg: "all parameters must be present", - }, - } - - for _, tc := range testCases { - tc := tc - s.Run(tc.name, func() { - s.SetupTest() - _, err := s.consensusParamsKeeper.SetParams(s.ctx, tc.input) - if tc.expErr { - s.Require().Error(err) - s.Require().Contains(err.Error(), tc.expErrMsg) - } else { - s.Require().NoError(err) - - res, err := s.consensusParamsKeeper.Params(s.ctx, &types.QueryParamsRequest{}) - s.Require().NoError(err) - - 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) - s.Require().Equal(tc.input.Version, res.Params.Version) + if tc.input.Abci != nil { + s.Require().Equal(tc.input.Abci.VoteExtensionsEnableHeight, + res.Params.Feature.VoteExtensionsEnableHeight.GetValue()) + } + if tc.input.Feature != nil { + if tc.input.Feature.VoteExtensionsEnableHeight != nil { + s.Require().Equal(tc.input.Feature.VoteExtensionsEnableHeight.GetValue(), + res.Params.Feature.VoteExtensionsEnableHeight.GetValue()) + } + if tc.input.Feature.PbtsEnableHeight != nil { + s.Require().Equal(tc.input.Feature.PbtsEnableHeight.GetValue(), + res.Params.Feature.PbtsEnableHeight.GetValue()) + } + } + if tc.input.Synchrony != nil { + if tc.input.Synchrony.MessageDelay != nil { + s.Require().Equal(tc.input.Synchrony.MessageDelay, + res.Params.Synchrony.MessageDelay) + } + if tc.input.Synchrony.Precision != nil { + s.Require().Equal(tc.input.Synchrony.Precision, + res.Params.Synchrony.Precision) + } + } } }) } diff --git a/x/consensus/types/consensus.go b/x/consensus/types/consensus.go deleted file mode 100644 index 07ca2dee2c..0000000000 --- a/x/consensus/types/consensus.go +++ /dev/null @@ -1,44 +0,0 @@ -package types - -import ( - "errors" - - cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" - cmttypes "github.com/cometbft/cometbft/types" - gogotypes "github.com/cosmos/gogoproto/types" -) - -func (msg ConsensusMsgParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, error) { - if msg.Evidence == nil || msg.Block == nil || msg.Validator == nil || msg.Version == nil { - return cmtproto.ConsensusParams{}, errors.New("all parameters must be present") - } - - cp := cmtproto.ConsensusParams{ - Block: &cmtproto.BlockParams{ - MaxBytes: msg.Block.MaxBytes, - MaxGas: msg.Block.MaxGas, - }, - Evidence: &cmtproto.EvidenceParams{ - MaxAgeNumBlocks: msg.Evidence.MaxAgeNumBlocks, - MaxAgeDuration: msg.Evidence.MaxAgeDuration, - MaxBytes: msg.Evidence.MaxBytes, - }, - Validator: &cmtproto.ValidatorParams{ - PubKeyTypes: msg.Validator.PubKeyTypes, - }, - - Version: cmttypes.DefaultConsensusParams().ToProto().Version, // Version is stored in x/upgrade - } - - if msg.Abci != nil { - cp.Feature = &cmtproto.FeatureParams{ - VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: msg.Abci.VoteExtensionsEnableHeight}, - } - } - - if msg.Version != nil { - cp.Version.App = msg.Version.App - } - - return cp, nil -} diff --git a/x/consensus/types/msgs.go b/x/consensus/types/msgs.go index 04dc6e1133..71c950aa29 100644 --- a/x/consensus/types/msgs.go +++ b/x/consensus/types/msgs.go @@ -5,6 +5,7 @@ import ( cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" cmttypes "github.com/cometbft/cometbft/types" + "github.com/cosmos/gogoproto/types" ) func (msg MsgUpdateParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, error) { @@ -12,6 +13,10 @@ func (msg MsgUpdateParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, e return cmtproto.ConsensusParams{}, errors.New("all parameters must be present") } + if msg.Abci != nil && msg.Feature != nil && msg.Feature.VoteExtensionsEnableHeight != nil { + return cmtproto.ConsensusParams{}, errors.New("abci in sections Feature and (deprecated) ABCI cannot be used simultaneously") + } + cp := cmtproto.ConsensusParams{ Block: &cmtproto.BlockParams{ MaxBytes: msg.Block.MaxBytes, @@ -25,18 +30,38 @@ func (msg MsgUpdateParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, e Validator: &cmtproto.ValidatorParams{ PubKeyTypes: msg.Validator.PubKeyTypes, }, - Version: cmttypes.DefaultConsensusParams().ToProto().Version, // Version is stored in x/upgrade + Version: cmttypes.DefaultConsensusParams().ToProto().Version, // Version is stored in x/upgrade + Feature: &cmtproto.FeatureParams{}, + Synchrony: &cmtproto.SynchronyParams{}, } - if msg.Feature != nil && msg.Feature.VoteExtensionsEnableHeight != nil && msg.Feature.PbtsEnableHeight != nil { - cp.Feature = &cmtproto.FeatureParams{ - VoteExtensionsEnableHeight: msg.Feature.VoteExtensionsEnableHeight, - PbtsEnableHeight: msg.Feature.PbtsEnableHeight, + + if msg.Abci != nil { + cp.Feature.VoteExtensionsEnableHeight = &types.Int64Value{ + Value: msg.Abci.VoteExtensionsEnableHeight, } } + + if msg.Feature != nil { + if msg.Feature.VoteExtensionsEnableHeight != nil { + cp.Feature.VoteExtensionsEnableHeight = &types.Int64Value{ + Value: msg.Feature.GetVoteExtensionsEnableHeight().GetValue(), + } + } + if msg.Feature.PbtsEnableHeight != nil { + cp.Feature.PbtsEnableHeight = &types.Int64Value{ + Value: msg.Feature.GetPbtsEnableHeight().GetValue(), + } + } + } + if msg.Synchrony != nil { - cp.Synchrony = &cmtproto.SynchronyParams{ - Precision: msg.Synchrony.Precision, - MessageDelay: msg.Synchrony.MessageDelay, + if msg.Synchrony.MessageDelay != nil { + delay := *msg.Synchrony.MessageDelay + cp.Synchrony.MessageDelay = &delay + } + if msg.Synchrony.Precision != nil { + precision := *msg.Synchrony.Precision + cp.Synchrony.Precision = &precision } }