fix for transaction with approval, feature to cancel transaction and …#255
fix for transaction with approval, feature to cancel transaction and …#255
Conversation
…persistence of transaction in send modal
WalkthroughThis pull request introduces a cancel action into the send modal components and integrates WalletConnect transaction handling. A new optional callback ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SB as SendModalBottomButtons
participant ST as SendModalTokensTabView
participant BP as BottomMenuModalProvider
U->>SB: Clicks Cancel button
SB->>ST: Invoke onCancel callback
ST->>ST: Reset state variables (recipient, asset, amount, warnings)
ST->>BP: Invoke hide() to close modal
sequenceDiagram
participant TX as Transaction Handler
participant WC as useWalletConnect hook
participant BP as BottomMenuModalProvider
participant ST as SendModalTokensTabView
TX->>WC: Detect request type (approval vs transaction)
WC->>BP: Set walletConnectPayload via setWalletConnectPayload
BP->>ST: Update modal based on payload (adjust allowBatching & onCancel)
Note over WC,BP: Reset walletConnectPayload after handling
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/services/walletConnect.ts (2)
487-523: Enhanced transaction handling with approval support.The code now properly differentiates between approval and regular transactions, creating appropriate requests for each case. This fixes the issue with WalletConnect sendTransaction approval.
Consider extracting the common steps (showTransactionConfirmation, setWalletConnectPayload, getTransactionHash) into a helper function to reduce code duplication.
if (request.method === ETH_SEND_TX) { const transaction = request.params[0]; const isApprovalTransaction = !!transaction.data.startsWith('0x095ea7b3'); + const handleTransaction = async (requestData) => { + showTransactionConfirmation(requestData); + setWalletConnectPayload(requestData); + return await getTransactionHash(); + }; if (isApprovalTransaction) { const approvalRequest = { title: 'WalletConnect Approval Request', description: `${dAppName} is requesting approval for a contract.`, transaction: { to: checksumAddress(transaction.to), data: transaction.data, chainId: chainIdNumber, }, }; - showTransactionConfirmation(approvalRequest); - setWalletConnectPayload(approvalRequest); - requestResponse = await getTransactionHash(); + requestResponse = await handleTransaction(approvalRequest); } else { const transactionRequest = { title: 'WalletConnect Transaction Request', description: `${dAppName} wants to send a transaction`, transaction: { to: checksumAddress(transaction.to), value: formatEther(hexToBigInt(transaction.value)), data: transaction.data, chainId: chainIdNumber, }, }; - showTransactionConfirmation(transactionRequest); - setWalletConnectPayload(transactionRequest); - requestResponse = await getTransactionHash(); + requestResponse = await handleTransaction(transactionRequest); } }
532-532: Proper cleanup of WalletConnect payload.The code correctly resets the WalletConnect payload after processing the transaction, regardless of success or failure. Consider using a finally block to reduce code duplication.
try { await walletKit?.respondSessionRequest({ topic, response: formatJsonRpcResult(id, requestResponse), }); setWalletConnectTxHash(undefined); - setWalletConnectPayload(undefined); } catch (e: any) { console.error('WalletConnect session request error:', e.message); await walletKit?.respondSessionRequest({ topic, response: formatJsonRpcError(id, e), }); setWalletConnectTxHash(undefined); - setWalletConnectPayload(undefined); showToast({ title: 'WalletConnect', subtitle: 'The request has failed - there was a session request error. Please try again.', }); + } finally { + setWalletConnectPayload(undefined); }Also applies to: 540-540
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
462-465: Consider extracting the WalletConnect check into a variableThe condition
payload.title.includes('WalletConnect')is repeated twice, which could lead to inconsistency if one instance is updated but not the other.+ const isWalletConnectPayload = payload.title.includes('WalletConnect'); <SendModalBottomButtons onSend={onSend} safetyWarningMessage={safetyWarningMessage} isSendDisabled={isSendDisabled} isSending={isSending} errorMessage={errorMessage} estimatedCostFormatted={estimatedCostFormatted} onAddToBatch={onAddToBatch} - allowBatching={!payload.title.includes('WalletConnect')} - onCancel={ - !payload.title.includes('WalletConnect') ? undefined : onCancel - } + allowBatching={!isWalletConnectPayload} + onCancel={!isWalletConnectPayload ? undefined : onCancel}
277-280: Consider aligning the WalletConnect detection logicThe condition on lines 277-280 uses specific title checks ("WalletConnect Approval Request" and "WalletConnect Transaction Request"), while the condition on lines 462-465 uses a more general
includes('WalletConnect'). For better maintainability, consider using a consistent approach.+ const isWalletConnectApprovalOrTransaction = + payload?.title === 'WalletConnect Approval Request' || + payload?.title === 'WalletConnect Transaction Request'; if (newUserOpHash) { - if ( - payload?.title === 'WalletConnect Approval Request' || - payload?.title === 'WalletConnect Transaction Request' - ) { + if (isWalletConnectApprovalOrTransaction) { const txHash = await getTransactionHash(newUserOpHash); if (!txHash) { setWalletConnectTxHash(undefined); } else { setWalletConnectTxHash(txHash); } } } // Later in the code... + const isWalletConnectPayload = payload.title.includes('WalletConnect'); <SendModalBottomButtons // other props... - allowBatching={!payload.title.includes('WalletConnect')} - onCancel={ - !payload.title.includes('WalletConnect') ? undefined : onCancel - } + allowBatching={!isWalletConnectPayload} + onCancel={isWalletConnectPayload ? onCancel : undefined}Also applies to: 462-465
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
src/components/BottomMenuModal/SendModal/SendModalBottomButtons.tsx(4 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(4 hunks)src/providers/BottomMenuModalProvider.tsx(4 hunks)src/services/walletConnect.ts(8 hunks)src/translations/en.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (18)
src/translations/en.json (1)
32-32: Good addition of the "cancel" translation.The new cancel action has been properly added to the existing action section, following the established structure and convention of the translations file.
src/components/BottomMenuModal/SendModal/SendModalBottomButtons.tsx (3)
19-19: Well-defined optional callback for cancel action.The new
onCanceloptional callback is properly typed in the interface, enhancing the component's flexibility.
31-31: Proper prop destructuring.The
onCancelcallback is correctly added to the component's destructured props.
42-64: Clean implementation of the cancel button.The code now properly handles conditionally rendering either the "Add to Batch" button or the new "Cancel" button based on the
allowBatchingprop. The Cancel button maintains UI consistency with proper styling and follows the existing component patterns.src/providers/BottomMenuModalProvider.tsx (5)
17-20: Good addition of WalletConnect payload state to context.The context interface is properly extended with the new state properties for managing WalletConnect payload data.
46-48: Well-implemented state management.The new state is correctly initialized with React's useState hook and properly typed.
66-69: Enhanced showSend method with WalletConnect support.The
showSendmethod now intelligently checks for the presence of WalletConnect payload data and includes it when available, improving the user experience for WalletConnect transactions.
78-79: Context data properly updated.The new state variables are correctly exposed through the context data object.
81-81: Correct dependency array update.The
useMemodependency array is properly updated to includewalletConnectPayload, ensuring the context is recalculated when this value changes.src/services/walletConnect.ts (6)
24-24: Proper import of required type.The SendModalData type is correctly imported for use with the new state management.
39-44: Complete hook destructuring.The additional properties from useBottomMenuModal are properly destructured to access the new WalletConnect payload functionality.
56-58: Good use of ref for tracking WalletConnect payload.Creating a ref to track the current WalletConnect payload ensures that the async operation has access to the most current value.
109-111: Proper ref synchronization.The useEffect hook correctly updates the ref whenever the WalletConnect payload changes.
125-127: Improved error handling.Added an early return when the payload is undefined, which prevents unnecessary waiting and improves user experience during cancellation.
142-142: More descriptive error message.The transaction timeout error message has been improved to provide clearer guidance to users.
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (3)
92-98: Properly integrates new WalletConnect state managementThe addition of
setWalletConnectPayloadfromuseBottomMenuModalhook enables the reset of WalletConnect transaction state when canceling operations, which aligns with the PR objective of supporting transaction cancellation.
277-280: Good enhancement to WalletConnect transaction detectionThe condition has been expanded to handle both "WalletConnect Approval Request" and "WalletConnect Transaction Request" types, making the transaction hash retrieval more robust and fixing the approval flow as mentioned in the PR objectives.
355-367: Well-implemented cancellation functionalityThe
onCancelfunction properly resets all relevant state variables and appropriately handles WalletConnect payloads, supporting the PR's goal of adding transaction cancellation capability.
Deploying x with
|
| Latest commit: |
7c79f14
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://390ff802.x-e62.pages.dev |
| Branch Preview URL: | https://fix-wallet-connect-qa-fixes.x-e62.pages.dev |
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
Just a small defensive coding request (after what's been happening) but otherwise looking good!
2 participants
…persistence of transaction in send modal
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit