Skip to content

refactor(snap-solana-wallet): align Solana AssetsService read API with snap-networks-utils - #120

Open
ulissesferreira wants to merge 6 commits into
mainfrom
ulissesferreira/wpn-1652-align-assets-service-api-6620
Open

refactor(snap-solana-wallet): align Solana AssetsService read API with snap-networks-utils#120
ulissesferreira wants to merge 6 commits into
mainfrom
ulissesferreira/wpn-1652-align-assets-service-api-6620

Conversation

@ulissesferreira

@ulissesferreira ulissesferreira commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Explanation

Part of migrating the Solana assets-controller staged migration from snap-solana-wallet#635 into this monorepo (PR 1 of 6, Lane A / feature-flag path only).

  • Add shared read API methods on AssetsService: getAccountAssetByID, getAccountAssetsByIDs, getAccountAssetsByScope, getAccountAssets
  • Wire accountsService into AssetsService (needed for account lookup by ID)
  • Update Keyring and Send to use the new API
  • No behavior change — still reads from Snap-owned assetEntities via AssetsRepository
  • Rebased onto latest main (includes chore(solana-wallet-snap): deprecate pre-Unified Send Flow code #130 removal of legacy send UI / refreshSend cron — those paths are no longer updated here)

Read API surface

Migration call sites use only these four methods, with return shapes aligned to AssetsProvider / AssetsController patterns (single element, map-by-ID, scoped array, full array):

Method Return type
getAccountAssetByID AssetEntity | null
getAccountAssetsByIDs Record<string, AssetEntity | null>
getAccountAssetsByScope AssetEntity[]
getAccountAssets AssetEntity[]

CI babysit fixes

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them
Open in Web Open in Cursor 

cursor Bot pushed a commit that referenced this pull request Aug 5, 2026
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@ulissesferreira
ulissesferreira marked this pull request as ready for review August 5, 2026 18:06
@ulissesferreira
ulissesferreira requested review from a team as code owners August 5, 2026 18:06
cursor Bot pushed a commit that referenced this pull request Aug 5, 2026
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@cursor
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 MetaMask deleted a comment from cursor Bot Aug 5, 2026
@ulissesferreira 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 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
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 MetaMask deleted a comment from cursor Bot Aug 6, 2026
@MetaMask MetaMask deleted a comment from cursor Bot Aug 6, 2026
@ulissesferreira

Copy link
Copy Markdown
Contributor Author

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

@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

Taking a look!

Open in Web Open in Cursor 

cursoragent and others added 3 commits August 6, 2026 15:12
…ks-utils

Add getAccountAssetByID, getAccountAssetsByIDs, getAccountAssetsByScope,
and getAccountAssetsForAllActiveScopes. Update Keyring, Send, send render,
and refreshSend to use the new API. No behavior change — still reads from
Snap-owned assetEntities via AssetsRepository.

Migrated from MetaMask/snap-solana-wallet#635.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Drop unused catch binding and account assignment left after the
AssetsService read API migration, and prune the stale suppression.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Remove getAccountAssetsForAllActiveScopes and use Promise.all with
getAccountAssetsByScope per active scope in Keyring. Refactor
getAccountAssetsByIDs to resolve assets via getAccountAssetsByScope.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@cursor
cursor Bot force-pushed the ulissesferreira/wpn-1652-align-assets-service-api-6620 branch from 11fe525 to eef1038 Compare August 6, 2026 15:18
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@ulissesferreira

Copy link
Copy Markdown
Contributor Author

@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

cursor Bot commented Aug 7, 2026

Copy link
Copy Markdown

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

Changes

  1. Added getAccountAssets to AssetsService — returns AssetEntity[] across all active networks (replaces the inlined getAccountAssetsByScope loop in Keyring).

  2. Updated Keyring.getAccountAssets — now calls this.#assetsService.getAccountAssets(accountId) instead of fanning out per scope.

  3. 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
  1. Tests updatedAssetsService.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.

Open in Web Open in Cursor 

Route Keyring asset listing through AssetsService.getAccountAssets instead
of calling getAccountAssetsByScope per active scope. Keeps the four-method
read surface: single, map-by-ID, scoped array, and full array.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
let mockKeyringAccountMonitor: KeyringAccountMonitor;
let mockConfigProvider: ConfigProvider;

beforeEach(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This, in the future, will probably be removed in favour of a setup function

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants