fix(tcp): don't handle application message before finish handshake

[317787106]

What does this PR do?

• Close connections that send application messages before handshake completion with BAD_PROTOCOL, without setting the handshake flag, registering the peer, or invoking onConnect() / onMessage().
• Validate networkId for inbound and outbound HELLO messages, and validate rejection codes on outbound handshake responses, before peer admission.
• Separate admission checks (checkPeer) from registration (addPeer). Run admission checks, handshake completion, and registration under the same ChannelManager lock to preserve connection limits during concurrent admission. Retain the existing single-argument processPeer API.
• Send the inbound HELLO response before enabling negotiated message compression. Reject self-connections before registration and prevent closed connections, including those closed while sending the HELLO reply, from completing admission.

Why are these changes required?

Receiving an application message before libp2p HELLO currently marks the handshake as complete, registers the peer, and invokes onConnect() without validating the HELLO network ID. The normal HELLO path also registers the peer before checking its network ID. This PR rejects premature application messages and completes HELLO validation and handshake processing before registering the connection or notifying application handlers.

This PR has been tested by:

• Added 15 EmbeddedChannel regression tests covering premature application messages, trusted and outbound connections, network mismatches, rejected HELLO responses, admission limits, self-connections, closure during handshake, registration/callback ordering, and compression after HELLO.

• All 25 targeted tests passed locally:

  ./gradlew test \
    --tests org.tron.p2p.connection.HandshakeAdmissionTest \
    --tests org.tron.p2p.connection.ChannelManagerTest \
    --tests org.tron.p2p.connection.MessageTest \
    --tests org.tron.p2p.connection.message.handshake.HelloMessageTest

• Running the new regression suite against the unchanged base (c564f26) produced 13 failures; all 15 pass with this fix.

• A broader connection-package test run encountered the existing ConnPoolServiceTest.getNodes_orderByUpdateTimeDesc failure. The same failure was reproduced on the unchanged base: the test expects strict ordering while getNodes() shuffles candidates.

Follow up

Pre-handshake TCP resource quotas and absolute handshake deadlines are addressed separately in #146.

Extra details

Outbound handshakes now require a matching networkId. The legacy fallback that accepted a matching version despite a different or absent networkId is removed.

[SeriousCoding789]

LGTM overall — one small thing worth changing:**

Skip the second HELLO reply on the UNKNOWN path.
In finishHandshake, the inbound NORMAL path has already sent sendHelloMsg(channel, NORMAL, ...) before the isDisconnect() || !isOpen() check. When that check fails and UNKNOWN is returned, the caller in processMessage runs if (!channel.isActive()) sendHelloMsg(channel, code, ...) again — so a closed inbound channel gets a second HELLO with code 256. It's harmless (either dropped by send()'s isDisconnect guard or a failed writeAndFlush with a WARN), but it's a wasted write plus a misleading log line. Suggest returning early for UNKNOWN without a reply, e.g.:

if (code != DisconnectCode.NORMAL) {
  if (!channel.isActive() && code != DisconnectCode.UNKNOWN) {
    sendHelloMsg(channel, code, msg.getTimestamp());
  }
  ...

[3for]

[SHOULD] This fixes the libp2p handshake gate, but java-tron has a separate application handshake that also needs admission checks. Please track a companion fix covering all dispatch paths, including the early PBFT branch, with integration tests.

[3for]

[QUESTION] The strict networkId check removes the legacy version fallback, as confirmed by protocolVersionCannotSubstituteForNetworkId. Peers using the v0.1 format, which only includes version, will now be rejected with DIFFERENT_VERSION. Is it intentional? If so, please document the breaking compatibility change in the release notes.

[317787106]

The stricter check is intentional, but standard v0.1.0 HELLO messages remain compatible. In the v0.1.0 schema, version uses field number 2, which is network_id in the current schema. Protobuf therefore decodes the old network identifier directly into networkId.

I verified that v0.1-format HELLO messages with a matching network identifier still complete both inbound and outbound handshakes.

The removed fallback accepted outbound HELLO messages with a mismatched networkId when the current version field (number 5) happened to match the local network identifier. The release notes should describe that stricter validation, without implying that v0.1 HELLO support was removed.

[317787106]

[SHOULD] This fixes the libp2p handshake gate, but java-tron has a separate application handshake that also needs admission checks. Please track a companion fix covering all dispatch paths, including the early PBFT branch, with integration tests.

Yes, the admission checks of java-tron will be added in another PR.

[3for]

[SHOULD] Please preserve a live, registered connection when a pending handshake claims the same nodeId. Hello does not authenticate that identity, so channel creation time should not authorize replacing an established connection. Please reject the conflicting handshake and add regression coverage, including simultaneous inbound/outbound dialing.

[317787106]

The startTime of the channel is used to distinguish the sequence of the two-way dialing; this startTime is derived from the local system clock. Prefer to perserve a earlier connection is easier to understand.

[3for]

I understand the intention to resolve simultaneous dialing by preferring the locally older channel. However, an older channel may still have a pending handshake:

1. Connection A opens first and delays its Hello.
2. Connection B completes its handshake and is registered.
3. A then sends a Hello with the correct networkId and B’s nodeId. The current logic closes B because A was created earlier.

Since Hello does not authenticate the claimed nodeId, a pending connection can evict an established peer without forging any timestamp.

Please preserve the live, registered connection and reject the conflicting handshake. Also add regression coverage for this delayed-Hello case and simultaneous inbound/outbound dialing to ensure both ends converge on the same surviving connection.

[317787106]

The delayed-Hello scenario is valid, but our current assessment is that the practical risk is low: the pending connection must be created before the established connection and send the conflicting Hello before timing out.

A robust fix would require coordinated changes to peer admission, pending outbound-handshake tracking, simultaneous-dial resolution, and disconnect/IP-ban handling. This adds significant complexity and regression risk, particularly around preserving connectivity during simultaneous dialing.

Given that trade-off, we prefer to keep the existing behavior for now and defer this fix.