Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions packages/transaction-controller/src/utils/feature-flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
87 changes: 61 additions & 26 deletions packages/transaction-controller/src/utils/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]?: {
Expand All @@ -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<Hex, EIP7702Contract[]>;

/** Chains enabled for EIP-7702 batch transactions. */
supportedChains?: Hex[];
Expand Down Expand Up @@ -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.
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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.
*
Expand Down
119 changes: 118 additions & 1 deletion packages/transaction-controller/src/utils/signature.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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(
Expand Down Expand Up @@ -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);
});
});
});
58 changes: 58 additions & 0 deletions packages/transaction-controller/src/utils/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, boolean>();

/**
* 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.
Expand All @@ -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);
Expand Down
Loading