This PR adds several functional test cases for disconnection reasons in the v2 connection handshake when this is not properly followed.
I think its good to have this cases, and the tests look thorough, however, I don’t like the current approach. All the logic is condensed in a single Python EncryptedP2PState and how to deviate from the protocol is performed based on a type that is passed to the test caller. This means that in order to see what a test is doing, we need to keep jumping conditionals which makes creating a mental model of what the actual instantiation of the P2P state is for the case fairly complex.
As an example, I’ve noticed that in the WRONG_GARBAGE_TERMINATOR case, we were modifying the sent garbage in two different ways, which seemed unnecessary. Removing one or the other made the test still passes. Turns out removing both, the tests still passes (the peer is disconnected), but that should not be the case, provided the base functionality of the class is to mimic a correct V2 handshake.
Initial review, I’m not a big fan of the current approach