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

Allow resolution of .eth names via .eth.link #6448

Merged
merged 2 commits into from
Aug 5, 2019
Merged

Conversation

mcdee
Copy link
Contributor

@mcdee mcdee commented Jun 13, 2019

As per https://github.com/protocol/collab-ens/issues/5 this PR handles the situation where IPNS names ending in .eth are unresolvable due to .eth not being a recognised TLD.

It does so by changing such names to end in .eth.link, which will be resolvable by the EthDNS nameserver to obtain data from the relevant ENS records on-chain in Ethereum.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

This PR would add support for ENS at /ipns/example.eth, however now I am having some concerns if patching /ipns/{fqdn} is worth merging given potential problems and known limitations:

What if we use /ens instead of /ipns?

  • Do not change /ipns/
    • /ipns/example.eth.link will still work thanks to regular DNSLink
  • Add /ens namespace as a copy of /ipns that does all DNSLink lookups via EthDNS
  • Support self-hosted resolvers out of the box
    • Let user choose custom resolver in config under Ens.Nameservers
    • If Ens.Nameservers is empty or missing, default to ones provided by ENS project

This is aligned with existing plan to use EthDNS for ENS lookups
and comes with pretty cool set of additional features:

  • Removes the problem with Ethiopia country code
  • Enables resolution of all TLDs imported to ENS via DNSSEC 🚀 ✨
  • Enables true decetralization: people could use custom EthDNS resolver (running on localhost or LAN)

Thoughts?
@parkan @Stebalien @ipfs/wg-web-browsers ipfs/notes#348 ipfs/notes#286 ipfs/notes#302 (comment) ipfs/specs#198

@mcdee
Copy link
Contributor Author

mcdee commented Jun 14, 2019

  • What if we use /ens instead of /ipns

Personally I think the idea of adding the resolution protocol to the path is overly proscriptive. It would also, in my mind, slow the transition to using ENS.

If paths are out there that already use /ipns/my.domain.com, for example, and my.domain.com moves to ENS, resolution will continue over DNS (successfully) even though ENS would be the primary source of the data and a preferable path to resolution.

There is an argument this could cause problems if DNS and ENS diverge, but this can be resolved by explicitly defining primacy (e.g. records in ENS will always override those in DNS).

@lidel
Copy link
Member

lidel commented Jun 14, 2019

@mcdee I agree that /ens discussion should not block us from enabling /ipns/example.eth as soon as possible to unlock use of .eth on gateways without Nginx hacks.

I also like the idea of overriding DNS servers via config of go-ipfs (we should support both array of hostnames at ipfs config Dns.NameServers or DoH endpoint at Dns.ResolverUrl). That would enable people to switch their Gateways to ENS for all TLDs if they choose to do so, while maintaining compatibility within existing convention. Should be a separate PR tho.

Sidenote:

