diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index d3b96ca2ea..b0a256d0bc 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Export `decodeAuthorizationSignature` utility that decodes a 65-byte EIP-7702 authorization signature into RLP-canonical `r`, `s`, and `yParity` ([#8656](https://github.com/MetaMask/core/pull/8656)) - All `eth_sendRawTransaction` failures are prefixed `RPC submit:` for failure-surface attribution in error metrics +- Add `revert?: RevertData` field to `TransactionMeta` exposing decoded revert reason and raw data from gas estimation, simulation, and receipt replay ([#8589](https://github.com/MetaMask/core/pull/8589)) ### Changed diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 83dbe27899..4a266a46b0 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -97,6 +97,7 @@ import type { TransactionControllerMethodActions } from './TransactionController import type { DappSuggestedGasFees, Layer1GasFeeFlow, + Revert, SavedGasFees, SecurityProviderRequest, SendFlowHistoryEntry, @@ -2765,6 +2766,14 @@ export class TransactionController extends BaseController< transactionMeta.txParams.gas = draftTransaction.txParams.gas; transactionMeta.simulationFails = draftTransaction.simulationFails; transactionMeta.gasLimitNoBuffer = draftTransaction.gasLimitNoBuffer; + + const draftGasRevert = draftTransaction.revert?.gas; + if (draftGasRevert) { + transactionMeta.revert = { + ...transactionMeta.revert, + gas: draftGasRevert, + }; + } }, ); @@ -4294,6 +4303,7 @@ export class TransactionController extends BaseController< let gasUsed: Hex | undefined; let gasFeeTokens: GasFeeToken[] = []; let isGasFeeSponsored = false; + let simulationRevert: Revert | undefined; const isBalanceChangesSkipped = this.#isBalanceChangesSkipped(transactionMeta); @@ -4319,6 +4329,7 @@ export class TransactionController extends BaseController< ); simulationData = balanceChangesResult.simulationData; gasUsed = balanceChangesResult.gasUsed; + simulationRevert = balanceChangesResult.simulationRevert; if ( blockTime && @@ -4362,6 +4373,13 @@ export class TransactionController extends BaseController< if (!this.#isBalanceChangesSkipped(txMeta)) { txMeta.simulationData = simulationData; + + if (simulationRevert) { + txMeta.revert = { + ...txMeta.revert, + simulation: simulationRevert, + }; + } } }, ); @@ -4538,6 +4556,7 @@ export class TransactionController extends BaseController< actionId?: string, ): void { let newTransactionMeta: TransactionMeta; + const { revert: receiptRevert } = error as { revert?: Revert }; try { newTransactionMeta = this.#updateTransactionInternal( @@ -4553,6 +4572,15 @@ export class TransactionController extends BaseController< status: TransactionStatus.failed; } ).error = normalizeTxError(error); + + if (receiptRevert === undefined) { + return; + } + + draftTransactionMeta.revert = { + ...draftTransactionMeta.revert, + receipt: receiptRevert, + }; }, ); } catch (caughtError: unknown) { @@ -4562,6 +4590,14 @@ export class TransactionController extends BaseController< ...transactionMeta, status: TransactionStatus.failed, error: normalizeTxError(error), + ...(receiptRevert === undefined + ? {} + : { + revert: { + ...transactionMeta.revert, + receipt: receiptRevert, + }, + }), }; } diff --git a/packages/transaction-controller/src/api/simulation-api.ts b/packages/transaction-controller/src/api/simulation-api.ts index 5e77dab80e..4cdd3bfa79 100644 --- a/packages/transaction-controller/src/api/simulation-api.ts +++ b/packages/transaction-controller/src/api/simulation-api.ts @@ -142,6 +142,9 @@ export type SimulationResponseCallTrace = { /** Raw event logs created by the call. */ logs?: SimulationResponseLog[] | null; + + /** Raw return data from the call (revert hex when reverted). */ + output?: Hex; }; /** diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index 128d4bb875..8ad6708e49 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -34,6 +34,9 @@ const RECEIPT_MOCK = { status: '0x1', }; +const REVERT_DATA_TRANSFER_EXCEEDS_BALANCE = + '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000'; + const BLOCK_MOCK = { baseFeePerGas: '0x456', timestamp: 123456, @@ -128,6 +131,7 @@ describe('PendingTransactionTracker', () => { const getTransactionByHashMock = jest.fn(); const getTransactionCountMock = jest.fn(); const getBlockByHashMock = jest.fn(); + const estimateGasMock = jest.fn(); let blockTracker: jest.Mocked; let pendingTransactionTracker: PendingTransactionTracker; @@ -182,11 +186,16 @@ describe('PendingTransactionTracker', () => { return getTransactionCountMock(...args); case 'eth_getBlockByHash': return getBlockByHashMock(...args); + case 'eth_estimateGas': + return estimateGasMock(...args); default: return undefined; } }); + estimateGasMock.mockReset(); + estimateGasMock.mockResolvedValue('0x5208'); + options = { blockTracker, getTransactions: jest.fn(), @@ -498,10 +507,59 @@ describe('PendingTransactionTracker', () => { await onPoll(); expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith( - TRANSACTION_SUBMITTED_MOCK, - new Error('Transaction failed on-chain'), + const [emittedTxMeta, emittedError] = listener.mock.calls[0]; + expect(emittedTxMeta).toStrictEqual(TRANSACTION_SUBMITTED_MOCK); + expect(emittedError.name).toBe('OnChainFailureError'); + expect(emittedError.message).toBe('Transaction failed on-chain'); + expect(emittedError.revertReason).toBeUndefined(); + }); + + it('with decoded revert reason when receipt has error status', async () => { + const listener = jest.fn(); + + const transactionMetaWithCall = { + ...TRANSACTION_SUBMITTED_MOCK, + txParams: { + ...TRANSACTION_SUBMITTED_MOCK.txParams, + from: `0x${'11'.repeat(20)}`, + to: `0x${'22'.repeat(20)}`, + data: '0xa9059cbb', + }, + } as unknown as TransactionMeta; + + pendingTransactionTracker = new PendingTransactionTracker({ + ...options, + getTransactions: (): TransactionMeta[] => + freeze([transactionMetaWithCall], true), + }); + + pendingTransactionTracker.hub.addListener( + 'transaction-failed', + listener, + ); + + getTransactionReceiptMock.mockResolvedValueOnce({ + ...RECEIPT_MOCK, + status: '0x0', + }); + + estimateGasMock.mockRejectedValueOnce({ + message: 'execution reverted', + data: REVERT_DATA_TRANSFER_EXCEEDS_BALANCE, + }); + + await onPoll(); + + expect(listener).toHaveBeenCalledTimes(1); + const [, emittedError] = listener.mock.calls[0]; + expect(emittedError.name).toBe('OnChainFailureError'); + expect(emittedError.message).toBe( + 'Transaction failed on-chain: ERC20: transfer amount exceeds balance', ); + expect(emittedError.revert).toStrictEqual({ + message: 'ERC20: transfer amount exceeds balance', + data: REVERT_DATA_TRANSFER_EXCEEDS_BALANCE, + }); }); }); @@ -1272,10 +1330,10 @@ describe('PendingTransactionTracker', () => { await tracker.forceCheckTransaction(transactionMeta); expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith( - transactionMeta, - new Error('Transaction failed on-chain'), - ); + const [, emittedError] = listener.mock.calls[0]; + expect(emittedError.name).toBe('OnChainFailureError'); + expect(emittedError.message).toBe('Transaction failed on-chain'); + expect(emittedError.revertReason).toBeUndefined(); }); it('should not change transaction status if receipt status is neither success nor failure', async () => { diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index b87b5b3847..683ebe57a3 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -17,6 +17,7 @@ import { getTimeoutAttempts, } from '../utils/feature-flags'; import { getChainId, rpcRequest } from '../utils/provider'; +import { extractRevert, OnChainFailureError } from '../utils/revert-reason'; import { TransactionPoller } from './TransactionPoller'; /** @@ -429,9 +430,20 @@ export class PendingTransactionTracker { const isFailure = receipt?.status === RECEIPT_STATUS_FAILURE; if (isFailure) { - this.#log('Transaction receipt has failed status'); + this.#log('Transaction receipt has failed status', { + id: txMeta.id, + hash: txMeta.hash, + chainId: txMeta.chainId, + blockNumber: receipt.blockNumber, + }); + + const revert = await extractRevert({ + messenger: this.#messenger, + networkClientId: txMeta.networkClientId, + txParams: txMeta.txParams, + }); - this.#failTransaction(txMeta, new Error('Transaction failed on-chain')); + this.#failTransaction(txMeta, new OnChainFailureError(revert)); return; } diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 8881fb7b1f..0f07ae1443 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -416,6 +416,9 @@ export type TransactionMeta = { */ retryCount?: number; + /** Decoded revert information from each lifecycle source. */ + revert?: RevertData; + /** * The transaction's 's' value as a hex string. */ @@ -2325,3 +2328,28 @@ export type RequiredAsset = { /** Token standard of the asset (e.g., 'erc20'). */ standard: string; }; + +/** + * Decoded revert from a single lifecycle source. + */ +export type Revert = { + /** Decoded human-readable revert reason. */ + message?: string; + + /** Raw revert data hex returned by the EVM. */ + data?: Hex; +}; + +/** + * Revert information across each stage where a transaction can fail. + */ +export type RevertData = { + /** Revert from pre-confirmation gas estimation. */ + gas?: Revert; + + /** Revert from the simulation API's root call frame. */ + simulation?: Revert; + + /** Revert from on-chain failure, via receipt replay. */ + receipt?: Revert; +}; diff --git a/packages/transaction-controller/src/utils/balance-changes.test.ts b/packages/transaction-controller/src/utils/balance-changes.test.ts index 55114b4a01..5013029f06 100644 --- a/packages/transaction-controller/src/utils/balance-changes.test.ts +++ b/packages/transaction-controller/src/utils/balance-changes.test.ts @@ -318,6 +318,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }, ); @@ -336,6 +337,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -358,6 +360,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); }); @@ -485,6 +488,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }, ); @@ -554,6 +558,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -596,6 +601,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -653,6 +659,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -765,6 +772,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -788,6 +796,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -816,6 +825,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -841,6 +851,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -872,6 +883,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -931,6 +943,7 @@ describe('Balance Change Utils', () => { ], }, gasUsed: undefined, + simulationRevert: undefined, }); }); }); @@ -963,6 +976,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1009,6 +1023,7 @@ describe('Balance Change Utils', () => { tokenBalanceChanges: [], }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1053,8 +1068,68 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); + + it('returns simulationRevert with decoded message and raw data when output is set', async () => { + const data = + '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000'; + + simulateTransactionsMock.mockResolvedValueOnce({ + transactions: [ + { + ...defaultResponseTx, + error: 'execution reverted', + callTrace: { + calls: [], + logs: [], + error: 'execution reverted', + output: data as Hex, + }, + }, + ], + sponsorship: { + isSponsored: false, + error: null, + }, + }); + + const result = await getBalanceChanges(REQUEST_MOCK); + + expect(result.simulationRevert).toStrictEqual({ + message: 'ERC20: transfer amount exceeds balance', + data, + }); + }); + + it('returns simulationRevert from root frame only, ignoring nested errors', async () => { + simulateTransactionsMock.mockResolvedValueOnce({ + transactions: [ + { + ...defaultResponseTx, + callTrace: { + calls: [ + { + calls: [], + logs: [], + error: 'execution reverted: nested caught error', + }, + ], + logs: [], + }, + }, + ], + sponsorship: { + isSponsored: false, + error: null, + }, + }); + + const result = await getBalanceChanges(REQUEST_MOCK); + + expect(result.simulationRevert).toBeUndefined(); + }); }); describe('returns error', () => { @@ -1076,6 +1151,7 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1096,6 +1172,7 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1120,6 +1197,7 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1149,6 +1227,7 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1178,6 +1257,7 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); @@ -1199,6 +1279,7 @@ describe('Balance Change Utils', () => { }, }, gasUsed: undefined, + simulationRevert: undefined, }); }); }); diff --git a/packages/transaction-controller/src/utils/balance-changes.ts b/packages/transaction-controller/src/utils/balance-changes.ts index b0ec964ff9..ba3ed0fe1e 100644 --- a/packages/transaction-controller/src/utils/balance-changes.ts +++ b/packages/transaction-controller/src/utils/balance-changes.ts @@ -28,6 +28,7 @@ import { import { projectLogger } from '../logger'; import type { TransactionControllerMessenger } from '../TransactionController'; import type { + Revert, SimulationBalanceChange, SimulationData, SimulationTokenBalanceChange, @@ -38,6 +39,7 @@ import type { } from '../types'; import { SimulationTokenStandard } from '../types'; import { getNativeBalance } from './balance'; +import { decodeRevert } from './revert-reason'; export enum SupportedToken { ERC20 = 'erc20', @@ -120,10 +122,15 @@ type BalanceTransactionMap = Map; */ export async function getBalanceChanges( request: GetBalanceChangesRequest, -): Promise<{ simulationData: SimulationData; gasUsed?: Hex }> { +): Promise<{ + simulationData: SimulationData; + gasUsed?: Hex; + simulationRevert?: Revert; +}> { log('Request', request); let callTraceErrors: string[] | undefined; + let simulationRevert: Revert | undefined; try { const response = await baseRequest({ @@ -136,6 +143,7 @@ export async function getBalanceChanges( const transactionResponse = response.transactions?.[0]; callTraceErrors = extractCallTraceErrors(transactionResponse?.callTrace); + simulationRevert = extractRootRevert(transactionResponse?.callTrace); const transactionError = transactionResponse?.error; if (transactionError) { @@ -156,7 +164,7 @@ export async function getBalanceChanges( tokenBalanceChanges, }; - return { simulationData, gasUsed }; + return { simulationData, gasUsed, simulationRevert }; } catch (error) { log('Failed to get balance changes', error, request); @@ -182,6 +190,7 @@ export async function getBalanceChanges( }, }, gasUsed: undefined, + simulationRevert, }; } } @@ -660,6 +669,25 @@ function extractCallTraceErrors(call?: SimulationResponseCallTrace): string[] { return [...errors, ...nestedErrors]; } +/** + * Build a `Revert` from the root call trace frame. Reads only the raw + * `output` hex and decodes it locally; the upstream `error` message string + * is never parsed. Only the root frame's output is considered, since + * nested frame errors may have been caught and recovered by their callers. + * + * @param call - The root call trace frame. + * @returns A `Revert` if the root frame reverted with output data, + * otherwise `undefined`. + */ +function extractRootRevert( + call?: SimulationResponseCallTrace, +): Revert | undefined { + if (!call?.error) { + return undefined; + } + return decodeRevert(call.output, 'simulation'); +} + /** * Generate balance change data from previous and new balances. * diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts index b453dd0976..89af467dfd 100644 --- a/packages/transaction-controller/src/utils/gas.test.ts +++ b/packages/transaction-controller/src/utils/gas.test.ts @@ -536,6 +536,29 @@ describe('gas', () => { }, }); }); + + it('sets revert.gas when the estimate error carries revert data', async () => { + const data = + '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000'; + + mockQuery({ + getBlockByNumberResponse: { + gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + number: BLOCK_NUMBER_MOCK, + }, + estimateGasError: { + message: 'execution reverted', + data, + }, + }); + + await updateGas(updateGasRequest); + + expect(updateGasRequest.txMeta.revert?.gas).toStrictEqual({ + message: 'ERC20: transfer amount exceeds balance', + data, + }); + }); }); }); @@ -558,6 +581,7 @@ describe('gas', () => { estimatedGas: toHex(GAS_MOCK), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), simulationFails: undefined, + gasRevert: undefined, isUpgradeWithData: false, }); }); @@ -582,6 +606,7 @@ describe('gas', () => { expect(result).toStrictEqual({ estimatedGas: expect.any(String), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + gasRevert: undefined, isUpgradeWithData: false, simulationFails: { reason: 'TestError', @@ -594,6 +619,55 @@ describe('gas', () => { }); }); + it('returns gasRevert decoded from estimate error data', async () => { + mockQuery({ + getBlockByNumberResponse: { + gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + number: BLOCK_NUMBER_MOCK, + }, + estimateGasError: { + message: 'execution reverted', + data: '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000', + }, + }); + + const result = await estimateGas({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + isSimulationEnabled: false, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + messenger: MESSENGER_MOCK, + txParams: TRANSACTION_META_MOCK.txParams, + }); + + expect(result.gasRevert).toStrictEqual({ + message: 'ERC20: transfer amount exceeds balance', + data: '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000', + }); + }); + + it('returns no gasRevert when error has only a message and no data', async () => { + mockQuery({ + getBlockByNumberResponse: { + gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + number: BLOCK_NUMBER_MOCK, + }, + estimateGasError: { + message: + 'execution reverted: NativeBalanceChangeEnforcer:exceeded-balance-decrease', + }, + }); + + const result = await estimateGas({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + isSimulationEnabled: false, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + messenger: MESSENGER_MOCK, + txParams: TRANSACTION_META_MOCK.txParams, + }); + + expect(result.gasRevert).toBeUndefined(); + }); + it('returns estimated gas as 35% of block gas limit on error', async () => { const fallbackGas = Math.floor( BLOCK_GAS_LIMIT_MOCK * FALLBACK_MULTIPLIER_35_PERCENT, @@ -618,6 +692,7 @@ describe('gas', () => { estimatedGas: toHex(fallbackGas), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), simulationFails: expect.any(Object), + gasRevert: undefined, isUpgradeWithData: false, }); }); @@ -643,6 +718,7 @@ describe('gas', () => { estimatedGas: toHex(FIXED_ESTIMATE_GAS_MOCK), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), simulationFails: expect.any(Object), + gasRevert: undefined, isUpgradeWithData: false, }); }); @@ -808,6 +884,7 @@ describe('gas', () => { estimatedGas: toHex(SIMULATE_GAS_MOCK), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), simulationFails: undefined, + gasRevert: undefined, isUpgradeWithData: false, }); }); @@ -860,6 +937,7 @@ describe('gas', () => { estimatedGas: toHex(GAS_2_MOCK + SIMULATE_GAS_MOCK - INTRINSIC_GAS), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), simulationFails: undefined, + gasRevert: undefined, isUpgradeWithData: true, }); }); @@ -993,6 +1071,7 @@ describe('gas', () => { estimatedGas: toHex(GAS_2_MOCK), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), simulationFails: undefined, + gasRevert: undefined, isUpgradeWithData: true, }); }); @@ -1028,6 +1107,7 @@ describe('gas', () => { expect(result).toStrictEqual({ estimatedGas: toHex(GAS_2_MOCK + SIMULATE_GAS_MOCK - INTRINSIC_GAS), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + gasRevert: undefined, isUpgradeWithData: true, simulationFails: undefined, }); @@ -1064,6 +1144,7 @@ describe('gas', () => { expect(result).toStrictEqual({ estimatedGas: expect.any(String), blockGasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + gasRevert: undefined, isUpgradeWithData: true, simulationFails: { debug: { diff --git a/packages/transaction-controller/src/utils/gas.ts b/packages/transaction-controller/src/utils/gas.ts index 1650bcb26f..4bf791ccc0 100644 --- a/packages/transaction-controller/src/utils/gas.ts +++ b/packages/transaction-controller/src/utils/gas.ts @@ -19,6 +19,7 @@ import type { GetSimulationConfig, IsAtomicBatchSupportedRequest, IsAtomicBatchSupportedResult, + Revert, TransactionBatchSingleRequest, TransactionMeta, TransactionParams, @@ -30,6 +31,7 @@ import { } from './eip7702'; import { getGasEstimateBuffer, getGasEstimateFallback } from './feature-flags'; import { getChainId, rpcRequest } from './provider'; +import { decodeRevert } from './revert-reason'; export type UpdateGasRequest = { isCustomNetwork: boolean; @@ -64,12 +66,17 @@ export async function updateGas(request: UpdateGasRequest): Promise { const { txMeta } = request; const initialParams = { ...txMeta.txParams }; - const [gas, simulationFails, gasLimitNoBuffer] = await getGas(request); + const [gas, simulationFails, gasLimitNoBuffer, gasRevert] = + await getGas(request); txMeta.txParams.gas = gas; txMeta.simulationFails = simulationFails; txMeta.gasLimitNoBuffer = gasLimitNoBuffer; + if (gasRevert) { + txMeta.revert = { ...txMeta.revert, gas: gasRevert }; + } + if (!initialParams.gas) { txMeta.originalGasEstimate = txMeta.txParams.gas; } @@ -108,6 +115,7 @@ export async function estimateGas({ }): Promise<{ blockGasLimit: string; estimatedGas: string; + gasRevert?: Revert; isUpgradeWithData: boolean; simulationFails: TransactionMeta['simulationFails']; }> { @@ -149,6 +157,7 @@ export async function estimateGas({ let estimatedGas = fallback; let simulationFails: TransactionMeta['simulationFails']; + let gasRevert: Revert | undefined; const isUpgradeWithData = txParams.type === TransactionEnvelopeType.setCode && @@ -185,12 +194,15 @@ export async function estimateGas({ }, }; - log('Estimation failed', { ...simulationFails, fallback }); + gasRevert = decodeRevert(error, 'gas'); + + log('Estimation failed', { ...simulationFails, fallback, gasRevert }); } return { blockGasLimit, estimatedGas, + gasRevert, isUpgradeWithData, simulationFails, }; @@ -422,7 +434,7 @@ export async function simulateGasBatch({ */ async function getGas( request: UpdateGasRequest, -): Promise<[string, TransactionMeta['simulationFails']?, string?]> { +): Promise<[string, TransactionMeta['simulationFails']?, string?, Revert?]> { const { isCustomNetwork, isSimulationEnabled, @@ -444,14 +456,19 @@ async function getGas( return [FIXED_GAS, undefined, FIXED_GAS]; } - const { blockGasLimit, estimatedGas, isUpgradeWithData, simulationFails } = - await estimateGas({ - isSimulationEnabled, - getSimulationConfig, - messenger, - networkClientId, - txParams: txMeta.txParams, - }); + const { + blockGasLimit, + estimatedGas, + gasRevert, + isUpgradeWithData, + simulationFails, + } = await estimateGas({ + isSimulationEnabled, + getSimulationConfig, + messenger, + networkClientId, + txParams: txMeta.txParams, + }); log('Original estimated gas', estimatedGas); @@ -464,7 +481,7 @@ async function getGas( } if (simulationFails || disableGasBuffer) { - return [estimatedGas, simulationFails, estimatedGas]; + return [estimatedGas, simulationFails, estimatedGas, gasRevert]; } const bufferMultiplier = getGasEstimateBuffer({ @@ -484,7 +501,7 @@ async function getGas( log('Buffered gas', bufferedGas); - return [bufferedGas, simulationFails, estimatedGas]; + return [bufferedGas, simulationFails, estimatedGas, gasRevert]; } /** diff --git a/packages/transaction-controller/src/utils/revert-reason.test.ts b/packages/transaction-controller/src/utils/revert-reason.test.ts new file mode 100644 index 0000000000..4133c8902e --- /dev/null +++ b/packages/transaction-controller/src/utils/revert-reason.test.ts @@ -0,0 +1,228 @@ +import type { TransactionControllerMessenger } from '../TransactionController'; +import type { TransactionMeta } from '../types'; +import { rpcRequest } from './provider'; +import { + decodeRevert, + extractRevert, + OnChainFailureError, +} from './revert-reason'; + +jest.mock('./provider', () => ({ + ...jest.requireActual('./provider'), + rpcRequest: jest.fn(), +})); + +const ERROR_DATA_TRANSFER_EXCEEDS_BALANCE = + '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000'; + +const PANIC_DATA_OVERFLOW = + '0x4e487b710000000000000000000000000000000000000000000000000000000000000011'; + +const CUSTOM_ERROR_DATA = + '0xdeadbeef000000000000000000000000000000000000000000000000000000000000007b0000000000000000000000001111111111111111111111111111111111111111'; + +const NETWORK_CLIENT_ID_MOCK = 'networkClient1'; + +const TX_PARAMS_MOCK = { + from: `0x${'11'.repeat(20)}`, + to: `0x${'22'.repeat(20)}`, + data: '0xa9059cbb', + value: '0x0', + gas: '0x5208', +} as unknown as TransactionMeta['txParams']; + +const messengerMock = {} as unknown as TransactionControllerMessenger; + +describe('decodeRevert', () => { + it('decodes Error(string) reverts', () => { + expect(decodeRevert(ERROR_DATA_TRANSFER_EXCEEDS_BALANCE)).toStrictEqual({ + message: 'ERC20: transfer amount exceeds balance', + data: ERROR_DATA_TRANSFER_EXCEEDS_BALANCE, + }); + }); + + it('decodes Panic(uint256) reverts with a known code', () => { + expect(decodeRevert(PANIC_DATA_OVERFLOW)).toStrictEqual({ + message: 'Panic: Arithmetic overflow or underflow', + data: PANIC_DATA_OVERFLOW, + }); + }); + + it('decodes Panic(uint256) reverts with an unknown code', () => { + const data = + '0x4e487b7100000000000000000000000000000000000000000000000000000000000000ff'; + expect(decodeRevert(data)).toStrictEqual({ + message: 'Panic: Unknown panic', + data, + }); + }); + + it('falls back to the selector for custom errors', () => { + expect(decodeRevert(CUSTOM_ERROR_DATA)).toStrictEqual({ + message: 'Custom error: 0xdeadbeef', + data: CUSTOM_ERROR_DATA, + }); + }); + + it('returns undefined for empty revert data', () => { + expect(decodeRevert('0x')).toBeUndefined(); + }); + + it('returns undefined for missing data', () => { + expect(decodeRevert(undefined)).toBeUndefined(); + }); + + it('surfaces raw hex for short data without a full selector', () => { + expect(decodeRevert('0x1234')).toStrictEqual({ + message: 'execution reverted (0x1234)', + data: '0x1234', + }); + }); + + it('returns data only when Error(string) payload is malformed', () => { + const data = '0x08c379a0deadbeef'; + expect(decodeRevert(data)).toStrictEqual({ data }); + }); + + it('returns data only when Panic(uint256) payload is malformed', () => { + const data = '0x4e487b71deadbeef'; + expect(decodeRevert(data)).toStrictEqual({ data }); + }); + + it('decodes data on a thrown JSON-RPC error', () => { + expect(decodeRevert({ data: PANIC_DATA_OVERFLOW })).toStrictEqual({ + message: 'Panic: Arithmetic overflow or underflow', + data: PANIC_DATA_OVERFLOW, + }); + }); + + it('decodes data nested one level deeper', () => { + expect(decodeRevert({ data: { data: PANIC_DATA_OVERFLOW } })).toStrictEqual( + { + message: 'Panic: Arithmetic overflow or underflow', + data: PANIC_DATA_OVERFLOW, + }, + ); + }); + + it('returns undefined when error has no hex data', () => { + expect(decodeRevert({ data: '0x' })).toBeUndefined(); + expect(decodeRevert(null)).toBeUndefined(); + }); +}); + +describe('extractRevert', () => { + const rpcRequestMock = jest.mocked(rpcRequest); + + beforeEach(() => { + rpcRequestMock.mockReset(); + }); + + it('returns a Revert with decoded message and raw data from provider error', async () => { + rpcRequestMock.mockRejectedValueOnce({ + message: 'execution reverted', + data: ERROR_DATA_TRANSFER_EXCEEDS_BALANCE, + }); + + expect( + await extractRevert({ + messenger: messengerMock, + networkClientId: NETWORK_CLIENT_ID_MOCK, + txParams: TX_PARAMS_MOCK, + }), + ).toStrictEqual({ + message: 'ERC20: transfer amount exceeds balance', + data: ERROR_DATA_TRANSFER_EXCEEDS_BALANCE, + }); + }); + + it('returns undefined when the replay does not revert', async () => { + rpcRequestMock.mockResolvedValueOnce('0x'); + + expect( + await extractRevert({ + messenger: messengerMock, + networkClientId: NETWORK_CLIENT_ID_MOCK, + txParams: TX_PARAMS_MOCK, + }), + ).toBeUndefined(); + }); + + it('returns undefined when the error has no data', async () => { + rpcRequestMock.mockRejectedValueOnce({ + message: 'execution reverted: Custom message from node', + }); + + expect( + await extractRevert({ + messenger: messengerMock, + networkClientId: NETWORK_CLIENT_ID_MOCK, + txParams: TX_PARAMS_MOCK, + }), + ).toBeUndefined(); + }); + + it('skips extraction when txParams has neither `to` nor `data`', async () => { + expect( + await extractRevert({ + messenger: messengerMock, + networkClientId: NETWORK_CLIENT_ID_MOCK, + txParams: {} as TransactionMeta['txParams'], + }), + ).toBeUndefined(); + expect(rpcRequestMock).not.toHaveBeenCalled(); + }); + + it('returns undefined for non-object provider errors', async () => { + rpcRequestMock.mockRejectedValueOnce('boom'); + + expect( + await extractRevert({ + messenger: messengerMock, + networkClientId: NETWORK_CLIENT_ID_MOCK, + txParams: TX_PARAMS_MOCK, + }), + ).toBeUndefined(); + }); + + it('does not pass `gas` from txParams (forces full estimation)', async () => { + rpcRequestMock.mockRejectedValueOnce({ + message: 'execution reverted', + data: ERROR_DATA_TRANSFER_EXCEEDS_BALANCE, + }); + + await extractRevert({ + messenger: messengerMock, + networkClientId: NETWORK_CLIENT_ID_MOCK, + txParams: TX_PARAMS_MOCK, + }); + + const [callArgs] = rpcRequestMock.mock.calls[0]; + expect( + (callArgs.params as [Record])[0], + ).not.toHaveProperty('gas'); + }); +}); + +describe('OnChainFailureError', () => { + it('formats the message with a revert reason when present', () => { + const error = new OnChainFailureError({ message: 'insufficient funds' }); + expect(error.message).toBe( + 'Transaction failed on-chain: insufficient funds', + ); + expect(error.revert).toStrictEqual({ message: 'insufficient funds' }); + expect(error.name).toBe('OnChainFailureError'); + }); + + it('formats the message with raw data fallback when message is missing', () => { + const error = new OnChainFailureError({ data: '0xabcd' }); + expect(error.message).toBe('Transaction failed on-chain'); + expect(error.revert).toStrictEqual({ data: '0xabcd' }); + }); + + it('formats the message without a suffix when revert is missing', () => { + const error = new OnChainFailureError(); + expect(error.message).toBe('Transaction failed on-chain'); + expect(error.revert).toBeUndefined(); + }); +}); diff --git a/packages/transaction-controller/src/utils/revert-reason.ts b/packages/transaction-controller/src/utils/revert-reason.ts new file mode 100644 index 0000000000..9079b4bade --- /dev/null +++ b/packages/transaction-controller/src/utils/revert-reason.ts @@ -0,0 +1,236 @@ +import { defaultAbiCoder } from '@ethersproject/abi'; +import type { NetworkClientId } from '@metamask/network-controller'; +import type { Hex } from '@metamask/utils'; + +import { createModuleLogger, projectLogger } from '../logger'; +import type { TransactionControllerMessenger } from '../TransactionController'; +import type { Revert, TransactionParams } from '../types'; +import { rpcRequest } from './provider'; + +const log = createModuleLogger(projectLogger, 'revert-reason'); + +const ERROR_SELECTOR = '0x08c379a0'; +const PANIC_SELECTOR = '0x4e487b71'; + +/** + * Solidity panic code descriptions. + * + * @see https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require + */ +const PANIC_CODE_MESSAGES: Record = { + '0x00': 'Generic compiler panic', + '0x01': 'Assertion failed', + '0x11': 'Arithmetic overflow or underflow', + '0x12': 'Division or modulo by zero', + '0x21': 'Invalid enum value', + '0x22': 'Incorrectly encoded storage byte array', + '0x31': 'Pop on empty array', + '0x32': 'Array index out of bounds', + '0x41': 'Memory allocation overflow', + '0x51': 'Call to zero-initialized function', +}; + +/** + * Input accepted by `decodeRevert`. Either raw revert data hex, or a + * thrown JSON-RPC error carrying the data on `data` (standard) or + * nested `data.data` (some older node forks). + */ +type RevertInput = + | Hex + | undefined + | { data?: Hex | { data?: Hex } | null } + | null; + +/** + * Error thrown by `PendingTransactionTracker` when a transaction failed + * on-chain, carrying any decoded revert. + */ +export class OnChainFailureError extends Error { + readonly revert?: Revert; + + constructor(revert?: Revert) { + const suffix = revert?.message ? `: ${revert.message}` : ''; + super(`Transaction failed on-chain${suffix}`); + this.name = 'OnChainFailureError'; + this.revert = revert; + } +} + +/** + * Decode an EVM revert into a `Revert` containing both the decoded + * human-readable message and the original raw `data`. Accepts either a + * raw revert data hex string, or a thrown JSON-RPC error of the shape + * `{ data: Hex | { data: Hex } }`. Handles `Error(string)`, + * `Panic(uint256)`, and falls back to a raw `Custom error: 0x` + * reference for unknown custom errors. + * + * @param input - Raw revert data hex, or a thrown JSON-RPC error. + * @param source - Optional label included in debug logs to identify the + * caller (e.g. `gas`, `simulation`). + * @returns A `Revert` if any data was found, otherwise `undefined`. + */ +export function decodeRevert( + input: RevertInput, + source?: string, +): Revert | undefined { + const data = toRevertDataHex(input); + if (!data) { + return undefined; + } + + const message = decodeMessage(data); + const revert: Revert = { + ...(message ? { message } : {}), + data, + }; + + logRevert(source, revert); + return revert; +} + +/** + * Replay a transaction that failed on-chain via `eth_estimateGas` to + * recover the revert reason. `eth_estimateGas` is used instead of + * `eth_call` to avoid `RetryOnEmptyMiddleware`, which retries reverted + * `eth_call` responses 10 times and discards the original error data. + * + * Always resolves; never throws. + * + * @param input - Extraction inputs. + * @param input.messenger - Transaction controller messenger. + * @param input.networkClientId - Network client ID to replay against. + * @param input.txParams - Transaction parameters for the failed tx. + * @returns A `Revert`, or `undefined` if none could be observed. + */ +export async function extractRevert({ + messenger, + networkClientId, + txParams, +}: { + messenger: TransactionControllerMessenger; + networkClientId: NetworkClientId; + txParams: TransactionParams; +}): Promise { + if (!txParams?.to && !txParams?.data) { + return undefined; + } + + const callParams: Record = {}; + if (txParams.from) { + callParams.from = txParams.from; + } + if (txParams.to) { + callParams.to = txParams.to; + } + if (txParams.data) { + callParams.data = txParams.data; + } + if (txParams.value) { + callParams.value = txParams.value; + } + + try { + await rpcRequest({ + messenger, + networkClientId, + // `eth_estimateGas` is used instead of `eth_call` to bypass + // `RetryOnEmptyMiddleware`, which retries reverted `eth_call` + // responses and discards the original revert data. + method: 'eth_estimateGas', + params: [callParams], + }); + return undefined; + } catch (error: unknown) { + return decodeRevert(error as RevertInput, 'receipt'); + } +} + +/** + * Coerce a `RevertInput` to a non-empty revert data hex string. + * + * @param input - Raw hex or thrown JSON-RPC error. + * @returns The revert data hex, or `undefined`. + */ +function toRevertDataHex(input: RevertInput): Hex | undefined { + if (isHex(input)) { + return input; + } + + if (typeof input !== 'object' || input === null) { + return undefined; + } + + const { data } = input; + if (isHex(data)) { + return data; + } + + if (typeof data === 'object' && data !== null && isHex(data.data)) { + return data.data; + } + + return undefined; +} + +/** + * Type guard for non-empty `0x`-prefixed hex strings. + * + * @param value - Value to test. + * @returns Whether the value is a non-empty hex string. + */ +function isHex(value: unknown): value is Hex { + return typeof value === 'string' && value.startsWith('0x') && value !== '0x'; +} + +/** + * Decode raw revert data into a human-readable string. + * + * @param data - Raw revert data hex. + * @returns Decoded message, or `undefined` if undecodable. + */ +function decodeMessage(data: Hex | undefined): string | undefined { + if (!data || data === '0x') { + return undefined; + } + + if (data.length < 10) { + return `execution reverted (${data})`; + } + + const selector = data.slice(0, 10).toLowerCase(); + const payload = `0x${data.slice(10)}`; + + if (selector === ERROR_SELECTOR) { + try { + const [reason] = defaultAbiCoder.decode(['string'], payload); + return typeof reason === 'string' ? reason : undefined; + } catch { + return undefined; + } + } + + if (selector === PANIC_SELECTOR) { + try { + const [code] = defaultAbiCoder.decode(['uint256'], payload) as [ + { toHexString: () => string }, + ]; + const codeHex = code.toHexString().toLowerCase() as Hex; + const description = PANIC_CODE_MESSAGES[codeHex] ?? 'Unknown panic'; + return `Panic: ${description}`; + } catch { + return undefined; + } + } + + return `Custom error: ${selector}`; +} + +/** + * Emit a single structured debug line for a decoded revert. + * + * @param source - Source label, when known. + * @param revert - Resolved Revert. + */ +function logRevert(source: string | undefined, revert: Revert): void { + log('Decoded revert', source ?? 'unknown', revert); +}