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

Support ENS names instead of fixed addresses #64

Open
protinam opened this issue Feb 25, 2018 · 7 comments
Open

Support ENS names instead of fixed addresses #64

protinam opened this issue Feb 25, 2018 · 7 comments

Comments

@protinam
Copy link
Contributor

Would ENS resolution be in the scope of what this metadata repository intends to support?

For example, the Wyvern Protocol is self-upgrading and may change contracts later, so it would be convenient to register the smart contract under an ENS name instead of a fixed address. Other projects with proxy contracts or upgrade mechanisms may share this preference.

@danfinlay
Copy link
Contributor

That’s interesting. I’d be willing to support both. If you want to add an ens validation for a new ens field to the test suite, I would merge it.

@protinam
Copy link
Contributor Author

protinam commented Mar 2, 2018

I'm quite willing to code it - I'm specifically referring to the ability to specify an ENS name instead of an Ethereum address (not just as an additional field), and for Metamask to resolve ENS names and display the associated image if the ENS name resolution matches the address the user is sending to.

Maybe a field would work too - but what would Metamask do if the resolved ENS name didn't match the address? The advantage from my perspective would be the ability to upgrade contracts without causing user confusion (and without requiring you to merge quite so many pull requests).

@danfinlay
Copy link
Contributor

I understand that. The first step is permitting the optionally alternative field here in the test suite, and making a major semver bump. Integrating into MetaMask is a larger but separate issue.

@danfinlay
Copy link
Contributor

I guess there are some open questions about how this would fit in the current format; this is an address map, so the change needs to be made thoughtfully. Interested to see proposed formats.

@protinam
Copy link
Contributor Author

protinam commented Mar 2, 2018

One option that maintains JSON format compatibility and provides some similar assurance properties as manually specified addresses (the current method) is to instead specify the address which owns the ENS name as the key, and the ENS name itself as a field.

Clients of this metadata repository (such as Metamask) could then opt-in to resolving ENS names when specified, and asserting that the owner of the ENS name is the manually specified address (which can be verified by repository maintainers as belonging to the project in question).

A disadvantage of this approach is that existing clients which ignore the ENS field and upgrade to the new version would mistakenly identify transactions to the owning address as transactions to the smart contract which the ENS name resolves to, which would be incorrect. Do you think a semver bump would be enough to make that an acceptable change?

If not, I think a separate mapping might be best, but I'm not quite sure how best to integrate that into this repository (which just exports a single JSON file as the default export).

@danfinlay
Copy link
Contributor

I agree, a second mapping may be best. It doesn't need to be the default export, but it could still be in the same repository, and an optional import. Something like require('eth-contract-metadata/ens') would be non-breaking, and allow consumers to opt-in as able.

@adibas03
Copy link
Contributor

is there an update to this issue. Would definitely be an interesting add

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

No branches or pull requests

4 participants
@danfinlay @adibas03 @protinam and others