Skip to content

fix: transaction id row - #44188

Merged
n3ps merged 1 commit into
mainfrom
n3ps/failed-tx-hash
Jul 9, 2026
Merged

fix: transaction id row#44188
n3ps merged 1 commit into
mainfrom
n3ps/failed-tx-hash

Conversation

@n3ps

@n3ps n3ps commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Check the tx identifier before rendering the Transaction Id row. Remediates the issue of always showing the Transaction Id row despite not having a valid transaction hash.

Changelog

CHANGELOG entry: fix: rejected tx showing transaction id link

Related issues

Fixes: #44136

Manual testing steps

  1. Start a send and reject on the confirmation screen.
  2. Open Activity and tap the failed send.
  3. Confirm there is no Transaction Id row and no View on block explorer button.
  4. Repeat with a confirmed or reverted on-chain send and confirm Transaction Id still appears with a valid hash.

Pre-merge author checklist

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.

Made with Cursor

Off-chain failures (e.g. user reject) fall back to MetaMask meta ids in
activity hash; only show Transaction Id and explorer links for valid EVM
hashes or non-EVM chain ids.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamask-ci metamask-ci Bot added the team-core-extension-ux Core Extension UX team label Jul 6, 2026
@metamask-ci

metamask-ci Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

✨ Files requiring CODEOWNER review ✨

👨‍🔧 @MetaMask/core-extension-ux (3 files, +25 -5)
  • 📁 ui/
    • 📁 pages/
      • 📁 details/
        • 📁 components/
          • 📄 sections.tsx +7 -1
          • 📄 shared.tsx +9 -3
        • 📁 templates/
          • 📄 bridge-details.tsx +9 -1

@n3ps n3ps changed the title fix(details): hide transaction id for off-chain failed sends fix: remove transaction id row for off-chain failures Jul 6, 2026
@github-actions github-actions Bot added the size-S label Jul 6, 2026
@n3ps
n3ps marked this pull request as ready for review July 6, 2026 04:22
@n3ps
n3ps requested a review from a team as a code owner July 6, 2026 04:22
@n3ps n3ps changed the title fix: remove transaction id row for off-chain failures fix: rejected transaction showing tx id row Jul 6, 2026
<Row
label={t('transactionIdLabel')}
value={<TransactionId value={sourceTxHash} />}
value={txId ? <TransactionId value={txId} /> : null}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Point of interest

@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
9.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@metamask-ci

metamask-ci Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor
Builds ready [6ac9e0b]
Deprecated Browserify fallback builds
⚡ Performance Benchmarks (Total: 🟢 17 pass · 🟡 8 warn · 🔴 0 fail)

Baseline (latest main): 1716549 | Date: 6/29/2026 | Pipeline: 28767466332 | Baseline logs

Interaction Benchmarks · Samples: 5
Benchmarkchrome-webpackfirefox-webpack
loadNewAccount
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]
confirmTx
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]
bridgeUserActions
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]
🟡 bridge_search_token

📈 Results compared to the previous 5 runs on main

  • loadNewAccount/load_new_account: -19%
  • loadNewAccount/total: -19%
  • loadNewAccount/inp: +15%
  • confirmTx/tbt: -11%
  • bridgeUserActions/bridge_load_page: +10%
  • bridgeUserActions/bridge_load_asset_picker: -22%
  • bridgeUserActions/longTaskCount: -17%
  • bridgeUserActions/longTaskTotalDuration: -15%
  • bridgeUserActions/tbt: -12%
  • bridgeUserActions/inp: -14%
  • loadNewAccount/load_new_account: +47%
  • loadNewAccount/total: +47%
  • loadNewAccount/fcp: -51%
  • loadNewAccount/lcp: +1040%
  • confirmTx/confirm_tx: +10%
  • confirmTx/longTaskCount: -100%
  • confirmTx/longTaskTotalDuration: -100%
  • confirmTx/longTaskMaxDuration: -100%
  • confirmTx/tbt: -100%
  • confirmTx/total: +10%
  • confirmTx/fcp: -48%
  • confirmTx/lcp: +1102%
  • bridgeUserActions/bridge_load_page: +101%
  • bridgeUserActions/bridge_load_asset_picker: +87%
  • bridgeUserActions/bridge_search_token: +243%
  • bridgeUserActions/longTaskCount: -100%
  • bridgeUserActions/longTaskTotalDuration: -100%
  • bridgeUserActions/longTaskMaxDuration: -100%
  • bridgeUserActions/tbt: -100%
  • bridgeUserActions/total: +197%
  • bridgeUserActions/inp: +38%
  • bridgeUserActions/fcp: -51%
  • bridgeUserActions/lcp: +1102%
Startup Benchmarks · Samples: 100
Benchmarkchrome-webpackfirefox-webpack
startupStandardHome
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]
startupPowerUserHome
[Sentry log · main/release]
🟡 [CI log]🟡 [CI log]

📈 Results compared to the previous 5 runs on main

  • startupStandardHome/numNetworkReqs: +10%
  • startupStandardHome/domInteractive: -23%
  • startupStandardHome/fcp: -22%
  • startupPowerUserHome/load: +13%
  • startupPowerUserHome/domContentLoaded: +13%
  • startupPowerUserHome/domInteractive: +30%
  • startupPowerUserHome/backgroundConnect: +98%
  • startupPowerUserHome/firstReactRender: +11%
  • startupPowerUserHome/loadScripts: +14%
  • startupPowerUserHome/inp: -12%
  • startupPowerUserHome/fcp: +28%

🌐 Core Web Vitals — 🟢 good · 🟡 needs improvement · 🔴 poor (web.dev thresholds)

  • 🔴 startupPowerUserHome/INP: p75 552ms
  • 🟡 startupPowerUserHome/LCP: p75 3.9s
User Journey Benchmarks · Samples: 5 · mock API
Benchmarkchrome-webpackfirefox-webpack
onboardingImportWallet
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]
onboardingNewWallet
[Sentry log · main/release]
🟢 [CI log]🟡 [CI log]
🟡 total
assetDetails
[Sentry log · main/release]
🟡 [CI log]🟡 [CI log]
solanaAssetDetails
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]
importSrpHome
[Sentry log · main/release]
🟡 [CI log]🟡 [CI log]
sendTransactions
[Sentry log · main/release]
🟡 [CI log]🟢 [CI log]
swap
[Sentry log · main/release]
🟢 [CI log]🟢 [CI log]

📈 Results compared to the previous 5 runs on main

  • onboardingImportWallet/metricsToWalletReadyScreen: -20%
  • onboardingImportWallet/doneButtonToHomeScreen: -86%
  • onboardingImportWallet/openAccountMenuToAccountListLoaded: -50%
  • onboardingImportWallet/longTaskCount: -60%
  • onboardingImportWallet/longTaskTotalDuration: -88%
  • onboardingImportWallet/longTaskMaxDuration: -91%
  • onboardingImportWallet/tbt: -99%
  • onboardingImportWallet/total: -83%
  • onboardingNewWallet/doneButtonToAssetList: -30%
  • onboardingNewWallet/longTaskCount: -50%
  • onboardingNewWallet/longTaskTotalDuration: -59%
  • onboardingNewWallet/longTaskMaxDuration: -28%
  • onboardingNewWallet/tbt: -67%
  • onboardingNewWallet/total: -26%
  • solanaAssetDetails/assetClickToPriceChart: -58%
  • solanaAssetDetails/total: -58%
  • solanaAssetDetails/cls: +1620%
  • importSrpHome/loginToHomeScreen: -16%
  • importSrpHome/homeAfterImportWithNewWallet: -18%
  • importSrpHome/longTaskCount: -27%
  • importSrpHome/longTaskTotalDuration: -32%
  • importSrpHome/longTaskMaxDuration: -17%
  • importSrpHome/tbt: -36%
  • importSrpHome/total: -17%
  • importSrpHome/inp: -21%
  • importSrpHome/lcp: -56%
  • importSrpHome/cls: -47%
  • sendTransactions/openSendPageFromHome: -36%
  • sendTransactions/selectTokenToSendFormLoaded: -57%
  • sendTransactions/reviewTransactionToConfirmationPage: -13%
  • sendTransactions/longTaskCount: -20%
  • sendTransactions/longTaskTotalDuration: -25%
  • sendTransactions/tbt: -47%
  • sendTransactions/total: -20%
  • sendTransactions/inp: -19%
  • sendTransactions/cls: -40%

🌐 Core Web Vitals — 🟢 good · 🟡 needs improvement · 🔴 poor (web.dev thresholds)

  • 🟡 assetDetails/FCP: p75 2.0s
  • 🟡 importSrpHome/INP: p75 208ms
  • 🟡 sendTransactions/FCP: p75 1.9s
  • 🟡 assetDetails/FCP: p75 1.8s
  • 🟡 importSrpHome/FCP: p75 1.9s
Dapp Page Load Benchmarks · Samples: 100
Benchmarkchrome-webpack
dappPageLoad
[Sentry log · main/release]
🟢 [CI log]
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 69.96 KiB (0.63%)
  • ui: 42.35 KiB (0.24%)
  • common: 0 Bytes (0%)
  • other: 0 Bytes (0%)
  • contentScripts: 726 Bytes (0.04%)
  • zip: 32.76 KiB (0.12%)

