Skip to content

fix(iroh): actually use user-provided bind addrs#3835

Merged
Frando merged 5 commits intomainfrom
Frando/fix-bind-addr
Jan 9, 2026
Merged

fix(iroh): actually use user-provided bind addrs#3835
Frando merged 5 commits intomainfrom
Frando/fix-bind-addr

Conversation

@Frando
Copy link
Copy Markdown
Member

@Frando Frando commented Jan 9, 2026

Description

The new user-defined IP transports added #3692 don't actually work because they are filtered out in Transports::bind.
Here's a fix, and a test that tests the currently implemented behavior. The test fails without this fix.

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, 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:

let bound_sockets = ep.bound_sockets();
assert_eq!(bound_sockets.len(), 1);
assert!(bound_sockets[0].is_ipv4());
assert_eq!(bound_sockets[0].port(), port);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unfortunately that isn't guaranteed I think. If the port is already in-use, we use a fallback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed to port 0 and binding to LOCALHOST instead to test difference to the default unspecified bind.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 9, 2026

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

Last updated: 2026-01-09T09:56:32Z

Comment on lines +2741 to +2743
// test 2: do not clear ip transports and add single non-default IPv4 bind
// this will bind three sockets: wildcard binds for IPv4 and IPv6, and our
// manually-added IPv4 bind.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm personally a fan of separating the tests into individual test fns 👀

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

Just some nits

// are free immediately after the endpoint is dropped.

// test 1: clear ip transports and add single IPv4 bind
let port: u16 = 12345;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd strongly recommend to use 0 for port numbers in the test. I don't think your test cares about ports. And hardcoding ports will be flaky at some point.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed to port 0 and binding to LOCALHOST instead to test difference to the default unspecified bind.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 9, 2026

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: 717b181

@n0bot n0bot bot added this to iroh Jan 9, 2026
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Jan 9, 2026
@Frando Frando requested review from flub and matheus23 January 9, 2026 09:14
@matheus23 matheus23 changed the title fix: actually use user-provided bind addrs fix(iroh): actually use user-provided bind addrs Jan 9, 2026
@Frando Frando enabled auto-merge January 9, 2026 09:38
@Frando Frando added this pull request to the merge queue Jan 9, 2026
Merged via the queue into main with commit fecc909 Jan 9, 2026
24 of 29 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Jan 9, 2026
@flub flub deleted the Frando/fix-bind-addr branch January 13, 2026 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants