diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 2faffdb1cc..15102cece6 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -17,6 +17,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `TransactionController:updateSelectedGasFeeToken` - `TransactionController:updateTransactionGasFees` - Corresponding action types are available as well. +- Surface `simulationFails` on `EstimateGasBatchResult` so callers can detect EIP-7702 batch gas estimation failures. + +### Fixed + +- Stop returning a block-gas-limit-derived value as the EIP-7702 batch gas limit when simulation fails. The fallback is now `sum(per-tx gas hints) + configured fallback`, applied once for the unsimulated portion, instead of a percentage of the block gas limit (or fixed) value, that gets surfaced as a successful estimate. ## [65.1.0] diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts index 89af467dfd..a9f36d8571 100644 --- a/packages/transaction-controller/src/utils/gas.test.ts +++ b/packages/transaction-controller/src/utils/gas.test.ts @@ -1,6 +1,7 @@ import type { NetworkClientId } from '@metamask/network-controller'; import { remove0x } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; +import { BigNumber } from 'bignumber.js'; import { cloneDeep } from 'lodash'; import type { @@ -1325,6 +1326,234 @@ describe('gas', () => { }); }); + it('falls back when EIP-7702 simulation fails and upgrade is required', async () => { + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: false, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + getGasEstimateFallbackMock.mockReturnValue( + GAS_ESTIMATE_FALLBACK_FIXED_MOCK, + ); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('estimate failed'), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_MOCK, + }); + + expect(result).toStrictEqual({ + gasLimits: [FIXED_ESTIMATE_GAS_MOCK], + requiresAuthorizationList: true, + simulationFails: expect.objectContaining({ + reason: 'estimate failed', + }), + totalGasLimit: FIXED_ESTIMATE_GAS_MOCK, + }); + }); + + it('falls back to feature-flag fallback when EIP-7702 simulation fails and no per-tx gas hints provided', async () => { + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: true, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('estimate failed'), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_MOCK, + }); + + const expectedFallback = Math.floor( + (BLOCK_GAS_LIMIT_MOCK * DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK) / 100, + ); + + expect(result).toStrictEqual({ + gasLimits: [expectedFallback], + simulationFails: expect.objectContaining({ + reason: 'estimate failed', + }), + totalGasLimit: expectedFallback, + }); + }); + + it('falls back to fixed feature-flag fallback when EIP-7702 simulation fails', async () => { + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: true, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + getGasEstimateFallbackMock.mockReturnValue( + GAS_ESTIMATE_FALLBACK_FIXED_MOCK, + ); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('estimate failed'), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_MOCK, + }); + + expect(result).toStrictEqual({ + gasLimits: [FIXED_ESTIMATE_GAS_MOCK], + simulationFails: expect.objectContaining({ + reason: 'estimate failed', + }), + totalGasLimit: FIXED_ESTIMATE_GAS_MOCK, + }); + }); + + it('sums per-tx gas hints with fallback once when EIP-7702 simulation fails', async () => { + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: true, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + getGasEstimateFallbackMock.mockReturnValue( + GAS_ESTIMATE_FALLBACK_FIXED_MOCK, + ); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('estimate failed'), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_WITH_GAS_MOCK, + }); + + const hintTotal = + new BigNumber(GAS_MOCK_1).toNumber() + + new BigNumber(GAS_MOCK_2).toNumber(); + const expectedTotal = hintTotal + FIXED_ESTIMATE_GAS_MOCK; + + expect(result).toStrictEqual({ + gasLimits: [expectedTotal], + simulationFails: expect.objectContaining({ + reason: 'estimate failed', + }), + totalGasLimit: expectedTotal, + }); + }); + + it('sums partial per-tx gas hints with fallback once when EIP-7702 simulation fails', async () => { + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: true, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + getGasEstimateFallbackMock.mockReturnValue( + GAS_ESTIMATE_FALLBACK_FIXED_MOCK, + ); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('estimate failed'), + }); + + const partialBatchParams: BatchTransactionParams[] = [ + { + data: DATA_MOCK, + to: TO_MOCK, + value: VALUE_MOCK, + gas: GAS_MOCK_1, + }, + { + data: DATA_MOCK_2, + to: TO_MOCK, + value: VALUE_MOCK_2, + }, + ]; + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: partialBatchParams, + }); + + const expectedTotal = + new BigNumber(GAS_MOCK_1).toNumber() + FIXED_ESTIMATE_GAS_MOCK; + + expect(result).toStrictEqual({ + gasLimits: [expectedTotal], + simulationFails: expect.objectContaining({ + reason: 'estimate failed', + }), + totalGasLimit: expectedTotal, + }); + }); + it('throws error when upgrade contract address not found', async () => { const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ { diff --git a/packages/transaction-controller/src/utils/gas.ts b/packages/transaction-controller/src/utils/gas.ts index 4bf791ccc0..40b6fd5ab7 100644 --- a/packages/transaction-controller/src/utils/gas.ts +++ b/packages/transaction-controller/src/utils/gas.ts @@ -44,6 +44,7 @@ export type UpdateGasRequest = { export type EstimateGasBatchResult = { gasLimits: number[]; requiresAuthorizationList?: true; + simulationFails?: TransactionMeta['simulationFails']; totalGasLimit: number; }; @@ -260,7 +261,11 @@ export async function estimateGasBatch({ type, }; - const { estimatedGas: gasLimitHex } = await estimateGas({ + const { + blockGasLimit, + estimatedGas: gasLimitHex, + simulationFails, + } = await estimateGas({ isSimulationEnabled: true, getSimulationConfig, messenger, @@ -268,6 +273,35 @@ export async function estimateGasBatch({ txParams: params, }); + if (simulationFails) { + // Estimation failed and `estimateGas` substituted a block-gas-limit-derived + // fallback into `gasLimitHex`. That value is unsafe to surface as a + // batch gas limit on EIP-7702 because callers (e.g. `transaction-pay-controller`) + // multiply it by the gas price to compute fiat estimates and quote + // submission gas, producing wildly inflated numbers. + // + // Instead, sum any caller-provided per-transaction gas hints and add the + // chain's configured fallback exactly once for the unknown portion. + const totalGasLimit = computeBatch7702FallbackTotal({ + blockGasLimit, + chainId, + messenger, + transactions, + }); + + log('EIP-7702 gas estimation failed, using fallback', { + simulationFails, + totalGasLimit, + }); + + return { + gasLimits: [totalGasLimit], + ...(isUpgradeRequired ? { requiresAuthorizationList: true } : {}), + simulationFails, + totalGasLimit, + }; + } + const totalGasLimit = new BigNumber(gasLimitHex).toNumber(); log('Estimated EIP-7702 gas limit', totalGasLimit); @@ -426,6 +460,52 @@ export async function simulateGasBatch({ } } +/** + * Compute a safe `totalGasLimit` for an EIP-7702 batch when simulation has + * failed. Sums any caller-provided per-transaction gas hints and adds the + * configured fallback exactly once for the unsimulated portion. + * + * @param options - Options object. + * @param options.blockGasLimit - The latest block gas limit (hex), used as the + * basis for the percentage-based fallback when no fixed value is configured. + * @param options.chainId - Chain ID. + * @param options.messenger - The controller messenger instance. + * @param options.transactions - The batch transactions, optionally carrying + * per-tx `gas` hints. + * @returns The fallback total gas limit. + */ +function computeBatch7702FallbackTotal({ + blockGasLimit, + chainId, + messenger, + transactions, +}: { + blockGasLimit: string; + chainId: Hex; + messenger: TransactionControllerMessenger; + transactions: BatchTransactionParams[]; +}): number { + const hintedGas = transactions.reduce((acc, transaction) => { + if (transaction.gas === undefined) { + return acc; + } + + return acc.plus(new BigNumber(transaction.gas)); + }, new BigNumber(0)); + + const { fixed, percentage } = getGasEstimateFallback(chainId, messenger); + + const fallbackBN = + fixed === undefined + ? new BigNumber(hexToBN(blockGasLimit).toString()) + .multipliedBy(percentage) + .dividedBy(100) + .integerValue(BigNumber.ROUND_FLOOR) + : new BigNumber(fixed); + + return hintedGas.plus(fallbackBN).toNumber(); +} + /** * Determine the gas for the provided request. * diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 512cddc57c..4c49e1d321 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/transaction-controller` from `^65.0.0` to `^65.1.0` ([#8691](https://github.com/MetaMask/core/pull/8691)) - Bump `@metamask/ramps-controller` from `^13.2.0` to `^13.3.0` ([#8698](https://github.com/MetaMask/core/pull/8698)) +### Fixed + +- Detect EIP-7702 batch gas estimation failures during quote calculation and substitute the configured `relayFallbackGas` (or caller-provided `fallbackGas`) instead of returning the upstream block-gas-limit fallback as if it were a successful estimate. When `fallbackOnSimulationFailure` is not set, batch failures now throw consistently with the single-transaction path. + ## [21.0.0] ### Added diff --git a/packages/transaction-pay-controller/src/utils/quote-gas.test.ts b/packages/transaction-pay-controller/src/utils/quote-gas.test.ts index 64cef9bcca..425bc07a96 100644 --- a/packages/transaction-pay-controller/src/utils/quote-gas.test.ts +++ b/packages/transaction-pay-controller/src/utils/quote-gas.test.ts @@ -1,12 +1,13 @@ import type { Hex } from '@metamask/utils'; import { getMessengerMock } from '../tests/messenger-mock'; -import { getGasBuffer } from './feature-flags'; +import { getFallbackGas, getGasBuffer } from './feature-flags'; import { estimateGasLimit } from './gas'; import { estimateQuoteGasLimits } from './quote-gas'; jest.mock('./feature-flags', () => ({ ...jest.requireActual('./feature-flags'), + getFallbackGas: jest.fn(), getGasBuffer: jest.fn(), })); @@ -16,6 +17,7 @@ jest.mock('./gas', () => ({ })); describe('quote gas estimation', () => { + const getFallbackGasMock = jest.mocked(getFallbackGas); const getGasBufferMock = jest.mocked(getGasBuffer); const estimateGasLimitMock = jest.mocked(estimateGasLimit); @@ -43,6 +45,7 @@ describe('quote gas estimation', () => { jest.resetAllMocks(); getGasBufferMock.mockReturnValue(1); + getFallbackGasMock.mockReturnValue({ estimate: 200000, max: 400000 }); }); it('throws when there are no transactions', async () => { @@ -357,4 +360,124 @@ describe('quote gas estimation', () => { ], }); }); + + describe('on batch simulation failure', () => { + const SIMULATION_FAILS_MOCK = { + reason: 'estimate failed', + debug: {}, + }; + + it('throws when simulation fails and fallbackOnSimulationFailure is false', async () => { + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 99999999, + gasLimits: [99999999], + simulationFails: SIMULATION_FAILS_MOCK, + }); + + await expect( + estimateQuoteGasLimits({ + messenger, + transactions: TRANSACTIONS_MOCK, + }), + ).rejects.toThrow('Batch gas estimation failed: estimate failed'); + }); + + it('throws with a generic reason when simulation fails without a reason', async () => { + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 99999999, + gasLimits: [99999999], + simulationFails: { debug: {} }, + }); + + await expect( + estimateQuoteGasLimits({ + messenger, + transactions: TRANSACTIONS_MOCK, + }), + ).rejects.toThrow('Batch gas estimation failed: unknown reason'); + }); + + it('uses feature-flag fallback for the EIP-7702 batch when simulation fails', async () => { + getGasBufferMock.mockReturnValue(1.5); + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 99999999, + gasLimits: [99999999], + simulationFails: SIMULATION_FAILS_MOCK, + requiresAuthorizationList: true, + }); + + const result = await estimateQuoteGasLimits({ + fallbackOnSimulationFailure: true, + messenger, + transactions: TRANSACTIONS_MOCK, + }); + + expect(result).toStrictEqual({ + batchGasLimit: { + estimate: 300000, + max: 600000, + }, + gasLimits: [ + { + estimate: 300000, + max: 600000, + }, + ], + is7702: true, + requiresAuthorizationList: true, + totalGasEstimate: 600000, + totalGasLimit: 600000, + usedBatch: true, + }); + }); + + it('uses caller-provided fallbackGas over the feature-flag fallback', async () => { + getGasBufferMock.mockReturnValue(1); + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 99999999, + gasLimits: [99999999], + simulationFails: SIMULATION_FAILS_MOCK, + }); + + const result = await estimateQuoteGasLimits({ + fallbackGas: { estimate: 50000, max: 75000 }, + fallbackOnSimulationFailure: true, + messenger, + transactions: TRANSACTIONS_MOCK, + }); + + expect(getFallbackGasMock).not.toHaveBeenCalled(); + expect(result.batchGasLimit).toStrictEqual({ + estimate: 50000, + max: 75000, + }); + expect(result.totalGasLimit).toBe(75000); + }); + + it('fans out the fallback per transaction for non-7702 batch on simulation failure', async () => { + getGasBufferMock.mockReturnValue(1); + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 99999999, + gasLimits: [99999999, 99999999], + simulationFails: SIMULATION_FAILS_MOCK, + }); + + const result = await estimateQuoteGasLimits({ + fallbackOnSimulationFailure: true, + messenger, + transactions: TRANSACTIONS_MOCK, + }); + + expect(result).toStrictEqual({ + gasLimits: [ + { estimate: 200000, max: 400000 }, + { estimate: 200000, max: 400000 }, + ], + is7702: false, + totalGasEstimate: 800000, + totalGasLimit: 800000, + usedBatch: true, + }); + }); + }); }); diff --git a/packages/transaction-pay-controller/src/utils/quote-gas.ts b/packages/transaction-pay-controller/src/utils/quote-gas.ts index 25c2823c1d..1f0d90ec73 100644 --- a/packages/transaction-pay-controller/src/utils/quote-gas.ts +++ b/packages/transaction-pay-controller/src/utils/quote-gas.ts @@ -6,7 +6,7 @@ import { BigNumber } from 'bignumber.js'; import type { TransactionPayControllerMessenger } from '..'; import { projectLogger } from '../logger'; -import { getGasBuffer } from './feature-flags'; +import { getFallbackGas, getGasBuffer } from './feature-flags'; import { estimateGasLimit } from './gas'; const log = createModuleLogger(projectLogger, 'quote-gas'); @@ -55,7 +55,12 @@ export async function estimateQuoteGasLimits({ if (useBatch) { return { - ...(await estimateQuoteGasLimitsBatch(transactions, messenger)), + ...(await estimateQuoteGasLimitsBatch({ + fallbackGas, + fallbackOnSimulationFailure, + messenger, + transactions, + })), usedBatch: true, }; } @@ -72,10 +77,20 @@ export async function estimateQuoteGasLimits({ }; } -async function estimateQuoteGasLimitsBatch( - transactions: QuoteGasTransaction[], - messenger: TransactionPayControllerMessenger, -): Promise<{ +async function estimateQuoteGasLimitsBatch({ + fallbackGas, + fallbackOnSimulationFailure, + messenger, + transactions, +}: { + fallbackGas?: { + estimate: number; + max: number; + }; + fallbackOnSimulationFailure: boolean; + messenger: TransactionPayControllerMessenger; + transactions: QuoteGasTransaction[]; +}): Promise<{ batchGasLimit?: QuoteGasLimit; gasLimits: QuoteGasLimit[]; is7702: boolean; @@ -90,19 +105,62 @@ async function estimateQuoteGasLimitsBatch( parseGasLimit(transaction.gas), ); - const { gasLimits, requiresAuthorizationList } = await messenger.call( - 'TransactionController:estimateGasBatch', - { + const { gasLimits, requiresAuthorizationList, simulationFails } = + await messenger.call('TransactionController:estimateGasBatch', { chainId: firstTransaction.chainId, from: firstTransaction.from, transactions: transactions.map(toBatchTransactionParams), - }, - ); + }); if (gasLimits.length !== 1 && gasLimits.length !== transactions.length) { throw new Error('Unexpected batch gas limit count'); } + const is7702 = gasLimits.length === 1; + + if (simulationFails) { + if (!fallbackOnSimulationFailure) { + throw new Error( + `Batch gas estimation failed: ${ + simulationFails.reason ?? 'unknown reason' + }`, + ); + } + + log('Batch gas estimate failed, using fallback', { + chainId: firstTransaction.chainId, + is7702, + simulationFails, + }); + + const resolvedFallback = fallbackGas ?? getFallbackGas(messenger); + const fallbackEstimate = Math.ceil(resolvedFallback.estimate * gasBuffer); + const fallbackMax = Math.ceil(resolvedFallback.max * gasBuffer); + + const fallbackGasLimit: QuoteGasLimit = { + estimate: fallbackEstimate, + max: fallbackMax, + }; + + const fallbackGasLimits: QuoteGasLimit[] = is7702 + ? [fallbackGasLimit] + : transactions.map(() => fallbackGasLimit); + + const fallbackTotal = fallbackGasLimits.reduce( + (acc, current) => acc + current.max, + 0, + ); + + return { + ...(is7702 ? { batchGasLimit: fallbackGasLimit } : {}), + gasLimits: fallbackGasLimits, + is7702, + ...(requiresAuthorizationList ? { requiresAuthorizationList } : {}), + totalGasEstimate: fallbackTotal, + totalGasLimit: fallbackTotal, + }; + } + const bufferedGasLimits = gasLimits.map((gasLimit, index) => { const providedGasLimit = paramGasLimits[index]; const providedGasWasPreserved = @@ -126,7 +184,6 @@ async function estimateQuoteGasLimitsBatch( (acc, gasLimit) => acc + gasLimit.max, 0, ); - const is7702 = bufferedGasLimits.length === 1; const batchGasLimit = is7702 ? bufferedGasLimits[0] : undefined; return {