feat: integrate cross-chain payment into EasyInvoice #42
Conversation
…tency across invoice components
…plement new alert component
…tion for mainnet currencies
WalkthroughThis pull request makes multiple updates across the application. It alters numerical formatting in invoice pages by replacing Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PS as PaymentSection
participant PR as PaymentRoute Component
participant IR as Invoice Router
U->>PS: Initiate Payment Request
PS->>IR: getPaymentRoutes(paymentReference, walletAddress)
IR-->>PS: Return available payment routes
U->>PS: Select a Payment Route
PS->>PR: Render selected route details
PS->>IR: sendPaymentIntent(paymentIntent, payload)
IR-->>PS: Return payment intent response
PS-->>U: Display Payment Confirmation/Feedback
Assessment against linked issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 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 (
|
…-easyinvoice---integrate-payment-routes-and-crosschain-payments
![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 (6)
src/lib/currencies.ts (1)
22-22: Expansion of INVOICE_CURRENCIES with MAINNET_CURRENCIES.Appending
...MAINNET_CURRENCIESensures that all mainnet currencies become valid invoice currencies. Consider verifying that duplicates won't arise in case of future changes, but currently there's no functional issue.src/server/routers/invoice.ts (3)
193-200: Extended payRequest input schema.Switching from a string to an object with optional fields (
wallet,chain,token) offers flexibility in specifying payment parameters. Consider adding minimal validation forchainandtokenif they must be from a known set.
211-222: Constructing paymentEndpoint with string concatenation.Appending query parameters conditionally is functional, but consider using
URLSearchParamsor a similar library for more robust handling of edge cases (e.g., ifchainortokencontain special characters).- let paymentEndpoint = `/v1/request/${invoice.paymentReference}/pay?wallet=${input.wallet}`; - if (input.chain) { - paymentEndpoint += `&chain=${input.chain}`; - } - if (input.token) { - paymentEndpoint += `&token=${input.token}`; - } + const url = new URL(`/v1/request/${invoice.paymentReference}/pay`, baseURL); + if (input.wallet) url.searchParams.set("wallet", input.wallet); + if (input.chain) url.searchParams.set("chain", input.chain); + if (input.token) url.searchParams.set("token", input.token); + const paymentEndpoint = url.toString();
299-314: sendPaymentIntent procedure lacks explicit error checks.After posting the payment intent, the code returns
response.datadirectly. If the request fails with a 4xx/5xx, an exception is thrown automatically, but consider adding a try/catch or verifying the response status code to provide custom error messages.+ try { const response = await apiClient.post( `/v1/request/${paymentIntent}/send`, payload, ); + if (response.status !== 200) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Failed to send payment intent", + }); + } return response.data; + } catch (error) { + // handle error + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Failed to send payment intent", + }); + }src/components/payment-section.tsx (2)
128-132: Auto-selecting the first route.Conveniently picks the first available route, but might consider a user preference if routes become more complex in the future.
166-229: Payment handling flow with EIP-2612 and paygrid support.The layered flow with “approving” and “paying” states is logical. Handling signatures via
_signTypedDatais effective. Consider a bit more robust error handling for any partial failures (like approval success but payment failure).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
public/arbitrum-logo.pngis excluded by!**/*.pngpublic/base-logo.pngis excluded by!**/*.pngpublic/ethereum-logo.pngis excluded by!**/*.pngpublic/optimism-logo.pngis excluded by!**/*.pngpublic/polygon-logo.pngis excluded by!**/*.pngpublic/request-logo.pngis excluded by!**/*.png
📒 Files selected for processing (11)
src/app/invoices/[ID]/page.tsx(2 hunks)src/components/app-kit.tsx(2 hunks)src/components/invoice-form.tsx(3 hunks)src/components/invoice-preview.tsx(2 hunks)src/components/payment-route.tsx(1 hunks)src/components/payment-section.tsx(7 hunks)src/components/ui/alert.tsx(1 hunks)src/lib/chains.ts(1 hunks)src/lib/currencies.ts(2 hunks)src/lib/types/index.ts(1 hunks)src/server/routers/invoice.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
src/app/invoices/[ID]/page.tsx (1)
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#21
File: src/app/invoices/[ID]/page.tsx:113-148
Timestamp: 2025-03-18T18:53:40.661Z
Learning: The easy-invoice project prefers simpler, direct implementations over abstract utilities. For example, using `.toFixed(2)` directly instead of creating separate number formatting utilities.
src/server/routers/invoice.ts (1)
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-03-18T18:53:33.360Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
src/components/invoice-preview.tsx (1)
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#21
File: src/app/invoices/[ID]/page.tsx:113-148
Timestamp: 2025-03-18T18:53:40.661Z
Learning: The easy-invoice project prefers simpler, direct implementations over abstract utilities. For example, using `.toFixed(2)` directly instead of creating separate number formatting utilities.
🧬 Code Definitions (4)
src/lib/types/index.ts (1)
src/components/payment-route.tsx (1) (1)
PaymentRoute(19-119)
src/components/payment-route.tsx (1)
src/lib/types/index.ts (1) (1)
PaymentRoute(1-8)
src/server/routers/invoice.ts (3)
src/server/db/schema.ts (1) (1)
requestTable(48-80)src/lib/axios.ts (1) (1)
apiClient(3-8)src/server/trpc.ts (1) (1)
publicProcedure(12-12)
src/components/invoice-form.tsx (2)
src/lib/currencies.ts (3) (3)
MAINNET_CURRENCIES(1-12)MainnetCurrency(14-14)formatCurrencyLabel(49-84)src/components/ui/alert.tsx (3) (3)
Alert(61-61)AlertTitle(61-61)AlertDescription(61-61)
🔇 Additional comments (39)
src/lib/types/index.ts (1)
1-8: Well-structured interface for cross-chain payment routesThe
PaymentRouteinterface is cleanly defined with all necessary properties for representing different payment routing options. This interface properly supports the cross-chain payment functionality being implemented in this PR.The
speedproperty's union type (number | "FAST") appropriately accommodates both numerical time estimates in seconds and qualitative speed indicators.src/lib/chains.ts (1)
1-26: Good implementation of chain mappings for cross-chain supportThe constants
CHAIN_TO_IDandID_TO_APPKIT_NETWORKare well-structured and provide a clean way to map between chain names, IDs, and network objects. This implementation nicely supports the cross-chain functionality described in the PR objectives.The inclusion of mainnet networks (BASE, ETHEREUM, ARBITRUM, OPTIMISM, POLYGON) aligns perfectly with the PR goal of enabling support for real cryptocurrency payments.
src/components/app-kit.tsx (2)
3-11: Clean import of additional network dependenciesThe updated imports are properly structured to include all required network constants from the
@reown/appkit/networkspackage.
30-30: Expanded network support matches PR objectivesThe networks array has been successfully expanded to include all the required networks mentioned in the PR objectives: sepolia (testnet), base, mainnet, arbitrum, optimism, and polygon. This change is essential for enabling the cross-chain payment functionality.
src/components/invoice-preview.tsx (1)
131-131: Verify numeric formatting change from toFixed(2) to toString()The change from
.toFixed(2)to.toString()will remove the fixed decimal precision in number display. While this aligns with the PR objective of "improving formatting by using toString() instead of toFixed(2)", it contradicts a previous learning that indicated the project prefers direct implementations like.toFixed(2).Consider confirming if this change is intentional, as it will affect how decimal numbers appear in invoices:
.toFixed(2)ensures consistent 2 decimal places (e.g., "10.50").toString()will show variable decimal places or none (e.g., "10.5" or "10")What are the differences between JavaScript's toFixed(2) and toString() for currency display?Also applies to: 134-134, 146-146, 152-152
Likely an incorrect or invalid review comment.
src/app/invoices/[ID]/page.tsx (1)
171-171: Improved cryptocurrency value display precision with toString()Changing from
toFixed(2)totoString()allows for displaying the precise value of cryptocurrency amounts without truncating decimal places, which is critical for accurate representation of cryptocurrency values that can have many significant decimal places.This change aligns with the PR objective of improving the formatting of cryptocurrency amounts for better accuracy.
Also applies to: 174-174, 187-187, 194-194
src/components/invoice-form.tsx (2)
229-231: Price input now properly handles decimal valuesAdding
step="any"andmin="0"to the price input addresses the previous issue where decimal values couldn't be entered properly and ensures that negative values cannot be input.
302-328: Well-implemented warning for real funds transactionsThis warning alert provides clear information to users when they select a mainnet currency for their invoice. It properly:
- Checks if the selected currency is in the MAINNET_CURRENCIES list
- Displays the specific currency name in the alert
- Clearly communicates the risk of using real cryptocurrency
- Informs users that this is a demonstration app only
This implementation directly addresses the PR objective of enhancing user awareness and security when dealing with real funds.
src/components/payment-route.tsx (2)
5-119: Well-designed payment route component with appropriate visual indicatorsThe
PaymentRoutecomponent effectively implements the different payment route types mentioned in the PR objectives:
- Direct payment routes
- Same-chain ERC20 payment routes
- Cross-chain payment routes
The implementation includes:
- Visual differentiators with appropriate icons and color schemes
- Network logos for different blockchain networks
- Clear display of fees and transaction speeds
- Proper state handling for selected/unselected routes
The component provides a good user experience by showing relevant information about each payment option in a consistent format.
72-82:Details
✅ Verification successful
Verify that all required network logos are available
The component attempts to load network logos from the public directory using dynamic paths like
/${route.chain.toLowerCase()}-logo.png. Ensure that all required logo images for supported networks (Base, Ethereum, Arbitrum, Optimism, Polygon) are present in the public directory.
🏁 Script executed:
#!/bin/bash # Check if all required network logo images exist in the public directory echo "Checking for required network logo files..." NETWORKS=("base" "ethereum" "arbitrum" "optimism" "polygon") for network in "${NETWORKS[@]}"; do if fd "${network}-logo.png" public/; then echo "✅ Found: ${network}-logo.png" else echo "❌ Missing: ${network}-logo.png" fi doneLength of output: 904
Payment Route: Network Logos Verified
The verification confirms that all required network logos for Base, Ethereum, Arbitrum, Optimism, and Polygon exist in the public directory. No further changes are needed.
src/components/ui/alert.tsx (1)
1-62: Well-structured alert component with appropriate variantsThis new Alert component system provides:
- A flexible base Alert component with support for different variants (default, destructive, warning)
- Consistent styling with good accessibility attributes (role="alert")
- Specialized AlertTitle and AlertDescription subcomponents for structured content
- Proper naming and React.forwardRef implementation for component composition
The warning variant is particularly useful for the mainnet currency alerts implemented in the invoice form, supporting the PR objective of enhancing user awareness when dealing with real funds.
src/lib/currencies.ts (7)
1-12: Good introduction of MAINNET_CURRENCIES array.Defining these mainnet tokens as a constant array using
as constprovides strong type safety and clarity. No issues spotted regarding correctness or naming consistency.
14-14: Type safety enhancement is appropriate.Exporting
MainnetCurrencyfrom thetypeof MAINNET_CURRENCIESarray is a good way to keep the type in sync with the actual tokens.
26-28: Use of Partial for PAYMENT_CURRENCIES is well-reasoned.Marking the mapping as partial is beneficial since not all invoice currencies need a mapping. This helps avoid TypeScript errors when some currencies are missing from the final object.
34-36: Automated generation of payment currency entries.Using
Object.fromEntriesto map each mainnet currency to itself is neat. It simplifies the code and limits duplication compared to manually listing each currency.
39-41: Refined PaymentCurrency type ensures proper indexing.
NonNullable<...>guarantees that the resulting type cannot beundefined. This helps avoid runtime errors when referencing index positions inPAYMENT_CURRENCIES.
46-46: Fallback to an empty array is a reliable safeguard.Returning
[]if the mapping is undefined ensures consumers can handle currencies gracefully without null checks.
61-80: User-friendly formatting logic.Providing clear and consistent currency labels for mainnet tokens is a robust usability improvement. The naming convention ("USDC (Polygon)") matches real-world chain references and avoids confusion.
src/server/routers/invoice.ts (3)
7-7: Importing isEthereumAddress is a valid approach.Using the
validatorlibrary for Ethereum address checks is simpler than rolling out your own regex or logic. No conflicts observed.
204-204: Query for invoice using paymentReference.Ensuring
eq(requestTable.paymentReference, input.paymentReference)precisely selects the correct invoice. Straightforward and correct.
268-297: New getPaymentRoutes procedure.Including a refinement on
walletAddressto ensure it's a valid Ethereum address is excellent. The error handling (throwingTRPCError) for non-200 responses is also appropriate. No immediate concerns.src/components/payment-section.tsx (18)
3-3: PaymentRoute import.Importing and rendering a dedicated
<PaymentRoute>component keeps the code modular and more maintainable.
7-7: Importing chain mappings.Using
CHAIN_TO_IDandID_TO_APPKIT_NETWORKcentralizes chain constants, reducing hardcoded string usage throughout the code.
8-8: MAINNET_CURRENCIES usage.Having direct access to
MAINNET_CURRENCIESavoids accidental duplication of mainnet currency references.
9-9: Importing PaymentRouteType.Typing the route object ensures correct usage of its fields in this component.
15-15: useAppKitNetwork addition.Provides chain context to handle chain switching logic. Good approach to unify network state management.
19-19: Icon imports for improved UI.Using these icons (
AlertCircle,CheckCircle,Clock, etc.) clarifies the payment flow and statuses for users.
28-32: Helper function getCurrencyChain.Extracting the chain from currency strings is concise. Consider validating or handling unexpected formats, but for known patterns, this is acceptable.
34-60: getRouteType logic is well-organized.Separating direct payment, same-chain ERC20, and cross-chain logic is a clear and extensible strategy. Each return object is explicit and easy to maintain.
63-70: Local state initialization for route selection.Maintaining
showRoutesandselectedRoutein component state is a straightforward way to manage route display logic.
76-78: sendPaymentIntent mutation.This ties server-side logic to the front-end. Good to see consistent usage of TRPC to handle API requests.
80-83: getPaymentRoutes query usage.Refetching available routes based on the user’s connected wallet. Enabling the query on
!!addressensures no unnecessary calls when not connected.
93-95: Deriving invoiceChain from getCurrencyChain.Ensures the component automatically adjusts functionality based on the chain embedded in the payment currency.
122-126: Refetch on address change.Automatically refreshing available routes whenever the user switches wallets or accounts is essential for accurate route data.
145-158: Network switching logic.Prompting the user to switch networks if the target chain differs from the current chain is vital for a smooth multi-chain experience. The user feedback via toast is a nice touch.
279-280: Computed hasRoutes variable is helpful.Clearly indicates if the user can proceed with a payment or if no route is available.
320-336: Real Cryptocurrency Warning block.Excellently informs users of real-fund risks on mainnet. This is a crucial user safety message.
467-544: Payment route selection UI.Displaying a user-friendly interface for choosing among multiple payment routes is a great UX improvement. Having a “See Other Options” toggle is particularly helpful.
550-562: Payment button state logic.Disabling the button when routes are unavailable or while processing avoids accidental double submissions. The loading indicator is well integrated.
MantisClone
left a comment
MantisClone
left a comment
There was a problem hiding this comment.
Cross-chain UI Concerns:
- "No fee" for direct payment, but they still have to pay the gas for approval and payment transactions, right?
- Request Network logo makes it look like we're a chain. Maybe it would be better to show the Base chain logo with a small Request Network logo in the bottom right - to make it clear that the payment is still processed on Base, but using Request Network protocol directly as opposed to via paygrid.
- Continuing that thinking - maybe we should show a little paygrid logo at the bottom right of all the Paygrid routes? - Leaning towards "No" but figured I'd ask the question.
MantisClone
left a comment
MantisClone
left a comment
There was a problem hiding this comment.
Preliminary Review. I haven't been able to run it locally yet for testing.
… and optimize logo rendering
…d-crosschain-payments
…n-payments' of github.com:RequestNetwork/easy-invoice into 36-easyinvoice---integrate-payment-routes-and-crosschain-payments
…nd user notifications
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/components/payment-route.tsx (3)
30-30: Consider a more scalable approach fornativeTokendetermination.The current implementation only handles ETH for most chains and POL for POLYGON. This hardcoding approach might not scale well as more chains are added to the platform.
- const nativeToken = route.chain === "POLYGON" ? "POL" : "ETH"; + // Map chains to their native tokens + const CHAIN_TO_NATIVE_TOKEN: Record<string, string> = { + POLYGON: "POL", + ETHEREUM: "ETH", + BASE: "ETH", + ARBITRUM: "ETH", + OPTIMISM: "ETH", + // Add other chains as needed + }; + const nativeToken = CHAIN_TO_NATIVE_TOKEN[route.chain] || "ETH";
74-80: Add error handling for image loading.The component doesn't handle potential errors when loading chain logo images. This could cause visual issues if a logo image is missing.
<Image src={`/${route.chain.toLowerCase()}-logo.png`} alt={`${route.chain} logo`} width={32} height={32} className="object-contain" + onError={(e) => { + // Fallback to a generic icon if image fails to load + e.currentTarget.src = "/generic-chain-logo.png"; + }} />
90-93: Improve readability of the conditional route label.The current conditional rendering for route type label is a bit hard to follow. Consider restructuring this for better readability.
-{routeType?.label || - (isDirectPayment ? "Direct Payment" : "via Paygrid")} +{routeType?.label || ( + isDirectPayment + ? "Direct Payment" + : "via Paygrid" +)}src/components/payment-section.tsx (3)
28-32: EnhancegetCurrencyChainwith error handling and validation.The current implementation of
getCurrencyChainmight return null but doesn't handle potential malformed currency strings well.const getCurrencyChain = (currency: string) => { // Extract chain from format like "USDC-base" const parts = currency.split("-"); - return parts.length > 1 ? parts[1].toLowerCase() : null; + if (parts.length > 1) { + const chain = parts[1].toLowerCase(); + // Validate that the chain is recognized + const validChains = Object.keys(REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK).map(c => c.toLowerCase()); + return validChains.includes(chain) ? chain : null; + } + return null; };
34-40: Make chain mapping more maintainable.This object mapping is crucial to the payment functionality. Consider making it more maintainable by adding a description and ensuring it's complete.
+/** + * Maps Request Network chain identifiers to their corresponding payment network identifiers. + * This mapping is used to translate between different naming conventions for the same networks. + */ const REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK = { matic: "polygon", base: "base", "arbitrum-one": "arbitrum", optimism: "optimism", mainnet: "ethereum", + // Add any additional networks here when supported };
340-357: Improve type safety for mainnet currency check.The use of
as anyin the includes check is a type safety concern.-{MAINNET_CURRENCIES.includes(invoice.invoiceCurrency as any) && ( +{(MAINNET_CURRENCIES as readonly string[]).includes(invoice.invoiceCurrency) && (Alternatively, define a proper type guard function:
const isMainnetCurrency = (currency: string): boolean => { return MAINNET_CURRENCIES.includes(currency as any); } // Then use: {isMainnetCurrency(invoice.invoiceCurrency) && ( // ... )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/optimism-logo.pngis excluded by!**/*.png
📒 Files selected for processing (2)
src/components/payment-route.tsx(1 hunks)src/components/payment-section.tsx(8 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/components/payment-route.tsx (1)
src/lib/types/index.ts (1)
PaymentRoute(1-8)
src/components/payment-section.tsx (5)
src/server/db/schema.ts (1)
Request(135-135)src/lib/chains.ts (1)
CHAIN_TO_ID(10-17)src/lib/currencies.ts (1)
MAINNET_CURRENCIES(1-12)src/components/payment-route.tsx (1)
PaymentRoute(19-117)src/lib/types/index.ts (1)
PaymentRoute(1-8)
🔇 Additional comments (2)
src/components/payment-section.tsx (2)
571-575: LGTM: Payment button disabled state properly handles all scenarios.The payment button's disabled state correctly accounts for various scenarios: when payment is in progress, when no routes are available, or when payment is already processing.
246-247:Details
✅ Verification successful
Implement immediate feedback with "processing" state.
Currently the payment status is set to "processing" here, but there's no immediate visual feedback to the user.
According to past review comments, there was a suggestion to display "processing" immediately after the user submits the signed payment intent. Let's verify the flow here:
🏁 Script executed:
#!/bin/bash # Checking for any UI indication of processing state rg -A 3 -B 3 "setPaymentStatus\(\"processing\"\)" --glob "src/**/*.tsx"Length of output: 1327
Immediate Visual Feedback is Now Implemented
The verification confirms that after setting the status to "processing" in
src/components/payment-section.tsx, a toast notification ("Payment is being processed") appears immediately, fulfilling the requirement for prompt visual feedback.
- In
src/components/payment-section.tsx, the call tosetPaymentStatus("processing")is promptly followed by a toast that clearly informs the user about the processing state.- Similarly, a consistent pattern is seen in
src/components/direct-payment.tsx.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/app/api/webhook/route.ts (1)
96-109: 💡 Verification agent🧩 Analysis chain
Verify behavior when multiple recurring invoices are created.
The implementation works well for generating sequential invoice numbers, but it's important to verify that the system behaves correctly when multiple recurring invoices are created in quick succession. Consider adding a test case for this scenario.
🏁 Script executed:
#!/bin/bash # Look for existing tests related to recurring invoice creation echo "Searching for tests related to recurring invoices..." rg -l "recurring" --glob "**/*.test.ts" --glob "**/*.spec.ts" # Look for implementation of getInvoiceCount and generateInvoiceNumber echo -e "\nSearching for implementation of invoice number generation functions..." rg -A 10 "getInvoiceCount" --glob "src/lib/invoice/**" rg -A 10 "generateInvoiceNumber" --glob "src/lib/invoice/**"Length of output: 1816
Action Required: Add tests to simulate concurrent recurring invoice creation
The current implementation appears to correctly generate sequential invoice numbers. However, we haven't found tests verifying behavior when multiple recurring invoices are created nearly simultaneously. Please add a test case that simulates concurrent recurring invoice creation to ensure that:
- Each invoice receives a unique, correctly incremented number.
- No race conditions occur in the process involving
getInvoiceCount(located insrc/lib/invoice/index.ts) andgenerateInvoiceNumber(located insrc/lib/invoice/client.ts).
🧹 Nitpick comments (1)
src/app/api/webhook/route.ts (1)
96-98: Consider adding error handling for invoice count retrieval.The code retrieves the invoice count and generates an invoice number, but lacks error handling if
getInvoiceCount()fails. SinceinvoiceNumberis a required field in the schema, this could cause transaction failures.- const invoiceCount = await getInvoiceCount(originalRequest.userId); - - const invoiceNumber = generateInvoiceNumber(invoiceCount); + let invoiceCount; + try { + invoiceCount = await getInvoiceCount(originalRequest.userId); + } catch (error) { + console.error("Failed to get invoice count:", error); + invoiceCount = 0; // Fallback to ensure we can still generate an invoice number + } + + const invoiceNumber = generateInvoiceNumber(invoiceCount);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/api/webhook/route.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/app/api/webhook/route.ts (1)
src/server/db/schema.ts (1)
requestTable(48-80)
🔇 Additional comments (2)
src/app/api/webhook/route.ts (2)
2-3: New imports enhance invoice number functionality.These imports are necessary to support the new invoice number generation for recurring requests, which is part of the cross-chain payment integration.
103-103: Invoice number integration looks good.The generated invoice number is correctly included in the data being inserted into the
requestTable, fulfilling the schema requirement.
MantisClone
left a comment
MantisClone
left a comment
There was a problem hiding this comment.
I don't want to hold this up. Approved 👍 but I'll do some testing on it once it's in staging.
|
I looked at the new screenshots in the Description. Looks good 👍 |
…t structure in payment section
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/components/payment-section.tsx (3)
159-171:⚠️ Potential issueNetwork switching logic lacks error handling.
The network switching logic doesn't handle potential errors if the chain ID isn't found in the mappings or if
switchNetworkfails.Implement proper error handling:
const targetChain = CHAIN_TO_ID[selectedRoute?.chain as keyof typeof CHAIN_TO_ID]; +if (!targetChain) { + toast.error("Network error", { + description: `Unsupported network: ${selectedRoute?.chain}`, + }); + setPaymentProgress("idle"); + return; +} if (targetChain !== chainId) { const targetAppkitNetwork = ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; + if (!targetAppkitNetwork) { + toast.error("Network error", { + description: `No AppKit configuration for network: ${selectedRoute?.chain}`, + }); + setPaymentProgress("idle"); + return; + } toast("Switching to network", { description: `Switching to ${targetAppkitNetwork.name} network`, }); - await switchNetwork(targetAppkitNetwork); + try { + await switchNetwork(targetAppkitNetwork); + } catch (error) { + console.error("Error switching network:", error); + toast.error("Network switch failed", { + description: "Could not switch to the required network", + }); + setPaymentProgress("idle"); + return; + } }
195-253: 🛠️ Refactor suggestionComplex Paygrid payment flow needs refactoring.
The Paygrid payment flow is complex with nested conditionals and multiple steps, making it hard to maintain.
Extract this logic into a separate function:
+// Helper function for Paygrid payments +const handlePaygridPayment = async (paymentData: any, signer: any) => { + const paymentIntent = JSON.parse(paymentData.paymentIntent); + const supportsEIP2612 = paymentData.metadata.supportsEIP2612; + let approvalSignature = undefined; + let approval = undefined; + + setPaymentProgress("approving"); + + if (supportsEIP2612) { + approval = JSON.parse(paymentData.approvalPermitPayload); + + approvalSignature = await signer._signTypedData( + approval.domain, + approval.types, + approval.values, + ); + } else { + const tx = await signer.sendTransaction(paymentData.approvalCalldata); + await tx.wait(); + } + + const paymentIntentSignature = await signer._signTypedData( + paymentIntent.domain, + paymentIntent.types, + paymentIntent.values, + ); + + const signedPermit = { + signedPaymentIntent: { + signature: paymentIntentSignature, + nonce: paymentIntent.values.nonce.toString(), + deadline: paymentIntent.values.deadline.toString(), + }, + signedApprovalPermit: approvalSignature + ? { + signature: approvalSignature, + nonce: approval.values.nonce.toString(), + deadline: approval?.values?.deadline + ? approval.values.deadline.toString() + : approval.values.expiry.toString(), + } + : undefined, + }; + + setPaymentProgress("paying"); + + await sendPaymentIntent({ + paymentIntent: paymentData.paymentIntentId, + payload: signedPermit, + }); + + setPaymentStatus("processing"); + + toast("Payment is being processed", { + description: "You can safely close this page.", + }); +}; // In the handlePayment function if (isPaygrid) { - const paymentIntent = JSON.parse(paymentData.paymentIntent); - const supportsEIP2612 = paymentData.metadata.supportsEIP2612; - let approvalSignature = undefined; - let approval = undefined; - - setPaymentProgress("approving"); - - if (supportsEIP2612) { - approval = JSON.parse(paymentData.approvalPermitPayload); - - approvalSignature = await signer._signTypedData( - approval.domain, - approval.types, - approval.values, - ); - } else { - const tx = await signer.sendTransaction(paymentData.approvalCalldata); - await tx.wait(); - } - - const paymentIntentSignature = await signer._signTypedData( - paymentIntent.domain, - paymentIntent.types, - paymentIntent.values, - ); - - const signedPermit = { - signedPaymentIntent: { - signature: paymentIntentSignature, - nonce: paymentIntent.values.nonce.toString(), - deadline: paymentIntent.values.deadline.toString(), - }, - signedApprovalPermit: approvalSignature - ? { - signature: approvalSignature, - nonce: approval.values.nonce.toString(), - deadline: approval?.values?.deadline - ? approval.values.deadline.toString() - : approval.values.expiry.toString(), - } - : undefined, - }; - - setPaymentProgress("paying"); - - await sendPaymentIntent({ - paymentIntent: paymentData.paymentIntentId, - payload: signedPermit, - }); - - setPaymentStatus("processing"); - - toast("Payment is being processed", { - description: "You can safely close this page.", - }); + await handlePaygridPayment(paymentData, signer); return; }
534-544:⚠️ Potential issueAdd a fallback for null selectedRoute.
The component doesn't handle the case when
selectedRouteis null but is still referenced.Add proper conditional rendering:
{/* Selected Route Preview */} -{selectedRoute && ( +{selectedRoute ? ( <PaymentRoute route={selectedRoute} isSelected={true} variant="selected" routeType={getRouteType( selectedRoute, invoiceChain, )} /> +) : ( + <div className="p-4 border border-dashed rounded-lg text-center text-zinc-500"> + No payment route selected + </div> )}
🧹 Nitpick comments (1)
src/components/payment-section.tsx (1)
28-32: Utility function appears robust but lacks edge case handling.The
getCurrencyChainfunction extracts the chain from formatted strings like "USDC-base", but it doesn't validate that the extracted chain is actually supported.Consider enhancing this with validation:
const getCurrencyChain = (currency: string) => { // Extract chain from format like "USDC-base" const parts = currency.split("-"); - return parts.length > 1 ? parts[1].toLowerCase() : null; + const chain = parts.length > 1 ? parts[1].toLowerCase() : null; + // Validate that the chain is supported + return chain && Object.keys(REQUEST_NETWORK_CHAIN_TO_PAYMENT_NETWORK).includes(chain) ? chain : null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/sepolia-logo.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/components/payment-section.tsx(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (13)
src/components/payment-section.tsx (13)
7-9: Good addition of blockchain-related imports.The import statements for chains, currencies, and types are well-structured and appropriate for the cross-chain payment feature.
34-40: Well-structured chain mapping.The mapping between Request Network chains and payment networks is clear and well-defined.
42-74: Route type determination logic is comprehensive.The
getRouteTypefunction handles different payment scenarios well, distinguishing between direct, same-chain, and cross-chain payments.
77-84: Good state management setup.The component properly sets up state variables for AppKit integration and payment route management.
91-105: Query setup follows best practices.The API query for payment routes is well structured with appropriate query parameters and enablement condition.
136-146: Effective use of useEffect for route management.The effects appropriately trigger route refetching when the wallet address changes and set the selected route when routes are loaded.
180-191: Payment request parameters are well structured.The payment request includes all necessary parameters based on the selected route.
246-250: Update to "processing" addresses feedback correctly.This implementation correctly sets the payment status to "processing" rather than "paid" immediately after submission, following the previous review comment.
340-357: Excellent security warning for mainnet transactions.The warning banner for real cryptocurrency transactions is well-designed and clearly communicates the risks to users.
364-364: Number formatting improvement.Using
.toString()instead of.toFixed(2)is a good change for cryptocurrency display, as it preserves the exact value without rounding.
487-565: Payment route selection UI is comprehensive.The UI for route selection handles loading states, empty states, and multiple routes very well.
571-575: Good button state management.The button is appropriately disabled when there are no routes, when payment is in progress, or when the payment is already processing.
577-587: Improved user feedback during payment.The conditional rendering for the button text provides clear feedback to users about the current state of the payment process.
…d-crosschain-payments
… Paygrid and direct payments, enhancing code organization and readability
…n-payments' of github.com:RequestNetwork/easy-invoice into 36-easyinvoice---integrate-payment-routes-and-crosschain-payments
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/payment-section.tsx (1)
254-271: 🛠️ Refactor suggestionImprove network switching error handling.
The network switching logic doesn't handle potential errors if the chain ID isn't found in the mappings.
const targetChain = CHAIN_TO_ID[selectedRoute?.chain as keyof typeof CHAIN_TO_ID]; +if (!targetChain) { + toast.error("Network error", { + description: `Unsupported network: ${selectedRoute?.chain}`, + }); + setPaymentProgress("idle"); + return; +} if (targetChain !== chainId) { const targetAppkitNetwork = ID_TO_APPKIT_NETWORK[targetChain as keyof typeof ID_TO_APPKIT_NETWORK]; + if (!targetAppkitNetwork) { + toast.error("Network error", { + description: `No AppKit configuration for network: ${selectedRoute?.chain}`, + }); + setPaymentProgress("idle"); + return; + } toast("Switching to network", { description: `Switching to ${targetAppkitNetwork.name} network`, }); try { await switchNetwork(targetAppkitNetwork); } catch (_) { toast("Error switching network"); + setPaymentProgress("idle"); return; } }
🧹 Nitpick comments (1)
src/components/payment-section.tsx (1)
154-210: Good extraction of Paygrid payment handling.Extracting the Paygrid payment logic into a separate function improves code organization and readability. The function handles the complex flow of approval permissions and payment intent signing in a structured way.
One minor suggestion would be to add more specific error handling for the different steps in this process.
Consider adding more specific error handling:
const handlePaygridPayments = async (paymentData: any, signer: any) => { try { const paymentIntent = JSON.parse(paymentData.paymentIntent); const supportsEIP2612 = paymentData.metadata.supportsEIP2612; let approvalSignature = undefined; let approval = undefined; setPaymentProgress("approving"); if (supportsEIP2612) { approval = JSON.parse(paymentData.approvalPermitPayload); approvalSignature = await signer._signTypedData( approval.domain, approval.types, approval.values, ); } else { const tx = await signer.sendTransaction(paymentData.approvalCalldata); await tx.wait(); } const paymentIntentSignature = await signer._signTypedData( paymentIntent.domain, paymentIntent.types, paymentIntent.values, ); const signedPermit = { signedPaymentIntent: { signature: paymentIntentSignature, nonce: paymentIntent.values.nonce.toString(), deadline: paymentIntent.values.deadline.toString(), }, signedApprovalPermit: approvalSignature ? { signature: approvalSignature, nonce: approval.values.nonce.toString(), deadline: approval?.values?.deadline ? approval.values.deadline.toString() : approval.values.expiry.toString(), } : undefined, }; setPaymentProgress("paying"); await sendPaymentIntent({ paymentIntent: paymentData.paymentIntentId, payload: signedPermit, }); setPaymentStatus("processing"); toast("Payment is being processed", { description: "You can safely close this page.", }); + } catch (error) { + console.error("Paygrid payment error:", error); + if (error.code === 4001) { + toast.error("Payment cancelled", { + description: "You cancelled the transaction in your wallet", + }); + } else { + toast.error("Payment approval failed", { + description: "Please try again or select a different payment route", + }); + } + throw error; // Rethrow to be handled by the main try/catch + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/payment-section.tsx(6 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/components/payment-section.tsx (4)
src/lib/chains.ts (2)
CHAIN_TO_ID(10-17)ID_TO_APPKIT_NETWORK(19-26)src/lib/currencies.ts (1)
MAINNET_CURRENCIES(1-12)src/components/payment-route.tsx (1)
PaymentRoute(19-117)src/lib/types/index.ts (1)
PaymentRoute(1-8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (14)
src/components/payment-section.tsx (14)
28-32: Good utility function for chain extraction.This function correctly extracts the chain information from the currency format string, which is essential for routing payments to the appropriate blockchain network. Nice clean implementation.
34-40: Well-structured mapping for network standardization.The mapping from Request Network chains to payment networks ensures consistent usage of network identifiers throughout the application. This is a good practice for maintaining standardization.
42-74: Good implementation of route type identification.The
getRouteTypefunction effectively categorizes payment routes into "direct", "same-chain-erc20", and "cross-chain" types, which helps provide relevant context to users. The descriptive labels and descriptions improve user understanding of each payment option.
81-84: Route management state variables added correctly.The addition of state variables for route management allows users to toggle between different payment routes. This is important for giving users flexibility in how they want to process their payments.
91-105: Enhanced API integration for payment routes.The updated API integration now correctly fetches payment routes based on the user's wallet address, which enables personalized payment options. Good use of the
enabledoption to prevent unnecessary queries.
136-146: Proper state management for payment routes.These two useEffect hooks ensure that:
- Payment routes are refetched when the wallet address changes
- The first available route is automatically selected when routes are loaded
This creates a seamless experience for the user without requiring manual intervention.
212-247: Good extraction of direct payment handling.Similar to the Paygrid payment handling, extracting direct payment logic improves code organization. The function properly handles the approval process when needed and provides appropriate user feedback through toast notifications.
279-299: Improved payment request with route parameters.The payment request now includes essential parameters from the selected route (chain and token), which enables routing the payment through the appropriate blockchain network. This is a key part of the cross-chain payment functionality.
352-369: Good mainnet currency warning.Adding a warning for real cryptocurrency transactions is a crucial safety feature. This helps users understand that they're working with actual value on mainnet blockchains and that transactions are irreversible.
376-376: Using toString() instead of toFixed() improves numerical accuracy.Replacing toFixed(2) with toString() prevents rounding errors and maintains the exact numerical value, which is important for cryptocurrency amounts where precision matters.
499-577: Well-designed payment route selection UI.The payment route selection UI provides clear feedback to users with loading states, error states, and an intuitive interface for selecting different payment routes. The implementation correctly handles all cases: loading, no routes available, single route, and multiple routes.
583-587: Good button state management.The payment button is correctly disabled when there are no routes available, when a payment is in progress, or when a payment is already being processed. This prevents users from initiating invalid or duplicate payment attempts.
589-598: Clear payment button states.The payment button text changes appropriately based on the current state, providing clear feedback to users about what's happening or what they need to do next.
205-209: Implement processing state based on previous feedback.This implementation addresses MantisClone's previous feedback about displaying a "processing" state immediately after the user submits the payment, rather than marking it as "paid" right away.
aimensahnoun
deleted the
36-easyinvoice---integrate-payment-routes-and-crosschain-payments
branch
Resolves #36
Cross-Chain Mainnet Payment Support and UI Improvements
This PR enables support for cross-chain cryptocurrency payments using real funds and improves the user experience by adding appropriate warnings and UI enhancements.
Changes:
Screenshots:
Real funds warning at invoice creation:
Real funds warning at payment time:
Payment Routes:
Status after payment:
Summary by CodeRabbit
Summary by CodeRabbit