From a7d2827cc1ef735d65ad9d1f69a8fcf4856e6756 Mon Sep 17 00:00:00 2001 From: Timidan Date: Sun, 23 Aug 2026 02:00:47 +0100 Subject: [PATCH] fix(intents): close funds-handling gaps in the escrow order flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six defects found while reviewing the intents integration before merge. The first three can cost users funds; the rest remove footguns. - Reject non-positive quote outputs. `amount` is a decimal string, so the `!previewAmount` guard let "0" through and built an order offering the whole input for nothing, which a solver can fill by transferring zero. Adds readQuoteOutputAmount() and uses it in all three quote paths. - Bind the order to the signing account. open() collects from msg.sender but delivers and refunds to order.user; the reset effects keyed on the `recipient` prop and not the connected address, so switching accounts after quoting let one account fund an order that pays another. Adds `address` to the reset deps and asserts the two match before signing. - Parse validUntil by shape. It arrives as a numeric Unix timestamp but was typed as a string and fed to Date.parse, which returns NaN for numeric input in either unit — the real expiry was discarded and replaced by an arbitrary now+15m. - Revalidate the fill window immediately before open(). Deadlines were fixed at quote time and never rechecked, and legs open sequentially, so a later leg could escrow into an order no solver would fill. - Record the open() hash before awaiting its receipt, on both the step components and the concierge pipeline. A receipt timeout on a tx that later mined left the escrow invisible, and retry minted a fresh nonce — two live orders, both fillable. Retry now refuses while a hash is held. - Approve the requested amount instead of MaxUint256 in the composer helper, and stop treating an unreadable allowance as zero, which skipped the USDT-style reset and sent a reverting approve. --- .../lifi-earn/IntentBridgeStep.tsx | 52 ++++++++++++++++--- .../lifi-earn/WithdrawIntentRouteStep.tsx | 44 +++++++++++++--- .../concierge/intent/useIntentLegPipeline.ts | 44 +++++++++++++--- .../lifi-earn/crossChainComposerDeposit.ts | 14 +++-- .../integrations/lifi-earn/intentsApi.ts | 19 ++++++- src/lib/intents/deadlines.ts | 52 +++++++++++++++---- 6 files changed, 189 insertions(+), 36 deletions(-) diff --git a/src/components/integrations/lifi-earn/IntentBridgeStep.tsx b/src/components/integrations/lifi-earn/IntentBridgeStep.tsx index d167e053..649804a9 100644 --- a/src/components/integrations/lifi-earn/IntentBridgeStep.tsx +++ b/src/components/integrations/lifi-earn/IntentBridgeStep.tsx @@ -24,13 +24,17 @@ import { requestIntentQuote, isDeliveredOrSettled, readDestinationTxHash, + readQuoteOutputAmount, type IntentQuote, } from "./intentsApi"; import { fetchComposerQuote } from "./earnApi"; import { IntentStatusTimeline } from "./IntentStatusTimeline"; import { useIntentOrderStatus } from "./useIntentOrderStatus"; import { encodeEip7930EvmAddress } from "../../../lib/intents/eip7930"; -import { buildDeadlinePlan } from "../../../lib/intents/deadlines"; +import { + buildDeadlinePlan, + assertFillWindowOpen, +} from "../../../lib/intents/deadlines"; import { nextOrderNonce } from "../../../lib/intents/nonce"; import { buildStandardOrder, @@ -134,7 +138,17 @@ export function IntentBridgeStep({ setDepositError(null); lastIntentStatusEventRef.current = null; lastDeliveredEventRef.current = null; - }, [sourceChainId, sourceToken.address, sourceAmountRaw, vault.address, recipient]); + // `address` matters: the order records the connected account as escrow + // depositor and refund payee, so a stale quote must not survive an account + // switch. + }, [ + sourceChainId, + sourceToken.address, + sourceAmountRaw, + vault.address, + recipient, + address, + ]); const explorerByChain = useMemo(() => { const map = new Map(); @@ -200,6 +214,14 @@ export function IntentBridgeStep({ async function handleQuote() { if (!recipientAddr || !outputToken) return; + // A broadcast open() may still mine after its receipt wait timed out. + // Re-quoting mints a fresh nonce, so both orders could fill. + if (openTxHash) { + setError( + "An order was already broadcast for this quote. Check that transaction before starting a new one — opening again could escrow your funds twice.", + ); + return; + } try { setStage("quoting"); setError(null); @@ -229,13 +251,13 @@ export function IntentBridgeStep({ }); const q = res.quotes?.[0]; - const previewAmount = q?.preview?.outputs?.[0]?.amount; - if (!q || !previewAmount) { + const previewAmount = readQuoteOutputAmount(q); + if (!q || previewAmount === null) { throw new Error("No quote available for this route"); } const deadlines = buildDeadlinePlan({ - quoteValidUntilIso: q.validUntil ?? null, + quoteValidUntil: q.validUntil ?? null, }); const built = buildStandardOrder({ @@ -246,7 +268,7 @@ export function IntentBridgeStep({ inputAmount: BigInt(sourceAmountRaw), targetChainId: vault.chainId, outputToken: outputToken.address as Address, - outputAmount: BigInt(previewAmount), + outputAmount: previewAmount, recipient: recipientAddr, expires: deadlines.expires, fillDeadline: deadlines.fillDeadline, @@ -273,6 +295,18 @@ export function IntentBridgeStep({ }); if (!walletClient) throw new Error("No wallet client for source chain"); + // open() collects from msg.sender but delivers and refunds to order.user. + // If they diverge, the signer funds an order that pays someone else. + if ( + walletClient.account.address.toLowerCase() !== order.user.toLowerCase() + ) { + throw new Error( + "The connected account changed after this quote was built — request a new quote before opening the order.", + ); + } + + assertFillWindowOpen(order.fillDeadline); + // Snapshot the destination underlying balance BEFORE we open the order. // CRITICAL: a failed pre-read must HARD-FAIL — otherwise the post-fill // delta calculation can't distinguish solver-delivered tokens from the @@ -322,6 +356,9 @@ export function IntentBridgeStep({ phase: "intent-open", txHash: hash, }); + // Record before waiting: a receipt timeout on a tx that later mines must + // not leave the escrow invisible, or retry would open a second order. + setOpenTxHash(hash); const receipt = await wagmiWaitForReceipt(config, { hash, chainId: sourceChainId, @@ -331,7 +368,6 @@ export function IntentBridgeStep({ throw new Error("open() reverted on-chain"); } - setOpenTxHash(hash); const decodedOrderId = extractOpenOrderId(receipt.logs); if (!decodedOrderId) { // Without an orderId we can't poll status; fail loudly instead of @@ -818,7 +854,7 @@ export function IntentBridgeStep({ size="sm" className="h-8 w-full gap-1 text-xs" onClick={handleQuote} - disabled={!isConnected} + disabled={!isConnected || !!openTxHash} > Retry intent quote diff --git a/src/components/integrations/lifi-earn/WithdrawIntentRouteStep.tsx b/src/components/integrations/lifi-earn/WithdrawIntentRouteStep.tsx index 25930299..6dd76b32 100644 --- a/src/components/integrations/lifi-earn/WithdrawIntentRouteStep.tsx +++ b/src/components/integrations/lifi-earn/WithdrawIntentRouteStep.tsx @@ -25,12 +25,16 @@ import { IntentStatusTimeline } from "./IntentStatusTimeline"; import { isDeliveredOrSettled, readDestinationTxHash, + readQuoteOutputAmount, requestIntentQuote, type IntentQuote, } from "./intentsApi"; import { useIntentOrderStatus } from "./useIntentOrderStatus"; import { encodeEip7930EvmAddress } from "../../../lib/intents/eip7930"; -import { buildDeadlinePlan } from "../../../lib/intents/deadlines"; +import { + buildDeadlinePlan, + assertFillWindowOpen, +} from "../../../lib/intents/deadlines"; import { nextOrderNonce } from "../../../lib/intents/nonce"; import { buildStandardOrder, @@ -111,6 +115,9 @@ export function WithdrawIntentRouteStep({ setOpenTxHash(null); setOrderId(null); deliveredNotifiedRef.current = false; + // `address` matters: the order records the connected account as escrow + // depositor and refund payee, so a stale quote must not survive an account + // switch. }, [ sourceChainId, sourceToken.address, @@ -118,6 +125,7 @@ export function WithdrawIntentRouteStep({ destinationChainId, destinationToken.address, recipient, + address, ]); useEffect(() => { @@ -151,6 +159,14 @@ export function WithdrawIntentRouteStep({ async function handleQuote() { if (!recipientAddr) return; + // A broadcast open() may still mine after its receipt wait timed out. + // Re-quoting mints a fresh nonce, so both orders could fill. + if (openTxHash) { + setError( + "An order was already broadcast for this quote. Check that transaction before starting a new one — opening again could escrow your funds twice.", + ); + return; + } try { setStage("quoting"); setError(null); @@ -189,13 +205,13 @@ export function WithdrawIntentRouteStep({ }); const q = res.quotes?.[0]; - const previewAmount = q?.preview?.outputs?.[0]?.amount; - if (!q || !previewAmount) { + const previewAmount = readQuoteOutputAmount(q); + if (!q || previewAmount === null) { throw new Error("No intent quote available for this receive route"); } const deadlines = buildDeadlinePlan({ - quoteValidUntilIso: q.validUntil ?? null, + quoteValidUntil: q.validUntil ?? null, }); const built = buildStandardOrder({ @@ -206,7 +222,7 @@ export function WithdrawIntentRouteStep({ inputAmount: BigInt(sourceAmountRaw), targetChainId: destinationChainId, outputToken: destinationToken.address as Address, - outputAmount: BigInt(previewAmount), + outputAmount: previewAmount, recipient: recipientAddr, expires: deadlines.expires, fillDeadline: deadlines.fillDeadline, @@ -234,6 +250,18 @@ export function WithdrawIntentRouteStep({ }); if (!walletClient) throw new Error("No wallet client for source chain"); + // open() collects from msg.sender but delivers and refunds to order.user. + // If they diverge, the signer funds an order that pays someone else. + if ( + walletClient.account.address.toLowerCase() !== order.user.toLowerCase() + ) { + throw new Error( + "The connected account changed after this quote was built — request a new quote before opening the order.", + ); + } + + assertFillWindowOpen(order.fillDeadline); + setStage("approving"); await safeApproveErc20({ wagmiConfig: config, @@ -255,6 +283,9 @@ export function WithdrawIntentRouteStep({ to: INPUT_SETTLER_ESCROW, data: openData, }); + // Record before waiting: a receipt timeout on a tx that later mines must + // not leave the escrow invisible, or retry would open a second order. + setOpenTxHash(hash); const receipt = await wagmiWaitForReceipt(config, { hash, chainId: sourceChainId, @@ -264,7 +295,6 @@ export function WithdrawIntentRouteStep({ throw new Error("open() reverted on-chain"); } - setOpenTxHash(hash); const decodedOrderId = extractOpenOrderId(receipt.logs); if (!decodedOrderId) { throw new Error( @@ -509,7 +539,7 @@ export function WithdrawIntentRouteStep({ size="sm" className="h-8 w-full gap-1 text-xs" onClick={handleQuote} - disabled={!isConnected} + disabled={!isConnected || !!openTxHash} > Retry route diff --git a/src/components/integrations/lifi-earn/concierge/intent/useIntentLegPipeline.ts b/src/components/integrations/lifi-earn/concierge/intent/useIntentLegPipeline.ts index 38160a81..4822dcde 100644 --- a/src/components/integrations/lifi-earn/concierge/intent/useIntentLegPipeline.ts +++ b/src/components/integrations/lifi-earn/concierge/intent/useIntentLegPipeline.ts @@ -15,11 +15,15 @@ import { import { useConfig } from "wagmi"; import { requestIntentQuote, + readQuoteOutputAmount, type IntentQuote, } from "../../intentsApi"; import { fetchComposerQuote } from "../../earnApi"; import { encodeEip7930EvmAddress } from "../../../../../lib/intents/eip7930"; -import { buildDeadlinePlan } from "../../../../../lib/intents/deadlines"; +import { + buildDeadlinePlan, + assertFillWindowOpen, +} from "../../../../../lib/intents/deadlines"; import { nextOrderNonce } from "../../../../../lib/intents/nonce"; import { buildStandardOrder, @@ -31,7 +35,7 @@ import { extractOpenOrderId, inputSettlerEscrowAbi, } from "../../../../../lib/intents/contracts"; -import { safeApproveErc20 } from "../../txUtils"; +import { safeApproveErc20, formatTxError } from "../../txUtils"; import type { IntentLegSpec } from "./intentLegs"; // Quote requests fan out in parallel; on-chain open() runs sequentially — @@ -158,17 +162,17 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { return { ...run, status: "failed", error: "No quote returned" }; } - const previewAmount = quote.preview?.outputs?.[0]?.amount; - if (!previewAmount) { + const previewAmount = readQuoteOutputAmount(quote); + if (previewAmount === null) { return { ...run, status: "failed", - error: "Quote missing preview output amount", + error: "Quote returned no usable output amount", }; } const deadlines = buildDeadlinePlan({ - quoteValidUntilIso: quote.validUntil ?? null, + quoteValidUntil: quote.validUntil ?? null, }); const order = buildStandardOrder({ @@ -179,7 +183,7 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { inputAmount: BigInt(spec.source.amountRaw), targetChainId: spec.destination.chainId, outputToken: spec.destination.outputToken, - outputAmount: BigInt(previewAmount), + outputAmount: previewAmount, recipient: spec.destination.recipient, expires: deadlines.expires, fillDeadline: deadlines.fillDeadline, @@ -235,6 +239,9 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { async (run: IntentLegRun): Promise => { if (!run.order) return run; const chainId = run.spec.source.chainId; + // Held outside the try so a receipt-wait timeout still reports the + // broadcast hash instead of losing the escrow. + let broadcastHash: Hex | undefined; try { const currentChain = wagmiGetAccount(config).chainId; @@ -246,6 +253,16 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { if (!walletClient) throw new Error("No wallet client for source chain"); const walletAddress = walletClient.account.address as Address; + // open() collects from msg.sender but delivers and refunds to + // order.user. If they diverge, the signer funds someone else's order. + if (walletAddress.toLowerCase() !== run.order.user.toLowerCase()) { + throw new Error( + "The connected account changed after this quote was built — re-quote this leg before opening it.", + ); + } + + assertFillWindowOpen(run.order.fillDeadline); + // Snapshot destination-chain balance of the underlying so the // post-delivery deposit step can use the actual delta. CRITICAL: // a failed read must HARD-FAIL — otherwise the post-fill delta @@ -290,6 +307,7 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { to: INPUT_SETTLER_ESCROW, data: openData, }); + broadcastHash = openHash; const receipt = await wagmiWaitForReceipt(config, { hash: openHash, chainId, @@ -323,7 +341,8 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { return { ...run, status: "failed", - error: err instanceof Error ? err.message : String(err), + openTxHash: broadcastHash ?? run.openTxHash, + error: formatTxError(err), }; } }, @@ -349,6 +368,15 @@ export function useIntentLegPipeline(): UseIntentLegPipelineReturn { if (!walletAddress) return; const current = runsRef.current.find((r) => r.spec.id === id); if (!current) return; + // A broadcast open() may still mine after its receipt wait timed out. + // Re-quoting mints a fresh nonce, so both orders could fill. + if (current.openTxHash) { + patch(id, { + error: + "An order was already broadcast for this leg. Check that transaction before retrying — opening again could escrow your funds twice.", + }); + return; + } patch(id, { status: "quoting", error: undefined }); const next = await quoteOne( { ...current, status: "quoting" }, diff --git a/src/components/integrations/lifi-earn/crossChainComposerDeposit.ts b/src/components/integrations/lifi-earn/crossChainComposerDeposit.ts index 8f318055..84c02fc7 100644 --- a/src/components/integrations/lifi-earn/crossChainComposerDeposit.ts +++ b/src/components/integrations/lifi-earn/crossChainComposerDeposit.ts @@ -266,7 +266,7 @@ async function approveWithReset(args: { const data = APPROVE_ABI.encodeFunctionData("approve", [ spender, - ethers.constants.MaxUint256, + amount, ]) as `0x${string}`; const hash = await walletClient.sendTransaction({ to: tokenAddress as `0x${string}`, @@ -465,7 +465,11 @@ export async function executeCrossChainComposerDeposit( sourceChainId, ); } catch { - currentAllowance = ethers.BigNumber.from(0); + // Treating an unreadable allowance as 0 would skip the USDT-style + // reset and send a nonzero-to-nonzero approve that reverts. + throw new Error( + "Couldn't read the current token allowance — refusing to approve blindly. Try again in a moment.", + ); } if (currentAllowance.lt(sourceAmountBN)) { onStateChange({ @@ -705,7 +709,11 @@ export async function executeCrossChainComposerDeposit( vault.chainId, ); } catch { - currentAllowance = ethers.BigNumber.from(0); + // Treating an unreadable allowance as 0 would skip the USDT-style + // reset and send a nonzero-to-nonzero approve that reverts. + throw new Error( + "Couldn't read the current token allowance — refusing to approve blindly. Try again in a moment.", + ); } if (currentAllowance.lt(depositAmountBN)) { onStateChange({ diff --git a/src/components/integrations/lifi-earn/intentsApi.ts b/src/components/integrations/lifi-earn/intentsApi.ts index 82a13603..57e01a11 100644 --- a/src/components/integrations/lifi-earn/intentsApi.ts +++ b/src/components/integrations/lifi-earn/intentsApi.ts @@ -39,7 +39,8 @@ export interface IntentQuote { }; /** Pass straight into outputs[].context for auction/limit handling. */ context?: Hex; - validUntil?: string; + /** Unix timestamp (seconds) in practice; ISO strings have also been seen. */ + validUntil?: string | number; solver?: string; [key: string]: unknown; } @@ -49,6 +50,22 @@ export interface IntentQuoteResponse { [key: string]: unknown; } +// `amount` is a decimal string, so a plain falsy check lets "0" through and +// builds an order that offers the whole input for nothing. Returns null for +// missing, unparseable, or non-positive amounts. +export function readQuoteOutputAmount( + quote: IntentQuote | null | undefined, +): bigint | null { + const raw = quote?.preview?.outputs?.[0]?.amount; + if (raw === null || raw === undefined) return null; + try { + const parsed = BigInt(raw); + return parsed > 0n ? parsed : null; + } catch { + return null; + } +} + // LI.FI surfaces tx hashes and solver under `meta.*`; older shapes (and our // previous typing) put them at the top level. Readers fall back to either. export interface IntentOrderStatus { diff --git a/src/lib/intents/deadlines.ts b/src/lib/intents/deadlines.ts index 1e618495..3f64a210 100644 --- a/src/lib/intents/deadlines.ts +++ b/src/lib/intents/deadlines.ts @@ -8,22 +8,56 @@ export interface DeadlinePlan { } interface DeadlineInput { - quoteValidUntilIso?: string | null; + quoteValidUntil?: string | number | null; nowMs?: number; maxFillTtlSec?: number; refundGraceSec?: number; } +// Minimum window we require between "now" and the fill cutoff, both when the +// plan is built and again immediately before open() is signed. +export const MIN_FILL_WINDOW_SEC = 30; + +// The API documents `validUntil` as a numeric Unix timestamp, but has shipped +// ISO strings too. Date.parse returns NaN for numeric input in either unit, so +// dispatch on shape rather than feeding everything to Date.parse. +export function parseQuoteValidUntil( + value: string | number | null | undefined, +): number | null { + if (value === null || value === undefined) return null; + + const asNumber = + typeof value === "number" + ? value + : /^\d+$/.test(value.trim()) + ? Number(value.trim()) + : null; + + if (asNumber !== null) { + if (!Number.isFinite(asNumber) || asNumber <= 0) return null; + // Anything beyond ~year 5138 in seconds is really milliseconds. + return Math.floor(asNumber > 1e11 ? asNumber / 1000 : asNumber); + } + + const parsed = Date.parse(String(value)); + return Number.isFinite(parsed) ? Math.floor(parsed / 1000) : null; +} + +// Throws if the quote's fill window has closed (or is about to) since the plan +// was built. Callers must run this immediately before signing open(). +export function assertFillWindowOpen(fillDeadline: number, nowMs?: number): void { + const nowSec = Math.floor((nowMs ?? Date.now()) / 1000); + if (fillDeadline <= nowSec + MIN_FILL_WINDOW_SEC) { + throw new Error( + "This quote expired before the order was opened — request a new quote.", + ); + } +} + export function buildDeadlinePlan(args: DeadlineInput = {}): DeadlinePlan { const nowSec = Math.floor((args.nowMs ?? Date.now()) / 1000); - let quoteValidUntilSec: number | null = null; - if (args.quoteValidUntilIso) { - const parsed = Date.parse(args.quoteValidUntilIso); - if (Number.isFinite(parsed)) { - quoteValidUntilSec = Math.floor(parsed / 1000); - } - } + const quoteValidUntilSec = parseQuoteValidUntil(args.quoteValidUntil); const maxFillTtl = args.maxFillTtlSec ?? 15 * 60; const grace = args.refundGraceSec ?? 30 * 60; @@ -32,7 +66,7 @@ export function buildDeadlinePlan(args: DeadlineInput = {}): DeadlinePlan { ? Math.min(quoteValidUntilSec, nowSec + maxFillTtl) : nowSec + maxFillTtl; - if (fillDeadline <= nowSec + 30) { + if (fillDeadline <= nowSec + MIN_FILL_WINDOW_SEC) { throw new Error("quote too close to expiry to safely open an order"); }