feat(tron): return live data from listAccountAssets and getAccountBalances - #388
ulissesferreira wants to merge 2 commits into
Conversation
3670b04 to
baff8e5
Compare
listAccountAssets and getAccountBalances
listAccountAssets and getAccountBalanceslistAccountAssets and getAccountBalances
ebc77b5 to
8133f76
Compare
822392c to
06f231f
Compare
|
| async fetchAccountAssets(account: KeyringAccount): Promise<AssetEntity[]> { | ||
| const results = await Promise.all( | ||
| account.scopes.map((scope) => | ||
| this.fetchAccountAssetsByScope(account, scope as Network), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Hmm very good idea, as is definitely to be avoided. Let me give it a try.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
…daries instead of casting
06f231f to
5612e01
Compare
5612e01 to
fe369ca
Compare
…etAccountBalances
fe369ca to
67d43ca
Compare



Explanation
listAccountAssetsandgetAccountBalancesin 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) andgetAccountBalancesnow fetch live assets and balances from the chain (viafetchAssetsAndBalancesForAccount) instead of reading persisted state. Fetch failures propagate to the caller.getAccountBalancesonly fetches live data for the scopes the requested assets belong to, avoiding unnecessary chain calls.References
Ticket: WPN-2217
Checklist