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
Copy link
Copy Markdown
Collaborator

@waynercheung waynercheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

5 participants