Skip to content

feat(transaction-controller): revert reasons#8589

Merged
matthewwalsh0 merged 8 commits into
mainfrom
feat/extract-revert-reason
May 4, 2026
Merged

feat(transaction-controller): revert reasons#8589
matthewwalsh0 merged 8 commits into
mainfrom
feat/extract-revert-reason

Conversation

@matthewwalsh0
Copy link
Copy Markdown
Member

@matthewwalsh0 matthewwalsh0 commented Apr 27, 2026

Explanation

When a transaction fails — on-chain or during gas estimation — MetaMask records a generic message with no reason. The actual reason (ERC20: transfer amount exceeds balance, Panic (0x11): Arithmetic overflow or underflow, etc.) is decodable from the raw revert data returned by any standard RPC.

This PR adds a single revert?: RevertSources field on TransactionMeta that captures decoded revert info from up to three independent sources:

  • revert.gas — set when eth_estimateGas reverts during transaction creation (predicted failure)
  • revert.simulation — set from the root call frame's output returned by the simulation API
  • revert.receipt — set when a receipt has status: 0x0, by replaying the call via eth_estimateGas

Each entry is a Revert { message?: string; data?: Hex }. The message is decoded only from the raw data; upstream RPC and simulation message strings are never parsed. Decoder handles Error(string), Panic(uint256) (with named codes), and falls back to Custom error: 0x<selector> for custom errors.

The receipt failure path also appends the decoded message to the existing error: "Transaction failed on-chain: <reason>".

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them — no breaking changes; field is additive

Note

Medium Risk
Touches gas estimation, simulation parsing, and on-chain failure handling to persist and emit decoded revert data; mistakes could change failure attribution or add extra RPC load on failed receipts.

Overview
Adds a new optional TransactionMeta.revert field (typed as RevertData) to persist decoded revert message and raw revert data captured from three sources: eth_estimateGas during gas estimation (revert.gas), simulation call-trace root output (revert.simulation), and on-chain failures by replaying failed receipts via eth_estimateGas (revert.receipt).

Updates gas estimation and simulation flows to extract/decode revert payloads and thread them into transaction updates, and changes pending receipt failure handling to emit an OnChainFailureError that includes the decoded reason in the error message when available. Includes new revert-reason utility (+ tests), extends simulation API types to include call-trace output, and updates unit tests accordingly.

Reviewed by Cursor Bugbot for commit bf8b9d3. Bugbot is set up for automated code reviews on this repo. Configure here.

@matthewwalsh0 matthewwalsh0 force-pushed the feat/extract-revert-reason branch from eff0b70 to f5196a9 Compare April 27, 2026 12:58
@matthewwalsh0 matthewwalsh0 force-pushed the feat/extract-revert-reason branch 2 times, most recently from 988450c to fcfa427 Compare April 29, 2026 15:43
@matthewwalsh0 matthewwalsh0 changed the title feat(transaction-controller): extract revert reason for on-chain failures feat(transaction-controller): extract revert reason for failed transactions Apr 29, 2026
@matthewwalsh0 matthewwalsh0 changed the title feat(transaction-controller): extract revert reason for failed transactions feat(transaction-controller): revert reasons May 1, 2026
…ctions

When a transaction fails on-chain (status 0x0) or is predicted to fail
during gas estimation, decode and surface the revert reason instead of
just "Transaction failed on-chain".

- Add extractRevertReason util that decodes Error(string), Panic(uint256)
  with named codes, custom error selectors, and empty reverts; falls back
  to the provider error message when no revert data is surfaced
- Add revertReason?: string on TransactionMeta, populated when an
  on-chain receipt has a failure status (replays via eth_estimateGas)
- Add gasRevertReason?: string on TransactionMeta, populated when
  eth_estimateGas reverts during transaction creation, allowing UI to
  warn the user before they confirm a tx that's predicted to fail
- Append the on-chain reason to the existing 'Transaction failed
  on-chain' error message via a new OnChainFailureError subclass

eth_estimateGas is used (instead of eth_call) for the on-chain replay
path as a workaround for a bug in eth-json-rpc-middleware's
RetryOnEmptyMiddleware: its isExecutionRevertedError check fails to
recognise EIP-1474 / Infura-style revert errors (code 3, suffixed
message), causing it to retry reverted calls 10 times and discard the
original error data. eth_estimateGas returns the same revert payload
but bypasses that middleware. A separate upstream PR fixes the
underlying bug.
@matthewwalsh0 matthewwalsh0 force-pushed the feat/extract-revert-reason branch from 14e1951 to af1eae9 Compare May 1, 2026 12:35
…tSources schema

Replaces the separate `gasRevertReason` and `revertReason` string fields
on TransactionMeta with a single `revert?: RevertSources` field. Each
source (`gas`, `simulation`, `receipt`) carries an optional decoded
`message` and the raw `data` hex. The decoded message is derived only
from the raw revert data — upstream RPC and Sentinel message strings are
never parsed.

The simulation source reads from the root call frame's `output` only,
ignoring nested errors that may have been caught and recovered. Adds a
unified `logRevert` debug logger emitting one structured line per source
under the `metamask:transaction-controller:revert` namespace.
@matthewwalsh0 matthewwalsh0 force-pushed the feat/extract-revert-reason branch 2 times, most recently from 4e7644e to 585ed5f Compare May 1, 2026 13:10
…t util

- Rename type `RevertSources` \u2192 `RevertData` (parent shape on TransactionMeta).
- Rename file `extract-revert-reason.ts` \u2192 `revert.ts`.
- New single public `decodeRevert(data)` accepting raw hex; returns `Revert`
  with both decoded `message` and the original `data`.
- `extractRevert` swallows all errors internally and returns `undefined`.
- Move simulation revert write inside the `isBalanceChangesSkipped` check
  so `revert.simulation` and `simulationData` stay in sync.
- Single `metamask:transaction-controller:revert` debug logger; drop the
  earlier `extract-revert-reason` namespace.
- Trim `TransactionMeta.revert` JSDoc to one line; details live on the
  per-source `Revert` and on `RevertData`.
- Link Solidity panic spec from `PANIC_CODE_MESSAGES` JSDoc; codes verified
  against https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require.
@matthewwalsh0 matthewwalsh0 force-pushed the feat/extract-revert-reason branch from 585ed5f to 774c7a8 Compare May 1, 2026 13:18
- decodeRevert now takes Hex | undefined; callers extract hex themselves
- Export extractRevertDataHex helper for unwrapping JSON-RPC errors
- Drop panic code from message (e.g. 'Arithmetic overflow or underflow')
- Add inline comment on eth_estimateGas explaining eth_call retry workaround
…efix

- decodeRevert accepts RevertInput (Hex | { data: Hex | { data: Hex } } | null | undefined) directly; no separate hex-extraction export
- Restore 'Panic: <description>' prefix on panic messages (without code)
- Move Revert/RevertData types to bottom of types.ts
- Cleaner logRevert: 'Decoded <source> revert' { message, data }
@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review May 1, 2026 14:57
@matthewwalsh0 matthewwalsh0 requested review from a team as code owners May 1, 2026 14:57
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bf8b9d3. Configure here.

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.

@matthewwalsh0 matthewwalsh0 added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit c8fcdfa May 4, 2026
715 of 725 checks passed
@matthewwalsh0 matthewwalsh0 deleted the feat/extract-revert-reason branch May 4, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants