feat(ramps): sync V2 ramps orders via User Storage - #9474
Open
georgeweiler wants to merge 39 commits into
Open
Conversation
georgeweiler
pushed a commit
that referenced
this pull request
Jul 10, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This was referenced Jul 10, 2026
georgeweiler
pushed a commit
that referenced
this pull request
Jul 11, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
georgeweiler
force-pushed
the
feat/ramps-order-syncing
branch
from
July 11, 2026 20:12
63d4658 to
1e2c65b
Compare
georgeweiler
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Jul 11, 2026
Wire RampsController order syncing into mobile: messenger delegation, Backup & Sync sub-toggle, identity-effect pull on unlock, and fixtures for isRampsSyncingEnabled. Depends on MetaMask/core#9474. Co-authored-by: Cursor <cursoragent@cursor.com>
2 tasks
Contributor
Author
|
@cursoragent review this PR |
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…9474) - Add order-syncing module storing RampsOrder objects (without paymentDetails) as per-order User Storage entries under rampsOrders feature - RampsController.syncOrdersWithUserStorage() performs bidirectional sync with timestamp-based conflict resolution and soft deletes - addOrder/removeOrder incrementally push when Backup & Sync and isRampsSyncingEnabled are on - Optional onOrderSyncErroneousSituation and trace callbacks for host observability BREAKING CHANGES: - RampsControllerMessenger requires UserStorageController storage actions and AuthenticationController:isSignedIn Fixes and improvements: - Default isRampsSyncingEnabled to true when absent from state - Route incremental writes through performBatchSetStorage for IDs with hyphens - Re-read local orders before upload so mid-sync additions sync correctly - Queue mid-sync deletes for tombstone writes at sync end - Coalesce overlapping sync calls - Preserve remote createdAt as lastUpdatedAt when importing - Apply LWW rules to tombstones - Prefer freshest copy across entropy profiles - Trim providerOrderId in key derivation - Clear polling metadata on removeOrder Co-authored-by: George Weiler <georgejweiler@gmail.com>
Key lastUpdatedAt preservation on the sync-apply phase so genuine local edits during a sync (e.g. order polling) still bump to now and win the queued follow-up sync under last-write-wins. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…eat/ramps-order-syncing # Conflicts: # packages/profile-sync-controller/CHANGELOG.md # packages/ramps-controller/CHANGELOG.md # packages/ramps-controller/package.json # packages/ramps-controller/src/RampsController-method-action-types.ts # packages/ramps-controller/src/RampsController.ts # packages/ramps-controller/src/order-syncing/utils.test.ts # packages/ramps-controller/src/order-syncing/utils.ts
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
This was referenced Sep 4, 2026
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/profile-sync-controller/CHANGELOG.md # packages/ramps-controller/package.json
Co-authored-by: Cursor <cursoragent@cursor.com>
amitabh94
previously approved these changes
Sep 9, 2026
mathieuartu
reviewed
Sep 9, 2026
| // A polling request can finish after removeOrder. Do not let that stale | ||
| // response recreate the local order and overwrite its remote tombstone. | ||
| if (!hadOrderAtRequestStart || orderStillExists) { | ||
| // Use addOrder to bump lastUpdatedAt and trigger incremental sync. |
Contributor
There was a problem hiding this comment.
This with LWW made me a bit uncomfy but I couldn't pinpoint what really was the problem so I asked Fable to have a look, WDYT? Fable said this:
Unconditional
lastUpdatedAtbump + remote write on everyaddOrdercauses steady-state write amplification on User Storage.
getOrder(L3905-3906) now routes every poll result throughaddOrder, andaddOrderstampslastUpdatedAt: Date.now()(L3026-3028) and callsupdateOrderInRemoteStorage(L3054) with no change detection on either side (controller-integration.tsL451-462 also writes unconditionally).#pollPendingOrdersfires every 30s (DEFAULT_POLLING_INTERVAL_MS) for every non-terminal order.Net effect: every pending order, on every unlocked device with Backup & Sync on, produces an encrypt +
PUTtouser-storage.api.cx.metamask.iotwice a minute for as long as the order is pending (bank transfers: hours to days), even when the payload is byte-identical. Two unlocked devices ping-pong the same key. Beyond the load, it makes an idle poller always win LWW over a real edit on another device.Bugbot flagged this as High ("Polling triggers sync side effects") and the thread was resolved without a change or a reply.
Fix: short-circuit when the payload is unchanged.
areOrdersEqualalready exists and already excludes sync metadata andpaymentDetails, so:const existing = this.state.orders.find( (o) => getInternalOrderCode(o) === internalOrderCode, ); if ( existing && !this.#isApplyingOrderSyncChanges && areOrdersEqual(existing, order) ) { return; // no timestamp bump, no remote write, no #orderSyncQueued }placed after the
internalOrderCodeguard (L3012).Tests to add:
- poll returning an unchanged order does not call
UserStorageController:performBatchSetStorageand does not bumplastUpdatedAt- poll returning a status change produces exactly one write
Related:
updateOrderInRemoteStorage(controller-integration.tsL451-459) has no way to know whether the order changed, so the check belongs inaddOrder. If that function stays the single write path, worth documenting that callers own change detection, otherwise the next caller reintroduces the amplification.
The part that matters to me as accounts / identity CO is the write volume to User Storage; the LWW side effect is a bonus fix
Contributor
Author
There was a problem hiding this comment.
Nice job Fable 👀 I've added areOrdersEqual so unchanged polls don't change lastUpdatedAt
Short-circuit addOrder when the syncable payload is unchanged so polling cannot amplify writes or steal LWW. Also rename incremental helpers to *UserStorage and trim a redundant changelog bullet. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Rename the shadowed findIndex param in addOrder and regenerate RampsController-method-action-types.ts for the updated addOrder doc comment. Co-authored-by: Cursor <cursoragent@cursor.com>
Move the #9474 changelog entries into a fresh Unreleased section above the 21.0.0 / 31.0.0 releases cut on main, and keep fast-deep-equal alongside main's dependency bumps. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursoragent please do a final review |
Final Review SummaryI've completed a comprehensive review of the ramps order syncing PR and identified and fixed a critical bug reported by Bugbot. Issue FixedBugbot Finding: Polls drop local payment details (Medium Severity) The problem was in Solution: Added explicit handling for the case where only
Test ResultsAll tests pass with excellent coverage:
Architecture ReviewThe implementation is well-structured and follows the existing account-tree/contact sync patterns: ✅ Strengths:
✅ Edge Cases Handled:
Ready to MergeThe PR is ready to merge pending:
Next steps for hosts: Mobile and Extension PRs can proceed once these packages are published, ensuring their messengers delegate the required User Storage and auth actions.   |
When a poll returns an order where only paymentDetails changed (everything else is identical), the previous implementation would return early due to areOrdersEqual returning true (it strips paymentDetails for comparison). This meant paymentDetails updates from polls were dropped. Now addOrder explicitly checks if paymentDetails differ when the syncable payload is unchanged, and merges them into local state without bumping lastUpdatedAt or writing remotely (since paymentDetails is local-only). Fixes Bugbot finding: polls drop local payment details Co-authored-by: George Weiler <georgejweiler@gmail.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes breaking messenger/state contracts and adds non-trivial bidirectional sync logic, and introduces a required state field that breaks a full-state construction in account-tree-controller, so it needs human review and cross-package fixes.
Review details
- Files reviewed: 21/22 changed files
- Comments generated: 2
- Review effort level: Balanced
| /** | ||
| * Condition used by UI to determine if ramps order syncing is enabled. | ||
| */ | ||
| isRampsSyncingEnabled: boolean; |
|
|
||
| - Add `NeoBankService` for MetaMask Ramp API neo-bank-proxy endpoints under the `/neobank` prefix on the Ramp API host, including messenger actions for `getAutoramp`, `registerPixAddress`, `getAutorampQuote`, `createAutoramp`, `getAutorampQuoteForAutoramp`, `attachAutorampQuote`, `getCustomerByExternalId`, `getMoonpayCustomerId`, `getWalletRegistrationStatus`, and `registerSelfHostedWallet`. Mutating POSTs do not retry (to avoid duplicate Pix/autoramp creates without a stable `Idempotency-Key`); GETs still retry 429/5xx/network errors. Optional `Idempotency-Key` is forwarded when callers supply one. Also exports `mapNeoBankAutorampToRemoteSnapshot`, `AutorampRemoteSnapshot`, and wallet-registration HTTP types (`WalletRegistrationError`, `RegistrationStatus`, `RegistrationOutcome`). ([#10031](https://github.com/MetaMask/core/pull/10031)) | ||
| - Add `RampsController` autoramp last-seen cursor and Money Account wallet registration: persisted `autoramps` state, `createAutoramp` / `refreshAutoramp(s)` / `applyAutorampStatusFromPush`, `registerMoneyAccountWallet`, and `RampsController:autorampStatusChanged`. MoonPay remains the source of truth; hosts should call `refreshAutoramps` on resume to catch webhooks missed while the app was closed. Hosts must delegate `RAMPS_CONTROLLER_REQUIRED_CONTROLLER_ACTIONS` (`AuthenticationController:getSessionProfile`, `KeyringController:signPersonalMessage`, `RemoteFeatureFlagController:getState`) plus the NeoBank actions listed in `RAMPS_CONTROLLER_REQUIRED_SERVICE_ACTIONS`. ([#10032](https://github.com/MetaMask/core/pull/10032)) | ||
| - Export `TERMINAL_ORDER_STATUSES` and `isTerminalOrderStatus()` so consuming clients can share the controller's terminal order status set instead of maintaining duplicate copies. ([#9679](https://github.com/MetaMask/core/pull/9679)) |
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 6b4819f. Configure here.
Keep UserStorageControllerState constructions complete, and drop a duplicated #9679 changelog entry that already lives under 20.1.0. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Today, buy/sell (ramps) order history lives only on the device where the purchase happened. If you buy crypto on mobile, that order does not show up on Extension (and vice versa).
This PR adds order history sync for V2 ramps orders through Profile Sync / Backup & Sync. Once enabled, orders created on one MetaMask client appear on the other after unlock.
How it fits together
Missing toggle values on upgraded wallets are treated as enabled (same default as new installs).
Consumer impact
@metamask/profile-sync-controllerpieces in the same release). Mobile and Extension PRs need the published packages from this work.RampsController: the messenger must also delegate User Storage batch get/set actions andAuthenticationController:isSignedIn.Companion PRs: Mobile and Extension follow once these packages are published.
Implementation notes (for reviewers)
@metamask/ramps-controllerunderorder-syncing/(same pattern as account-tree / contact sync — not new UserStorageController feature methods).rampsOrdersfeature.RampsController.syncOrdersWithUserStorage()on unlock;addOrder/removeOrderpush incrementally when Backup & Sync + ramps syncing are on and the user is signed in.paymentDetailsstay local only and are not written to User Storage.Test plan
yarn workspace @metamask/ramps-controller run jest --coverage(100% thresholds)yarn workspace @metamask/ramps-controller run messenger-action-types:checkNote
Medium Risk
Large new sync surface touches persisted user data and messenger wiring; conflict/tombstone logic is complex though heavily tested, and PII is explicitly stripped from remote writes.
Overview
Adds cross-device V2 ramps order history through Backup & Sync: orders sync under the User Storage
rampsOrdersfeature with last-write-wins timestamps, soft-delete tombstones, incremental pushes onaddOrder/removeOrder, and a host-facingRampsController:syncOrdersWithUserStorageentry point (with coalescing for overlapping syncs).paymentDetailsstay local; remote payloads strip them, and unchanged poll/update payloads skip remote writes.Profile Sync gains an independent
rampsSyncingfeature flag and persistedisRampsSyncingEnabled(defaults on; absent state treated as enabled). Disabling ramps sync does not turn off main Backup & Sync.Host integration (breaking):
UserStorageControllerStatemust includeisRampsSyncingEnabledwhen constructing full state;RampsControllerMessengermust newly delegateUserStorageController:getState,performGetStorageAllFeatureEntries,performBatchSetStorage, andAuthenticationController:isSignedIn. Order state uses optionallastUpdatedAtfor LWW;getInternalOrderCode/ remove-order behavior is tightened for polling and mid-sync races.Reviewed by Cursor Bugbot for commit 1bcfd7d. Bugbot is set up for automated code reviews on this repo. Configure here.