-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose API for PublicAddresses
#212
Merged
Merged
Changes from 2 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
8502645
identify: Expose API to inject listen addresses
lexnv e452745
tests: Check identify public addresses are propagated
lexnv 0e42027
listen-addr: Add listen address struct to a separate module
lexnv 890df79
listen-addr/tests: Check listen addr interface functionality
lexnv 59668c9
listen-addr: Use a lock guard for custom iteration over the addresses
lexnv 56ed02a
litep2p: Store shared listenAddresses on Litep2p object
lexnv a4bbf06
identify: Use the new interface of public addresses
lexnv f2a1c51
listen-addr: Check listen address contains p2p protocol
lexnv 67b617d
Polish up the API
lexnv 38d8ca3
listen-addr: Register lsiten addresses with adding the local peerID
lexnv 2a45422
Adjust testing to the new interface
lexnv fc0c700
Use ListenAddresses everywhere
lexnv 7f07e9d
listen-addr: Add better documentation
lexnv 794eba5
listen-addr: Add contains and remove partial methods
lexnv 5e9930a
Merge remote-tracking branch 'origin/master' into lexnv/indentify-con…
lexnv e11a996
Rename to ExternalAddresses for clarity
lexnv 09b6fee
Refactor and adjust testing
lexnv c11f468
Merge remote-tracking branch 'origin/master' into lexnv/indentify-con…
lexnv 68c76d2
Apply fmt
lexnv 6161233
Fix cargo doc and clippy
lexnv dacd665
pub-addr: Rename API methods
lexnv 1194a29
Introduce ListenAddresses object
lexnv ba6e198
Use listenAddresses added interface
lexnv e7a7d80
identify: Use user-provided, listen and public addresses
lexnv 9ad762b
manager: Remove transport_manager::register_listen_address
lexnv d0ebcd2
Adjust testing
lexnv 41c1e23
Fix documentation
lexnv fd23d15
address: Introduce insertion error for better reporting
lexnv be8d663
Remove ListenAddresses
lexnv af8fe60
Adjust testing
lexnv 39153f1
Adjust testing
lexnv 6b7b7b1
identify: Remove public addr from list config
lexnv a633b68
Merge branch 'master' into lexnv/indentify-confirmed-addresses
lexnv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could return
IdentifyHandle
which would holdIdentifyPublicAddresses
and would implementStream<Item = IdentifyEvent>
, following the design of other protocols.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that idea, will certainly implement it! Thanks for the review!
Have put this on pending shortly, I have a hunch we'll need to expose an
ExternalAddrConfirmed
(like libp2p) to suffice other usecases (#211). Will wait for Dmitry to confirm that these are needed for the bootnodes on DHT🙏There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to have not responded here. We discussed #211 with @lexnv, and my idea was to introduce a globally accessible list of public addresses in litep2p. If some protocol needs external addresses, it can fetch them. This includes
Identify
andKademlia
protocols now.As a first step, we can manually notify
litep2p
about confirmed external addresses from the client code. As a second step, we can move the heuristic of confirming external addresses seen by three or more peers frompolkadot-sdk
tolitep2p
. I have updated #211 to be more clear about this.I don't think
ExternalAddrConfirmed
event will help with providing up-to-date peer addresses for local content providers in Kademlia from withinlitep2p
, but the option to get addresses from the global list will.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your plan sounds good. The easiest place to store the external addresses is probably
TransportManager
, partly since there already exists code for storing known addresses viaTransportManagerHandle
and since every protocol has a handle toTransportManager
, it should require little architectural changes.