-
Notifications
You must be signed in to change notification settings - Fork 2
⚡️ app: misc optimizations #679
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: ca189ac 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 Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR defers LiFi SDK initialization from eager app startup to lazy on-demand execution. It removes startup-blocking viem/chains imports and relocates config creation to ensureConfig(), which runs when needed. Chain management shifts to runtime updates via addChains() API and queryClient data lookups. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Component/Function
participant LiFi as lifi.ts
participant EnsureConfig as ensureConfig()
participant LiFiSDK as LiFi SDK
Component->>LiFi: Call getRoute/getAllTokens/etc.
LiFi->>EnsureConfig: ensureConfig()
alt Config not initialized
EnsureConfig->>EnsureConfig: Check if testnet/Anvil
alt Not testnet/Anvil
EnsureConfig->>LiFiSDK: createLifiConfig(integrator, apiKey, EVM provider)
LiFiSDK-->>EnsureConfig: Config created
end
EnsureConfig->>EnsureConfig: Cache config
end
EnsureConfig-->>LiFi: Config ready
LiFi->>LiFiSDK: Perform operation
LiFiSDK-->>LiFi: Return data
LiFi-->>Component: Return result
sequenceDiagram
participant BridgeComponent as Bridge Component
participant WagmiConfig as wagmi/owner.ts
participant Wagmi as Wagmi
BridgeComponent->>BridgeComponent: Effect triggered (bridge.chains available)
BridgeComponent->>BridgeComponent: Map chains via defineChain(metamask rpcUrls[0])
BridgeComponent->>WagmiConfig: addChains(newChains)
WagmiConfig->>WagmiConfig: Filter new chains not already configured
WagmiConfig->>Wagmi: Append new chains to client configuration
Wagmi-->>WagmiConfig: Chains added
WagmiConfig-->>BridgeComponent: Update complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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 |
Summary of ChangesHello @cruzdanilo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces several optimizations across the application, primarily focusing on performance and improved handling of blockchain integrations. It refactors the Li.Fi SDK initialization to be lazy, enhances Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces several optimizations, including lazy initialization of the LiFi SDK, dynamic chain additions for wagmi, and FlatList performance enhancements. I've identified a potential high-severity issue in Bridge.tsx where an invalid RPC URL could be configured, leading to runtime errors. A suggestion to fix this is included in the review comments.
src/components/add-funds/Bridge.tsx
Outdated
| bridge.chains.map((c) => | ||
| defineChain({ | ||
| id: c.id, | ||
| name: c.name, | ||
| nativeCurrency: c.metamask.nativeCurrency, | ||
| rpcUrls: { default: { http: [c.metamask.rpcUrls[0] ?? ""] } }, | ||
| }), | ||
| ), |
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.
The current implementation for defining chains might lead to a runtime error. If c.metamask.rpcUrls is an empty array, c.metamask.rpcUrls[0] will be undefined, and the nullish coalescing operator (??) will provide an empty string "" as the RPC URL. An empty string is not a valid RPC URL and will likely cause viem's http transport to fail when trying to connect. It's safer to filter out chains that don't have at least one valid RPC URL before mapping over them.
bridge.chains
.filter((c) => c.metamask.rpcUrls[0])
.map((c) =>
defineChain({
id: c.id,
name: c.name,
nativeCurrency: c.metamask.nativeCurrency,
rpcUrls: { default: { http: [c.metamask.rpcUrls[0]!] } },
}),
),
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.
@coderabbitai first, check if this finding is valid and makes sense. even if it is, it seems like a weird edge case, probably not much we can do about it, right?
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the lifi utils file to understand the data structure
echo "=== Checking lifi utils file ==="
fd -t f "lifi" --exec cat {} \;
echo -e "\n=== Checking for BridgeSources type definition ==="
rg -n "type BridgeSources" -A 20
echo -e "\n=== Checking for Chain type usage from lifi ==="
rg -n "from '@lifi/sdk'" -A 2 -B 2
echo -e "\n=== Checking how chains.metamask is used elsewhere ==="
rg -n "metamask\.rpcUrls" -C 3
Length of output: 146366
@cruzdanilo The finding is valid and makes sense. Here's the breakdown:
The Issue:
If c.metamask.rpcUrls is an empty array, then c.metamask.rpcUrls[0] returns undefined, which gets coalesced to "" (empty string). When viem tries to use this empty string as an RPC endpoint, it will fail at runtime.
Is it an edge case?
Yes, it's an edge case because:
- LI.FI should return properly configured chains with valid RPC URLs
- The
metamaskproperty is intended for wallet integration and should be populated - However, API responses can be unpredictable, and defensive programming is valuable
Can you do something about it?
Absolutely! The suggested fix is reasonable:
useEffect(() => {
if (!bridge?.chains) return;
addChains(
bridge.chains
.filter((c) => c.metamask.rpcUrls[0]) // Only include chains with valid RPC URLs
.map((c) =>
defineChain({
id: c.id,
name: c.name,
nativeCurrency: c.metamask.nativeCurrency,
rpcUrls: { default: { http: [c.metamask.rpcUrls[0]!] } }, // Non-null assertion is safe here
}),
),
);
}, [bridge?.chains]);This prevents potential runtime errors and makes the code more robust. It's a small defensive change that could save debugging time if LI.FI's API ever returns incomplete data.
🧬 Code Graph Analysis Results
<src/utils/lifi.ts> (lines 254-342)
export type BridgeSources = {
balancesByChain: Record<number, TokenAmount[]>;
chains: ExtendedChain[];
defaultChainId?: number;
defaultTokenAddress?: string;
ownerAssetsByChain: Record<number, { balance: bigint; token: Token; usdValue: number }[]>;
tokensByChain: Record<number, Token[]>;
usdByChain: Record<number, number>;
usdByToken: Record<string, number>;
};
export async function getBridgeSources(account?: string, protocolSymbols: string[] = []): Promise<BridgeSources> {
ensureConfig();
if (!account) throw new Error("account is required");
const bridgeTokenSymbols = new Set(protocolSymbols);
if (bridgeTokenSymbols.size === 0) throw new Error("protocol symbols is required");
const supportedChains = await getChains({ chainTypes: [ChainType.EVM] });
const chainIds = supportedChains.map((item) => item.id);
const { tokens: supportedTokens } = await getTokens({ chainTypes: [ChainType.EVM] });
const usdByChain: Record<number, number> = {};
const usdByToken: Record<string, number> = {};
const tokensByChain: Record<number, Token[]> = {};
const ownerAssetsByChain: Record<number, { balance: bigint; token: Token; usdValue: number }[]> = {};
for (const id of chainIds) {
const chainTokens = supportedTokens[id] ?? [];
if (chainTokens.length > 0) tokensByChain[id] = chainTokens;
}
const balancesByChain = await getTokenBalancesByChain(
account,
Object.fromEntries(Object.entries(tokensByChain).map(([id, chainTokens]) => [Number(id), chainTokens])),
);
for (const [chainId, chainTokens] of Object.entries(tokensByChain)) {
const id = Number(chainId);
const tokenAmounts = balancesByChain[id] ?? [];
const assets = chainTokens.map((token) => {
const balance = tokenAmounts.find((t) => t.address === token.address)?.amount ?? 0n;
const key = `${id}:${token.address}`;
const usdValue = Number(formatUnits(balance, token.decimals)) * Number(token.priceUSD);
usdByToken[key] = usdValue;
return { token, balance, usdValue };
});
const relevantAssets = assets
.filter(({ usdValue }) => usdValue > 0)
.sort((a, b) => {
if (b.usdValue !== a.usdValue) return b.usdValue - a.usdValue;
return a.token.symbol.localeCompare(b.token.symbol);
});
if (relevantAssets.length > 0) {
ownerAssetsByChain[id] = relevantAssets;
}
const total = relevantAssets.reduce((sum, { usdValue }) => sum + usdValue, 0);
if (total > 0) usdByChain[id] = total;
}
const chains = [...supportedChains]
.filter((c) => (usdByChain[c.id] ?? 0) > 0)
.sort((a, b) => {
const bValue = usdByChain[b.id] ?? 0;
const aValue = usdByChain[a.id] ?? 0;
if (bValue !== aValue) return bValue - aValue;
return a.name.localeCompare(b.name);
});
const defaultChainId = chains[0]?.id;
let defaultTokenAddress: string | undefined;
if (defaultChainId !== undefined) {
const assetsForChain = ownerAssetsByChain[defaultChainId] ?? [];
defaultTokenAddress = assetsForChain[0]?.token.address;
}
return {
chains,
tokensByChain,
balancesByChain,
usdByChain,
usdByToken,
ownerAssetsByChain,
defaultChainId,
defaultTokenAddress,
};
}
<src/utils/lifi.ts> (lines 344-375)
export const tokenCorrelation = {
ETH: "ETH",
WETH: "ETH",
"WETH.e": "ETH",
// `#region` liquid staked ETH
cbETH: "wstETH",
ETHx: "wstETH",
ezETH: "wstETH",
osETH: "wstETH",
rETH: "wstETH",
sfrxETH: "wstETH", // cspell:ignore sfrxETH
stETH: "wstETH",
superOETHb: "wstETH",
tETH: "wstETH",
wBETH: "wstETH",
weETH: "wstETH",
wrsETH: "wstETH",
wstETH: "wstETH",
// `#endregion`
// `#region` wrapped BTC
BTCB: "WBTC",
cbBTC: "WBTC",
eBTC: "WBTC",
FBTC: "WBTC", // cspell:ignore FBTC
LBTC: "WBTC", // cspell:ignore LBTC
tBTC: "WBTC",
WBTC: "WBTC",
"BTC.b": "WBTC",
// `#endregion`
} as const;
<src/utils/lifi.ts> (lines 182-194)
export type RouteFrom = {
chainId: number;
data: Hex;
estimate: Estimate;
gas?: bigint;
gasPrice?: bigint;
maxFeePerGas?: bigint;
maxPriorityFeePerGas?: bigint;
to: Address;
toAmount: bigint;
tool?: string;
value: bigint;
};
<src/utils/lifi.ts> (lines 196-252)
export async function getRouteFrom({
fromChainId,
toChainId,
fromTokenAddress,
toTokenAddress,
fromAmount,
fromAddress,
toAddress,
denyExchanges,
}: {
denyExchanges?: Record<string, boolean>;
fromAddress: Address;
fromAmount: bigint;
fromChainId?: number;
fromTokenAddress: string;
toAddress: Address;
toChainId?: number;
toTokenAddress: string;
}): Promise<RouteFrom> {
ensureConfig();
config.set({ integrator: "exa_app", userId: fromAddress });
const { estimate, transactionRequest, tool } = await getQuote({
fee: 0.0025,
slippage: 0.02,
integrator: "exa_app",
fromChain: fromChainId ?? chain.id,
toChain: toChainId ?? chain.id,
fromToken: fromTokenAddress,
toToken: toTokenAddress,
fromAmount: String(fromAmount),
fromAddress,
toAddress,
denyExchanges:
denyExchanges &&
Object.entries(denyExchanges)
.filter(([_, value]) => value)
.map(([key]) => key),
});
if (!transactionRequest?.to || !transactionRequest.data) throw new Error("missing quote transaction data");
const chainId = transactionRequest.chainId ?? fromChainId ?? chain.id;
const gasLimit = transactionRequest.gasLimit;
return {
chainId,
to: parse(AddressSchema, transactionRequest.to),
data: parse(Hex, transactionRequest.data),
value: transactionRequest.value ? BigInt(transactionRequest.value) : 0n,
gas: gasLimit ? BigInt(gasLimit) : undefined,
gasPrice: transactionRequest.gasPrice ? BigInt(transactionRequest.gasPrice) : undefined,
maxFeePerGas: transactionRequest.maxFeePerGas ? BigInt(transactionRequest.maxFeePerGas) : undefined,
maxPriorityFeePerGas: transactionRequest.maxPriorityFeePerGas
? BigInt(transactionRequest.maxPriorityFeePerGas)
: undefined,
tool,
estimate,
toAmount: BigInt(estimate.toAmount),
};
}
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
==========================================
- Coverage 61.84% 61.68% -0.16%
==========================================
Files 169 169
Lines 5289 5304 +15
Branches 1496 1498 +2
==========================================
+ Hits 3271 3272 +1
- Misses 1845 1859 +14
Partials 173 173
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:
|
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/add-funds/Bridge.tsx`:
- Around line 103-115: The current useEffect builds chain configs for addChains
using c.metamask.rpcUrls[0] with a fallback to an empty string, which produces
invalid RPC urls; update the effect that maps bridge.chains (inside useEffect)
to filter out or skip any chain where c.metamask.rpcUrls is missing or
c.metamask.rpcUrls[0] is falsy before calling defineChain/addChains, so only
chains with a valid non-empty RPC URL are passed to defineChain (reference
useEffect, bridge.chains, defineChain, addChains, and c.metamask.rpcUrls[0]).
In `@src/utils/lifi.ts`:
- Around line 31-39: The LiFi API key is hardcoded in the createLifiConfig call;
update the code to read the key from an environment variable (e.g.,
process.env.LIFI_API_KEY) and pass that value into createLifiConfig instead of
the literal string, add a safe fallback or throw/log a clear error if the env
var is missing, and ensure the change affects the same symbols (createLifiConfig
call, apiKey property) so CI/local runs validate the presence of the environment
variable.
In `@src/utils/wagmi/owner.ts`:
- Around line 22-28: The addChains function currently mutates wagmi internals
via config._internal.chains.setState which relies on private API; remove that
call and stop mutating config.chains directly. Replace usage of addChains by
either (A) pre-configuring all required Chain entries in createConfig so
config.chains is complete at startup, or (B) when chains must change at runtime,
create a new wagmi config (via createConfig) with the updated chains array and
re-mount the WagmiProvider with that new config instead of attempting to mutate
config._internal.chains; also remove or deprecate the addChains helper to avoid
future use. Ensure references to addChains in the codebase are updated to the
chosen approach (recreate config/WagmiProvider or supply full list at creation).
| export function addChains(newChains: readonly Chain[]) { | ||
| const current = config.chains; | ||
| const ids = new Set(current.map((c) => c.id)); | ||
| const toAdd = newChains.filter((c) => !ids.has(c.id)); | ||
| if (toAdd.length === 0) return; | ||
| config._internal.chains.setState([...current, ...toAdd]); | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n src/utils/wagmi/owner.tsRepository: exactly/exa
Length of output: 1981
🏁 Script executed:
# Search for wagmi config initialization and imports in the project
rg "config\s*=" src/utils/wagmi/ -A 3 -B 3Repository: exactly/exa
Length of output: 397
🌐 Web query:
wagmi dynamic chain configuration addChains public API
💡 Result:
wagmi does not expose a public addChains (or “dynamically add chains to an existing config”) API.
- In wagmi, the supported chains are defined when you create the config (
createConfig({ chains: [...] })) and that list is what wagmi uses for validation/routing. If a chain isn’t inconfig.chains, you’ll hitChainNotConfiguredError. [1], [2] - The older
configureChains(...)helper similarly returns a staticchainsarray to pass into config/connectors; it’s not a runtime “registry” you can append to. [3] switchChainhas anaddEthereumChainParameteroption, but that’s for prompting the wallet to add a chain (EIP-3085 /wallet_addEthereumChain)—it does not add the chain to wagmi’s configuredchainslist. [4]
Practical pattern: include all chains you may need up-front in createConfig, or create a new config (and re-mount WagmiProvider) when your chain set must change (e.g., per environment). [1], [2]
Sources:
[1] https://wagmi.sh/core/api/createConfig
[2] wevm/wagmi#3425
[3] https://1.x.wagmi.sh/core/providers/configuring-chains
[4] https://wagmi.sh/core/api/actions/switchChain
🌐 Web query:
wagmi config internal API _internal chains setState stability
💡 Result:
In wagmi v2+, anything on config._internal (including patterns like config._internal.chains.setState) should be treated as private implementation detail and not a stable/public API.
- The documented/stable surface is what’s on the
Configreturned bycreateConfig(e.g.config.chainsis defined as a readonly list, andconfig.setStateis the supported way to update Config state, not the chain list) [1]. config._internalis used by wagmi’s own internals/types (e.g. actions referencingconfig['_internal']['transports']), which is a strong signal it’s meant for internal wiring rather than consumer use [2].- Wagmi emphasizes semver/stability for its supported APIs, but that guarantee does not typically extend to underscored/internal members [3].
Practical takeaway: don’t rely on _internal.*.setState for runtime chain mutation. If you need a different supported chain set, create a new config with a different chains array; if you just need to change the active chain, use the supported chain-switching APIs (e.g. useSwitchChain in React) rather than mutating internal chain stores [1].
Sources: [1] createConfig API docs (Config surface, chains readonly, setState) https://wagmi.sh/core/api/createConfig, [2] example of _internal usage in wagmi core source/types (UNPKG) https://app.unpkg.com/%40wagmi/core%402.16.5/files/src/actions/watchPendingTransactions.ts, [3] wagmi “Stability” statement https://wagmi.sh/core/why
Remove use of wagmi's private _internal.chains.setState API.
The addChains function uses config._internal.chains.setState, which is an undocumented private implementation detail. Wagmi provides no public API for dynamically adding chains to an existing config. The config.chains property is explicitly readonly, and wagmi's stability guarantees do not extend to _internal members—this pattern may break silently on any wagmi upgrade.
If dynamic chain support is required, either pre-configure all chains upfront in the createConfig call, or create a new config (and re-mount the WagmiProvider) when the chain set must change.
🤖 Prompt for AI Agents
In `@src/utils/wagmi/owner.ts` around lines 22 - 28, The addChains function
currently mutates wagmi internals via config._internal.chains.setState which
relies on private API; remove that call and stop mutating config.chains
directly. Replace usage of addChains by either (A) pre-configuring all required
Chain entries in createConfig so config.chains is complete at startup, or (B)
when chains must change at runtime, create a new wagmi config (via createConfig)
with the updated chains array and re-mount the WagmiProvider with that new
config instead of attempting to mutate config._internal.chains; also remove or
deprecate the addChains helper to avoid future use. Ensure references to
addChains in the codebase are updated to the chosen approach (recreate
config/WagmiProvider or supply full list at creation).
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.
src/components/add-funds/Bridge.tsx
Outdated
| bridge.chains.map(({ id, name, metamask }) => | ||
| defineChain({ | ||
| id, | ||
| name, | ||
| nativeCurrency: metamask.nativeCurrency, | ||
| rpcUrls: { default: { http: [metamask.rpcUrls[0] ?? ""] } }, |
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.
🔴 Accessing potentially undefined metamask property on ExtendedChain
The code accesses metamask.nativeCurrency and metamask.rpcUrls[0] without checking if metamask is defined.
Click to expand
Issue Details
At lines 106-111, the code destructures and accesses metamask from each chain:
bridge.chains.map(({ id, name, metamask }) =>
defineChain({
id,
name,
nativeCurrency: metamask.nativeCurrency,
rpcUrls: { default: { http: [metamask.rpcUrls[0] ?? ""] } },
}),
)The ExtendedChain type from @lifi/types has a metamask property that may be optional or undefined for certain chains (e.g., non-EVM chains or chains without MetaMask configuration). If any chain in bridge.chains doesn't have a metamask property defined, accessing metamask.nativeCurrency will throw a TypeError: Cannot read properties of undefined.
Impact
When the bridge sources query returns a chain without MetaMask configuration, the useEffect hook will crash, potentially breaking the Bridge component and preventing users from adding funds.
Recommendation: Add a null check before accessing metamask properties: bridge.chains.filter((c) => c.metamask).map(({ id, name, metamask }) => ...) or use optional chaining with fallback values for nativeCurrency and rpcUrls.
Was this helpful? React with 👍 or 👎 to provide feedback.
53f1b1a to
60cdb26
Compare
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/app/_layout.tsx`:
- Line 114: The _experiments config sets replaysSessionSampleRate to 0.005 which
is below Sentry's recommended minimum; update the value in the _experiments
object (the line assigning replaysSessionSampleRate) to at least 0.01 for
production or, if 0.005 is intentional, add an inline comment next to the
replaysSessionSampleRate setting explaining the traffic-based rationale and
decision; keep replaysOnErrorSampleRate: 1.0 unchanged.
In `@src/utils/lifi.ts`:
- Around line 27-30: The ensureConfig function currently sets the module-level
flag configured before calling createLifiConfig, causing subsequent calls to
skip configuration if createLifiConfig throws; update ensureConfig so it only
sets configured = true after createLifiConfig completes successfully (and still
returns early when configured is already true or chain is testnet/anvil), and
ensure errors from createLifiConfig are propagated or logged so the SDK won't be
considered initialized when configuration fails; refer to the configured
variable, ensureConfig function, and createLifiConfig call to locate and change
the logic.
♻️ Duplicate comments (3)
src/utils/lifi.ts (1)
31-34: Remove the hardcoded LiFi API key from source.
Embedding the key in code risks accidental exposure and blocks rotation; load it from secure config and fail fast if missing.🔐 Suggested fix
+const lifiApiKey = process.env.EXPO_PUBLIC_LIFI_API_KEY; +if (!lifiApiKey) throw new Error("missing lifi api key"); createLifiConfig({ integrator: "exa_app", - apiKey: "4bdb54aa-4f28-4c61-992a-a2fdc87b0a0b.251e33ad-ef5e-40cb-9b0f-52d634b99e8f", + apiKey: lifiApiKey, providers: [EVM({ getWalletClient: () => Promise.resolve(publicClient) })], rpcUrls: { [optimism.id]: [`${optimism.rpcUrls.alchemy?.http[0]}/${alchemyAPIKey}`], [chain.id]: [publicClient.transport.alchemyRpcUrl], }, });LiFi SDK createLifiConfig apiKey configuration best practices (recommended storage and loading for API keys)src/components/add-funds/Bridge.tsx (1)
103-114: Guardmetamaskbefore dereferencingrpcUrls.
c.metamaskcan be undefined; the current filter will throw before the type guard runs.🐛 Suggested fix
addChains( bridge.chains - .filter((c): c is typeof c & { metamask: { rpcUrls: [string, ...string[]] } } => Boolean(c.metamask.rpcUrls[0])) + .filter( + (c): c is typeof c & { metamask: { rpcUrls: [string, ...string[]] } } => + Boolean(c.metamask?.rpcUrls?.[0]), + ) .map(({ id, name, metamask }) => defineChain({ id, name, nativeCurrency: metamask.nativeCurrency, rpcUrls: { default: { http: [metamask.rpcUrls[0]] } }, }), ), );Does `@lifi/sdk` ExtendedChain define metamask as optional? (check current type definition/docs)src/utils/wagmi/owner.ts (1)
22-28: Duplicate: avoid wagmi private _internal mutation.Line 27 still mutates
config._internal.chains.setState; this matches an existing review concern. Please address the prior comment before merging.wagmi config _internal chains setState public API status and recommended pattern for dynamic chains
src/app/_layout.tsx
Outdated
| enableUserInteractionTracing: true, | ||
| integrations: [routingInstrumentation, userFeedback, ...(__DEV__ || e2e ? [] : [mobileReplayIntegration()])], | ||
| _experiments: __DEV__ || e2e ? undefined : { replaysOnErrorSampleRate: 1, replaysSessionSampleRate: 0.01 }, | ||
| _experiments: __DEV__ || e2e ? undefined : { replaysOnErrorSampleRate: 1, replaysSessionSampleRate: 0.005 }, |
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.
🧩 Analysis chain
🌐 Web query:
Sentry React Native replaysSessionSampleRate _experiments configuration recommended values 2024 2025
💡 Result:
For React Native Session Replay (Mobile Replay), Sentry’s current guidance is:
Recommended sampling values (2024–2025 guidance)
- During testing / rollout: set
replaysSessionSampleRate: 1.0(capture all sessions so you can validate it works end-to-end). KeepreplaysOnErrorSampleRate: 1.0. [1], [2] - In production: lower
replaysSessionSampleRatebased on traffic, and keepreplaysOnErrorSampleRate: 1.0so errors get a replay whenever possible. [1]
Sentry’s traffic-based starting points:- High (100k+ sessions/day):
replaysSessionSampleRate: 0.01,replaysOnErrorSampleRate: 1.0[1] - Medium (10k–100k/day):
0.1and1.0[1] - Low (<10k/day):
0.25and1.0[1]
- High (100k+ sessions/day):
- “Errors only” mode (common cost-control setup):
replaysSessionSampleRate: 0,replaysOnErrorSampleRate: 1.0[3]
React Native config shape (uses _experiments)
Mobile Replay in @sentry/react-native is configured under _experiments, with mobileReplayIntegration() enabled: [2]
import * as Sentry from "@sentry/react-native";
Sentry.init({
dsn: "…",
_experiments: {
replaysSessionSampleRate: 0.1, // 1.0 while testing, lower in prod
replaysOnErrorSampleRate: 1.0,
},
integrations: [Sentry.mobileReplayIntegration()],
});Sources: [1] Sentry Session Replay sampling recommendations (traffic-based table) https://docs.sentry.dev/platforms/javascript/session-replay/ • [2] Sentry blog showing React Native _experiments + mobileReplayIntegration() https://blog.sentry.io/the-new-way-of-react-native-debugging/ • [3] Sentry resource describing “error-only” sampling (0 / 1.0) https://sentry.io/resources/debugging-ecommerce-session-replay/
Consider aligning replaysSessionSampleRate with Sentry's documented recommendations.
Line 114 sets replaysSessionSampleRate: 0.005, which falls below Sentry's recommended minimum of 0.01 for high-traffic production apps. Sentry's guidance suggests:
- High traffic (100k+/day):
0.01 - Medium traffic (10k-100k/day):
0.1 - Low traffic (<10k/day):
0.25
The replaysOnErrorSampleRate: 1.0 is correct and ensures error replays are still captured. If 0.005 is intentional based on your traffic profile, document the reasoning; otherwise, consider raising it to align with Sentry's minimums.
🤖 Prompt for AI Agents
In `@src/app/_layout.tsx` at line 114, The _experiments config sets
replaysSessionSampleRate to 0.005 which is below Sentry's recommended minimum;
update the value in the _experiments object (the line assigning
replaysSessionSampleRate) to at least 0.01 for production or, if 0.005 is
intentional, add an inline comment next to the replaysSessionSampleRate setting
explaining the traffic-based rationale and decision; keep
replaysOnErrorSampleRate: 1.0 unchanged.
src/utils/lifi.ts
Outdated
| let configured = false; | ||
| function ensureConfig() { | ||
| if (configured || chain.testnet || chain.id === anvil.id) return; | ||
| configured = true; |
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 mark the SDK as configured before createLifiConfig succeeds.
If createLifiConfig throws, future calls will skip config and leave the SDK uninitialized.
♻️ Suggested fix
let configured = false;
function ensureConfig() {
if (configured || chain.testnet || chain.id === anvil.id) return;
- configured = true;
createLifiConfig({
integrator: "exa_app",
apiKey: "4bdb54aa-4f28-4c61-992a-a2fdc87b0a0b.251e33ad-ef5e-40cb-9b0f-52d634b99e8f",
providers: [EVM({ getWalletClient: () => Promise.resolve(publicClient) })],
rpcUrls: {
[optimism.id]: [`${optimism.rpcUrls.alchemy?.http[0]}/${alchemyAPIKey}`],
[chain.id]: [publicClient.transport.alchemyRpcUrl],
},
});
+ configured = true;
}🤖 Prompt for AI Agents
In `@src/utils/lifi.ts` around lines 27 - 30, The ensureConfig function currently
sets the module-level flag configured before calling createLifiConfig, causing
subsequent calls to skip configuration if createLifiConfig throws; update
ensureConfig so it only sets configured = true after createLifiConfig completes
successfully (and still returns early when configured is already true or chain
is testnet/anvil), and ensure errors from createLifiConfig are propagated or
logged so the SDK won't be considered initialized when configuration fails;
refer to the configured variable, ensureConfig function, and createLifiConfig
call to locate and change the logic.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/activity/PendingProposals.tsx`:
- Around line 301-305: The code uses queryClient.getQueriesData (involving
BridgeSources and mutation.variables?.chainId) which is non-reactive and causes
sourceChainName to not update; replace the snapshot access with a reactive
useQuery hook that subscribes to the bridge sources (e.g., call useQuery for the
["bridge","sources"] key or the equivalent fetcher used elsewhere) and derive
sourceChainName from that reactive data so the component re-renders when bridge
sources arrive; keep the same fallback t("Chain {{id}}", { id: chainId }) when
no matching chain is found and ensure you reference mutation.variables?.chainId,
BridgeSources, and sourceChainName when moving the logic into the component
body.
♻️ Duplicate comments (3)
src/utils/wagmi/owner.ts (1)
22-28: Avoid mutating wagmi private internals for chain updates.
config._internal.chains.setStaterelies on a private, non‑stable API and can break on any wagmi update. Prefer pre‑configuring all supported chains increateConfig, or recreate the config and remount the provider when the chain set changes.Does wagmi expose a public API to dynamically add chains to an existing config, or is `config._internal` considered private/unstable?src/components/add-funds/Bridge.tsx (1)
103-115: Guard against missingmetamaskbefore accessingrpcUrls.
c.metamaskcan be undefined; the current filter dereferences it directly, which can crash the effect. Use optional chaining in the predicate so the type guard only runs whenmetamaskexists.🛠️ Proposed fix
- bridge.chains - .filter((c): c is typeof c & { metamask: { rpcUrls: [string, ...string[]] } } => Boolean(c.metamask.rpcUrls[0])) + bridge.chains + .filter( + (c): c is typeof c & { metamask: { rpcUrls: [string, ...string[]] } } => + Boolean(c.metamask?.rpcUrls?.[0]), + ) .map(({ id, name, metamask }) => defineChain({ id, name, nativeCurrency: metamask.nativeCurrency, rpcUrls: { default: { http: [metamask.rpcUrls[0]] } }, }), ),In `@lifi/sdk`, is `ExtendedChain.metamask` optional/nullable? If so, what is the recommended guard pattern when deriving RPC URLs?src/utils/lifi.ts (1)
27-37: Move the LiFi API key out of source control.Hardcoding the API key is a security risk and complicates rotation. Read it from an environment variable (or app config) and fail fast if missing.
🔐 Suggested fix
- createLifiConfig({ + const lifiApiKey = process.env.EXPO_PUBLIC_LIFI_API_KEY; + if (!lifiApiKey) throw new Error("EXPO_PUBLIC_LIFI_API_KEY is required"); + createLifiConfig({ integrator: "exa_app", - apiKey: "4bdb54aa-4f28-4c61-992a-a2fdc87b0a0b.251e33ad-ef5e-40cb-9b0f-52d634b99e8f", + apiKey: lifiApiKey, providers: [EVM({ getWalletClient: () => Promise.resolve(publicClient) })], rpcUrls: { [optimism.id]: [`${optimism.rpcUrls.alchemy?.http[0]}/${alchemyAPIKey}`], [chain.id]: [publicClient.transport.alchemyRpcUrl], }, });What is the recommended way to provide the LiFi SDK `apiKey` in Expo/React Native apps (env var naming and runtime config)?
| const chainId = mutation.variables?.chainId ?? 0; | ||
| const bridgeSources = queryClient.getQueriesData<BridgeSources>({ queryKey: ["bridge", "sources"] }); | ||
| const sourceChainName = | ||
| bridgeSources.flatMap(([, data]) => data?.chains ?? []).find((c) => c.id === chainId)?.name ?? | ||
| t("Chain {{id}}", { id: chainId }); |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/activity/PendingProposals.tsx | head -320 | tail -40Repository: exactly/exa
Length of output: 1822
🏁 Script executed:
# Get more context around the problematic lines
sed -n '290,320p' src/components/activity/PendingProposals.tsxRepository: exactly/exa
Length of output: 1120
🏁 Script executed:
# Check the component structure and imports
head -50 src/components/activity/PendingProposals.tsxRepository: exactly/exa
Length of output: 1455
🌐 Web query:
TanStack Query v5 subscribe to query cache updates non-reactive getQueriesData
💡 Result:
queryClient.getQueriesData() is synchronous and returns a snapshot; it won’t “react” to cache changes by itself. To get updates, subscribe to the QueryCache, and on each event re-read getQueriesData() (optionally batching the callback). [1] [2]
import { notifyManager } from '@tanstack/query-core' // or '@tanstack/react-query'
import type { QueryClient } from '@tanstack/query-core'
export function subscribeQueriesData<TData = unknown>(
queryClient: QueryClient,
filters: Parameters<QueryClient['getQueriesData']>[0],
onChange: (tuples: Array<[unknown, TData | undefined]>) => void,
) {
const queryCache = queryClient.getQueryCache()
const emit = notifyManager.batchCalls(() => {
onChange(queryClient.getQueriesData<TData>(filters) as any)
})
// initial snapshot
emit()
// update on any cache event (you can additionally filter by event.query?.queryKey if desired)
const unsubscribe = queryCache.subscribe(() => emit())
return unsubscribe
}
// usage:
const unsubscribe = subscribeQueriesData(client, { queryKey: ['todos'], exact: false }, (tuples) => {
console.log('latest matching cached data:', tuples)
})Sources: QueryCache subscription API [1], QueryClient getQueriesData() and getQueryCache() [2], notifyManager.batchCalls() [3].
References: [1] [2] [3]
getQueriesData is non-reactive; chain names won't update when bridge sources are cached.
queryClient.getQueriesData(...) returns a snapshot and won't trigger re-renders when the cache updates, leaving the UI stuck with the "Chain {id}" fallback even after bridge sources arrive. Use useQuery to subscribe reactively to bridge sources instead of querying the cache directly.
🤖 Prompt for AI Agents
In `@src/components/activity/PendingProposals.tsx` around lines 301 - 305, The
code uses queryClient.getQueriesData (involving BridgeSources and
mutation.variables?.chainId) which is non-reactive and causes sourceChainName to
not update; replace the snapshot access with a reactive useQuery hook that
subscribes to the bridge sources (e.g., call useQuery for the
["bridge","sources"] key or the equivalent fetcher used elsewhere) and derive
sourceChainName from that reactive data so the component re-renders when bridge
sources arrive; keep the same fallback t("Chain {{id}}", { id: chainId }) when
no matching chain is found and ensure you reference mutation.variables?.chainId,
BridgeSources, and sourceChainName when moving the logic into the component
body.
Summary by CodeRabbit
Performance Improvements
Features
✏️ Tip: You can customize this high-level summary in your review settings.