feat: add IPNS and DNSLink resolution support #114
Conversation
- resolve IPNS names and DNSLinks to CIDs - detect and handle PeerIDs (CIDv1 and legacy formats) - use delegated-ipfs.dev DoH endpoint for consistent DNS resolution - disable caching for fresh diagnostic results - show resolution info in UI with diagnostic links
lidel
force-pushed
the
feat/ipns-dnslink-resolution
branch
from
f6a7978 to
4c2622d
Compare
| // Try to decode as legacy PeerID (base58btc multihash) | ||
| if _, err := peer.Decode(input); err == nil { |
There was a problem hiding this comment.
Isn't there an awkward case where you can get a base58btc sha2-256-256 multihash and not know if it's meant as a CIDv0 or an IPNS ID for an RSA key?
Maybe we just ignore this, because getting a hold of a non-ed25519 key that's also not using CID encoding is much less common now than it used to be.
There was a problem hiding this comment.
ipfs-check always treats ambiguous Qm... strings (with no namespace prefix) as CIDv0 content, never as IPNS IDs.
Theoretical overlap is:
- CIDv0: base58btc-encoded sha2-256 multihash (starts with Qm...)
- RSA Peer IDs (in theory could be used for IPNS): base58btc-encoded sha2-256 hash of the public key (also starts with Qm...)
The way code is in this PR, and various other places is to prioritize based on context. Here, we prioritize CIDs, as it is the main expected input for checker.
cid.Decode()will succeed (CIDv0 is valid)- The code returns it as content immediately
peer.Decode()is never reached
So ipfs-check always treats ambiguous Qm... strings as CIDv0 content. If one wants to interpret it as IPNS Name, they can prefix it with /ipns/. In practice this edge case wont occur in real world, subdomains gateways normalize these RSA legacies to use CIDv1 with libp2p-key, and error pages on gateways will use CIDv1.
IMO this is a reasonable default (prefer immutable content over mutable pointers).
main.go
Outdated
| // Try multihash decode as fallback | ||
| if c, ok := tryDecodeMultihash(input); ok { |
There was a problem hiding this comment.
Does this do anything? Won't this have already been decoded as a CID (v0) earlier?
| } | ||
|
|
||
| // Attempt IPNS/DNSLink resolution | ||
| result, err := ns.Resolve(ctx, p) |
There was a problem hiding this comment.
Perhaps fine for now, but this does full resolution rather than step-wise which could be confusing (e.g. for people who do DNSLink -> IPNS -> IPFS mappings). Should probably have the UI text indicate this is fully resolved rather than partial (again maybe fine, similar to how following CNAMEs, HTTP redirects, etc. is implied in lots of tools)
There was a problem hiding this comment.
Yes, future work. We can improve resolution details here, but if we start doing that, there are way more important places to improve visibility first (e.g. HTTP retrieval failures: headers, content types, status codes, and hash check results are hidden atm, and that is hot path vs niche use case here).
| // that should always query current DNS state rather than serve potentially stale cached results. | ||
| dnsResolver, err := gateway.NewDNSResolver( | ||
| map[string]string{ | ||
| ".": "https://delegated-ipfs.dev/dns-query", |
There was a problem hiding this comment.
Seems fine for now, but a little awkward given how for whatever reason there's an IPNI endpoint configurable.
| }}, nil | ||
| } | ||
|
|
||
| type MutableResolution struct { |
There was a problem hiding this comment.
We can kick the can down the road here, but I wonder if adding some explicit versioning to the backend would help us here with upgrades and not feeling bad about shipping something rough and needs to be improved later but that's net an improvement.
|
Thanks for pushing this along, looks pretty good 🙏 |
all valid inputs are already handled by cid.Decode() and peer.Decode()
- add tests for buildDiagnosticURL, resolveInput, resolveMutablePath - cover all 3 PeerID formats from libp2p spec (CIDv1, legacy Qm, Ed25519) - fix bug: PeerID v1 now correctly sets IsMutableInput=true
delegated-ipfs.devDoH endpoint for consistent DNS resolution and ENS support (match Helia ecosystem in browsers)dnslink.devoripns.ipfs.networkWhy
We either already show or want to show "Check CID Reviewability" button on gateway errors for
/ipns/namespace (IPNS Records or DNSLinks).Having built-in support for mutable pointers and showing Resolution step at the top builds proper mental model for developers and users who want to understand what went wrong.
Demos
|