Skip to content

Commit 354378d

Browse files
rootulpclaude
authored andcommitted
fix: enforce maxRequestsPerPeer on direct SeenTx path in CAT mempool (#2880)
## Summary - Enforce `maxRequestsPerPeer` check before calling `requestTx` on the direct SeenTx handler path (when signer/sequence info is missing) - Previously a single peer could bypass the intended 30-request per-peer cap by sending SeenTx messages with nil signer and zero sequence - Add regression test `TestSeenTxDirectPathEnforcesPerPeerRequestLimit` ## Test plan - [x] New test `TestSeenTxDirectPathEnforcesPerPeerRequestLimit` fails before fix (120 requests) and passes after (capped at 30) - [x] All existing `mempool/cat` tests pass Closes https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-235 Closes https://linear.app/celestia/issue/PROTOCO-1295/triage-celestia-235-should-be-treated-as-main 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/celestiaorg/celestia-core/pull/2880" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (cherry picked from commit 88d3c08)
1 parent 252b1bd commit 354378d

3 files changed

Lines changed: 59 additions & 1 deletion

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- Enforce `maxRequestsPerPeer` on the direct SeenTx→requestTx path in the
2+
CAT mempool reactor. Previously, a peer could bypass the 30-request per-peer
3+
cap by sending SeenTx messages without signer/sequence information.

mempool/cat/reactor.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,17 @@ func (memR *Reactor) Receive(e p2p.Envelope) {
422422
}
423423

424424
// We don't have the transaction, nor are we requesting it so we send the node
425-
// a want msg
425+
// a want msg. Enforce the per-peer request limit so a single peer cannot
426+
// drive unbounded outstanding requests via the direct SeenTx path.
427+
if memR.requests.CountForPeer(peerID) >= maxRequestsPerPeer {
428+
memR.Logger.Debug(
429+
"not requesting tx from peer: at capacity",
430+
"peer", peerID,
431+
"txKey", txKey,
432+
"maxRequestsPerPeer", maxRequestsPerPeer,
433+
)
434+
return
435+
}
426436
memR.requestTx(txKey, e.Src)
427437

428438
// A peer is requesting a transaction that we have claimed to have. Find the specified

mempool/cat/reactor_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,3 +1516,48 @@ func TestSeenTxWithEmptySignerNotBanned(t *testing.T) {
15161516
// The peer should NOT be disconnected for empty signer
15171517
require.Len(t, reactor0.Switch.Peers().List(), 1, "peer should not be disconnected for empty signer")
15181518
}
1519+
1520+
// TestSeenTxDirectPathEnforcesPerPeerRequestLimit verifies that the direct
1521+
// SeenTx→requestTx code path enforces the maxRequestsPerPeer limit. A single
1522+
// peer sending more than maxRequestsPerPeer unique SeenTx messages (with nil
1523+
// signer / zero sequence) should NOT result in more than maxRequestsPerPeer
1524+
// outstanding requests to that peer.
1525+
func TestSeenTxDirectPathEnforcesPerPeerRequestLimit(t *testing.T) {
1526+
reactor, _ := setupReactor(t)
1527+
1528+
peer := genPeer()
1529+
// Allow unlimited WantTx sends (we're counting how many get through)
1530+
peer.On("TrySend", mock.MatchedBy(func(env p2p.Envelope) bool {
1531+
return env.ChannelID == MempoolWantsChannel
1532+
})).Return(true)
1533+
1534+
_, err := reactor.InitPeer(peer)
1535+
require.NoError(t, err)
1536+
peerID := reactor.ids.GetIDForPeer(peer.ID())
1537+
1538+
totalMessages := maxRequestsPerPeer * 4 // 120 messages from one peer
1539+
1540+
for i := 0; i < totalMessages; i++ {
1541+
tx := types.Tx(fmt.Sprintf("fake-seen-tx-%d", i))
1542+
key := tx.Key()
1543+
reactor.Receive(p2p.Envelope{
1544+
ChannelID: MempoolDataChannel,
1545+
Message: &protomem.SeenTx{
1546+
TxKey: key[:],
1547+
Signer: nil,
1548+
Sequence: 0,
1549+
},
1550+
Src: peer,
1551+
})
1552+
}
1553+
1554+
requestCount := reactor.requests.CountForPeer(peerID)
1555+
t.Logf("peer=%d requests=%d maxRequestsPerPeer=%d", peerID, requestCount, maxRequestsPerPeer)
1556+
1557+
// The per-peer request limit should be enforced on the direct SeenTx path.
1558+
// If this assertion fails, it means the SeenTx handler is calling requestTx
1559+
// without checking maxRequestsPerPeer first.
1560+
assert.LessOrEqual(t, requestCount, maxRequestsPerPeer,
1561+
"SeenTx direct path should not exceed maxRequestsPerPeer (%d) but got %d requests",
1562+
maxRequestsPerPeer, requestCount)
1563+
}

0 commit comments

Comments
 (0)