Delay the promotion of the peer from connecting to connected #3418
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
Signed-off-by: ljedrz <[email protected]>
Signed-off-by: ljedrz <[email protected]> Co-authored-by: Niklas <[email protected]>
alzger
requested review from
evanmarshall,
lukenewman,
asharma13524,
fulltimemike,
zkxuerb,
zklimaleo and
a team
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.
Motivation
Replaces #3332
Original message:
A re-opening of #2690; there is also #2729 which I had considered to be a simpler alternative, but I believe that both changes are desirable (with this one going in first) for a more foolproof setup.
The Aleo network stack is two-tiered, and both the lower- and higher-level layers recognize that connections may have been started but not fully finalized yet, which corresponds to the "connecting" and "connected" states of connections and peers respectively.
This PR ensures that the lower networking layer is the first one to conclude its connection setup, which is done by marking the (higher-level) peers as "connected" only via OnConnect, which is triggered only after the lower-level layer has concluded all its work. To do this, the Peer object (instead of just the address) is persisted in the connecting_peers collection past the handshake, and moved to connected_peers only during OnConnect::on_connect.
Test Plan
The existing tests are adjusted to account for this; I've also tested it locally in a --dev network.
Fixes #3331 and all other potential issues of its kind (i.e. attempting to treat a connection as fully established before the lower-level TCP stack recognizes it as such).