Repository navigation
feat(assets-controller): mirror custom token changes to Authenticated User Storage - #10336
Merged
Merged
Conversation
Prithpal-Sooriya
added a commit
that referenced
this pull request
Sep 21, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This was referenced Sep 21, 2026
Prithpal-Sooriya
marked this pull request as draft
September 22, 2026 07:58
…orted/hidden assets) Add a user-assets domain to the Authenticated User Storage SDK, backed by the new GET/PUT /preferences/user-assets endpoints: - getUserAssets / setUserAssets: low-level blob access, mirroring the assets-watchlist pattern (404 -> null on read, strict write validation) - importTokens / hideTokens: high-level API that handles deduplication and mutual exclusivity internally, returning the resolved blob - Fail-open conflict resolution: an identifier present in both importedAssets and hiddenAssets stays imported and is removed from hiddenAssets; writes are never rejected due to a conflict - Strict write-side schema validation: every entry must be a CAIP-19 asset identifier (CaipAssetTypeStruct); reads stay lenient Jira: ASSETS-3937
Per review feedback on #10233: keep JSDoc to concise summaries with the necessary param/returns/throws tags instead of multi-paragraph essays. The generated action-types file is regenerated to match. No functional change.
…s to it.each tables Per review feedback on #10233: the mirrored importTokens/hideTokens tests, the error-status and malformed-blob cases, and the cache invalidation trio are now table-driven via it.each, split per scenario. Adds a hiddenAssets-side malformed-blob row; all existing assertions preserved.
…assets writes Per review feedback on #10233: add a UserAssetsBlobNormalizedWriteSchema refinement asserting no identifier appears in both importedAssets and hiddenAssets, applied to the normalized blob right before the write. Structural validation of the raw input still fails fast, but the exclusivity check runs after fail-open normalization, so a write can never be rejected because of a conflict — it only guards against internal logic bugs.
…-tokens API contract Per the backend integration (va-mmcx-notify-users-storage#128): - Point getUserAssets/setUserAssets (and their mock URL) at the GET/PUT /api/v1/custom-tokens endpoints instead of the provisional /preferences/user-assets path. - Mirror the API's server-side 100-entries-per-list cap with USER_ASSETS_MAX_ASSETS, enforced via size() on the write schema (same convention as ASSETS_WATCHLIST_MAX_ASSETS); the lenient read schema stays uncapped so existing server data is never rejected. - README/CHANGELOG updated with the endpoint path and the cap.
…om tokens
Adds a clean-slate wipe: clearUserAssets() persists an empty blob
({ version: 1, importedAssets: [], hiddenAssets: [] }) via the existing
PUT /custom-tokens endpoint, which is the wipe mechanism supported by
the current API contract (the backend exposes record deletion only
through internal auth-event/GDPR flows, not a public DELETE endpoint).
Idempotent, works whether or not a blob exists, and invalidates the
getUserAssets cache. Exposed on the messenger with a regenerated
action type.
…itest migration - Replace one `jest.spyOn` with `vi.spyOn` in the user-assets tests; main migrated this package from Jest to Vitest (#10951). - Update `oxlint-suppressions.json`: one additional `no-unsafe-assignment`/`no-unsafe-return` pair from the new `getUserAssets` method (same `fetchQuery` idiom as the existing getters) and one `vitest/no-conditional-in-test` suppression for the cache-invalidation table test, matching repo-wide norms.
Prithpal-Sooriya
force-pushed
the
feature/act-as-a-senior-soft-rhj
branch
from
October 26, 2026 17:15
3b999be to
3208a82
Compare
A delayed add sync could overwrite a later remove that had already stripped the token from AUS, and a post-mutation fetch failure skipped the mirror of an already-applied local add. Queue each sync when the decorated method returns (its local mutation is already committed) and run syncs strictly in invocation order. Also fix oxfmt formatting, the tsconfig.lint.json authenticated-user-storage reference, and the README dependency graph.
Remove the FIFO sync chain; the decorator now fires the AUS mirror directly once the decorated method returns, since every configured method applies its local mutation before returning or first awaiting. Also trim JSDoc to terse one-line summaries per repo conventions.
- Replace Jest idioms (jest.fn/jest.Mock) with Vitest (vi.fn/Mock) in the AUS sync tests, mirroring the sibling authenticated-user-storage adaptation on the base branch. - Await the now-async unhideAsset in tests and fold the AUS-sync unhide tests into the existing unhideAsset describe block (no-identical-title). - Bump the @metamask/authenticated-user-storage devDependency range to ^4.1.0 to satisfy yarn constraints against the workspace version.
Prithpal-Sooriya
force-pushed
the
feat/assets-aus-custom-token-sync
branch
from
October 26, 2026 17:30
eaa8eb4 to
da2d647
Compare
… idiom Main migrated nock to v15 and made the nock-based tests independent of node-fetch (#11077, #11055) by replacing reply-callback request-body assertions with `nock.RequestBodyMatcher` arguments plus `.isDone()` checks. Convert `handleMockSetUserAssets` and its seven call sites to the same idiom — the old callback form hangs under nock v15 with native fetch.
…' into feat/assets-aus-custom-token-sync # Conflicts: # yarn.lock
…se merge The merge of origin/feature/act-as-a-senior-soft-rhj auto-placed the #10336 changelog entry into the released [20.0.0] section (leaving Unreleased empty). Move it back under Unreleased/Added so the changelog check passes and the entry stays unreleased.
Contributor
Author
|
@metamaskbot publish-preview |
…' into feat/assets-aus-custom-token-sync-1
…m-token-sync-1 # Conflicts: # packages/assets-controller/CHANGELOG.md # packages/assets-controller/package.json # packages/assets-controller/src/AssetsController.test.ts # yarn.lock
Contributor
Author
|
@metamaskbot publish-preview |
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-preview |
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 Nov 10, 2026
Prithpal-Sooriya
marked this pull request as ready for review
November 10, 2026 17:53
…m-token-sync-1 # Conflicts: # packages/assets-controller/CHANGELOG.md # packages/assets-controller/src/AssetsController.ts
Contributor
Author
|
🤖 Merged latest |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort 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 420cd37. Configure here.
Prithpal-Sooriya
enabled auto-merge
November 11, 2026 12:00
…r instance A strip (removeCustomAsset/unhideAsset) could read the AUS blob while an earlier importTokens/hideTokens write was still in flight, see the asset as absent and skip its write, so the earlier merge undid the strip. Queue syncs per instance so each read-modify-write starts only after the previous one has landed.
Prithpal-Sooriya
disabled auto-merge
November 11, 2026 12:08
Prithpal-Sooriya
enabled auto-merge
November 11, 2026 12:08
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.
Explanation
Custom token changes made locally (
addCustomAsset,removeCustomAsset,hideAsset,unhideAsset) are now mirrored to Authenticated User Storage (AUS) so that user asset intent follows the account across clients.@syncAusUserAssetsmethod decorator (syncAusUserAssets.ts) applied to the four custom-token mutation methods. The mapping from method to AUS behavior is centralized in a name-keyed config that fails fast (at class-definition time) if applied to an unconfigured method.addCustomAsset→importTokens;hideAsset→hideTokens(each a single high-level SDK call with the checksummed CAIP-19 id, preserving the SDK's mutual-exclusivity semantics).removeCustomAsset/unhideAsset→ strip-only: readgetUserAssets, filter the id out ofimportedAssets/hiddenAssetsrespectively, andsetUserAssetsthe result (no write when the blob is null or the id is already absent).messenger.captureException(anError('Failed to sync user assets to AUS', { cause }), matching the account-tree-controller / DeFiPositionsControllerV2 pattern; a throwing reporter is swallowed), never blocking or rolling back the local change. A locally failing method never writes to AUS.#removeCustomAssetInternalso graduation never records user intent in AUS.@metamask/authenticated-user-storageas a dev dependency (type-only usage) with project references, and registers the four AUS action types on the controller messenger's allowed actions.Known limitations
importedAssetslist in the blob. This is a product decision and is out of scope here.Adversarial review (adverse)
Final verdict (round 1/3): SHIP-WITH-CAVEATS (3/3 ship, 0/3 block)
Findings: 0 critical · 11 warning · 7 info. Fixed: none in this round. The reviewed change is the per-instance sync serialization fix for Bugbot's in-flight merge race. Declined as out of scope for that fix: all 11 warnings. Each targets the PR's existing design or a product decision, not the serialization change. They cover per-account vs user-level removal (open decision), decorator vs explicit call, async-method firing before settle, the AUS package as devDependency in exported messenger types, consent/basic-functionality gating, unwired-client error noise, and test-fixture fidelity. They are flagged for the PR owner.
Reviewer: claude -p via adverse 0.2.1 (3 personas, 2 rounds).
References
main).@trackAssetsLoading).Note
Medium Risk
Changes how user asset preferences are persisted to remote storage; failures are non-blocking but cross-client sync correctness depends on the new decorator and queue behavior.
Overview
User-driven custom token changes in AssetsController now mirror to Authenticated User Storage (AUS) so imported and hidden assets can follow the user across clients.
A new
@syncAusUserAssetsdecorator onaddCustomAsset,removeCustomAsset,hideAsset, andunhideAssetfires fire-and-forget syncs as soon as local state is applied: adds/hides callimportTokens/hideTokenswith normalized CAIP-19 ids; removes/unhides strip the id from the stored blob viagetUserAssets→ filter →setUserAssets(no write when the blob is missing or the id is already absent). Syncs are serialized per controller through a weak-map queue so overlapping read-modify-writes do not race. AUS failures are logged and reported withcaptureExceptionand never roll back local state; validation failures never touch AUS. Auto-graduation removal uses new#removeCustomAssetInternalso automatic detection does not erase user intent in AUS.Wiring adds the four AUS messenger actions,
@metamask/authenticated-user-storageas a dev/type dependency, changelog entry, and broad unit/integration tests for the decorator and controller paths.Reviewed by Cursor Bugbot for commit 0ca2b9a. Bugbot is set up for automated code reviews on this repo. Configure here.
Video Walkthrough
assets-aus-custom-token-sync.mp4