diff --git a/mempool/mempool.go b/mempool/mempool.go index 15e3119c4..b84eb4a68 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -11,7 +11,6 @@ import ( "github.com/pkg/errors" - amino "github.com/tendermint/go-amino" abci "github.com/tendermint/tendermint/abci/types" cfg "github.com/tendermint/tendermint/config" auto "github.com/tendermint/tendermint/libs/autofile" @@ -88,8 +87,12 @@ func IsPreCheckError(err error) bool { func PreCheckAminoMaxBytes(maxBytes int64) PreCheckFunc { return func(tx types.Tx) error { // We have to account for the amino overhead in the tx size as well - aminoOverhead := amino.UvarintSize(uint64(len(tx))) - txSize := int64(len(tx) + aminoOverhead) + // NOTE: fieldNum = 1 as types.Block.Data contains Txs []Tx as first field. + // If this field order ever changes this needs to updated here accordingly. + // NOTE: if some []Tx are encoded without a parenting struct, the + // fieldNum is also equal to 1. + aminoOverhead := types.ComputeAminoOverhead(tx, 1) + txSize := int64(len(tx)) + aminoOverhead if txSize > maxBytes { return fmt.Errorf("Tx size (including amino overhead) is too big: %d, max: %d", txSize, maxBytes) @@ -482,7 +485,7 @@ func (mem *Mempool) ReapMaxBytesMaxGas(maxBytes, maxGas int64) types.Txs { for e := mem.txs.Front(); e != nil; e = e.Next() { memTx := e.Value.(*mempoolTx) // Check total size requirement - aminoOverhead := int64(amino.UvarintSize(uint64(len(memTx.tx)))) + aminoOverhead := types.ComputeAminoOverhead(memTx.tx, 1) if maxBytes > -1 && totalBytes+int64(len(memTx.tx))+aminoOverhead > maxBytes { return txs } diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index e25da7fed..d7ab82737 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -107,11 +107,11 @@ func TestReapMaxBytesMaxGas(t *testing.T) { {20, 0, -1, 0}, {20, 0, 10, 0}, {20, 10, 10, 0}, - {20, 21, 10, 1}, - {20, 210, -1, 10}, - {20, 210, 5, 5}, - {20, 210, 10, 10}, - {20, 210, 15, 10}, + {20, 22, 10, 1}, + {20, 220, -1, 10}, + {20, 220, 5, 5}, + {20, 220, 10, 10}, + {20, 220, 15, 10}, {20, 20000, -1, 20}, {20, 20000, 5, 5}, {20, 20000, 30, 20}, @@ -145,15 +145,15 @@ func TestMempoolFilters(t *testing.T) { {10, nopPreFilter, nopPostFilter, 10}, {10, PreCheckAminoMaxBytes(10), nopPostFilter, 0}, {10, PreCheckAminoMaxBytes(20), nopPostFilter, 0}, - {10, PreCheckAminoMaxBytes(21), nopPostFilter, 10}, + {10, PreCheckAminoMaxBytes(22), nopPostFilter, 10}, {10, nopPreFilter, PostCheckMaxGas(-1), 10}, {10, nopPreFilter, PostCheckMaxGas(0), 0}, {10, nopPreFilter, PostCheckMaxGas(1), 10}, {10, nopPreFilter, PostCheckMaxGas(3000), 10}, {10, PreCheckAminoMaxBytes(10), PostCheckMaxGas(20), 0}, {10, PreCheckAminoMaxBytes(30), PostCheckMaxGas(20), 10}, - {10, PreCheckAminoMaxBytes(21), PostCheckMaxGas(1), 10}, - {10, PreCheckAminoMaxBytes(21), PostCheckMaxGas(0), 0}, + {10, PreCheckAminoMaxBytes(22), PostCheckMaxGas(1), 10}, + {10, PreCheckAminoMaxBytes(22), PostCheckMaxGas(0), 0}, } for tcIndex, tt := range tests { mempool.Update(1, emptyTxArr, tt.preFilter, tt.postFilter) diff --git a/node/node.go b/node/node.go index 4710397fa..0652a392f 100644 --- a/node/node.go +++ b/node/node.go @@ -265,17 +265,8 @@ func NewNode(config *cfg.Config, proxyApp.Mempool(), state.LastBlockHeight, mempl.WithMetrics(memplMetrics), - mempl.WithPreCheck( - mempl.PreCheckAminoMaxBytes( - types.MaxDataBytesUnknownEvidence( - state.ConsensusParams.BlockSize.MaxBytes, - state.Validators.Size(), - ), - ), - ), - mempl.WithPostCheck( - mempl.PostCheckMaxGas(state.ConsensusParams.BlockSize.MaxGas), - ), + mempl.WithPreCheck(sm.TxPreCheck(state)), + mempl.WithPostCheck(sm.TxPostCheck(state)), ) mempoolLogger := logger.With("module", "mempool") mempool.SetLogger(mempoolLogger) diff --git a/state/execution.go b/state/execution.go index cc8e7e75f..4f5a1545c 100644 --- a/state/execution.go +++ b/state/execution.go @@ -8,7 +8,6 @@ import ( dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/fail" "github.com/tendermint/tendermint/libs/log" - "github.com/tendermint/tendermint/mempool" "github.com/tendermint/tendermint/proxy" "github.com/tendermint/tendermint/types" ) @@ -180,13 +179,8 @@ func (blockExec *BlockExecutor) Commit( err = blockExec.mempool.Update( block.Height, block.Txs, - mempool.PreCheckAminoMaxBytes( - types.MaxDataBytesUnknownEvidence( - state.ConsensusParams.BlockSize.MaxBytes, - state.Validators.Size(), - ), - ), - mempool.PostCheckMaxGas(state.ConsensusParams.BlockSize.MaxGas), + TxPreCheck(state), + TxPostCheck(state), ) return res.Data, err diff --git a/state/tx_filter.go b/state/tx_filter.go index b8882d8ed..518eb1877 100644 --- a/state/tx_filter.go +++ b/state/tx_filter.go @@ -1,15 +1,22 @@ package state import ( + mempl "github.com/tendermint/tendermint/mempool" "github.com/tendermint/tendermint/types" ) -// TxFilter returns a function to filter transactions. The function limits the -// size of a transaction to the maximum block's data size. -func TxFilter(state State) func(tx types.Tx) bool { +// TxPreCheck returns a function to filter transactions before processing. +// The function limits the size of a transaction to the block's maximum data size. +func TxPreCheck(state State) mempl.PreCheckFunc { maxDataBytes := types.MaxDataBytesUnknownEvidence( state.ConsensusParams.BlockSize.MaxBytes, state.Validators.Size(), ) - return func(tx types.Tx) bool { return int64(len(tx)) <= maxDataBytes } + return mempl.PreCheckAminoMaxBytes(maxDataBytes) +} + +// TxPostCheck returns a function to filter transactions after processing. +// The function limits the gas wanted by a transaction to the block's maximum total gas. +func TxPostCheck(state State) mempl.PostCheckFunc { + return mempl.PostCheckMaxGas(state.ConsensusParams.BlockSize.MaxGas) } diff --git a/state/tx_filter_test.go b/state/tx_filter_test.go index e6b8999f4..52ae396bf 100644 --- a/state/tx_filter_test.go +++ b/state/tx_filter_test.go @@ -18,12 +18,18 @@ func TestTxFilter(t *testing.T) { genDoc := randomGenesisDoc() genDoc.ConsensusParams.BlockSize.MaxBytes = 3000 + // Max size of Txs is much smaller than size of block, + // since we need to account for commits and evidence. testCases := []struct { - tx types.Tx - isTxValid bool + tx types.Tx + isErr bool }{ - {types.Tx(cmn.RandBytes(250)), true}, - {types.Tx(cmn.RandBytes(3001)), false}, + {types.Tx(cmn.RandBytes(250)), false}, + {types.Tx(cmn.RandBytes(1809)), false}, + {types.Tx(cmn.RandBytes(1810)), false}, + {types.Tx(cmn.RandBytes(1811)), true}, + {types.Tx(cmn.RandBytes(1812)), true}, + {types.Tx(cmn.RandBytes(3000)), true}, } for i, tc := range testCases { @@ -31,8 +37,12 @@ func TestTxFilter(t *testing.T) { state, err := LoadStateFromDBOrGenesisDoc(stateDB, genDoc) require.NoError(t, err) - f := TxFilter(state) - assert.Equal(t, tc.isTxValid, f(tc.tx), "#%v", i) + f := TxPreCheck(state) + if tc.isErr { + assert.NotNil(t, f(tc.tx), "#%v", i) + } else { + assert.Nil(t, f(tc.tx), "#%v", i) + } } } diff --git a/types/block.go b/types/block.go index 4ae51d4df..b2cddb5e3 100644 --- a/types/block.go +++ b/types/block.go @@ -21,6 +21,8 @@ const ( // MaxAminoOverheadForBlock - maximum amino overhead to encode a block (up to // MaxBlockSizeBytes in size) not including it's parts except Data. + // This means it also excludes the overhead for individual transactions. + // To compute individual transactions' overhead use types.ComputeAminoOverhead(tx types.Tx, fieldNum int). // // Uvarint length of MaxBlockSizeBytes: 4 bytes // 2 fields (2 embedded): 2 bytes diff --git a/types/block_test.go b/types/block_test.go index cdea293f0..bedd8c8da 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -250,7 +250,7 @@ func TestMaxHeaderBytes(t *testing.T) { timestamp := time.Date(math.MaxInt64, 0, 0, 0, 0, 0, math.MaxInt64, time.UTC) h := Header{ - Version: version.Consensus{math.MaxInt64, math.MaxInt64}, + Version: version.Consensus{Block: math.MaxInt64, App: math.MaxInt64}, ChainID: maxChainID, Height: math.MaxInt64, Time: timestamp, diff --git a/types/tx.go b/types/tx.go index 10c097e36..41be77946 100644 --- a/types/tx.go +++ b/types/tx.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" + "github.com/tendermint/go-amino" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/crypto/tmhash" @@ -118,3 +120,18 @@ type TxResult struct { Tx Tx `json:"tx"` Result abci.ResponseDeliverTx `json:"result"` } + +// ComputeAminoOverhead calculates the overhead for amino encoding a transaction. +// The overhead consists of varint encoding the field number and the wire type +// (= length-delimited = 2), and another varint encoding the length of the +// transaction. +// The field number can be the field number of the particular transaction, or +// the field number of the parenting struct that contains the transactions []Tx +// as a field (this field number is repeated for each contained Tx). +// If some []Tx are encoded directly (without a parenting struct), the default +// fieldNum is also 1 (see BinFieldNum in amino.MarshalBinaryBare). +func ComputeAminoOverhead(tx Tx, fieldNum int) int64 { + fnum := uint64(fieldNum) + typ3AndFieldNum := (uint64(fnum) << 3) | uint64(amino.Typ3_ByteLength) + return int64(amino.UvarintSize(typ3AndFieldNum)) + int64(amino.UvarintSize(uint64(len(tx)))) +} diff --git a/types/tx_test.go b/types/tx_test.go index 6ce23d6f5..3afaaccc1 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -96,6 +96,63 @@ func TestTxProofUnchangable(t *testing.T) { } } +func TestComputeTxsOverhead(t *testing.T) { + cases := []struct { + txs Txs + wantOverhead int + }{ + {Txs{[]byte{6, 6, 6, 6, 6, 6}}, 2}, + // one 21 Mb transaction: + {Txs{make([]byte, 22020096, 22020096)}, 5}, + // two 21Mb/2 sized transactions: + {Txs{make([]byte, 11010048, 11010048), make([]byte, 11010048, 11010048)}, 10}, + {Txs{[]byte{1, 2, 3}, []byte{1, 2, 3}, []byte{4, 5, 6}}, 6}, + {Txs{[]byte{100, 5, 64}, []byte{42, 116, 118}, []byte{6, 6, 6}, []byte{6, 6, 6}}, 8}, + } + + for _, tc := range cases { + totalBytes := int64(0) + totalOverhead := int64(0) + for _, tx := range tc.txs { + aminoOverhead := ComputeAminoOverhead(tx, 1) + totalOverhead += aminoOverhead + totalBytes += aminoOverhead + int64(len(tx)) + } + bz, err := cdc.MarshalBinaryBare(tc.txs) + assert.EqualValues(t, tc.wantOverhead, totalOverhead) + assert.NoError(t, err) + assert.EqualValues(t, len(bz), totalBytes) + } +} + +func TestComputeAminoOverhead(t *testing.T) { + cases := []struct { + tx Tx + fieldNum int + want int + }{ + {[]byte{6, 6, 6}, 1, 2}, + {[]byte{6, 6, 6}, 16, 3}, + {[]byte{6, 6, 6}, 32, 3}, + {[]byte{6, 6, 6}, 64, 3}, + {[]byte{6, 6, 6}, 512, 3}, + {[]byte{6, 6, 6}, 1024, 3}, + {[]byte{6, 6, 6}, 2048, 4}, + {make([]byte, 64), 1, 2}, + {make([]byte, 65), 1, 2}, + {make([]byte, 127), 1, 2}, + {make([]byte, 128), 1, 3}, + {make([]byte, 256), 1, 3}, + {make([]byte, 512), 1, 3}, + {make([]byte, 1024), 1, 3}, + {make([]byte, 128), 16, 4}, + } + for _, tc := range cases { + got := ComputeAminoOverhead(tc.tx, tc.fieldNum) + assert.EqualValues(t, tc.want, got) + } +} + func testTxProofUnchangable(t *testing.T) { // make some proof txs := makeTxs(randInt(2, 100), randInt(16, 128))