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鈥檛 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鈥檝e 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.
The approach has been reworked to follow my recommendations from the last review. The code is much easier to follow now. I left a few comments, but nothing is really blocking. Tests look good overall and it is good to have coverage for all the disconnection cases added.
The code was further reworked based on an earlier comment of mine from a previous PR. A lot of the boilerplate is gone now 馃槃
Initial review, I鈥檓 not a big fan of the current approach
Second pass:
ACK: