fix(quotes): harden quote -> rank -> pay pipeline across all providers#37
Conversation
…ins before fan-out
…generic no-routes
…TP false-success hash
…r non-EVM sources
|
@Shreyassp002 is attempting to deploy a commit to the Shreyas' projects Team on Vercel. A member of the Team first needs to authorize it. |
⚡ Flash Review
See inline comments for details. ⚡ Powered by Flash Review
|
| { event: 'transaction/poll' }, | ||
| async ({ event, step }) => { | ||
| const { transactionId, txHash, fromChainId, toChainId, bridge, provider: providerName, requestId, depositAddress, attempt = 1 } = event.data | ||
| const { transactionId, txHash, fromChainId, toChainId, bridge, provider: providerName, requestId, depositAddress, depositMemo, attempt = 1 } = event.data |
There was a problem hiding this comment.
⚡ Flash Review
🚨 Security: The new depositMemo field is destructured from event.data and subsequently passed to a cross-chain provider. If this data originates from user input (e.g., during payment link creation or customer payment initiation), it must be thoroughly validated to prevent potential injection attacks or unexpected behavior when interacting with external provider APIs.
Fix: Ensure depositMemo is explicitly defined and validated within the Zod schema for the transaction/poll Inngest event. For instance, use z.string().max(256).optional() if it's an optional string memo, or z.string().min(1).max(256) if required.
| import { PublicKey } from '@solana/web3.js' | ||
| import { resolveExecutionPath, isCrossChainSwap } from '@/lib/execution-path' | ||
| import { PublicKey, VersionedTransaction, Transaction } from '@solana/web3.js' | ||
|
|
There was a problem hiding this comment.
⚡ Flash Review
🚨 Security: Hardcoding a fallback API key directly in the client-side bundle is a severe security risk. This key can be easily extracted by anyone inspecting the client-side code, potentially leading to abuse of the Rango API or rate limit exhaustion.
Fix: Remove the hardcoded fallback. The NEXT_PUBLIC_RANGO_API_KEY should only be sourced from environment variables. If it's missing, the application should fail gracefully or prompt for configuration, not expose a default key.
| } else { | ||
| // Fallback if SDK doesn't return explicit hash in expected place | ||
| setStatus('completed') | ||
| return '0x' |
There was a problem hiding this comment.
⚡ Flash Review
🐛 Bug: Adding memo to submitDepositTx is a critical improvement. For certain chains (e.g., Cosmos, XRP, Stellar), a deposit memo is essential for correctly attributing funds to the user's account. Without it, funds could be lost or significantly delayed.
Fix: This change is a fix. Ensure quote.metadata?.depositMemo is correctly populated by the quote aggregator when required.
| // 1. Extract Deposit Address | ||
| const depositAddress = | ||
| quote.transactionRequest?.depositAddress || quote.metadata?.depositAddress | ||
| if (!depositAddress) throw new Error('Deposit address missing for Near Intents') |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: The 1.0 slippage value is a magic number. While it's a common default, it's better to define it as a named constant (e.g., DEFAULT_RANGO_SLIPPAGE_PERCENT) for clarity and easier modification.
Fix: Define const DEFAULT_RANGO_SLIPPAGE_PERCENT = 1.0 and use it here.
| disableEstimate: false | ||
| } | ||
| // 2. Call Swap API (Step 1) | ||
| // We pass the full request to get the tx data |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: The // @ts-ignore comment indicates a type safety issue. Relying on @ts-ignore can hide potential bugs and makes the code harder to maintain. The statusRes object should have a properly defined type that accurately reflects its structure, including the status property.
Fix: Define a proper TypeScript interface or type for statusRes that accurately reflects its structure, including the status property, and remove the @ts-ignore.
This audits and fixes the quote -> rank -> pay pipeline across all six providers (LiFi, Rango, Rubic, Symbiosis, NEAR, CCTP). Each provider fix was verified against the live provider APIs, not just by reading code, and everything is unit-tested. Gates on the final commit: tsc 0 errors, vitest 185 passing, build compiles.
What this changes