-
Notifications
You must be signed in to change notification settings - Fork 392
feat: wallet pay dapp #926
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
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # advanced/dapps/react-dapp-v2/pnpm-lock.yaml
|
The latest updates on your projects. Learn more about Vercel for GitHub.
8 Skipped Deployments
|
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 implements WalletConnect Pay functionality based on CAIP-358, adding comprehensive payment processing capabilities to both wallet and dapp sides with authentication support.
- Adds WalletConnect Pay support for wallet-to-dapp USDC transfers on multiple EVM chains
- Implements authentication message handling and verification across multiple blockchain namespaces
- Creates a dedicated POS terminal dapp for processing cryptocurrency payments
Reviewed Changes
Copilot reviewed 29 out of 44 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| advanced/wallets/react-wallet-v2/src/views/SessionProposalModal.tsx | Adds wallet pay UI components and authentication message handling |
| advanced/wallets/react-wallet-v2/src/utils/WalletPay.ts | Implements wallet pay processing logic |
| advanced/wallets/react-wallet-v2/src/utils/AuthUtil.ts | Adds authentication message preparation and signing utilities |
| advanced/wallets/react-wallet-v2/src/lib/EIP155Lib.ts | Implements ERC20 token transfer for wallet pay |
| advanced/dapps/walletconnect-pay-dapp/src/app/page.tsx | Creates POS terminal interface for payment processing |
| advanced/dapps/react-dapp-v2/src/contexts/ClientContext.tsx | Adds authentication request handling and verification |
| advanced/dapps/react-dapp-v2/src/components/Blockchain.tsx | Adds visual authentication indicators |
Files not reviewed (1)
- advanced/wallets/react-wallet-v2/pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const { getAvailableSmartAccountsOnNamespaceChains } = useSmartAccounts() | ||
| const [walletPay, setWalletPay] = useState(proposal?.params?.requests?.walletPay) | ||
| const [processPayment, setProcessPayment] = useState(true) | ||
| console.log('proposal', JSON.stringify(proposal, null, 2)) |
Copilot
AI
Oct 7, 2025
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.
Debug console.log statement should be removed from production code to avoid cluttering the console output.
| console.log('proposal', JSON.stringify(proposal, null, 2)) |
| export async function processWalletPay({ walletPay }: { walletPay: EngineTypes.WalletPayParams }) { | ||
| const chainId = parseChainId(walletPay.acceptedPayments[0].recipient) | ||
| const address = getAddress(`${chainId.namespace}:${chainId.reference}`) | ||
| const wallet = (await getWallet(address)) as EIP155Lib | ||
| return wallet.walletPay(walletPay) | ||
| } |
Copilot
AI
Oct 7, 2025
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.
The function assumes acceptedPayments[0] exists without validation. Add a check to ensure acceptedPayments array is not empty before accessing the first element to prevent potential runtime errors.
| const tx = await token.transfer(recepientAddress, assetAmount, { | ||
| gasLimit: 100_000n // manually override | ||
| }) |
Copilot
AI
Oct 7, 2025
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.
Corrected spelling of 'recipientAddress' variable name."
| const tx = await token.transfer(recepientAddress, assetAmount, { | ||
| gasLimit: 100_000n // manually override |
Copilot
AI
Oct 7, 2025
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.
Hard-coded gas limit should be calculated dynamically or made configurable. A fixed value of 100,000 may be insufficient for complex token transfers or cause failures on congested networks.
| const tx = await token.transfer(recepientAddress, assetAmount, { | |
| gasLimit: 100_000n // manually override | |
| const estimatedGas = await token.estimateGas.transfer(recepientAddress, assetAmount); | |
| const gasLimit = estimatedGas + estimatedGas / 10n; // add 10% buffer | |
| const tx = await token.transfer(recepientAddress, assetAmount, { | |
| gasLimit |
| // @ts-expect-error - slight type mismatch due to new universal provider version | ||
| universalProvider: _up, |
Copilot
AI
Oct 7, 2025
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.
TypeScript error suppression should be addressed by fixing the underlying type mismatch rather than ignoring it. This creates technical debt and potential runtime issues.
| // @ts-expect-error - slight type mismatch due to new universal provider version | |
| universalProvider: _up, | |
| universalProvider: _up as InstanceType<typeof UniversalProvider>, |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
WalletConnect Pay based on https://eip.tools/caip/358