-
Notifications
You must be signed in to change notification settings - Fork 1
feat: make the agent know which specific evm chains the wallet supports #141
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds fetching of WalletConnect-approved chain IDs on the client, threads those IDs through the chat transport, and updates the server chat route and prompt/context builders to filter and label wallets/networks based on the provided approvedChainIds. Changes
Sequence Diagram(s)sequenceDiagram
participant ClientApp as Client App
participant Hook as useApprovedChains
participant WC as WalletConnect Provider
participant ChatProv as ChatProvider
participant Server as Agentic Server
participant Prompt as Prompt Builders / LLM
ClientApp->>Hook: mount / call
Hook->>WC: read wagmi connectors -> WalletConnect connector
WC->>Hook: return session namespaces (eip155, solana)
Hook->>Hook: extract & dedupe chain IDs (approvedChainIds)
Hook-->>ClientApp: approvedChainIds
ClientApp->>ChatProv: compose transport (evm, solana, approvedChainIds)
ChatProv->>Server: POST /chat with transport body (includes approvedChainIds)
Server->>Server: handleChatRequest -> parse approvedChainIds
Server->>Prompt: buildWalletContext/buildSystemPrompt(approvedChainIds)
Prompt->>Server: system prompt + wallet context (filtered & labeled)
Server->>Prompt: call LLM / continue chat flow
Prompt-->>Server: LLM response
Server-->>ChatProv: chat response
ChatProv-->>ClientApp: present response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/agentic-server/src/routes/chat.ts (1)
61-88: Verify injected wallet behavior.The logic assumes injected wallets (like MetaMask) won't provide
approvedChainIds, falling back to all chains. Ensure that:
- Injected wallets actually send
undefinedor empty array forapprovedChainIds- An empty array (
[]) is handled correctly - currentlyapprovedChainIds.length > 0would treat empty array same as undefinedConsider clarifying the empty array case:
- const chainsToRegister = - approvedChainIds && approvedChainIds.length > 0 - ? allEvmChainIds.filter(chainId => approvedChainIds.includes(chainId)) - : allEvmChainIds + const chainsToRegister = + approvedChainIds?.length + ? allEvmChainIds.filter(chainId => approvedChainIds.includes(chainId)) + : allEvmChainIdsAnd similarly for Solana:
const solanaApproved = - !approvedChainIds || approvedChainIds.length === 0 || approvedChainIds.includes(solanaChainId) + !approvedChainIds?.length || approvedChainIds.includes(solanaChainId)
🧹 Nitpick comments (2)
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
60-71: Consider session change scenarios.The
staleTime: Infinityprevents automatic refetching. While the query key includesconnector?.id(which should trigger refetch on connector change), verify that this covers all scenarios where approved chains might change during a session (e.g., user approves additional networks in their wallet).If approved chains can change mid-session, consider:
- Reducing
staleTimeto a finite value (e.g., 30 seconds)- Or adding a mechanism to manually invalidate the query when wallet events indicate session updates
apps/agentic-server/src/routes/chat.ts (1)
178-211: Consider refining the "Not Connected" message.The logic correctly distinguishes between connected and not-connected networks. However, the message "Not Connected by Wallet (user must add these networks in their wallet to use them)" is quite verbose for a system prompt.
Consider shortening to:
- prompt += `\n**Not Connected by Wallet (user must add these networks in their wallet to use them):** ${notConnectedNames.join(', ')}` + prompt += `\n**Not Connected (add in wallet to enable):** ${notConnectedNames.join(', ')}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/agentic-chat/src/hooks/useApprovedChains.ts(1 hunks)apps/agentic-chat/src/hooks/useWalletConnection.ts(2 hunks)apps/agentic-chat/src/providers/ChatProvider.tsx(2 hunks)apps/agentic-server/src/routes/chat.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
apps/agentic-chat/src/lib/wagmi-config.ts (1)
wagmiConfig(19-19)
apps/agentic-chat/src/hooks/useWalletConnection.ts (1)
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
useApprovedChains(60-71)
apps/agentic-chat/src/providers/ChatProvider.tsx (1)
apps/agentic-chat/src/hooks/useWalletConnection.ts (1)
useWalletConnection(6-20)
apps/agentic-server/src/routes/chat.ts (2)
packages/caip/src/constants.ts (9)
ethChainId(28-28)arbitrumChainId(34-34)optimismChainId(30-30)baseChainId(36-36)polygonChainId(32-32)avalancheChainId(29-29)bscChainId(31-31)gnosisChainId(33-33)solanaChainId(40-40)apps/agentic-server/src/utils/walletContextSimple.ts (1)
WalletContext(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (9)
apps/agentic-chat/src/hooks/useWalletConnection.ts (1)
4-5: LGTM!The integration of
useApprovedChainsis clean and follows the existing pattern. The hook correctly extends the wallet connection state with approved chain IDs.Also applies to: 9-9, 18-18
apps/agentic-chat/src/providers/ChatProvider.tsx (1)
46-46: LGTM!The transport payload and memoization are correctly updated to include
approvedChainIds. The dependency array properly reflects all values used in the transport body.Also applies to: 64-66, 69-69
apps/agentic-chat/src/hooks/useApprovedChains.ts (3)
7-20: LGTM!The type definitions correctly model the WalletConnect session structure for both EIP-155 (EVM) and Solana namespaces.
22-24: LGTM!The extraction logic correctly parses CAIP-2 chain identifiers from WalletConnect account strings (format:
namespace:chainId:address) and properly deduplicates the results.
26-58: No action needed — the WalletConnect connector ID is correct.The connector ID
'walletConnect'matches the official Reown AppKit specification. No changes are required.apps/agentic-server/src/routes/chat.ts (4)
36-47: LGTM!The chain ID collections are correctly defined and comprehensive, covering all supported EVM chains and Solana.
166-176: LGTM!The chain ID to name mapping is complete and uses appropriate human-readable names for all supported networks.
213-216: LGTM!The system prompt correctly propagates the
approvedChainIdsparameter to the connected wallets prompt.
293-301: LGTM!The request handler correctly extracts
approvedChainIdsfrom the request body and propagates it through the wallet context and system prompt construction.Also applies to: 309-309
a35166b to
d0bdf1e
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: 0
🧹 Nitpick comments (4)
apps/agentic-chat/src/hooks/useApprovedChains.ts (2)
22-24: Clarify CAIP parsing assumptions inextractChainIdsFromAccountsThis correctly reduces CAIP-style account strings to
namespace:referencechain IDs and dedupes them, but it relies on the account format always beingnamespace:reference:address.If there’s any chance of other formats slipping in (e.g., legacy non‑CAIP accounts), consider making the parsing a bit more explicit and defensive so the intent is clearer:
-function extractChainIdsFromAccounts(accounts: string[]): string[] { - return [...new Set(accounts.map(account => account.split(':').slice(0, 2).join(':')).filter(id => id.includes(':')))] -} +function extractChainIdsFromAccounts(accounts: string[]): string[] { + const chainIds = accounts + .map(account => { + const parts = account.split(':') + if (parts.length < 2) return null + const [namespace, reference] = parts + if (!namespace || !reference) return null + return `${namespace}:${reference}` + }) + .filter((id): id is string => !!id) + + return [...new Set(chainIds)] +}This keeps behavior identical for CAIP-10 accounts while avoiding surprises if formats vary slightly.
26-32: Scope approved-chain query to WalletConnect and consider refresh behaviorFunctionally this works, but there are a couple of behavioral edge cases worth tightening:
Connector mismatch:
getApprovedChainsFromSessionalways looks forconnector.id === 'walletConnect'across all connections, whileuseApprovedChainsonly gates onisConnected. If your app ever allows being connected with a non‑WalletConnect connector (e.g., injected) while a WalletConnect session is also present, you could end up:
- Using an injected
evmAddress, but- Applying
approvedChainIdsderived from a different WalletConnect session.A minimal fix is to only enable the query when WalletConnect is actually the active connector:
export function useApprovedChains(): string[] { const { isConnected, connector } = useAccount() const { data = [] } = useQuery({ queryKey: ['approvedChains', isConnected, connector?.id], queryFn: getApprovedChainsFromSession,
enabled: isConnected,
enabled: isConnected && connector?.id === 'walletConnect', staleTime: Infinity,})
return data
}This keeps the rest of the implementation intact but prevents mixing approvals from WalletConnect with non‑WalletConnect connections.
- Approvals never refresh with
staleTime: Infinity:
With infinite stale time, the first successful fetch for a given key is effectively frozen; adding/removing approved chains in the wallet mid‑session won’t be picked up unless you manually invalidate this query. If you expect users to change approvals without reconnecting, consider a finitestaleTimeor an explicit invalidation trigger when the WalletConnect session changes.If your UX guarantees that only a single connector is ever connected and approvals don't change mid‑session, current behavior is acceptable but a short comment documenting those assumptions would help future maintainers.
Also applies to: 61-68
apps/agentic-server/src/routes/chat.ts (2)
36-48: Centralized chain ID lists are clear; keep them in sync with other configsDefining
allEvmChainIdsandallSupportedChainIdshere makes the filtering logic inbuildWalletContextand prompts much easier to reason about.The only thing to watch going forward is drift vs. other “supported chains” definitions (e.g.,
supportedChainsContext, any tool-level configs). When adding/removing networks, ensure these arrays andchainIdToNameare updated together so prompts and walletContext stay aligned.
166-211: Connected/Not‑Connected networks prompt is helpful; consider aligning it with supported chain setThe prompt construction reads well and correctly:
- Uses
chainIdToNameso only known networks are rendered.- Derives
notConnectedNamesfromallSupportedChainIds, so “Not Connected by Wallet” is only about chains you actually support.A couple of minor considerations:
- If new supported chains are added to
allEvmChainIds/allSupportedChainIdsbutchainIdToNameisn’t updated, they’ll silently drop out of the networks lists (filtered by theBooleanguard). That’s safe but a small maintenance footgun—worth a short comment or reusing a shared chain config if you have one.connectedNamesis driven purely byapprovedChainIds, not by what actually made it intowalletContext.connectedWallets. If there’s any path where a chain can be “approved” but no address is registered for it (e.g., Solana present in approvals butsolanaAddressis undefined), the LLM will be told that network is “Connected” even though tools can’t actually use it. If that scenario is possible, you might want to baseconnectedNamesonObject.keys(walletContext.connectedWallets)instead, or at least document the assumption that approvals imply an address is available.If you confirm that
approvedChainIdsalways corresponds to chains with registered addresses in this route, current behavior is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/agentic-chat/src/hooks/useApprovedChains.ts(1 hunks)apps/agentic-chat/src/hooks/useWalletConnection.ts(2 hunks)apps/agentic-chat/src/providers/ChatProvider.tsx(3 hunks)apps/agentic-server/src/routes/chat.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/agentic-chat/src/providers/ChatProvider.tsx
- apps/agentic-chat/src/hooks/useWalletConnection.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
apps/agentic-chat/src/lib/wagmi-config.ts (1)
wagmiConfig(19-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (4)
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
34-57: Error handling and safe fallbacks look solidThe session lookup is defensive (
!wcConnection, missing namespaces) and any failures ingetProvider()or parsing just log a warning and return an empty list. That’s a safe default given the server treats an empty/absentapprovedChainIdsas “no filter.”apps/agentic-server/src/routes/chat.ts (3)
61-85: Wallet context filtering byapprovedChainIdsmatches the intended semanticsThe EVM/Solana registration logic looks good:
- For EVM, you register the same
evmAddresson all EVM chains, but constrain it toapprovedChainIdswhen provided and non‑empty; otherwise you fall back to all EVM chains.- For Solana, you only register when either there’s no filter or the filter explicitly includes
solanaChainId.This gives “capability gating” semantics without silently registering unsupported networks, and if
approvedChainIdsis malformed or doesn’t intersectallEvmChainIds, you simply end up with no EVM entries, which is a safe failure mode.No changes needed here from a correctness standpoint.
213-288: ThreadingapprovedChainIdsintobuildSystemPromptcleanly surfaces network capabilitiesInjecting the
buildConnectedWalletsPrompt(evmAddress, solanaAddress, approvedChainIds)into the system prompt is a nice way to give the model explicit context about which networks are usable, without altering the rest of the instructions.Assuming all call sites of
buildSystemPromptare updated to the new signature (it appears so here), this change looks good.
293-302: Client-suppliedapprovedChainIdsare treated as advisory filters—confirm no security reliance
approvedChainIdsare parsed directly from the request body and used to:
- Filter which chains are registered in
walletContext.- Inform the LLM via
buildSystemPrompt.Given that the client already supplies
evmAddress/solanaAddress, treatingapprovedChainIdsas advisory UX/prompt data (not as an authorization mechanism) is reasonable: a malicious client could lie about approvals but still can’t bypass wallet signatures.If you ever intend to use
approvedChainIdsas a security or authorization control (e.g., to restrict which chains server-side tools are allowed to touch), you’d want to derive them from a trusted source (e.g., server-side session or WalletConnect state) rather than trusting the request body.As it stands, for prompt shaping and default-chain selection, this is acceptable.
Also applies to: 309-310
d0bdf1e to
5d805fb
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: 0
🧹 Nitpick comments (2)
apps/agentic-server/src/routes/chat.ts (1)
213-288: Harden approvedChainIds parsing at the request boundaryRight now
approvedChainIdsis taken directly frombodyand assumed to bestring[]. Since this is user input, a malformed payload (e.g.approvedChainIds: "eip155:1"or a mixed-type array) would cause runtime issues when you call.length/.includesand would also bypass your intended filter semantics.You can defensively normalize to a
string[] | undefinedbefore passing intobuildWalletContext/buildSystemPrompt:- const { messages, evmAddress, solanaAddress, approvedChainIds } = body as { - messages: unknown - evmAddress?: string - solanaAddress?: string - approvedChainIds?: string[] - } - - // Build wallet context from addresses (filtered by approved chains if provided) - const walletContext = buildWalletContext(evmAddress, solanaAddress, approvedChainIds) + const { + messages, + evmAddress, + solanaAddress, + approvedChainIds: rawApprovedChainIds, + } = body as { + messages: unknown + evmAddress?: string + solanaAddress?: string + approvedChainIds?: unknown + } + + const approvedChainIds = + Array.isArray(rawApprovedChainIds) ? rawApprovedChainIds.filter((id): id is string => typeof id === 'string') : undefined + + // Build wallet context from addresses (filtered by approved chains if provided) + const walletContext = buildWalletContext(evmAddress, solanaAddress, approvedChainIds) @@ - system: buildSystemPrompt(evmAddress, solanaAddress, approvedChainIds), + system: buildSystemPrompt(evmAddress, solanaAddress, approvedChainIds),This keeps behavior identical for well-formed clients while avoiding crashes and silently discarding bad values.
Also applies to: 293-302, 309-309
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
32-63: Only enable the query for WalletConnect connectors (and trim unused sessionTopic)Two small refinements to tighten behavior:
- Avoid running the query for non-WalletConnect connectors
Today
enabledis justisConnected, so you'll still schedule the query when the user is connected via an injected/EVM-only connector;getApprovedChainsFromSessionthen just returns an empty list. You can cheaply skip this by checking the active connector id:export function useApprovedChains(): string[] { const { isConnected, connector } = useAccount() const { data } = useQuery({ queryKey: ['approvedChains', connector?.id], queryFn: getApprovedChainsFromSession, - enabled: isConnected, + enabled: isConnected && connector?.id === 'walletConnect', staleTime: Infinity, gcTime: 0, }) return data?.chains ?? [] }
- sessionTopic field is currently unused
ApprovedChainsResult.sessionTopicis never read, so you can either drop it from the result type and return value, or (if you plan to invalidate on session changes later) incorporate it into your query key once you have a way to surface it into the hook’s dependencies.Also applies to: 65-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/agentic-chat/src/hooks/useApprovedChains.ts(1 hunks)apps/agentic-chat/src/hooks/useWalletConnection.ts(2 hunks)apps/agentic-chat/src/providers/ChatProvider.tsx(3 hunks)apps/agentic-server/src/routes/chat.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/agentic-chat/src/hooks/useWalletConnection.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
apps/agentic-chat/src/lib/wagmi-config.ts (1)
wagmiConfig(19-19)
apps/agentic-chat/src/providers/ChatProvider.tsx (1)
apps/agentic-chat/src/hooks/useWalletConnection.ts (1)
useWalletConnection(6-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (4)
apps/agentic-server/src/routes/chat.ts (2)
36-48: Network lists and connected-networks prompt look coherent and UX-friendlyUsing
allEvmChainIds/allSupportedChainIdspluschainIdToNameto drive both wallet context and the "Connected / Not Connected" sections is clean and keeps supported networks centralized. The filtering onapprovedChainIdswith name lookup and sorted output gives the model clear, human-readable state without over-exposing CAIP details. This wiring looks solid as-is.Also applies to: 166-176, 178-211
61-88: Wallet context filtering behavior matches WalletConnect vs injected expectationsThe way
buildWalletContextonly filters EVM chains whenapprovedChainIds.length > 0, while treating the empty/undefined case as "all EVM chains" (for injected wallets), and gating Solana only when a non-empty filter is present, lines up well with the front-end behavior. Combined withbuildConnectedWalletsPromptonly rendering network sections when approvals are present, this gives a good separation between WalletConnect-scoped and generic connections.Also applies to: 178-211
apps/agentic-chat/src/providers/ChatProvider.tsx (1)
45-47: Ref-based wiring of approvedChainIds into transport is soundThreading
approvedChainIdsthroughuseWalletConnection, storing it in a ref alongside the addresses, and reading it in theDefaultChatTransportbody()callback gives you a stable transport while still sending up-to-date wallet/chain info. This is a nice pattern for avoiding hook dependency churn on the transport instance.Also applies to: 59-69, 70-81
apps/agentic-chat/src/hooks/useApprovedChains.ts (1)
23-25: Session parsing and chain-ID extraction align with WalletConnect CAIP accounts
extractChainIdsFromAccountscorrectly normalizes WalletConnect account strings down to uniquenamespace:chainIdpairs, andgetApprovedChainsFromSessioncleanly aggregates EVM (eip155) and Solana namespaces with defensive fallbacks on missing session/namespaces. Surfacing this viauseQueryand returning a plainstring[]keeps the consumer API simple while hiding WC/Wagmi specifics.Also applies to: 32-63, 65-77
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.