Refactor libp2p code #154
Merged
Refactor libp2p code #154
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
altonen
force-pushed
the
libp2p-refactor
branch
3 times, most recently
from
ef6f37d
to
2180615
Compare
|
This PR is ready for review. Mock interface and |
iljakuklic
approved these changes
May 5, 2022
altonen
force-pushed
the
libp2p-refactor
branch
3 times, most recently
from
76c5540
to
132a84a
Compare
muursh
approved these changes
May 11, 2022
altonen
force-pushed
the
libp2p-refactor
branch
2 times, most recently
from
bb12802
to
81f5773
Compare
TheQuantumPhysicist
approved these changes
May 16, 2022
Rename common.rs to types.rs to follow naming convention with the mock interface.
This commit does not implement the now-required message validation API for the mock interface.
Peer object was used to implement multiplexing between SwarmManager and SyncManager, to handle handshaking and ping/pong connectivity checks. Now that responsibility of providing that functionality is given to the network service provider, the peer object doesn't have a use in its current form.
From now on it's required that multiplexing is provided by the network service provider and that the users of the protocol can issue generic request-response type messages.
Now that handshaking is part of the connection establishment and that the front-end gets only handshaked connections, peer don't need to be registered to the backend anymore.
This service is used instead the old SocketService + Peer combo by allowing the network service provider to do the socket multiplexing as it sees fit. It simplifies the design on the upper-level and allows Mintlayer to better utilize the tools libp2p provides.
- configure Ping and Identify protocols - move protocol handlers to separate files - creat/interpret protocol string correctly Unit tests for the protocol handlers are missing
Inbound connections also carry the address of the remote peer which was previously discarded in the backend. While there, rename the events more logically.
- rename Strategy to DiscoveryStrategy - do not use hard-coded ports in tests - rename addr to bind_addr - rename config to chain_config - format source code - gate `create_custom()` behind "testing" attribute
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.
Highlights:
libp2p::floodsubwithlibp2p::gossipsublibp2p::streaming(our fork) and the peer objectlibp2p::pingfor keep-alivelibp2p::identifyfor exchanging peer informationSocketServiceand introduce a new request/response-basedSyncingServiceI'm leaning towards the viewpoint that
SwarmManagershould ultimately be the one that decided whether inbound/outbound peer is kept in our swarm and not thelibp2p::identifyhandler. The reason for that is because the manager must anyway check the incoming peer information against the info it has (unless we start adding panics to the code) so it would be cleaner if that check was done in one place. I'll experiment with tests to see which is nicer (duplicate check or onlySwarmManagerchecking) and decide based on that. Let me know what you think about it.For now this is a draft as I still need to write more tests for the libp2p implementation and sync the mock interface with these new changes. I don't expect the tests to reveal a lot of need for changes so you're free to start reviewing if you feel like it.