BreachOfProtocol on network id mismatch

[arnetheduck]

In RLPx, BreachOfProtocol is used for malformed messages (invalid RLP) and other protocol violations.

However, when testing nimbus with nethermind it appears we're receiving it on networkid mismatch, ie when connecting a mainnet node to a holesky nethermind and vice versa. Normally, one would have expected a UselessPeer error.

nethermind/src/Nethermind/Nethermind.Network.Stats/Model/DisconnectReason.cs

Line 98 in 82c78e7

return EthDisconnectReason.BreachOfProtocol;

Is this intentional?

[Marchhill]

@arnetheduck yes this seems to be the case, from the docs you sent there aren't any descriptions of when the different disconnection messages should be used. BreachOfProtocol seems like a reasonable error to throw here, is there a spec that says UselessPeer should be thrown?

[arnetheduck]

Well, the linked spec says about BreachOfProtocol:

Breach of protocol, e.g. a malformed message, bad RLP, ...

I read that as a category of errors relating to structural protocol/encoding issues rather than valid data with mismatching higher-level interpretation. A breach of protcol would effectively be a bug in the implementation or malicious behavior whereas a network id mismatch is a benign issue that happens in a normally operating network with honest peers. Sure, the spec doesn't say this out loud but UselessPeer is what we get from other impls in this case.

[Marchhill]

Well, the linked spec says about BreachOfProtocol:

Breach of protocol, e.g. a malformed message, bad RLP, ...

I read that as a category of errors relating to structural protocol/encoding issues rather than valid data with mismatching higher-level interpretation. A breach of protcol would effectively be a bug in the implementation or malicious behavior whereas a network id mismatch is a benign issue that happens in a normally operating network with honest peers. Sure, the spec doesn't say this out loud but UselessPeer is what we get from other impls in this case.

The chain number could be considered part of the encoding though, if we use version id x on chain y then this message is malformed. UselessPeer sounds more like a networking issue

[arnetheduck]

I mean, it ultimately doesn't matter greatly tbh, except for some penalty time which probably only affects testnets practically - I noted that nethermind sends a different value than other clients in this particular case while looking into why there were so many protocol breaches from nethermind but not from others, to make sure nimbus is indeed not sending actual malformed messages that nethermind can't read. In other words, it's "normal" that we encounter peers on different chains - it would not be normal for nimbus to send malformed messages.

Feel free to close this issue if you're not planning to change it, I don't think it's worth that much.

The chain number could be considered part of the encoding though, if we use version id x

This would be a very unusual protocol, if "well-formed message with unknown version" is treated the same as "malformed message" - the reason to have a version field to begin with is to allow the consuming side to treat it gracefully rather than as a protocol breach. Nitpicking, this is a "well-formed message whose content I cannot interpret" (assuming the RLP is valid and the message follows all other formulaic constraints). Anyway.

[Marchhill]

I mean, it ultimately doesn't matter greatly tbh, except for some penalty time which probably only affects testnets practically - I noted that nethermind sends a different value than other clients in this particular case while looking into why there were so many protocol breaches from nethermind but not from others, to make sure nimbus is indeed not sending actual malformed messages that nethermind can't read. In other words, it's "normal" that we encounter peers on different chains - it would not be normal for nimbus to send malformed messages.

Feel free to close this issue if you're not planning to change it, I don't think it's worth that much.

The chain number could be considered part of the encoding though, if we use version id x

This would be a very unusual protocol, if "well-formed message with unknown version" is treated the same as "malformed message" - the reason to have a version field to begin with is to allow the consuming side to treat it gracefully rather than as a protocol breach. Nitpicking, this is a "well-formed message whose content I cannot interpret" (assuming the RLP is valid and the message follows all other formulaic constraints). Anyway.

It's unclear to me what it should be since the spec is so vague. Do you know if other clients send UselessPeer in this situation?

[arnetheduck]

It's unclear to me what it should be since the spec is so vague. Do you know if other clients send UselessPeer in this situation?

other clients send UselessPeer on network id mismatch, yes.

[Marchhill]

@arnetheduck I implemented a PR for this, could you check if it works? Also how can I reproduce your test?