Skip to content

Commit ccaba50

Browse files
healthykimhowjmay
authored andcommitted
eth/fetcher: fix announcement drop logic (ethereum#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
1 parent 7ed848c commit ccaba50

File tree

2 files changed

+29
-2
lines changed

2 files changed

+29
-2
lines changed

eth/fetcher/tx_fetcher.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,8 +439,8 @@ func (f *TxFetcher) loop() {
439439
if want > maxTxAnnounces {
440440
txAnnounceDOSMeter.Mark(int64(want - maxTxAnnounces))
441441

442-
ann.hashes = ann.hashes[:want-maxTxAnnounces]
443-
ann.metas = ann.metas[:want-maxTxAnnounces]
442+
ann.hashes = ann.hashes[:maxTxAnnounces-used]
443+
ann.metas = ann.metas[:maxTxAnnounces-used]
444444
}
445445
// All is well, schedule the remainder of the transactions
446446
var (

eth/fetcher/tx_fetcher_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,24 @@ func TestTransactionFetcherDoSProtection(t *testing.T) {
11791179
size: 111,
11801180
})
11811181
}
1182+
var (
1183+
hashesC []common.Hash
1184+
typesC []byte
1185+
sizesC []uint32
1186+
announceC []announce
1187+
)
1188+
for i := 0; i < maxTxAnnounces+2; i++ {
1189+
hash := common.Hash{0x03, byte(i / 256), byte(i % 256)}
1190+
hashesC = append(hashesC, hash)
1191+
typesC = append(typesC, types.LegacyTxType)
1192+
sizesC = append(sizesC, 111)
1193+
1194+
announceC = append(announceC, announce{
1195+
hash: hash,
1196+
kind: types.LegacyTxType,
1197+
size: 111,
1198+
})
1199+
}
11821200
testTransactionFetcherParallel(t, txFetcherTest{
11831201
init: func() *TxFetcher {
11841202
return NewTxFetcher(
@@ -1192,43 +1210,52 @@ func TestTransactionFetcherDoSProtection(t *testing.T) {
11921210
// Announce half of the transaction and wait for them to be scheduled
11931211
doTxNotify{peer: "A", hashes: hashesA[:maxTxAnnounces/2], types: typesA[:maxTxAnnounces/2], sizes: sizesA[:maxTxAnnounces/2]},
11941212
doTxNotify{peer: "B", hashes: hashesB[:maxTxAnnounces/2-1], types: typesB[:maxTxAnnounces/2-1], sizes: sizesB[:maxTxAnnounces/2-1]},
1213+
doTxNotify{peer: "C", hashes: hashesC[:maxTxAnnounces/2-1], types: typesC[:maxTxAnnounces/2-1], sizes: sizesC[:maxTxAnnounces/2-1]},
11951214
doWait{time: txArriveTimeout, step: true},
11961215

11971216
// Announce the second half and keep them in the wait list
11981217
doTxNotify{peer: "A", hashes: hashesA[maxTxAnnounces/2 : maxTxAnnounces], types: typesA[maxTxAnnounces/2 : maxTxAnnounces], sizes: sizesA[maxTxAnnounces/2 : maxTxAnnounces]},
11991218
doTxNotify{peer: "B", hashes: hashesB[maxTxAnnounces/2-1 : maxTxAnnounces-1], types: typesB[maxTxAnnounces/2-1 : maxTxAnnounces-1], sizes: sizesB[maxTxAnnounces/2-1 : maxTxAnnounces-1]},
1219+
doTxNotify{peer: "C", hashes: hashesC[maxTxAnnounces/2-1 : maxTxAnnounces-1], types: typesC[maxTxAnnounces/2-1 : maxTxAnnounces-1], sizes: sizesC[maxTxAnnounces/2-1 : maxTxAnnounces-1]},
12001220

12011221
// Ensure the hashes are split half and half
12021222
isWaiting(map[string][]announce{
12031223
"A": announceA[maxTxAnnounces/2 : maxTxAnnounces],
12041224
"B": announceB[maxTxAnnounces/2-1 : maxTxAnnounces-1],
1225+
"C": announceC[maxTxAnnounces/2-1 : maxTxAnnounces-1],
12051226
}),
12061227
isScheduled{
12071228
tracking: map[string][]announce{
12081229
"A": announceA[:maxTxAnnounces/2],
12091230
"B": announceB[:maxTxAnnounces/2-1],
1231+
"C": announceC[:maxTxAnnounces/2-1],
12101232
},
12111233
fetching: map[string][]common.Hash{
12121234
"A": hashesA[:maxTxRetrievals],
12131235
"B": hashesB[:maxTxRetrievals],
1236+
"C": hashesC[:maxTxRetrievals],
12141237
},
12151238
},
12161239
// Ensure that adding even one more hash results in dropping the hash
12171240
doTxNotify{peer: "A", hashes: []common.Hash{hashesA[maxTxAnnounces]}, types: []byte{typesA[maxTxAnnounces]}, sizes: []uint32{sizesA[maxTxAnnounces]}},
12181241
doTxNotify{peer: "B", hashes: hashesB[maxTxAnnounces-1 : maxTxAnnounces+1], types: typesB[maxTxAnnounces-1 : maxTxAnnounces+1], sizes: sizesB[maxTxAnnounces-1 : maxTxAnnounces+1]},
1242+
doTxNotify{peer: "C", hashes: hashesC[maxTxAnnounces-1 : maxTxAnnounces+2], types: typesC[maxTxAnnounces-1 : maxTxAnnounces+2], sizes: sizesC[maxTxAnnounces-1 : maxTxAnnounces+2]},
12191243

12201244
isWaiting(map[string][]announce{
12211245
"A": announceA[maxTxAnnounces/2 : maxTxAnnounces],
12221246
"B": announceB[maxTxAnnounces/2-1 : maxTxAnnounces],
1247+
"C": announceC[maxTxAnnounces/2-1 : maxTxAnnounces],
12231248
}),
12241249
isScheduled{
12251250
tracking: map[string][]announce{
12261251
"A": announceA[:maxTxAnnounces/2],
12271252
"B": announceB[:maxTxAnnounces/2-1],
1253+
"C": announceC[:maxTxAnnounces/2-1],
12281254
},
12291255
fetching: map[string][]common.Hash{
12301256
"A": hashesA[:maxTxRetrievals],
12311257
"B": hashesB[:maxTxRetrievals],
1258+
"C": hashesC[:maxTxRetrievals],
12321259
},
12331260
},
12341261
},

0 commit comments

Comments
 (0)