From 166fd82b70b76f0216cf3dacda9714e9eb19ba1c Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 4 Sep 2018 11:46:34 +0400 Subject: [PATCH] max-bytes PR follow-up (#2318) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ReapMaxTxs: return all txs if max is negative this mirrors ReapMaxBytes behavior See https://github.com/tendermint/tendermint/pull/2184#discussion_r214439950 * increase MaxAminoOverheadForBlock tested with: ``` func TestMaxAminoOverheadForBlock(t *testing.T) { maxChainID := "" for i := 0; i < MaxChainIDLen; i++ { maxChainID += "𠜎" } h := Header{ ChainID: maxChainID, Height: 10, Time: time.Now().UTC(), NumTxs: 100, TotalTxs: 200, LastBlockID: makeBlockID(make([]byte, 20), 300, make([]byte, 20)), LastCommitHash: tmhash.Sum([]byte("last_commit_hash")), DataHash: tmhash.Sum([]byte("data_hash")), ValidatorsHash: tmhash.Sum([]byte("validators_hash")), NextValidatorsHash: tmhash.Sum([]byte("next_validators_hash")), ConsensusHash: tmhash.Sum([]byte("consensus_hash")), AppHash: tmhash.Sum([]byte("app_hash")), LastResultsHash: tmhash.Sum([]byte("last_results_hash")), EvidenceHash: tmhash.Sum([]byte("evidence_hash")), ProposerAddress: tmhash.Sum([]byte("proposer_address")), } b := Block{ Header: h, Data: Data{Txs: makeTxs(10000, 100)}, Evidence: EvidenceData{}, LastCommit: &Commit{}, } bz, err := cdc.MarshalBinary(b) require.NoError(t, err) assert.Equal(t, MaxHeaderBytes+MaxAminoOverheadForBlock-2, len(bz)-1000000-20000-1) } ``` * fix MaxYYY constants calculation by using math.MaxInt64 See https://github.com/tendermint/tendermint/pull/2184#discussion_r214444244 * pass mempool filter as an option See https://github.com/tendermint/tendermint/pull/2184#discussion_r214445869 * fixes after Dev's comments --- mempool/mempool.go | 13 ++++++------- node/node.go | 6 +++--- types/block.go | 11 ++++++----- types/block_test.go | 9 +++++---- types/evidence.go | 2 +- types/evidence_test.go | 9 +++++---- types/vote.go | 2 +- types/vote_test.go | 18 +++++++++++++++++- 8 files changed, 44 insertions(+), 26 deletions(-) diff --git a/mempool/mempool.go b/mempool/mempool.go index 6ee6c42bc..381653e64 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -142,12 +142,10 @@ func (mem *Mempool) SetLogger(l log.Logger) { mem.logger = l } -// SetFilter sets a filter for mempool to only accept txs for which f(tx) +// WithFilter sets a filter for mempool to only accept txs for which f(tx) // returns true. -func (mem *Mempool) SetFilter(f func(types.Tx) bool) { - mem.proxyMtx.Lock() - mem.filter = f - mem.proxyMtx.Unlock() +func WithFilter(f func(types.Tx) bool) MempoolOption { + return func(mem *Mempool) { mem.filter = f } } // WithMetrics sets the metrics. @@ -415,13 +413,14 @@ func (mem *Mempool) ReapMaxBytes(max int) types.Txs { } // ReapMaxTxs reaps up to max transactions from the mempool. -// If max is negative, function panics. +// If max is negative, there is no cap on the size of all returned +// transactions (~ all available transactions). func (mem *Mempool) ReapMaxTxs(max int) types.Txs { mem.proxyMtx.Lock() defer mem.proxyMtx.Unlock() if max < 0 { - panic("Called ReapMaxTxs with negative max") + max = mem.txs.Len() } for atomic.LoadInt32(&mem.rechecking) > 0 { diff --git a/node/node.go b/node/node.go index 7e6603d9d..330095f8a 100644 --- a/node/node.go +++ b/node/node.go @@ -240,16 +240,16 @@ func NewNode(config *cfg.Config, csMetrics, p2pMetrics, memplMetrics := metricsProvider() // Make MempoolReactor - mempoolLogger := logger.With("module", "mempool") + maxBytes := state.ConsensusParams.TxSize.MaxBytes mempool := mempl.NewMempool( config.Mempool, proxyApp.Mempool(), state.LastBlockHeight, mempl.WithMetrics(memplMetrics), + mempl.WithFilter(func(tx types.Tx) bool { return len(tx) <= maxBytes }), ) + mempoolLogger := logger.With("module", "mempool") mempool.SetLogger(mempoolLogger) - maxBytes := state.ConsensusParams.TxSize.MaxBytes - mempool.SetFilter(func(tx types.Tx) bool { return len(tx) <= maxBytes }) mempool.InitWAL() // no need to have the mempool wal during tests mempoolReactor := mempl.NewMempoolReactor(config.Mempool, mempool) mempoolReactor.SetLogger(mempoolLogger) diff --git a/types/block.go b/types/block.go index 7d29c5789..f705c7db3 100644 --- a/types/block.go +++ b/types/block.go @@ -15,15 +15,16 @@ import ( const ( // MaxHeaderBytes is a maximum header size (including amino overhead). - MaxHeaderBytes = 478 + MaxHeaderBytes = 511 // MaxAminoOverheadForBlock - maximum amino overhead to encode a block (up to - // MaxBlockSizeBytes in size) not including it's parts (only varint len + - // fields without data). + // MaxBlockSizeBytes in size) not including it's parts except Data. // // Uvarint length of MaxBlockSizeBytes: 4 bytes - // 4 fields: 4 bytes - MaxAminoOverheadForBlock = 8 + // 2 fields (2 embedded): 2 bytes + // Uvarint length of Data.Txs: 4 bytes + // Data.Txs field: 1 byte + MaxAminoOverheadForBlock = 11 ) // Block defines the atomic unit of a Tendermint blockchain. diff --git a/types/block_test.go b/types/block_test.go index 4c74439c1..c2a73bf88 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -1,6 +1,7 @@ package types import ( + "math" "testing" "time" @@ -246,11 +247,11 @@ func TestMaxHeaderBytes(t *testing.T) { h := Header{ ChainID: maxChainID, - Height: 10, + Height: math.MaxInt64, Time: time.Now().UTC(), - NumTxs: 100, - TotalTxs: 200, - LastBlockID: makeBlockID(make([]byte, 20), 300, make([]byte, 20)), + NumTxs: math.MaxInt64, + TotalTxs: math.MaxInt64, + LastBlockID: makeBlockID(make([]byte, tmhash.Size), math.MaxInt64, make([]byte, tmhash.Size)), LastCommitHash: tmhash.Sum([]byte("last_commit_hash")), DataHash: tmhash.Sum([]byte("data_hash")), ValidatorsHash: tmhash.Sum([]byte("validators_hash")), diff --git a/types/evidence.go b/types/evidence.go index 096e3503f..8377fcd7f 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -12,7 +12,7 @@ import ( const ( // MaxEvidenceBytes is a maximum size of any evidence (including amino overhead). - MaxEvidenceBytes = 364 + MaxEvidenceBytes = 440 ) // ErrEvidenceInvalid wraps a piece of evidence and the error denoting how or why it is invalid. diff --git a/types/evidence_test.go b/types/evidence_test.go index fab26b618..68c683518 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -1,6 +1,7 @@ package types import ( + "math" "testing" "github.com/stretchr/testify/assert" @@ -95,13 +96,13 @@ func TestEvidenceList(t *testing.T) { func TestMaxEvidenceBytes(t *testing.T) { val := NewMockPV() - blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), 1000, tmhash.Sum([]byte("partshash"))) - blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), 1000, tmhash.Sum([]byte("partshash"))) + blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt64, tmhash.Sum([]byte("partshash"))) + blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), math.MaxInt64, tmhash.Sum([]byte("partshash"))) const chainID = "mychain" ev := &DuplicateVoteEvidence{ PubKey: secp256k1.GenPrivKey().PubKey(), // use secp because it's pubkey is longer - VoteA: makeVote(val, chainID, 0, 10, 2, 1, blockID), - VoteB: makeVote(val, chainID, 0, 10, 2, 1, blockID2), + VoteA: makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID), + VoteB: makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID2), } bz, err := cdc.MarshalBinary(ev) diff --git a/types/vote.go b/types/vote.go index ac163e8de..6481f56b9 100644 --- a/types/vote.go +++ b/types/vote.go @@ -12,7 +12,7 @@ import ( const ( // MaxVoteBytes is a maximum vote size (including amino overhead). - MaxVoteBytes = 170 + MaxVoteBytes = 200 ) var ( diff --git a/types/vote_test.go b/types/vote_test.go index 679fcd569..4f5449359 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -1,6 +1,7 @@ package types import ( + "math" "testing" "time" @@ -8,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/tmhash" + tmtime "github.com/tendermint/tendermint/types/time" ) func examplePrevote() *Vote { @@ -122,7 +124,21 @@ func TestVoteVerify(t *testing.T) { } func TestMaxVoteBytes(t *testing.T) { - vote := examplePrevote() + vote := &Vote{ + ValidatorAddress: tmhash.Sum([]byte("validator_address")), + ValidatorIndex: math.MaxInt64, + Height: math.MaxInt64, + Round: math.MaxInt64, + Timestamp: tmtime.Now(), + Type: VoteTypePrevote, + BlockID: BlockID{ + Hash: tmhash.Sum([]byte("blockID_hash")), + PartsHeader: PartSetHeader{ + Total: math.MaxInt64, + Hash: tmhash.Sum([]byte("blockID_part_set_header_hash")), + }, + }, + } privVal := NewMockPV() err := privVal.SignVote("test_chain_id", vote)