@n3ps
n3ps enabled auto-merge July 6, 2026 13:43
@n3ps n3ps changed the title fix: rejected transaction showing tx id row fix: transaction id row Jul 6, 2026
@n3ps
n3ps added this pull request to the merge queue Jul 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 6, 2026
@n3ps
n3ps added this pull request to the merge queue Jul 9, 2026
@n3ps n3ps added the retry-ci Tells GitHub Actions to retry failed jobs, label removed automatically before the retry label Jul 9, 2026
@github-actions github-actions Bot removed the retry-ci Tells GitHub Actions to retry failed jobs, label removed automatically before the retry label Jul 9, 2026
@HowardBraham HowardBraham added the retry-ci Tells GitHub Actions to retry failed jobs, label removed automatically before the retry label Jul 9, 2026
@github-actions github-actions Bot removed the retry-ci Tells GitHub Actions to retry failed jobs, label removed automatically before the retry label Jul 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 9, 2026
@n3ps
n3ps added this pull request to the merge queue Jul 9, 2026
@n3ps n3ps added the retry-ci Tells GitHub Actions to retry failed jobs, label removed automatically before the retry label Jul 9, 2026
Merged via the queue into main with commit 2ea4542 Jul 9, 2026
238 of 240 checks passed
@n3ps
n3ps deleted the n3ps/failed-tx-hash branch July 9, 2026 02:08
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 9, 2026
@github-actions github-actions Bot removed the retry-ci Tells GitHub Actions to retry failed jobs, label removed automatically before the retry label Jul 9, 2026
@metamaskbot metamaskbot added the release-13.40.0 Issue or pull request that will be included in release 13.40.0 label Jul 9, 2026
@MajorLift

MajorLift commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

🧪 Validation Run

Verdict: ⚠️ the predicate already exists in the tree, and one of its two conjuncts is untested — Claim: isValidTransactionHash gives the details rows a validity check for EVM transaction hashes. head 2ea45420776 · 2026-08-02 · duplicate-symbol and mutation checks

Note

Trial run of the MetaMask evidence skills
feedback welcome, on the finding or on whether this format is useful to a reviewer.
Not a review verdict; nothing here blocks the PR.

The same function, same body, is exported from two modules at this commit:

Both are hash.length === 66 && isStrictHexString(hash). They agree today, which is what makes the divergence risk live rather than theoretical — a fix applied to one passes review against the other unchanged. shared/lib is the better home, so the shorter path is claims-form/utils.ts importing it.

Separately, each conjunct of the new copy was deleted on its own in CI and the suite re-run.

Falsification probe — falsifying

Arm Mutation Result
A — baseline none Test Suites: 1 passed, 1 total Tests: 23 passed, 23 total
B — mutant shared/lib/transactions.utils.ts:9 replaced Test Suites: 1 failed, 1 total Tests: 1 failed, 22 passed, 23 total

The suite fails when the mechanism is removed and passes when restored, running the same 23 tests in both arms. The test has power.

Failing under mutation:

  • ● Transactions utils › isValidTransactionHash › returns false for truncated EVM hashes

Produced by falsify-probe.sh at 2ea45420776402150d9ae18c7e6eb437120cb900 · node v24.13.1 · yarn.lock 09f97e3657b5e353 · 0 tracked changes. Run: https://github.com/MajorLift/metamask-skills/actions/runs/30750283675 — logs and artifacts attached there.

Falsification probe — vacuous

Arm Mutation Result
A — baseline none Test Suites: 1 passed, 1 total Tests: 23 passed, 23 total
B — mutant shared/lib/transactions.utils.ts:9 replaced Test Suites: 1 passed, 1 total Tests: 23 passed, 23 total

The suite passes with the mechanism removed. It does not test what it appears to test.

Produced by falsify-probe.sh at 2ea45420776402150d9ae18c7e6eb437120cb900 · node v24.13.1 · yarn.lock 09f97e3657b5e353 · 0 tracked changes. Run: https://github.com/MajorLift/metamask-skills/actions/runs/30748922870 — logs and artifacts attached there.

Follows from the arms above

  • The length check is guarded: dropping it fails returns false for truncated EVM hashes.
  • isStrictHexString is not. The tested inputs are a valid hash, a 36-character UUID, and 0x1234 — all rejected on length, so nothing distinguishes a 66-character hex string from a 66-character non-hex one.
  • The guard decides whether getExplorerTxUrl builds a block-explorer link, so a value passing it wrongly produces a link that resolves to nothing.

Open for review: the duplicate is the actionable half. The untested conjunct matters only if a producer can put a 66-character non-hex value on that path — the UUID case this change is for is caught on length, and nothing here says what else reaches txHash.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-13.40.0 Issue or pull request that will be included in release 13.40.0 risk:high size-S team-core-extension-ux Core Extension UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Activity - Rejected transaction displays incorrect tx id value (not a hex tx id)

6 participants