Skip to content

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

Merged
lvs0075 merged 1 commit into
tronprotocol:developfrom
xxo1shine:feat/p2p-inv-rate-limit
May 7, 2026
Merged

feat(net): fix TRX inv rate limit bug and add BLOCK inv rate limit#6731
lvs0075 merged 1 commit into
tronprotocol:developfrom
xxo1shine:feat/p2p-inv-rate-limit

Conversation

@xxo1shine

@xxo1shine xxo1shine commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

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

@waynercheung waynercheung left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@lvs0075
lvs0075 merged commit 6c56161 into tronprotocol:develop May 7, 2026
16 checks passed
@github-project-automation github-project-automation Bot moved this to Done in java-tron May 7, 2026
@xxo1shine
xxo1shine deleted the feat/p2p-inv-rate-limit branch June 3, 2026 03:23
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

Status: Done

Development

Successfully merging this pull request may close these issues.

Unify and improve rate limiting for TRX and BLOCK INVENTORY Messages

7 participants