max-bytes PR follow-up (#2318)

* 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

@@ -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) {
+func WithFilter(f func(types.Tx) bool) MempoolOption {
-	mem.proxyMtx.Lock()
+	return func(mem *Mempool) { mem.filter = f }
-	mem.filter = f
-	mem.proxyMtx.Unlock()
 }
 
 // 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 {

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)

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 +
+	// MaxBlockSizeBytes in size) not including it's parts except Data.
-	// fields without data).
 	//
 	// Uvarint length of MaxBlockSizeBytes: 4 bytes
-	//														4 fields: 4 bytes
+	// 2 fields (2 embedded):               2 bytes
-	MaxAminoOverheadForBlock = 8
+	// Uvarint length of Data.Txs:          4 bytes
+	// Data.Txs field:                      1 byte
+	MaxAminoOverheadForBlock = 11
 )
 
 // Block defines the atomic unit of a Tendermint blockchain.

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,
+		NumTxs:             math.MaxInt64,
-		TotalTxs:           200,
+		TotalTxs:           math.MaxInt64,
-		LastBlockID:        makeBlockID(make([]byte, 20), 300, make([]byte, 20)),
+		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")),

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.

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")))
+	blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt64, tmhash.Sum([]byte("partshash")))
-	blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), 1000, 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),
+		VoteA:  makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID),
-		VoteB:  makeVote(val, chainID, 0, 10, 2, 1, blockID2),
+		VoteB:  makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, blockID2),
 	}
 
 	bz, err := cdc.MarshalBinary(ev)

types/vote.go

@@ -12,7 +12,7 @@ import (
 
 const (
 	// MaxVoteBytes is a maximum vote size (including amino overhead).
-	MaxVoteBytes = 170
+	MaxVoteBytes = 200
 )
 
 var (

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)