Skip to content

feat(net): fix TRX inv rate limit bug and add BLOCK inv rate limit#6731

Open
xxo1shine wants to merge 1 commit intotronprotocol:developfrom
xxo1shine:feat/p2p-inv-rate-limit
Open

feat(net): fix TRX inv rate limit bug and add BLOCK inv rate limit#6731
xxo1shine wants to merge 1 commit intotronprotocol:developfrom
xxo1shine:feat/p2p-inv-rate-limit

Conversation

@xxo1shine
Copy link
Copy Markdown
Collaborator

@xxo1shine xxo1shine commented Apr 29, 2026

What does this PR do?

Tighten and extend the inbound inventory rate limit on the P2P layer. Two fixes plus a new knob:

  1. Fix the TRX-inv counting bugP2pEventHandlerImpl previously checked count > maxCountIn10s against the existing 10-second window count only, ignoring
    the size of the incoming InventoryMessage. A peer sending a single message with thousands of hashes could slip through whenever the window count alone was still
    under the limit. The check is now count + currentSize > maxCountIn10s, which compares the projected window against the cap.

  2. Add a BLOCK-inv rate limit — block inventories were previously unbounded. New per-peer cap on BLOCK inventory hashes per 10s, controlled by:

    • node.maxBlockInvPerSecond (default 10, minimum 1)
    • Wired through NodeConfig.maxBlockInvPerSecond (auto-bound by ConfigBeanFactory, clamped in postProcess())
    • Bridged to runtime via Args.applyNodeConfigCommonParameter.maxBlockInvPerSecond
    • Default value mirrored in reference.conf
  3. Refactor for readability — the inline TRX check inside processMessage was extracted into checkInvRateLimit(PeerConnection, InventoryMessage), which now
    handles both TRX and BLOCK branches uniformly.

  4. TestsP2pEventHandlerImplTest adds testCheckInvRateLimitTrxBoundary and testCheckInvRateLimitBlockBoundary to cover the at-limit and over-limit cases
    for both inventory types.

Why are these changes required?

  1. The original TRX check was a TOCTOU-style undercount. A burst peer could legitimately stay below count while still pushing the system over maxCountIn10s
    in a single message — the gating condition simply did not include the new payload. This let an attacker amplify by batching: one message of 10,000 inventory hashes
    was indistinguishable from one of 10. The fix restores the intended invariant: projected window size must stay under the cap, not the current window alone.

  2. Unbounded BLOCK-inv was an unmonitored attack surface. TRX inv had a cap; block inv did not. A misbehaving or malicious peer could flood block-inv hashes —
    each requiring lookup work on the receiver — without any throttling, consuming I/O and CPU. Adding a bounded maxBlockInvPerSecond closes the gap symmetrically,
    with a default tuned for the protocol's healthy block rate (~3 s interval, so 10/s is generous headroom for retransmits and forks).

  3. Surfacing the limit as a config knob (not a constant) lets operators tune it without a release, which is important the first time a new limit ships in case
    the default proves too tight on real networks.

  4. Refactoring out checkInvRateLimit keeps the dispatch path in processMessage short and makes the two inventory-type branches uniform — easier to reason
    about, and easier to add a third inventory type later without re-introducing the old undercount bug.

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

Fixes #6659

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot requested a review from 317787106 April 29, 2026 03:35
@halibobo1205 halibobo1205 added the topic:net p2p net work, synchronization label Apr 29, 2026
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone Apr 29, 2026
@lvs0075 lvs0075 requested a review from waynercheung April 29, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:net p2p net work, synchronization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify and improve rate limiting for TRX and BLOCK INVENTORY Messages

4 participants