feat(relay): implement autorelay - #6156
Conversation
…and get pending amount
…on after an error or failed reservation
| return; | ||
| } | ||
|
|
||
| if let Some((peer_id, connection_id)) = self.reservations.get(&listener_id).copied() |
There was a problem hiding this comment.
I wonder if we should also store any new reservation that been established outside of auto relay. Thoughts?
There was a problem hiding this comment.
do you use auto relay with relay in parallel? What would be the advantage?
There was a problem hiding this comment.
Yes, it should run along side the relay client and not built in, keeping logic separate (and also following the original idea of having a separate module).
jxs
left a comment
jxs
left a comment
There was a problem hiding this comment.
Thanks for this Darius, overall looks good to me! Left some comments
| } | ||
| } | ||
|
|
||
| fn is_peer_idle(&self, peer_id: &PeerId) -> bool { |
There was a problem hiding this comment.
for this and subsequent methods that take the PeerId as input parameter, return Option<> on the return signatures for when the peer doesn't exist in the connections?
| .any(|((pid, _), info)| pid == peer_id && !info.address.is_relayed()) | ||
| } | ||
|
|
||
| fn evict_for_static_peer(&mut self, new_static: PeerId) { |
There was a problem hiding this comment.
return a bool for then the eviction was successful?
There was a problem hiding this comment.
Not that im against it, but would it make sense to return a bool if the callers dont check its return? We could definitely use some logging though.
There was a problem hiding this comment.
yeah indeed, let's go for logging then. Thanks Darius
| use libp2p_core::{Multiaddr, multiaddr::Protocol}; | ||
| use libp2p_identity::PeerId; | ||
|
|
||
| pub(crate) trait MultiaddrExt { |
There was a problem hiding this comment.
since this trait is only implemented on Multiaddr there's no need for polymorphism, why not a function that takes Multiaddr?
There was a problem hiding this comment.
For the additional member of the trait, i was just reusing the trait there and expanding on it (since it does make sense to a degree), but I can move it to a free function instead that takes Multiaddr, but for the other trait member, that might be more suited for a separate PR (just to keep things consistent and reduce touching additional parts of the code).
| return; | ||
| } | ||
|
|
||
| if let Some((peer_id, connection_id)) = self.reservations.get(&listener_id).copied() |
There was a problem hiding this comment.
do you use auto relay with relay in parallel? What would be the advantage?
2 participants
Description
This PR implements a basic autorelay system that would automatically make reservation as soon as it is discovered that the connected node supports HOP protocol.
resolves #4651.
Notes & open questions
Currently, this PR is a PoC, but ideally would be the initial foundation for autorelay implementation.Currently, we will only support making reservation as we discover them, but ideally, we should implement events to notify swarm about no relays available, which would allow the node to perform its own discovery of relays (ie walking DHT, etc), as well as supporting static relays that would have priority or be used in the event of no relays being discovered, although it is all up for discussion to determine what is needed vs wants.Change checklist