-
Notifications
You must be signed in to change notification settings - Fork 152
Volume fees in the orderbook #3900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Reminder: Please consider backward compatibility when modifying the API specification.
Caused by: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds volume-based protocol fee support to the orderbook's quote API, allowing users to see auction fees before submitting orders. The implementation follows the driver's fee calculation logic to ensure quotes remain fillable after fees are applied during auction competition.
Key changes:
- Added
--volume-feeCLI argument accepting decimal values in range[0, 1)representing the fee factor - Extended
OrderQuoteResponsewith optionalprotocolFeeBpsandprotocolFeeSellAmountfields - Implemented fee calculation logic that adjusts quote amounts based on order side (reduces buy amount for sell orders, increases sell amount for buy orders)
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/orderbook/src/arguments.rs | Adds FeeFactor type and --volume-fee CLI argument with validation |
| crates/orderbook/src/run.rs | Passes volume fee configuration to QuoteHandler |
| crates/orderbook/src/quoter.rs | Implements fee calculation logic and quote adjustment with comprehensive unit tests |
| crates/orderbook/src/api/post_quote.rs | Updates test to include new protocol fee fields |
| crates/orderbook/openapi.yml | Documents new API response fields for protocol fees |
| crates/model/src/quote.rs | Adds optional protocol fee fields to OrderQuoteResponse model |
| crates/e2e/tests/e2e/quoting.rs | Adds end-to-end test validating volume fee behavior for both sell and buy orders |
| crates/orderbook/Cargo.toml | Adds derive_more dependency for Into trait |
| Cargo.lock | Updates lock file with derive_more dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
m-sz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, small nit.
| let test_cases = vec![ | ||
| (0.0001, "1"), // 0.01% = 1 bps | ||
| (0.001, "10"), // 0.1% = 10 bps | ||
| (0.01, "100"), // 1% = 100 bps | ||
| (0.05, "500"), // 5% = 500 bps | ||
| (0.1, "1000"), // 10% = 1000 bps | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The comments could be put as 3rd element in the tuple and used as custom assert string on line 395, which would include message on the exact test case that has failed if it ever does.
I know it's a simple code change so I don't insist on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this assertion fails, it will show the expected bps, so there is no need for a detailed error message.
services/crates/orderbook/src/quoter.rs
Line 403 in ea0c5f7
| assert_eq!(result.protocol_fee_bps, Some(expected_bps.to_string())); |
Description
Adds protocol volume fee support to the orderbook quote API in order to reflect the auction fees after submitting an order using the same quote. The quote response now includes
protocolFeeBpsandfieldprotocolFeeSellAmountswhen volume fees are configured, allowing users to see the fee before placing orders.Volume fees are applied to the surplus token (buy token for sell orders, sell token for buy orders) following the same logic as the driver. The orderbook adjusts quote amounts so that orders can be signed with amounts that will be fillable after the driver applies fees during auction competition.
The PriceImprovement and Surplus fees are based on the quotes, so it doesn't make any sense to support them in the quote API.
Changes
--volume-feeCLI argument to orderbook (e.g.,--volume-fee=0.0002for 0.02% or 2 basis points)[0, 1)representing the fee factorprotocolFeeBps(string)andfieldprotocolFeeSellAmount(U256)stoOrderQuoteResponsesare only present when--volume-feeis configured in the orderbookImplementation details
services/crates/driver/src/domain/competition/solution/fee.rs
Lines 185 to 202 in 31ea719
buy_amount, reduces the buy amount returned in quotesell_amount+ network fee, increases the sell amount returned in quoteFee amounts are always converted to sell token for theprotocolFeeSellAmountfieldUses
quote.sell_amount/buy_amount(final computed amounts after network fees) rather thanquote.data.quoted_sell_amount/quoted_buy_amount(original exchange rate amounts)How to test
New unit and e2e tests.
Further configuration
This feature needs to be enabled only on a specific block. This logic will be implemented in a follow-up PR.