MetaMask / specifications

Specifications that we're riffing on and want to keep secret.
3 stars 2 forks source link

Support for ENS wildcard and offchain resolution #9

Open Arachnid opened 2 years ago

Arachnid commented 2 years ago

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

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

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

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

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

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

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 commented 1 year ago

Resolved by https://github.com/MetaMask/metamask-extension/pull/14675