P2p protocol versioning support #1182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KnownTransactions and handle_result were moved out of peer.rs to reduce code duplication between peers; peer.rs was duplicated into peer_v2.rs, BlockSyncManager chooses one based on the protocol version; NetworkProtocolVersion is now a newtype; SupportedProtocolVersion was introduced; NETWORK_PROTOCOL_CURRENT was renamed to CURRENT_PROTOCOL_VERSION and NetworkProtocolVersion to ProtocolVersion; CategorizedMessage and some test utilities were added (this is an artifact of previous experiments);
… now use a separate TEST_PROTOCOL_VERSION constant; for_each_protocol_version was moved to public test utils.
ImplOfAnImpl
requested review from
muursh,
erubboli,
iljakuklic,
TheQuantumPhysicist and
azarovh
as code owners
…version P2p: handling bad handshake and version
…e the CURRENT_PROTOCOL_VERSION constant from the rest of the code; the constant was renamed to PREFERRED_PROTOCOL_VERSION
TheQuantumPhysicist
approved these changes
Sep 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR supersedes https://git.mintlayer.org/mintlayer/mintlayer-core/-/merge_requests/1129
Original description:
NetworkProtocolVersionwas made a newtype and renamed toProtocolVersion;SupportedProtocolVersionwas added on top of that, which is an enum.peer_v2.rsis currently a copy ofpeer_v1.rs;Some common code was moved to the
peer_commonmodule:KnownTransactionsencapsulate the rolling filter of known transactions;peer_common::handle_resultis identical to the oldPeer::handle_result;All sync manager tests are run for all existing versions of the protocol. To simplify this, I added a helper function
for_each_protocol_version. The idea is that the tests don't need to know what versions are currently available and still be run for all of them. Unfortunately, this doesn't work for tests that expect a panic, so for them I had to list the existing versions explicitly viarstest's#[case]attribute.There also some artefacts of earlier experiments:
CategorizedMessageenum andcategorizemethod of backend'sMessage, which simplify the conversion ofMessageto the front-end message types.cmd_to_peer_man_msgtest function (which usesCategorizedMessageunder the hood) is used in tests in order to avoid directly matching againstMessage(which I was actively changing at the moment).assert_matches/assert_matches_return_valmacros (used by and in conjunction withcmd_to_peer_man_msg)All those changes are not really needed by the current implementation, but they are kind of useful, so I kept them.
P.S. There was no need to disable V2 for old peers, because they don't really mind (they only check that the received protocol version is bigger than the minimum).