Ambiguous Reason Code in Disconnection Message

[jwrct]

Software Versions

OS : Linux
JVM : Oracle Corporation 1.8.0_161 amd64
Git : a8ad2a169e58946b5b8de6ecf7b7ef5b8db05aff
Version : 4.7.5
Code : 18306

Expected behaviour

When disconnected, the exact reason for the disconnection can be obtained through the disconnection message.

Actual behaviour

When running Java-tron, I found that if I wanted to disconnect a peer in certain scenarios, the disconnect message sent carried an ambiguous reason code. You can take a look at the example below.

Example:
The node receives the block sent by the peer and then disconnects the peer due to block signature verification failure, but the reason for the disconnection message being sent is UNKNOWN.

10:17:39.014 WARN  [peerWorker-1] [net](P2pEventHandlerImpl.java:258) Message from /172.31.0.220:10850 process failed, type: BLOCK Num:58698013,ID:00000000037fa91d2bbac58ad77bd462a2878178e730eb93a4d3e00e21cbe52e, trx size: 4 type: (15, block sign error), detail: valid signature failed.

10:17:39.014 INFO  [peerWorker-1] [net](PeerConnection.java:175) Send peer /172.31.0.220:10850 message type: P2P_DISCONNECT reason: UNKNOWN

Therefore, after receiving the disconnection message, the peer cannot accurately know the specific reason for being disconnected. I think the reason code carried in the disconnect message in this example can be clearer. In addition, do you think it is necessary to check whether there are other disconnection scenarios with similar problems?

Frequency

It occurs every time in the above scenarios.

Steps to reproduce the behaviour

Send a block with an invalid signature to the node.

Backtrace

[backtrace]

When submitting logs: please submit them as text and not screenshots.

[abn2357]

Except block signature verification failure, is there any other usual peer disconnection scenario?

[jwrct]

@abn2357 There are many scenarios where the connection with the peer will be disconnected, such as: handshake failure, data acquisition timeout, receiving messages sent by the peer that do not conform to the protocol, etc.

[fyyhtx]

Are there any other disconnect scenarios that have an ambiguous reason code?

[jwrct]

I later discovered a scenario where the reason code was unreasonable:
When handling HelloMessage in processHelloMessage function in HandshakeService, a check is made:

if (!msg.valid()) { 
  ...						
 peer.disconnect(ReasonCode.UNEXPECTED_IDENTITY);

When an error occurs, the error code of UNEXPECTED_IDENTITY will be returned, but no identity-related validity check is actually performed.

[xxo1shine]

@jwrct I see that BAD_PROTOCOL is used in many scenarios. When I receive a disconnection reason of BAD_PROTOCOL, how can I know which scenario the peer has triggered? Do you need to solve this problem?

[jwrct]

@xxo1shine Can you specifically list the scenarios where BAD_PROTOCOL would be used? Also, I think you might want to consider whether it's appropriate to define this as a problem, since the original design was to carry the BAD_PROTOCOL reason code for all behaviors that do not comply with the protocol and lead to disconnection.

[fyyhtx]

@jwrct Have there been any new discoveries since the last reply?

[jwrct]

@fyyhtx Apart from the two scenarios mentioned earlier, no new ambiguous reason codes have been found so far.

[317787106]

So, how to make disconnected message more clear? give more detailed reason behind UNKNOWN ?

[jwrct]

@317787106 It is sufficient to use reasonable and accurate reason codes based on the specific scenario, for example, using BAD_BLOCK when the block validation signature fails, and using INCOMPATIBLE_PROTOCOL when the handshake message verification fails.

[xxo1shine]

@jwrct For example, BAD_PROTOCOL is used in both places below.

one.ifPresent(peer -> disconnectFromPeer(peer, ReasonCode.BAD_PROTOCOL,
          DisconnectCause.ISOLATE2_ACTIVE));

one.ifPresent(peer -> disconnectFromPeer(peer, ReasonCode.BAD_PROTOCOL,
          DisconnectCause.ISOLATE2_ACTIVE));

When I receive a disconnect message from node A with reason code BAD_PROTOCOL, I don’t know which logic caused the problem on the other node.

I just want to know whether this issue is going to solve this problem, or whether there are plans to solve this problem in the future, or whether this problem needs to be solved.

[jwrct]

@xxo1shine I understand that you mean to refine the scenarios of BAD_PROTOCOL and then provide more specific reason codes based on different scenarios, right? I think this is worth discussing, and we can ask for more user suggestions.