Skip to content

Commit fdda245

Browse files
Add ConsensusParams updates handling logic (#1367) (#1420)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> Duplicate of #1367 Reference to corresponding [cometBFT code](https://github.com/cometbft/cometbft/blob/6c1deb50b146bf8d27a65fbce8dbba89052dd607/internal/state/execution.go#L615). Removes validator updates code as well that is now unnecessary due to switch to single sequencer. <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [x] Linked issues closed with keywords <!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> ## Checklist <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Search bar implemented, enhancing the user interface for better navigation and search capabilities. - **Improvements** - Updated consensus parameter retrieval for improved consistency and reliability. - Refined state update process to better handle consensus parameter updates. - **Bug Fixes** - Adjusted range checking logic to ensure accurate query bounds validation. - **Tests** - Added new tests to ensure consensus parameters are validated and updated correctly. - **Documentation** - Updated comments to clarify the purpose of key functions and processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent d7359b0 commit fdda245

8 files changed

Lines changed: 237 additions & 49 deletions

File tree

node/full_client.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,12 @@ func (c *FullClient) ConsensusState(ctx context.Context) (*ctypes.ResultConsensu
376376
}
377377

378378
// ConsensusParams returns consensus params at given height.
379-
//
380-
// Currently, consensus params changes are not supported and this method returns params as defined in genesis.
381379
func (c *FullClient) ConsensusParams(ctx context.Context, height *int64) (*ctypes.ResultConsensusParams, error) {
382-
// TODO(tzdybal): implement consensus params handling: https://github.com/rollkit/rollkit/issues/291
383-
params := c.node.GetGenesis().ConsensusParams
380+
state, err := c.node.Store.GetState()
381+
if err != nil {
382+
return nil, err
383+
}
384+
params := state.ConsensusParams
384385
return &ctypes.ResultConsensusParams{
385386
BlockHeight: int64(c.normalizeHeight(height)),
386387
ConsensusParams: cmtypes.ConsensusParams{

state/executor.go

Lines changed: 29 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"time"
99

1010
abci "github.com/cometbft/cometbft/abci/types"
11-
cryptoenc "github.com/cometbft/cometbft/crypto/encoding"
1211
cmbytes "github.com/cometbft/cometbft/libs/bytes"
1312
cmproto "github.com/cometbft/cometbft/proto/tendermint/types"
1413
"github.com/cometbft/cometbft/proxy"
@@ -167,7 +166,7 @@ func (e *BlockExecutor) CreateBlock(height uint64, lastCommit *types.Commit, las
167166
return block, nil
168167
}
169168

170-
// ProcessProposal processes the proposal.
169+
// ProcessProposal calls the corresponding ABCI method on the app.
171170
func (e *BlockExecutor) ProcessProposal(
172171
block *types.Block,
173172
state types.State,
@@ -215,29 +214,15 @@ func (e *BlockExecutor) ApplyBlock(ctx context.Context, state types.State, block
215214
return types.State{}, nil, err
216215
}
217216

218-
abciValUpdates := resp.ValidatorUpdates
219-
220-
err = validateValidatorUpdates(abciValUpdates, state.ConsensusParams.Validator)
217+
state, err = e.updateState(state, block, resp)
221218
if err != nil {
222-
return state, nil, fmt.Errorf("error in validator updates: %v", err)
219+
return types.State{}, nil, err
223220
}
224221

225-
validatorUpdates, err := cmtypes.PB2TM.ValidatorUpdates(abciValUpdates)
226-
if err != nil {
227-
return state, nil, err
228-
}
229-
if len(validatorUpdates) > 0 {
230-
e.logger.Debug("updates to validators", "updates", cmtypes.ValidatorListString(validatorUpdates))
231-
}
232222
if state.ConsensusParams.Block.MaxBytes == 0 {
233223
e.logger.Error("maxBytes=0", "state.ConsensusParams.Block", state.ConsensusParams.Block, "block", block)
234224
}
235225

236-
state, err = e.updateState(state, block, resp, validatorUpdates)
237-
if err != nil {
238-
return types.State{}, nil, err
239-
}
240-
241226
return state, resp, nil
242227
}
243228

@@ -255,12 +240,36 @@ func (e *BlockExecutor) Commit(ctx context.Context, state types.State, block *ty
255240
return appHash, retainHeight, nil
256241
}
257242

258-
func (e *BlockExecutor) updateState(state types.State, block *types.Block, finalizeBlockResponse *abci.ResponseFinalizeBlock, validatorUpdates []*cmtypes.Validator) (types.State, error) {
243+
// updateConsensusParams updates the consensus parameters based on the provided updates.
244+
func (e *BlockExecutor) updateConsensusParams(height uint64, params cmtypes.ConsensusParams, consensusParamUpdates *cmproto.ConsensusParams) (cmproto.ConsensusParams, uint64, error) {
245+
nextParams := params.Update(consensusParamUpdates)
246+
if err := types.ConsensusParamsValidateBasic(nextParams); err != nil {
247+
return cmproto.ConsensusParams{}, 0, fmt.Errorf("validating new consensus params: %w", err)
248+
}
249+
if err := nextParams.ValidateUpdate(consensusParamUpdates, int64(height)); err != nil {
250+
return cmproto.ConsensusParams{}, 0, fmt.Errorf("updating consensus params: %w", err)
251+
}
252+
return nextParams.ToProto(), nextParams.Version.App, nil
253+
}
254+
255+
func (e *BlockExecutor) updateState(state types.State, block *types.Block, finalizeBlockResponse *abci.ResponseFinalizeBlock) (types.State, error) {
256+
height := block.Height()
257+
if finalizeBlockResponse.ConsensusParamUpdates != nil {
258+
nextParamsProto, appVersion, err := e.updateConsensusParams(height, types.ConsensusParamsFromProto(state.ConsensusParams), finalizeBlockResponse.ConsensusParamUpdates)
259+
if err != nil {
260+
return state, err
261+
}
262+
// Change results from this height but only applies to the next height.
263+
state.LastHeightConsensusParamsChanged = height + 1
264+
state.Version.Consensus.App = appVersion
265+
state.ConsensusParams = nextParamsProto
266+
}
267+
259268
s := types.State{
260269
Version: state.Version,
261270
ChainID: state.ChainID,
262271
InitialHeight: state.InitialHeight,
263-
LastBlockHeight: block.Height(),
272+
LastBlockHeight: height,
264273
LastBlockTime: block.Time(),
265274
LastBlockID: cmtypes.BlockID{
266275
Hash: cmbytes.HexBytes(block.Hash()),
@@ -456,27 +465,3 @@ func fromRollkitTxs(rollkitTxs types.Txs) cmtypes.Txs {
456465
}
457466
return txs
458467
}
459-
460-
func validateValidatorUpdates(abciUpdates []abci.ValidatorUpdate, params *cmproto.ValidatorParams) error {
461-
for _, valUpdate := range abciUpdates {
462-
if valUpdate.GetPower() < 0 {
463-
return fmt.Errorf("voting power can't be negative %v", valUpdate)
464-
} else if valUpdate.GetPower() == 0 {
465-
// continue, since this is deleting the validator, and thus there is no
466-
// pubkey to check
467-
continue
468-
}
469-
470-
// Check if validator's pubkey matches an ABCI type in the consensus params
471-
pk, err := cryptoenc.PubKeyFromProto(valUpdate.PubKey)
472-
if err != nil {
473-
return err
474-
}
475-
476-
if !cmtypes.IsValidPubkeyType(cmtypes.ValidatorParams{PubKeyTypes: params.PubKeyTypes}, pk.Type()) {
477-
return fmt.Errorf("validator %v is using pubkey %s, which is unsupported for consensus",
478-
valUpdate, pk.Type())
479-
}
480-
}
481-
return nil
482-
}

state/executor_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,3 +247,67 @@ func doTestApplyBlock(t *testing.T) {
247247
func TestApplyBlockWithFraudProofsDisabled(t *testing.T) {
248248
doTestApplyBlock(t)
249249
}
250+
251+
func TestUpdateStateConsensusParams(t *testing.T) {
252+
logger := log.TestingLogger()
253+
app := &mocks.Application{}
254+
client, err := proxy.NewLocalClientCreator(app).NewABCIClient()
255+
require.NoError(t, err)
256+
require.NotNil(t, client)
257+
258+
chainID := "test"
259+
260+
mpool := mempool.NewCListMempool(cfg.DefaultMempoolConfig(), proxy.NewAppConnMempool(client, proxy.NopMetrics()), 0)
261+
eventBus := cmtypes.NewEventBus()
262+
require.NoError(t, eventBus.Start())
263+
executor := NewBlockExecutor([]byte("test address"), chainID, mpool, proxy.NewAppConnConsensus(client, proxy.NopMetrics()), eventBus, logger)
264+
265+
state := types.State{
266+
ConsensusParams: cmproto.ConsensusParams{
267+
Block: &cmproto.BlockParams{
268+
MaxBytes: 100,
269+
MaxGas: 100000,
270+
},
271+
Validator: &cmproto.ValidatorParams{
272+
PubKeyTypes: []string{cmtypes.ABCIPubKeyTypeEd25519},
273+
},
274+
Version: &cmproto.VersionParams{
275+
App: 1,
276+
},
277+
Abci: &cmproto.ABCIParams{},
278+
},
279+
}
280+
281+
block := types.GetRandomBlock(1234, 2)
282+
283+
txResults := make([]*abci.ExecTxResult, len(block.Data.Txs))
284+
for idx := range block.Data.Txs {
285+
txResults[idx] = &abci.ExecTxResult{
286+
Code: abci.CodeTypeOK,
287+
}
288+
}
289+
290+
resp := &abci.ResponseFinalizeBlock{
291+
ConsensusParamUpdates: &cmproto.ConsensusParams{
292+
Block: &cmproto.BlockParams{
293+
MaxBytes: 200,
294+
MaxGas: 200000,
295+
},
296+
Validator: &cmproto.ValidatorParams{
297+
PubKeyTypes: []string{cmtypes.ABCIPubKeyTypeEd25519},
298+
},
299+
Version: &cmproto.VersionParams{
300+
App: 2,
301+
},
302+
},
303+
TxResults: txResults,
304+
}
305+
306+
updatedState, err := executor.updateState(state, block, resp)
307+
require.NoError(t, err)
308+
309+
assert.Equal(t, uint64(1235), updatedState.LastHeightConsensusParamsChanged)
310+
assert.Equal(t, int64(200), updatedState.ConsensusParams.Block.MaxBytes)
311+
assert.Equal(t, int64(200000), updatedState.ConsensusParams.Block.MaxGas)
312+
assert.Equal(t, uint64(2), updatedState.ConsensusParams.Version.App)
313+
}

state/indexer_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func compareInt(op1 *big.Int, op2 interface{}) (int, bool, error) {
4343
}
4444
}
4545

46-
// CheckBounds checks if the value of the event is within the bounds of the query.
46+
// CheckBounds checks if the given value falls within the specified query range.
4747
func CheckBounds(ranges indexer.QueryRange, v interface{}) (bool, error) {
4848
// These functions fetch the lower and upper bounds of the query
4949
// It is expected that for x > 5, the value of lowerBound is 6.

types/params.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package types
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
7+
cmtypes "github.com/cometbft/cometbft/types"
8+
)
9+
10+
// ConsensusParamsValidateBasic validates the ConsensusParams to ensure all values are within their
11+
// allowed limits, and returns an error if they are not.
12+
func ConsensusParamsValidateBasic(params cmtypes.ConsensusParams) error {
13+
if params.Block.MaxBytes == 0 {
14+
return fmt.Errorf("block.MaxBytes cannot be 0")
15+
}
16+
if params.Block.MaxBytes < -1 {
17+
return fmt.Errorf("block.MaxBytes must be -1 or greater than 0. Got %d",
18+
19+
params.Block.MaxBytes)
20+
}
21+
if params.Block.MaxBytes > cmtypes.MaxBlockSizeBytes {
22+
return fmt.Errorf("block.MaxBytes is too big. %d > %d",
23+
params.Block.MaxBytes, cmtypes.MaxBlockSizeBytes)
24+
}
25+
26+
if params.Block.MaxGas < -1 {
27+
return fmt.Errorf("block.MaxGas must be greater or equal to -1. Got %d",
28+
params.Block.MaxGas)
29+
}
30+
31+
if params.ABCI.VoteExtensionsEnableHeight < 0 {
32+
return fmt.Errorf("ABCI.VoteExtensionsEnableHeight cannot be negative. Got: %d", params.ABCI.VoteExtensionsEnableHeight)
33+
}
34+
35+
if len(params.Validator.PubKeyTypes) == 0 {
36+
return errors.New("len(Validator.PubKeyTypes) must be greater than 0")
37+
}
38+
39+
// Check if keyType is a known ABCIPubKeyType
40+
for i := 0; i < len(params.Validator.PubKeyTypes); i++ {
41+
keyType := params.Validator.PubKeyTypes[i]
42+
if _, ok := cmtypes.ABCIPubKeyTypesToNames[keyType]; !ok {
43+
return fmt.Errorf("params.Validator.PubKeyTypes[%d], %s, is an unknown pubkey type",
44+
i, keyType)
45+
}
46+
}
47+
48+
return nil
49+
}

types/params_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package types
2+
3+
import (
4+
"testing"
5+
6+
cmtypes "github.com/cometbft/cometbft/types"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestConsensusParamsValidateBasic(t *testing.T) {
11+
testCases := []struct {
12+
name string
13+
prepare func() cmtypes.ConsensusParams // Function to prepare the test case
14+
err error
15+
}{
16+
// 1. Test valid params
17+
{
18+
name: "valid params",
19+
prepare: func() cmtypes.ConsensusParams {
20+
return cmtypes.ConsensusParams{
21+
Block: cmtypes.BlockParams{
22+
MaxBytes: 12345,
23+
MaxGas: 6543234,
24+
},
25+
Validator: cmtypes.ValidatorParams{
26+
PubKeyTypes: []string{cmtypes.ABCIPubKeyTypeEd25519},
27+
},
28+
Version: cmtypes.VersionParams{
29+
App: 42,
30+
},
31+
}
32+
},
33+
err: nil,
34+
},
35+
}
36+
37+
for _, tc := range testCases {
38+
t.Run(tc.name, func(t *testing.T) {
39+
err := ConsensusParamsValidateBasic(tc.prepare())
40+
assert.ErrorIs(t, err, tc.err)
41+
})
42+
}
43+
}

types/serialization.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package types
22

33
import (
4+
cmproto "github.com/cometbft/cometbft/proto/tendermint/types"
45
"github.com/cometbft/cometbft/types"
56

67
pb "github.com/rollkit/rollkit/types/pb/rollkit"
@@ -335,3 +336,23 @@ func byteSlicesToSignatures(bytes [][]byte) []Signature {
335336
}
336337
return sigs
337338
}
339+
340+
// ConsensusParamsFromProto converts protobuf consensus parameters to consensus parameters
341+
func ConsensusParamsFromProto(pbParams cmproto.ConsensusParams) types.ConsensusParams {
342+
c := types.ConsensusParams{
343+
Block: types.BlockParams{
344+
MaxBytes: pbParams.Block.MaxBytes,
345+
MaxGas: pbParams.Block.MaxGas,
346+
},
347+
Validator: types.ValidatorParams{
348+
PubKeyTypes: pbParams.Validator.PubKeyTypes,
349+
},
350+
Version: types.VersionParams{
351+
App: pbParams.Version.App,
352+
},
353+
}
354+
if pbParams.Abci != nil {
355+
c.ABCI.VoteExtensionsEnableHeight = pbParams.Abci.GetVoteExtensionsEnableHeight()
356+
}
357+
return c
358+
}

types/serialization_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,28 @@ func TestSignaturesRoundtrip(t *testing.T) {
236236
assert.Equal(t, newSigs[i], sigs[i])
237237
}
238238
}
239+
240+
func TestConsensusParamsFromProto(t *testing.T) {
241+
// Prepare test case
242+
pbParams := cmproto.ConsensusParams{
243+
Block: &cmproto.BlockParams{
244+
MaxBytes: 12345,
245+
MaxGas: 67890,
246+
},
247+
Validator: &cmproto.ValidatorParams{
248+
PubKeyTypes: []string{cmtypes.ABCIPubKeyTypeEd25519},
249+
},
250+
Version: &cmproto.VersionParams{
251+
App: 42,
252+
},
253+
}
254+
255+
// Call the function to be tested
256+
params := ConsensusParamsFromProto(pbParams)
257+
258+
// Check the results
259+
assert.Equal(t, int64(12345), params.Block.MaxBytes)
260+
assert.Equal(t, int64(67890), params.Block.MaxGas)
261+
assert.Equal(t, uint64(42), params.Version.App)
262+
assert.Equal(t, []string{cmtypes.ABCIPubKeyTypeEd25519}, params.Validator.PubKeyTypes)
263+
}

0 commit comments

Comments
 (0)