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

feat: support ipns address translations #8502

Merged
merged 3 commits into from
May 14, 2020
Merged

feat: support ipns address translations #8502

merged 3 commits into from
May 14, 2020

Conversation

hacdias
Copy link
Contributor

@hacdias hacdias commented May 3, 2020

Hi! 👋

This adds support for IPNS Links which are also part of IPFS spec. The address resolution is exactly the same since they're encoded the same way.

I needed to change the default ipfsGateway from ipfs.dweb.link to ipns.dweb.link which is similar to the defaults of IPFS Companion. I added a migration to replace whatever value the user has there by the new default.

Screenshot 2020-05-03 at 16 13 59

This does not add support for DNSLink though. /cc @lidel

@lidel
Copy link

lidel commented May 4, 2020

FYSA dweb.link is migrating from nginx to native subdomain support in go-ipfs 0.5, so we need to wait until migration is complete and *.ipns.dweb.link works.

Also, with go-ipfs 0.5 other gateway providers such as Infura are now able to run their own subdomain gateways, as described here.

app/scripts/migrations/045.js Outdated Show resolved Hide resolved
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

One small suggestion

app/scripts/migrations/045.js Outdated Show resolved Hide resolved
@whymarrh
Copy link
Contributor

whymarrh commented May 4, 2020

@lidel what does your comment mean for the timeline of this PR? Is that a follow-up change?

@lidel
Copy link

lidel commented May 6, 2020

@whymarrh no follow-up is needed, but the feature introduced in this PR won't work until IPFS project updates its DNS to support hostnames under *.ipns.dweb.link (eg. https://bafzbeiaqarwnhp7vmqgkhfbqyf32qgkf65xluww5vr2xlang2rekfiquku.ipns.dweb.link).
It should be fixed soon, will post when ready.

@whymarrh
Copy link
Contributor

whymarrh commented May 6, 2020

Sounds good, thanks

@lidel
Copy link

lidel commented May 13, 2020

@hacdias I believe IPNS support is ready:

https://dweb.link/ipns/QmPRB9SNEfYaTaZ88VLCpddTYD45B53sC8yH8xYZXGPLJp
is now upgraded to case-insensitive identifier and redirected to own Origin:
https://bafzbeiaqarwnhp7vmqgkhfbqyf32qgkf65xluww5vr2xlang2rekfiquku.ipns.dweb.link

app/scripts/migrations/045.js Outdated Show resolved Hide resolved
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias hacdias marked this pull request as ready for review May 13, 2020 20:23
@hacdias hacdias requested a review from a team as a code owner May 13, 2020 20:23
@hacdias hacdias requested a review from whymarrh May 13, 2020 20:23
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

Two small suggestions

app/scripts/migrations/045.js Outdated Show resolved Hide resolved
app/scripts/migrations/045.js Outdated Show resolved Hide resolved
hacdias and others added 2 commits May 14, 2020 08:03
Co-authored-by: Whymarrh Whitby <whymarrh.whitby@gmail.com>
Co-authored-by: Whymarrh Whitby <whymarrh.whitby@gmail.com>
@hacdias hacdias requested a review from whymarrh May 14, 2020 07:29
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hacdias!

@whymarrh whymarrh merged commit 890bc25 into MetaMask:develop May 14, 2020
@whymarrh
Copy link
Contributor

We're building up to a version 8 release in the next couple of weeks—this will be part of that

@hacdias hacdias deleted the support-ipns branch May 14, 2020 10:30
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.

3 participants