refactor(x/consensus): audit QA (#21151)

cosmos · Aug 9, 2024 · a9a687c · a9a687c
1 parent bc698d8
commit a9a687c
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -57,7 +57,6 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (baseapp) [#18499](https://github.com/cosmos/cosmos-sdk/pull/18499) Add `MsgRouter` response type from message name function.
 * (types) [#18768](https://github.com/cosmos/cosmos-sdk/pull/18768) Add MustValAddressFromBech32 function.
 * (gRPC) [#19049](https://github.com/cosmos/cosmos-sdk/pull/19049) Add debug log prints for each gRPC request.
-* (x/consensus) [#19483](https://github.com/cosmos/cosmos-sdk/pull/19483) Add consensus messages registration to consensus module.
 * (types) [#19759](https://github.com/cosmos/cosmos-sdk/pull/19759) Align SignerExtractionAdapter in PriorityNonceMempool Remove.
 * (client) [#19870](https://github.com/cosmos/cosmos-sdk/pull/19870) Add new query command `wait-tx`. Alias `event-query-tx-for` to `wait-tx` for backward compatibility.
 * (crypto/keyring) [#20212](https://github.com/cosmos/cosmos-sdk/pull/20212) Expose the db keyring used in the keystore.
@@ -144,7 +143,6 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
     * The module manager now can do everything that the basic manager was doing.
     * When using runtime, just inject the module manager when needed using your app config.
     * All `AppModuleBasic` structs have been removed.
-* (x/consensus) [#19488](https://github.com/cosmos/cosmos-sdk/pull/19488) Consensus module creation takes `appmodule.Environment` instead of individual services.
 * (server) [#18303](https://github.com/cosmos/cosmos-sdk/pull/18303) `x/genutil` now handles the application export. `server.AddCommands` does not take an `AppExporter` but instead `genutilcli.Commands` does.
 * (x/gov/testutil) [#17986](https://github.com/cosmos/cosmos-sdk/pull/18036) `MsgDeposit` has been removed because of AutoCLI migration.
 * (x/staking/testutil) [#17986](https://github.com/cosmos/cosmos-sdk/pull/17986) `MsgRedelegateExec`, `MsgUnbondExec` has been removed because of AutoCLI migration.
@@ -174,7 +172,6 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (x/staking) [#18257](https://github.com/cosmos/cosmos-sdk/pull/18257) Staking module was moved to its own go.mod `cosmossdk.io/x/staking`
 * (x/authz) [#18265](https://github.com/cosmos/cosmos-sdk/pull/18265) Authz module was moved to its own go.mod `cosmossdk.io/x/authz`
 * (x/mint) [#18283](https://github.com/cosmos/cosmos-sdk/pull/18283) Mint module was moved to its own go.mod `cosmossdk.io/x/mint`
-* (x/consensus) [#18041](https://github.com/cosmos/cosmos-sdk/pull/18041) `ToProtoConsensusParams()` returns an error
 * (x/slashing) [#18115](https://github.com/cosmos/cosmos-sdk/pull/18115) `NewValidatorSigningInfo` takes strings instead of `sdk.AccAddress`
 * (types) [#18268](https://github.com/cosmos/cosmos-sdk/pull/18268) Remove global setting of basedenom. Use the staking module parameter instead
 * (x/auth) [#18351](https://github.com/cosmos/cosmos-sdk/pull/18351) Auth module was moved to its own go.mod `cosmossdk.io/x/auth`

diff --git a/api/cosmos/consensus/v1/tx.pulsar.go b/api/cosmos/consensus/v1/tx.pulsar.go
diff --git a/x/consensus/CHANGELOG.md b/x/consensus/CHANGELOG.md
@@ -31,4 +31,13 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### Features
+
+* (x/consensus) [#19483](https://github.com/cosmos/cosmos-sdk/pull/19483) Add consensus messages registration to consensus module.
 * [#20615](https://github.com/cosmos/cosmos-sdk/pull/20615) Add consensus messages to add cometinfo to consensus modules
+
+### API Breaking Changes
+
+* (x/consensus) [#19488](https://github.com/cosmos/cosmos-sdk/pull/19488) Consensus module creation takes `appmodule.Environment` instead of individual services.
+* (x/consensus) [#18041](https://github.com/cosmos/cosmos-sdk/pull/18041) `ToProtoConsensusParams()` returns an error
+
diff --git a/x/consensus/keeper/keeper.go b/x/consensus/keeper/keeper.go
@@ -19,8 +19,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 )
 
-var StoreKey = "Consensus"
-
 type Keeper struct {
 	appmodule.Environment
 

diff --git a/x/consensus/keeper/keeper_test.go b/x/consensus/keeper/keeper_test.go
@@ -38,7 +38,7 @@ func getDuration(d time.Duration) *time.Duration {
 }
 
 func (s *KeeperTestSuite) SetupTest(enabledFeatures bool) {
-	key := storetypes.NewKVStoreKey(consensusparamkeeper.StoreKey)
+	key := storetypes.NewKVStoreKey(types.StoreKey)
 	testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
 	ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Height: 5})
 	encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{})

diff --git a/x/consensus/proto/cosmos/consensus/v1/tx.proto b/x/consensus/proto/cosmos/consensus/v1/tx.proto
@@ -38,9 +38,9 @@ message MsgUpdateParams {
   cometbft.types.v1.EvidenceParams  evidence  = 3;
   cometbft.types.v1.ValidatorParams validator = 4;
 
-  // Since: cosmos-sdk 0.51
-  cometbft.types.v1.ABCIParams      abci = 5 [deprecated = true, (cosmos_proto.field_added_in) = "cosmos-sdk 0.50"];
-  cometbft.types.v1.SynchronyParams synchrony = 6 [(cosmos_proto.field_added_in) = "cosmos-sdk 0.51"];
+  cometbft.types.v1.ABCIParams abci = 5 [deprecated = true, (cosmos_proto.field_added_in) = "cosmos-sdk 0.50"];
+
+  cometbft.types.v1.SynchronyParams synchrony = 6 [(cosmos_proto.field_added_in) = "cosmos-sdk 0.52"];
   cometbft.types.v1.FeatureParams   feature   = 7 [(cosmos_proto.field_added_in) = "cosmos-sdk 0.52"];
 }
 

diff --git a/x/consensus/types/msgs.go b/x/consensus/types/msgs.go
@@ -31,37 +31,17 @@ func (msg MsgUpdateParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, e
 			PubKeyTypes: msg.Validator.PubKeyTypes,
 		},
 		Version:   cmttypes.DefaultConsensusParams().ToProto().Version, // Version is stored in x/upgrade
-		Feature:   &cmtproto.FeatureParams{},
-		Synchrony: &cmtproto.SynchronyParams{},
+		Feature:   msg.Feature,
+		Synchrony: msg.Synchrony,
 	}
 
 	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 cp.Feature == nil {
+			cp.Feature = &cmtproto.FeatureParams{}
 		}
-		if msg.Feature.PbtsEnableHeight != nil {
-			cp.Feature.PbtsEnableHeight = &types.Int64Value{
-				Value: msg.Feature.GetPbtsEnableHeight().GetValue(),
-			}
-		}
-	}
 
-	if msg.Synchrony != nil {
-		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
+		cp.Feature.VoteExtensionsEnableHeight = &types.Int64Value{
+			Value: msg.Abci.VoteExtensionsEnableHeight,
 		}
 	}
 

diff --git a/x/consensus/types/tx.pb.go b/x/consensus/types/tx.pb.go