Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(consensus): add consensus message #19483

Merged
merged 13 commits into from
Feb 23, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (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.
* (server) [#19280](https://github.com/cosmos/cosmos-sdk/pull/19280) Adds in-place testnet CLI command.
* (x/consensus) [#19483](https://github.com/cosmos/cosmos-sdk/pull/19483) Add consensus messages registration to consensus module.

### Improvements

Expand Down
1,367 changes: 1,367 additions & 0 deletions api/cosmos/consensus/v1/consensus.pulsar.go

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions proto/cosmos/consensus/v1/consensus.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Since: cosmos-sdk 0.51
syntax = "proto3";
package cosmos.consensus.v1;

import "tendermint/types/params.proto";

option go_package = "github.com/cosmos/cosmos-sdk/x/consensus/types";

// ConsensusMsgParams is the Msg/Params request type. This is a consensus message that is sent from cometbft.
message ConsensusMsgParams {
// params defines the x/consensus parameters to be passed from comet.
//
// NOTE: All parameters must be supplied.
tendermint.types.VersionParams version = 1;
tendermint.types.BlockParams block = 2;
tendermint.types.EvidenceParams evidence = 3;
tendermint.types.ValidatorParams validator = 4;
tendermint.types.ABCIParams abci = 5;
}

// ConsensusMsgParamsResponse defines the response structure for executing a
// ConsensusMsgParams message.
message ConsensusMsgParamsResponse {}
72 changes: 72 additions & 0 deletions x/consensus/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,76 @@ sidebar_position: 1

# `x/consensus`

## Abstract

Comment on lines +7 to +8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The abstract succinctly introduces the functionality of modifying CometBFT's ABCI consensus parameters. However, it could be enhanced by briefly explaining the significance of this capability in the broader context of blockchain consensus mechanisms.

Consider expanding the abstract to briefly explain why modifying ABCI consensus parameters is crucial for blockchain networks using CometBFT, highlighting its impact on network flexibility, security, and performance.

Functionality to modify CometBFT's ABCI consensus params.

## Contents

* [State](#state)
* [Params](#params)
* [Keepers](#keepers)
* [Messages](#messages)
* [Consensus Messages](#consensus-messages)
* [Events](#events)
* [Message Events](#message-events)
* [Keeper Events](#keeper-events)
* [Parameters](#parameters)
* [SendEnabled](#sendenabled)
* [DefaultSendEnabled](#defaultsendenabled)


## State

The `x/consensus` module keeps state of the consensus params from cometbft.:
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The State section provides a clear but brief description of the module's state management concerning consensus parameters. It would be beneficial to include more detail on how these parameters influence CometBFT's behavior and the overall consensus process.

Consider adding a sentence or two explaining the implications of consensus parameter state changes on CometBFT's operation, such as how they can affect block validation, transaction throughput, or network security.


## Params

The bank module stores it's params in state with the prefix of `0x05`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/bank/consensus

it can be updated with governance or the address with authority.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a minor inconsistency in the documentation under the Params section. It mentions the "bank module" instead of the "consensus module," which could confuse readers.

- The bank module stores it's params in state with the prefix of `0x05`,
+ The consensus module stores its params in state with the prefix of `0x05`,

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
The bank module stores it's params in state with the prefix of `0x05`,
it can be updated with governance or the address with authority.
The consensus module stores its params in state with the prefix of `0x05`,
it can be updated with governance or the address with authority.


* Params: `0x05 | ProtocolBuffer(cometbft.ConsensusParams)`

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/381de6452693a9338371223c232fba0c42773a4b/proto/cosmos/consensus/v1/consensus.proto#L11-L18
```

## Keepers

The consensus module provides methods to Set and Get consensus params. It is recommended to use the `x/consensus` module keeper to get consensus params instead of accessing them through the context.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Keepers section mentions the provision of methods to set and get consensus parameters but lacks an example. Including a code snippet would significantly enhance comprehension and usability for developers.

Consider adding an example code snippet showing how to use the x/consensus module keeper for setting or getting consensus parameters, demonstrating the practical application of these methods.


## Messages

### UpdateParams

Update consensus params.

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/381de6452693a9338371223c232fba0c42773a4b/proto/cosmos/consensus/v1/tx.proto#L12-L47
```

The message will fail under the following conditions:

* The signer is not the set authority
* Not all values are set

Comment on lines +51 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Messages section outlines the conditions under which the UpdateParams message will fail but does not specify what values can be set or their significance. Providing this information would greatly aid understanding and usability.

Consider adding details about the specific consensus parameters that can be updated with the UpdateParams message, including their significance and potential impact on the network's operation.

## Consensus Messages

The consensus module has a consensus message that is used to set the consensus params when the chain initializes. It is similar to the `UpdateParams` message but it is only used once at the start of the chain.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Consensus Messages section introduces a crucial functionality for chain initialization but lacks detail on how this differs from regular parameter updates post-initialization. Clarifying this distinction could provide valuable context for developers.

Consider adding a brief explanation of how consensus messages during chain initialization differ from regular consensus parameter updates, highlighting why this distinction is important for the initial setup of a blockchain network.


```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/381de6452693a9338371223c232fba0c42773a4b/proto/cosmos/consensus/v1/consensus.proto#L9-L24
```

## Events

The consensus module emits the following events:

### Message Events

#### MsgUpdateParams

| Type | Attribute Key | Attribute Value |
|--------|---------------|---------------------|
| string | authority | msg.Signer |
| string | parameters | consensus Parmeters |
Comment on lines +66 to +75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Events section, particularly the Message Events subsection, is well-documented. However, including examples of how these events can be subscribed to or used within the Cosmos SDK framework would enhance practical understanding.

Consider adding examples or guidance on subscribing to or handling these events within the Cosmos SDK framework, providing developers with actionable insights into leveraging these events for custom logic or monitoring.

18 changes: 18 additions & 0 deletions x/consensus/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,21 @@ 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
}
114 changes: 114 additions & 0 deletions x/consensus/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: defaultConsensusParams.Abci,
}
_, err := s.consensusParamsKeeper.UpdateParams(s.ctx, input)
s.Require().NoError(err)
Expand All @@ -79,6 +80,7 @@
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Version: defaultConsensusParams.Version,
Abci: defaultConsensusParams.Abci,
},
},
true,
Expand Down Expand Up @@ -150,6 +152,7 @@
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: defaultConsensusParams.Abci,
},
expErr: false,
expErrMsg: "",
Expand All @@ -161,6 +164,7 @@
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",
Expand All @@ -172,6 +176,7 @@
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: defaultConsensusParams.Abci,
},
expErr: true,
expErrMsg: "invalid authority",
Expand All @@ -183,6 +188,7 @@
Block: defaultConsensusParams.Block,
Validator: defaultConsensusParams.Validator,
Evidence: nil,
Abci: defaultConsensusParams.Abci,
},
expErr: true,
expErrMsg: "all parameters must be present",
Expand All @@ -194,6 +200,7 @@
Block: nil,
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
Abci: defaultConsensusParams.Abci,
},
expErr: true,
expErrMsg: "all parameters must be present",
Expand All @@ -205,6 +212,7 @@
Block: defaultConsensusParams.Block,
Validator: nil,
Evidence: defaultConsensusParams.Evidence,
Abci: defaultConsensusParams.Abci,
},
expErr: true,
expErrMsg: "all parameters must be present",
Expand Down Expand Up @@ -233,3 +241,109 @@
})
}
}

func (s *KeeperTestSuite) TestSetParams() {
defaultConsensusParams := cmttypes.DefaultConsensusParams().ToProto()
testCases := []struct {
name string
input *types.ConMsgParams

Check failure on line 249 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 249 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams

Check failure on line 249 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams

Check failure on line 249 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams
expErr bool
expErrMsg string
}{
{
name: "valid params",
input: &types.ConMsgParams{

Check failure on line 255 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 255 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams

Check failure on line 255 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams

Check failure on line 255 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams
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.ConMsgParams{

Check failure on line 267 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 267 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams

Check failure on line 267 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams

Check failure on line 267 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams
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.ConMsgParams{

Check failure on line 279 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 279 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams

Check failure on line 279 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams
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.ConMsgParams{

Check failure on line 291 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 291 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams

Check failure on line 291 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams
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.ConMsgParams{

Check failure on line 303 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 303 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams

Check failure on line 303 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams
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.ConMsgParams{

Check failure on line 315 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.ConMsgParams

Check failure on line 315 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: types.ConMsgParams (typecheck)

Check failure on line 315 in x/consensus/keeper/keeper_test.go

View workflow job for this annotation

GitHub Actions / Analyze

undefined: types.ConMsgParams (typecheck)
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)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
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)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
s.Require().Equal(tc.input.Version, res.Params.Version)
}
})
}
}
5 changes: 5 additions & 0 deletions x/consensus/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,8 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule {

// ConsensusVersion implements AppModule/ConsensusVersion.
func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }

// RegisterConsensusMessages registers the consensus module's messages.
func (am AppModule) RegisterConsensusMessages(builder any) {
// std.RegisterConsensusHandler(builder ,am.keeper.SetParams) // TODO uncomment when api is available
}
42 changes: 42 additions & 0 deletions x/consensus/types/consensus.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package types

import (
"errors"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
cmttypes "github.com/cometbft/cometbft/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.Abci = &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: msg.Abci.VoteExtensionsEnableHeight,
}
}

if msg.Version != nil {
cp.Version.App = msg.Version.App
}

return cp, nil
}
Loading
Loading