This repository has been archived by the owner on Oct 23, 2022. It is now read-only.
Swarm cleanup following libp2p upgrade to v0.39.1 #473
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
…0.39 Get rid of eq_greedy.
…erId
Enforce internal represenatation for `SwarmApi::pending_{addresses, connections}` to explicitly show that all multiaddrs include peer IDs by using `MultiaddrWithPeerId`.
koivunej
approved these changes
Aug 23, 2021
There was a problem hiding this comment.
Looking good! I think there's further simplification possible by using direct dials with a .../p2p/PeerId multiaddr, but this is good enough for now.
Windows test failure is again ... the transient one. Should really look into what is happening there.
bors r+
👍 I think it is even worth exploring including the change above (and thus enforcing more than just multiaddr syntax at the type level) in https://github.com/multiformats/rust-multiaddr directly. |
|
Build succeeded: |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 is a follow up of #472. There are two changes introduced:
MultiaddrinSwarmApi::pending_{addresses, connections}contains p2p, align conversions accordingly (136496f),MultiaddrWithPeerIdinSwarmApi::pending_{addresses, connections}(3b6f695).The former (136496f) works fine without the latter (3b6f695), however I am leaning more towards keeping both changes. Encoding the information about peer id presence/absence in Multiaddr in type instead of relying on comments and
expect()s improves readability and developer experience.Please let me know what your thoughts are.