Note that DNSLink is already confusing: /ipns/{fqdn} has no relation to actual IPNS (which operates on /ipns/{libp2p-key}), it is actually something like /dns/{fqdn} (ipfs/notes#348). It is a technical/ux debt we did not deal with yet. We are revisiting "what should be a separate namespace and why" in ipfs/specs#198

@Stebalien would it be acceptable to merge this PR as a temporary measure?

Rationale: .eth does not exist in public DNS, so it is safe to do this for now. If .eth becomes a real TLD in the future, then we will add configuration option to disable this rewrite, or move to /ens by then.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM. This seems like a minimal base that's:

  1. Trivial to maintain.
  2. Unlike to conflict with a future change.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Actually, this should have a test. @mcdee, could you add a test to namesys/dns_test.go (just verify that we rewrite .eth domains to .eth.link)?

@Stebalien
Copy link
Member

Longer comment:

It may cause problem in the future if .eth becomes a real TLD owned by Ethiopia.

.et is the TLD for Ethiopia. Has anyone submitted a RFC with the IETF for .ETH?

Prior discussions we had ended with consensus that resolvers other than "real" DNS should get own namespaces (cc @Stebalien mentioning ENS in ipfs/notes#302 (comment))

I agree that the DNS namespace shouldn't necessarily be tied to the DNS protocol.

There is an argument this could cause problems if DNS and ENS diverge, but this can be resolved by explicitly defining primacy (e.g. records in ENS will always override those in DNS).

We can make that decision when we get there.

@aschmahmann
Copy link
Contributor

Wanted to surface here the comment by @Stebalien at multiformats/multicodec#136 (review) regarding how the IPNS multicodec should not be used for things other than IPNS.

In general, people should also not expect that the syntax /ipns/something-other-than-an-IPNS-key will be supported indefinitely, it's super confusing.

@Stebalien
Copy link
Member

In general, people should also not expect that the syntax /ipns/something-other-than-an-IPNS-key will be supported indefinitely, it's super confusing.

Note: We're going to support that indefinitely as we don't break links. However, there's no reason to introduce the same confusion when encoding them into a compact binary format.

@mcdee
Copy link
Contributor Author

mcdee commented Jul 4, 2019

@Stebalien added suitable tests.

Regarding the .eth domain: ICANN have put all of the 3-character ccTLDs in limbo for the moment, with the idea that countries may want them one day (ignoring the fact that they all have 2-character ccTLDs and some of them are already used, but hey) so .eth is unavailable to us.

@Stebalien
Copy link
Member

Regarding the .eth domain: ICANN have put all of the 3-character ccTLDs in limbo for the moment, with the idea that countries may want them one day (ignoring the fact that they all have 2-character ccTLDs and some of them are already used, but hey) so .eth is unavailable to us.

Have you talked to the IETF? That's how .onion did it.

@Stebalien
Copy link
Member

@aschmahmann would you like to continue our discussion here?

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Ok, we had various discussions since then, and given the surgical scope of this PR and the fact that:

Regarding the .eth domain: ICANN have put all of the 3-character ccTLDs in limbo for the moment, with the idea that countries may want them one day (ignoring the fact that they all have 2-character ccTLDs and some of them are already used, but hey) so .eth is unavailable to us.

And:

.et is the country code top-level domain (ccTLD) for Ethiopia.

@Stebalien I feel its ok to merge this.

tl;dr this PR unlocks use of /ipns/ipfs.enstest.eth and custom code can be removed if .eth becomes real TLD st some point in the future.

@Stebalien
Copy link
Member

@lidel thanks for writing up the decision.

@Stebalien Stebalien merged commit 5f652ca into ipfs:master Aug 5, 2019
@lidel
Copy link
Member

lidel commented Aug 14, 2019

@Stebalien just for the record: will this be released in v0.5.0, correct?

v0.4.22:

$ curl http://127.0.0.1:8080/ipns/ipfs.enstest.eth/
ipfs resolve -r /ipns/ipfs.enstest.eth/: not a valid proquint string

$ ipfs name resolve /ipns/ipfs.enstest.eth
Error: not a valid proquint string

master is 👌

$ ipfs name resolve /ipns/ipfs.enstest.eth.link
/ipfs/QmRAQB6YaCyidP37UdDnjFY5vQuiBrcqdyoW1CuDgwxkD4

$ ipfs name resolve /ipns/ipfs.enstest.eth
/ipfs/QmRAQB6YaCyidP37UdDnjFY5vQuiBrcqdyoW1CuDgwxkD4

@Stebalien
Copy link
Member

Yes.

lidel added a commit to ipfs/js-ipfs that referenced this pull request Aug 20, 2019
.eth TLD is not recognised by mainstream DNS,
however ENS provides DNS compatibility service at .eth.link

This change enables resolution of ENS names ending with .eth
on systems that  have regular DNS set up

More context at the go-ipfs counterpart:
ipfs/kubo#6448

License: MIT
Signed-off-by: Marcin Rataj <[email protected]>
alanshaw pushed a commit to ipfs/js-ipfs that referenced this pull request Aug 21, 2019
.eth TLD is not recognised by mainstream DNS,
however ENS provides DNS compatibility service at .eth.link

This change enables resolution of ENS names ending with .eth
on systems that  have regular DNS set up

More context at the go-ipfs counterpart:
ipfs/kubo#6448

License: MIT
Signed-off-by: Marcin Rataj <[email protected]>
@parkan
Copy link

parkan commented Oct 7, 2019

this doesn’t seem to be the issue

hmm am I right in thinking that the sync Resolve code path here doesn’t hit this change?

https://github.com/ipfs/go-ipfs/blob/b15edf287df6f7d8d99f7b7befb548469579a20c/namesys/namesys.go#L54

I’m having a bit of trouble figuring out what’s calling what but this would be consistent with the failure of the gateway to load .eth names via Host: header (works fine via /ipns/foo.eth)

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.

5 participants