From e937541eef6fb4cb136ba2459d1e6cf6e64889ce Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 28 Feb 2024 14:58:49 +0100 Subject: [PATCH 1/5] Replace EnsController with core implementation --- app/scripts/controllers/ens/ens.js | 29 ----- app/scripts/controllers/ens/index.js | 101 ---------------- app/scripts/controllers/ens/index.test.js | 133 ---------------------- app/scripts/metamask-controller.js | 12 +- package.json | 1 + yarn.lock | 17 +++ 6 files changed, 24 insertions(+), 269 deletions(-) delete mode 100644 app/scripts/controllers/ens/ens.js delete mode 100644 app/scripts/controllers/ens/index.js delete mode 100644 app/scripts/controllers/ens/index.test.js diff --git a/app/scripts/controllers/ens/ens.js b/app/scripts/controllers/ens/ens.js deleted file mode 100644 index 62779bccaf8d..000000000000 --- a/app/scripts/controllers/ens/ens.js +++ /dev/null @@ -1,29 +0,0 @@ -import { Web3Provider } from '@ethersproject/providers'; -import ensNetworkMap from 'ethereum-ens-network-map'; -import { CHAIN_ID_TO_ETHERS_NETWORK_NAME_MAP } from '../../../../shared/constants/network'; - -export default class Ens { - static getChainEnsSupport(chainId) { - return Boolean(ensNetworkMap[parseInt(chainId, 16).toString()]); - } - - constructor({ chainId, provider } = {}) { - const networkName = CHAIN_ID_TO_ETHERS_NETWORK_NAME_MAP[chainId]; - const chainIdInt = parseInt(chainId, 16); - const ensAddress = ensNetworkMap[chainIdInt.toString()]; - const ethProvider = new Web3Provider(provider, { - chainId: chainIdInt, - name: networkName, - ensAddress, - }); - this._ethProvider = ethProvider; - } - - lookup(ensName) { - return this._ethProvider.resolveName(ensName); - } - - reverse(address) { - return this._ethProvider.lookupAddress(address); - } -} diff --git a/app/scripts/controllers/ens/index.js b/app/scripts/controllers/ens/index.js deleted file mode 100644 index efea49e85567..000000000000 --- a/app/scripts/controllers/ens/index.js +++ /dev/null @@ -1,101 +0,0 @@ -import punycode from 'punycode/punycode'; -import { ObservableStore } from '@metamask/obs-store'; -import log from 'loglevel'; -import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; -import Ens from './ens'; - -const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; -const ZERO_X_ERROR_ADDRESS = '0x'; - -export default class EnsController { - constructor({ ens, provider, onNetworkDidChange, getCurrentChainId } = {}) { - const initState = { - ensResolutionsByAddress: {}, - }; - - this._ens = ens; - if (!this._ens) { - const chainId = getCurrentChainId(); - if (Ens.getChainEnsSupport(chainId)) { - this._ens = new Ens({ - chainId, - provider, - }); - } - } - - this.store = new ObservableStore(initState); - - this.resetState = () => { - this.store.updateState(initState); - }; - - onNetworkDidChange(() => { - this.store.putState(initState); - const chainId = getCurrentChainId(); - if (Ens.getChainEnsSupport(chainId)) { - this._ens = new Ens({ - chainId, - provider, - }); - } else { - delete this._ens; - } - }); - } - - reverseResolveAddress(address) { - return this._reverseResolveAddress(toChecksumHexAddress(address)); - } - - async _reverseResolveAddress(address) { - if (!this._ens) { - return undefined; - } - - const state = this.store.getState(); - if (state.ensResolutionsByAddress[address]) { - return state.ensResolutionsByAddress[address]; - } - - let domain; - try { - domain = await this._ens.reverse(address); - } catch (error) { - log.debug(error); - return undefined; - } - - let registeredAddress; - try { - registeredAddress = await this._ens.lookup(domain); - } catch (error) { - log.debug(error); - return undefined; - } - - if ( - registeredAddress === ZERO_ADDRESS || - registeredAddress === ZERO_X_ERROR_ADDRESS - ) { - return undefined; - } - - if (toChecksumHexAddress(registeredAddress) !== address) { - return undefined; - } - - this._updateResolutionsByAddress(address, punycode.toASCII(domain)); - return domain; - } - - _updateResolutionsByAddress(address, domain) { - const oldState = this.store.getState(); - this.store.putState({ - ensResolutionsByAddress: { - ...oldState.ensResolutionsByAddress, - [address]: domain, - }, - }); - } -} diff --git a/app/scripts/controllers/ens/index.test.js b/app/scripts/controllers/ens/index.test.js deleted file mode 100644 index aa7ed8c04a98..000000000000 --- a/app/scripts/controllers/ens/index.test.js +++ /dev/null @@ -1,133 +0,0 @@ -import { strict as assert } from 'assert'; -import sinon from 'sinon'; -import EnsController from '.'; - -const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; -const ZERO_X_ERROR_ADDRESS = '0x'; - -describe('EnsController', function () { - let currentChainId; - let getCurrentChainId; - let onNetworkDidChange; - beforeEach(function () { - currentChainId = '0x5'; - getCurrentChainId = () => currentChainId; - onNetworkDidChange = sinon.spy(); - }); - afterEach(function () { - sinon.restore(); - }); - describe('#constructor', function () { - it('should construct the controller given a provider and a network', async function () { - const ens = new EnsController({ - provider: sinon.fake(), - getCurrentChainId, - onNetworkDidChange, - }); - - assert.ok(ens._ens); - }); - - it('should construct the controller given an existing ENS instance', async function () { - const ens = new EnsController({ - ens: {}, - getCurrentChainId, - onNetworkDidChange, - }); - - assert.ok(ens._ens); - }); - }); - - describe('#reverseResolveName', function () { - it('should resolve to an ENS name', async function () { - const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5'; - const ens = new EnsController({ - ens: { - reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), - lookup: sinon.stub().withArgs('peaksignal.eth').returns(address), - }, - onNetworkDidChange, - getCurrentChainId, - }); - - const name = await ens.reverseResolveAddress(address); - assert.equal(name, 'peaksignal.eth'); - }); - - it('should only resolve an ENS name once', async function () { - const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5'; - const reverse = sinon.stub().withArgs(address).returns('peaksignal.eth'); - const lookup = sinon.stub().withArgs('peaksignal.eth').returns(address); - const ens = new EnsController({ - ens: { - reverse, - lookup, - }, - getCurrentChainId, - onNetworkDidChange, - }); - - assert.equal(await ens.reverseResolveAddress(address), 'peaksignal.eth'); - assert.equal(await ens.reverseResolveAddress(address), 'peaksignal.eth'); - assert.ok(lookup.calledOnce); - assert.ok(reverse.calledOnce); - }); - - it('should fail if the name is registered to a different address than the reverse-resolved', async function () { - const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5'; - const ens = new EnsController({ - ens: { - reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), - lookup: sinon.stub().withArgs('peaksignal.eth').returns('0x00'), - }, - onNetworkDidChange, - getCurrentChainId, - }); - - const name = await ens.reverseResolveAddress(address); - assert.strictEqual(name, undefined); - }); - - it('should throw an error when the lookup resolves to the zero address', async function () { - const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5'; - const ens = new EnsController({ - ens: { - reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), - lookup: sinon.stub().withArgs('peaksignal.eth').returns(ZERO_ADDRESS), - }, - getCurrentChainId, - onNetworkDidChange, - }); - - try { - await ens.reverseResolveAddress(address); - assert.fail('#reverseResolveAddress did not throw'); - } catch (e) { - assert.ok(e); - } - }); - - it('should throw an error the lookup resolves to the zero x address', async function () { - const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5'; - const ens = new EnsController({ - ens: { - reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), - lookup: sinon - .stub() - .withArgs('peaksignal.eth') - .returns(ZERO_X_ERROR_ADDRESS), - }, - onNetworkDidChange, - getCurrentChainId, - }); - - try { - await ens.reverseResolveAddress(address); - assert.fail('#reverseResolveAddress did not throw'); - } catch (e) { - assert.ok(e); - } - }); - }); -}); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 50c5bef3e23b..932f96f0bee3 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -55,7 +55,7 @@ import { ApprovalRequestNotFoundError, } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; - +import { EnsController } from '@metamask/ens-controller'; import { PhishingController } from '@metamask/phishing-controller'; import { AnnouncementController } from '@metamask/announcement-controller'; import { NetworkController } from '@metamask/network-controller'; @@ -256,7 +256,6 @@ import { NetworkOrderController } from './controllers/network-order'; import { AccountOrderController } from './controllers/account-order'; import createOnboardingMiddleware from './lib/createOnboardingMiddleware'; import { setupMultiplex } from './lib/stream-utils'; -import EnsController from './controllers/ens'; import PreferencesController from './controllers/preferences'; import AppStateController from './controllers/app-state'; import AlertController from './controllers/alert'; @@ -901,9 +900,10 @@ export default class MetamaskController extends EventEmitter { ); this.ensController = new EnsController({ + messenger: this.controllerMessenger.getRestricted({ + name: 'EnsController', + }), provider: this.provider, - getCurrentChainId: () => - this.networkController.state.providerConfig.chainId, onNetworkDidChange: networkControllerMessenger.subscribe.bind( networkControllerMessenger, 'NetworkController:networkDidChange', @@ -1934,7 +1934,7 @@ export default class MetamaskController extends EventEmitter { EncryptionPublicKeyController: this.encryptionPublicKeyController, SignatureController: this.signatureController, SwapsController: this.swapsController.store, - EnsController: this.ensController.store, + EnsController: this.ensController, ApprovalController: this.approvalController, ///: BEGIN:ONLY_INCLUDE_IF(blockaid) PPOMController: this.ppomController, @@ -2054,7 +2054,7 @@ export default class MetamaskController extends EventEmitter { ), this.signatureController.resetState.bind(this.signatureController), this.swapsController.resetState, - this.ensController.resetState, + this.ensController.resetState.bind(this.ensController), this.approvalController.clear.bind(this.approvalController), // WE SHOULD ADD TokenListController.resetState here too. But it's not implemented yet. ]; diff --git a/package.json b/package.json index a9f983f158c4..de427faf47bd 100644 --- a/package.json +++ b/package.json @@ -256,6 +256,7 @@ "@metamask/controller-utils": "^8.0.1", "@metamask/design-tokens": "^2.1.1", "@metamask/desktop": "^0.3.0", + "@metamask/ens-controller": "^9.0.0", "@metamask/eth-json-rpc-middleware": "^12.1.0", "@metamask/eth-keyring-controller": "^15.1.0", "@metamask/eth-ledger-bridge-keyring": "^2.0.1", diff --git a/yarn.lock b/yarn.lock index 1514ad331f2b..00529e6a6a99 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4040,6 +4040,22 @@ __metadata: languageName: node linkType: hard +"@metamask/ens-controller@npm:^9.0.0": + version: 9.0.0 + resolution: "@metamask/ens-controller@npm:9.0.0" + dependencies: + "@ethersproject/providers": "npm:^5.7.0" + "@metamask/base-controller": "npm:^4.1.1" + "@metamask/controller-utils": "npm:^8.0.2" + "@metamask/utils": "npm:^8.3.0" + ethereum-ens-network-map: "npm:^1.0.2" + punycode: "npm:^2.1.1" + peerDependencies: + "@metamask/network-controller": ^17.2.0 + checksum: d1f14db169614b3c403660eae3b1e7b74fd8347a5df6e94b67cd1f47f0bb43513dd109683568c284b6edfe1bf68088777fba55f59c1c2adad152f86932982623 + languageName: node + linkType: hard + "@metamask/eslint-config-jest@npm:^9.0.0": version: 9.0.0 resolution: "@metamask/eslint-config-jest@npm:9.0.0" @@ -24554,6 +24570,7 @@ __metadata: "@metamask/controller-utils": "npm:^8.0.1" "@metamask/design-tokens": "npm:^2.1.1" "@metamask/desktop": "npm:^0.3.0" + "@metamask/ens-controller": "npm:^9.0.0" "@metamask/eslint-config": "npm:^9.0.0" "@metamask/eslint-config-jest": "npm:^9.0.0" "@metamask/eslint-config-mocha": "npm:^9.0.0" From 75fcbd9a2f8e4d42f92c043e0ef039bcd0d9b18c Mon Sep 17 00:00:00 2001 From: MetaMask Bot Date: Wed, 28 Feb 2024 14:12:44 +0000 Subject: [PATCH 2/5] Update LavaMoat policies --- lavamoat/browserify/beta/policy.json | 10 ++++++++++ lavamoat/browserify/desktop/policy.json | 10 ++++++++++ lavamoat/browserify/flask/policy.json | 10 ++++++++++ lavamoat/browserify/main/policy.json | 10 ++++++++++ lavamoat/browserify/mmi/policy.json | 10 ++++++++++ 5 files changed, 50 insertions(+) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 714522ccdd29..91bea13dd23a 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -924,6 +924,16 @@ "fetch": true } }, + "@metamask/ens-controller": { + "packages": { + "@ethersproject/providers": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, + "@metamask/utils": true, + "ethereum-ens-network-map": true, + "punycode": true + } + }, "@metamask/eth-json-rpc-middleware": { "globals": { "URL": true, diff --git a/lavamoat/browserify/desktop/policy.json b/lavamoat/browserify/desktop/policy.json index 177ad47d1013..b39ae05a2ac5 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -1001,6 +1001,16 @@ "define": true } }, + "@metamask/ens-controller": { + "packages": { + "@ethersproject/providers": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, + "@metamask/utils": true, + "ethereum-ens-network-map": true, + "punycode": true + } + }, "@metamask/eth-json-rpc-middleware": { "globals": { "URL": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 0172201e93f5..757f4dc20c5a 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -1001,6 +1001,16 @@ "define": true } }, + "@metamask/ens-controller": { + "packages": { + "@ethersproject/providers": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, + "@metamask/utils": true, + "ethereum-ens-network-map": true, + "punycode": true + } + }, "@metamask/eth-json-rpc-middleware": { "globals": { "URL": true, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 87e1fd867369..e0eab09e02e2 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -924,6 +924,16 @@ "fetch": true } }, + "@metamask/ens-controller": { + "packages": { + "@ethersproject/providers": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, + "@metamask/utils": true, + "ethereum-ens-network-map": true, + "punycode": true + } + }, "@metamask/eth-json-rpc-middleware": { "globals": { "URL": true, diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 14f328fd3e2a..515dc1d0854a 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -1056,6 +1056,16 @@ "fetch": true } }, + "@metamask/ens-controller": { + "packages": { + "@ethersproject/providers": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, + "@metamask/utils": true, + "ethereum-ens-network-map": true, + "punycode": true + } + }, "@metamask/eth-json-rpc-middleware": { "globals": { "URL": true, From f62da2feb4e9d0b0cd47ac7eceb0bf7e1de2e51c Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 28 Feb 2024 20:57:53 +0100 Subject: [PATCH 3/5] fix: remove ensEntries from sentry payload --- app/scripts/lib/setupSentry.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 9a0eb9e9199d..03555503bd34 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -114,6 +114,7 @@ export const SENTRY_BACKGROUND_STATE = { }, EnsController: { ensResolutionsByAddress: false, + ensEntries: false, }, GasFeeController: { estimatedGasFeeTimeBounds: true, From 4e5124a8749b1a7dda0be4ae65df87e201349b20 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 28 Feb 2024 21:29:00 +0100 Subject: [PATCH 4/5] fix: e2e --- .../errors-after-init-opt-in-background-state.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json index d3269fd906e7..a383c75126a4 100644 --- a/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json @@ -71,7 +71,10 @@ "unapprovedEncryptionPublicKeyMsgs": "object", "unapprovedEncryptionPublicKeyMsgCount": 0 }, - "EnsController": { "ensResolutionsByAddress": "object" }, + "EnsController": { + "ensEntries": "object", + "ensResolutionsByAddress": "object" + }, "GasFeeController": { "gasFeeEstimatesByChainId": {}, "gasFeeEstimates": {}, From 28d5e6ded7f0cd589c057f09b9878248879d5e49 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 28 Feb 2024 21:47:40 +0100 Subject: [PATCH 5/5] fix: e2e --- .../tests/state-snapshots/errors-after-init-opt-in-ui-state.json | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json index de685a700d37..951ea1bd9aff 100644 --- a/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -217,6 +217,7 @@ "swapsStxGetTransactionsRefreshTime": 10000, "swapsStxMaxFeeMultiplier": 2 }, + "ensEntries": "object", "ensResolutionsByAddress": "object", "pendingApprovals": "object", "pendingApprovalCount": "number",