From 34e8d3014f9376e265787250bb492a8415203faf Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Mon, 27 Jan 2025 22:48:53 +0100 Subject: [PATCH 1/5] chore: remove eth-method-registry package --- packages/transaction-controller/package.json | 1 - .../src/TransactionController.ts | 1 - .../src/helpers/MethodDataHelper.test.ts | 78 ++++++------ .../src/helpers/MethodDataHelper.ts | 62 ++++------ yarn.lock | 114 ------------------ 5 files changed, 62 insertions(+), 194 deletions(-) diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 91ff685b317..d2761a5979f 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -62,7 +62,6 @@ "@metamask/utils": "^11.0.1", "async-mutex": "^0.5.0", "bn.js": "^5.2.1", - "eth-method-registry": "^4.0.0", "fast-json-patch": "^3.1.1", "lodash": "^4.17.21", "uuid": "^8.3.2" diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index a92967f2105..e1d122f71c2 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -881,7 +881,6 @@ export class TransactionController extends BaseController< ); this.#methodDataHelper = new MethodDataHelper({ - getProvider: (networkClientId) => this.#getProvider({ networkClientId }), getState: () => this.state.methodData, }); diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts index 934819c1221..87922920adb 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts @@ -1,9 +1,9 @@ -import { MethodRegistry } from 'eth-method-registry'; +import { type FunctionFragment, Interface } from '@ethersproject/abi'; -import type { MethodData } from '../TransactionController'; import { MethodDataHelper } from './MethodDataHelper'; +import type { MethodData } from '../TransactionController'; -jest.mock('eth-method-registry'); +jest.mock('@ethersproject/abi'); const FOUR_BYTE_PREFIX_MOCK = '0x12345678'; const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId'; @@ -18,18 +18,18 @@ const METHOD_DATA_MOCK: MethodData = { }; /** - * Creates a mock MethodRegistry instance. - * @returns The mocked MethodRegistry instance. + * Creates a mock Interface instance. + * + * @returns The mocked Interface instance. */ -function createMethodRegistryMock() { +function createInterfaceMock() { return { - lookup: jest.fn(), - parse: jest.fn(), - } as unknown as jest.Mocked; + getFunction: jest.fn(), + } as unknown as jest.Mocked; } describe('MethodDataHelper', () => { - const methodRegistryClassMock = jest.mocked(MethodRegistry); + const interfaceClassMock = jest.mocked(Interface); beforeEach(() => { jest.resetAllMocks(); @@ -38,7 +38,6 @@ describe('MethodDataHelper', () => { describe('lookup', () => { it('returns method data from cache', async () => { const methodDataHelper = new MethodDataHelper({ - getProvider: jest.fn(), getState: () => ({ [FOUR_BYTE_PREFIX_MOCK]: METHOD_DATA_MOCK }), }); @@ -50,17 +49,17 @@ describe('MethodDataHelper', () => { expect(result).toStrictEqual(METHOD_DATA_MOCK); }); - it('returns method data from registry lookup', async () => { - const methodRegistryMock = createMethodRegistryMock(); - methodRegistryMock.lookup.mockResolvedValueOnce(SIGNATURE_MOCK); - methodRegistryMock.parse.mockReturnValueOnce( - METHOD_DATA_MOCK.parsedRegistryMethod, - ); + it('returns method data from interface lookup', async () => { + const interfaceMock = createInterfaceMock(); + interfaceMock.getFunction.mockReturnValueOnce({ + name: 'testMethod', + inputs: [{ type: 'uint256' }, { type: 'uint256' }], + format: jest.fn(() => SIGNATURE_MOCK), + } as unknown as FunctionFragment); - methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + interfaceClassMock.mockReturnValueOnce(interfaceMock); const methodDataHelper = new MethodDataHelper({ - getProvider: jest.fn(), getState: () => ({}), }); @@ -72,14 +71,15 @@ describe('MethodDataHelper', () => { expect(result).toStrictEqual(METHOD_DATA_MOCK); }); - it('returns empty method data if not found in registry', async () => { - const methodRegistryMock = createMethodRegistryMock(); - methodRegistryMock.lookup.mockResolvedValueOnce(undefined); + it('returns empty method data if not found in interface', async () => { + const interfaceMock = createInterfaceMock(); + interfaceMock.getFunction.mockImplementationOnce(() => { + throw new Error('Function not found'); + }); - methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + interfaceClassMock.mockReturnValueOnce(interfaceMock); const methodDataHelper = new MethodDataHelper({ - getProvider: jest.fn(), getState: () => ({}), }); @@ -94,16 +94,15 @@ describe('MethodDataHelper', () => { }); }); - it('creates registry instance for each unique network client ID', async () => { - const getProviderMock = jest.fn(); - - const methodRegistryMock = createMethodRegistryMock(); - methodRegistryMock.lookup.mockResolvedValueOnce(undefined); + it('creates interface instance for each unique network client ID', async () => { + const interfaceMock = createInterfaceMock(); + interfaceMock.getFunction.mockImplementationOnce(() => { + throw new Error('Function not found'); + }); - methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + interfaceClassMock.mockReturnValueOnce(interfaceMock); const methodDataHelper = new MethodDataHelper({ - getProvider: getProviderMock, getState: () => ({}), }); @@ -122,21 +121,20 @@ describe('MethodDataHelper', () => { 'anotherNetworkClientId', ); - expect(methodRegistryClassMock).toHaveBeenCalledTimes(2); - expect(getProviderMock).toHaveBeenCalledTimes(2); + expect(interfaceClassMock).toHaveBeenCalledTimes(2); }); it('emits event when method data is fetched', async () => { - const methodRegistryMock = createMethodRegistryMock(); - methodRegistryMock.lookup.mockResolvedValueOnce(SIGNATURE_MOCK); - methodRegistryMock.parse.mockReturnValueOnce( - METHOD_DATA_MOCK.parsedRegistryMethod, - ); + const interfaceMock = createInterfaceMock(); + interfaceMock.getFunction.mockReturnValueOnce({ + name: 'testMethod', + inputs: [{ type: 'uint256' }, { type: 'uint256' }], + format: jest.fn(() => SIGNATURE_MOCK), + } as unknown as FunctionFragment); - methodRegistryClassMock.mockReturnValueOnce(methodRegistryMock); + interfaceClassMock.mockReturnValueOnce(interfaceMock); const methodDataHelper = new MethodDataHelper({ - getProvider: jest.fn(), getState: () => ({}), }); diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.ts index 2542cb67178..0409dce1220 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.ts @@ -1,7 +1,7 @@ -import type { NetworkClientId, Provider } from '@metamask/network-controller'; +import { Interface } from '@ethersproject/abi'; +import type { NetworkClientId } from '@metamask/network-controller'; import { createModuleLogger } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { MethodRegistry } from 'eth-method-registry'; // This package purposefully relies on Node's EventEmitter module. // eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; @@ -14,26 +14,17 @@ const log = createModuleLogger(projectLogger, 'method-data'); export class MethodDataHelper { hub: EventEmitter; - #getProvider: (networkClientId: NetworkClientId) => Provider; - #getState: () => Record; - #methodRegistryByNetworkClientId: Map; + #interfaceByNetworkClientId: Map; #mutex = new Mutex(); - constructor({ - getProvider, - getState, - }: { - getProvider: (networkClientId: NetworkClientId) => Provider; - getState: () => Record; - }) { + constructor({ getState }: { getState: () => Record }) { this.hub = new EventEmitter(); - this.#getProvider = getProvider; this.#getState = getState; - this.#methodRegistryByNetworkClientId = new Map(); + this.#interfaceByNetworkClientId = new Map(); } async lookup( @@ -52,20 +43,15 @@ export class MethodDataHelper { return cachedResult; } - let registry = this.#methodRegistryByNetworkClientId.get(networkClientId); - - if (!registry) { - const provider = this.#getProvider(networkClientId); - - // @ts-expect-error Type in eth-method-registry is inappropriate and should be changed - registry = new MethodRegistry({ provider }); + let iface = this.#interfaceByNetworkClientId.get(networkClientId); - this.#methodRegistryByNetworkClientId.set(networkClientId, registry); - - log('Created registry', networkClientId); + if (!iface) { + iface = new Interface([]); + this.#interfaceByNetworkClientId.set(networkClientId, iface); + log('Created interface', networkClientId); } - const methodData = await this.#registryLookup(fourBytePrefix, registry); + const methodData = await this.#interfaceLookup(fourBytePrefix, iface); log('Result', methodData); @@ -77,25 +63,25 @@ export class MethodDataHelper { } } - async #registryLookup( + async #interfaceLookup( fourBytePrefix: string, - methodRegistry: MethodRegistry, + iface: Interface, ): Promise { - const registryMethod = await methodRegistry.lookup(fourBytePrefix); - - if (!registryMethod) { - log('No method found', fourBytePrefix); - + try { + const functionFragment = iface.getFunction(fourBytePrefix); + return { + registryMethod: functionFragment.format(), + parsedRegistryMethod: { + name: functionFragment.name, + args: functionFragment.inputs.map(({ type }) => ({ type })), + }, + }; + } catch { + log('No method found or invalid signature', fourBytePrefix); return { registryMethod: '', parsedRegistryMethod: { name: undefined, args: undefined }, }; } - - log('Parsing', registryMethod); - - const parsedRegistryMethod = methodRegistry.parse(registryMethod); - - return { registryMethod, parsedRegistryMethod }; } } diff --git a/yarn.lock b/yarn.lock index 40d8c3058be..b1bb0202753 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2943,30 +2943,6 @@ __metadata: languageName: node linkType: hard -"@metamask/ethjs-contract@npm:^0.4.1": - version: 0.4.1 - resolution: "@metamask/ethjs-contract@npm:0.4.1" - dependencies: - "@metamask/ethjs-filter": "npm:^0.3.0" - "@metamask/ethjs-util": "npm:^0.3.0" - ethjs-abi: "npm:^0.2.0" - js-sha3: "npm:^0.9.2" - promise-to-callback: "npm:^1.0.0" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/2187f62336c2afd46b51bda87b96e1537dd7dfe0f621defc3cb5ccb68add482004674f3c11ced95c65b7ecbe845592541f1b268c97b0b42c939d8d5677523097 - languageName: node - linkType: hard - -"@metamask/ethjs-filter@npm:^0.3.0": - version: 0.3.0 - resolution: "@metamask/ethjs-filter@npm:0.3.0" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/1b7565045e638f60d39ae8d8ca020b6e3ac2e4e91f220f343e499b09a7adeb9e4273249e3c3f7366acc2e44bc0b7fe3dd3ac84c66e9a0853352bfb0e116c1d87 - languageName: node - linkType: hard - "@metamask/ethjs-format@npm:^0.2.9": version: 0.2.9 resolution: "@metamask/ethjs-format@npm:0.2.9" @@ -2981,22 +2957,6 @@ __metadata: languageName: node linkType: hard -"@metamask/ethjs-format@npm:^0.3.0": - version: 0.3.0 - resolution: "@metamask/ethjs-format@npm:0.3.0" - dependencies: - "@metamask/ethjs-util": "npm:^0.3.0" - "@metamask/number-to-bn": "npm:^1.7.1" - bn.js: "npm:^5.2.1" - ethjs-schema: "npm:0.2.1" - is-hex-prefixed: "npm:1.0.0" - strip-hex-prefix: "npm:1.0.0" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/4ae73716e0dd76f7b780ede79b3989d241ee8654b7aa9ec23678529a1916f77ab101552f7aabe56c8e26506a33f34181f40624a68a317e7cd3ae45fb0da2fa73 - languageName: node - linkType: hard - "@metamask/ethjs-provider-http@npm:^0.3.0": version: 0.3.0 resolution: "@metamask/ethjs-provider-http@npm:0.3.0" @@ -3020,19 +2980,6 @@ __metadata: languageName: node linkType: hard -"@metamask/ethjs-query@npm:^0.7.1": - version: 0.7.1 - resolution: "@metamask/ethjs-query@npm:0.7.1" - dependencies: - "@metamask/ethjs-format": "npm:^0.3.0" - "@metamask/ethjs-rpc": "npm:^0.4.0" - promise-to-callback: "npm:^1.0.0" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/a9d54677cf062267f7e781996edfadfc2a5a2f3589266b7f20386e8109b7cecdfdc47c6aba4bb68d0c4cb387cfdb463d6316d506c06138531ca1e81ab201e5dc - languageName: node - linkType: hard - "@metamask/ethjs-rpc@npm:0.3.0 || ^0.3.2": version: 0.3.2 resolution: "@metamask/ethjs-rpc@npm:0.3.2" @@ -3042,17 +2989,6 @@ __metadata: languageName: node linkType: hard -"@metamask/ethjs-rpc@npm:^0.4.0": - version: 0.4.0 - resolution: "@metamask/ethjs-rpc@npm:0.4.0" - dependencies: - promise-to-callback: "npm:^1.0.0" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/e126535ff7599ef584b0084dae846294d505b69141a143543d7580cdce4b297646e796fbd0cecc5a824dce8b5e3b179bb177ceff06fd50ad584e7fce00ab081f - languageName: node - linkType: hard - "@metamask/ethjs-unit@npm:^0.3.0": version: 0.3.0 resolution: "@metamask/ethjs-unit@npm:0.3.0" @@ -3075,18 +3011,6 @@ __metadata: languageName: node linkType: hard -"@metamask/ethjs-util@npm:^0.3.0": - version: 0.3.0 - resolution: "@metamask/ethjs-util@npm:0.3.0" - dependencies: - is-hex-prefixed: "npm:1.0.0" - strip-hex-prefix: "npm:1.0.0" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/aaecae091dd16362f048cbba27629d67d10530c704facf80f430db546c8ea32fa627cd9560bd3eca1e6ce651d2b7a7df9fdc3677e0ae2814954e3c950cee70da - languageName: node - linkType: hard - "@metamask/example-controllers@workspace:examples/example-controllers": version: 0.0.0-use.local resolution: "@metamask/example-controllers@workspace:examples/example-controllers" @@ -4069,7 +3993,6 @@ __metadata: async-mutex: "npm:^0.5.0" bn.js: "npm:^5.2.1" deepmerge: "npm:^4.2.2" - eth-method-registry: "npm:^4.0.0" fast-json-patch: "npm:^3.1.1" immer: "npm:^9.0.6" jest: "npm:^27.5.1" @@ -7926,18 +7849,6 @@ __metadata: languageName: node linkType: hard -"eth-method-registry@npm:^4.0.0": - version: 4.0.0 - resolution: "eth-method-registry@npm:4.0.0" - dependencies: - "@metamask/ethjs-contract": "npm:^0.4.1" - "@metamask/ethjs-query": "npm:^0.7.1" - peerDependencies: - "@babel/runtime": ^7.0.0 - checksum: 10/959e833327960f40fab8693997e071def75103a694e931ab11313e642b6958d4f2878000d91ddfd6ba3c158bdc3f381c63eae0dd26738790e2aca5004ceb0387 - languageName: node - linkType: hard - "ethereum-cryptography@npm:^0.1.3": version: 0.1.3 resolution: "ethereum-cryptography@npm:0.1.3" @@ -8017,17 +7928,6 @@ __metadata: languageName: node linkType: hard -"ethjs-abi@npm:^0.2.0": - version: 0.2.1 - resolution: "ethjs-abi@npm:0.2.1" - dependencies: - bn.js: "npm:4.11.6" - js-sha3: "npm:0.5.5" - number-to-bn: "npm:1.7.0" - checksum: 10/733b2b95ae102f0813ad70848d42424b4b6204598892bee0e0fb435aafc7d356e479e489ee2d6b81fb2bd6e5ae6a9cdc0479bc35244b2efbe1a00a8da6735fbb - languageName: node - linkType: hard - "ethjs-schema@npm:0.2.1": version: 0.2.1 resolution: "ethjs-schema@npm:0.2.1" @@ -10108,13 +10008,6 @@ __metadata: languageName: node linkType: hard -"js-sha3@npm:0.5.5": - version: 0.5.5 - resolution: "js-sha3@npm:0.5.5" - checksum: 10/9ce8bfabdba2cfb94b911125fc278e2f46cc01b6590c98833d9361199e2c2b4bca0427d04da6aa083f05c2c3982029200964a3d6e417b0c126c80f2e32c2d5eb - languageName: node - linkType: hard - "js-sha3@npm:0.8.0": version: 0.8.0 resolution: "js-sha3@npm:0.8.0" @@ -10129,13 +10022,6 @@ __metadata: languageName: node linkType: hard -"js-sha3@npm:^0.9.2": - version: 0.9.3 - resolution: "js-sha3@npm:0.9.3" - checksum: 10/8daacb93b18609a0dc081f2f6199b80a96df36f9975b4b9c7476ae92822e07100b9e1969fc76f4b58e703cd6175f0de7656a99cbb2335cfb554c66f988fbead5 - languageName: node - linkType: hard - "js-tokens@npm:^3.0.0 || ^4.0.0, js-tokens@npm:^4.0.0": version: 4.0.0 resolution: "js-tokens@npm:4.0.0" From 77c9cd8648f5bbeabab0de94cfb21e6f0c3ade53 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Mon, 27 Jan 2025 22:57:27 +0100 Subject: [PATCH 2/5] chore: update changelog --- packages/transaction-controller/CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index e3de4de9635..94f20401a01 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Replaced `eth-method-registry` dependency with `@ethersproject/abi` for ABI method lookups ([#5203](https://github.com/MetaMask/core/pull/5203)) + - The `MethodDataHelper` class now uses ethers.js `Interface` class to parse and lookup method signatures + - Method data is now derived directly from the ABI interface + - The structure of returned method data remains compatible with previous implementation + ## [43.0.0] ### Added From 4bfc7f444d1b2829f95db7236c30f4c7682af1c5 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Tue, 28 Jan 2025 18:34:55 +0100 Subject: [PATCH 3/5] fix: add missing abi --- .../src/helpers/MethodDataHelper.ts | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.ts index 0409dce1220..f15a5119c2e 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.ts @@ -11,6 +11,77 @@ import type { MethodData } from '../TransactionController'; const log = createModuleLogger(projectLogger, 'method-data'); +const methodRegistryAbi = [ + { + constant: false, + inputs: [{ name: '_new', type: 'address' }], + name: 'setOwner', + outputs: [], + payable: false, + type: 'function', + }, + { + constant: true, + inputs: [], + name: 'totalSignatures', + outputs: [{ name: '', type: 'uint256' }], + payable: false, + type: 'function', + }, + { + constant: true, + inputs: [], + name: 'owner', + outputs: [{ name: '', type: 'address' }], + payable: false, + type: 'function', + }, + { + constant: false, + inputs: [], + name: 'drain', + outputs: [], + payable: false, + type: 'function', + }, + { + constant: true, + inputs: [{ name: '', type: 'bytes4' }], + name: 'entries', + outputs: [{ name: '', type: 'string' }], + payable: false, + type: 'function', + }, + { + constant: false, + inputs: [{ name: '_method', type: 'string' }], + name: 'register', + outputs: [{ name: '', type: 'bool' }], + payable: false, + type: 'function', + }, + { inputs: [], type: 'constructor' }, + { + anonymous: false, + inputs: [ + { indexed: true, name: 'creator', type: 'address' }, + { indexed: true, name: 'signature', type: 'bytes4' }, + { indexed: false, name: 'method', type: 'string' }, + ], + name: 'Registered', + type: 'event', + }, + { + anonymous: false, + inputs: [ + { indexed: true, name: 'old', type: 'address' }, + { indexed: true, name: 'current', type: 'address' }, + ], + name: 'NewOwner', + type: 'event', + }, +]; + export class MethodDataHelper { hub: EventEmitter; @@ -46,7 +117,7 @@ export class MethodDataHelper { let iface = this.#interfaceByNetworkClientId.get(networkClientId); if (!iface) { - iface = new Interface([]); + iface = new Interface(methodRegistryAbi); this.#interfaceByNetworkClientId.set(networkClientId, iface); log('Created interface', networkClientId); } From 934d4c6700cf35f9a3a36bbb41e5a1aa8e83140d Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 29 Jan 2025 16:32:10 +0100 Subject: [PATCH 4/5] refactor: move method registry ABI --- .../transaction-controller/src/constants.ts | 71 ++++++++++++++++++ .../src/helpers/MethodDataHelper.ts | 74 +------------------ 2 files changed, 73 insertions(+), 72 deletions(-) diff --git a/packages/transaction-controller/src/constants.ts b/packages/transaction-controller/src/constants.ts index cc769bf3413..1eadf32def4 100644 --- a/packages/transaction-controller/src/constants.ts +++ b/packages/transaction-controller/src/constants.ts @@ -83,3 +83,74 @@ export const ABI_SIMULATION_ERC721_LEGACY = [ type: 'event', }, ]; + +export const METHOD_REGISTRY_ABI = [ + { + constant: false, + inputs: [{ name: '_new', type: 'address' }], + name: 'setOwner', + outputs: [], + payable: false, + type: 'function', + }, + { + constant: true, + inputs: [], + name: 'totalSignatures', + outputs: [{ name: '', type: 'uint256' }], + payable: false, + type: 'function', + }, + { + constant: true, + inputs: [], + name: 'owner', + outputs: [{ name: '', type: 'address' }], + payable: false, + type: 'function', + }, + { + constant: false, + inputs: [], + name: 'drain', + outputs: [], + payable: false, + type: 'function', + }, + { + constant: true, + inputs: [{ name: '', type: 'bytes4' }], + name: 'entries', + outputs: [{ name: '', type: 'string' }], + payable: false, + type: 'function', + }, + { + constant: false, + inputs: [{ name: '_method', type: 'string' }], + name: 'register', + outputs: [{ name: '', type: 'bool' }], + payable: false, + type: 'function', + }, + { inputs: [], type: 'constructor' }, + { + anonymous: false, + inputs: [ + { indexed: true, name: 'creator', type: 'address' }, + { indexed: true, name: 'signature', type: 'bytes4' }, + { indexed: false, name: 'method', type: 'string' }, + ], + name: 'Registered', + type: 'event', + }, + { + anonymous: false, + inputs: [ + { indexed: true, name: 'old', type: 'address' }, + { indexed: true, name: 'current', type: 'address' }, + ], + name: 'NewOwner', + type: 'event', + }, +]; diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.ts index f15a5119c2e..d41fefb1ff5 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.ts @@ -6,82 +6,12 @@ import { Mutex } from 'async-mutex'; // eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; +import { METHOD_REGISTRY_ABI } from '../constants'; import { projectLogger } from '../logger'; import type { MethodData } from '../TransactionController'; const log = createModuleLogger(projectLogger, 'method-data'); -const methodRegistryAbi = [ - { - constant: false, - inputs: [{ name: '_new', type: 'address' }], - name: 'setOwner', - outputs: [], - payable: false, - type: 'function', - }, - { - constant: true, - inputs: [], - name: 'totalSignatures', - outputs: [{ name: '', type: 'uint256' }], - payable: false, - type: 'function', - }, - { - constant: true, - inputs: [], - name: 'owner', - outputs: [{ name: '', type: 'address' }], - payable: false, - type: 'function', - }, - { - constant: false, - inputs: [], - name: 'drain', - outputs: [], - payable: false, - type: 'function', - }, - { - constant: true, - inputs: [{ name: '', type: 'bytes4' }], - name: 'entries', - outputs: [{ name: '', type: 'string' }], - payable: false, - type: 'function', - }, - { - constant: false, - inputs: [{ name: '_method', type: 'string' }], - name: 'register', - outputs: [{ name: '', type: 'bool' }], - payable: false, - type: 'function', - }, - { inputs: [], type: 'constructor' }, - { - anonymous: false, - inputs: [ - { indexed: true, name: 'creator', type: 'address' }, - { indexed: true, name: 'signature', type: 'bytes4' }, - { indexed: false, name: 'method', type: 'string' }, - ], - name: 'Registered', - type: 'event', - }, - { - anonymous: false, - inputs: [ - { indexed: true, name: 'old', type: 'address' }, - { indexed: true, name: 'current', type: 'address' }, - ], - name: 'NewOwner', - type: 'event', - }, -]; - export class MethodDataHelper { hub: EventEmitter; @@ -117,7 +47,7 @@ export class MethodDataHelper { let iface = this.#interfaceByNetworkClientId.get(networkClientId); if (!iface) { - iface = new Interface(methodRegistryAbi); + iface = new Interface(METHOD_REGISTRY_ABI); this.#interfaceByNetworkClientId.set(networkClientId, iface); log('Created interface', networkClientId); } From 455ba549669123ae927dabf5c346588fdaa1101d Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 29 Jan 2025 16:44:36 +0100 Subject: [PATCH 5/5] refactor: remove Interface Mock --- .../src/helpers/MethodDataHelper.test.ts | 125 ++++++++---------- 1 file changed, 55 insertions(+), 70 deletions(-) diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts index 87922920adb..7e4b779a53f 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.test.ts @@ -1,44 +1,26 @@ -import { type FunctionFragment, Interface } from '@ethersproject/abi'; - import { MethodDataHelper } from './MethodDataHelper'; import type { MethodData } from '../TransactionController'; -jest.mock('@ethersproject/abi'); - const FOUR_BYTE_PREFIX_MOCK = '0x12345678'; const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId'; -const SIGNATURE_MOCK = 'testMethod(uint256,uint256)'; - -const METHOD_DATA_MOCK: MethodData = { - registryMethod: SIGNATURE_MOCK, - parsedRegistryMethod: { - name: 'testMethod', - args: [{ type: 'uint256' }, { type: 'uint256' }], - }, -}; - -/** - * Creates a mock Interface instance. - * - * @returns The mocked Interface instance. - */ -function createInterfaceMock() { - return { - getFunction: jest.fn(), - } as unknown as jest.Mocked; -} describe('MethodDataHelper', () => { - const interfaceClassMock = jest.mocked(Interface); - beforeEach(() => { jest.resetAllMocks(); }); describe('lookup', () => { it('returns method data from cache', async () => { + const cachedMethodData: MethodData = { + registryMethod: 'cached()', + parsedRegistryMethod: { + name: 'cached', + args: [], + }, + }; + const methodDataHelper = new MethodDataHelper({ - getState: () => ({ [FOUR_BYTE_PREFIX_MOCK]: METHOD_DATA_MOCK }), + getState: () => ({ [FOUR_BYTE_PREFIX_MOCK]: cachedMethodData }), }); const result = await methodDataHelper.lookup( @@ -46,39 +28,31 @@ describe('MethodDataHelper', () => { NETWORK_CLIENT_ID_MOCK, ); - expect(result).toStrictEqual(METHOD_DATA_MOCK); + expect(result).toStrictEqual(cachedMethodData); }); - it('returns method data from interface lookup', async () => { - const interfaceMock = createInterfaceMock(); - interfaceMock.getFunction.mockReturnValueOnce({ - name: 'testMethod', - inputs: [{ type: 'uint256' }, { type: 'uint256' }], - format: jest.fn(() => SIGNATURE_MOCK), - } as unknown as FunctionFragment); - - interfaceClassMock.mockReturnValueOnce(interfaceMock); - + it('correctly parses a known method signature', async () => { const methodDataHelper = new MethodDataHelper({ getState: () => ({}), }); + const fourBytePrefix = '0x13af4035'; + const result = await methodDataHelper.lookup( - FOUR_BYTE_PREFIX_MOCK, + fourBytePrefix, NETWORK_CLIENT_ID_MOCK, ); - expect(result).toStrictEqual(METHOD_DATA_MOCK); - }); - - it('returns empty method data if not found in interface', async () => { - const interfaceMock = createInterfaceMock(); - interfaceMock.getFunction.mockImplementationOnce(() => { - throw new Error('Function not found'); + expect(result).toStrictEqual({ + registryMethod: 'setOwner(address)', + parsedRegistryMethod: { + name: 'setOwner', + args: [{ type: 'address' }], + }, }); + }); - interfaceClassMock.mockReturnValueOnce(interfaceMock); - + it('returns empty result for unknown method signature', async () => { const methodDataHelper = new MethodDataHelper({ getState: () => ({}), }); @@ -95,12 +69,22 @@ describe('MethodDataHelper', () => { }); it('creates interface instance for each unique network client ID', async () => { - const interfaceMock = createInterfaceMock(); - interfaceMock.getFunction.mockImplementationOnce(() => { - throw new Error('Function not found'); - }); + const mockInterface = jest.fn().mockImplementation(() => ({ + getFunction: jest.fn().mockImplementation(() => { + throw new Error('Function not found'); + }), + })); + + jest.doMock('@ethersproject/abi', () => ({ + Interface: mockInterface, + })); - interfaceClassMock.mockReturnValueOnce(interfaceMock); + // Clear the module cache + jest.resetModules(); + + // Re-import dependencies after mocking + // eslint-disable-next-line @typescript-eslint/no-require-imports, n/global-require, @typescript-eslint/no-shadow + const { MethodDataHelper } = require('./MethodDataHelper'); const methodDataHelper = new MethodDataHelper({ getState: () => ({}), @@ -121,19 +105,17 @@ describe('MethodDataHelper', () => { 'anotherNetworkClientId', ); - expect(interfaceClassMock).toHaveBeenCalledTimes(2); - }); + await methodDataHelper.lookup( + FOUR_BYTE_PREFIX_MOCK, + 'anotherNetworkClientId', + ); - it('emits event when method data is fetched', async () => { - const interfaceMock = createInterfaceMock(); - interfaceMock.getFunction.mockReturnValueOnce({ - name: 'testMethod', - inputs: [{ type: 'uint256' }, { type: 'uint256' }], - format: jest.fn(() => SIGNATURE_MOCK), - } as unknown as FunctionFragment); + expect(mockInterface).toHaveBeenCalledTimes(2); - interfaceClassMock.mockReturnValueOnce(interfaceMock); + jest.unmock('@ethersproject/abi'); + }); + it('emits an update event for new lookups', async () => { const methodDataHelper = new MethodDataHelper({ getState: () => ({}), }); @@ -141,15 +123,18 @@ describe('MethodDataHelper', () => { const updateListener = jest.fn(); methodDataHelper.hub.on('update', updateListener); - await methodDataHelper.lookup( - FOUR_BYTE_PREFIX_MOCK, - NETWORK_CLIENT_ID_MOCK, - ); + const fourBytePrefix = '0x13af4035'; + await methodDataHelper.lookup(fourBytePrefix, NETWORK_CLIENT_ID_MOCK); - expect(updateListener).toHaveBeenCalledTimes(1); expect(updateListener).toHaveBeenCalledWith({ - fourBytePrefix: FOUR_BYTE_PREFIX_MOCK, - methodData: METHOD_DATA_MOCK, + fourBytePrefix, + methodData: { + registryMethod: 'setOwner(address)', + parsedRegistryMethod: { + name: 'setOwner', + args: [{ type: 'address' }], + }, + }, }); }); });