fix: fail cancelled smart transactions through the standard failure path - #9400
Conversation
When the relay cancels a smart transaction (e.g. FAILED_GAS_TOO_LOW), the smart-transactions-controller marked the associated regular transaction as failed via `TransactionController:updateTransaction`, which only patches state and does not emit any transaction lifecycle events. Consumers that react to `transactionFailed`/`transactionStatusUpdated` — notably the bridge status controller and transaction metrics — were therefore never notified, leaving cancelled smart transactions (such as bridges) stuck as pending indefinitely. - transaction-controller: add a public `failTransaction(transactionId, error)` method and `TransactionController:failTransaction` messenger action that fails a transaction through the internal fail path, emitting `transactionFailed`, `transactionStatusUpdated`, and `transactionFinished`. - smart-transactions-controller: use `failTransaction` instead of `updateTransaction` when marking regular transactions as failed.
| * @param transactionId - The ID of the transaction to mark as failed. | ||
| * @param error - The error describing why the transaction failed. | ||
| */ | ||
| failTransaction(transactionId: string, error: Error): void { |
There was a problem hiding this comment.
This will ultimately generate the same state as the previous update, except the explicit calls to #onTransactionStatusChange means the status change events are a conscious decision rather than derived from the state which should be the source of truth.
Could we avoid any changes to the SmartTransactionController and capture all status changes entirely automatically, if we instead scrap the calls to #onTransactionStatusChange and instead diff the previous and new status inside #updateTransactionInternal and throw the event(s) there after the update call?
Or Confirmations team could do that in a follow up to minimise effort here if time-sensitive.
There was a problem hiding this comment.
I think this is the cleanest approach but it has more overhead given the additional events too, will come back to this internally later given the risk.
There was a problem hiding this comment.
Yeah, agreed that's the cleaner long-term shape. Deriving the events from the status diff in #updateTransactionInternal would kill off this whole class of "caller forgot to emit" bugs and let us drop the manual #onTransactionStatusChange calls plus this failTransaction wrapper.
I kept it narrow here mainly because it's not just transactionStatusUpdated. Each transition also fires transactionFailed/transactionConfirmed/transactionDropped/transactionFinished, so doing it right means deriving all of those from the diff and pulling out the ~8 manual call sites so we don't double-fire. Felt like too much surface area to fold into a stuck-bridge fix. This version just goes through the existing #failTransaction, so the end state is identical (like you said) and it matches the other failure paths.
I was hoping we could get in this targeted fix now. I opened an issue for the follow-up refactor so Confirmations can pick it up when there's bandwidth: #9412
|
|
||
| ### Fixed | ||
|
|
||
| - Fail the associated regular transaction via the new `TransactionController:failTransaction` action instead of `TransactionController:updateTransaction` when a smart transaction is cancelled ([#9400](https://github.com/MetaMask/core/pull/9400)) |
There was a problem hiding this comment.
Isn't this a breaking change ?
…ask#33175) ## **Description** Integrates the published core fix for stuck bridge smart transactions into mobile. Changes: - Bump `@metamask/smart-transactions-controller` `^24.2.2` → `25.0.0` - Bump `@metamask/transaction-controller` to `68.4.0` (dependency `^68.3.0` + resolution `68.4.0`) - Update the smart transactions controller messenger to delegate `TransactionController:failTransaction` instead of `TransactionController:updateTransaction` **Why:** When the relay cancelled a smart transaction, the STX controller previously called `updateTransaction`, which only patches state and does not emit transaction lifecycle events. Consumers that react to `transactionFailed`/`transactionStatusUpdated` (the bridge status controller and metrics) were never notified, so cancelled bridge smart transactions stayed **stuck pending indefinitely**. The core fix ([MetaMask/core#9400](MetaMask/core#9400)) adds a `failTransaction` action that fails the tx through the standard path and emits those events; this PR wires mobile's STX messenger to use it. **Note on versions:** `smart-transactions-controller` is pinned to `25.0.0` (and the `transaction-controller` resolution to `68.4.0`) to keep `transaction-controller` within `68.x`. `stx@25.0.1` requires `transaction-controller@^69.0.0`, a larger major bump out of scope for this fix. ## **Changelog** CHANGELOG entry: Fixed bridge smart transactions that could remain stuck as pending after being cancelled by the relay ## **Related issues** Refs: - [MetaMask/core#9400](MetaMask/core#9400) — fail cancelled smart transactions through the standard path - [MetaMask/core#9401](MetaMask/core#9401) — ignore saved gas fees for internal transactions - Released in [MetaMask/core#9421](MetaMask/core#9421) ## **Manual testing steps** 1. Build the branch and set up a wallet with a bridge route on a supported chain. 2. Initiate a bridge that is submitted as a smart transaction. 3. Observe a relay-side cancellation of the smart transaction. 4. Confirm the transaction transitions to **failed** in the activity list (instead of remaining pending) and the bridge status updates accordingly. ## **Screenshots/Recordings** N/A — dependency bump + messenger wiring (no UI changes). ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches transaction lifecycle integration for smart transactions; incorrect wiring could affect how cancelled STX are surfaced, but scope is limited to messenger delegation and a controlled dependency bump. > > **Overview** > **Integrates the core fix for bridge smart transactions that stayed pending after relay cancellation.** > > Bumps `@metamask/smart-transactions-controller` from `^24.2.2` to `25.0.0` and updates the Smart Transactions controller messenger (and related test harnesses) to delegate **`TransactionController:failTransaction`** instead of **`TransactionController:updateTransaction`**. The upgraded STX controller uses the fail path when a relay cancels a smart transaction so **`transactionFailed` / `transactionStatusUpdated`** fire and bridge status and activity UI can leave the pending state. > > No app-layer transaction logic changes beyond messenger wiring and dependency lock updates. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 02840f1. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…ask#44372) ## **Description** Integrates the published core fix for stuck bridge smart transactions into the extension. Changes: - Bump `@metamask/smart-transactions-controller` `^24.2.2` → `25.0.0` - Bump `@metamask/transaction-controller` `^68.2.2` → `^68.3.0` (resolves to `68.4.0`) - Update the smart transactions controller messenger to delegate `TransactionController:failTransaction` instead of `TransactionController:updateTransaction` **Why:** When the relay cancelled a smart transaction, the STX controller previously called `updateTransaction`, which only patches state and does not emit transaction lifecycle events. Consumers that react to `transactionFailed`/`transactionStatusUpdated` (the bridge status controller and metrics) were never notified, so cancelled bridge smart transactions stayed **stuck pending indefinitely**. The core fix ([MetaMask/core#9400](MetaMask/core#9400)) adds a `failTransaction` action that fails the tx through the standard path and emits those events; this PR wires the extension's STX messenger to use it. **Note on versions:** `smart-transactions-controller` is pinned to `25.0.0` (not a caret range) to keep `transaction-controller` within `68.x`. `stx@25.0.1` requires `transaction-controller@^69.0.0`, a larger major bump out of scope for this fix. ## **Changelog** CHANGELOG entry: Fixed bridge smart transactions that could remain stuck as pending after being cancelled by the relay ## **Related issues** Integrates the published core fix: - [MetaMask/core#9400](MetaMask/core#9400) — fail cancelled smart transactions through the standard path - [MetaMask/core#9401](MetaMask/core#9401) — ignore saved gas fees for internal transactions - Released in [MetaMask/core#9421](MetaMask/core#9421) ## **Manual testing steps** 1. Build the branch and set up a wallet with a bridge route on a supported chain. 2. Initiate a bridge that is submitted as a smart transaction. 3. Observe a relay-side cancellation of the smart transaction. 4. Confirm the transaction transitions to **failed** in the activity list (instead of remaining pending) and the bridge status updates accordingly. ## **Screenshots/Recordings** N/A — dependency bump + messenger wiring (no UI changes). ### **Before** Cancelled bridge smart transactions stayed pending indefinitely. ### **After** Cancelled bridge smart transactions transition to failed and notify the bridge status controller / metrics. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches smart-transaction and transaction lifecycle integration; behavior change when relays cancel STXs, but scoped to messenger delegation and a targeted dependency bump. > > **Overview** > Integrates the core fix for **bridge smart transactions stuck pending** after relay cancellation by bumping `@metamask/smart-transactions-controller` to **25.0.0** (pinned) and aligning the lockfile. > > The extension wires the smart transactions controller messenger to delegate **`TransactionController:failTransaction`** instead of **`TransactionController:updateTransaction`**, in both production init and unit test setup, so cancelled STXs go through the standard failure path and emit **`transactionFailed`** / **`transactionStatusUpdated`** for bridge status and metrics. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3985dc9. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## **Description** Integrates the published core fix for stuck bridge smart transactions into the extension. Changes: - Bump `@metamask/smart-transactions-controller` `^24.2.2` → `25.0.0` - Bump `@metamask/transaction-controller` `^68.2.2` → `^68.3.0` (resolves to `68.4.0`) - Update the smart transactions controller messenger to delegate `TransactionController:failTransaction` instead of `TransactionController:updateTransaction` **Why:** When the relay cancelled a smart transaction, the STX controller previously called `updateTransaction`, which only patches state and does not emit transaction lifecycle events. Consumers that react to `transactionFailed`/`transactionStatusUpdated` (the bridge status controller and metrics) were never notified, so cancelled bridge smart transactions stayed **stuck pending indefinitely**. The core fix ([MetaMask/core#9400](MetaMask/core#9400)) adds a `failTransaction` action that fails the tx through the standard path and emits those events; this PR wires the extension's STX messenger to use it. **Note on versions:** `smart-transactions-controller` is pinned to `25.0.0` (not a caret range) to keep `transaction-controller` within `68.x`. `stx@25.0.1` requires `transaction-controller@^69.0.0`, a larger major bump out of scope for this fix. ## **Changelog** CHANGELOG entry: Fixed bridge smart transactions that could remain stuck as pending after being cancelled by the relay ## **Related issues** Integrates the published core fix: - [MetaMask/core#9400](MetaMask/core#9400) — fail cancelled smart transactions through the standard path - [MetaMask/core#9401](MetaMask/core#9401) — ignore saved gas fees for internal transactions - Released in [MetaMask/core#9421](MetaMask/core#9421) ## **Manual testing steps** 1. Build the branch and set up a wallet with a bridge route on a supported chain. 2. Initiate a bridge that is submitted as a smart transaction. 3. Observe a relay-side cancellation of the smart transaction. 4. Confirm the transaction transitions to **failed** in the activity list (instead of remaining pending) and the bridge status updates accordingly. ## **Screenshots/Recordings** N/A — dependency bump + messenger wiring (no UI changes). ### **Before** Cancelled bridge smart transactions stayed pending indefinitely. ### **After** Cancelled bridge smart transactions transition to failed and notify the bridge status controller / metrics. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches smart-transaction and transaction lifecycle integration; behavior change when relays cancel STXs, but scoped to messenger delegation and a targeted dependency bump. > > **Overview** > Integrates the core fix for **bridge smart transactions stuck pending** after relay cancellation by bumping `@metamask/smart-transactions-controller` to **25.0.0** (pinned) and aligning the lockfile. > > The extension wires the smart transactions controller messenger to delegate **`TransactionController:failTransaction`** instead of **`TransactionController:updateTransaction`**, in both production init and unit test setup, so cancelled STXs go through the standard failure path and emit **`transactionFailed`** / **`transactionStatusUpdated`** for bridge status and metrics. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3985dc9. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Explanation
When the transaction relay cancels a smart transaction (e.g.
FAILED_GAS_TOO_LOW— the smart tx never lands on chain),SmartTransactionsControllermarks the associated regular transaction as failed via theTransactionController:updateTransactionaction.updateTransactiononly patches state — it does not emit any transaction lifecycle events. As a result, consumers that react totransactionFailed/transactionStatusUpdatedare never notified:BridgeStatusControllermarks a bridge item failed only from thetransactionStatusUpdated/transactionFailedevent. Since that event never fires for a cancelled smart tx, the bridge history item staysPENDINGforever — the transaction shows as failed, but the bridge UI remains stuck on "Pending".Transaction Finalized) are likely under-reported for the same reason.This was observed in production with bridge smart transactions repeatedly stuck as pending (source tx
status: failed, bridgestatus.status: PENDING), while the relay'sbatchStatusreportedminedTx: cancelled/cancellationReason: too_cheap.Fix
Route the cancellation failure through the standard fail path so the lifecycle events fire.
transaction-controller: add a publicfailTransaction(transactionId, error)method and correspondingTransactionController:failTransactionmessenger action. It fails the transaction through the internal#failTransaction, emittingtransactionFailed,transactionStatusUpdated, andtransactionFinished.smart-transactions-controller:markRegularTransactionsAsFailednow callsfailTransactioninstead ofupdateTransaction.References
Changelog
@metamask/transaction-controllerfailTransactionmethod andTransactionController:failTransactionmessenger action.@metamask/smart-transactions-controllerTransactionController:failTransaction(wasupdateTransaction) so transaction lifecycle events are emitted and downstream consumers no longer leave cancelled smart transactions stuck as pending.TransactionController:failTransaction(previouslyTransactionController:updateTransaction).Checklist
Note
Medium Risk
Touches core transaction lifecycle and messenger permissions for smart-transactions; behavior change is intentional but clients must update delegated actions or cancellation failures will not propagate.
Overview
When a smart transaction is cancelled, the linked regular transaction is now failed through
TransactionController:failTransactioninstead ofupdateTransaction, sotransactionFailed,transactionStatusUpdated, andtransactionFinishedare emitted. That fixes cases where the tx looked failed but bridge history and similar subscribers stayed pending because they only react to those events.@metamask/transaction-controllerexposes publicfailTransaction(transactionId, error)and theTransactionController:failTransactionmessenger action, delegating to the existing internal failure path.@metamask/smart-transactions-controllerupdatesmarkRegularTransactionsAsFailedand messenger allowances to call it with aSmartTransactionFailederror (same matching rules for tx id / hashes; still skips already-failed txs).Integrators: grant
TransactionController:failTransactionto the smart transactions controller messenger (replacingTransactionController:updateTransactionfor this flow).Reviewed by Cursor Bugbot for commit 1cdd855. Bugbot is set up for automated code reviews on this repo. Configure here.