Skip to content

feat(tron): return live data from listAccountAssets and getAccountBalances - #388

Open
ulissesferreira wants to merge 2 commits into
WPN-2217-remove-scope-network-forced-castsfrom
WPN-2217-list-account-assets-live-data
Open

ulissesferreira wants to merge 2 commits into
WPN-2217-remove-scope-network-forced-castsfrom
WPN-2217-list-account-assets-live-data

Conversation

@ulissesferreira

@ulissesferreira ulissesferreira commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Explanation

listAccountAssets and getAccountBalances in the Tron Snap previously returned whatever was persisted in the Snap's state, so the data was only as fresh as the last cronjob indexation. This is needed so the assets controller can use the Snap as the data source for reconcile when the asset migration feature flag switches back.

Changes:

  • getAccountAssets (listAccountAssets) and getAccountBalances now fetch live assets and balances from the chain (via fetchAssetsAndBalancesForAccount) instead of reading persisted state. Fetch failures propagate to the caller.
  • getAccountBalances only fetches live data for the scopes the requested assets belong to, avoiding unnecessary chain calls.

References

Ticket: WPN-2217

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

@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch 3 times, most recently from 3670b04 to baff8e5 Compare September 30, 2026 11:13
@ulissesferreira ulissesferreira changed the title feat(WPN-2217): return live data from listAccountAssets and getAccountBalances feat(tron-wallet-snap): return live data from listAccountAssets and getAccountBalances Sep 30, 2026
@ulissesferreira ulissesferreira changed the title feat(tron-wallet-snap): return live data from listAccountAssets and getAccountBalances feat(tron-wallet-snap): return live data from listAccountAssets and getAccountBalances Sep 30, 2026
@ulissesferreira ulissesferreira changed the title feat(tron-wallet-snap): return live data from listAccountAssets and getAccountBalances feat(tron): return live data from listAccountAssets and getAccountBalances Sep 30, 2026
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch 2 times, most recently from ebc77b5 to 8133f76 Compare September 30, 2026 13:12
@ulissesferreira
ulissesferreira marked this pull request as ready for review September 30, 2026 13:25
@ulissesferreira
ulissesferreira requested a review from a team as a code owner September 30, 2026 13:25
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch 4 times, most recently from 822392c to 06f231f Compare September 30, 2026 13:45
@sonarqubecloud

Copy link
Copy Markdown

async fetchAccountAssets(account: KeyringAccount): Promise<AssetEntity[]> {
const results = await Promise.all(
account.scopes.map((scope) =>
this.fetchAccountAssetsByScope(account, scope as Network),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we relax fetchAccountAssetsByScope's scope argument type to accept ${string}:${string}, or add a type guard here so we don't need this cast?

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.

Hmm very good idea, as is definitely to be avoided. Let me give it a try.

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.

The problem is we are using Network everywhere as the required type. So basically, somehow, we need to convert the input on handlers and then have Network internally which is more specific

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if we want to keep using Network we should add a type guard to narrow down the type from ${string}:${string} so that we throw on unsupported scopes while also making typescript happy - this function seems to be a good place to do it since we take the scope value directly from the KeyringAccount

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.

100%. I am going to look around the code and see if it makes sense to add some more of that here or open a PR right next to it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should be no more than three lines of code before this Promise.all call, so IMO we can do it in this PR, but your call

@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch from 06f231f to 5612e01 Compare September 30, 2026 15:42
@ulissesferreira
ulissesferreira changed the base branch from main to WPN-2217-remove-scope-network-forced-casts September 30, 2026 15:42
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch from 5612e01 to fe369ca Compare September 30, 2026 15:45
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch from fe369ca to 67d43ca Compare September 30, 2026 15:56
@ulissesferreira
ulissesferreira added this pull request to stack #395 September 30, 2026 16:06

This branch was successfully deployed

1 active (outdated) deployment
default-branch — 8133f763 Deployed Sep 30, 2026 by ulissesferreira via Determine whether this PR is a release PR #1385
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.

2 participants