-
Notifications
You must be signed in to change notification settings - Fork 7
feat: enable multi-approval processing fixing USDT payment issue #132
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
feat: enable multi-approval processing fixing USDT payment issue #132
Conversation
WalkthroughRefactors direct payment logic to use paymentTransactionIndex from metadata, iterating approvals across all non-payment transactions before submitting the designated payment transaction. Removes fixed index logic, adds optional chaining for network-switch toast, and leaves cross-chain flow unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as PaymentSection
participant Wallet as Wallet/Provider
participant Chain as Blockchain
User->>UI: Initiate direct payment
UI->>UI: Read paymentTransactionIndex from metadata
alt needsApproval
loop For each txn ≠ paymentTransactionIndex
UI->>Wallet: Send approval transaction
Wallet->>Chain: Broadcast approval
Chain-->>Wallet: Approval receipt
Wallet-->>UI: Approval result
end
end
UI->>Wallet: Send payment transaction (at paymentTransactionIndex)
Wallet->>Chain: Broadcast payment
Chain-->>Wallet: Payment receipt
Wallet-->>UI: Payment result
note over UI: Network-switch toast uses optional chaining for name
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/payment-section.tsx (3)
297-305: Drive approvals via an explicit index list; preserves order and clarifies intentCurrent loop is fine, but iterating all indices and skipping the payment index is a bit opaque and couples logic to equality checks. Building an approvals index list improves readability and makes room for progress UX if you want to show “Approval 1/2…”. It also leverages the sanitized transactions array from the previous comment.
Apply this diff:
- // Execute all approval transactions (all transactions except the payment transaction) - for (let i = 0; i < paymentData.transactions.length; i++) { - if (i !== paymentTransactionIndex) { - const approvalTransaction = await signer.sendTransaction( - paymentData.transactions[i], - ); - await approvalTransaction.wait(); - } - } + // Execute all approval transactions (every tx except the payment one), preserving order from the API + const approvalTxIndices = transactions + .map((_, i) => i) + .filter((i) => i !== paymentTransactionIndex); + for (const i of approvalTxIndices) { + const approvalTx = await signer.sendTransaction(transactions[i]); + await approvalTx.wait(); + }Optional UX tweak (outside this range): update the approval toast to reflect multiple approvals, e.g., “Please approve 2 transactions in your wallet.”
315-316: Use the sanitized transactions array for the payment submissionMinor reliability/readability improvement: use the validated transactions array introduced above.
- const paymentTransaction = await signer.sendTransaction( - paymentData.transactions[paymentTransactionIndex], - ); + const paymentTransaction = await signer.sendTransaction( + transactions[paymentTransactionIndex], + );
335-349: Fail fast if targetAppkitNetwork cannot be resolved; improve user feedbackToday you always attempt switchNetwork even if the mapping is undefined; this is caught and a generic error toast is shown. Provide an earlier, clearer message and avoid making a doomed switch call.
if (targetChain !== chainId) { const targetAppkitNetwork = ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; - toast("Switching to network", { - description: `Switching to ${targetAppkitNetwork?.name} network`, - }); + if (!targetAppkitNetwork) { + toast("Unsupported network", { + description: `Cannot resolve network for ${selectedRoute?.chain}.`, + }); + return; + } + + toast("Switching to network", { + description: `Switching to ${targetAppkitNetwork.name} network`, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/payment-section.tsx(3 hunks)
🔇 Additional comments (1)
src/components/payment-section.tsx (1)
340-340: Nice null-safety on network name in toastThe optional chaining prevents a runtime error when the network mapping is temporarily unavailable. Good defensive tweak.
Problem
Due to the way USDT works in mainnet and a bug in our API, first USDT invoice payments goes through, second payment would prompt for an approval again, which is blocked by metamask because it will automatically fail because USDT on mainnet blocks non zero approvals if a user already has allowance.
This PR builds on top of changes made to Request API , and fully depends on that PR.
Summary
Enable multi-approval processing for Ethereum USDT direct payments to fix approval transaction
failures.
Changes
multiple approval transactions instead of assuming a single approval transaction
paymentTransactionIndexmetadata to correctly identify which transaction is the actual payment
switching
Technical Details
The previous implementation assumed only one approval transaction was needed, but some payment
scenarios require multiple approvals. This PR:
Files Modified
src/components/payment-section.tsx- UpdatedhandleDirectPaymentsfunction to supportmultiple approval transactions
Screenshot:
All of these invoices were paid in a row
Summary by CodeRabbit