From 99f9ee0f63627462e4daa3e785ec23086aa3a0b5 Mon Sep 17 00:00:00 2001 From: William Banfield <4561443+williambanfield@users.noreply.github.com> Date: Fri, 1 Apr 2022 17:44:38 -0400 Subject: [PATCH] abci++: correct max-size check to only operate on added and unmodified (#8242) --- types/tx.go | 14 ++++++++------ types/tx_test.go | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/types/tx.go b/types/tx.go index b81114452..0c354a941 100644 --- a/types/tx.go +++ b/types/tx.go @@ -188,13 +188,7 @@ func (t TxRecordSet) Validate(maxSizeBytes int64, otxs Txs) error { // Only the slices are copied, the transaction contents are shared. allCopy := sortedCopy(t.all) - var size int64 for i, cur := range allCopy { - size += int64(len(cur)) - if size > maxSizeBytes { - return fmt.Errorf("transaction data size %d exceeds maximum %d", size, maxSizeBytes) - } - // allCopy is sorted, so any duplicated data will be adjacent. if i+1 < len(allCopy) && bytes.Equal(cur, allCopy[i+1]) { return fmt.Errorf("found duplicate transaction with hash: %x", cur.Hash()) @@ -207,6 +201,14 @@ func (t TxRecordSet) Validate(maxSizeBytes int64, otxs Txs) error { removedCopy := sortedCopy(t.removed) unmodifiedCopy := sortedCopy(t.unmodified) + var size int64 + for _, cur := range append(unmodifiedCopy, addedCopy...) { + size += int64(len(cur)) + if size > maxSizeBytes { + return fmt.Errorf("transaction data size %d exceeds maximum %d", size, maxSizeBytes) + } + } + // make a defensive copy of otxs so that the order of // the caller's data is not altered. otxsCopy := sortedCopy(otxs) diff --git a/types/tx_test.go b/types/tx_test.go index d8737e9f0..77afa1b23 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -64,6 +64,25 @@ func TestValidateTxRecordSet(t *testing.T) { err := txrSet.Validate(9, []Tx{}) require.Error(t, err) }) + t.Run("should not error on removed transaction size exceeding max data size", func(t *testing.T) { + trs := []*abci.TxRecord{ + { + Action: abci.TxRecord_ADDED, + Tx: Tx([]byte{1, 2, 3, 4, 5}), + }, + { + Action: abci.TxRecord_ADDED, + Tx: Tx([]byte{6, 7, 8, 9}), + }, + { + Action: abci.TxRecord_REMOVED, + Tx: Tx([]byte{10}), + }, + } + txrSet := NewTxRecordSet(trs) + err := txrSet.Validate(9, []Tx{[]byte{10}}) + require.NoError(t, err) + }) t.Run("should error on duplicate transactions with the same action", func(t *testing.T) { trs := []*abci.TxRecord{ {