Skip to content

feat(tron-wallet-snap): route fungible reads through Core AssetsController - #96

Closed
ulissesferreira wants to merge 3 commits into
WPN-1497-messenger-plumbingfrom
WPN-1497-core-adapter-routing
Closed

feat(tron-wallet-snap): route fungible reads through Core AssetsController#96
ulissesferreira wants to merge 3 commits into
WPN-1497-messenger-plumbingfrom
WPN-1497-core-adapter-routing

Conversation

@ulissesferreira

Copy link
Copy Markdown
Contributor

Summary

  • Introduce CoreAssetsAdapter and mapControllerAsset for Core AssetsController reads
  • Route fungible asset reads (TRX, TRC10, TRC20) based on resolved migration stage (Off / ReadWithFallback / Read)
  • Snap-owned assets always use SnapAssetsAdapter; narrow sync/saveMany to snap-owned when Core stage is active
  • Manifest endows AssetsController:getAsset and AssetsController:getAssets

Part 2 of 2 split from #94. Part 1: WPN-1497-messenger-plumbing.

Jira

Test plan

  • yarn workspace @metamask/tron-wallet-snap build
  • yarn workspace @metamask/tron-wallet-snap run jest --no-coverage (898 tests)

Stacks on PR 1 (WPN-1497-messenger-plumbing), which stacks on #92.

Supersedes #94.

@ulissesferreira
ulissesferreira requested a review from a team as a code owner July 31, 2026 22:08
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch from 2339dbc to 1413a43 Compare July 31, 2026 22:32
@ulissesferreira
ulissesferreira requested a review from a team as a code owner July 31, 2026 22:32
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-core-adapter-routing branch from 3e08eaa to c278782 Compare July 31, 2026 22:36
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-core-adapter-routing branch from e6ff082 to 7e28dbf Compare July 31, 2026 22:45
@ulissesferreira
ulissesferreira marked this pull request as draft August 2, 2026 17:39
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch from e2a7495 to 64e6d11 Compare August 2, 2026 20:48
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-core-adapter-routing branch 2 times, most recently from 01f94f8 to aec97ce Compare August 2, 2026 20:59
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch from b73d987 to 7302e9d Compare August 2, 2026 23:20
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-core-adapter-routing branch from 6f74385 to 1ad0e7a Compare August 2, 2026 23:20
): Promise<AssetEntity | null> {
if (isSnapOwnedAsset(assetId)) {
return this.#snapAdapter.getAccountAssetByID(accountId, assetId);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This shouldn't exist

return ASSETS_MIGRATION_STAGE;
}

async #getProviderAccountAssetByID(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary indirection

return mapControllerAsset(accountId, asset);
}

async #getProviderAccountAssetsByIDs(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary indirection

);
}

async #getProviderAccountAssetsByScope(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary indirection

}

return this.#getProviderAccountAssetByID(accountId, assetId);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't need the fallback thing. Just an if with an early return and everything else

@cursor
cursor Bot force-pushed the WPN-1497-messenger-plumbing branch from f5bde1f to f53d7bf Compare August 4, 2026 11:09
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch from f53d7bf to c9d4f0c Compare August 4, 2026 13:45
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-core-adapter-routing branch from 1a63feb to 3dc547d Compare August 4, 2026 13:45
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch from c9d4f0c to d5a852a Compare August 4, 2026 13:54
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-core-adapter-routing branch from 3dc547d to d387059 Compare August 4, 2026 13:54
Instantiate Core messenger via getMessenger and endow
RemoteFeatureFlagController:getState plus AssetsController read actions.
Wire RemoteFeatureFlagsProvider and AssetsProvider from
@metamask/snap-networks-utils@1.0.0. No assets migration stage
resolution or Core routing yet.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@cursor
cursor Bot force-pushed the WPN-1497-messenger-plumbing branch from d5a852a to 053386c Compare August 5, 2026 08:15
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch from 053386c to c4d7646 Compare August 5, 2026 08:42
@ulissesferreira

Copy link
Copy Markdown
Contributor Author

@cursoragent rebase with latest base. remove unnecessary indirection due to the private methods: getProviderAccountAssetsByScope, getProviderAccountAssetByID, etc. also Snap owned assets, although they will still be synced by the snap, are now read the same way as other assets. Meaning the only routing the AssetsService reads should have is a simple if: check against the migration stage and either read from Snap adapter or Core adapter

@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

Addressed the review feedback: rebased onto the latest base and simplified AssetsService read routing.

Rebase

Reset onto origin/WPN-1497-messenger-plumbing (c4d76460) and reapplied the routing work as a single commit on top, since the old branch had duplicate history that conflicted during rebase.

AssetsService simplification

Read routing is now a single migration-stage check with an early return:

  • OffSnapAssetsAdapter
  • Any other stageAssetsProvider (Core AssetsController)

Changes:

  • Removed private indirection helpers (#getProviderAccountAssetByID, #getProviderAccountAssetsByIDs, #getProviderAccountAssetsByScope)
  • Removed snap-owned special-casing on reads — all assets use the same path
  • Removed ReadAssetsControllerWithFallback try/catch fallback logic
  • getByKeyringAccountId delegates to getAccountAssetsByScope

Tests

  • Updated migration-mode tests for unified controller reads
  • Removed the fallback-mode test
  • All 908 tests pass

Branch WPN-1497-core-adapter-routing is force-pushed with commit ea6cdf43.

Open in Web Open in Cursor 

Rebase onto latest WPN-1497-messenger-plumbing and simplify asset read
routing per review feedback:

- Remove private provider indirection helpers; inline AssetsProvider calls
- Route all asset reads (including snap-owned) by migration stage only
- Use early return: Off → SnapAssetsAdapter, otherwise → AssetsProvider
- Drop ReadAssetsControllerWithFallback try/catch fallback paths
- Update tests to match unified controller read path

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@cursor
cursor Bot force-pushed the WPN-1497-core-adapter-routing branch from d387059 to ea6cdf4 Compare August 5, 2026 10:16
@ulissesferreira
ulissesferreira force-pushed the WPN-1497-messenger-plumbing branch 6 times, most recently from 91b9f68 to b24a871 Compare August 5, 2026 11:47
jeremytsng pushed a commit that referenced this pull request Aug 5, 2026
## Explanation


This PR continues the “transaction scan” work by centralizing Stellar
operation type string literals into a shared enum and updating
transaction-related code to use it, while also adding a small utility
for working with record-of-arrays state.

**Changes:**
- Add `StellarOperationType` (and `TransactionOrder`) enums and replace
several hard-coded operation type strings with enum references across
transaction parsing/mapping/simulation and XDR validation.
- Add `pushToRecordArray` helper plus unit tests.
- Introduce small type aliases (`KeyringAccountId`, `TransactionId`) to
clarify intent in state and API typing.


<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## 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](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants