Skip to content

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

Open
ulissesferreira wants to merge 1 commit into
mainfrom
WPN-2217-list-account-assets-live-data
Open

ulissesferreira wants to merge 1 commit into
mainfrom
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
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

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.

Due to the downstream effects of the type change I am going to address in a follow up PR with more type hardening on all things related to Network, scopes, etc.

@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-remove-scope-network-forced-casts branch from b3bdb30 to 0ddf3d2 Compare September 30, 2026 15:44
@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-remove-scope-network-forced-casts branch 2 times, most recently from 995dd3d to 08849c2 Compare September 30, 2026 15:56
@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
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-remove-scope-network-forced-casts branch 5 times, most recently from b4b5901 to 090e695 Compare October 1, 2026 14:23
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-remove-scope-network-forced-casts branch from 090e695 to 7a86d64 Compare October 1, 2026 14:31
@ulissesferreira
ulissesferreira force-pushed the WPN-2217-list-account-assets-live-data branch from 67d43ca to bdbe277 Compare October 1, 2026 15:16
@ulissesferreira
ulissesferreira removed this pull request from stack #395 October 1, 2026 15:17
@ulissesferreira
ulissesferreira changed the base branch from WPN-2217-remove-scope-network-forced-casts to main October 1, 2026 15:17
@sonarqubecloud

sonarqubecloud Bot commented Oct 1, 2026

Copy link
Copy Markdown

@stanleyyconsensys stanleyyconsensys left a comment •

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.

i have a question

when we use listAccountAssets and getAccountBalances from controller
means there will be a case that they dont wanna to use accounts API (becoz accounts API failed)

if im not miss taken, the live data from those 2 methods are returning the result from accounts API

wdyt?

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.

3 participants