diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 4b2327c9356..7b7293e3da1 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `authorizationList` to `TransactionBatchRequest` so callers can include pre-signed EIP-7702 authorizations (e.g. Money Account upgrades) alongside any upgrade authorization generated for the batch payer (`from`) ([#9765](https://github.com/MetaMask/core/pull/9765)) + ### Changed - Bump `@metamask/network-controller` from `^35.0.0` to `^35.0.1` ([#9758](https://github.com/MetaMask/core/pull/9758)) @@ -24,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Refresh gas fee token quotes independently of balance changes for enforced-simulation transactions, and prevent stale simulation responses from overwriting newer transaction state. ([#9757](https://github.com/MetaMask/core/pull/9757)) +- Retain pre-signed entries in `signAuthorizationList` instead of re-signing them with `txParams.from`, so authorizations signed by a different account survive publish ([#9765](https://github.com/MetaMask/core/pull/9765)) +- Attribute EIP-7702 authorization nonces to their recovered authorities in the nonce tracker, so foreign authorizations (e.g. Money Account upgrades on same-chain pay batches) do not inflate the batch payer's pending nonce ([#9765](https://github.com/MetaMask/core/pull/9765)) ## [69.4.0] diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 2c9c2b8cb33..dfe3e709410 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -56,6 +56,7 @@ }, "dependencies": { "@ethereumjs/common": "^4.4.0", + "@ethereumjs/rlp": "^5.0.2", "@ethereumjs/tx": "^5.4.0", "@ethereumjs/util": "^9.1.0", "@ethersproject/abi": "^5.7.0", @@ -78,6 +79,7 @@ "bignumber.js": "^9.1.2", "bn.js": "^5.2.1", "eth-method-registry": "^4.0.0", + "ethereum-cryptography": "^2.1.2", "fast-json-patch": "^3.1.1", "lodash": "^4.17.21", "uuid": "^8.3.2" diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 96cd15dd08c..83b19561560 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -1831,6 +1831,16 @@ export type TransactionBatchRequest = { */ atomic?: boolean; + /** + * Pre-signed or unsigned EIP-7702 authorizations to include on the batch + * type-4 transaction, in addition to any upgrade authorization generated for + * `from` when the batch payer is not yet upgraded. + * + * Used when vaulting/delegation requires upgrading a different account than + * `from` (e.g. Money Account deposits paid by an EOA with account override). + */ + authorizationList?: AuthorizationList; + batchId?: Hex; /** Whether to disable batch transaction processing via an EIP-7702 upgraded account. */ diff --git a/packages/transaction-controller/src/utils/batch.test.ts b/packages/transaction-controller/src/utils/batch.test.ts index a642704bc7d..b546577fff1 100644 --- a/packages/transaction-controller/src/utils/batch.test.ts +++ b/packages/transaction-controller/src/utils/batch.test.ts @@ -830,6 +830,87 @@ describe('Batch Utils', () => { ); }); + it('merges provided authorizationList with from upgrade authorization', async () => { + isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ + delegationAddress: undefined, + isSupported: false, + }); + + addTransactionMock.mockResolvedValueOnce({ + transactionMeta: TRANSACTION_META_MOCK, + result: Promise.resolve(''), + }); + + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); + + getEIP7702UpgradeContractAddressMock.mockReturnValueOnce( + CONTRACT_ADDRESS_MOCK, + ); + + const providedAuthorization = { + address: '0x1234567890123456789012345678901234567890' as const, + chainId: '0x1' as const, + nonce: '0x5' as const, + r: '0xabc' as const, + s: '0xdef' as const, + yParity: '0x1' as const, + }; + + request.request.authorizationList = [providedAuthorization]; + + await addTransactionBatch(request); + + expect(addTransactionMock).toHaveBeenCalledWith( + expect.objectContaining({ + type: TransactionEnvelopeType.setCode, + authorizationList: [ + { address: CONTRACT_ADDRESS_MOCK }, + providedAuthorization, + ], + }), + expect.anything(), + ); + }); + + it('includes provided authorizationList when from is already upgraded', async () => { + isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ + delegationAddress: CONTRACT_ADDRESS_MOCK, + isSupported: true, + }); + + addTransactionMock.mockResolvedValueOnce({ + transactionMeta: TRANSACTION_META_MOCK, + result: Promise.resolve(''), + }); + + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); + + const providedAuthorization = { + address: '0x1234567890123456789012345678901234567890' as const, + chainId: '0x1' as const, + nonce: '0x5' as const, + r: '0xabc' as const, + s: '0xdef' as const, + yParity: '0x1' as const, + }; + + request.request.authorizationList = [providedAuthorization]; + + await addTransactionBatch(request); + + expect(addTransactionMock).toHaveBeenCalledWith( + expect.objectContaining({ + type: TransactionEnvelopeType.setCode, + authorizationList: [providedAuthorization], + }), + expect.anything(), + ); + }); + it('does not use type 4 if not upgraded but disableUpgrade set', async () => { isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, @@ -1390,6 +1471,47 @@ describe('Batch Utils', () => { CHAIN_ID_MOCK, ); }); + + it('does not use provided foreign authorization as delegation mock when from is upgraded', async () => { + isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ + delegationAddress: CONTRACT_ADDRESS_MOCK, + isSupported: true, + }); + + addTransactionMock.mockResolvedValueOnce({ + transactionMeta: TRANSACTION_META_MOCK, + result: Promise.resolve(''), + }); + + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); + + const providedAuthorization = { + address: '0x1234567890123456789012345678901234567890' as const, + chainId: '0x1' as const, + nonce: '0x5' as const, + r: '0xabc' as const, + s: '0xdef' as const, + yParity: '0x1' as const, + }; + + request.request.authorizationList = [providedAuthorization]; + + const validateSecurityMock = jest.fn(); + validateSecurityMock.mockResolvedValueOnce({}); + + request.request.validateSecurity = validateSecurityMock; + + await addTransactionBatch(request); + + expect(validateSecurityMock).toHaveBeenCalledWith( + expect.objectContaining({ + delegationMock: undefined, + }), + CHAIN_ID_MOCK, + ); + }); }); describe('with publish batch hook', () => { diff --git a/packages/transaction-controller/src/utils/batch.ts b/packages/transaction-controller/src/utils/batch.ts index 268f1885b9d..5978fbf2778 100644 --- a/packages/transaction-controller/src/utils/batch.ts +++ b/packages/transaction-controller/src/utils/batch.ts @@ -36,6 +36,7 @@ import type { import { projectLogger } from '../logger.js'; import { TransactionEnvelopeType, TransactionType } from '../types.js'; import type { + AuthorizationList, NestedTransactionMetadata, SecurityAlertResponse, TransactionBatchSingleRequest, @@ -282,6 +283,58 @@ async function getNestedTransactionMeta( }; } +/** + * Build the authorization list for an EIP-7702 batch transaction. + * + * When the batch payer (`from`) requires an upgrade, an unsigned upgrade + * authorization for that account is included first. Any caller-provided + * authorizations (e.g. a pre-signed Money Account upgrade) are appended so + * both can be submitted on the same type-4 transaction. + * + * @param options - Options bag. + * @param options.chainId - Chain ID of the batch. + * @param options.messenger - Controller messenger. + * @param options.providedAuthorizationList - Optional authorizations from the batch request. + * @param options.publicKeyEIP7702 - Public key used to resolve the upgrade contract. + * @param options.requiresUpgrade - Whether the batch payer requires an EIP-7702 upgrade. + * @returns The combined authorization list, or undefined when none are needed. + */ +function buildBatchAuthorizationList({ + chainId, + messenger, + providedAuthorizationList, + publicKeyEIP7702, + requiresUpgrade, +}: { + chainId: Hex; + messenger: TransactionControllerMessenger; + providedAuthorizationList?: AuthorizationList; + publicKeyEIP7702: Hex; + requiresUpgrade: boolean; +}): AuthorizationList | undefined { + const authorizationList: AuthorizationList = []; + + if (requiresUpgrade) { + const upgradeContractAddress = getEIP7702UpgradeContractAddress( + chainId, + messenger, + publicKeyEIP7702, + ); + + if (!upgradeContractAddress) { + throw rpcErrors.internal(ERROR_MESSAGE_NO_UPGRADE_CONTRACT); + } + + authorizationList.push({ address: upgradeContractAddress }); + } + + if (providedAuthorizationList?.length) { + authorizationList.push(...providedAuthorizationList); + } + + return authorizationList.length ? authorizationList : undefined; +} + /** * Process a batch transaction using an EIP-7702 transaction. * @@ -300,6 +353,7 @@ async function addTransactionBatchWith7702( const { atomic, + authorizationList: providedAuthorizationList, batchId: batchIdOverride, disableUpgrade, from, @@ -382,22 +436,23 @@ async function addTransactionBatchWith7702( maxPriorityFeePerGas: nestedTransactions[0]?.maxPriorityFeePerGas, }; - if (requiresUpgrade) { - const upgradeContractAddress = getEIP7702UpgradeContractAddress( - chainId, - messenger, - publicKeyEIP7702, - ); - - if (!upgradeContractAddress) { - throw rpcErrors.internal(ERROR_MESSAGE_NO_UPGRADE_CONTRACT); - } + const authorizationList = buildBatchAuthorizationList({ + chainId, + messenger, + providedAuthorizationList, + publicKeyEIP7702, + requiresUpgrade, + }); + if (authorizationList?.length) { txParams.type = TransactionEnvelopeType.setCode; - txParams.authorizationList = [{ address: upgradeContractAddress }]; + txParams.authorizationList = authorizationList; } if (validateSecurity) { + // `delegationMock` applies to the batch payer (`from`) only. When + // `requiresUpgrade` is true, that upgrade authorization is always first in + // the list. Caller-provided auths for other accounts must not be used here. const securityRequest: ValidateSecurityRequest = { method: 'eth_sendTransaction', params: [ @@ -407,7 +462,9 @@ async function addTransactionBatchWith7702( type: TransactionEnvelopeType.feeMarket, }, ], - delegationMock: txParams.authorizationList?.[0]?.address, + delegationMock: requiresUpgrade + ? authorizationList?.[0]?.address + : undefined, origin, }; diff --git a/packages/transaction-controller/src/utils/eip7702.test.ts b/packages/transaction-controller/src/utils/eip7702.test.ts index 85a025a74fc..e7a33c26cf8 100644 --- a/packages/transaction-controller/src/utils/eip7702.test.ts +++ b/packages/transaction-controller/src/utils/eip7702.test.ts @@ -22,8 +22,10 @@ import { doesAccountSupportEIP7702, doesChainSupportEIP7702, generateEIP7702BatchTransaction, + getAuthorizationAuthority, getDelegationAddress, isAccountUpgradedToEIP7702, + recoverAuthorizationAuthority, signAuthorizationList, updateEIP7702BatchData, } from './eip7702.js'; @@ -282,6 +284,96 @@ describe('EIP-7702 Utils', () => { ); expect(result?.[0]?.yParity).toBe('0x1'); }); + + it('retains pre-signed authorizations without re-signing', async () => { + const preSignedAuthorization = { + address: AUTHORIZATION_LIST_MOCK[0].address, + chainId: AUTHORIZATION_LIST_MOCK[0].chainId, + nonce: AUTHORIZATION_LIST_MOCK[0].nonce, + r: '0xpreSignedR' as Hex, + s: '0xpreSignedS' as Hex, + yParity: '0x0' as Hex, + }; + + const result = await signAuthorizationList({ + authorizationList: [ + preSignedAuthorization, + { address: AUTHORIZATION_LIST_MOCK[0].address }, + ], + messenger: controllerMessenger, + transactionMeta: TRANSACTION_META_MOCK, + }); + + expect(signAuthorizationMock).toHaveBeenCalledTimes(1); + expect(result).toStrictEqual([ + preSignedAuthorization, + { + address: AUTHORIZATION_LIST_MOCK[0].address, + chainId: TRANSACTION_META_MOCK.chainId, + nonce: '0x125', + r: '0xf85c827a6994663f3ad617193148711d28f5334ee4ed070166028080a040e292', + s: '0xda533253143f134643a03405f1af1de1d305526f44ed27e62061368d4ea051cf', + yParity: '0x1', + }, + ]); + }); + }); + + describe('recoverAuthorizationAuthority', () => { + it('recovers the signer of a valid EIP-7702 authorization', () => { + const authorization = { + address: '0xcccccccccccccccccccccccccccccccccccccccc' as Hex, + chainId: '0x1' as Hex, + nonce: '0x6' as Hex, + r: '0xe2582357434f268c18dd7e2920dd3a911bfc6fc5e498bf7eb33fa0f484bd1488' as Hex, + s: '0x6300c28d38a634904af92933b5b31433536a7cee8a505945c13ade9f3a1a5427' as Hex, + yParity: '0x0' as Hex, + }; + + expect(recoverAuthorizationAuthority(authorization)).toBe( + '0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266', + ); + }); + + it('returns undefined for an invalid signature', () => { + const authorization = { + address: '0xcccccccccccccccccccccccccccccccccccccccc' as Hex, + chainId: '0x1' as Hex, + nonce: '0x6' as Hex, + r: '0x0' as Hex, + s: '0x0' as Hex, + yParity: '0x0' as Hex, + }; + + expect(recoverAuthorizationAuthority(authorization)).toBeUndefined(); + }); + }); + + describe('getAuthorizationAuthority', () => { + it('returns the transaction sender for unsigned authorizations', () => { + expect( + getAuthorizationAuthority( + { address: '0xcccccccccccccccccccccccccccccccccccccccc' }, + '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266', + ), + ).toBe('0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266'); + }); + + it('recovers the signer for signed authorizations', () => { + expect( + getAuthorizationAuthority( + { + address: '0xcccccccccccccccccccccccccccccccccccccccc', + chainId: '0x1', + nonce: '0x6', + r: '0xe2582357434f268c18dd7e2920dd3a911bfc6fc5e498bf7eb33fa0f484bd1488', + s: '0x6300c28d38a634904af92933b5b31433536a7cee8a505945c13ade9f3a1a5427', + yParity: '0x0', + }, + '0xother', + ), + ).toBe('0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266'); + }); }); describe('decodeAuthorizationSignature', () => { diff --git a/packages/transaction-controller/src/utils/eip7702.ts b/packages/transaction-controller/src/utils/eip7702.ts index 0a99859a56b..f92a4b0eb37 100644 --- a/packages/transaction-controller/src/utils/eip7702.ts +++ b/packages/transaction-controller/src/utils/eip7702.ts @@ -1,9 +1,18 @@ +import { RLP } from '@ethereumjs/rlp'; +import { + bigIntToUnpaddedBytes, + bytesToHex, + ecrecover, + hexToBytes, + pubToAddress, +} from '@ethereumjs/util'; import { defaultAbiCoder } from '@ethersproject/abi'; import { Contract } from '@ethersproject/contracts'; import { toHex } from '@metamask/controller-utils'; import type { NetworkClientId } from '@metamask/network-controller'; import { createModuleLogger, add0x } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; +import { keccak256 } from 'ethereum-cryptography/keccak'; import { ABI_IERC7821 } from '../constants.js'; import { projectLogger } from '../logger.js'; @@ -26,6 +35,9 @@ export const BATCH_FUNCTION_NAME = 'execute'; export const CALLS_SIGNATURE = '(address,uint256,bytes)[]'; export const ERROR_MESSGE_PUBLIC_KEY = 'EIP-7702 public key not specified'; +/** EIP-7702 authorization signature magic prefix. */ +const EIP7702_AUTHORIZATION_MAGIC = 0x05; + /** * ERC-7579 ModeCode encoding for the ERC-7821 `execute` function. * @@ -362,6 +374,13 @@ async function signAuthorization( messenger: TransactionControllerMessenger, index: number, ): Promise> { + // Retain pre-signed authorizations (e.g. Money Account upgrades signed by an + // account other than `txParams.from`) instead of re-signing with `from`. + if (isAuthorizationSigned(authorization)) { + log('Retaining pre-signed authorization', authorization); + return authorization; + } + const finalAuthorization = prepareAuthorization( authorization, transactionMeta, @@ -400,6 +419,78 @@ async function signAuthorization( return result; } +/** + * Whether an authorization already includes a complete EIP-7702 signature. + * + * @param authorization - Authorization to check. + * @returns True when chainId, nonce, and signature components are all present. + */ +export function isAuthorizationSigned( + authorization: Authorization, +): authorization is Required { + return Boolean( + authorization.chainId && + authorization.nonce !== undefined && + authorization.r && + authorization.s && + authorization.yParity !== undefined, + ); +} + +/** + * Recover the authority (signer) of a fully signed EIP-7702 authorization. + * + * Per EIP-7702: `authority = ecrecover(keccak(MAGIC || rlp([chain_id, address, nonce])), y_parity, r, s)`. + * + * @param authorization - Fully signed authorization. + * @returns The recovered authority address, or undefined if recovery fails. + */ +export function recoverAuthorizationAuthority( + authorization: Required, +): Hex | undefined { + try { + const chainIdBytes = bigIntToUnpaddedBytes(BigInt(authorization.chainId)); + const addressBytes = hexToBytes(authorization.address); + const nonceBytes = bigIntToUnpaddedBytes(BigInt(authorization.nonce)); + const rlpEncoded = RLP.encode([chainIdBytes, addressBytes, nonceBytes]); + const messageHash = keccak256( + Uint8Array.from([EIP7702_AUTHORIZATION_MAGIC, ...rlpEncoded]), + ); + const publicKey = ecrecover( + messageHash, + BigInt(authorization.yParity), + bigIntToUnpaddedBytes(BigInt(authorization.r)), + bigIntToUnpaddedBytes(BigInt(authorization.s)), + ); + + return bytesToHex(pubToAddress(publicKey)); + } catch (error) { + log('Failed to recover authorization authority', { authorization, error }); + return undefined; + } +} + +/** + * Resolve which account an authorization nonce belongs to for nonce tracking. + * + * Unsigned authorizations are signed by the outer transaction sender (`from`). + * Signed authorizations recover their authority independently per EIP-7702. + * + * @param authorization - Authorization entry. + * @param transactionFrom - Outer transaction sender. + * @returns Authority address, or undefined when a signed auth cannot be recovered. + */ +export function getAuthorizationAuthority( + authorization: Authorization, + transactionFrom: string, +): Hex | undefined { + if (!isAuthorizationSigned(authorization)) { + return transactionFrom as Hex; + } + + return recoverAuthorizationAuthority(authorization); +} + /** * Prepares an authorization for signing by populating the chainId and nonce. * diff --git a/packages/transaction-controller/src/utils/nonce.test.ts b/packages/transaction-controller/src/utils/nonce.test.ts index f1355aaa51e..db8fd3f37b0 100644 --- a/packages/transaction-controller/src/utils/nonce.test.ts +++ b/packages/transaction-controller/src/utils/nonce.test.ts @@ -2,8 +2,9 @@ import type { NonceLock, Transaction as NonceTrackerTransaction, } from '@metamask/nonce-tracker'; +import type { Hex } from '@metamask/utils'; -import type { TransactionMeta } from '../types.js'; +import type { Authorization, TransactionMeta } from '../types.js'; import { TransactionStatus } from '../types.js'; import { getAndFormatTransactionsForNonceTracker, @@ -21,6 +22,28 @@ const TRANSACTION_META_MOCK: TransactionMeta = { }, }; +// Signed by 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 (Hardhat account #0) +const EOA_ADDRESS = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266'; +const EOA_AUTHORIZATION: Authorization = { + address: '0xcccccccccccccccccccccccccccccccccccccccc', + chainId: '0x1', + nonce: '0x6', + r: '0xe2582357434f268c18dd7e2920dd3a911bfc6fc5e498bf7eb33fa0f484bd1488', + s: '0x6300c28d38a634904af92933b5b31433536a7cee8a505945c13ade9f3a1a5427', + yParity: '0x0', +}; + +// Signed by 0x70997970C51812dc3A010C7d01b50e0d17dc79C8 (Hardhat account #1) +const MONEY_ACCOUNT_ADDRESS = '0x70997970C51812dc3A010C7d01b50e0d17dc79C8'; +const MONEY_ACCOUNT_AUTHORIZATION: Authorization = { + address: '0xdddddddddddddddddddddddddddddddddddddddd', + chainId: '0x1', + nonce: '0x14', + r: '0x5d89be0904cbf8acd68969ac2a9974f8d959418b9386a65bf3477f0a57f07e01', + s: '0x1ad94d72ad05346cfc97f71b35d0d1b569ccc8fd04f3001fc389f0f2d1348cce', + yParity: '0x1', +}; + describe('nonce', () => { describe('getNextNonce', () => { it('returns custom nonce if provided', async () => { @@ -219,8 +242,7 @@ describe('nonce', () => { expect(result).toStrictEqual(expectedResult); }); - it('includes authorization nonces from authorizationList', () => { - const fromAddress = '0x123'; + it('includes unsigned authorization nonces attributed to the transaction sender', () => { const inputTransactions: TransactionMeta[] = [ { id: '1', @@ -228,26 +250,18 @@ describe('nonce', () => { networkClientId: 'testNetworkClientId', time: 123456, txParams: { - from: fromAddress, + from: EOA_ADDRESS, gas: '0x100', value: '0x200', nonce: '0x1', authorizationList: [ { - chainId: '0x1', - address: '0xabc', + address: '0xabc' as Hex, nonce: '0x2', - r: '0x0', - s: '0x0', - yParity: '0x0', }, { - chainId: '0x1', - address: '0xdef', + address: '0xdef' as Hex, nonce: '0x3', - r: '0x0', - s: '0x0', - yParity: '0x0', }, ], }, @@ -257,18 +271,112 @@ describe('nonce', () => { const result = getAndFormatTransactionsForNonceTracker( '0x1', - fromAddress, + EOA_ADDRESS, [TransactionStatus.confirmed], inputTransactions, ); expect(result).toHaveLength(3); - expect(result[0].txParams.nonce).toBe('0x1'); - expect(result[1].txParams.nonce).toBe('0x2'); - expect(result[2].txParams.nonce).toBe('0x3'); - expect(result[0].status).toBe(TransactionStatus.confirmed); - expect(result[1].status).toBe(TransactionStatus.confirmed); - expect(result[2].status).toBe(TransactionStatus.confirmed); + expect(result.map((tx) => tx.txParams.nonce)).toStrictEqual([ + '0x1', + '0x2', + '0x3', + ]); + expect(result.map((tx) => tx.txParams.from)).toStrictEqual([ + EOA_ADDRESS, + EOA_ADDRESS, + EOA_ADDRESS, + ]); + }); + + it('attributes mixed-authority authorization nonces to recovered signers only', () => { + const inputTransactions: TransactionMeta[] = [ + { + id: '1', + chainId: '0x1', + networkClientId: 'testNetworkClientId', + time: 123456, + txParams: { + from: EOA_ADDRESS, + gas: '0x100', + value: '0x200', + // EOA tx nonce 5, EOA auth nonce 6, Money Account auth nonce 20 + nonce: '0x5', + authorizationList: [EOA_AUTHORIZATION, MONEY_ACCOUNT_AUTHORIZATION], + }, + status: TransactionStatus.submitted, + }, + ]; + + const eoaResult = getAndFormatTransactionsForNonceTracker( + '0x1', + EOA_ADDRESS, + [TransactionStatus.submitted], + inputTransactions, + ); + + expect(eoaResult).toHaveLength(2); + expect(eoaResult.map((tx) => tx.txParams.nonce)).toStrictEqual([ + '0x5', + '0x6', + ]); + expect(eoaResult[0].txParams.from).toBe(EOA_ADDRESS); + expect(eoaResult[1].txParams.from.toLowerCase()).toBe( + EOA_ADDRESS.toLowerCase(), + ); + + const moneyAccountResult = getAndFormatTransactionsForNonceTracker( + '0x1', + MONEY_ACCOUNT_ADDRESS, + [TransactionStatus.submitted], + inputTransactions, + ); + + expect(moneyAccountResult).toHaveLength(1); + expect(moneyAccountResult[0].txParams.nonce).toBe('0x14'); + expect(moneyAccountResult[0].txParams.from.toLowerCase()).toBe( + MONEY_ACCOUNT_ADDRESS.toLowerCase(), + ); + expect(moneyAccountResult[0].txParams.gas).toBe('0x100'); + expect(moneyAccountResult[0].txParams.value).toBe('0x200'); + }); + + it('excludes signed authorizations that fail authority recovery', () => { + const inputTransactions: TransactionMeta[] = [ + { + id: '1', + chainId: '0x1', + networkClientId: 'testNetworkClientId', + time: 123456, + txParams: { + from: EOA_ADDRESS, + gas: '0x100', + value: '0x200', + nonce: '0x5', + authorizationList: [ + { + address: '0xabc' as Hex, + chainId: '0x1', + nonce: '0x6', + r: '0x0', + s: '0x0', + yParity: '0x0', + }, + ], + }, + status: TransactionStatus.submitted, + }, + ]; + + const result = getAndFormatTransactionsForNonceTracker( + '0x1', + EOA_ADDRESS, + [TransactionStatus.submitted], + inputTransactions, + ); + + expect(result).toHaveLength(1); + expect(result[0].txParams.nonce).toBe('0x5'); }); }); }); diff --git a/packages/transaction-controller/src/utils/nonce.ts b/packages/transaction-controller/src/utils/nonce.ts index 49c65bf336f..550f434adcf 100644 --- a/packages/transaction-controller/src/utils/nonce.ts +++ b/packages/transaction-controller/src/utils/nonce.ts @@ -6,6 +6,7 @@ import type { import { createModuleLogger, projectLogger } from '../logger.js'; import type { TransactionMeta, TransactionStatus } from '../types.js'; +import { getAuthorizationAuthority } from './eip7702.js'; const log = createModuleLogger(projectLogger, 'nonce'); @@ -55,6 +56,11 @@ export async function getNextNonce( /** * Filter and format transactions for the nonce tracker. * + * Includes the outer transaction nonce when `txParams.from` matches, and EIP-7702 + * authorization nonces only when their recovered authority matches `fromAddress`. + * This keeps foreign authorizations (e.g. Money Account upgrades on a same-chain + * pay batch) out of the payer's nonce history. + * * @param currentChainId - Chain ID of the current network. * @param fromAddress - Address of the account from which the transactions to filter from are sent. * @param transactionStatuses - Status of the transactions for which to filter. @@ -67,45 +73,59 @@ export function getAndFormatTransactionsForNonceTracker( transactionStatuses: TransactionStatus[], transactions: TransactionMeta[], ): NonceTrackerTransaction[] { + const normalizedFromAddress = fromAddress.toLowerCase(); + return transactions .filter( - ({ - chainId, - isTransfer, - isUserOperation, - status, - txParams: { from, nonce }, - }) => + ({ chainId, isTransfer, isUserOperation, status, txParams: { nonce } }) => !isTransfer && !isUserOperation && chainId === currentChainId && transactionStatuses.includes(status) && - from.toLowerCase() === fromAddress.toLowerCase() && - nonce, + Boolean(nonce), ) - .flatMap( - ({ + .flatMap(({ status, txParams }) => { + const { authorizationList, from, gas, value, nonce } = txParams; + // the only value we care about is the nonce + // but we need to return the other values to satisfy the type + // TODO: refactor nonceTracker to not require this + /* istanbul ignore next */ + const toNonceTrackerTransaction = ( + currentNonce: string, + authority: string, + ): NonceTrackerTransaction => ({ status, - txParams: { authorizationList, from, gas, value, nonce }, - }) => { - const authorizationNonces = (authorizationList ?? []) - .map((authorization) => authorization.nonce) - .filter((authorizationNonce) => authorizationNonce !== undefined); - - // the only value we care about is the nonce - // but we need to return the other values to satisfy the type - // TODO: refactor nonceTracker to not require this - /* istanbul ignore next */ - return [nonce, ...authorizationNonces].map((currentNonce) => ({ - status, - history: [{}], - txParams: { - from: from ?? '', - gas: gas ?? '', - value: value ?? '', - nonce: currentNonce ?? '', - }, - })); - }, - ); + history: [{}], + txParams: { + from: authority, + gas: gas ?? '', + value: value ?? '', + nonce: currentNonce, + }, + }); + + const formatted: NonceTrackerTransaction[] = []; + + if (from.toLowerCase() === normalizedFromAddress && nonce) { + formatted.push(toNonceTrackerTransaction(nonce, from)); + } + + for (const authorization of authorizationList ?? []) { + if (authorization.nonce === undefined) { + continue; + } + + const authority = getAuthorizationAuthority(authorization, from); + + if (authority?.toLowerCase() !== normalizedFromAddress) { + continue; + } + + formatted.push( + toNonceTrackerTransaction(authorization.nonce, authority), + ); + } + + return formatted; + }); } diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index f13c16a6cb4..ddb38679225 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Pass the quote's pre-signed `authorizationList` through `addTransactionBatch` for same-chain Relay submits when an account override is active, so Money Account vault upgrades are not dropped on the multi-step batch path ([#9765](https://github.com/MetaMask/core/pull/9765)) + ## [26.2.1] ### Changed diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts index 42e017c9673..8fbf3cd3e2c 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts @@ -546,6 +546,81 @@ describe('Relay Submit Utils', () => { }); }); + it('passes signed authorizationList on batch when same-chain with account override', async () => { + const moneyAccountFrom = '0xmoneyaccount' as Hex; + + request.transaction = { + ...request.transaction, + txParams: { from: moneyAccountFrom }, + } as TransactionMeta; + + request.quotes[0].original.details.currencyOut.currency.chainId = 1; + request.quotes[0].original.request = { + authorizationList: [ + { + address: '0xabc' as Hex, + chainId: 1, + nonce: 2, + r: '0xr' as Hex, + s: '0xs' as Hex, + yParity: 1, + }, + ], + } as never; + + request.quotes[0].original.steps[0].items.push({ + ...request.quotes[0].original.steps[0].items[0], + }); + + await submitRelayQuotes(request); + + expect(addTransactionBatchMock).toHaveBeenCalledTimes(1); + expect(addTransactionBatchMock).toHaveBeenCalledWith( + expect.objectContaining({ + from: FROM_MOCK, + authorizationList: [ + { + address: '0xabc', + chainId: '0x1', + nonce: '0x2', + r: '0xr', + s: '0xs', + yParity: '0x1', + }, + ], + }), + ); + }); + + it('does not pass authorizationList on batch when from matches transaction from', async () => { + request.quotes[0].original.details.currencyOut.currency.chainId = 1; + request.quotes[0].original.request = { + authorizationList: [ + { + address: '0xabc' as Hex, + chainId: 1, + nonce: 2, + r: '0xr' as Hex, + s: '0xs' as Hex, + yParity: 1, + }, + ], + } as never; + + request.quotes[0].original.steps[0].items.push({ + ...request.quotes[0].original.steps[0].items[0], + }); + + await submitRelayQuotes(request); + + expect(addTransactionBatchMock).toHaveBeenCalledTimes(1); + expect(addTransactionBatchMock).toHaveBeenCalledWith( + expect.not.objectContaining({ + authorizationList: expect.anything(), + }), + ); + }); + it('uses mapped relay deposit type in batch when parent is predictDeposit', async () => { request.transaction = { ...request.transaction, diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts index e2ac51c15a0..32c3dfa3ab8 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts @@ -900,6 +900,9 @@ async function submitViaTransactionController( quote.original.details.currencyIn.currency.chainId === quote.original.details.currencyOut.currency.chainId; + // Single-tx keeps address/chainId only — TransactionController re-signs with + // `from`. Batch may need a different signer (Money Account) than the batch + // payer (EOA account override), so preserve the full pre-signed list. const authorizationList: AuthorizationList | undefined = isSameChain && quote.original.request.authorizationList?.length ? quote.original.request.authorizationList.map((a) => ({ @@ -908,6 +911,19 @@ async function submitViaTransactionController( })) : undefined; + const hasAccountOverride = + quote.request.from.toLowerCase() !== + (transaction.txParams.from as Hex).toLowerCase(); + + const batchAuthorizationList = + isSameChain && + hasAccountOverride && + quote.original.request.authorizationList?.length + ? mapSignedQuoteAuthorizationList( + quote.original.request.authorizationList, + ) + : undefined; + const { metamask } = quote.original; const { gasLimits } = metamask; @@ -936,6 +952,7 @@ async function submitViaTransactionController( 'TransactionController:addTransactionBatch', buildRelayTransactionBatchRequest({ allParams, + authorizationList: batchAuthorizationList, isGasFeeSponsored: isSourceGasFeeSponsored, messenger, normalizedParams, @@ -973,8 +990,22 @@ async function submitViaTransactionController( return hash as Hex; } +function mapSignedQuoteAuthorizationList( + authorizationList: NonNullable, +): AuthorizationList { + return authorizationList.map((a) => ({ + address: a.address, + chainId: toHex(a.chainId), + nonce: toHex(a.nonce), + r: a.r, + s: a.s, + yParity: toHex(a.yParity), + })); +} + function buildRelayTransactionBatchRequest({ allParams, + authorizationList, isGasFeeSponsored, messenger, normalizedParams, @@ -982,6 +1013,7 @@ function buildRelayTransactionBatchRequest({ transaction, }: { allParams: TransactionParams[]; + authorizationList?: AuthorizationList; isGasFeeSponsored: boolean | undefined; messenger: TransactionPayControllerMessenger; normalizedParams: TransactionParams[]; @@ -998,6 +1030,7 @@ function buildRelayTransactionBatchRequest({ return { from, + ...(authorizationList?.length ? { authorizationList } : {}), disable7702: !gasLimit7702, disableHook: Boolean(gasLimit7702), disableSequential: Boolean(gasLimit7702), diff --git a/yarn.lock b/yarn.lock index 4fae11566d8..a5561d619e9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9300,6 +9300,7 @@ __metadata: dependencies: "@babel/runtime": "npm:^7.23.9" "@ethereumjs/common": "npm:^4.4.0" + "@ethereumjs/rlp": "npm:^5.0.2" "@ethereumjs/tx": "npm:^5.4.0" "@ethereumjs/util": "npm:^9.1.0" "@ethersproject/abi": "npm:^5.7.0" @@ -9332,6 +9333,7 @@ __metadata: bn.js: "npm:^5.2.1" deepmerge: "npm:^4.2.2" eth-method-registry: "npm:^4.0.0" + ethereum-cryptography: "npm:^2.1.2" fast-json-patch: "npm:^3.1.1" immer: "npm:^9.0.6" jest: "npm:^30.4.2"