Skip to content
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

fix!: move dnsResolvers to options #13

Closed
wants to merge 3 commits into from
Closed

Conversation

2color
Copy link
Member

@2color 2color commented Mar 6, 2024

Description

As per ipfs/helia#445 (comment), this PR changes the API of createVerifiedFetch so as to allow passing a custom instance of Helia (for example, with a custom blockstore) in addition to customising the dnsResolvers.

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@2color 2color requested a review from a team as a code owner March 6, 2024 13:11
@achingbrain achingbrain changed the title chore!: move dnsResolvers to options fix!: move dnsResolvers to options Mar 6, 2024
@achingbrain
Copy link
Member

achingbrain commented Mar 6, 2024

N.b. I renamed chore!: to fix!: in the issue title because we want a release from this - chore will not cause one but fix will.

@achingbrain
Copy link
Member

I wonder if we could take this opportunity to make DNS resolvers more configurable in the stack.

Right now the other place we use DNS resolvers is in @multiformats/multiaddr to turn /dns/example.com into /ip4/123.123.123.123. We do this in browsers through hard-coded HTTP-DNS servers which are globally modifiable 🤮

I think we should:

  1. Refactor Multiaddr.resolve to take a list of resolvers or a resolver object*
  2. Use that in preference to the global list if passed
  3. Add it to the libp2p config and pass it/them to ma.resolve during dialing
  4. Add it to the Helia config and make it available as part of the Helia interface
  5. Refactor @helia/ipns to use the DNS resolvers from the Helia interface

* = a resolver object rather than a simple list might give us a bit of flexibility to accommodate things like tld-specific resolvers.

achingbrain added a commit to ipfs/helia that referenced this pull request Mar 12, 2024
Exposes a `.dns` property on the Helia interface for use with other
modules such as @helia/ipns.

Refs: ipfs/helia-verified-fetch#13 (comment)
achingbrain added a commit to ipfs/helia that referenced this pull request Mar 12, 2024
Exposes a `.dns` property on the Helia interface for use with other
modules such as @helia/ipns.

Refs: ipfs/helia-verified-fetch#13 (comment)
achingbrain added a commit to ipfs/helia that referenced this pull request Mar 13, 2024
Exposes a `.dns` property on the Helia interface for use with other
modules such as @helia/ipns.

Refs: ipfs/helia-verified-fetch#13 (comment)
achingbrain added a commit to ipfs/helia that referenced this pull request Mar 14, 2024
Uses the `.dns` property from #465 to resolve DNS `TXT` records.

This allows configuring discrete resolvers for different TLDs, unifies caching across different use of DNS (e.g. dnsaddr multiaddrs), etc.

Refs: ipfs/helia-verified-fetch#13 (comment)
Fixes: #369

BREAKING CHANGE: requires @helia/interface@4.1.x or later, `resolveDns` has been renamed `resolveDNSLink`
@2color
Copy link
Member Author

2color commented Mar 14, 2024

Closing in favour of #18

@2color 2color closed this Mar 14, 2024
@2color 2color deleted the chore/move-dns-opts branch March 14, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants