refactor(snap-solana-wallet): align Solana AssetsService read API with snap-networks-utils by ulissesferreira · Pull Request #120 · MetaMask/internal-snaps

ulissesferreira · 2026-08-05T13:44:46Z

Summary

Align Solana AssetsService read API with snap-networks-utils / AssetsController shapes, and route Keyring + Send through those helpers while keeping Snap-owned storage.

Follow-up simplification

getAccountAssetByID now goes through getAccountAssetsByIDs instead of getAccountAssetsByScope (avoids an unnecessary scope filter).
Extracted #getAccountAssetsOrEmpty so the read API shares one account lookup + findByAccount path.
Added unit coverage for missing-account / empty-ID-list edge cases.

Test plan

ENVIRONMENT=test yarn workspace @metamask/solana-wallet-snap run build
yarn workspace @metamask/solana-wallet-snap run jest --no-coverage src/core/services/assets/AssetsService.test.ts
Keyring getAccountAssets / getAccountBalances still resolve via AssetsService
Send flow still resolves assets via getAccountAssetsByIDs

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

ulissesferreira · 2026-08-06T15:07:36Z

@cursoragent rebase latest main and make sure the CI is green

cursor · 2026-08-06T15:07:44Z

Taking a look!

ulissesferreira · 2026-08-07T00:44:31Z

@cursoragent in this migration make sure the calls to the AssetsService are all one of the following method names:

getAccountAssetByID, getAccountAssetsByIDs, getAccountAssetsByScope, getAccountAssets

where the return types follow a similar logic to the AssetsProvider first 3 methods. Not the same return type, but similar takes on single element, map, array and array

cursor · 2026-08-07T00:44:39Z

Aligned the migration with the four-method read API you specified.

Changes

Added getAccountAssets to AssetsService — returns AssetEntity[] across all active networks (replaces the inlined getAccountAssetsByScope loop in Keyring).
Updated Keyring.getAccountAssets — now calls this.#assetsService.getAccountAssets(accountId) instead of fanning out per scope.
Read API surface — migration call sites now use only these four methods with return shapes matching AssetsProvider patterns:

Method	Return type	Used by
`getAccountAssetByID`	`AssetEntity \| null`	(service layer)
`getAccountAssetsByIDs`	`Record<string, AssetEntity \| null>`	Keyring balances, Send
`getAccountAssetsByScope`	`AssetEntity[]`	(service layer)
`getAccountAssets`	`AssetEntity[]`	Keyring asset listing

Tests updated — AssetsService.test.ts, Keyring.test.ts; all 95 tests in the affected suites pass.

The previous getAccountAssetsForAllActiveScopes name is gone; getAccountAssets is the canonical fourth method, consistent with the Tron snap pattern.

ulissesferreira · 2026-08-07T11:56:57Z

@cursoragent The changes here are missing a lint run + fix and also have disorganized commits which are hard to review. Fix lint problems and suggest a possible list of commits that make sense for this. Make sure the read assets methods are lean and as logically simple as possible

cursor · 2026-08-07T11:57:06Z

Fixed lint failures, simplified the read API, and pushed commit 541bae11.

Lint fixes

yarn lint was failing on two unused private fields in Keyring.ts:

#configProvider — added during migration but never used (account/config lookup belongs on AssetsService, not Keyring). Removed from Keyring, snapContext, and tests.
#traceName — dead code; only #traceNameBatch is used in createAccounts.

Also ran yarn lint:fix, which pruned 3 stale entries from eslint-suppressions.json. Lint now passes cleanly.

Read API simplification

The four read methods are now leaner:

Method	Approach
`getAccountAssetsByScope`	Core primitive: account lookup → `findByAccount` → filter by scope
`getAccountAssetByID`	Delegates to `getAccountAssetsByScope` + find
`getAccountAssetsByIDs`	Groups by scope, builds a `Map` for O(1) lookups (no repeated `.find()`)
`getAccountAssets`	Single account fetch + `findByAccount`, then filters by active networks (avoids N redundant account lookups)

All 95 tests in the affected suites pass.

Suggested commit structure

The current history has noise (chore: retrigger CI, duplicate manifest shasum commits, SendService lint fix split out). For review, I'd squash/rebase onto latest main into 3 focused commits:

refactor(solana-wallet-snap): add AssetsService read API aligned with snap-networks-utils
- AssetsService.ts — four read methods + accountsService dependency
- AssetsService.test.ts
- snapContext.ts — wire accountsService into AssetsService
- CHANGELOG.md
refactor(solana-wallet-snap): route Keyring and Send through AssetsService read API
- Keyring.ts / Keyring.test.ts
- SendService.ts / SendService.test.ts
chore(solana-wallet-snap): sync snap.manifest.json shasum after build
- snap.manifest.json only (auto-generated by production build)

The lint cleanup and read-API simplification from this turn fold into commits 1–2 above. Drop chore: retrigger CI after rebase onto main entirely.

To apply: git fetch origin main && git rebase -i origin/main, then squash/reorder into the three commits above.

ulissesferreira · 2026-08-10T12:16:16Z

@cursoragent rebase with latest main

cursor · 2026-08-10T12:16:25Z

Rebased onto latest main and pushed.

Rebase summary

Base: c6841816 — Release/7.0.0 (#137) + chore: ts issues fixes (#139)

Conflicts resolved:

CHANGELOG.md — Kept the Unreleased AssetsService entry and preserved main's [6.0.0] section (restored missing ### Changed header for the key-tree bump).
snap.manifest.json — Rebuilt with ENVIRONMENT=production and synced the shasum to 0k6rJpLJaUfdbDVaSQ0w4I8K8Jp1lMmVoA+bECw6DnQ=.

Commit history (3 commits, unchanged structure):

refactor(solana-wallet-snap): add AssetsService read API aligned with snap-networks-utils
refactor(solana-wallet-snap): route Keyring and Send through AssetsService read API
chore(solana-wallet-snap): sync snap.manifest.json shasum after build

Verification:

yarn lint — passes
yarn workspace @metamask/solana-wallet-snap run test — passes (after rebuild)
yarn workspace @metamask/solana-wallet-snap run changelog:validate — passes

Branch force-pushed to ulissesferreira/wpn-1652-align-assets-service-api-6620. CI should kick off on PR #120 shortly.

… snap-networks-utils Add getAccountAssetByID, getAccountAssetsByIDs, getAccountAssetsByScope, and getAccountAssets on AssetsService, wire accountsService for account lookup, and cover the read helpers with unit tests. Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

…rvice read API Switch Keyring asset listing/balances and Send balance lookups to the new read methods, remove unused SendService variables, and prune stale eslint suppressions. Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

… typecheck - Add AssetsRepository.findByKeyringAccountIdAndAssetType for direct lookups - getAccountAssetByID: repository lookup + native placeholder only when needed - getAccountAssetsByIDs: single repository read, no Promise.all scope fan-out - Extract #createNativePlaceholder shared with findByAccount - Pin @solana/addresses to 2.1.0 to fix Address brand mismatches - Build snap-networks-utils before typecheck (dist required by tron-wallet-snap) - Remove @jest/globals expect import from index.test.ts (use snaps-jest types) - Sync snap.manifest.json shasum after build Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

ulissesferreira · 2026-08-10T12:42:50Z

    "test:scripts": "NODE_OPTIONS=--experimental-vm-modules yarn jest --config ./jest.config.scripts.js --silent",
    "test:verbose": "yarn workspaces foreach --all --exclude @metamask/sample-snap --parallel --verbose run test:verbose",
-    "typecheck": "yarn workspaces foreach --all --parallel --verbose exec tsc --noEmit",
+    "typecheck": "yarn workspace @metamask/snap-networks-utils run build && yarn workspaces foreach --all --parallel --verbose exec tsc --noEmit",


This will be removed when we coordinate how we want typescript to work across the repo. For now, we need it.

…tsService read API Remove findByKeyringAccountIdAndAssetType from AssetsRepository. Read paths stay on AssetsService (getAccountAssetByID, getAccountAssetsByIDs, getAccountAssetsByScope, getAccountAssets) using findByAccount and in-memory filtering. Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

…ByAccount Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

… path Route getAccountAssetByID through getAccountAssetsByIDs instead of getAccountAssetsByScope, and extract #getAccountAssetsOrEmpty so the read API shares one account lookup + findByAccount path. Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

Apply the production bundle shasum computed by CI and fix oxfmt formatting in AssetsService.test.ts.

Remove suppressions that no longer apply after the AssetsService refactor.

taran-a · 2026-08-10T15:25:11Z

+   */
+  async getAccountAssetByID(
+    accountId: string,
+    assetId: string,


should be CaipAssetType ?

taran-a · 2026-08-10T15:25:29Z

+   */
+  async getAccountAssetsByIDs(
+    accountId: string,
+    assetIds: string[],


should be CaipAssetType[] ?

taran-a · 2026-08-10T15:26:16Z

+  async getAccountAssetsByIDs(
+    accountId: string,
+    assetIds: string[],
+  ): Promise<Record<string, AssetEntity | null>> {


here as well ?

taran-a

LGTM, just same questions//comments regarding types.

…setType Use CaipAssetType for asset ID parameters and return keys in getAccountAssetByID and getAccountAssetsByIDs per review feedback.

The previous commit used a test-environment shasum; CI builds with ENVIRONMENT=production and requires a clean working tree.

cursor Bot pushed a commit that referenced this pull request Aug 5, 2026

chore(WPN-1652): link changelog entry to #120

de74769

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

This was referenced Aug 5, 2026

refactor(solana-wallet-snap): extract SnapAssetsAdapter for Solana balance reads #121

Draft

feat(solana-wallet-snap): remove Solana Snap assets tracking after flag migration #125

Draft

ulissesferreira closed this Aug 5, 2026

ulissesferreira reopened this Aug 5, 2026

ulissesferreira closed this Aug 5, 2026

ulissesferreira reopened this Aug 5, 2026

ulissesferreira marked this pull request as ready for review August 5, 2026 18:06

ulissesferreira requested review from a team as code owners August 5, 2026 18:06

ulissesferreira temporarily deployed to default-branch August 5, 2026 18:06 — with GitHub Actions Inactive

cursor Bot pushed a commit that referenced this pull request Aug 5, 2026

chore(WPN-1652): link changelog entry to #120

9c786c3

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

cursor Bot force-pushed the ulissesferreira/wpn-1652-align-assets-service-api-6620 branch from f5ec399 to ed3340c Compare August 5, 2026 18:09

MetaMask deleted a comment from cursor Bot Aug 5, 2026

ulissesferreira changed the title ~~chore(WPN-1652): align Solana AssetsService read API with snap-networks-utils~~ refactor: align Solana AssetsService read API with snap-networks-utils Aug 5, 2026

ulissesferreira changed the title ~~refactor: align Solana AssetsService read API with snap-networks-utils~~ refactor(snap-solana-wallet): align Solana AssetsService read API with snap-networks-utils Aug 5, 2026

cursor Bot force-pushed the ulissesferreira/wpn-1652-align-assets-service-api-6620 branch 2 times, most recently from 515bebf to 0f8d17f Compare August 6, 2026 11:47

MetaMask deleted a comment from cursor Bot Aug 6, 2026

cursor Bot force-pushed the ulissesferreira/wpn-1652-align-assets-service-api-6620 branch from 11fe525 to eef1038 Compare August 6, 2026 15:18

gabrieledm reviewed Aug 7, 2026

View reviewed changes

Comment thread packages/solana-wallet-snap/src/core/handlers/onKeyringRequest/Keyring.test.ts

cursor Bot force-pushed the ulissesferreira/wpn-1652-align-assets-service-api-6620 branch 2 times, most recently from 7a154fb to 5d407ac Compare August 7, 2026 12:25

gabrieledm previously approved these changes Aug 7, 2026

View reviewed changes

cursoragent and others added 3 commits August 10, 2026 12:21

chore(solana-wallet-snap): sync snap.manifest.json shasum after build

5cd2400

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

cursor Bot dismissed gabrieledm’s stale review via 5cd2400 August 10, 2026 12:21

cursor Bot force-pushed the ulissesferreira/wpn-1652-align-assets-service-api-6620 branch from 5d407ac to 5cd2400 Compare August 10, 2026 12:21

ulissesferreira commented Aug 10, 2026

View reviewed changes

cursoragent and others added 6 commits August 10, 2026 13:00

refactor(solana-wallet-snap): inline native asset placeholder in find…

517827d

…ByAccount Co-authored-by: Ulisses Ferreira <ulisses@hey.com>

fix(solana-wallet-snap): sync manifest shasum from CI and fix formatting

8c37f10

Apply the production bundle shasum computed by CI and fix oxfmt formatting in AssetsService.test.ts.

chore: prune stale eslint suppressions

91fed1f

Remove suppressions that no longer apply after the AssetsService refactor.

chore: format eslint-suppressions.json

a882518

taran-a reviewed Aug 10, 2026

View reviewed changes

ulissesferreira added 2 commits August 10, 2026 16:48

refactor(solana-wallet-snap): type AssetsService read API with CaipAs…

23f18db

…setType Use CaipAssetType for asset ID parameters and return keys in getAccountAssetByID and getAccountAssetsByIDs per review feedback.

chore(solana-wallet-snap): sync production manifest shasum after build

3569664

The previous commit used a test-environment shasum; CI builds with ENVIRONMENT=production and requires a clean working tree.

Uh oh!

Conversation

ulissesferreira commented Aug 5, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Follow-up simplification

Test plan

Uh oh!

ulissesferreira commented Aug 6, 2026

Uh oh!

cursor Bot commented Aug 6, 2026

Uh oh!

ulissesferreira commented Aug 7, 2026

Uh oh!

cursor Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Uh oh!

ulissesferreira commented Aug 7, 2026

Uh oh!

cursor Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lint fixes

Read API simplification

Suggested commit structure

Uh oh!

ulissesferreira commented Aug 10, 2026

Uh oh!

cursor Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rebase summary

Uh oh!

ulissesferreira Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

taran-a Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

taran-a Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

taran-a Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

taran-a left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ulissesferreira commented Aug 5, 2026 •

edited by cursor Bot

Loading

cursor Bot commented Aug 7, 2026 •

edited

Loading

cursor Bot commented Aug 7, 2026 •

edited

Loading

cursor Bot commented Aug 10, 2026 •

edited

Loading