feat: WalletConnect support via Reown AppKit + wagmi#16
Merged
Conversation
The previous Connect Wallet flow used raw EIP-6963 discovery, which only sees browser-extension wallets (MetaMask, Rabby). Mobile wallets like MetaMask Mobile, Trust, Rainbow, Coinbase Wallet were invisible to the tool — blocking signers whose authorised keys live only on a phone. Adopt the same wallet stack tradingstrategy-ai/frontend already uses (@reown/appkit + @reown/appkit-adapter-wagmi + @wagmi/core + @wagmi/connectors) on top of the existing viem dependency. AppKit's modal handles browser extensions and WalletConnect-bridged mobile wallets in one UI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Why: prevent accidental secret commits from a single, well-known scanner (gitleaks). Two layers — server-side via GitHub Action so nothing slips past `git commit --no-verify`, and local via husky pre-commit hook so devs get fast feedback before pushing. Lessons learnt: husky's pre-commit hook gracefully no-ops with a warning if gitleaks isn't installed locally; CI is still the source of truth. The .gitleaks.toml allowlist covers .env.example, CHANGELOG.md, and pnpm-lock.yaml to avoid false positives. Summary: - Add .github/workflows/secret-scan.yml running gitleaks/gitleaks-action@v2 on every PR and push to main. - Add .gitleaks.toml extending default rules with project-specific allowlist paths. - Add husky as a devDependency, .husky/pre-commit running `gitleaks protect --staged`, and update prepare script. - Document setup in README and CHANGELOG. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gitleaks/gitleaks-action@v2 requires a paid licence for repos owned by organisations and fails with "missing gitleaks license" otherwise. The gitleaks binary itself is MIT-licensed and free — install it from the upstream release in the workflow and call it directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The current Connect Wallet flow uses raw EIP-6963 discovery, which only sees browser-extension wallets (MetaMask, Rabby). Mobile wallets like MetaMask Mobile, Trust Wallet, Rainbow, Coinbase Wallet were invisible to the tool — blocking signers whose authorised keys live only on a phone.
This came up concretely while approving an agent on a production multi-sig: a co-signer's authorised wallet sits on a mobile app, and the only fallback was "export your seed phrase to a desktop wallet" — insecure and high-friction.
Lessons learnt
+ 'WALLET_CONNECT_PROJECT_ID' +repo secret.+ 'getWallet()' +interface byte-identical means the migration is contained — no diff in+ 'signing.ts' +,+ 'l1signing.ts' +,+ 'execute.ts' +,+ 'WrongChainAlert.svelte' +,+ 'SessionShare.svelte' +,+ 'EIP712Preview.svelte' +, or routes.Summary
+ '@reown/appkit' +,+ '@reown/appkit-adapter-wagmi' +,+ '@wagmi/core' +,+ '@wagmi/connectors' +to+ 'package.json' +(versions pinned to match+ 'tradingstrategy-ai/frontend' +).+ 'src/lib/wallet/client.ts' +: AppKit setup with chains+ '[arbitrumSepolia, arbitrum]' +(Sepolia is mandatory because Hyperliquid hardcodes its chain ID 421614 in the EIP-712 signing domain).+ 'src/lib/wallet.svelte.ts' +on top of wagmi's+ 'getAccount' +/+ 'watchAccount' +/+ 'reconnect' +/+ 'switchChain' +actions. Public+ 'getWallet()' +interface unchanged (same getters, same methods); the+ 'discoveredWallets' +/+ 'showPicker' +/+ 'connectProvider' +members tied to the bespoke EIP-6963 picker are dropped because AppKit owns the modal now.+ 'src/lib/components/ConnectWallet.svelte' +becomes a thin Connect/Disconnect button — picker UI lives in AppKit.+ 'src/lib/config.ts' +reads+ 'TS_PUBLIC_WALLET_CONNECT_PROJECT_ID' +from+ '$env/dynamic/public' +and warns if unset.+ 'svelte.config.js' +sets+ 'kit.env.publicPrefix: "TS_PUBLIC_"' +per project convention (CLAUDE.md).+ '.env.example' +documents the new env var;+ '.gitignore' +already covers+ '.env' +.+ '.github/workflows/deploy.yml' +wires the+ 'WALLET_CONNECT_PROJECT_ID' +repo secret into the build step.+ 'README.md' +updated with one-line setup instruction.+ 'CHANGELOG.md' +created with the entry per project convention.Notes for the reviewer
+ 'WALLET_CONNECT_PROJECT_ID' +repo secret has already been set on this repo (verified with+ 'gh secret list' +).+ 'pnpm install' +locally before merging to refresh+ 'pnpm-lock.yaml' +, then+ 'pnpm run check' +and+ 'pnpm run format' +.+ 'approveAgent' +session URL.