fix(intents): close funds-handling gaps in the escrow order flow - #28
Conversation
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.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:a7d2827cc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| } | ||
| assertFillWindowOpen(order.fillDeadline); |
There was a problem hiding this comment.
When the ERC-20 allowance is insufficient—especially for a token requiring a reset transaction—this check runs before the destination balance read and one or two approval confirmations. A quote with slightly more than 30 seconds remaining can therefore expire before open() is submitted, leaving funds in an order that cannot be filled until refund expiry. Move or repeat the assertion immediately before sendTransaction; the same premature ordering also occurs in WithdrawIntentRouteStep.handleOpen and useIntentLegPipeline.openOne.
Useful? React with 👍 / 👎.
| throw new Error( | ||
| "Couldn't read the current token allowance — refusing to approve blindly. Try again in a moment.", | ||
| ); |
There was a problem hiding this comment.
If this RPC read fails after the bridge has settled, throwing directly bypasses fail(), which is responsible for emitting phase: "failed", failedAfterBridge, and the bridge metadata. DepositFlow consequently retains phase: "quoting-deposit", treats the operation as perpetually in flight, disables the main CTA, and never exposes the retry-deposit action even though the user's funds have already reached the destination chain. Route this error through fail() as the surrounding failure paths do.
Useful? React with 👍 / 👎.
| // 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); |
There was a problem hiding this comment.
When the receipt resolves with status === "reverted", this hash has already been persisted; the catch then enters failed, while the retry button is disabled whenever openTxHash is set. A confirmed revert cannot later create an escrow, so retaining the timeout-oriented lock unnecessarily prevents obtaining another intent quote until the component is reset or reloaded. Clear the hash on the explicit reverted-receipt path, with the analogous path in WithdrawIntentRouteStep needing the same treatment.
Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.
Six defects found while reviewing the intents integration before merge. The first three can cost users funds; the rest remove footguns.