Support for ENS wildcard and offchain resolution

[makoto]

NOTE: This is retry of #14526 as it included some commits I made but for some reason it didn't show my name

Explanation

This PR addresses MetaMask/specifications#9

NOTE: Support for contenthash will be separate PR.

More information

Please read https://medium.com/the-ethereum-name-service/upgrade-ethers-js-to-5-6-1-to-activate-ens-l2-offchain-integration-40ee1a0fdf2a for more deail.

Screenshots/Screencaps

Display forward lookup

Display reverse record (aka primary name)

Manual testing steps

• 1. Select testnet (Mainnet/Ropsten/Rinkeby/Goerli network)
• 2. Lookup 1.offchainexample.eth to lookup ENS name
• 3. Send ETH to 1.offchainexample.eth and check it shows in the transaction log

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[makoto]

Hi @mrbaguvix @jacobc-eth . Just wondering if anything else required to get this merged

[seaona]

Some findings/questions from QA:

1. It looks like ENS resolution stopped working with emojis in this branch. See difference between this branch (left) and production Extension (right)
   

2. When I look for a subdomain containing emojis, the error Error: STRINGPREP_CONTAINS_UNASSIGNED is thrown on the console and I see ENS Lookup failed. on the UI. Are emojis allowed on subdomains?

3. When an address is not registered (on mainnet), we used to saw the message No address has been set (right). Now we don't see any message, but we cannot proceed with the tx neither.
   I read that you can actually send assets to an ENS name that has never been registered. So I'm wondering in this case, how should we proceed on the UI. Should we allow to go to the next screen and send eth to that ENS address? and/or should we display any message/warning? At the moment, none of these options is happening and maybe it confuses the user

4. On the Address book, we have some differences between this branch (left) and production version (right) when trying to add non-registered ENS. Before we saw a warning text, saying name was not registered, and when proceed, contact was not added and we saw error on update the address book (I don't think it's an ideal flow). Now the process is the same, but we don't see any warning text. I think both approaches could be improved in terms of UI. Not sure how though

address-book.mp4

5. I can include non-valid address and still be able to click them, while "Recipient address is invalid" message is displayed. I.e. try with a.b..offchainexample.eth or a.b.c..offchainexample.eth

subdomains.mp4

6. FYI: opened this bug, related to red char highlight, as a separate issue, as it's an old one

7. FYI: this bug is related to the OffchainResolver contract - it's a separate issue, but I also link it here for context

[makoto]

Hi @seaona thank you for the detailed feedback.
I will look into 3~5.
For 1 and 2, the current ENS name resolution follows a rule specified by IDNA

According to the specification, the emojis like ❤❤❤.eth are supported and both the Metamask and our ens-app website resolve correctly (eg: https://app.ens.domains/name/%E2%9D%A4%E2%9D%A4%E2%9D%A4.eth/details).

However, 😂😂😂.eth (aka "face with tearful joy") uses 0x1f602 which falls in the unassigned range 1D800-1FFFD according to https://datatracker.ietf.org/doc/html/draft-ietf-idn-nameprep#appendix-A.1 hence I would say it is correct behavior not being able to resole these names.

[seaona]

Thank you so much for the explanation of the emojis, @makoto.

I include below another item, which I think it's not the expected behaviour, but would be great if you could also confirm/take a look.
Basically, when I'm on a testnet (Rinkeby) and I send ETH to a subdomain, I see the ENS domain in the first tx screen, but then, it's not displayed anymore as ENS, but rather as the Address name of Metamask (on confirmation screen) and the Ethereum address (on the tx details).

subdomain-not-displayed.mp4

That's different than the behaviour of a "main" domain, where I see always the ENS name:

[makoto]

Hi, @seaona I actually can't replicate your case as mariona.eth doesn't seem to have any address set on rinkeby , so not sure why you can even resolve.

In terms of the difference of subdomain (test.mariona.eth) and parent domain (mariona.eth), does this behavior only happen on my PR branch, but not the main branch? I didn't touch UI component so can't think of any reason why it behaves differently unless my ens lookup is throwing different error than main branch hence UI component is not doing correct error handling.

BTW, I am currently doing yarn setup && yarn dist to generate each browser extension and install it to my browser to test but it takes quite a long time to compile. Are there any other ways to test UI behavior?

[makoto]

Hi @seaona . I tested again on my normal chrome extension (not the one built from the source) and it doesn't show ENS name on the step 2 nor 3.

[seaona]

hi @makoto thank you very much for your reply and your investigation. Let me check this again and will write back findings in this thread (most likely on Monday)

[makoto]

Hi @seaona , I just fixed 3 - 5. PTAL

[seaona]

hi @makoto :) thank you for the updates!!
For having a quick build, instead of building production (yarn dist) you can do yarn start . This will go much faster and it's more convenient while you are developing. Steps are:

1. Update dependencies: yarn setup
2. Start project: yarn start - when you see Bundle end: "primary" means that project is ready
3. Open browser
4. Upload file from dist/ folder and the appropiate browser folder

Now, everytime you change something on the codebase, you don't need to upload again the source, as it will be updated automatically :)

I've re-tested all the changes and here my updates:
5. I see it fixed
6. I see it fixed
7. I see it fixed

For the mariona.eth I'm seeing the same as you now, might be that while doing some testing I've updated the address resolver 🤔​ I'll do some more testing to confirm and let you know if anything else comes up.
Thank you!!

[makoto]

Hi, is there anything else I can help to get this PR merged?

[jacobc-eth]

Hey @makoto were you still working on also submitting the companion PR on MetaMask Mobile (we'd discussed in this thread: #14526)? This sort of feature really needs cross-client compatibility, and we'd strongly prefer to have it available on MetaMask Mobile in order to merge.

[makoto]

@jacobc-eth yes I raised PR on MM Mobile a month ago MetaMask/metamask-mobile#4322 though I couldn't setup the environment by myself so I couldn't test by myself

[Arachnid]

@jacobc-eth Would it be possible to get some movement on this? Major partners are rolling out subdomains for users soon, and it would be really great to have MM as one of the first wallets to support it.

[makoto]

Hi @jacobc-eth @seaona , any update on this?

[jacobc-eth]

hey @makoto so sorry for the delay and confusion in this thread. thanks so much for submitting the PR for mobile, too. I've personally been extremely sick with COVID and am just now getting caught back up. I'm going to pass along these PRs to the relevant stakeholders and we should be in touch.

[seaona]

@makoto I shared the PR with the extension dev team, to see if we can get the needed approvals

[darkwing]

Code looks good and works lovely! Thank you @makoto

[omnat]

Hey @makoto, thanks for opening a PR on mobile repo as well! We are looking to release that PR, but it has some merge conflicts that needs your attention. Team is keen to hear if you'll have some time to look into it? Looking forward to hear back. Thank you!!