-
Notifications
You must be signed in to change notification settings - Fork 1k
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
protocols/dcutr/example: Wait for relay to accept reservation request #2642
Conversation
When in listening mode, wait for the relay to accept our reservation request. Only then can a client in dialing mode establish a relayed connection to us via the relay. See also libp2p#2621 (comment)
//CC @Foemass |
@elenaf9 @thomaseizinger can one of you give this a review? |
protocols/dcutr/examples/client.rs
Outdated
})) => { | ||
info!("Relay accepted our reservation request."); | ||
relay_accepted_reservation = true | ||
} | ||
SwarmEvent::Behaviour(Event::Relay(_)) => {} |
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.
Maybe also catch the case of client::Event::ReservationReqFailed
and return an error in that case.
Shouldn't happen in practice but I'd rather have an explicit error than a non-terminating program.
Alternatively, I think there is no valid case in which any other relay event should occur anyway/ So we could remove the match block here and let it fall to the blank panic below.
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.
LGTM
protocols/dcutr/examples/client.rs
Outdated
if learned_observed_addr | ||
&& (matches!(opts.mode, Mode::Dial) || relay_accepted_reservation) |
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.
Would this be easier to understand if we matched explicitly on Mode
and returned true
for dial and relay_accepted_reservation
for listen?
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.
What do you think of 75697c2?
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.
Much better, thank you!
Description
When in listening mode, wait for the relay to accept our reservation
request. Only then can a client in dialing mode establish a relayed
connection to us via the relay.
Links to any relevant issues
Might help with #2621 (comment).
Open Questions
Change checklist