Avoid duplicate connection attempts to active nodes before handshake completion

[zeusoo001]

Rationale

Currently, the active connection task is executed every 3.6 seconds. During each execution, the task attempts to actively connect to all active nodes defined in the configuration file that have not yet successfully established a connection. The logic is illustrated as follows:

We have observed the following situation:

When the peer node experiences network latency or high system load, the P2P handshake process may be significantly delayed. As a result, multiple scheduled connection tasks may initiate several concurrent connection attempts to the same peer before the handshake completes. This causes the peer node to receive and process multiple handshake messages at nearly the same time, leading to unnecessary system resource consumption.
For example, node A initiates a connection to node B every 3.6 seconds, as shown in the following logs:

If node B is under heavy load and messages are queued, it might process multiple hello messages simultaneously at 17:05:56, as shown below:

Implementation

Add a handshake timeout mechanism for active nodes. If the handshake for a given node has not yet timed out, the system should not initiate a new connection request to the same node.

When initiating an active connection, the node address is cached for 120 seconds.
During the connection task, when selecting nodes (except those explicitly listed as active nodes), the system checks whether the node’s address already exists in the cache. If it does, it indicates that a connection attempt has already been made within the last 120 seconds, and the system will not initiate another one until the cache entry expires.

Proposed Optimization
Extend the same caching logic to active nodes:
Before initiating a connection to an active node, check whether its address exists in the cache. If it does, skip the connection attempt.

To ensure that disconnected active nodes can reconnect properly, remove their address from the cache when the connection is closed.

[jakamobiii]

Why didn’t the previous caching logic cover active nodes?

[zeusoo001]

@jakamobiii The original design assumed that the number of active nodes was relatively small and that their connection quality was generally high, so they were not included in the caching restrictions.
However, in real-world operation we observed that even active nodes can experience handshake delays due to high load or network issues, which can still trigger repeated connection attempts. Therefore, it is necessary to apply the same caching logic uniformly.

[jakamobiii]

Will this optimization affect existing network behavior or protocol compatibility?

[zeusoo001]

@jakamobiii No. This optimization only affects the frequency of connection initiation. It does not change the handshake protocol itself, nor does it affect message formats or network topology.
From an external perspective, the behavior remains standard libp2p connection behavior, with the only difference being that unnecessary repeated connection attempts are avoided.

[GrothenDI]

How is this 120-second cache time determined?

[zeusoo001]

@GrothenDI This is a peer cooldown / backoff period. After a disconnect, we temporarily avoid reconnecting in both directions to prevent frequent flapping or unstable peers from repeatedly triggering connect/disconnect cycles, which would otherwise put unnecessary pressure on connection management and system resources.

The 2-minute duration is a trade-off chosen based on historical observations of common network jitter patterns and peer stability behavior.

[lxcmyf]

@zeusoo001
The issue mentions extending the cache check logic to active peers as well. Could this potentially affect legitimate reconnection attempts to non-active peers (e.g. after a long disconnect)? How is false-positive duplicate detection avoided?

Has there been any consideration of limiting concurrent handshake attempts per remote peer (e.g. at most one in-flight handshake)? Is something like this already enforced at the connection manager layer?

[warku123]

Hi @zeusoo001,

I have one small suggestion: could we also make the 120-second cache timeout configurable in P2pConfig, because different node has different web situation.

[zeusoo001]

@lxcmyf The logic for non-active peers remains unchanged and will not be affected. Currently, there can only be one ongoing handshake at most; however, no record is saved until the TCP three-way handshake is complete. Saving records would likely require significant code modifications, which is not highly recommended.

[zeusoo001]

@warku123 This configuration is not related to network conditions. It is mainly defined as twice the disallowed reconnection period after a disconnect, in order to prevent malicious peers from repeatedly initiating connections.

[zeusoo001]

Following discussions at a developer meeting, the above solution was tentatively agreed upon for further optimization.