From f24f03906f663caab69a333b2db20f8e41797c17 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Fri, 20 Oct 2017 00:08:39 -0600 Subject: [PATCH] types: ConsensusParams: add feedback from @ebuchman and @melekes --- types/genesis_test.go | 28 ---------------- types/params.go | 12 +++---- types/params_test.go | 78 +++++++++++++++---------------------------- 3 files changed, 30 insertions(+), 88 deletions(-) diff --git a/types/genesis_test.go b/types/genesis_test.go index 0ffce4b53..0028f51ed 100644 --- a/types/genesis_test.go +++ b/types/genesis_test.go @@ -60,31 +60,3 @@ func TestGenesis(t *testing.T) { genDoc, err = GenesisDocFromJSON(genDocBytes) assert.Error(t, err, "expected error for genDoc json with block size of 0") } - -func newConsensusParams(blockSize, partSize int) ConsensusParams { - return ConsensusParams{ - BlockSizeParams: BlockSizeParams{MaxBytes: blockSize}, - BlockGossipParams: BlockGossipParams{BlockPartSizeBytes: partSize}, - } - -} - -func TestConsensusParams(t *testing.T) { - - testCases := []struct { - params ConsensusParams - valid bool - }{ - {newConsensusParams(1, 1), true}, - {newConsensusParams(1, 0), false}, - {newConsensusParams(0, 1), false}, - {newConsensusParams(0, 0), false}, - } - for _, testCase := range testCases { - if testCase.valid { - assert.NoError(t, testCase.params.Validate(), "expected no error for valid params") - } else { - assert.Error(t, testCase.params.Validate(), "expected error for non valid params") - } - } -} diff --git a/types/params.go b/types/params.go index dc811df25..322cba612 100644 --- a/types/params.go +++ b/types/params.go @@ -5,7 +5,7 @@ import ( ) const ( - _100MiB = 104857600 + maxBlockSizeBytes = 104857600 // 100MB ) // ConsensusParams contains consensus critical parameters @@ -18,7 +18,7 @@ type ConsensusParams struct { // BlockSizeParams contain limits on the block size. type BlockSizeParams struct { - MaxBytes int `json:"max_bytes"` // NOTE: must not be 0 + MaxBytes int `json:"max_bytes"` // NOTE: must not be 0 nor greater than 100MB MaxTxs int `json:"max_txs"` MaxGas int `json:"max_gas"` } @@ -69,10 +69,6 @@ func DefaultBlockGossipParams() BlockGossipParams { // Validate validates the ConsensusParams to ensure all values // are within their allowed limits, and returns an error if they are not. -// Expecting: -// * BlockSizeParams.MaxBytes > 0 -// * BlockGossipParams.BlockPartSizeBytes > 0 -// * BlockSizeParams.MaxBytes <= 100MiB func (params *ConsensusParams) Validate() error { // ensure some values are greater than 0 if params.BlockSizeParams.MaxBytes <= 0 { @@ -83,9 +79,9 @@ func (params *ConsensusParams) Validate() error { } // ensure blocks aren't too big - if params.BlockSizeParams.MaxBytes > _100MiB { + if params.BlockSizeParams.MaxBytes > maxBlockSizeBytes { return errors.Errorf("BlockSizeParams.MaxBytes is too big. %d > %d", - params.BlockSizeParams.MaxBytes, _100MiB) + params.BlockSizeParams.MaxBytes, maxBlockSizeBytes) } return nil } diff --git a/types/params_test.go b/types/params_test.go index 17a5cc0b0..507c85139 100644 --- a/types/params_test.go +++ b/types/params_test.go @@ -1,66 +1,40 @@ -package types_test +package types import ( "testing" "github.com/stretchr/testify/assert" - - "github.com/tendermint/tendermint/types" ) +func newConsensusParams(blockSize, partSize int) ConsensusParams { + return ConsensusParams{ + BlockSizeParams: BlockSizeParams{MaxBytes: blockSize}, + BlockGossipParams: BlockGossipParams{BlockPartSizeBytes: partSize}, + } +} + func TestConsensusParamsValidation(t *testing.T) { - tests := [...]struct { - params *types.ConsensusParams - wantErr string + testCases := []struct { + params ConsensusParams + valid bool }{ - {&types.ConsensusParams{}, "BlockSizeParams.MaxBytes must be greater than 0"}, - { - &types.ConsensusParams{BlockSizeParams: types.BlockSizeParams{MaxBytes: 10}}, - "BlockGossipParams.BlockPartSizeBytes must be greater than 0", - }, - { - &types.ConsensusParams{ - BlockSizeParams: types.BlockSizeParams{MaxBytes: 10}, - BlockGossipParams: types.BlockGossipParams{BlockPartSizeBytes: -1}, - }, "BlockGossipParams.BlockPartSizeBytes must be greater than 0", - }, - { - &types.ConsensusParams{ - BlockSizeParams: types.BlockSizeParams{MaxBytes: 10}, - BlockGossipParams: types.BlockGossipParams{BlockPartSizeBytes: 400}, - }, ""}, - { - &types.ConsensusParams{ - BlockSizeParams: types.BlockSizeParams{MaxBytes: 1024 * 1024 * 47}, - BlockGossipParams: types.BlockGossipParams{BlockPartSizeBytes: 400}, - }, "", - }, - { - &types.ConsensusParams{ - BlockSizeParams: types.BlockSizeParams{MaxBytes: 1024 * 1024 * 100}, - BlockGossipParams: types.BlockGossipParams{BlockPartSizeBytes: 400}, - }, "", - }, - { - &types.ConsensusParams{ - BlockSizeParams: types.BlockSizeParams{MaxBytes: 1024 * 1024 * 101}, - BlockGossipParams: types.BlockGossipParams{BlockPartSizeBytes: 400}, - }, "BlockSizeParams.MaxBytes is too big", - }, - { - &types.ConsensusParams{ - BlockSizeParams: types.BlockSizeParams{MaxBytes: 1024 * 1024 * 1024}, - BlockGossipParams: types.BlockGossipParams{BlockPartSizeBytes: 400}, - }, "BlockSizeParams.MaxBytes is too big", - }, + {newConsensusParams(1, 1), true}, + {newConsensusParams(1, 0), false}, + {newConsensusParams(0, 1), false}, + {newConsensusParams(0, 0), false}, + {newConsensusParams(0, 10), false}, + {newConsensusParams(10, -1), false}, + {newConsensusParams(47*1024*1024, 400), true}, + {newConsensusParams(10, 400), true}, + {newConsensusParams(100*1024*1024, 400), true}, + {newConsensusParams(101*1024*1024, 400), false}, + {newConsensusParams(1024*1024*1024, 400), false}, } - - for i, tt := range tests { - err := tt.params.Validate() - if tt.wantErr != "" { - assert.Contains(t, err.Error(), tt.wantErr, "#%d: params: %#v", i, tt.params) + for _, testCase := range testCases { + if testCase.valid { + assert.NoError(t, testCase.params.Validate(), "expected no error for valid params") } else { - assert.Nil(t, err, "#%d: want nil error; Params: %#v", i, tt.params) + assert.Error(t, testCase.params.Validate(), "expected error for non valid params") } } }