✨ app: provision alchemy rpc urls across lifi chains#833
✨ app: provision alchemy rpc urls across lifi chains#833cruzdanilo wants to merge 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: a3f82fa 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 |
WalkthroughAdds a new changeset announcing a patch release for dynamic Alchemy RPC URL provisioning. Refactors RPC configuration to dynamically build RPC URL mappings from infra exports and adds asynchronous chain loading to fetch available EVM chains at runtime. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Config as ensureConfig
participant Infra as Infra Exports
participant LiFi as LiFi Config
participant Loader as Chain Loader
participant MetaMask as MetaMask
App->>Config: Call ensureConfig()
Config->>Infra: Scan for id & rpcUrls.alchemy.http
Infra-->>Config: Return RPC URLs map
Config->>LiFi: Update rpcUrls & preloadChains
Config->>Loader: Trigger config.loading (async)
Loader->>Loader: Fetch available EVM chains
Loader->>MetaMask: Reset rpcUrls
Loader->>LiFi: Update config.chains
Loader-->>Config: Complete (errors reported)
Config-->>App: Return configured state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 significantly enhances the application's ability to interact with various blockchain networks by implementing a dynamic system for provisioning Alchemy RPC URLs across all supported LiFi chains. This change improves the flexibility and scalability of the LiFi integration, ensuring that the application can automatically configure RPC endpoints for new or existing chains without manual updates, leading to a more robust and maintainable system. Highlights
Changelog
Activity
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
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .catch((error: unknown) => { | ||
| reportError(error); | ||
| }); | ||
| configured = true; |
There was a problem hiding this comment.
race condition: configured is set before config.loading completes. subsequent calls to functions like getRoute (line 135) or getAllTokens (line 196) won't re-run ensureConfig, but chains may not be fully loaded yet, potentially causing issues if they rely on config.setChains being complete.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| config.loading = getChains({ chainTypes: [ChainType.EVM] }) | ||
| .then((availableChains) => { | ||
| const rpcs = config.get().rpcUrls as Partial<Record<number, readonly string[]>>; | ||
| config.setChains( | ||
| availableChains.map((c) => (rpcs[c.id]?.length ? { ...c, metamask: { ...c.metamask, rpcUrls: [] } } : c)), | ||
| ); | ||
| }) | ||
| .catch((error: unknown) => { | ||
| reportError(error); | ||
| }); | ||
| configured = true; | ||
| queryClient.prefetchQuery(lifiTokensOptions).catch(reportError); | ||
| queryClient.prefetchQuery(lifiChainsOptions).catch(reportError); |
There was a problem hiding this comment.
🚩 Duplicate getChains call — config.loading and lifiChainsOptions both fetch the same data
The new config.loading at src/utils/lifi.ts:112 calls getChains({ chainTypes: [ChainType.EVM] }), and then src/utils/lifi.ts:124 prefetches lifiChainsOptions which also calls getChains({ chainTypes: [ChainType.EVM] }) (defined at src/utils/lifi.ts:37). The lifi SDK internally awaits config.loading before API operations, so the lifiChainsOptions prefetch will first wait for the config.loading chain fetch to complete, then make a second identical API call.
This is functionally correct but results in two sequential getChains calls on initialization — the first to configure chain RPC overrides, the second to populate the query cache. Consider reusing the chains fetched by config.loading to populate the query cache directly, avoiding the redundant network request.
Was this helpful? React with 👍 or 👎 to provide feedback.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| .then((availableChains) => { | ||
| const rpcs = config.get().rpcUrls as Partial<Record<number, readonly string[]>>; | ||
| config.setChains( | ||
| availableChains.map((c) => (rpcs[c.id]?.length ? { ...c, metamask: { ...c.metamask, rpcUrls: [] } } : c)), |
There was a problem hiding this comment.
Bug: The code spreads c.metamask without checking if it exists. If c.metamask is undefined, this creates a new, incomplete metamask object, leading to incorrect LiFi SDK configuration.
Severity: MEDIUM
Suggested Fix
Conditionally update the metamask property only if it already exists on the chain object c. For example, change the logic to rpcs[c.id]?.length && c.metamask ? { ...c, metamask: { ...c.metamask, rpcUrls: [] } } : c.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/utils/lifi.ts#L116
Potential issue: When configuring chains for the LiFi SDK, the code maps over
`availableChains`. If a chain has a configured RPC, it attempts to modify the chain's
`metamask` property by spreading it: `{ ...c, metamask: { ...c.metamask, rpcUrls: [] }
}`. The `metamask` property is optional on a chain object `c`. If `c.metamask` is
`undefined`, the spread results in a new, incomplete object `{ rpcUrls: [] }`. This
creates a malformed `metamask` configuration, as it's missing other required properties
like `chainId` and `chainName`, which can cause downstream errors within the LiFi SDK.
Did we get this right? 👍 / 👎 to inform future reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the LiFi configuration to dynamically provision Alchemy RPC URLs for all supported EVM chains from @account-kit/infra, which is a good improvement over hardcoding them. However, a high-severity security vulnerability was identified: a hardcoded LiFi API key in src/utils/lifi.ts that must be externalized to an environment variable to prevent secret exposure. Additionally, two potential runtime errors were found in the new logic: a missing null check when iterating through infra chains and an unguarded spread of an optional property when modifying chain configurations, both of which could lead to crashes. The rest of the changes, including @lifi/sdk dependency modifications, do not appear to introduce new security risks and include beneficial validation enhancements.
| if (configured || chain.testnet || chain.id === anvil.id) return; | ||
| const rpcUrls = { | ||
| ...Object.values(infra).reduce<Record<number, string[]>>((result, item) => { | ||
| if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result; |
There was a problem hiding this comment.
The check typeof item !== "object" is not sufficient to guard against null, because typeof null is 'object'. If item is null, the expression !("id" in item) will throw a TypeError. You should add a check for item === null.
| if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result; | |
| if (typeof item !== "object" || item === null || !("id" in item) || !("rpcUrls" in item)) return result; |
| .then((availableChains) => { | ||
| const rpcs = config.get().rpcUrls as Partial<Record<number, readonly string[]>>; | ||
| config.setChains( | ||
| availableChains.map((c) => (rpcs[c.id]?.length ? { ...c, metamask: { ...c.metamask, rpcUrls: [] } } : c)), |
There was a problem hiding this comment.
The metamask property on ExtendedChain (the type of c) is optional. If c.metamask is undefined, the spread operator ...c.metamask will throw a TypeError. You should handle this case, for example by providing a default empty object.
| availableChains.map((c) => (rpcs[c.id]?.length ? { ...c, metamask: { ...c.metamask, rpcUrls: [] } } : c)), | |
| availableChains.map((c) => (rpcs[c.id]?.length ? { ...c, metamask: { ...(c.metamask ?? {}), rpcUrls: [] } } : c)), |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #833 +/- ##
==========================================
- Coverage 68.95% 68.82% -0.13%
==========================================
Files 211 211
Lines 7444 7459 +15
Branches 2385 2388 +3
==========================================
+ Hits 5133 5134 +1
- Misses 2096 2109 +13
- Partials 215 216 +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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| const rpcUrls = { | ||
| ...Object.values(infra).reduce<Record<number, string[]>>((result, item) => { | ||
| if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result; | ||
| const candidate = item as { id: number; rpcUrls: { alchemy?: { http?: string[] } } }; | ||
| const alchemyRpcUrl = candidate.rpcUrls.alchemy?.http?.[0]; | ||
| if (!alchemyRpcUrl) return result; | ||
| result[candidate.id] = [`${alchemyRpcUrl}/${alchemyAPIKey}`]; | ||
| return result; | ||
| }, {}), | ||
| [chain.id]: [publicClient.transport.alchemyRpcUrl], | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
rpcUrls is used in a single place — consider inlining per project guidelines.
The rpcUrls variable is only consumed in the createLifiConfig call on line 110. As per coding guidelines, single-use values should be kept inline rather than extracted into a named variable.
♻️ Proposed inline refactor
- const rpcUrls = {
- ...Object.values(infra).reduce<Record<number, string[]>>((result, item) => {
- if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result;
- const candidate = item as { id: number; rpcUrls: { alchemy?: { http?: string[] } } };
- const alchemyRpcUrl = candidate.rpcUrls.alchemy?.http?.[0];
- if (!alchemyRpcUrl) return result;
- result[candidate.id] = [`${alchemyRpcUrl}/${alchemyAPIKey}`];
- return result;
- }, {}),
- [chain.id]: [publicClient.transport.alchemyRpcUrl],
- };
createLifiConfig({
integrator: "exa_app",
apiKey: "4bdb54aa-4f28-4c61-992a-a2fdc87b0a0b.251e33ad-ef5e-40cb-9b0f-52d634b99e8f",
preloadChains: false,
providers: [EVM({ getWalletClient: () => Promise.resolve(publicClient) })],
- rpcUrls,
+ rpcUrls: {
+ ...Object.values(infra).reduce<Record<number, string[]>>((result, item) => {
+ if (typeof item !== "object" || !item || !("id" in item) || !("rpcUrls" in item)) return result;
+ const candidate = item as { id: number; rpcUrls: { alchemy?: { http?: string[] } } };
+ const alchemyRpcUrl = candidate.rpcUrls.alchemy?.http?.[0];
+ if (!alchemyRpcUrl) return result;
+ result[candidate.id] = [`${alchemyRpcUrl}/${alchemyAPIKey}`];
+ return result;
+ }, {}),
+ [chain.id]: [publicClient.transport.alchemyRpcUrl],
+ },
});Note: the
!itemguard is included to prevent aTypeErrorwhen"id" in itemis evaluated against anullvalue (see below).
As per coding guidelines: "Do not extract a value into a variable or logic into a function unless it is used in two or more places; keep single-use values and functions inline."
| if (configured || chain.testnet || chain.id === anvil.id) return; | ||
| const rpcUrls = { | ||
| ...Object.values(infra).reduce<Record<number, string[]>>((result, item) => { | ||
| if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result; |
There was a problem hiding this comment.
null slips past the typeof item !== "object" guard.
typeof null === "object" in JavaScript, so null values are not filtered by the typeof check. The subsequent "id" in item call on null throws a TypeError at runtime. While @account-kit/infra is unlikely to export null today, this is fragile.
🛡️ Proposed fix
- if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result;
+ if (!item || typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result; | |
| if (!item || typeof item !== "object" || !("id" in item) || !("rpcUrls" in item)) return result; |
| result[candidate.id] = [`${alchemyRpcUrl}/${alchemyAPIKey}`]; | ||
| return result; | ||
| }, {}), | ||
| [chain.id]: [publicClient.transport.alchemyRpcUrl], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the lifi.ts file around line 103
fd -e ts -path "*utils/lifi*" --exclude "*.test.*"Repository: exactly/exa
Length of output: 223
🏁 Script executed:
# Find publicClient definition
fd -e ts "publicClient" --exclude "*.test.*" --exclude node_modulesRepository: exactly/exa
Length of output: 109
🏁 Script executed:
# Search for how alchemyRpcUrl is defined or used
rg "alchemyRpcUrl" --type tsRepository: exactly/exa
Length of output: 357
🏁 Script executed:
# Read the lifi.ts file to see full context around line 103
cat -n src/utils/lifi.ts | head -120 | tail -30Repository: exactly/exa
Length of output: 1596
🏁 Script executed:
# Read publicClient.ts to understand the transport type
cat -n src/utils/publicClient.tsRepository: exactly/exa
Length of output: 487
🏁 Script executed:
# Check if there's any guard or conditional before line 103
cat -n src/utils/lifi.ts | sed -n '90,110p'Repository: exactly/exa
Length of output: 1132
🏁 Script executed:
# Search for alchemy transport typing and alchemyRpcUrl property
rg "alchemyRpcUrl" -B 2 -A 2 --type-listRepository: exactly/exa
Length of output: 444
🏁 Script executed:
# Check what `@account-kit/infra` exports and how alchemy transport is typed
rg "@account-kit/infra" src/utils/publicClient.tsRepository: exactly/exa
Length of output: 130
🏁 Script executed:
# Check how alchemyAPIKey is imported and what its default value is
rg "alchemyAPIKey" src/utils/lifi.ts -B 2 -A 2Repository: exactly/exa
Length of output: 527
🏁 Script executed:
# Find the alchemyAPIKey definition/export
fd -e ts -e js "alchemyAPIKey" --exclude node_modulesRepository: exactly/exa
Length of output: 78
🏁 Script executed:
# Read the alchemyAPIKey definition
cat -n common/alchemyAPIKey.tsRepository: exactly/exa
Length of output: 1036
🏁 Script executed:
# Also check for type definitions in publicClient.ts to see if alchemyRpcUrl is properly typed
rg "transport" src/utils/publicClient.ts -B 2 -A 2Repository: exactly/exa
Length of output: 198
🏁 Script executed:
# Check if line 103 execution is guarded for anvil chain
cat -n src/utils/lifi.ts | sed -n '92,104p'Repository: exactly/exa
Length of output: 795
Add a guard to ensure publicClient.transport.alchemyRpcUrl is defined before using it.
If the alchemy transport doesn't expose the alchemyRpcUrl property or it's undefined, the code will add an undefined value to the RPC URLs array, silently breaking LiFi's RPC configuration for the active chain. Consider adding a check like:
[chain.id]: publicClient.transport.alchemyRpcUrl ? [publicClient.transport.alchemyRpcUrl] : [],Or retrieve the URL from the same infra object used above to maintain consistency.
1 participant
Greptile Summary
replaces hardcoded optimism rpc url with dynamic provisioning across all lifi-supported evm chains by iterating through
@account-kit/infraexports. addspreloadChains: falseand async chain loading viaconfig.loadingto defer chain configuration until after the initial lifi setup.@account-kit/infrapackageconfig.loadingpromiseconfigured = trueis set beforeconfig.loadingcompletes, which may cause subsequent function calls to proceed before chains are fully loadedConfidence Score: 2/5
configuredis set totruebefore the asyncconfig.loadingpromise completes could cause functions likegetRoute,getAllTokens, and others to execute before chains are fully configured viaconfig.setChains. this may lead to unpredictable behavior when lifi operations are called quickly after initialization.ensureConfigfunctionImportant Files Changed
@account-kit/infraexports and adds async chain loading withpreloadChains: falseLast reviewed commit: a3f82fa
Summary by CodeRabbit
Release Notes