-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding wallet get assets usage on appkit send #3827
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Skipped Deployments
|
|
….fetchTokenBalance
@KannuSingh Can we add the Showcase section to the PR? Might be easier to understand what you did. Also the description is not very useful. It explains what you did but not why. |
|
||
if (!balances) { | ||
const networkTokenBalances = SendApiUtil.mapBalancesToSwapTokens(state.tokenBalances) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, the implementation retrieved balances using:
const balances = await SwapApiUtil.getMyTokensWithBalance();
Inside SwapApiUtil.getMyTokensWithBalance(), it first fetched balances from BlockchainApi, then mapped them using mapBalancesToSwapTokens before returning the result.
In the current implementation, since SendController already maintains the token balance state, I’m now directly using it to call mapBalancesToSwapTokens, avoiding the API call.
async fetchTokenBalance(onError?: (error: unknown) => void): Promise<Balance[]> { | ||
state.loading = true | ||
const chainId = ChainController.state.activeCaipNetwork?.caipNetworkId | ||
const chain = ChainController.state.activeCaipNetwork?.chainNamespace | ||
const caipAddress = ChainController.state.activeCaipAddress | ||
const address = caipAddress ? CoreHelperUtil.getPlainAddress(caipAddress) : undefined | ||
if ( | ||
state.lastRetry && | ||
!CoreHelperUtil.isAllowedRetry(state.lastRetry, 30 * ConstantsUtil.ONE_SEC_MS) | ||
) { | ||
state.loading = false | ||
|
||
return [] | ||
} | ||
|
||
try { | ||
if (address && chainId && chain) { | ||
const balances = await SendApiUtil.getMyTokensWithBalance() | ||
state.tokenBalances = balances | ||
state.lastRetry = undefined | ||
|
||
return balances | ||
} | ||
} catch (error) { | ||
state.lastRetry = Date.now() | ||
|
||
onError?.(error) | ||
SnackController.showError('Token Balance Unavailable') | ||
} finally { | ||
state.loading = false | ||
} | ||
|
||
return [] | ||
}, | ||
|
||
fetchNetworkBalance() { | ||
if (state.tokenBalances.length === 0) { | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we already have this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a similar method in AccountController, but currently, we're maintaining separate token balance lists—one for CA balances (wallet_getAssets
) in SendController and another for active chain balances in AccountController.
This method is called only when Send UI is opened.
SendController.subscribe(val => { | ||
this.tokenBalances = val.tokenBalances |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm. why move the state to SendController? I mean the tokens would also be required on portfolio in some cases so I'd think it's better to keep them in the account state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why move the state to SendController?
added this state to SendController bc this hold the tokens balance retrieved via wallet_getAssets
rpc (CA balances). The AccountController holds the token balances from connected chain only.
I mean the tokens would also be required on portfolio in some cases
yes, but right now we don't have design to show both the balances nicely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukaisailovic food for thought but aren't portolio api and the CA api related? are we using diff providers?
feels kind of weird having to mantain 2 pieces of code / reconciliate, seems like they are doing basically the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can utilize 7811 in portfolio if we wanted. I think in this case, we need to keep the portfolio call separate (inside the secure site) in order to have 1 place where we have the real user balance (no overrides). I think we can utilize 7811 appkit wide probably but requires more effort
Description
Enhance Send Flow with
wallet_getAssets
for CA Balance DisplayThis PR integrates the
wallet_getAssets
method into the Send Flow to display the user's Chain Abstraction (CA) balance. Since the embedded wallet supports CA for USDC and USDT and implements the wallet_getAssets RPC (indicated by the assetDiscovery capability), the AppKit Send Flow now:Checks the connected chain's namespace and the wallet's capabilities.
If assetDiscovery is supported, it calls wallet_getAssets to fetch and display token balances.
Otherwise, it falls back to blockchainApi for retrieving account balances.
Type of change
Associated Issues
For Linear issues: Closes APKT-xxx
For GH issues: closes #...
Showcase (Optional)
Appkit.CA.Send.mp4
Checklist