✨ app: add support for multi-chain asset recovery#970
✨ app: add support for multi-chain asset recovery#970dieguezguille wants to merge 12 commits intotransportfrom
Conversation
🦋 Changeset detectedLatest commit: 7b68045 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds cross-chain asset discovery and execution, portfolio restructuring, new UI sheets for external/collateral/unsupported networks, cross-chain account client routing, multiple query helpers for deployed/wallet balances, i18n updates, and assorted UI/layout tweaks. Includes many small changesets for patch releases. Changes
Sequence DiagramsequenceDiagram
participant User
participant Bridge as Bridge Component
participant Portfolio as Portfolio Screen
participant QueryClient as Query Client
participant LiFi as LiFi API
participant Deployed as Deployed Check
User->>Portfolio: Open portfolio
Portfolio->>QueryClient: fetch markets, same-chain & wallet balances
QueryClient->>LiFi: request tokenBalances & walletBalances
LiFi-->>QueryClient: return balances per chain
QueryClient->>Deployed: check contract bytecode per chain
Deployed-->>Portfolio: deployed status
Portfolio->>User: render collateral + external assets grouped by chain
User->>Bridge: select external asset -> navigate with sender, sourceChain, sourceToken
Bridge->>Bridge: classify mode (isTransfer/isSwap/isRecovery)
alt isTransfer (same-chain)
Bridge->>Wallet: send native transaction
Wallet->>Network: broadcast tx
else cross-chain / recovery
Bridge->>AccountClient: request cross-chain client for targetChain
AccountClient->>CrossChainClient: build/sign UserOperation
CrossChainClient->>Network: submit UserOperation
Network-->>CrossChainClient: execution/result
CrossChainClient-->>Bridge: return composite id/result
end
Bridge->>User: show success/failure
sequenceDiagram
participant User
participant ExternalAssets as ExternalAssets Component
participant QueryClient as Query Client
participant Deployed as Deployed Check
participant Unsupported as UnsupportedNetworksSheet
User->>ExternalAssets: Tap non-collateral asset
ExternalAssets->>QueryClient: ensure deployedOptions(account, chainId) queries
QueryClient->>Deployed: return deployed/not-deployed
alt not supported or not deployed
ExternalAssets->>Unsupported: open UnsupportedNetworksSheet with formatted recovery message
Unsupported->>User: show contact support flow (Intercom)
else supported & deployed
ExternalAssets->>Bridge: navigate to Bridge with asset params
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## transport #970 +/- ##
=============================================
- Coverage 71.91% 70.74% -1.17%
=============================================
Files 229 236 +7
Lines 8441 8656 +215
Branches 2710 2771 +61
=============================================
+ Hits 6070 6124 +54
- Misses 2140 2302 +162
+ Partials 231 230 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4868ef9 to
eb6d3a6
Compare
4bcfa86 to
c19bd5d
Compare
c19bd5d to
e79dd5b
Compare
e79dd5b to
aae3144
Compare
aae3144 to
5c6c745
Compare
b50b099 to
de51847
Compare
de51847 to
fd9ee46
Compare
| async function getWalletBalances(account: Address) { | ||
| const balances: Record<number, TokenAmount[]> = {}; | ||
| const lifiConfig = config.get(); | ||
| let offset: string | undefined; | ||
| do { | ||
| const url = new URL(`${lifiConfig.apiUrl}/wallets/${account}/balances`); | ||
| url.searchParams.set("extended", "true"); | ||
| url.searchParams.set("limit", "1000"); | ||
| if (offset) url.searchParams.set("offset", offset); | ||
| const response = await fetch(url, { | ||
| headers: { | ||
| ...(lifiConfig.apiKey && { "x-lifi-api-key": lifiConfig.apiKey }), | ||
| ...(lifiConfig.integrator && { "x-lifi-integrator": lifiConfig.integrator }), | ||
| }, | ||
| }); | ||
| if (!response.ok) throw new Error("wallet balances request failed"); | ||
| const json = parse( | ||
| object({ | ||
| balances: optional( | ||
| record( | ||
| string(), | ||
| array( | ||
| looseObject({ | ||
| chainId: number(), | ||
| address: string(), | ||
| symbol: string(), | ||
| decimals: number(), | ||
| name: string(), | ||
| priceUSD: optional(string(), "0"), | ||
| logoURI: optional(string()), | ||
| amount: pipe(string(), regex(/^\d+$/)), | ||
| }), | ||
| ), | ||
| ), | ||
| ), | ||
| offset: optional(string()), | ||
| }), | ||
| await response.json(), | ||
| ); | ||
| for (const [chainId, tokens] of Object.entries(json.balances ?? {})) { | ||
| const id = Number(chainId); | ||
| if (!Number.isInteger(id)) continue; | ||
| balances[id] = [ | ||
| ...(balances[id] ?? []), | ||
| ...tokens.map(({ amount, ...token }) => ({ ...token, amount: BigInt(amount) })), | ||
| ]; | ||
| } | ||
| offset = json.offset; | ||
| } while (offset); | ||
| return balances; |
There was a problem hiding this comment.
🚩 Wallet balances API may return tokens LI.FI cannot route
The new getWalletBalances function (src/utils/lifi.ts:454-503) fetches ALL token balances for a wallet via GET /wallets/{address}/balances, replacing the old approach of passing a curated token list to getTokenBalancesByChain. This means the bridge asset selector in Bridge.tsx may now show tokens that LI.FI has no swap/bridge route for. A user selecting such a token would get a 'unable to fetch bridge quote' error after entering an amount. The old code only showed tokens LI.FI explicitly supported. This behavioral change is likely intentional (broader asset visibility for recovery flows) but worth confirming.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2db4dd6a65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| error: deploymentError, | ||
| isLoading: isCheckingDeployment, | ||
| } = useQuery({ ...deployedOptions(senderAddress, source?.chain), enabled: isRecovery }); | ||
| const notDeployed = isRecovery && deployed === false; |
There was a problem hiding this comment.
Remove undeployed-account block from recovery submit path
This marks recovery as unavailable whenever deployed === false, which blocks users whose exa account has balances on a supported chain but has never been deployed there yet. In this same commit, createAccountClient builds cross-chain accounts with getAccountInitCode and sends user operations on target chains, so first-use deployment is expected to be possible; treating notDeployed as a hard blocker prevents that first recovery transaction from ever being submitted.
Useful? React with 👍 / 👎.
fd9ee46 to
ebb61b1
Compare
| if (classifyError(error).authKnown) { | ||
| setBridgePreview(undefined); | ||
| resetBridgeMutation(); | ||
| return; |
There was a problem hiding this comment.
🟡 Auth-known bridge/transfer errors silently swallowed instead of reported to Sentry
The onError handlers for both executeBridge and executeTransfer mutations use classifyError(error).authKnown to detect auth-related errors and return early without calling reportError. Every other mutation error handler in the codebase (Repay.tsx:439, Repay.tsx:510, Swaps.tsx:371, Amount.tsx:153, Review.tsx:160, RollDebt.tsx:295, UpgradeAccount.tsx:106) uses the established pattern reportError(error).authKnown, which first reports the error to Sentry (as a warning-level event for auth-known errors) and then checks the classification. The new Bridge pattern skips Sentry reporting entirely for auth-known errors. This violates the codebase's established error-handling convention and creates an observability gap where passkey cancellations, wallet rejections, and bundle cancellations during bridge/transfer operations are invisible to monitoring.
established pattern vs new pattern
Established pattern (7 other files):
onError(error) {
if (reportError(error).authKnown) resetMutation();
}New Bridge pattern (2 handlers):
onError(error) {
if (classifyError(error).authKnown) {
setBridgePreview(undefined);
resetBridgeMutation();
return; // error NOT reported to Sentry
}
reportError(error);
...
}Prompt for agents
The onError handlers for executeBridge (line 438) and executeTransfer (line 497) in Bridge.tsx use classifyError(error).authKnown which only classifies the error without reporting it to Sentry. Every other mutation error handler in the codebase uses reportError(error).authKnown which both reports to Sentry AND returns the classification. This creates an observability gap for auth-known errors (passkey cancellations, wallet rejections) during bridge and transfer operations.
To fix, replace classifyError(error).authKnown with reportError(error).authKnown in both the executeBridge onError (line 438) and executeTransfer onError (line 497). Keep the additional setBridgePreview(undefined) and reset calls which are a good UX improvement, but ensure the error still reaches Sentry as a warning-level event consistent with the rest of the codebase.
Was this helpful? React with 👍 or 👎 to provide feedback.
ebb61b1 to
20f2565
Compare
20f2565 to
381d1b1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 003024cc80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export default function Portfolio() { | ||
| const { address } = useAccount(); | ||
| const { averageRate, portfolio, totalBalanceUSD } = usePortfolio(); | ||
| const { averageRate, portfolio, totalBalanceUSD, isPending } = usePortfolio(); |
There was a problem hiding this comment.
Keep portfolio loading state until balances are loaded
This screen now renders the non-skeleton view as soon as isPending is false, but isPending from usePortfolio no longer includes the LI.FI balances fetch; if markets resolve first, the page shows a portfolio total and asset sections computed without external/cross-chain balances, which can temporarily underreport holdings and present incomplete data as final. Include isBalancesPending in the loading gate (or fold it back into isPending) so the first rendered totals are not partial.
Useful? React with 👍 / 👎.
| address !== undefined && chainId !== undefined | ||
| ? async () => !!(await getBytecode(exaConfig, { address, chainId })) | ||
| : skipToken, | ||
| staleTime: (query) => (query.state.data === undefined ? 0 : Infinity), |
There was a problem hiding this comment.
🔴 staleTime: Infinity for deployed === false permanently caches "not deployed" status
The deployedOptions query sets staleTime: Infinity whenever data is not undefined — including when data === false. Combined with gcTime: Infinity and the fact that the "deployed" query key is NOT in the persistence exclusion list (queryClient.ts:77), a false result (account not yet deployed on a chain) is persisted to disk and never considered stale. If the user's account is later deployed on that chain, the app continues reading the cached false and shows "Non-supported network" permanently until the user clears app data.
Staleness logic detail
The condition query.state.data === undefined ? 0 : Infinity treats both true and false identically with Infinity staleTime. Only true (deployed) is a permanent state — false can change to true when the account is deployed on a new chain. The staleTime for false should be finite (or 0) so the query refetches periodically.
| staleTime: (query) => (query.state.data === undefined ? 0 : Infinity), | |
| staleTime: (query) => (query.state.data === true ? Infinity : 0), |
Was this helpful? React with 👍 or 👎 to provide feedback.
| }) | ||
| : undefined, | ||
| ]); | ||
| if (exa) balances[chain.id] = [exa, ...(balances[chain.id] ?? [])]; |
There was a problem hiding this comment.
🚩 EXA token may appear as duplicate in same-chain balances on Optimism
In balancesOptions (lifi.ts:93), the EXA token balance is explicitly fetched via getTokenBalancesByChain and prepended to balances[chain.id]. If the wallet balances API (getWalletBalances) also returns the EXA token, it would appear twice in the same-chain list. This could cause duplicate entries in externalAssets (usePortfolio) and sameChainAssets (Bridge). In practice, the LiFi wallet balances API may not include the EXA token (it's explicitly added because it's a smaller token), so duplicates may not manifest. If they do, the impact is cosmetic (duplicate rows in asset lists).
Was this helpful? React with 👍 or 👎 to provide feedback.
closes #854 , closes #649
Summary by CodeRabbit
New Features
Bug Fixes
Refactors
Localization