Base p2p handshake code doesn't like Disconnect as first message across the connection.

[pipermerriam]

What is wrong?

I'm seeing this line show up in logs a lot:

   DEBUG  09-18 13:18:03           ETHPeerPool  Could not complete handshake with <Node(0x0d51ceff21eb6eefd3e11417ea22358f16acec6dbfd97263339a7d4cabbd65bbb664aa2d7782ca5f79f734313e718ddd10c96a91d1b52022c453b3a104230e1c@95.216.227.170:30303)>: HandshakeFailure('First message across the DevP2P connection must be a Hello msg, got Disconnect (cmd_id=1), disconnecting',)

I suspect these are common because when we try to dial a peer that has a full peer pool, they may just immediately send the disconnect message rather than handshaking.

We should handle this gracefully as it happens a lot and it seems reasonable.

How can it be fixed

trinity/p2p/handshake.py

Lines 126 to 130 in 83a2963

	if not isinstance(cmd, Hello):
	raise HandshakeFailure(
	f"First message across the DevP2P connection must be a Hello "
	f"msg, got {cmd}, disconnecting"
	)

THis code should be updated to special case the Disconnect message. The handshake still needs to fail so we could just raise a HandshakeFailure exception with a more appropriate message.

[gsalgado]

I was going through the open issues and this one caught my attention because I remember investigating something like this in the past, but it turns out it was a similar issue during the auth handshake: ethereum/py-evm#901

Anyway, I've confirmed that attempting to connect to a (geth) node with maxed out peers is the cause for that HandshakeFailure, but as it is a generic handshake failure, the peer connection tracker will retry that peer after every 10 seconds. I believe the correct solution is to raise a TooManyPeers (which is a subclass of HandshakeFailure) as that will cause the connection tracker to retry only after 60 seconds and the log to clearly state the peer disconnected because it is already full