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

✔️ Issue #5742 update ens resolver to eip 1577 #6402

Merged
merged 9 commits into from
May 30, 2019
Merged

✔️ Issue #5742 update ens resolver to eip 1577 #6402

merged 9 commits into from
May 30, 2019

Conversation

pldespaigne
Copy link
Contributor

@pldespaigne pldespaigne commented Apr 4, 2019

For solving this issue I have used a small lib called content-hash. I have developped this lib to implement the content-hash encoding specified by the EIP1577. The code of the lib can be found here. This lib is trusted by the ENS team and it is currently used in the official ENS manager web app (discussion here, commit here).
Because the purpose of the content-hash lib is to solve this exact issue, I have included it in the package.json file.

Detailed realisations :

  • resolve eip1577 compliant swarm content-hash to the swarm gateway
  • resolve eip1577 compliant ipfs content-hash to the ipfs gateway
  • resolve legacy swarm content to the swarm gateway
  • redirect to the ENS manager on error
  • use ENS logo on the loading page

You can test on ropsten with those link
errordomain.eth -> error : redirect to the ENS manager
dappsnation.eth -> legacy swarm content
ipfs.dappsnation.eth ->ipfs content-hash
swarm.dappsnation.eth ->ipfs content-hash

@pldespaigne
Copy link
Contributor Author

see issue #5742

@pldespaigne pldespaigne changed the title [WIP do not merge yet]Issue #5742 update ens resolver to eip 1577 [💣 WIP do not merge yet 💣]Issue #5742 update ens resolver to eip 1577 Apr 11, 2019
@pldespaigne pldespaigne changed the title [💣 WIP do not merge yet 💣]Issue #5742 update ens resolver to eip 1577 Issue #5742 update ens resolver to eip 1577 May 4, 2019
@pldespaigne pldespaigne changed the title Issue #5742 update ens resolver to eip 1577 ✔️ Issue #5742 update ens resolver to eip 1577 May 4, 2019
@pldespaigne
Copy link
Contributor Author

@danfinlay @kumavis this is ready for review 😁

Arachnid
Arachnid previously approved these changes May 4, 2019
@pldespaigne
Copy link
Contributor Author

Guys ? Is there anybody to review my PR ?
@danfinlay @kumavis @bdresser

@tmashuang
Copy link
Contributor

Will need to fix package-lock conflicts, since it's been updated after this PR. Regression, so we aren't allowing access to legacy domains like http://portalnetwork.eth?

@pldespaigne
Copy link
Contributor Author

  • This PR supports legacy resolver with content field as defined by the old ens docs
  • It support the new contenthash field as defined by the EIP1577
  • It does NOT supports multihash field as proposed by EIP1062
  • It does NOT supports ipfs content resolution via text['dnslink'] ala Dappnode

multihash is not widely used and kinda deprecated by contenthash and resolution via the text field is not standard at all and used solely by Dappnode. Moreover it is really hard to find doc about both, that's why I haven't implemented them into this PR. I could do it if needed but I'm not sure it is a good idea, @Arachnid what are your thought about that ?

PS: Here you can read discussion about the EIP1062, dappnode and the EIP1577.

@Arachnid
Copy link

@pldespaigne I think supporting only content (as legacy) and contenthash is the best way to go.

@tmashuang
Copy link
Contributor

Looks good to me then, just need to resolve the package-lock.json. A quick solution would be to rebase with develop, remove the package-lock.json, npm i, and commit the new package-lock.json.

@pldespaigne
Copy link
Contributor Author

@danfinlay @bdresser @kumavis @whymarrh The conflict is solved, I just need a review now :)

@tmashuang tmashuang merged commit d837703 into MetaMask:develop May 30, 2019
@pldespaigne pldespaigne mentioned this pull request Nov 7, 2019
3 tasks
Gudahtt added a commit that referenced this pull request Feb 12, 2021
This dependency was used in our IPFS ENS resolver, but it has not been
used directly since #6402.
@Gudahtt Gudahtt mentioned this pull request Feb 12, 2021
Gudahtt added a commit that referenced this pull request Feb 12, 2021
This dependency was used in our IPFS ENS resolver, but it has not been
used directly since #6402.
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.

4 participants