From 3a03fe5a15e012d1fd2cf61cd8c8e44cdf109a62 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sat, 16 Sep 2017 00:16:49 -0400 Subject: [PATCH] updated to match adr 005 --- blockchain/reactor.go | 2 +- consensus/state.go | 2 +- docs/architecture/adr-005-consensus-params.md | 24 +++++--- types/genesis_test.go | 19 ++++-- types/params.go | 60 ++++++++++++++++--- 5 files changed, 84 insertions(+), 23 deletions(-) diff --git a/blockchain/reactor.go b/blockchain/reactor.go index fccfcdf8f..fb68aadda 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -165,7 +165,7 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) // maxMsgSize returns the maximum allowable size of a // message on the blockchain reactor. func (bcR *BlockchainReactor) maxMsgSize() int { - return bcR.state.Params().MaxBlockSizeBytes + 2 + return bcR.state.Params().BlockSizeParams.MaxBytes + 2 } // Handle messages from the poolReactor telling the reactor what to do. diff --git a/consensus/state.go b/consensus/state.go index 1a5b26e62..ac1656209 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -1419,7 +1419,7 @@ func (cs *ConsensusState) addProposalBlockPart(height int, part *types.Part, ver var n int var err error cs.ProposalBlock = wire.ReadBinary(&types.Block{}, cs.ProposalBlockParts.GetReader(), - cs.state.Params().MaxBlockSizeBytes, &n, &err).(*types.Block) + cs.state.Params().BlockSizeParams.MaxBytes, &n, &err).(*types.Block) // NOTE: it's possible to receive complete proposal blocks for future rounds without having the proposal cs.Logger.Info("Received complete proposal block", "height", cs.ProposalBlock.Height, "hash", cs.ProposalBlock.Hash()) if cs.Step == RoundStepPropose && cs.isProposalComplete() { diff --git a/docs/architecture/adr-005-consensus-params.md b/docs/architecture/adr-005-consensus-params.md index 6f43fe7d9..678de42eb 100644 --- a/docs/architecture/adr-005-consensus-params.md +++ b/docs/architecture/adr-005-consensus-params.md @@ -13,10 +13,12 @@ such as a period over which evidence is valid, or the frequency of checkpoints. ### ConsensusParams +No consensus critical parameters should ever be found in the `config.toml`. + A new `ConsensusParams` is optionally included in the `genesis.json` file, and loaded into the `State`. Any items not included are set to their default value. A value of 0 is undefined (see ABCI, below). A value of -1 is used to indicate the parameter does not apply. -No consensus critical parameters should ever be found in the `config.toml`. +The parameters are used to determine the validity of a block (and tx) via the union of all relevant parameters. ``` type ConsensusParams struct { @@ -26,14 +28,14 @@ type ConsensusParams struct { } type BlockSizeParams struct { - BlockSizeBytes int - BlockSizeTxs int - BlockSizeGas int + MaxBytes int + MaxTxs int + MaxGas int } type TxSizeParams struct { - TxSizeBytes int - TxSizeGas int + MaxBytes int + MaxGas int } type BlockGossipParams struct { @@ -43,12 +45,16 @@ type BlockGossipParams struct { The `ConsensusParams` can evolve over time by adding new structs that cover different aspects of the consensus rules. +The `BlockPartSizeBytes` and the `BlockSizeParams.MaxBytes` are enforced to be greater than 0. +The former because we need a part size, the latter so that we always have at least some sanity check over the size of blocks. + ### ABCI #### InitChain -InitChain currently takes the initial validator set. It should be extended to also take the ConsensusParams. -In fact, it might as well just consume the whole Genesis. +InitChain currently takes the initial validator set. It should be extended to also take parts of the ConsensusParams. +There is some case to be made for it to take the entire Genesis, except there may be things in the genesis, +like the BlockPartSize, that the app shouldn't really know about. #### EndBlock @@ -75,3 +81,5 @@ Proposed. - Different rules at different heights in the blockchain complicates fast sync ### Neutral + +- The TxSizeParams, which checks validity, may be in conflict with the config's `max_block_size_tx`, which determines proposal sizes diff --git a/types/genesis_test.go b/types/genesis_test.go index 9743a9610..7d7731f13 100644 --- a/types/genesis_test.go +++ b/types/genesis_test.go @@ -52,22 +52,31 @@ func TestGenesis(t *testing.T) { assert.Nil(t, err, "expected no error for valid genDoc json") // test with invalid consensus params - genDoc.ConsensusParams.MaxBlockSizeBytes = 0 + genDoc.ConsensusParams.BlockSizeParams.MaxBytes = 0 genDocBytes, err = json.Marshal(genDoc) assert.Nil(t, err, "error marshalling genDoc") genDoc, err = GenesisDocFromJSON(genDocBytes) assert.NotNil(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 }{ - {&ConsensusParams{1, 1}, true}, - {&ConsensusParams{1, 0}, false}, - {&ConsensusParams{0, 1}, false}, - {&ConsensusParams{0, 0}, false}, + {newConsensusParams(1, 1), true}, + {newConsensusParams(1, 0), false}, + {newConsensusParams(0, 1), false}, + {newConsensusParams(0, 0), false}, } for _, testCase := range testCases { if testCase.valid { diff --git a/types/params.go b/types/params.go index cff9ae3d3..71ffe60c9 100644 --- a/types/params.go +++ b/types/params.go @@ -7,26 +7,70 @@ import ( // ConsensusParams contains consensus critical parameters // that determine the validity of blocks. type ConsensusParams struct { - MaxBlockSizeBytes int `json:"max_block_size_bytes"` - BlockPartSizeBytes int `json:"block_part_size_bytes"` + *BlockSizeParams `json:"block_size_params"` + *TxSizeParams `json:"tx_size_params"` + *BlockGossipParams `json:"block_gossip_params"` +} + +// BlockSizeParams contain limits on the block size. +type BlockSizeParams struct { + MaxBytes int `json:"max_bytes"` // NOTE: must not be 0 + MaxTxs int `json:"max_txs"` + MaxGas int `json:"max_gas"` +} + +// TxSizeParams contain limits on the tx size. +type TxSizeParams struct { + MaxBytes int `json:"max_bytes"` + MaxGas int `json:"max_gas"` +} + +// BlockGossipParams determine consensus critical elements of how blocks are gossiped +type BlockGossipParams struct { + BlockPartSizeBytes int `json:"block_part_size_bytes"` // NOTE: must not be 0 } // DefaultConsensusParams returns a default ConsensusParams. func DefaultConsensusParams() *ConsensusParams { return &ConsensusParams{ - MaxBlockSizeBytes: 22020096, // 21MB - BlockPartSizeBytes: 65536, // 64kB, + DefaultBlockSizeParams(), + DefaultTxSizeParams(), + DefaultBlockGossipParams(), + } +} + +// DefaultBlockSizeParams returns a default BlockSizeParams. +func DefaultBlockSizeParams() *BlockSizeParams { + return &BlockSizeParams{ + MaxBytes: 22020096, // 21MB + MaxTxs: 100000, + MaxGas: -1, + } +} + +// DefaultTxSizeParams returns a default TxSizeParams. +func DefaultTxSizeParams() *TxSizeParams { + return &TxSizeParams{ + MaxBytes: 10240, // 10kB + MaxGas: -1, + } +} + +// DefaultBlockGossipParams returns a default BlockGossipParams. +func DefaultBlockGossipParams() *BlockGossipParams { + return &BlockGossipParams{ + BlockPartSizeBytes: 65536, // 64kB, } } // Validate validates the ConsensusParams to ensure all values // are within their allowed limits, and returns an error if they are not. func (params *ConsensusParams) Validate() error { - if params.MaxBlockSizeBytes <= 0 { - return errors.Errorf("MaxBlockSizeBytes must be greater than 0. Got %d", params.MaxBlockSizeBytes) + if params.BlockSizeParams.MaxBytes <= 0 { + return errors.Errorf("BlockSizeParams.MaxBytes must be greater than 0. Got %d", params.BlockSizeParams.MaxBytes) } - if params.BlockPartSizeBytes <= 0 { - return errors.Errorf("BlockPartSizeBytes must be greater than 0. Got %d", params.BlockPartSizeBytes) + if params.BlockGossipParams.BlockPartSizeBytes <= 0 { + return errors.Errorf("BlockGossipParams.BlockPartSizeBytes must be greater than 0. Got %d", params.BlockGossipParams.BlockPartSizeBytes) } return nil }