Skip to content

Ens multicoin#18035

Closed
makoto wants to merge 23 commits intoMetaMask:developfrom
makoto:ens-multicoin
Closed

Ens multicoin#18035
makoto wants to merge 23 commits intoMetaMask:developfrom
makoto:ens-multicoin

Conversation

@makoto
Copy link
Copy Markdown
Contributor

@makoto makoto commented Mar 7, 2023

Explanation

This PR is to resolve MetaMask/specifications#11

Screenshots/Screencaps

Screenshot 2023-03-13 at 15 13 59

Screenshot 2023-03-13 at 15 13 45

Screenshot 2023-03-13 at 15 13 33

Before

  • When Metamask was connected to Mainnet, it resolves ETH address on ENS Mainnet
  • When Metamask was connected to Testnet (Goerli, Ropsten, Rinkeby), it resolves ETH address on ENS Mainnet

After

  • When Metamask was connected to Mainnet, it resolves ETH address on ENS Mainnet
  • When Metamask was connected to Testnet (Goerli, Ropsten, Rinkeby), it resolves ETH address on ENS Mainnet
  • When Metamask was connected to Other network(eg: OP), it resolves OP address on Optimism network

Manual Testing Steps

  • When Metamask was connected to Mainnet, looking up matoken.eth should return 0x5a

  • When Metamask was connected to Mainnet, looking up makoto.eth should return 0xfFD

  • When Metamask was connected to Mainnet, looking up 0xmatoken.eth should return no address has been set for this name

  • When Metamask was connected to Testnet (Goerli), looking up makoto.eth should return no address has been set for this name

  • When Metamask was connected to Testnet (Goerli), looking up 0xmatoken.eth should return 0x5a

  • When Metamask was connected to Other network(eg: OP), looking up matoken.eth should return no address has been set for this name

  • When Metamask was connected to Other network(eg: OP), looking up bertho.eth should return 0x837

Pre-merge author checklist

  • I've clearly explained:
  • What problem this PR is solving
    • [x ] How this problem was solved
    • [x ] How reviewers can test my changes
  • Sufficient automated test coverage has been added = The majority of test on ens.js can be moved into ethers.js once it's upgraded into v6 plugin (currently under discussion with metamask team and ethers.js team)

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

makoto added 2 commits March 7, 2023 15:36
When fetching address in other network, query address with the chainId converted into the coinType
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 7, 2023

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.

Wallyworldg02
Wallyworldg02 previously approved these changes Mar 14, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 14, 2023

Codecov Report

Merging #18035 (2ce1440) into develop (1d5e8a7) will decrease coverage by 0.03%.
The diff coverage is 18.75%.

@@             Coverage Diff             @@
##           develop   #18035      +/-   ##
===========================================
- Coverage    64.01%   63.98%   -0.03%     
===========================================
  Files          918      919       +1     
  Lines        35310    35329      +19     
  Branches      8993     9000       +7     
===========================================
+ Hits         22602    22603       +1     
- Misses       12708    12726      +18     
Impacted Files Coverage Δ
ui/ducks/ens.js 13.04% <13.04%> (ø)
ui/ducks/domains.js 30.39% <33.33%> (+1.15%) ⬆️

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@makoto makoto marked this pull request as ready for review March 20, 2023 19:57
@makoto makoto requested a review from a team as a code owner March 20, 2023 19:57
@legobeat
Copy link
Copy Markdown
Contributor

legobeat commented Apr 11, 2023

@makoto Thank you for your efforts and sorry to leave this hanging. FWIW, I pushed a rebased version of your branch here.

You may also be interested to know that separately, this controller is slated for migration to another repo:

@github-actions
Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Jul 20, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 3, 2023

This PR was closed because there has been no follow up activity in the last 14 days. Thank you for your contributions.

@github-actions github-actions bot closed this Aug 3, 2023
mirceanis added a commit that referenced this pull request Sep 18, 2024
…esolver snap (#26242)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

The embedded ENS resolution logic is being replaced by snaps that use
`endowment:name-lookup`.
This change-set also includes a preinstalled snap that does ENS
resolution including ENS multi-coin functionality.

The ENS resolver snap will use the Ethereum provider from the extension
on Ethereum mainnet and ENS supported testnets and will revert to an
Infura JSON-RPC when the extension is connected to other networks. On
other networks it tries to resolve the [network-specific ENS
address](https://eips.ethereum.org/EIPS/eip-2304) and if none exists
will try to resolve the mainnet address (with a ⚠️ warning) as long as
that address appears to be an externally owned account.

Most of the relevant changes are in `domains.js`.
Since the preinstalled snap uses ethers v6 while the extension depends
on ethers v5.7 there are some changes in the lavamoat policies. Also,
the more recent ethers version requires a different set of mocks,
requiring a small change in the e2e test.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26242?quickstart=1)

## **Related issues**

fixes https://github.com/MetaMask/MetaMask-planning/issues/2403
closes #18035
fixes #18648
fixes #22797
fixes #8556
fixes MetaMask/specifications#11

## **Manual testing steps**

1. While on Ethereum mainnet, start a send flow
2. Type an ENS name in the recipient field(example _vitalik.eth_), pick
the resulting address. (this should be identical to previous behavior)
3. Switch to a L2
4. Type an EOA ENS name in the recipient field (example _vitalik.eth_),
observe an address being resolved with a ⚠️warning.
5. Type an ENS name that would resolve to a contract on mainnet (example
_uniswap.eth_). Observe `No resolution for domain provided.` error.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->



https://github.com/user-attachments/assets/6e8866ef-f478-4647-a0f0-f1c13342edd1



## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Signed-off-by: Mircea Nistor <mirceanis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale issues and PRs marked as stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for ENS Multicoin when connected to other EVM chain

3 participants