-
Notifications
You must be signed in to change notification settings - Fork 51
feat: new wallet sample #306
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
* feat(wallet-pay): implement EVM wallet with secure key management - Add viem-based EVM wallet implementation (lib/chains/evm/) - Add secure mnemonic storage using expo-secure-store - Add wallet store with Zustand for state management - Add crypto polyfills for react-native-quick-crypto - Add bip39 patch for native PBKDF2 performance - Update WalletKit integration to use wallet addresses - Configure babel module resolver for crypto aliases Security: Mnemonic is not stored in wallet class memory, only in secure storage. HDAccount with private key is kept for signing operations. Architecture follows plugin-based design for future multi-chain support (Solana, Sui, TON, Tron). * fix(wallet-pay): address PR review feedback Security fixes: - Disable eth_sign method (auto-reject) - phishing attack vector - Add mnemonic validation before wallet restoration - Add WalletConnect URI validation in QR scanner - Add try-catch for JSON.parse in typed data parsing Reliability fixes: - Add initialization lock to prevent WalletKit race conditions - Add error state to wallet initialization hook Code quality: - Wrap console.logs with __DEV__ check for production - Add TODO comments for toast notifications * docs(wallet-pay): update README with security documentation - Rename to Expo Wallet - Add architecture overview - Document security considerations and hot wallet trade-offs - List protected data and storage mechanisms - Note eth_sign is disabled for security * chore: address feedback
AGENTS.md: - Detailed project overview and tech stack - Architecture diagrams (mermaid) for init flow, plugin system, data flow - Complete project structure documentation - Coding conventions and patterns - Security notes and common tasks README.md: - Add badges for Expo, React Native, TypeScript, WalletConnect - Add architecture mermaid diagram - Improve structure with emojis and sections - Add roadmap section - Link to AGENTS.md for detailed docs
…ons (#305) * feat(wallet-pay): add session request modal for signing and transactions - Add session-request.tsx modal screen for WalletConnect session_request events - Create modal-content/ folder with modular content components: - SignMessageContent for personal_sign/eth_signTypedData requests - SendTransactionContent for eth_sendTransaction requests with accordion - Add VerifyBadge component for WalletConnect Verify API status display - Register session_request listener in _layout.tsx - Add verify badge to session-proposal.tsx connect modal - Fix hex message decoding in evm-wallet.ts for personal_sign - Add helper functions: getRequestIntention, getSignParamsMessage, formatEthValue, getNativeTokenSymbol, truncateAddress - Add caret-up-down.png icon for accordion toggle * chore: address review comments
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @ignaciosantise's task in 1m 54s —— View job Code Review CompleteReviewed PR #306: feat: new wallet sample - Comprehensive WalletConnect wallet implementation with secure key management. Found 5 issue(s)Issue 1: Console.log in production code without DEV guardID: session-propo-console-log-a8f2 Line 47 contains an unguarded console.log that will execute in production builds: console.log('session', session);Recommendation: Wrap in if (__DEV__) {
console.log('session', session);
}Issue 2: Missing error handling for WalletKit operationsID: session-reque-error-handling-c3d1 The Recommendation: Display error message to user or implement toast notifications as indicated by TODOs in the codebase. Issue 3: Race condition in WalletKit initialization checkID: walletkit-race-condition-b5e7 The race condition protection uses Recommendation: Use a Promise-based singleton pattern or atomic flag: let initPromise: Promise<IWalletKit> | null = null;
async function init() {
if (!walletReady || !evmAddress) return;
if (walletKit) return;
if (initPromise) {
await initPromise;
return;
}
initPromise = WalletKit.init({...});
try {
walletKit = await initPromise;
stateListeners.forEach((listener) => listener(true));
} finally {
initPromise = null;
}
}Issue 4: Potential buffer overflow in hexToUtf8 conversionID: helpers-hex-conversion-d9a4 The Recommendation: Add length validation: function hexToUtf8(hex: string): string {
try {
const cleanHex = hex.startsWith('0x') ? hex.slice(2) : hex;
if (cleanHex.length % 2 !== 0) return hex;
if (cleanHex.length > 10000) return hex; // Max ~5KB string
// ... rest of conversion
} catch {
return hex;
}
}Issue 5: eth_sendTransaction not fully implementedID: evm-handler-send-tx-incomplete-f1b3 The Recommendation: Either implement full broadcast functionality with RPC provider integration, or return a more appropriate error indicating the method is not yet supported: case EIP155_SIGNING_METHODS.ETH_SEND_TRANSACTION: {
// eth_sendTransaction requires broadcasting, which needs RPC provider setup
return formatJsonRpcError(
id,
'eth_sendTransaction not yet supported. Use eth_signTransaction instead.'
);
}Automated Checks✅ No external domain URLs detected outside approved domains (reown.com, walletconnect.com, walletconnect.org) SummaryThis is a well-structured wallet implementation with good security practices. The main issues are:
The HIGH severity issue with |
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.
Pull request overview
This PR introduces Expo Wallet, a complete sample WalletConnect wallet implementation built with Expo SDK 54 and React Native 0.81. The implementation demonstrates secure key management using platform-native secure storage (iOS Keychain/Android Keystore), WalletKit integration for dApp connections, and a plugin-based multi-chain architecture currently supporting EVM chains with extensibility for future blockchain additions.
Changes:
- Complete Expo wallet application structure with secure mnemonic storage, WalletKit integration, and QR scanning capabilities
- EVM wallet implementation using viem with signing request handlers for personal_sign, eth_signTypedData, and transaction signing
- Plugin-based multi-chain architecture with IWallet interface for future blockchain support (Solana, Sui, TON, Tron)
Reviewed changes
Copilot reviewed 52 out of 91 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
utils/*.ts |
Storage utilities, polyfills, mnemonic generation, and helper functions |
stores/*.ts |
Zustand state management for wallet and settings |
lib/**/*.ts |
Core wallet logic including IWallet interface and EVM implementation |
hooks/*.ts |
React hooks for wallet initialization, WalletKit integration, and theming |
components/**/*.tsx |
UI components including modals, buttons, text primitives, and scanner frame |
app/**/*.tsx |
Expo Router pages for tabs, scanner, and session handling |
constants/*.ts |
Theme configuration, EVM chains, signing methods, and spacing |
| Configuration files | Package.json, babel, eslint, tsconfig, and app.json setup |
| Documentation | README.md and AGENTS.md providing comprehensive project documentation |
Comments suppressed due to low confidence (2)
wallets/wallet-pay/constants/theme.ts:1
- The TODO comment suggests uncertainty about the Fonts configuration. This should be verified and either confirmed as correct or fixed, then the TODO removed. The font configurations appear complete for different platforms (iOS, web, default), so if they're working correctly, this comment should be removed.
wallets/wallet-pay/components/primitives/text.tsx:1 - The hardcoded font family 'KH Teka' in the styles should reference the font configuration from
constants/theme.ts(theFontsconstant) for consistency and maintainability. This ensures font changes are managed in a single location.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This PR introduces Expo Wallet — a complete sample WalletConnect wallet built with Expo SDK 54 and React Native 0.81. It demonstrates secure key management, WalletKit integration, and a plugin-based multi-chain architecture.
Features
expo-secure-storereact-native-quick-cryptoArchitecture
graph TB subgraph "App Entry" A[_layout.tsx] --> B[Polyfills] B --> C[useWalletInitialization] end subgraph "Secure Storage" D[(expo-secure-store<br/>Mnemonic)] end subgraph "Wallet Layer" E[EvmWallet<br/>viem HDAccount] F[WalletStore<br/>Zustand] end subgraph "WalletConnect" G[WalletKit] H[Request Handlers] end C --> D D --> E E --> F F --> G G --> H H --> EPlugin Architecture (Multi-Chain)
graph TB subgraph "Wallet Store" Store[useWalletStore] end subgraph "Base Interface" IWallet[IWallet Interface] end subgraph "Chain Implementations" EVM[EvmWallet ✅] SOL[SolanaWallet 🔜] SUI[SuiWallet 🔜] TON[TonWallet 🔜] end IWallet --> EVM IWallet -.-> SOL IWallet -.-> SUI IWallet -.-> TON EVM --> StoreData Flow
flowchart LR subgraph "Secure Storage" Mnemonic[(Mnemonic<br/>encrypted)] end subgraph "Memory Only" HDAccount[HDAccount<br/>private key] end subgraph "Public State" Address[Wallet Address] Sessions[WC Sessions] end Mnemonic -->|derive on init| HDAccount HDAccount -->|extract| Address Address --> SessionsProject Structure
Tech Stack
Security
expo-secure-storeSecurity Measures
eth_signdisabled (phishing attack vector)__DEV__checksChanges
Added
app/,lib/,stores/,hooks/,utils/,components/,constants/)lib/chains/evm/)Stats
Testing
npm installDocumentation
Screenshots
Add screenshots of the wallet UI here
Related