From ca61e566ffec8ae419115c672e0258af9b794812 Mon Sep 17 00:00:00 2001 From: lklimek <842586+lklimek@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:56:16 +0200 Subject: [PATCH] fix(rpc): panic on block_results when consensus params change (#923) * fix(rpc): panic on block_results when consensus params change * test(rpc): test non-nil consensus params * chore: add comment to trigger tests --- internal/rpc/core/blocks_test.go | 40 +++++++++++++++++++++----------- types/params.go | 34 ++++++++++++++++++++------- types/params_test.go | 11 +++++++++ 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/internal/rpc/core/blocks_test.go b/internal/rpc/core/blocks_test.go index 2b761c3bf..7caef8921 100644 --- a/internal/rpc/core/blocks_test.go +++ b/internal/rpc/core/blocks_test.go @@ -14,7 +14,9 @@ import ( sm "github.com/dashpay/tenderdash/internal/state" "github.com/dashpay/tenderdash/internal/state/mocks" "github.com/dashpay/tenderdash/proto/tendermint/state" + "github.com/dashpay/tenderdash/proto/tendermint/types" "github.com/dashpay/tenderdash/rpc/coretypes" + tmtypes "github.com/dashpay/tenderdash/types" ) func TestBlockchainInfo(t *testing.T) { @@ -77,6 +79,11 @@ func TestBlockResults(t *testing.T) { {Code: 0, Data: []byte{0x02}, Log: "ok", GasUsed: 5}, {Code: 1, Log: "not ok", GasUsed: 0}, }, + ConsensusParamUpdates: &types.ConsensusParams{ + Version: &types.VersionParams{ + AppVersion: 1, + }, + }, }, } @@ -98,25 +105,30 @@ func TestBlockResults(t *testing.T) { {0, true, nil}, {101, true, nil}, {100, false, &coretypes.ResultBlockResults{ - Height: 100, - TxsResults: results.ProcessProposal.TxResults, - TotalGasUsed: 15, - FinalizeBlockEvents: results.ProcessProposal.Events, - ValidatorSetUpdate: results.ProcessProposal.ValidatorSetUpdate, - ConsensusParamUpdates: consensusParamsPtrFromProtoPtr(results.ProcessProposal.ConsensusParamUpdates), + Height: 100, + TxsResults: results.ProcessProposal.TxResults, + TotalGasUsed: 15, + FinalizeBlockEvents: results.ProcessProposal.Events, + ValidatorSetUpdate: results.ProcessProposal.ValidatorSetUpdate, + ConsensusParamUpdates: &tmtypes.ConsensusParams{ + Version: tmtypes.VersionParams{AppVersion: 1}, + }, }}, } ctx := context.Background() for _, tc := range testCases { - res, err := env.BlockResults(ctx, &coretypes.RequestBlockInfo{ - Height: (*coretypes.Int64)(&tc.height), + t.Run("", func(t *testing.T) { + res, err := env.BlockResults(ctx, &coretypes.RequestBlockInfo{ + Height: (*coretypes.Int64)(&tc.height), + }) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + t.Logf("Consensus params: %+v", res.ConsensusParamUpdates) + assert.Equal(t, tc.wantRes, res) + } }) - if tc.wantErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, tc.wantRes, res) - } } } diff --git a/types/params.go b/types/params.go index 8c8d11f99..fa7243825 100644 --- a/types/params.go +++ b/types/params.go @@ -462,25 +462,38 @@ func (params *ConsensusParams) ToProto() tmproto.ConsensusParams { } } +// ConsensusParamsFromProto returns a ConsensusParams from a protobuf representation. func ConsensusParamsFromProto(pbParams tmproto.ConsensusParams) ConsensusParams { - c := ConsensusParams{ - Block: BlockParams{ + + c := ConsensusParams{} + if pbParams.Block != nil { + c.Block = BlockParams{ MaxBytes: pbParams.Block.MaxBytes, MaxGas: pbParams.Block.MaxGas, - }, - Evidence: EvidenceParams{ + } + } + + if pbParams.Evidence != nil { + c.Evidence = EvidenceParams{ MaxAgeNumBlocks: pbParams.Evidence.MaxAgeNumBlocks, MaxAgeDuration: pbParams.Evidence.MaxAgeDuration, MaxBytes: pbParams.Evidence.MaxBytes, - }, - Validator: ValidatorParams{ + } + } + + if pbParams.Validator != nil { + c.Validator = ValidatorParams{ PubKeyTypes: pbParams.Validator.PubKeyTypes, - }, - Version: VersionParams{ + } + } + + if pbParams.Version != nil { + c.Version = VersionParams{ AppVersion: pbParams.Version.AppVersion, ConsensusVersion: int32(pbParams.Version.ConsensusVersion), - }, + } } + if pbParams.Synchrony != nil { if pbParams.Synchrony.MessageDelay != nil { c.Synchrony.MessageDelay = *pbParams.Synchrony.GetMessageDelay() @@ -489,6 +502,7 @@ func ConsensusParamsFromProto(pbParams tmproto.ConsensusParams) ConsensusParams c.Synchrony.Precision = *pbParams.Synchrony.GetPrecision() } } + if pbParams.Timeout != nil { if pbParams.Timeout.Propose != nil { c.Timeout.Propose = *pbParams.Timeout.GetPropose() @@ -503,8 +517,10 @@ func ConsensusParamsFromProto(pbParams tmproto.ConsensusParams) ConsensusParams c.Timeout.VoteDelta = *pbParams.Timeout.GetVoteDelta() } } + if pbParams.Abci != nil { c.ABCI.RecheckTx = pbParams.Abci.GetRecheckTx() } + return c } diff --git a/types/params_test.go b/types/params_test.go index 502b248b6..ccd18e8e7 100644 --- a/types/params_test.go +++ b/types/params_test.go @@ -428,6 +428,17 @@ func TestProto(t *testing.T) { } } +func TestFromProto(t *testing.T) { + params := []tmproto.ConsensusParams{ + {}, + } + + for i := range params { + pbParams := params[i] + assert.NotPanics(t, func() { ConsensusParamsFromProto(pbParams) }) + } +} + func durationPtr(t time.Duration) *time.Duration { return &t }