feat: cosmos wcv2 wallet support#11909
feat: cosmos wcv2 wallet support#11909gomes-bot wants to merge 18 commits intoshapeshift:developfrom
Conversation
Add Cosmos chain support to WalletConnect v2 wallet integration. - Inject cosmos optionalNamespaces into WC session via signer monkey-patch - Implement cosmosGetAddress using CAIP-10 session accounts - Implement cosmosSignTx using OfflineAminoSigner backed by WC cosmos_signAmino - Add cosmos account paths, path description, and WalletInfo/Wallet interfaces Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@gomesalexandre review me senpai UwU 🥺✨ pls give cosmos the love it deserves 🌌💫 |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Cosmos support to WalletConnect V2: new Cosmos harness, address extraction from CAIP‑10 sessions, OfflineAmino signing flow, Cosmos account path utilities, and multi‑namespace integration in the WalletConnectV2 wallet implementation. UI checks updated to exclude WalletConnect V2 where appropriate. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI
participant WC as WalletConnect<br/>Provider
participant Signer as OfflineAmino<br/>Signer
participant TxBuilder as ProtoTx<br/>Builder
participant Cosmos as Cosmos<br/>Network
User->>UI: Initiate Cosmos tx
UI->>WC: Read CAIP-10 account from session
WC-->>UI: Cosmos address & pubkey
UI->>Signer: Construct OfflineAminoSigner (addr + pubkey)
UI->>WC: Request `cosmos_signAmino`
WC-->>UI: Amino signature
UI->>TxBuilder: Build SignerData & assemble signed tx
UI->>Cosmos: Broadcast signed tx
Cosmos-->>UI: Tx receipt
UI-->>User: Confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- re-export cosmosDescribePath from core instead of trivial wrapper - remove unused params from standalone helper functions - remove unused CosmosGetAddress import Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move EVM from required to optional namespaces (fixes "params.requiredNamespaces.eip155 is not allowed" error) - Add patchProviderConnectForMultiNamespace to handle modal subscribeState abort and enable() eth_requestAccounts failure for cosmos-only wallets - Make _supportsETH a dynamic getter checking session namespaces - Add getDeviceID fallback chain (ETH -> cosmos accounts -> unknown) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Bypass offline-signing guard for WalletConnectV2 in send/staking flows - Code simplifier cleanup: extract mergeUnique helper, WcCosmosAccount type, simplify CAIP-10 parsing and address caching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
packages/hdwallet-walletconnectv2/src/walletconnectV2.ts (3)
240-270:patchProviderConnectForMultiNamespacesilently swallowsenable()errors when a session exists.Line 266: if
provider.signer?.sessionis truthy, any error fromenable()(including network errors, user rejections, or auth failures) is swallowed and an empty array is returned. This is intentional for the Cosmos-only case whereeth_requestAccountsfails, but it could mask legitimate EVM connection failures for wallets that do support EVM. Consider narrowing the catch to the specific error type/message frometh_requestAccountsif feasible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts` around lines 240 - 270, The catch in patchProviderConnectForMultiNamespace around provider.enable() is too broad and returns [] for any error when provider.signer?.session exists; narrow it to only swallow the specific eth_requestAccounts/user-rejection error for Cosmos-only flows by inspecting the thrown error in the catch of provider.enable() (e.g., check error.name/message/code or a provider-specific marker indicating eth_requestAccounts rejection) and return [] only for that case, otherwise rethrow the error; update the provider.enable override in patchProviderConnectForMultiNamespace to perform this conditional check so legitimate EVM/network/auth errors are not silently suppressed.
480-488:getDeviceID()may return different IDs across sessions for the same wallet.If a wallet supports both ETH and Cosmos,
getDeviceIDreturns'wc:' + ethAddr. If it later reconnects as Cosmos-only (e.g., session namespace changes), it returns'wc:' + cosmosAddr. SincewalletIdis used for portfolio state filtering and persistence, a changing device ID for the same physical wallet could cause state mismatches. This may be acceptable for the current WC flow but worth keeping in mind.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts` around lines 480 - 488, getDeviceID() can flip between eth and cosmos addresses across sessions causing an unstable walletId; update getDeviceID to produce a deterministic, stable identifier by combining available addresses or using a persisted stable id: call ethGetAddress() and cosmosGetAddress() and if both exist return a composite string like 'wc:eth:{ethAddr}|cosmos:{cosmosAddr}', if only one exists return that same prefixed form, and optionally fall back to a stored persistent id (persisted key tied to this connection) to ensure stability across namespace changes; adjust getDeviceID, ethGetAddress, and cosmosGetAddress usage accordingly so walletId remains consistent.
522-527:cosmosGetAddressis synchronous but wrapped inasync— cached addresses are never cleared on session change or disconnect.Both
this.ethAddress(line 424) andthis.cosmosAddress(line 525) persist after being set, even if the WalletConnect session is replaced or the wallet reconnects with a different account. Thedisconnect()method only callsprovider.disconnect()without clearing these cached values. Consider clearing both cached addresses indisconnect()orclearSession()to prevent stale address usage after reconnection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts` around lines 522 - 527, The cosmosGetAddress method is an unnecessary async wrapper around the synchronous cosmosGetAddress() helper and cached addresses (this.cosmosAddress and this.ethAddress) are never cleared on session/disconnect; change cosmosGetAddress to a synchronous method that returns string | null (remove async/Promise) and keep using the helper cosmosGetAddress(this.provider) to populate this.cosmosAddress, and update disconnect() and/or clearSession() to null out both this.cosmosAddress and this.ethAddress (in addition to calling provider.disconnect()) so stale addresses cannot persist across session replacement or reconnection.packages/hdwallet-walletconnectv2/src/cosmos.ts (2)
65-66:algocast to literal'secp256k1'is unchecked.The
algovalue coming from the WC wallet is cast to'secp256k1'on line 70 without validation. If a wallet returns'ed25519'or another algorithm, the signing flow will silently proceed with an incorrect key type, producing an invalid signature. A runtime guard would prevent hard-to-debug signing failures.🛡️ Add a guard
const { pubkey: pubkeyBase64, algo } = wcAccounts[0] + if (algo !== 'secp256k1') return null const pubkey = new Uint8Array(Buffer.from(pubkeyBase64, 'base64'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hdwallet-walletconnectv2/src/cosmos.ts` around lines 65 - 66, Validate the WC account's algorithm before treating it as secp256k1: check wcAccounts[0].algo (the variable algo) and if it is not exactly 'secp256k1' throw or return a clear error (or handle other algorithms explicitly) instead of casting; ensure this guard is placed before creating pubkey/new Uint8Array(Buffer.from(...)) and before any signing logic that assumes secp256k1 so the code using algo and pubkey fails fast for unsupported algorithms.
48-89: Inconsistent error handling:nullreturn vsthrowin the same flow.
cosmosSignTxreturnsnullon line 53 when the signer address is missing from the session, but throws on line 63 whencosmos_getAccountsreturns no accounts. Both represent "wallet can't sign Cosmos" — the caller must handle both anullreturn and catch an exception. Consider returningnullconsistently or throwing consistently.♻️ Consistent null-return approach
- if (!wcAccounts?.length) throw new Error('No cosmos accounts from WalletConnect') + if (!wcAccounts?.length) return null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hdwallet-walletconnectv2/src/cosmos.ts` around lines 48 - 89, cosmosSignTx mixes return-null (when getCosmosAddressFromSession() yields no address) and throwing an exception when provider.signer.request(...) for 'cosmos_getAccounts' returns no accounts; make this consistent by returning null instead of throwing: replace the Error throw after checking wcAccounts?.length with a null return (optionally logging the missing accounts) so callers only need to handle a null result from cosmosSignTx; keep all other flow (pubkey parsing, offlineSigner, sign(...)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/hdwallet-walletconnectv2/src/cosmos.ts`:
- Around line 65-66: Validate the WC account's algorithm before treating it as
secp256k1: check wcAccounts[0].algo (the variable algo) and if it is not exactly
'secp256k1' throw or return a clear error (or handle other algorithms
explicitly) instead of casting; ensure this guard is placed before creating
pubkey/new Uint8Array(Buffer.from(...)) and before any signing logic that
assumes secp256k1 so the code using algo and pubkey fails fast for unsupported
algorithms.
- Around line 48-89: cosmosSignTx mixes return-null (when
getCosmosAddressFromSession() yields no address) and throwing an exception when
provider.signer.request(...) for 'cosmos_getAccounts' returns no accounts; make
this consistent by returning null instead of throwing: replace the Error throw
after checking wcAccounts?.length with a null return (optionally logging the
missing accounts) so callers only need to handle a null result from
cosmosSignTx; keep all other flow (pubkey parsing, offlineSigner, sign(...))
unchanged.
In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts`:
- Around line 240-270: The catch in patchProviderConnectForMultiNamespace around
provider.enable() is too broad and returns [] for any error when
provider.signer?.session exists; narrow it to only swallow the specific
eth_requestAccounts/user-rejection error for Cosmos-only flows by inspecting the
thrown error in the catch of provider.enable() (e.g., check
error.name/message/code or a provider-specific marker indicating
eth_requestAccounts rejection) and return [] only for that case, otherwise
rethrow the error; update the provider.enable override in
patchProviderConnectForMultiNamespace to perform this conditional check so
legitimate EVM/network/auth errors are not silently suppressed.
- Around line 480-488: getDeviceID() can flip between eth and cosmos addresses
across sessions causing an unstable walletId; update getDeviceID to produce a
deterministic, stable identifier by combining available addresses or using a
persisted stable id: call ethGetAddress() and cosmosGetAddress() and if both
exist return a composite string like 'wc:eth:{ethAddr}|cosmos:{cosmosAddr}', if
only one exists return that same prefixed form, and optionally fall back to a
stored persistent id (persisted key tied to this connection) to ensure stability
across namespace changes; adjust getDeviceID, ethGetAddress, and
cosmosGetAddress usage accordingly so walletId remains consistent.
- Around line 522-527: The cosmosGetAddress method is an unnecessary async
wrapper around the synchronous cosmosGetAddress() helper and cached addresses
(this.cosmosAddress and this.ethAddress) are never cleared on
session/disconnect; change cosmosGetAddress to a synchronous method that returns
string | null (remove async/Promise) and keep using the helper
cosmosGetAddress(this.provider) to populate this.cosmosAddress, and update
disconnect() and/or clearSession() to null out both this.cosmosAddress and
this.ethAddress (in addition to calling provider.disconnect()) so stale
addresses cannot persist across session replacement or reconnection.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/hdwallet-walletconnectv2/src/walletconnectV2.ts (1)
262-265: Avoidanyin the provider patch boundaryLine 262 and Line 265 introduce
anyin critical connection code paths. Preferunknown+ narrow typed shape for patched provider methods.As per coding guidelines
**/*.{ts,tsx}: "NEVER useanytype unless absolutely necessary in TypeScript."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts` around lines 262 - 265, The patched provider currently uses the unsafe any type (const provider = this.provider as any) and should be changed to use unknown with a narrow interface: declare a local type (e.g., PatchableProvider) describing the connect method signature and bindable methods, cast this.provider to unknown then perform a runtime type-check (type guard) to ensure it matches PatchableProvider before assigning originalConnect = provider.connect.bind(provider) and overriding provider.connect; update references to originalConnect and provider.connect accordingly so the compiler knows the exact method shape and you avoid any usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts`:
- Around line 242-256: The current rewrite clears caller-provided namespaces by
passing namespaces: {} into originalConnect; preserve any non-eip155 namespaces
by merging instead of replacing. In the return from the connect wrapper, merge
params.namespaces and params.optionalNamespaces with the constructed eip155
payloads (e.g. compute mergedNamespaces = { ...(params.namespaces || {}),
eip155: /* requiredEvm merged into required namespace */ } and
mergedOptionalNamespaces = { ...(params.optionalNamespaces || {}), eip155: {
chains: mergeUnique(...), methods: mergeUnique(...), events: mergeUnique(...),
rpcMap: { ...(requiredEvm as any)?.rpcMap, ...(optionalEvm as any)?.rpcMap } },
cosmos: COSMOS_OPTIONAL_NAMESPACE }, then call originalConnect({ ...params,
namespaces: mergedNamespaces, optionalNamespaces: mergedOptionalNamespaces }).
This preserves caller non-EVM namespaces while applying the eip155 and cosmos
merges.
- Around line 269-271: The stub for modal.subscribeState is wrong: replace the
current no-arg noop with a function that accepts the callback parameter and
returns an unsubscribe function so callers won't get undefined; specifically
update modal.subscribeState to a signature that takes callback (e.g., callback:
any or (newState: PublicStateControllerState) => void) and returns a () => void
unsubscribe closure so originalConnect() or other callers can invoke it and
receive a valid unsubscribe function.
---
Nitpick comments:
In `@packages/hdwallet-walletconnectv2/src/walletconnectV2.ts`:
- Around line 262-265: The patched provider currently uses the unsafe any type
(const provider = this.provider as any) and should be changed to use unknown
with a narrow interface: declare a local type (e.g., PatchableProvider)
describing the connect method signature and bindable methods, cast this.provider
to unknown then perform a runtime type-check (type guard) to ensure it matches
PatchableProvider before assigning originalConnect =
provider.connect.bind(provider) and overriding provider.connect; update
references to originalConnect and provider.connect accordingly so the compiler
knows the exact method shape and you avoid any usage.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/hdwallet-walletconnectv2/src/walletconnectV2.ts
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
EVM and Cosmos is working through WC on Keplr mobile, tested account exposure, send/sign txs
- Spread other required/optional namespaces so non-eip155 namespaces (e.g. solana) aren't silently dropped during connect - Fix modal.subscribeState stub to accept callback param and return unsubscribe function matching Web3Modal's expected signature
Head branch was pushed to by a user without write access
…allet # Conflicts: # packages/hdwallet-walletconnectv2/package.json # packages/hdwallet-walletconnectv2/src/index.ts # packages/hdwallet-walletconnectv2/src/walletconnectV2.ts
3 participants
Description
Adds Cosmos chain support to the WalletConnect v2 wallet integration. When a user connects via WC, the session proposal now includes cosmos as an optional namespace, so wallets that support Cosmos (e.g. Keplr, Leap) will expose their cosmos accounts.
The approach is surgical - we keep EthereumProvider as the main provider and monkey-patch its internal
signer.connect()to inject cosmosoptionalNamespaces. This means zero changes to the web app layer (adapter, config, WalletProvider, etc.) - all the magic happens in hdwallet-walletconnectv2.What's new:
cosmos.ts- cosmos-specific WC methods (getAddress from CAIP-10 session, signTx via OfflineAminoSigner backed bycosmos_signAmino)walletconnectV2.ts- implements CosmosWallet/CosmosWalletInfo interfaces,_supportsCosmosdynamic getter based on session namespacespatchSignerForNonEvmNamespaces()- moves all EVM namespaces from required to optional (CAIP-25 compliant), adds cosmos as optional namespacepatchProviderConnectForMultiNamespace()- patches provider.connect/enable to handle modal abort and eth_requestAccounts failures for non-EVM wallets_supportsETHis now a dynamic getter checking session namespaces (was hardcodedtrue)getDeviceID()has a fallback chain: ETH -> Cosmos -> 'wc:unknown'Issue (if applicable)
Risk
Low - additive only. EVM WC flow is unchanged in behavior:
patchProviderConnectForMultiNamespaceonly catches errors for non-EVM sessions, EVM enable() succeeds as before_supportsETHdynamic getter returnstrueafter session establishment (same timing as all other wallet checks)Testing
Engineering
Cosmos WC wallet (new):
cosmos_signAminoEVM WC wallet (regression):
Operations
WC cosmos wallet connection + cosmos sends via WC-connected wallets.
Screenshots
https://jam.dev/c/0fb9a376-b3a3-4ba3-9120-a4539ee16647
Summary by CodeRabbit
New Features
Bug Fixes