feat: integrate WalletConnect Pay SDK with payment modal #310
Conversation
- Add PayStore for Pay SDK state management and initialization - Add PaymentOptionsModal with collect data form, payment options selection, and signing flow - Add useInitializePaySDK hook for SDK initialization - Add wallet import functionality (mnemonic/private key) in Settings - Improve keyboard handling in payment modal forms - Format currency amounts with 2 decimal places minimum - Add ENV_PAY_API_KEY environment variable support
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @ganchoradkov's task in 2m 3s —— View job Code Review CompleteFound multiple security and code quality issues in the WalletConnect Pay integration. Found 7 issue(s)Issue 1: Insecure storage of private keys and mnemonicsID: importwallet-insecure-storage-a7b3 The
Recommendation:
// In ImportWalletModal.tsx line 75:
secureTextEntry={true}
// Consider adding secure storage wrapper:
import * as Keychain from 'react-native-keychain';
// Then replace storage.setItem with Keychain.setGenericPasswordIssue 2: Missing input validation in wallet importID: eip155wallet-input-validation-c9e1 The
This can lead to cryptic errors from ethers.js instead of user-friendly validation messages. Recommendation: Add comprehensive input validation: import { utils } from 'ethers';
export function loadEIP155Wallet(input: string): {
address: string;
wallet: EIP155Lib;
} {
const trimmedInput = input.trim();
// Validate private key
if (trimmedInput.startsWith('0x')) {
if (trimmedInput.length !== 66) {
throw new Error('Private key must be 66 characters (including 0x prefix)');
}
if (!/^0x[0-9a-fA-F]{64}$/.test(trimmedInput)) {
throw new Error('Private key must contain only hexadecimal characters');
}
} else {
// Validate mnemonic
const words = trimmedInput.split(/\s+/);
if (![12, 15, 18, 21, 24].includes(words.length)) {
throw new Error(`Mnemonic must be 12, 15, 18, 21, or 24 words (got ${words.length})`);
}
if (!utils.isValidMnemonic(trimmedInput)) {
throw new Error('Invalid mnemonic phrase');
}
}
// Rest of the function...
}Issue 3: Missing error handling for Pay SDK initializationID: paystore-init-error-d4f2 The Recommendation: Store initialization error state and expose it: let clientInstance: PayClientInstance | null = null;
let initializationError: Error | null = null;
const PayStore = {
initialize(options: PayClientOptions) {
if (!PayClient.isAvailable()) {
initializationError = new Error('Native provider not available');
console.warn('[PayStore] Native provider not available');
return;
}
try {
clientInstance = new PayClient(options);
initializationError = null;
console.log('[PayStore] PayClient initialized');
} catch (error) {
console.error('[PayStore] Failed to initialize PayClient:', error);
clientInstance = null;
initializationError = error instanceof Error ? error : new Error(String(error));
}
},
getInitializationError(): Error | null {
return initializationError;
},
// ... rest of methods
};Issue 4: Type coercion with
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| export default function Connections({route}: Props) { | ||
| // Check if a URI is a WalletConnect Pay payment link | ||
| function isPaymentLink(uri: string): boolean { | ||
| return uri.includes('pay.walletconnect.com') && uri.includes('pid='); |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 17 hours ago
In general, this should be fixed by parsing the URI and validating its host (and scheme) explicitly instead of using includes on the raw string. That avoids cases where pay.walletconnect.com appears in non-host parts of the URL or as part of a larger, attacker-controlled hostname. We should also avoid relying on substring checks for subdomain handling; instead, compare hosts against an explicit allowlist such as ['pay.walletconnect.com'].
The best targeted fix here is to replace isPaymentLink with a version that: (1) attempts to parse the URI using the standard URL class, (2) ensures that the scheme is one we expect (for a web link, typically https:), (3) ensures that the hostname is exactly pay.walletconnect.com (or matches a small, explicit list if needed), and (4) verifies that the pid parameter exists via the URL search parameters rather than as a raw substring. This keeps existing functionality (detecting WalletConnect Pay links containing a pid parameter) while making the detection correct and robust.
Concretely, in wallets/rn_cli_wallet/src/screens/Connections/index.tsx, we will rewrite isPaymentLink to use new URL(uri) with a try/catch to handle invalid URLs, check url.hostname === 'pay.walletconnect.com', and check url.searchParams.has('pid'). No new imports are needed, as URL is available in modern React Native / JS environments; if not, this remains a safe, minimal change within the provided snippet.
| @@ -13,7 +13,20 @@ | ||
|
|
||
| // Check if a URI is a WalletConnect Pay payment link | ||
| function isPaymentLink(uri: string): boolean { | ||
| return uri.includes('pay.walletconnect.com') && uri.includes('pid='); | ||
| try { | ||
| const url = new URL(uri); | ||
|
|
||
| // Ensure the link targets the official WalletConnect Pay host | ||
| if (url.hostname !== 'pay.walletconnect.com') { | ||
| return false; | ||
| } | ||
|
|
||
| // Ensure there is a "pid" query parameter | ||
| return url.searchParams.has('pid'); | ||
| } catch { | ||
| // If the URI cannot be parsed as a URL, it is not a valid payment link | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export default function Connections({ route }: Props) { |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| Enter a mnemonic phrase or private key to import an existing wallet. | ||
| </Text> | ||
|
|
||
| <TextInput |
There was a problem hiding this comment.
🤖 Auto Review Issue: Insecure storage of private keys and mnemonics
Severity: HIGH
Category: security
Tool: Claude Auto Review
Recommendation: - Set secureTextEntry={true} on the TextInput (line 75) to mask sensitive input
- Consider implementing secure storage using
react-native-keychainor platform-specific secure storage instead of AsyncStorage - Add a warning message informing users about security implications of importing private keys
// In ImportWalletModal.tsx line 75:
secureTextEntry={true}
// Consider adding secure storage wrapper:
import * as Keychain from 'react-native-keychain';
// Then replace storage.setItem with Keychain.setGenericPassword| address: string; | ||
| wallet: EIP155Lib; | ||
| } { | ||
| const trimmedInput = input.trim(); |
There was a problem hiding this comment.
🤖 Auto Review Issue: Missing input validation in wallet import
Severity: MEDIUM
Category: code_quality
Tool: Claude Auto Review
Recommendation: Add comprehensive input validation:
import { utils } from 'ethers';
export function loadEIP155Wallet(input: string): {
address: string;
wallet: EIP155Lib;
} {
const trimmedInput = input.trim();
// Validate private key
if (trimmedInput.startsWith('0x')) {
if (trimmedInput.length !== 66) {
throw new Error('Private key must be 66 characters (including 0x prefix)');
}
if (!/^0x[0-9a-fA-F]{64}$/.test(trimmedInput)) {
throw new Error('Private key must contain only hexadecimal characters');
}
} else {
// Validate mnemonic
const words = trimmedInput.split(/\s+/);
if (![12, 15, 18, 21, 24].includes(words.length)) {
throw new Error(`Mnemonic must be 12, 15, 18, 21, or 24 words (got ${words.length})`);
}
if (!utils.isValidMnemonic(trimmedInput)) {
throw new Error('Invalid mnemonic phrase');
}
}
// Rest of the function...
}| let clientInstance: PayClientInstance | null = null; | ||
|
|
||
| const PayStore = { | ||
| initialize(options: PayClientOptions) { |
There was a problem hiding this comment.
🤖 Auto Review Issue: Missing error handling for Pay SDK initialization
Severity: MEDIUM
Category: code_quality
Tool: Claude Auto Review
Recommendation: Store initialization error state and expose it:
let clientInstance: PayClientInstance | null = null;
let initializationError: Error | null = null;
const PayStore = {
initialize(options: PayClientOptions) {
if (!PayClient.isAvailable()) {
initializationError = new Error('Native provider not available');
console.warn('[PayStore] Native provider not available');
return;
}
try {
clientInstance = new PayClient(options);
initializationError = null;
console.log('[PayStore] PayClient initialized');
} catch (error) {
console.error('[PayStore] Failed to initialize PayClient:', error);
clientInstance = null;
initializationError = error instanceof Error ? error : new Error(String(error));
}
},
getInitializationError(): Error | null {
return initializationError;
},
// ... rest of methods
};| const [selectedOption, setSelectedOption] = useState<PaymentOption | null>( | ||
| null, | ||
| ); | ||
| const [paymentActions, setPaymentActions] = useState<any[] | null>(null); |
There was a problem hiding this comment.
🤖 Auto Review Issue: Type coercion with any weakens type safety
Severity: LOW
Category: code_quality
Tool: Claude Auto Review
Recommendation: Define proper types for payment actions:
import type { PaymentAction } from '@walletconnect/pay';
// Replace line 104:
const [paymentActions, setPaymentActions] = useState<PaymentAction[] | null>(null);
// For error handling, use:
} catch (error: unknown) {
const message = error instanceof Error ? error.message : 'Unknown error';
console.error('[Pay] Error:', error);
}| try { | ||
| // Get wallet accounts - Base only for testing | ||
| const eip155Address = SettingsStore.state.eip155Address; | ||
| const accounts = eip155Address ? [`eip155:8453:${eip155Address}`] : []; |
There was a problem hiding this comment.
🤖 Auto Review Issue: Hardcoded chain ID for payment accounts
Severity: MEDIUM
Category: code_quality
Tool: Claude Auto Review
Recommendation: Either make chain ID configurable or dynamically determine supported chains from payment options:
// Option 1: Make configurable
const SUPPORTED_PAYMENT_CHAINS = [1, 8453, 137]; // Ethereum, Base, Polygon
const accounts = eip155Address
? SUPPORTED_PAYMENT_CHAINS.map(chainId => `eip155:${chainId}:${eip155Address}`)
: [];
// Option 2: Get from payment options if available in the future
// const chains = paymentOptions?.supportedChains || [8453];| } | ||
|
|
||
| // Validate date format (YYYY-MM-DD) and check if it's a valid past date | ||
| function isValidDateOfBirth(dateStr: string): boolean { |
There was a problem hiding this comment.
🤖 Auto Review Issue: Date validation doesn't check for future dates in form
Severity: LOW
Category: code_quality
Tool: Claude Auto Review
Recommendation: Consider adding real-time validation feedback in the updateCollectedField callback:
const updateCollectedField = useCallback(
(fieldId: string, value: string, fieldType?: string) => {
const formattedValue = fieldType === 'date' ? formatDateInput(value) : value;
setCollectedData(prev => ({ ...prev, [fieldId]: formattedValue }));
// Clear error and provide immediate feedback for complete dates
if (fieldType === 'date' && formattedValue.length === 10) {
if (!isValidDateOfBirth(formattedValue)) {
setCollectDataError('Date must be in the past and use format YYYY-MM-DD');
} else {
setCollectDataError(null);
}
} else {
setCollectDataError(null);
}
},
[],
);| console.log('[Pay] Fetching payment options for:', paymentLink); | ||
| console.log('[Pay] Accounts:', accounts); | ||
|
|
||
| const paymentOptions = await payClient.getPaymentOptions({ |
There was a problem hiding this comment.
🤖 Auto Review Issue: Missing timeout and retry logic for external API calls
Severity: MEDIUM
Category: code_quality
Tool: Claude Auto Review
Recommendation: Add timeout and retry with exponential backoff:
async function withTimeout<T>(
promise: Promise<T>,
timeoutMs: number,
errorMessage: string
): Promise<T> {
return Promise.race([
promise,
new Promise<T>((_, reject) =>
setTimeout(() => reject(new Error(errorMessage)), timeoutMs)
),
]);
}
// In handlePaymentLink:
const paymentOptions = await withTimeout(
payClient.getPaymentOptions({
paymentLink,
accounts,
includePaymentInfo: true,
}),
15000, // 15 second timeout
'Payment options request timed out. Please try again.'
);
There was a problem hiding this comment.
Pull request overview
This PR integrates the WalletConnect Pay SDK to enable crypto payment functionality within the React Native wallet app. It adds comprehensive payment flow support including data collection, payment method selection, and transaction signing, along with wallet import capabilities for custom mnemonic phrases and private keys.
Changes:
- Integration of WalletConnect Pay SDK v0.0.0-canary.1 with new PayStore and initialization hook
- New PaymentOptionsModal supporting full payment workflow (data collection, option selection, signing)
- Wallet import modal allowing users to load existing wallets via mnemonic or private key
- Enhanced EIP155Lib to support private key initialization alongside mnemonic-based wallets
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock, package.json |
Added @walletconnect/pay and updated react-native-compat to canary versions |
tsconfig.json, babel.config.js |
Added @/lib path alias for library imports |
declarations.d.ts, .env.example |
Added ENV_PAY_API_KEY configuration |
EIP155Lib.ts |
Extended to support private key initialization and added getter methods |
EIP155WalletUtil.ts |
Added loadEIP155Wallet function for importing wallets with validation |
PayStore.ts |
New store managing PayClient singleton with availability checks |
ModalStore.ts |
Added PaymentOptionsModal and ImportWalletModal to modal types |
useInitializePaySDK.ts |
New hook for Pay SDK initialization with project configuration |
PaymentOptionsModal.tsx |
New modal with complete payment flow: data collection, method selection, and signing |
ImportWalletModal.tsx |
New modal for importing wallets via mnemonic or private key |
Modal.tsx |
Registered new modal components |
App.tsx |
Integrated Pay SDK initialization hook |
Connections/index.tsx |
Added payment link detection and handling logic |
Settings/index.tsx |
Added "Import EVM Wallet" button |
CopyURIDialog.tsx |
Updated text to support both WalletConnect URIs and Pay links |
android/settings.gradle |
Formatting cleanup (added blank lines) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| try { | ||
| const payClient = PayStore.getClient(); | ||
| const wallet = eip155Wallets[SettingsStore.state.eip155Address]; |
There was a problem hiding this comment.
The wallet retrieval doesn't handle the case where the wallet might be undefined. If SettingsStore.state.eip155Address doesn't exist in eip155Wallets, accessing wallet._signTypedData later will cause a runtime error. Add validation to check if the wallet exists before proceeding with the payment approval.
| const wallet = eip155Wallets[SettingsStore.state.eip155Address]; | |
| const address = SettingsStore.state.eip155Address; | |
| const wallet = address ? eip155Wallets[address] : undefined; | |
| if (!wallet) { | |
| setActionsError('No wallet available to sign this payment.'); | |
| return; | |
| } |
| // Remove EIP712Domain from types as ethers handles it | ||
| delete types.EIP712Domain; | ||
| const signature = await wallet._signTypedData( | ||
| domain, | ||
| types, |
There was a problem hiding this comment.
Directly mutating the types object by deleting EIP712Domain is problematic. This modifies the original typedData object which could cause issues if the data is reused or if the mutation affects the signature verification. Instead, create a copy of the types object before deleting EIP712Domain.
| // Remove EIP712Domain from types as ethers handles it | |
| delete types.EIP712Domain; | |
| const signature = await wallet._signTypedData( | |
| domain, | |
| types, | |
| // Remove EIP712Domain from a copy of types as ethers handles it | |
| const typesWithoutDomain = { ...types }; | |
| delete typesWithoutDomain.EIP712Domain; | |
| const signature = await wallet._signTypedData( | |
| domain, | |
| typesWithoutDomain, |
| messageData, | ||
| ); | ||
| console.log('[Pay] Signature:', signature); | ||
| signatures.push(signature); |
There was a problem hiding this comment.
There's no validation to ensure that signatures were actually collected for all required actions. If an action has a walletRpc but the method doesn't match any of the expected signing methods, no signature is pushed to the array. This could lead to submitting an incomplete set of signatures. Add validation to ensure the number of signatures matches the number of required actions, or add an else clause to handle unexpected methods.
| signatures.push(signature); | |
| signatures.push(signature); | |
| } else { | |
| // Handle unexpected wallet RPC methods explicitly to avoid incomplete signatures | |
| console.error('[Pay] Unsupported wallet RPC method:', method); | |
| throw new Error(`Unsupported wallet RPC method: ${method}`); |
| const handleImport = async () => { | ||
| if (!input.trim()) { | ||
| Alert.alert('Error', 'Please enter a mnemonic or private key'); | ||
| return; | ||
| } | ||
|
|
||
| setIsLoading(true); | ||
| try { | ||
| const { address } = loadEIP155Wallet(input); | ||
| Alert.alert('Success', `Wallet imported!\n\nNew address: ${address}`); | ||
| ModalStore.close(); |
There was a problem hiding this comment.
There's no warning or confirmation dialog before replacing the existing wallet. Importing a new wallet will permanently overwrite the current wallet data in storage without user confirmation. This could lead to accidental loss of access to the previous wallet. Consider adding a confirmation dialog that warns the user they will lose access to their current wallet unless they have backed up their credentials.
| initialize(options: PayClientOptions) { | ||
| // Check availability at call time, not import time | ||
| if (!PayClient.isAvailable()) { | ||
| console.warn('[PayStore] Native provider not available'); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| clientInstance = new PayClient(options); | ||
| console.log( | ||
| '[PayStore] PayClient initialized with projectId:', | ||
| clientInstance.projectId, | ||
| ); | ||
| } catch (error) { | ||
| console.error('[PayStore] Failed to initialize PayClient:', error); | ||
| clientInstance = null; | ||
| } | ||
| }, |
There was a problem hiding this comment.
The PayStore allows re-initialization without any safeguards. If initialize() is called multiple times, it will create a new PayClient instance each time, potentially leaving the old instance in an inconsistent state without proper cleanup. Consider adding a check to prevent re-initialization or properly disposing of the old client instance before creating a new one.
| const PAY_CONFIG = { | ||
| projectId: Config.ENV_PROJECT_ID || '', | ||
| apiKey: Config.ENV_PAY_API_KEY || '', | ||
| metadata: { | ||
| name: 'RN Web3Wallet', | ||
| bundleId: 'com.walletconnect.web3wallet.rnsample', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
The Pay SDK initialization proceeds with empty strings for projectId or apiKey if the environment variables are not set. This will likely cause the SDK to fail silently or produce confusing errors. Consider adding validation to check if these required configuration values are present before attempting initialization, and log a clear error message if they're missing.
| // Get wallet accounts - Base only for testing | ||
| const eip155Address = SettingsStore.state.eip155Address; | ||
| const accounts = eip155Address ? [`eip155:8453:${eip155Address}`] : []; |
There was a problem hiding this comment.
The hardcoded chain ID 8453 (Base) is used for all payment requests without checking which networks the payment actually supports or which networks the user has available. This could cause payment failures if the payment doesn't support Base or if the user needs to pay on a different network. Consider making the chain ID configurable or deriving it from the payment options response.
| <Image | ||
| source={{ uri: info.merchant.iconUrl }} | ||
| style={styles.merchantIcon} | ||
| /> |
There was a problem hiding this comment.
Image components are loading remote URLs without any error handling or fallback. If the iconUrl fails to load (network error, invalid URL, or missing resource), the user will see a broken image. Consider adding error handling with onError prop or providing a fallback placeholder image for both merchant icons and payment option icons.
| export default function Connections({route}: Props) { | ||
| // Check if a URI is a WalletConnect Pay payment link | ||
| function isPaymentLink(uri: string): boolean { | ||
| return uri.includes('pay.walletconnect.com') && uri.includes('pid='); |
There was a problem hiding this comment.
The payment link detection using string matching is fragile and could lead to false positives or false negatives. URLs could have different formats (http vs https, with/without www, different query parameter orders). Consider using URL parsing to more reliably check if a URI is a payment link by parsing the URL and checking the hostname and query parameters properly.
| return uri.includes('pay.walletconnect.com') && uri.includes('pid='); | |
| try { | |
| const url = new URL(uri); | |
| const hostname = url.hostname.toLowerCase(); | |
| // Accept both pay.walletconnect.com and www.pay.walletconnect.com | |
| const isPayHost = | |
| hostname === 'pay.walletconnect.com' || | |
| hostname === 'www.pay.walletconnect.com'; | |
| if (!isPayHost) { | |
| return false; | |
| } | |
| const pid = url.searchParams.get('pid'); | |
| return typeof pid === 'string' && pid.length > 0; | |
| } catch { | |
| // If the URI cannot be parsed as a URL, it's not a valid payment link | |
| return false; | |
| } |
| // Format amount with decimals | ||
| function formatAmount( | ||
| value: string, | ||
| decimals: number, | ||
| minDecimals: number = 0, | ||
| ): string { | ||
| const num = BigInt(value); | ||
| const divisor = BigInt(10 ** decimals); | ||
| const integerPart = num / divisor; | ||
| const fractionalPart = num % divisor; | ||
|
|
||
| if (fractionalPart === BigInt(0)) { | ||
| if (minDecimals > 0) { | ||
| return `${integerPart}.${'0'.repeat(minDecimals)}`; | ||
| } | ||
| return integerPart.toString(); | ||
| } | ||
|
|
||
| const fractionalStr = fractionalPart.toString().padStart(decimals, '0'); | ||
| // Remove trailing zeros, but keep at least minDecimals | ||
| let trimmedFractional = fractionalStr.replace(/0+$/, ''); | ||
| if (trimmedFractional.length < minDecimals) { | ||
| trimmedFractional = trimmedFractional.padEnd(minDecimals, '0'); | ||
| } | ||
| return `${integerPart}.${trimmedFractional}`; | ||
| } |
There was a problem hiding this comment.
The formatAmount function doesn't handle potential errors from invalid input. If the value parameter is not a valid numeric string that can be converted to BigInt, this function will throw an error at line 70. Consider adding input validation or error handling to gracefully handle invalid inputs, especially since this is used to format user-facing amounts.
| @@ -0,0 +1,842 @@ | |||
| import { useCallback, useState } from 'react'; | |||
There was a problem hiding this comment.
this component is huge, what do you think of moving each sub-view/step to a separate file? so each step has its own file, styles and logic
Summary
Payment Flow
flowchart TD A[Scan Pay QR Code] --> B{Has Collect Data?} B -->|Yes| C[Additional Information Form] B -->|No| D[Payment Options List] C --> |Continue| D D --> |Select Option| E[Confirm Payment Screen] E --> F[Get Required Actions] F --> G{Sign Transaction} G -->|Approve| H[Submit Signatures] H --> I[Payment Confirmed] G -->|Back| D C -->|Cancel| J[Close Modal] D -->|Cancel| JArchitecture
graph TB subgraph Components A[App.tsx] --> B[useInitializePaySDK] C[Connections Screen] --> D[PaymentOptionsModal] end subgraph Stores B --> E[PayStore] D --> E D --> F[ModalStore] end subgraph PaySDK[Pay SDK] E --> G[WalletConnect Pay] G --> H[Native Module] end subgraph Wallet D --> I[EIP155WalletUtil] I --> J[EIP155Lib] K[ImportWalletModal] --> I endChanges
New Files
PayStore.ts- State management for Pay SDK initialization and client accessPaymentOptionsModal.tsx- Full payment flow UI (collect data → select option → confirm & sign)useInitializePaySDK.ts- Hook for SDK initializationImportWalletModal.tsx- Modal for importing EVM wallets via mnemonic or private keyModified Files
EIP155Lib.ts- Support for private key initialization and new getter methodsEIP155WalletUtil.ts- Wallet loading and persistence logicModal.tsx- Register new modalsSettings/index.tsx- Add "Import EVM Wallet" buttonModalStore.ts- Add new modal typesENV_PAY_API_KEYsupportTest Plan