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

refactor(x/consensus): audit QA v0.52 (#21416)

Browse Source
This commit is contained in:
Julián Toledano 2024-08-27 13:22:47 +02:00 committed by GitHub
parent 355f748add
commit 8e0efbdff9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 25 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
x/consensus/README.md
View File

@ -31,7 +31,7 @@ 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
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/consensus/proto/cosmos/consensus/v1/consensus.proto#L9-L15
```
## Keepers
@ -45,7 +45,7 @@ The consensus module provides methods to Set and Get consensus params. It is rec
Update consensus params.
```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/381de6452693a9338371223c232fba0c42773a4b/proto/cosmos/consensus/v1/tx.proto#L12-L47
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/consensus/proto/cosmos/consensus/v1/tx.proto#L23-L44
```
The message will fail under the following conditions:
@ -53,14 +53,6 @@ The message will fail under the following conditions:
* The signer is not the set authority
* Not all values are set
## 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.
```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:

17
x/consensus/exported/exported.go
View File

@ -6,13 +6,10 @@ import (
cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
)
type (
// ConsensusParamSetter defines the interface fulfilled by BaseApp's
// ParamStore which allows setting its appVersion field.
ConsensusParamSetter interface {
Get(ctx context.Context) (cmtproto.ConsensusParams, error)
Has(ctx context.Context) (bool, error)
Set(ctx context.Context, cp cmtproto.ConsensusParams) error
}
)
// ConsensusParamSetter defines the interface fulfilled by BaseApp's
// ParamStore which allows setting its appVersion field.
type ConsensusParamSetter interface {
Get(ctx context.Context) (cmtproto.ConsensusParams, error)
Has(ctx context.Context) (bool, error)
Set(ctx context.Context, cp cmtproto.ConsensusParams) error
}

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

@ -29,6 +29,7 @@ type Keeper struct {
var _ exported.ConsensusParamSetter = Keeper{}.ParamsStore
// NewKeeper creates a new Keeper instance.
func NewKeeper(cdc codec.BinaryCodec, env appmodule.Environment, authority string) Keeper {
sb := collections.NewSchemaBuilder(env.KVStoreService)
return Keeper{
@ -38,6 +39,8 @@ func NewKeeper(cdc codec.BinaryCodec, env appmodule.Environment, authority strin
}
}
// GetAuthority returns the authority address for the consensus module.
// This address has the permission to update consensus parameters.
func (k *Keeper) GetAuthority() string {
return k.authority
}
@ -45,14 +48,10 @@ func (k *Keeper) GetAuthority() string {
// InitGenesis initializes the initial state of the module
func (k *Keeper) InitGenesis(ctx context.Context) error {
value, ok := ctx.Value(corecontext.InitInfoKey).(*types.MsgUpdateParams)
if !ok {
if !ok || value == nil {
// no error for appv1 and appv2
return nil
}
if value == nil {
// no error for appv1
return nil
}
consensusParams, err := value.ToProtoConsensusParams()
if err != nil {
@ -85,6 +84,7 @@ func (k Keeper) Params(ctx context.Context, _ *types.QueryParamsRequest) (*types
var _ types.MsgServer = Keeper{}
// UpdateParams updates the consensus parameters.
func (k Keeper) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
if k.GetAuthority() != msg.Authority {
return nil, fmt.Errorf("invalid authority; expected %s, got %s", k.GetAuthority(), msg.Authority)
@ -116,17 +116,15 @@ func (k Keeper) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) (*
// paramCheck validates the consensus params
func (k Keeper) paramCheck(ctx context.Context, consensusParams cmtproto.ConsensusParams) (*cmttypes.ConsensusParams, error) {
paramsProto, err := k.ParamsStore.Get(ctx)
var params cmttypes.ConsensusParams
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
params = cmttypes.ConsensusParams{}
} else {
return nil, err
}
} else {
paramsProto, err := k.ParamsStore.Get(ctx)
if err == nil {
params = cmttypes.ConsensusParamsFromProto(paramsProto)
} else if errors.Is(err, collections.ErrNotFound) {
params = cmttypes.ConsensusParams{}
} else {
return nil, err
}
nextParams := params.Update(&consensusParams)

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

@ -152,8 +152,6 @@ func (s *KeeperTestSuite) TestGRPCQueryConsensusParams() {
}
for _, tc := range testCases {
tc := tc
s.Run(tc.msg, func() {
s.SetupTest(false) // reset
@ -189,8 +187,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
Validator: defaultConsensusParams.Validator,
Evidence: defaultConsensusParams.Evidence,
},
expErr: false,
expErrMsg: "",
},
{
name: "invalid params",
@ -258,8 +254,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
VoteExtensionsEnableHeight: &gogotypes.Int64Value{Value: 300},
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid Feature update - pbts",
@ -272,8 +266,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
PbtsEnableHeight: &gogotypes.Int64Value{Value: 150},
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid Feature update - vote extensions + pbts",
@ -287,8 +279,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
PbtsEnableHeight: &gogotypes.Int64Value{Value: 110},
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid noop Feature update - vote extensions + pbts (enabled feature)",
@ -303,8 +293,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
PbtsEnableHeight: &gogotypes.Int64Value{Value: 5},
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid (deprecated) ABCI update",
@ -317,8 +305,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
VoteExtensionsEnableHeight: 90,
},
},
expErr: false,
expErrMsg: "",
},
{
name: "invalid Feature + (deprecated) ABCI vote extensions update",
@ -462,8 +448,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
Precision: getDuration(3 * time.Second),
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid Synchrony update - delay",
@ -476,8 +460,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
MessageDelay: getDuration(10 * time.Second),
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid Synchrony update - precision + delay",
@ -491,8 +473,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
MessageDelay: getDuration(11 * time.Second),
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid Synchrony update - 0 precision",
@ -505,8 +485,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
Precision: getDuration(0),
},
},
expErr: false,
expErrMsg: "",
},
{
name: "valid Synchrony update - 0 delay",
@ -519,8 +497,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
MessageDelay: getDuration(0),
},
},
expErr: false,
expErrMsg: "",
},
{
name: "invalid Synchrony update - 0 precision with PBTS set",
@ -559,7 +535,6 @@ func (s *KeeperTestSuite) TestUpdateParams() {
}
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
s.SetupTest(tc.enabledFeatures)
_, err := s.consensusParamsKeeper.UpdateParams(s.ctx, tc.input)

1
x/consensus/types/codec.go
View File

@ -9,6 +9,7 @@ import (
"github.com/cosmos/cosmos-sdk/types/msgservice"
)
// RegisterInterfaces registers the interfaces types with the interface registry.
func RegisterInterfaces(registrar registry.InterfaceRegistrar) {
registrar.RegisterImplementations(
(*coretransaction.Msg)(nil),

3
x/consensus/types/msgs.go
View File

@ -8,6 +8,9 @@ import (
"github.com/cosmos/gogoproto/types"
)
// ToProtoConsensusParams converts MsgUpdateParams to cmtproto.ConsensusParams.
// It returns an error if any required parameters are missing or if there's a conflict
// between ABCI and Feature parameters.
func (msg MsgUpdateParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, error) {
if msg.Evidence == nil || msg.Block == nil || msg.Validator == nil {
return cmtproto.ConsensusParams{}, errors.New("all parameters must be present")