diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 32ef301339c..7c2f62b2c72 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Reduce the cost of EIP-7702 capability checks by removing redundant contract signature verification ([#10397](https://github.com/MetaMask/core/pull/10397)) + - Verification results are now memoized, so each configured contract is verified at most once rather than on every lookup. + - `getEIP7702UpgradeContractAddress` now stops verifying once an authentic contract is found, instead of verifying every contract configured for the chain. + ## [71.0.0] ### Added diff --git a/packages/transaction-controller/src/utils/feature-flags.test.ts b/packages/transaction-controller/src/utils/feature-flags.test.ts index bbbac8a27f8..a803139dccf 100644 --- a/packages/transaction-controller/src/utils/feature-flags.test.ts +++ b/packages/transaction-controller/src/utils/feature-flags.test.ts @@ -334,6 +334,27 @@ describe('Feature Flags Utils', () => { ), ).toStrictEqual(ADDRESS_2_MOCK); }); + + it('does not validate remaining contracts once an authentic contract is found', () => { + mockFeatureFlags({ + [FeatureFlag.EIP7702]: { + contracts: { + [CHAIN_ID_MOCK]: [ + { address: ADDRESS_MOCK, signature: SIGNATURE_MOCK }, + { address: ADDRESS_2_MOCK, signature: SIGNATURE_MOCK }, + ], + }, + }, + }); + + getEIP7702UpgradeContractAddress( + CHAIN_ID_MOCK, + controllerMessenger, + PUBLIC_KEY_MOCK, + ); + + expect(isValidSignatureMock).toHaveBeenCalledTimes(1); + }); }); describe('getBatchSizeLimit', () => { diff --git a/packages/transaction-controller/src/utils/feature-flags.ts b/packages/transaction-controller/src/utils/feature-flags.ts index a1c4d215813..c8f469d526c 100644 --- a/packages/transaction-controller/src/utils/feature-flags.ts +++ b/packages/transaction-controller/src/utils/feature-flags.ts @@ -44,6 +44,15 @@ type GasEstimateFallback = { maxGasLimit?: number; }; +/** A smart contract that supports EIP-7702 batch transactions. */ +type EIP7702Contract = { + /** Address of the smart contract. */ + address: Hex; + + /** Signature to verify the contract is authentic. */ + signature: Hex; +}; + export type TransactionControllerFeatureFlags = { /** Feature flags to support EIP-7702 / type-4 transactions. */ [FeatureFlag.EIP7702]?: { @@ -52,16 +61,7 @@ export type TransactionControllerFeatureFlags = { * Keyed by chain ID. * First entry in each array is the contract that standard EOAs will be upgraded to. */ - contracts?: Record< - Hex, - { - /** Address of the smart contract. */ - address: Hex; - - /** Signature to verify the contract is authentic. */ - signature: Hex; - }[] - >; + contracts?: Record; /** Chains enabled for EIP-7702 batch transactions. */ supportedChains?: Hex[]; @@ -257,27 +257,18 @@ export function getEIP7702ContractAddresses( messenger: TransactionControllerMessenger, publicKey: Hex, ): Hex[] { - const featureFlags = getFeatureFlags(messenger); - - const contracts = - featureFlags?.[FeatureFlag.EIP7702]?.contracts?.[ - chainId.toLowerCase() as Hex - ] ?? []; - - return contracts - .filter((contract) => - isValidSignature( - [contract.address, padHexToEvenLength(chainId) as Hex], - contract.signature, - publicKey, - ), - ) + return getEIP7702Contracts(chainId, messenger) + .filter((contract) => isContractAuthentic(contract, chainId, publicKey)) .map((contract) => contract.address); } /** * Retrieves the EIP-7702 upgrade contract address. * + * Equivalent to the first entry of `getEIP7702ContractAddresses`, but stops + * validating once an authentic contract is found rather than validating every + * contract for the chain. + * * @param chainId - The chain ID. * @param messenger - The controller messenger instance. * @param publicKey - The public key used to validate the contract authenticity. @@ -288,7 +279,9 @@ export function getEIP7702UpgradeContractAddress( messenger: TransactionControllerMessenger, publicKey: Hex, ): Hex | undefined { - return getEIP7702ContractAddresses(chainId, messenger, publicKey)?.[0]; + return getEIP7702Contracts(chainId, messenger).find((contract) => + isContractAuthentic(contract, chainId, publicKey), + )?.address; } /** @@ -553,6 +546,48 @@ export function getReplaceUnderpricedSavedGasFeesEnabled( ); } +/** + * Retrieves the configured EIP-7702 contracts for a given chain ID, without + * validating their authenticity. + * + * @param chainId - The chain ID. + * @param messenger - The controller messenger instance. + * @returns The configured contracts. + */ +function getEIP7702Contracts( + chainId: Hex, + messenger: TransactionControllerMessenger, +): EIP7702Contract[] { + const featureFlags = getFeatureFlags(messenger); + + return ( + featureFlags?.[FeatureFlag.EIP7702]?.contracts?.[ + chainId.toLowerCase() as Hex + ] ?? [] + ); +} + +/** + * Determines whether an EIP-7702 contract is authentic by verifying its + * signature against the given public key. + * + * @param contract - The contract to validate. + * @param chainId - The chain ID the contract is configured for. + * @param publicKey - The public key used to validate the contract authenticity. + * @returns True if the contract signature is valid, false otherwise. + */ +function isContractAuthentic( + contract: EIP7702Contract, + chainId: Hex, + publicKey: Hex, +): boolean { + return isValidSignature( + [contract.address, padHexToEvenLength(chainId) as Hex], + contract.signature, + publicKey, + ); +} + /** * Retrieves the relevant feature flags from the remote feature flag controller. * diff --git a/packages/transaction-controller/src/utils/signature.test.ts b/packages/transaction-controller/src/utils/signature.test.ts index 346c1ae45a8..3348984c214 100644 --- a/packages/transaction-controller/src/utils/signature.test.ts +++ b/packages/transaction-controller/src/utils/signature.test.ts @@ -1,4 +1,19 @@ -import { isValidSignature } from './signature.js'; +import { verifyMessage } from '@ethersproject/wallet'; + +import { + MAX_CACHE_SIZE, + clearSignatureCache, + isValidSignature, +} from './signature.js'; + +jest.mock('@ethersproject/wallet', () => ({ + ...jest.requireActual('@ethersproject/wallet'), + verifyMessage: jest.fn(), +})); + +const { verifyMessage: actualVerifyMessage } = jest.requireActual< + typeof import('@ethersproject/wallet') +>('@ethersproject/wallet'); const VALUE_1_MOCK = '0x12345678'; const VALUE_2_MOCK = '0xabcabcabcabcabcabcabcabc'; @@ -13,6 +28,14 @@ const SIGNATURE_MULTIPLE_MOCK = '0x777b4f4461009b937de6a5ea7b0640c783433ad2cb50b0d0822a9ce9cadea12c5614b47927ddffc2e855b6932adb6a6e0a558667fd188eaf120a153c6b693dcb1b'; describe('Signature Utils', () => { + const verifyMessageMock = jest.mocked(verifyMessage); + + beforeEach(() => { + // `resetMocks` clears the implementation between tests. + verifyMessageMock.mockImplementation(actualVerifyMessage); + clearSignatureCache(); + }); + describe('isValidSignature', () => { it('returns true if signature correct and single data', async () => { expect( @@ -49,5 +72,99 @@ describe('Signature Utils', () => { ), ).toBe(false); }); + + it('verifies once only if called repeatedly with identical arguments', () => { + const results = [1, 2, 3].map(() => + isValidSignature( + [VALUE_1_MOCK], + SIGNATURE_SINGLE_MOCK, + PUBLIC_KEY_MOCK, + ), + ); + + expect(results).toStrictEqual([true, true, true]); + expect(verifyMessageMock).toHaveBeenCalledTimes(1); + }); + + it('caches negative results', () => { + const results = [1, 2, 3].map(() => + isValidSignature(['0x123'], SIGNATURE_SINGLE_MOCK, PUBLIC_KEY_MOCK), + ); + + expect(results).toStrictEqual([false, false, false]); + expect(verifyMessageMock).toHaveBeenCalledTimes(1); + }); + + it('verifies again if data differs', () => { + isValidSignature([VALUE_1_MOCK], SIGNATURE_SINGLE_MOCK, PUBLIC_KEY_MOCK); + isValidSignature([VALUE_2_MOCK], SIGNATURE_SINGLE_MOCK, PUBLIC_KEY_MOCK); + + expect(verifyMessageMock).toHaveBeenCalledTimes(2); + }); + + it('verifies again if signature differs', () => { + isValidSignature([VALUE_1_MOCK], SIGNATURE_SINGLE_MOCK, PUBLIC_KEY_MOCK); + isValidSignature( + [VALUE_1_MOCK], + SIGNATURE_MULTIPLE_MOCK, + PUBLIC_KEY_MOCK, + ); + + expect(verifyMessageMock).toHaveBeenCalledTimes(2); + }); + + it('does not return a cached result if public key differs', () => { + const first = isValidSignature( + [VALUE_1_MOCK], + SIGNATURE_SINGLE_MOCK, + PUBLIC_KEY_MOCK, + ); + + const second = isValidSignature( + [VALUE_1_MOCK], + SIGNATURE_SINGLE_MOCK, + '0x1234567890123456789012345678901234567890', + ); + + expect(first).toBe(true); + expect(second).toBe(false); + expect(verifyMessageMock).toHaveBeenCalledTimes(2); + }); + + it('clears the cache once it reaches the maximum size', () => { + verifyMessageMock.mockReturnValue(PUBLIC_KEY_MOCK); + + isValidSignature([VALUE_1_MOCK], SIGNATURE_SINGLE_MOCK, PUBLIC_KEY_MOCK); + + for (let index = 0; index < MAX_CACHE_SIZE; index++) { + isValidSignature( + [`0x${index}`], + SIGNATURE_SINGLE_MOCK, + PUBLIC_KEY_MOCK, + ); + } + + verifyMessageMock.mockClear(); + + isValidSignature([VALUE_1_MOCK], SIGNATURE_SINGLE_MOCK, PUBLIC_KEY_MOCK); + + expect(verifyMessageMock).toHaveBeenCalledTimes(1); + }); + + it('distinguishes data entries that concatenate identically', () => { + isValidSignature( + [VALUE_1_MOCK, VALUE_2_MOCK], + SIGNATURE_MULTIPLE_MOCK, + PUBLIC_KEY_MOCK, + ); + + isValidSignature( + [`${VALUE_1_MOCK}${VALUE_2_MOCK.slice(2)}` as never], + SIGNATURE_MULTIPLE_MOCK, + PUBLIC_KEY_MOCK, + ); + + expect(verifyMessageMock).toHaveBeenCalledTimes(2); + }); }); }); diff --git a/packages/transaction-controller/src/utils/signature.ts b/packages/transaction-controller/src/utils/signature.ts index 3b447d0c8e6..13395ae58be 100644 --- a/packages/transaction-controller/src/utils/signature.ts +++ b/packages/transaction-controller/src/utils/signature.ts @@ -2,9 +2,31 @@ import { verifyMessage } from '@ethersproject/wallet'; import type { Hex } from '@metamask/utils'; import { add0x, hexToBytes, remove0x } from '@metamask/utils'; +/** + * Maximum number of cached signature verification results. + * + * Sized well above the expected working set (one entry per signed contract per + * chain in the EIP-7702 feature flag) so the cache never evicts in practice, + * while still bounding memory if the flag data ever grows unexpectedly. + */ +export const MAX_CACHE_SIZE = 1000; + +/** + * Cache of signature verification results, keyed by data, signature, and + * public key. + * + * Verification is a pure function of those three inputs, so a cached result can + * never become stale and requires no invalidation. New feature flag data simply + * produces new keys. + */ +const verificationCache = new Map(); + /** * Verify if the signature is the specified data signed by the specified public key. * + * Results are memoized as the underlying elliptic curve recovery is expensive + * and is otherwise repeated for identical inputs on hot paths. + * * @param data - The data to check. * @param signature - The signature to check. * @param publicKey - The public key to check. @@ -15,6 +37,42 @@ export function isValidSignature( signature: Hex, publicKey: Hex, ): boolean { + const cacheKey = `${data.join(',')}:${signature}:${publicKey}`; + const cachedResult = verificationCache.get(cacheKey); + + if (cachedResult !== undefined) { + return cachedResult; + } + + const result = verifySignature(data, signature, publicKey); + + if (verificationCache.size >= MAX_CACHE_SIZE) { + verificationCache.clear(); + } + + verificationCache.set(cacheKey, result); + + return result; +} + +/** + * Clear the signature verification cache. + * + * Intended for tests only, as cached results can never become stale. + */ +export function clearSignatureCache(): void { + verificationCache.clear(); +} + +/** + * Verify if the signature is the specified data signed by the specified public key. + * + * @param data - The data to check. + * @param signature - The signature to check. + * @param publicKey - The public key to check. + * @returns True if the signature is correct, false otherwise. + */ +function verifySignature(data: Hex[], signature: Hex, publicKey: Hex): boolean { try { const joinedHex = add0x(data.map(remove0x).join('')); const dataBytes = hexToBytes(joinedHex);