You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
libp2p-kad: provide a way to get known addresses of a peer from address book #3633
Since Kademlia is basically an address book of peers (aka routing table), it would be great to be able to actually read from that address book. Check whether peer is presented in that address book or not.
For example, one might (and we do, see for example our fn discover_peer) use Kademlia to discover Peer address, establish connection, and exchange some messages.
But since addresses_of_peer is deprecated, and Kademlia::kbucket is private, how to read from the address book?
There's handle_pending_outbound_connection which is proposed to replace addresses_of_peer, but it implies an effect, since it signifies that a new connection happened. So it is a bad fit for the goal of "read address from Kademlia address book".
That being said, what are alternatives that you see viable?
Motivation
We need to read addresses from the Kademlia by PeerId, because we use Kademlia as a peers' addresses discovery mechanism.
In addition, I think it makes sense to separate "read model" from effectful methods like handle_pending_outbound_connection.
Current Implementation
We've relied on addresses_of_peer, but it is now removed from Kademlia completely.
Are you planning to do it yourself in a pull request?
Maybe, but need a guidance on the design and alignment with rust-libp2p team's vision.
Let's discuss!
I'm happy to provide more information on our use-case and discuss different design approaches.
Thank you for fast-paced development of rust-libp2p, it is a tremendous effort, and I really appreciate the complexity you are tackling! ❤️
I can agree that there should be some way in going about it since handle_pending_outbound_connection intentions is more related to a pending connection (although not sure if that was also a similar intention to addresses_of_peer. Although for me, I would be able to begin to move away from it, but for you, I would assume using Kademlia::kbuckets or Kademlia::kbucket is not an option? Might add a little more work but you should be able to get the addresses from the peer there.
I read our and rust-libp2p code for a bit, and it seems that we do not depend on addresses being available in queries. That's because we rely on peers being reachable and connected after ClosestQueryIter is finished.
I'll try and see how our tests are feeling, and get back with results!
Thanks for the detailed report. I always considered the NetworkBehaviour::addresses_of_peer an internal method, i.e. a method that is called by libp2p-swarm only. Thus I did not think of the deprecation breaking something downstream. I understand your use-case and want to support it with libp2p-kad. In case the Kademlia::kbucket API is not enough, let's discuss introducing a new method on Kademlia (not on NetworkBehaviour).
Hey, @mxinden I work with @folex
I have run our tests with handle_pending_outbound_connection and pure kbucket fetch. The second option doesn't work for our case. Using handle_pending_outbound_connection looks strange to me because of unused parameters and unclear (name that implies effect) naming.
I suggest adding a method
fn addresses_of_peer(
&mut self,
peer_id: PeerId,
) -> Vec<Multiaddr> {
// We should order addresses from decreasing likelyhood of connectivity, so start with
// the addresses of that peer in the k-buckets.
let key = kbucket::Key::from(peer_id);
let mut peer_addrs =
if let kbucket::Entry::Present(mut entry, _) = self.kbuckets.entry(&key) {
let addrs = entry.value().iter().cloned().collect::<Vec<_>>();
debug_assert!(!addrs.is_empty(), "Empty peer addresses in routing table.");
addrs
} else {
Vec::new()
};
// We add to that a temporary list of addresses from the ongoing queries.
for query in self.queries.iter() {
if let Some(addrs) = query.inner.addresses.get(&peer_id) {
peer_addrs.extend(addrs.iter().cloned())
}
}
peer_addrs
}
into Kademlia impl and reuse it into handle_pending_outbound_connection.
Hey, @mxinden I work with @folex I have run our tests with handle_pending_outbound_connection and pure kbucket fetch. The second option doesn't work for our case. Using handle_pending_outbound_connection looks strange to me because of unused parameters and unclear (name that implies effect) naming. I suggest adding a method
fn addresses_of_peer(
&mut self,
peer_id: PeerId,
) -> Vec<Multiaddr> {
// We should order addresses from decreasing likelyhood of connectivity, so start with
// the addresses of that peer in the k-buckets.
let key = kbucket::Key::from(peer_id);
let mut peer_addrs =
if let kbucket::Entry::Present(mut entry, _) = self.kbuckets.entry(&key) {
let addrs = entry.value().iter().cloned().collect::<Vec<_>>();
debug_assert!(!addrs.is_empty(), "Empty peer addresses in routing table.");
addrs
} else {
Vec::new()
};
// We add to that a temporary list of addresses from the ongoing queries.
for query in self.queries.iter() {
if let Some(addrs) = query.inner.addresses.get(&peer_id) {
peer_addrs.extend(addrs.iter().cloned())
}
}
peer_addrs
}
into Kademlia impl and reuse it into handle_pending_outbound_connection.
I am curious. Is there anything about Kademlia::kbucket that doesnt work for you or are you looking for the addresses from the query iterator too?
I am curious. Is there anything about Kademlia::kbucket that doesnt work for you or are you looking for the addresses from the query iterator too?
It seems that address inside the Query Iterator are crucial: we were not able to do a discovery based on get_closest_peers to make discovered addresses available in Kademlia::kbucket after GetClosest request is finished. They are only available in the iterator at that point in time.
Activity
dariusc93 commentedon Mar 19, 2023
I can agree that there should be some way in going about it since
handle_pending_outbound_connectionintentions is more related to a pending connection (although not sure if that was also a similar intention toaddresses_of_peer. Although for me, I would be able to begin to move away from it, but for you, I would assume usingKademlia::kbucketsorKademlia::kbucketis not an option? Might add a little more work but you should be able to get the addresses from the peer there.folex commentedon Mar 19, 2023
Kademlia::kbucketmight be an answer :)I read our and rust-libp2p code for a bit, and it seems that we do not depend on addresses being available in
queries. That's because we rely on peers being reachable and connected afterClosestQueryIteris finished.I'll try and see how our tests are feeling, and get back with results!
dariusc93 commentedon Mar 22, 2023
Hey @folex were you able to get the results you were looking for?
mxinden commentedon Mar 22, 2023
Thanks for the detailed report. I always considered the
NetworkBehaviour::addresses_of_peeran internal method, i.e. a method that is called bylibp2p-swarmonly. Thus I did not think of the deprecation breaking something downstream. I understand your use-case and want to support it withlibp2p-kad. In case theKademlia::kbucketAPI is not enough, let's discuss introducing a new method onKademlia(not onNetworkBehaviour).gurinderu commentedon Mar 22, 2023
Hey, @mxinden I work with @folex
I have run our tests with handle_pending_outbound_connection and pure kbucket fetch. The second option doesn't work for our case. Using handle_pending_outbound_connection looks strange to me because of unused parameters and unclear (name that implies effect) naming.
I suggest adding a method
into Kademlia impl and reuse it into handle_pending_outbound_connection.
dariusc93 commentedon Mar 22, 2023
I am curious. Is there anything about
Kademlia::kbucketthat doesnt work for you or are you looking for the addresses from the query iterator too?github-actions commentedon Jun 22, 2023
Oops, seems like we needed more information for this issue, please comment with more details or this issue will be closed in 7 days.
folex commentedon Jun 22, 2023
It seems that address inside the Query Iterator are crucial: we were not able to do a discovery based on
get_closest_peersto make discovered addresses available inKademlia::kbucketafterGetClosestrequest is finished. They are only available in the iterator at that point in time.thomaseizinger commentedon Jun 22, 2023
Related: #4101.
github-actions commentedon Jun 30, 2023
Oops, seems like we needed more information for this issue, please comment with more details or this issue will be closed in 7 days.
github-actions commentedon Jul 7, 2023
This issue was closed because it is missing author input.