From e9e12a97d27ccaa3a11b80f03b897a515ed44aa2 Mon Sep 17 00:00:00 2001 From: Bosul Mun Date: Mon, 14 Jul 2025 15:33:24 +0200 Subject: [PATCH] eth/fetcher: fix announcement drop logic (#32210) This PR fixes an issue in the tx_fetcher DoS prevention logic where the code keeps the overflow amount (`want - maxTxAnnounces`) instead of the allowed amount (`maxTxAnnounces - used`). The specific changes are: - Correct slice indexing in the announcement drop logic - Extend the overflow test case to cover the inversion scenario --- eth/fetcher/tx_fetcher.go | 4 ++-- eth/fetcher/tx_fetcher_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/eth/fetcher/tx_fetcher.go b/eth/fetcher/tx_fetcher.go index 98a1c6e9a6..3e050320e9 100644 --- a/eth/fetcher/tx_fetcher.go +++ b/eth/fetcher/tx_fetcher.go @@ -439,8 +439,8 @@ func (f *TxFetcher) loop() { if want > maxTxAnnounces { txAnnounceDOSMeter.Mark(int64(want - maxTxAnnounces)) - ann.hashes = ann.hashes[:want-maxTxAnnounces] - ann.metas = ann.metas[:want-maxTxAnnounces] + ann.hashes = ann.hashes[:maxTxAnnounces-used] + ann.metas = ann.metas[:maxTxAnnounces-used] } // All is well, schedule the remainder of the transactions var ( diff --git a/eth/fetcher/tx_fetcher_test.go b/eth/fetcher/tx_fetcher_test.go index c4c8cac56e..0f05a1c995 100644 --- a/eth/fetcher/tx_fetcher_test.go +++ b/eth/fetcher/tx_fetcher_test.go @@ -1179,6 +1179,24 @@ func TestTransactionFetcherDoSProtection(t *testing.T) { size: 111, }) } + var ( + hashesC []common.Hash + typesC []byte + sizesC []uint32 + announceC []announce + ) + for i := 0; i < maxTxAnnounces+2; i++ { + hash := common.Hash{0x03, byte(i / 256), byte(i % 256)} + hashesC = append(hashesC, hash) + typesC = append(typesC, types.LegacyTxType) + sizesC = append(sizesC, 111) + + announceC = append(announceC, announce{ + hash: hash, + kind: types.LegacyTxType, + size: 111, + }) + } testTransactionFetcherParallel(t, txFetcherTest{ init: func() *TxFetcher { return NewTxFetcher( @@ -1192,43 +1210,52 @@ func TestTransactionFetcherDoSProtection(t *testing.T) { // Announce half of the transaction and wait for them to be scheduled doTxNotify{peer: "A", hashes: hashesA[:maxTxAnnounces/2], types: typesA[:maxTxAnnounces/2], sizes: sizesA[:maxTxAnnounces/2]}, doTxNotify{peer: "B", hashes: hashesB[:maxTxAnnounces/2-1], types: typesB[:maxTxAnnounces/2-1], sizes: sizesB[:maxTxAnnounces/2-1]}, + doTxNotify{peer: "C", hashes: hashesC[:maxTxAnnounces/2-1], types: typesC[:maxTxAnnounces/2-1], sizes: sizesC[:maxTxAnnounces/2-1]}, doWait{time: txArriveTimeout, step: true}, // Announce the second half and keep them in the wait list doTxNotify{peer: "A", hashes: hashesA[maxTxAnnounces/2 : maxTxAnnounces], types: typesA[maxTxAnnounces/2 : maxTxAnnounces], sizes: sizesA[maxTxAnnounces/2 : maxTxAnnounces]}, doTxNotify{peer: "B", hashes: hashesB[maxTxAnnounces/2-1 : maxTxAnnounces-1], types: typesB[maxTxAnnounces/2-1 : maxTxAnnounces-1], sizes: sizesB[maxTxAnnounces/2-1 : maxTxAnnounces-1]}, + doTxNotify{peer: "C", hashes: hashesC[maxTxAnnounces/2-1 : maxTxAnnounces-1], types: typesC[maxTxAnnounces/2-1 : maxTxAnnounces-1], sizes: sizesC[maxTxAnnounces/2-1 : maxTxAnnounces-1]}, // Ensure the hashes are split half and half isWaiting(map[string][]announce{ "A": announceA[maxTxAnnounces/2 : maxTxAnnounces], "B": announceB[maxTxAnnounces/2-1 : maxTxAnnounces-1], + "C": announceC[maxTxAnnounces/2-1 : maxTxAnnounces-1], }), isScheduled{ tracking: map[string][]announce{ "A": announceA[:maxTxAnnounces/2], "B": announceB[:maxTxAnnounces/2-1], + "C": announceC[:maxTxAnnounces/2-1], }, fetching: map[string][]common.Hash{ "A": hashesA[:maxTxRetrievals], "B": hashesB[:maxTxRetrievals], + "C": hashesC[:maxTxRetrievals], }, }, // Ensure that adding even one more hash results in dropping the hash doTxNotify{peer: "A", hashes: []common.Hash{hashesA[maxTxAnnounces]}, types: []byte{typesA[maxTxAnnounces]}, sizes: []uint32{sizesA[maxTxAnnounces]}}, doTxNotify{peer: "B", hashes: hashesB[maxTxAnnounces-1 : maxTxAnnounces+1], types: typesB[maxTxAnnounces-1 : maxTxAnnounces+1], sizes: sizesB[maxTxAnnounces-1 : maxTxAnnounces+1]}, + doTxNotify{peer: "C", hashes: hashesC[maxTxAnnounces-1 : maxTxAnnounces+2], types: typesC[maxTxAnnounces-1 : maxTxAnnounces+2], sizes: sizesC[maxTxAnnounces-1 : maxTxAnnounces+2]}, isWaiting(map[string][]announce{ "A": announceA[maxTxAnnounces/2 : maxTxAnnounces], "B": announceB[maxTxAnnounces/2-1 : maxTxAnnounces], + "C": announceC[maxTxAnnounces/2-1 : maxTxAnnounces], }), isScheduled{ tracking: map[string][]announce{ "A": announceA[:maxTxAnnounces/2], "B": announceB[:maxTxAnnounces/2-1], + "C": announceC[:maxTxAnnounces/2-1], }, fetching: map[string][]common.Hash{ "A": hashesA[:maxTxRetrievals], "B": hashesB[:maxTxRetrievals], + "C": hashesC[:maxTxRetrievals], }, }, },