Improve docs for solver API: limit amounts vs full amounts - #650
Open
tamir-cow wants to merge 4 commits into
Open
Improve docs for solver API: limit amounts vs full amounts#650tamir-cow wants to merge 4 commits into
tamir-cow wants to merge 4 commits into
Conversation
…led) amount, while fullSellAmount/fullBuyAmount are the original signed amounts that stay constant across auctions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe order schema now distinguishes remaining fee-adjusted executable amounts from original signed amounts. The example includes the full amount fields and explains partial fills and volume-based fees. ChangesOrder amount schema
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/cow-protocol/reference/core/auctions/schema.md`:
- Line 121: Update the limit sell order explanation in the entry beginning “The
above entry should be interpreted as follows” to state that the example executes
1000 COW atoms and the user transfers that executed amount plus any
solver-computed fee in COW. Remove the claim that the total transfer is exactly
1000000000000000000000 COW atoms, while preserving the guaranteed USDC amount
and surrounding order semantics.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1a2083d-6d59-41da-bf42-223cc1b7004e
📒 Files selected for processing (1)
docs/cow-protocol/reference/core/auctions/schema.md
| ``` | ||
|
|
||
| The above entry should be interpreted as follows. It is a Fill-or-Kill order since the flag `partiallyFillable` is set to `false`. Moreover, it is a sell order since its `kind` is set to `sell`. Finally, this is a `limit` order, meaning that it has a zero-signed fee, which implies that the solver is free to choose an appropriate fee to cover its execution cost. This means that, if executed, the user will send a total of 1000000000000000000000 COW atoms to the settlement contract and, no matter how much fee the solver will charge, the user is guaranteed to receive at least 284138335 USDC atoms. | ||
| The above entry should be interpreted as follows. It is a Fill-or-Kill order since the flag `partiallyFillable` is set to `false`. Moreover, it is a sell order since its `kind` is set to `sell`. Finally, this is a `limit` order, meaning that it has a zero-signed fee, which implies that the solver is free to choose an appropriate fee to cover its execution cost. This means that, if executed, the user will send a total of 1000000000000000000000 COW atoms to the settlement contract and, no matter how much fee the solver will charge, the user is guaranteed to receive at least 284138335 USDC atoms. Since this order has not been partially filled yet, `sellAmount`/`buyAmount` and `fullSellAmount`/`fullBuyAmount` coincide here; for a partially fillable order that already has a non-zero `executed` amount from a previous auction, `sellAmount` and `buyAmount` would instead reflect only the remaining, unfilled portion, while `fullSellAmount` and `fullBuyAmount` would still refer to the order's original amounts. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the fee explanation for limit sell orders.
For a kind: "sell" limit order, the user transfers executedAmount + fee in the sell token. The current text says the user transfers exactly 1000000000000000000000 COW atoms, even though it also says the solver can charge a fee. State that the example executes 1000 COW atoms and that any solver-computed fee is added to the transfer.
🤖 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 `@docs/cow-protocol/reference/core/auctions/schema.md` at line 121, Update the
limit sell order explanation in the entry beginning “The above entry should be
interpreted as follows” to state that the example executes 1000 COW atoms and
the user transfers that executed amount plus any solver-computed fee in COW.
Remove the claim that the total transfer is exactly 1000000000000000000000 COW
atoms, while preserving the guaranteed USDC amount and surrounding order
semantics.
fhenneke
requested changes
Aug 3, 2026
…uyAmount: partial fills reduce them to the remaining unfilled amount, and volume-based protocol/partner fees additionally scale them so solvers can route on sellAmount/buyAmount while ignoring fees, and the driver can charge those fees afterwards without violating the user's original signed limit.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/cow-protocol/reference/core/auctions/schema.md`:
- Line 123: The example explanation should not state that the user sends exactly
1000000000000000000000 COW atoms when a solver fee may apply. Update the
sell-fulfillment wording to describe the transfer as 1000 COW atoms plus any
solver-computed fee, while preserving the existing minimum USDC guarantee and
other order details.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c816f04-5735-4455-9471-98dedcac92a0
📒 Files selected for processing (1)
docs/cow-protocol/reference/core/auctions/schema.md
…uyAmount: partial fills reduce them to the remaining unfilled amount, and volume-based protocol/partner fees additionally scale them so solvers can route on sellAmount/buyAmount while ignoring fees, and the driver can charge those fees afterwards without violating the user's original signed limit.
fhenneke
reviewed
Aug 7, 2026
| ``` | ||
|
|
||
| The above entry should be interpreted as follows. It is a Fill-or-Kill order since the flag `partiallyFillable` is set to `false`. Moreover, it is a sell order since its `kind` is set to `sell`. Finally, this is a `limit` order, meaning that it has a zero-signed fee, which implies that the solver is free to choose an appropriate fee to cover its execution cost. This means that, if executed, the user will send a total of 1000000000000000000000 COW atoms to the settlement contract and, no matter how much fee the solver will charge, the user is guaranteed to receive at least 284138335 USDC atoms. | ||
| The above entry should be interpreted as follows. It is a Fill-or-Kill order since the flag `partiallyFillable` is set to `false`. Moreover, it is a sell order since its `kind` is set to `sell`. Finally, this is a `limit` order, meaning that it has a zero-signed fee, which implies that the solver is free to choose an appropriate fee to cover its execution cost. This means that, if executed, the user will send a total of 1000000000000000000000 COW atoms to the settlement contract and, no matter how much fee the solver will charge, the user is guaranteed to receive at least 284138335 USDC atoms. Here, `sellAmount`/`buyAmount` equal `fullSellAmount`/`fullBuyAmount` because the order has not been partially filled and has no `protocolFees` applied. In general they can differ: partial fills reduce `sellAmount`/`buyAmount` to the remaining unfilled amount, and volume-based fees scale them down further, so the driver can charge those fees afterwards without violating the user's original limit. |
Contributor
There was a problem hiding this comment.
This case of buyAmount = fullBuyAmount does not happen in practice on most chains. This example should cover the usual case, I think, of a 2bps volume fee. The given signed amount of 284138335 would be consistent with a buyAmount of 284138335 / (1 - 0.0002) = 284195174.03... rounded to 284195174.
fhenneke
requested changes
Aug 10, 2026
fhenneke
left a comment
fhenneke
left a comment
Contributor
There was a problem hiding this comment.
I would remove the info on the fee policy entry. Otherwise looks good now.
Comment on lines
-113
to
+123
| "class": "limit" | ||
| "class": "limit", | ||
| "protocolFees": [ | ||
| { | ||
| "kind": "volume", | ||
| "factor": 0.0002 | ||
| } | ||
| ] |
Contributor
There was a problem hiding this comment.
I would revert this change. Fee policies are not always forwarded to solvers. (This is also not documented cleanly anywhere, I had to ask Claude to give some pointers on this.)
As an example, this auction instance does have the modified amounts but not any fee policies.
| ``` | ||
|
|
||
| The above entry should be interpreted as follows. It is a Fill-or-Kill order since the flag `partiallyFillable` is set to `false`. Moreover, it is a sell order since its `kind` is set to `sell`. Finally, this is a `limit` order, meaning that it has a zero-signed fee, which implies that the solver is free to choose an appropriate fee to cover its execution cost. This means that, if executed, the user will send a total of 1000000000000000000000 COW atoms to the settlement contract and, no matter how much fee the solver will charge, the user is guaranteed to receive at least 284138335 USDC atoms. | ||
| The above entry should be interpreted as follows. It is a Fill-or-Kill order since the flag `partiallyFillable` is set to `false`. Moreover, it is a sell order since its `kind` is set to `sell`. Finally, this is a `limit` order, meaning that it has a zero-signed fee, which implies that the solver is free to choose an appropriate fee to cover its execution cost. This means that, if executed, the user will send a total of 1000000000000000000000 COW atoms to the settlement contract, and the user has signed for a minimum of 284138335 USDC atoms (`fullBuyAmount`). The order also carries a `protocolFees` entry for a 2 bps volume fee, which the driver will deduct from the buy side once the trade settles. Since the order has not been partially filled, `sellAmount` still equals `fullSellAmount`, but `buyAmount` is scaled up from `fullBuyAmount` to net out that fee: `buyAmount` = `fullBuyAmount` / (1 - 0.0002) = 284138335 / 0.9998 ≈ 284195174.03, rounded down to 284195174. Solvers can then route on `sellAmount`/`buyAmount` while ignoring fees entirely, and once the driver charges its 2 bps cut of the 284195174 USDC bought (≈56839 USDC atoms), the user is left with exactly the signed minimum of 284138335 USDC atoms — so the original limit is never violated. This is the usual case in practice: on most chains essentially every order carries some volume-based protocol or partner fee, so `buyAmount`/`sellAmount` differ from `fullBuyAmount`/`fullSellAmount` even without any partial fill; the two only coincide when there is no such fee, which is rare. |
Contributor
There was a problem hiding this comment.
Probably rephrase this then from
The order also carries a
protocolFeesentry for a 2 bps volume fee, which the driver will deduct from the buy side once the trade settles.
to
The order also carries a 2 bps volume fee, which the driver will deduct from the buy side once the trade settles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clarify that sellAmount/buyAmount reflect an order's remaining (unfilled) amount, while fullSellAmount/fullBuyAmount are the original signed amounts that stay constant across auctions.
Summary by CodeRabbit
fullSellAmountandfullBuyAmountfields in the order schema.