Skip to content

feat(iroh): allow propagation of addr filters through the endpoint#4010

Merged
dignifiedquire merged 3 commits intomainfrom
fix-ip-publish
Mar 12, 2026
Merged

feat(iroh): allow propagation of addr filters through the endpoint#4010
dignifiedquire merged 3 commits intomainfrom
fix-ip-publish

Conversation

@dignifiedquire
Copy link
Copy Markdown
Contributor

@dignifiedquire dignifiedquire commented Mar 11, 2026

This allows propagating the filters to all added services, ensuring they are actually applied.

The issue in #4009 happened because the preset filter wasn't applied.

Fixes #4009

@dignifiedquire dignifiedquire requested a review from Frando March 11, 2026 16:10
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 11, 2026

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

Last updated: 2026-03-11T16:56:38Z

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 11, 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: b7ddb86

Comment on lines +469 to +473
if self.no_relay {
builder = builder.addr_filter(AddrFilter::ip_only());
} else {
builder = builder.addr_filter(AddrFilter::relay_only());
}
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 don't think we want to decide this on the endpoint level. E.g. mdns is likely to want to publish IP addrs, even when relays are enabled, while pkarr doesn't want to publish ip addrs.

IMO we should have PkarrPublisherBuilder have an addr filter on it by default (maybe implemented via the AddrFilter wrapper), and have that be overridable via a PkarrPublisherBuilder::addr_filter function.

And the same for MdnsLookup.

The only problem I don't really know how to solve is that the pkarr publisher needs to know if relays are enabled before it can decide whether to enable IP publishing or not. Perhaps we can delay the choice of default addr filter until the pkarr publisher is built?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think not doing it at the endpoint level optionally is a massive footgun, literally the authors of this new mechanism didn't see the issue in updating these examples

@n0bot n0bot bot added this to iroh Mar 11, 2026
@github-project-automation github-project-automation bot moved this to 🚑 Needs Triage in iroh Mar 11, 2026
@rklaehn
Copy link
Copy Markdown
Contributor

rklaehn commented Mar 11, 2026

Just looked at the mdns example.

It adds the mdns address lookup to the existing endpoint, after the builder is done.

ep.address_lookup()?.add(mdns.clone());

So on the plus side, it still works. On the minus side, it is kind of confusing that for adding address lookups to an existing endpoint the filter does not apply...

@dignifiedquire
Copy link
Copy Markdown
Contributor Author

So on the plus side, it still works. On the minus side, it is kind of confusing that for adding address lookups to an existing endpoint the filter does not apply...

it should still apply, because this inserts it into the concurrent setup, which now always filters

@rklaehn
Copy link
Copy Markdown
Contributor

rklaehn commented Mar 11, 2026

So on the plus side, it still works. On the minus side, it is kind of confusing that for adding address lookups to an existing endpoint the filter does not apply...

it should still apply, because this inserts it into the concurrent setup, which now always filters

Ah, I thought you meant apply during building of the concurrent address lookup, but you actually integrated it into the concurrent lookup. THen it's fine.

Copy link
Copy Markdown
Contributor

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

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

All lookup services that rely on ip addrs being available need a comment that the n0 presets set relay only.

Other than that it's ready to go as far as I am concerned.

matheus23 pushed a commit that referenced this pull request Mar 12, 2026
## Description

Based on #4012 which is based on #4010

## 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)
@dignifiedquire
Copy link
Copy Markdown
Contributor Author

Merging, but we will have more follow up work as indicated in the referenced PRs

@dignifiedquire dignifiedquire changed the title fix(iroh): allow propagation of addr filters through the endpoint feat(iroh): allow propagation of addr filters through the endpoint Mar 12, 2026
@dignifiedquire dignifiedquire added this pull request to the merge queue Mar 12, 2026
Merged via the queue into main with commit 5df183d Mar 12, 2026
30 checks passed
@github-project-automation github-project-automation bot moved this from 🚑 Needs Triage to ✅ Done in iroh Mar 12, 2026
@matheus23 matheus23 deleted the fix-ip-publish branch March 12, 2026 11:50
matheus23 pushed a commit that referenced this pull request Mar 12, 2026
## Description

Based on #4012 which is based on #4010

## 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)
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.

Accidental publishing of IP addresses in DNS

3 participants