feat(ramps): surface ramp orders in Activity, details, and status toasts - #44948
georgeweiler wants to merge 28 commits into
Conversation
|
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. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/core-extension-ux (31 files, +2130 -41)
🔒 @MetaMask/extension-security-team (1 files, +1 -0)
👨🔧 @MetaMask/money-movement (31 files, +2607 -293)
📜 @MetaMask/policy-reviewers (8 files, +8 -0)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. |
Builds ready [97ca454]
⚡ Performance Benchmarks (Total: 🟢 14 pass · 🟡 9 warn · 🔴 1 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
97ca454 to
f9150ee
Compare
Builds ready [f9150ee]
⚡ Performance Benchmarks (Total: 🟢 17 pass · 🟡 7 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
f9150ee to
63a8c1a
Compare
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Builds ready [e885d8e]
⚡ Performance Benchmarks (Total: 🟢 12 pass · 🟡 0 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [310bc7d]
⚡ Performance Benchmarks (Total: 🟢 17 pass · 🟡 0 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| label={t('network')} | ||
| value={<NetworkName chainId={item.chainId} />} | ||
| /> | ||
| {item.chainId ? ( |
There was a problem hiding this comment.
Was this rendering even when empty?
There was a problem hiding this comment.
yes. This PR adds a lot of defensive code like this because the chainID on ramps orders can be missing until providers update it.
| value={<TransactionStatus status={item.status} hash={item.hash} />} | ||
| value={ | ||
| <div className="flex flex-col items-end gap-0.5"> | ||
| <TransactionStatus status={item.status} hash={item.hash} /> |
There was a problem hiding this comment.
For items that only apply to Ramps, can it be scoped within the Ramps template(s)?
| 'date-header', | ||
| 'item', | ||
| ]); | ||
| expect(grouped).toMatchSnapshot(); |
There was a problem hiding this comment.
Prefer unit tests on functionality over snapshots
|
|
||
| const { container } = render(<ActivityList />); | ||
|
|
||
| expect(container).toMatchSnapshot(); |
There was a problem hiding this comment.
Prefer unit tests on functionality over snapshots
| jest.mock('../../hooks/useFormatters', () => ({ | ||
| useFormatters: () => ({ | ||
| formatMediumDate: (date: Date) => date.toISOString(), | ||
| formatMediumDate: (date: Date | number) => new Date(date).toISOString(), |
There was a problem hiding this comment.
Can the call-site be updated instead?
There was a problem hiding this comment.
Good call. But I think the mock is wrong here, not the call-site. formatMediumDate already takes string | number and the list passes row.date as a timestamp number. It is probably best to update the mock to match that signature instead of changing the call-site to pass a Date.
Keep transaction-details generic by moving ramps order lookup and mapping into templates/ramps. Co-authored-by: Cursor <cursoragent@cursor.com>
Move ramps status/tokens/routing into templates/ramps, restore generic details chrome, and replace activity snapshot assertions with unit checks. Co-authored-by: Cursor <cursoragent@cursor.com>
Stop wrapping generic /tx and activity-list with ramps facades; toasts and deep links use /ramps/order, while the list dialog seeds the selected item into TransactionDetails. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop toast-body onClick from the shared toast API and navigate from ramps status toasts through the existing actionText/onActionClick pattern. Co-authored-by: Cursor <cursoragent@cursor.com>
| return localItems.filter((item) => selectedNetworks.has(item.chainId)); | ||
| return localItems.filter( | ||
| (item) => | ||
| item.chainId !== undefined && selectedNetworks.has(item.chainId), |
There was a problem hiding this comment.
Defensive code needed here because ramps orders can have undefined chain ID because they rely on providers to populate data
|
|
||
| const selectedNetworks = new Set(networks); | ||
| return nonEvmItems.filter((item) => selectedNetworks.has(item.chainId)); | ||
| return nonEvmItems.filter( |
There was a problem hiding this comment.
Defensive code needed here because ramps orders can have undefined chain ID because they rely on providers to populate data
Track dialog details hash open state with a ref so activity-list no longer hardcodes /tx or ramps order routes for push/replace/back. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 742d269. Configure here.
| return; | ||
| } | ||
|
|
||
| detailsHashOpenRef.current = true; |
There was a problem hiding this comment.
Forward leaves stale details history
Low Severity
Replacing the hash-based “already on details” check with detailsHashOpenRef misses browser Forward. After Back, popstate clears the ref and closes the dialog; Forward restores a details hash without reopening the dialog or setting the ref. The next row click then pushStates instead of replaceStates, so Close can land on a stale details URL with the dialog closed.
Reviewed by Cursor Bugbot for commit 742d269. Configure here.
| // `@metamask/client-utils` omits zero native `txParams.value` from mapped | ||
| // tokens but still provides symbol/asset metadata. Treat that as 0 so | ||
| // Activity can render "-0 ETH" for zero-value contract calls / sends. | ||
| if (token.symbol || token.assetId) { |
There was a problem hiding this comment.
this is an edgecase found by bugbot, but valid. When a token has a symbol/asset but there is no amount we treat the amount as 0 so Activity can show -0 ETH instead of a blank row.
|
|
||
| const handleClick = (item: ActivityListItem) => { | ||
| if (!item.hash) { | ||
| const detailsPath = getActivityDetailsPath(item); |
There was a problem hiding this comment.
extracting the path such as #${TX_DETAILS_ROUTE}/${item.chainId}/${item.hash} to it's own helper (https://github.com/MetaMask/metamask-extension/pull/44948/changes#diff-0b6d11323768c8684ae4142a6214b5b2c44ecd1e792682fdf4b4c25449c0fec7R136) because I've added some ramp-specific path checking.
Narrow optional activity.chainId at the call site instead of teaching the helper to accept undefined. Co-authored-by: Cursor <cursoragent@cursor.com>
Match the mock signature to string | number instead of Date | number. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cursoragent please review |
Final PR Review: Ramp Orders UI Integration🟢 Overall Assessment: Ready to Merge with 2 Minor IssuesThe PR is in excellent shape and all previous critical issues have been resolved. Test coverage has improved to 86.7%, and the implementation is solid. There are 2 new minor issues identified by Bugbot that warrant attention. 🟡 New Issues Identified1. Activity Details Freeze on Open (Medium Severity)Location: Issue: The Activity dialog now always seeds Impact: If a user has order details open and the order transitions from pending to success, the details page won't update until they close and reopen it. Toast navigation via Suggested fix: Either:
2. Forward Leaves Stale Details History (Low Severity)Location: Issue: Replacing the hash-based "already on details" check with Impact: Minor navigation quirk where closing details after Back→Forward can land on a stale details URL with the dialog closed. Suggested fix: Listen for ✅ Resolved IssuesAll previously flagged issues have been addressed:
📊 Updated Metrics
🎯 Code Quality HighlightsExcellent Additions
Design Decisions Validated
💬 Code Review CommentsFrom @n3ps
Your response: Updated mock to match actual signature ( This was the right call—fixing the mock rather than changing all call-sites is cleaner. 🚀 Final Checklist
📝 Recommendation🟢 Approve - The PR is ready to merge. The two remaining issues are:
Both are non-blocking given:
If you want to address these before merge, they're straightforward fixes. Otherwise, they could be follow-up improvements. Overall verdict: This is solid, production-ready code. The ramps order UI integration is comprehensive and well-tested. Ship it! 🚀 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 76 changed files in this pull request and generated no new comments.
Suppressed comments (1)
ui/hooks/ramps/useRampsOrderEventToasts.ts:193
- PR description/manual testing steps state the whole toast body should be clickable with no separate “View” button, but the ramps order toast logic still passes
actionText: t('view')andonActionClickintoshow*Toast(...). In the current toast implementation (ui/components/ui/toast/toast.tsx), providingonActionClickrenders a secondary button, so this will still show a “View” CTA instead of making the toast body itself clickable.
|
Builds ready [4d7e486] [reused from 93a6526]
⚡ Performance Benchmarks (Total: 🟢 12 pass · 🟡 9 warn · 🔴 2 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| "@metamask/claims-controller": "^0.4.2", | ||
| "@metamask/client-controller": "^1.0.0", | ||
| "@metamask/client-utils": "patch:@metamask/client-utils@npm%3A1.2.0#~/.yarn/patches/@metamask-client-utils-npm-1.2.0-1adc40c3f3.patch", | ||
| "@metamask/client-utils": "^1.4.0", |
There was a problem hiding this comment.
One way to reduce diff and the number of reviewers for this PR would be to split this dependency bump out.
There was a problem hiding this comment.
Hey @FrederikBolding, that's a great point. I've opened this PR for bumping client-utils. I would really appreciate your review.







Description
After a native Buy/Sell checkout, users had almost no in-wallet trail of the order — just the “buy tab opened” toast. This PR wires resolved ramp orders into the places people already look: Activity, transaction details, and status toasts.
User-facing flow
getOrderFromCallback, withgetOrder(orderCode)fallback) andaddOrders it. No optimistic PRECREATED stubs.rampBuy/rampSellrows (Buy & Sell filter). Row / toast click opens RampOrderDetails (paid with, fee, status, order id, provider link, Buy again).Implementation notes for reviewers
mapRampsOrderSafelywraps shared@metamask/client-utilsmapRampsOrderand normalizes provider quirks (missing chain /txHash).getInternalOrderCode(slash-safe), not raw provider paths.TransactionDetailslooks up ramp orders by id or settlement hash and does not treat order ids as EVM tx hashes for the accounts API.@metamask/client-utils^1.4.0(lockfile1.5.0); LavaMoat/attributions updated for the bump.Out of scope / intentional non-goals
Changelog
CHANGELOG entry: Added ramp order Activity rows, order details, and status toasts for native buy/sell.
Related issues
Fixes: TRAM-3718
Core: MetaMask/core#9650
Manual testing steps
yarn start, reload unpacked extension fromdist/chrome.Screenshots/Recordings
After
Video demo of tab watcher and order processing: https://www.loom.com/share/4bd6baed6e2b4a1996901e7cd5448335
ramp-checkout-2.mp4
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches fiat on-ramp checkout lifecycle, background tab handling, and Activity/dedup behavior; mistakes could strand orders or mis-route users, but scope is mostly UI plus isolated background watcher changes with solid test coverage.
Overview
Ramp buy/sell orders now show up in Activity, order details, and status toasts after checkout, instead of only the “tab opened” toast.
Checkout (background): Continue no longer opens the provider tab from the popup or seeds PRECREATED orders / in-memory quote previews. It calls
watchRampsCheckoutTabwith the checkout URL; the background script opens the tab, resolves the order on callback (getOrderFromCallback,getOrderfallback), opens Activity, then closes checkout.Activity & navigation:
useRampsOrderActivitymerges ramp rows (deduped ahead of txs sharing a settlement hash). Details use/ramps/order/:chain/:idviagetInternalOrderCode, with toast actions and list clicks wired through shared helpers.TransactionDetailsaccepts a pre-resolved item and skips treating order ids as EVM hashes for the accounts API.UI: New
RampOrderDetailspage (fees, payment method, provider link, buy again), ramp row copy/placeholders, i18n, anduseRampsOrderEventToasts(buy/sell copy, account-switch cleanup).@metamask/client-utilsbumped to ^1.4.0 (patch removed).Reviewed by Cursor Bugbot for commit 4d7e486. Bugbot is set up for automated code reviews on this repo. Configure here.