Skip to content

Commit

Permalink
fix(rpc): panic on block_results when consensus params change (#923)
Browse files Browse the repository at this point in the history
* fix(rpc): panic on block_results when consensus params change

* test(rpc): test non-nil consensus params

* chore: add comment to trigger tests
  • Loading branch information
lklimek committed Sep 19, 2024
1 parent dce91b6 commit ca61e56
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 23 deletions.
40 changes: 26 additions & 14 deletions internal/rpc/core/blocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
},
},
},
}

Expand All @@ -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)
}
}
}
34 changes: 25 additions & 9 deletions types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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
}
11 changes: 11 additions & 0 deletions types/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit ca61e56

Please sign in to comment.