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 for ENS wildcard and offchain resolution #9

Open
Arachnid opened this issue Mar 24, 2022 · 6 comments
Open

Support for ENS wildcard and offchain resolution #9

Arachnid opened this issue Mar 24, 2022 · 6 comments

Comments

@Arachnid
Copy link

ENS has recently added new functionality in the form of two major upgrades:

  • Wildcard resolution (ENSIP 10) makes it possible to define wildcard subdomains, so that one resolver can provide records for all subdomains of a name.
  • CCIP Read (EIP 3668) makes it possible for names to be resolved using offchain data.

The combination of these two features mean it's now possible to have ENS names that are stored on L2s or on other offchain data stores such as private databases.

ethers.js has already added support for these features in v5.6.1. If MetaMask is using ethers for ENS resolution, you can simply update to the latest version. If not, adding this feature would require implementing the above two specifications - or transitioning to using ethers.js for ENS resolution instead.

We'd love to see MetaMask support this new functionality, so please let us know if there's anything we can do to facilitate it.

@fredlacs
Copy link

I'm not sure if this was considered in the spec/impl, but I believe wallets should limit to only resolve to addresses that are EOAs.
For example, if my ENS domain resolves to a smart contract wallet on Ethereum, that doesn't necessarily mean I also control a smart contract wallet at the same address in a given L2 (this is particularly error prone with contract factories).

I'm not sure on the best way for Metamask to protect their users from potential misuse with this, but an interesting mitigation could be to show an extra warning on the UI if there is smart contract code associated to the resolved address in any given network.

@Arachnid
Copy link
Author

Arachnid commented May 11, 2022

I'm not sure if this was considered in the spec/impl, but I believe wallets should limit to only resolve to addresses that are EOAs.

This is a bad idea and would significantly limit the usefulness of ENS. We're working on support for explicitly tagging which EVM-compatible chains an address is for.

There's also no way to distinguish a clean EOA address from one that just hasn't had a contract deployed to it yet.

@fredlacs
Copy link

I'm not sure if this was considered in the spec/impl, but I believe wallets should limit to only resolve to addresses that are EOAs.

This is a bad idea and would significantly limit the usefulness of ENS. We're working on support for explicitly tagging which EVM-compatible chains an address is for.

I meant to highlight that there is risk of misuse if ENS is integrated in multiple domains as-is.

I suggested a potential mitigation, but maybe waiting for the aforementioned tagging feature might be needed.

@Arachnid
Copy link
Author

This isn't specific to wildcard and offchain resolution, so I don't see why it would affect this integration.

Perhaps an acceptable intermediate solution would be to show a warning in the UX when resolving ENS names on non-Ethereum chains that the user should check the target address is correct? It's somewhat academic at the moment, as I don't believe MetaMask actually supports resolving ENS names on networks other than Mainnet and test nets.

@fredlacs
Copy link

fredlacs commented Jun 9, 2022

My bad, I thought offchain resolution and CCIP were directly related to multichain support - which prompted me to bring up these points.
Do you have another discussion thread where it would be more productive to bring up multichain concerns and potential mitigations?

Tbh after the wintermute <> optimism incident, I'm not sure users tagging chains that an address is for is the ideal solution.

@kevinghim
Copy link

Resolved by MetaMask/metamask-extension#14675

@kevinghim kevinghim reopened this Jan 24, 2023
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

3 participants