Skip to content
Merged
1 change: 1 addition & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 36 additions & 0 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ import type { TransactionControllerMethodActions } from './TransactionController
import type {
DappSuggestedGasFees,
Layer1GasFeeFlow,
Revert,
SavedGasFees,
SecurityProviderRequest,
SendFlowHistoryEntry,
Expand Down Expand Up @@ -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,
};
}
},
);

Expand Down Expand Up @@ -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);
Expand All @@ -4319,6 +4329,7 @@ export class TransactionController extends BaseController<
);
simulationData = balanceChangesResult.simulationData;
gasUsed = balanceChangesResult.gasUsed;
simulationRevert = balanceChangesResult.simulationRevert;

if (
blockTime &&
Expand Down Expand Up @@ -4362,6 +4373,13 @@ export class TransactionController extends BaseController<

if (!this.#isBalanceChangesSkipped(txMeta)) {
txMeta.simulationData = simulationData;

if (simulationRevert) {
txMeta.revert = {
...txMeta.revert,
simulation: simulationRevert,
};
}
}
},
);
Expand Down Expand Up @@ -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(
Expand All @@ -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) {
Expand All @@ -4562,6 +4590,14 @@ export class TransactionController extends BaseController<
...transactionMeta,
status: TransactionStatus.failed,
error: normalizeTxError(error),
...(receiptRevert === undefined
? {}
: {
revert: {
...transactionMeta.revert,
receipt: receiptRevert,
},
}),
};
}

Expand Down
3 changes: 3 additions & 0 deletions packages/transaction-controller/src/api/simulation-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ const RECEIPT_MOCK = {
status: '0x1',
};

const REVERT_DATA_TRANSFER_EXCEEDS_BALANCE =
'0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000';

const BLOCK_MOCK = {
baseFeePerGas: '0x456',
timestamp: 123456,
Expand Down Expand Up @@ -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<BlockTracker>;
let pendingTransactionTracker: PendingTransactionTracker;
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test checks nonexistent property name revertReason instead of revert

Medium Severity

The test asserts emittedError.revertReason is undefined, but OnChainFailureError defines its property as revert, not revertReason. This assertion is vacuously true because a nonexistent property is always undefined, meaning the test provides no regression protection — it would still pass even if .revert were incorrectly populated. The companion test at line 559 correctly uses .revert.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bf8b9d3. Configure here.

});

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,
});
});
});

Expand Down Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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;
}
Expand Down
28 changes: 28 additions & 0 deletions packages/transaction-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
};
Loading
Loading