-
Notifications
You must be signed in to change notification settings - Fork 97
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
Fee tests #2510
Fee tests #2510
Conversation
Co-authored-by: Felix Leupold <felixleupold90@gmail.com>
Co-authored-by: Felix Leupold <felixleupold90@gmail.com>
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.
Looks good overall (but I'm biased) but I think there could be a bit more consistency and some more small cleanups.
crates/driver/src/tests/setup/mod.rs
Outdated
"factor": factor | ||
} | ||
}), | ||
pub mod fee { |
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.
I think this one needs a bit of clean up. Either revert or move this module into its own file please.
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.
Done
async fn price_improvement_fee_sell_out_of_market_order() { | ||
let fee_policy = Policy::PriceImprovement { | ||
factor: 0.5, | ||
max_volume_factor: 1.0, |
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.
Do we have any tests checking the max volume factor for this policy?
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.
Added
Co-authored-by: Felix Leupold <felixleupold90@gmail.com>
Co-authored-by: Felix Leupold <felixleupold90@gmail.com>
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.
Lg. Tests are pretty readable.
What's missing to have a complete suite of tests are capped versions for price improvement and maybe versions with partially fillable orders (but that might be out of the scope of this PR)
Description
Proposal on how to improve the readability of #2467
It only implements two surplus and one price improvement test case as an example, the others still need to be converted.
Changes
quoted_order
amounts fromexecution
amounts. This makes it so that a test case can specify a concrete limit price (QuotedOrders
currently always use the AMM to compute their buy amounts).in_given_out
on pool to correctly compute the executed amounts for buy orders.TestCase
is easier to readHow to test
Driver tests
Fixes #2479