feat(public-api): approval txs on quote, allowance contract on rates - #12517
Conversation
Populate approval.approvalTxs on the quote response: ready-to-sign exact-amount approve transactions in broadcast order, with a preceding approve(spender, 0) when the token requires resetting a non-zero allowance before changing it (USDT-style, detected by simulating the approve as the owner - no token list). Replaces the never-populated singular approvalTx field. Expose the first-hop allowanceContract on rates so clients can check or set allowances manually before quoting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe API now exposes ChangesAllowance-aware approvals
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RatesAPI
participant QuoteAPI
participant ERC20Token
Client->>RatesAPI: request rates
RatesAPI-->>Client: allowanceContract
Client->>QuoteAPI: request quote
QuoteAPI->>ERC20Token: check allowance and simulate approval
ERC20Token-->>QuoteAPI: allowance state
QuoteAPI-->>Client: ordered approvalTxs
Client->>ERC20Token: sign and broadcast approvals
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/public-api/docs/rest-api-guide.md`:
- Line 53: Update the approval guidance around buildApprovalInfo to clarify that
approvalTxs may be empty when the existing allowance covers the required amount;
describe approvals as exact only when the current allowance is insufficient, and
remove the claim that every swap needs a fresh approval.
In `@packages/public-api/src/routes/quote/utils.ts`:
- Around line 55-60: Update the needsReset calculation around the simulated
approve call to decode the ERC-20 boolean return value and only treat a
fulfilled simulation returning false as requiring a reset; preserve revert
handling separately. Do not catch RPC or transport failures as
needsReset—propagate them through the existing quote-error path, using the
surrounding quote utility’s established decoding and error-handling symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f08504ba-9f96-4fdd-b9c3-44764c93dc3c
📒 Files selected for processing (6)
packages/public-api/docs/rest-api-guide.mdpackages/public-api/src/routes/quote/types.tspackages/public-api/src/routes/quote/utils.tspackages/public-api/src/routes/rates/getRates.tspackages/public-api/src/routes/rates/types.tspackages/swap-widget/src/types/index.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/public-api/src/routes/quote/utils.ts (1)
80-84: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate the swap widget approval executor.
Line 83 can return
[resetTx, approveTx], butpackages/swap-widget/src/hooks/useSwapApproval.ts:25-104ignoresapprovalTxsand sends one exact approval. For a reset-required token, that approval can revert.Execute each
quote.approval.approvalTxsentry in order and wait for each receipt before reporting approval success.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/public-api/src/routes/quote/utils.ts` around lines 80 - 84, Update the approval flow in useSwapApproval to iterate over every entry in quote.approval.approvalTxs sequentially, submit each transaction, and await its receipt before sending the next; only report approval success after all entries complete, preserving the reset-then-approve order.
🧹 Nitpick comments (1)
packages/public-api/src/routes/quote/utils.ts (1)
43-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit approval transaction type.
Declare an
ApprovalTransactiontype alias and annotate bothapproveTxandresetTx. This keeps the public transaction payload explicit at its construction sites.As per coding guidelines, “ALWAYS use explicit types for object shapes using interfaces or type aliases in TypeScript.”
Also applies to: 70-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/public-api/src/routes/quote/utils.ts` around lines 43 - 51, Define an ApprovalTransaction type alias for the approval payload shape in the surrounding quote utilities, then explicitly annotate both approveTx and resetTx with that type. Keep their existing fields and construction behavior unchanged while replacing inferred object shapes with the shared alias.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/public-api/src/routes/quote/utils.ts`:
- Around line 80-84: Update the approval flow in useSwapApproval to iterate over
every entry in quote.approval.approvalTxs sequentially, submit each transaction,
and await its receipt before sending the next; only report approval success
after all entries complete, preserving the reset-then-approve order.
---
Nitpick comments:
In `@packages/public-api/src/routes/quote/utils.ts`:
- Around line 43-51: Define an ApprovalTransaction type alias for the approval
payload shape in the surrounding quote utilities, then explicitly annotate both
approveTx and resetTx with that type. Keep their existing fields and
construction behavior unchanged while replacing inferred object shapes with the
shared alias.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: af5410d8-1d6a-46fe-8776-1cae140595a1
📒 Files selected for processing (3)
packages/public-api/docs/rest-api-guide.mdpackages/public-api/src/routes/quote/types.tspackages/public-api/src/routes/quote/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/public-api/src/routes/quote/types.ts
- packages/public-api/docs/rest-api-guide.md
Probe via simulateContract so a token whose approve() fulfills but returns false also lands on the reset side (return-less tokens reject decoding and do too). Docs no longer claim every swap needs a fresh approval - approvalTxs is empty when the current allowance covers the amount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c7fa6f9 to
8f9d84f
Compare
1 participant
Description
Follow-up to #12515 — quotes are approval-aware, this puts the approval itself on the wire.
approval.approvalTxson the quote response: ready-to-sign approval transactions in broadcast order, replacing the never-populated singularapprovalTxfield. Approvals are exact — sized to the step's sell amount, encoded locally (encodeFunctionData, zero extra RPCs on the happy path). Tokens that require resetting a non-zero allowance before changing it (USDT-stylerequire(allowed == 0)) get a precedingapprove(spender, 0), detected without a token list: when the current allowance is non-zero, the approve is simulated viaeth_callas the owner — a revert means reset-required. A spurious reset from an RPC hiccup is harmless (an extra always-valid tx), and the probe only runs on the isRequired + non-zero-allowance path, so it is rare. Clients preferring unlimited approvals keep building their own approve toapproval.spender.allowanceContracton rates: the first-hop approval spender is now exposed per rate, so clients that want to handle approvals manually — checking the current allowance or setting an unlimited approval while still comparing rates — can do so before quoting.Docs updated accordingly (
rest-api-guide.md): approvalTxs execution order, exact-approval semantics, and the manual-approval path from the rate. The swap-widget's mirroredApprovalInfotype is updated to match the wire (it only usesapproval.spenderat runtime; no behavior change, rides the unpublished 0.6.0).Issue (if applicable)
closes #
Risk
Low. Additive wire changes plus removal of a response field that was never populated. The reset probe adds at most one
eth_callper quote, only when an approval is required and the current allowance is non-zero.Public API quote/rate responses only. No transaction construction or broadcast logic is touched — approval transactions are encoded but signed/broadcast by API clients.
Testing
Engineering
eth_callstate override: approve from a non-zero allowance reverts, from zero succeeds.isRequired+ single exact approve (decoded: correct spender, amount = sell amount)approve(spender, 0)then exact approveisRequired: false, emptyapprovalTxsallowanceContractper swapper.Operations
approval.approvalTxsshould contain a ready-to-sign approve for the exact sell amount.Screenshots (if applicable)
🤖 Generated with Claude Code
Summary by CodeRabbit