Derive transaction status-change events from state in `#updateTransactionInternal`

Open
  • Is something wrong?
#9,412 0 comments 0 reactions 1 assignee View on GitHub

Maintainers usually reply within 1 day

@matthewwalsh0 is already working on this.

Since Jul 7, 2026.

Assessment

This issue has not been assessed yet.

Description

team-confirmations

What

Emit transaction status-change events by diffing status inside #updateTransactionInternal, instead of relying on callers to manually invoke #onTransactionStatusChange (and the status-specific transactionFailed / transactionConfirmed / transactionDropped / transactionFinished publishes).

Why

Today these events are fired manually at ~8 separate call sites. That makes emitting an event a "conscious decision" the author has to remember, rather than something derived from the state, which should be the source of truth. Any code path that mutates a transaction's status without also calling the emit helpers silently skips the events.

This is exactly what caused https://github.com/MetaMask/core/pull/9400: SmartTransactionsController marked a transaction failed via the plain updateTransaction action, which doesn't emit transactionStatusUpdated / transactionFailed, so the bridge status controller was never notified and the transaction stayed stuck as pending.

#9400 fixed the symptom with a targeted failTransaction action, but the underlying fragility remains.

Proposal

  • Diff previous vs new status inside #updateTransactionInternal and publish the appropriate event(s) automatically after the update.
  • Remove the ~8 manual #onTransactionStatusChange call sites once the diff covers them.
  • Consider whether the public failTransaction action (added in #9400) can then be removed.

Risk / notes

Not just transactionStatusUpdated — the status-specific events (transactionFailed, transactionConfirmed, transactionDropped, transactionFinished) also need to be derived from the diff, and every existing manual emit must be removed to avoid double-firing. This touches every add/approve/sign/submit/confirm/drop/fail path, so it needs careful review and test coverage. Raised as a follow-up from #9400 rather than folded into that fix.

Context: review discussion on https://github.com/MetaMask/core/pull/9400

Dominant language
TypeScript
Stars
422
Forks
313
Avg merge
1d 8h
Merged PRs (30d)
414

Getting set up

  • No Dockerfile or Docker Compose file
  • Has a pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MetaMask/core

All issues in MetaMask/core

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.