Skip to content

resolve onion address#6869

Merged
whymarrh merged 2 commits intoMetaMask:developfrom
ricott1:develop
Jul 23, 2019
Merged

resolve onion address#6869
whymarrh merged 2 commits intoMetaMask:developfrom
ricott1:develop

Conversation

@ricott1
Copy link
Contributor

@ricott1 ricott1 commented Jul 16, 2019

We extended the Ipfs name resolution to include the possibility to resolve an onion address.

This pull works in tandem with the ens-app pull.

@ricott1 ricott1 requested a review from whymarrh as a code owner July 16, 2019 20:27
@whymarrh
Copy link
Contributor

This pull works in tandem with the ens-app pull.

Does that PR need to be merged in first? How are the different PRs coordinated here? The code changes here look good, I want to make sure I understand the context for all the different things that are working together to make this functionality useful to users.

@ricott1
Copy link
Contributor Author

ricott1 commented Jul 18, 2019

Does that PR need to be merged in first?

Yes, I think that is a better idea.

There is also the question of whether or not use a gateway to the tor network, or just resolve the onion address, in which case the address would be reachable only if using the tor browser.

@ricott1
Copy link
Contributor Author

ricott1 commented Jul 23, 2019

The ens-app pull has been merged. I updated the onion address resolution, but I'm still resolving the address directly, without any gateway.

@whymarrh
Copy link
Contributor

@ricott1 I rebased this PR on our latest develop branch and updated content-hash to the latest version. I'm good with these changes if you are.

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.

1ca21fd LGTM

@ricott1
Copy link
Contributor Author

ricott1 commented Jul 23, 2019

It's good for me, thanks

@whymarrh whymarrh merged commit e81aa60 into MetaMask:develop Jul 23, 2019
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