Skip to content

Conversation

dignifiedquire
Copy link
Contributor

@dignifiedquire dignifiedquire commented Oct 16, 2025

Description

This marks the first public step towards generic transports. For now there are two types of TransportAddrs, Relay and Ip which, replace the previous explicit fields in EndpointAddr of relay_url and direct_addrs.

Closes #3417

Breaking Changes

  • direct_addresses are now called ip_addresses
  • EndpointAddr type is completely changed
  • APIs with addresse(s) are now normalized to addr(s) to be consistent in the code base

@dignifiedquire dignifiedquire marked this pull request as draft October 16, 2025 20:21
@n0bot n0bot bot added this to iroh Oct 16, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Oct 16, 2025
Copy link

github-actions bot commented Oct 20, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3554/docs/iroh/

Last updated: 2025-10-20T13:47:08Z

@dignifiedquire dignifiedquire changed the title [WIP] feat!: transport generic EndpointAddr feat!: transport generic EndpointAddr Oct 20, 2025
@dignifiedquire dignifiedquire marked this pull request as ready for review October 20, 2025 08:49
Copy link

github-actions bot commented Oct 20, 2025

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: a14f5ef

Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM.

I'd like to treat the doc comments of EndpointAddr and TransportAddr and their fields as if the user encounters all those concepts for the first time. Like explain that endpoint_id is unique to this node. addr is a list of transient addresses, they may change, or may never have worked. Discovery is a thing. TransprotsAddr::Relay should give a short intro to relays, TransportsAddr::Ip explain that they may need holepuching and all such things.

impl EndpointAddr {
/// Creates a new [`EndpointAddr`] with no `relay_url` and no `direct_addresses`.
pub fn new(endpoint_id: PublicKey) -> Self {
/// Creates a new [`EndpointAddr`] with no addresses.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be verbose! Explain that you can still use this using discovery. Users will read this without any context and never have heard of iroh. They'll have skipped all the nice intro docs we have (we do have those, right?)

#[derive(Serialize, Deserialize)]
enum TicketWireFormat {
Variant0(Variant0NodeTicket),
Variant0(Variant1EndpointTicket),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bother adding Variant1 if you then break Variant0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to rename them all to variant1 as this is the variant for 1.0? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to be called Variant1

/// [`Watcher`]: n0_watcher::Watcher
/// [`Watcher::initialized`]: n0_watcher::Watcher::initialized
pub(crate) fn direct_addresses(&self) -> n0_watcher::Direct<BTreeSet<DirectAddr>> {
pub(crate) fn ip_addresses(&self) -> n0_watcher::Direct<BTreeSet<DirectAddr>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a curious and tricky rename. The type still says DirectAddr. But it's also an internal API so whatever, not important enough to hold this up. We can figure out this naming iteratively.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah..that's where I ended up with as well, wasn't sure how much to change the types, or leave it be for now

@dignifiedquire
Copy link
Contributor Author

minimal versions failing because of rustls/rustls-platform-verifier#164

Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking forward to use this on the multipath branch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

feat: Future-proof NodeAddr

3 participants