-
Notifications
You must be signed in to change notification settings - Fork 315
feat: relay only configuration #3651
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
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3651/docs/iroh/ Last updated: 2025-11-17T10:34:52Z |
| impl From<RelayMode> for Option<TransportConfig> { | ||
| fn from(mode: RelayMode) -> Self { | ||
| match mode { | ||
| RelayMode::Disabled => None, |
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 is a change in behavior, isn't? Currently RelayMode::Disabled means the endpoint has no home relay, but it will still dial the home relays of other peers if you get addrs with relay urls. Now, it would mean the relay transport is entirely disabled, right?
But it makes sense, and the previous variant can still be done by setting RelayMode::Custom with an empty relay map I guess? Should be clearly documented though.
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.
yeah, I think this is the more "correct" interpretation of Disabled
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.
added some docs
d7d3df2 to
6fe4d5b
Compare
ramfox
left a comment
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.
looks good to me! two nits
Only question mark was around disable_ip and disable_relay methods. I think we are good to have them, especially since we have the expectation of only ip or relay in our other APIs. But curious what this will look like as we add transports or allow others to put in custom transports.
LGTM
| #[derive(Default, Debug, Clone)] | ||
| pub struct ConnectOptions { | ||
| transport_config: Option<Arc<TransportConfig>>, | ||
| transport_config: Option<Arc<quinn::TransportConfig>>, |
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.
should we be wrapping this foreign type?
|
oh also, lot's of |
6fe4d5b to
c82c149
Compare
| pub const DEFAULT_FAKE_ADDR: SocketAddrV6 = SocketAddrV6::new( | ||
| Ipv6Addr::new( | ||
| u16::from_be_bytes([ADDR_PREFIXL, 21]), | ||
| u16::from_be_bytes([7, 10]), |
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.
Is there any meaning to these numbers? In either case, maybe a short comment on how they're chosen would be nice.
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.
these are the same numbers we use for the other fake addrs
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.
added a comment
c82c149 to
effd5e3
Compare
## Description Remove the test-only `Endpoint::path_selection` API and instead use `Endpoint::clear_ip_transports` for `PathSelection::RelayOnly `, now that this public API was added in #3651. ## Breaking Changes <!-- Optional, if there are any breaking changes document them, including how to migrate older code. --> ## Notes & open questions <!-- Any notes, remarks or open questions you have to make about the PR. --> ## Change checklist <!-- Remove any that are not relevant. --> - [ ] Self-review. - [ ] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [ ] Tests if relevant. - [ ] All breaking changes documented. - [ ] List all breaking changes in the above "Breaking Changes" section. - [ ] Open an issue or PR on any number0 repos that are affected by this breaking change. Give guidance on how the updates should be handled or do the actual updates themselves. The major ones are: - [ ] [`quic-rpc`](https://github.com/n0-computer/quic-rpc) - [ ] [`iroh-gossip`](https://github.com/n0-computer/iroh-gossip) - [ ] [`iroh-blobs`](https://github.com/n0-computer/iroh-blobs) - [ ] [`dumbpipe`](https://github.com/n0-computer/dumbpipe) - [ ] [`sendme`](https://github.com/n0-computer/sendme)
This is a next step into the world of configurable transports. We now allow disabling the IP based transports entirely.
Internally this starts to prepare for a world where the user can configure multiple different transports, IP, relay and others in the future.
Closes #2957