refactor(iroh): use combinator approach for address filtering#3987
Merged
refactor(iroh): use combinator approach for address filtering#3987
Conversation
4 tasks
f01c58c to
73a9657
Compare
ramfox
reviewed
Mar 2, 2026
Member
There was a problem hiding this comment.
Yes, I think this is a better way to think about/handle adding a filter.
I had half a thought in my head that said we should allow services to disregard user provided filters if they needed to, but I actually think that's wrong. If the user supplied a filter that filters out all of the addresses that can actually be published to a service, I'd rather assume they did it on purpose and not publish things they explicitly asked to keep private.
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3987/docs/iroh/ Last updated: 2026-03-05T10:31:34Z |
dignifiedquire
approved these changes
Mar 6, 2026
This was referenced Mar 12, 2026
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 16, 2026
…lt (#4012) ## Description Partially reverts #3987 so we have more control over the default `AddrFilter` from within the `PkarrPublisherBuilder`, and because we need it for pkarr, we keep this consistent across publishing services. As a bonus, now pure resolvers don't suddently have a builder method for address filtering that cannot apply to them (but they can still be wrapped in `FilteredAddressLookup`, even though that's less discoverable). Fixes #4009 ## Breaking Changes - **behavioral**: `PkarrPublisher` and `DhtPublisher` will now by default publish *only* the relay URL, instead of detecting if `RelayMode::Disabled` was set and in such cases publishing IP addresses. We recommend users to explicitly set `.addr_mode(AddrFilter::unfiltered())` if they set `relay_mode(RelayMode::Disabled)`, to re-enable publishing IP addresses publicly. --------- Co-authored-by: Franz Heinzmann <frando@unbiskant.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This changes the address filtering API added in #3960: Instead of having each service deal with the filtering, we layer this as a combinator on top.
AddressLookupBuildernow has awith_addr_filtermethod with a default impl, and returns aFilteredBuilderwhich again implsAddressLookupBuilderitself. Through this, this all works transparently as currently, but the individual services don't need to care about the filtering at all.Breaking Changes
Notes & open questions
Are there any downsides? This would save people who impl address lookup services from having to deal with the filtering themselves.
Change checklist
quic-rpciroh-gossipiroh-blobsdumbpipesendme