feat(net): add rate limiting logic for P2P messages by fyyhtx · Pull Request #6393 · tronprotocol/java-tron

fyyhtx · 2025-07-09T02:36:50Z

What does this PR do?
Add rate limiting logic for P2P messages. Closes #6297
Why are these changes required?

This PR has been tested by:

Unit Tests
Manual Testing

Follow up

Extra details

Sunny6889 · 2025-07-10T03:45:42Z

framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java

-          peer.getChannel().close();
-          peer.getNodeStatistics()
-                  .nodeDisconnectedRemote(((DisconnectMessage)msg).getReason());
+          if (peer.getP2pRateLimiter().tryAcquire(type.asByte())) {


why disconnect need a rate limiter? As in your issue mentioned "After receiving the message, the connection will be disconnected and no response will be given."

Another question: is this rate limit set per Peer or shared by all Peers? Will this rate limiter effect the normal disconnect logic?

Another question: is this rate limit set per Peer or shared by all Peers? Will this rate limiter effect the normal disconnect logic?

This rate limit is set per peer. This rate limiter does not affect the normal disconnection logic.

why disconnect need a rate limiter? As in your issue mentioned "After receiving the message, the connection will be disconnected and no response will be given."

To prevent the peer from sending a large number of repeated disconnect messages at once, it will not respond to the message, but will execute the disconnection logic.

Sunny6889 · 2025-07-10T03:50:46Z

framework/src/main/java/org/tron/core/net/P2pRateLimiter.java

+
+public class P2pRateLimiter {
+  private final Cache<Byte, RateLimiter> rateLimiters = CacheBuilder.newBuilder()
+      .maximumSize(32).build();


here is a hidden size 32 limit, can make it a large one so it normally won't never hit the limit?

The message types defined by the current protocol are very limited, and the limit of 32 is sufficient for the foreseeable future.

Sunny6889 · 2025-07-10T03:52:04Z

framework/src/main/java/org/tron/core/net/peer/PeerConnection.java

    lastInteractiveTime = System.currentTimeMillis();
+    p2pRateLimiter.register(SYNC_BLOCK_CHAIN.asByte(), 2);
+    p2pRateLimiter.register(FETCH_INV_DATA.asByte(), 2);
+    p2pRateLimiter.register(P2P_DISCONNECT.asByte(), 1);


make this hardcode 2,2,1 number to use a static variable

Will relay nodes trigger speed limits? What about busy sync nodes on the network?

Will relay nodes trigger speed limits? What about busy sync nodes on the network?

For synchronization logic, relay nodes are no different from ordinary full nodes.

317787106 · 2025-07-14T09:44:38Z

framework/src/main/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandler.java

        if (!peer.isNeedSyncFromUs()) {
          throw new P2pException(TypeEnum.BAD_MESSAGE, "no need sync");
        }
+        if (!peer.getP2pRateLimiter().tryAcquire(fetchInvDataMsg.getType().asByte())) {


consumerInvToFetch task is scheduled every 30ms, but the limit FETCH_INV_DATA_RATE is 3 times/s, there will be conflict.

This rate limit is based on the premise of block synchronization.

common/src/main/java/org/tron/core/Constant.java

halibobo1205 · 2025-07-29T08:06:27Z

framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java

          code = Protocol.ReasonCode.NO_SUCH_MESSAGE;
          break;
        case BAD_MESSAGE:
+        case RATE_LIMIT_EXCEEDED:


This mapping doesn't seem quite right. RATE_LIMIT_EXCEEDED is a server capacity issue rather than a protocol violation. Should we use a more specific error type or create a dedicated rate limit handler?

According to normal protocol logic, RATE_LIMIT_EXCEEDED will not appear. Once it appears, it is considered a protocol violation.

halibobo1205 · 2025-07-29T08:10:05Z

framework/src/main/java/org/tron/core/config/args/Args.java


+    PARAMETER.rateLimiterSyncBlockChain =
+        config.hasPath(Constant.RATE_LIMITER_P2P_SYNC_BLOCK_CHAIN) ? config
+            .getDouble(Constant.RATE_LIMITER_P2P_SYNC_BLOCK_CHAIN) : 3.0;


Please add a comment specifying the rate limit unit: is this per second, per minute, per hour, or something?

If no special instructions are given, the default is qps.

waynercheung · 2025-07-31T16:54:13Z

common/src/main/java/org/tron/core/Constant.java

  public static final String RATE_LIMITER_RPC = "rate.limiter.rpc";
+  public static final String RATE_LIMITER_P2P_SYNC_BLOCK_CHAIN = "rate.limiter.p2p.syncBlockChain";
+  public static final String RATE_LIMITER_P2P_FETCH_INV_DATA = "rate.limiter.p2p.fetchInvData";
+  public static final String RATE_LIMITER_P2P_DISCONNECT = "rate.limiter.p2p.disconnect";


As user can set the value, so add this configuration to config file? such as config.conf.

This is a repeated question; please refer to the previous response for details.

waynercheung · 2025-07-31T17:08:46Z

framework/src/main/java/org/tron/core/net/P2pRateLimiter.java

+      .maximumSize(32).build();
+
+  public void register(Byte type, double rate) {
+    RateLimiter rateLimiter = RateLimiter.create(Double.POSITIVE_INFINITY);


what about create the RateLimter with rate, such as:
RateLimiter rateLimiter = RateLimiter.create(rate);

The approach you mentioned has a problem: the initial capacity of the token bucket is only 1. Once the peer completes the handshake, it immediately enters the synchronization state. If the initial token count is 1, it may lead to insufficient tokens.

waynercheung · 2025-07-31T17:10:56Z

framework/src/main/java/org/tron/core/net/P2pRateLimiter.java

+  public boolean tryAcquire(Byte type) {
+    RateLimiter rateLimiter = rateLimiters.getIfPresent(type);
+    if (rateLimiter == null) {
+      return true;


Is there any risk if return true for the unregistered type(rateLimter == null)?

Unregistered types return true, meaning no speed limit.

waynercheung · 2025-07-31T17:15:46Z

framework/src/main/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandler.java

+              + " message exceeds the rate limit");
+        }
+        if (fetchInvDataMsg.getHashList().size() > NetConstants.MAX_BLOCK_FETCH_PER_PEER) {
+          throw new P2pException(TypeEnum.BAD_MESSAGE, "fetch too more blocks, size:"


use "fetch too many blocks" instead of "fetch too more blocks".

waynercheung · 2025-07-31T17:18:30Z

framework/src/test/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandlerTest.java

+    try {
+      fetchInvDataMsgHandler.processMessage(peer, msg);
+    } catch (Exception e) {
+      Assert.assertEquals("fetch too more blocks, size:101", e.getMessage());


use "fetch too many blocks" instead of "fetch too more blocks".

jwrct added 5 commits May 6, 2025 18:01

feat(net):P2P message rate limit

aa682c9

feat(net):test for P2P message rate limit

e50e11f

fix(net):fix P2P message rate limit issue

c2adb7d

fix(net):delete unused import

3b3ddac

feat(net):optimize rate limit logic

2454a92

kuny0707 assigned fyyhtx Jul 9, 2025

kuny0707 added this to java-tron Jul 9, 2025

kuny0707 moved this to In Review in java-tron Jul 9, 2025

fyyhtx changed the base branch from develop to release_v4.8.1 July 9, 2025 04:00

fyyhtx changed the title ~~feat(net): add P2P message rate limit logic~~ feat(net): add rate limiting logic for P2P messages Jul 9, 2025

Sunny6889 reviewed Jul 10, 2025

View reviewed changes

feat(net):Optimize rate limiting logic and add tests

81c6528

xxo1shine approved these changes Jul 14, 2025

View reviewed changes

317787106 reviewed Jul 14, 2025

View reviewed changes

feat(net): add configuration for p2p message rate limit

26d5fe4

Sunny6889 reviewed Jul 22, 2025

View reviewed changes

common/src/main/java/org/tron/core/Constant.java Show resolved Hide resolved

halibobo1205 reviewed Jul 29, 2025

View reviewed changes

waynercheung reviewed Jul 31, 2025

View reviewed changes

fix(log): fix log issue

46b1c09

lvs0075 approved these changes Aug 8, 2025

View reviewed changes

kuny0707 approved these changes Aug 13, 2025

View reviewed changes

kuny0707 merged commit c1f3956 into tronprotocol:release_v4.8.1 Aug 13, 2025
7 checks passed

github-project-automation bot moved this from In Review to Done in java-tron Aug 13, 2025

Conversation

fyyhtx commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sunny6889 Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

fyyhtx commented Jul 9, 2025 •

edited

Loading

Sunny6889 Jul 10, 2025 •

edited

Loading