search

MetaMask / contract-metadata

A mapping of ethereum contract addresses to broadly accepted icons for those addresses.
ISC License
448 stars 1.1k forks source link

Support ENS names instead of fixed addresses #64

Open protinam opened 6 years ago

protinam commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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