Skip to content
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.

Sign up for GitHub

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

Price improvement tests #2467

Closed
wants to merge 14 commits into from
Closed

Price improvement tests #2467

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9b3a0ee
Price improvement tests
squadgazzz Mar 4, 2024
dd233e2
Adjusted values
squadgazzz Mar 5, 2024
3f7bf5e
Update crates/driver/src/tests/cases/protocol_fees.rs
squadgazzz Mar 5, 2024
4b70eb7
Update crates/driver/src/tests/cases/protocol_fees.rs
squadgazzz Mar 5, 2024
38135e8
Adjusted values
squadgazzz Mar 5, 2024
287c1f9
Explicit fee amounts
squadgazzz Mar 5, 2024
08c7027
Network fee
squadgazzz Mar 5, 2024
758ebde
Adjusted fees
squadgazzz Mar 6, 2024
3a80fe6
Adjusted values
squadgazzz Mar 6, 2024
75d99c1
Order surplus factor=1
squadgazzz Mar 8, 2024
47dd239
Merge branch 'main' into 2287/price-improvement-tests
squadgazzz Mar 8, 2024
1a47343
Fixed values
squadgazzz Mar 8, 2024
9daf971
Refactoring
squadgazzz Mar 8, 2024
886fc2f
Wrong comment
squadgazzz Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 127 additions & 8 deletions crates/driver/src/tests/cases/protocol_fees.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
ab_solution,
ExpectedOrderAmounts,
FeePolicy,
PriceImprovementQuote,
Test,
},
},
Expand All @@ -18,7 +19,7 @@ struct TestCase {
order_side: order::Side,
fee_policy: FeePolicy,
order_sell_amount: eth::U256,
solver_fee: Option<eth::U256>,
network_fee: Option<eth::U256>,
quote_sell_amount: eth::U256,
quote_buy_amount: eth::U256,
executed: eth::U256,
Expand All @@ -43,7 +44,7 @@ async fn protocol_fee_test_case(test_case: TestCase) {
.kind(order::Kind::Limit)
.sell_amount(test_case.order_sell_amount)
.side(test_case.order_side)
.solver_fee(test_case.solver_fee)
.solver_fee(test_case.network_fee)
.fee_policy(test_case.fee_policy)
.executed(test_case.executed)
.expected_amounts(expected_amounts);
Expand All @@ -70,7 +71,7 @@ async fn surplus_protocol_fee_buy_order_not_capped() {
order_side: order::Side::Buy,
fee_policy,
order_sell_amount: 50000000000000000000u128.into(),
solver_fee: Some(10000000000000000000u128.into()),
network_fee: Some(10000000000000000000u128.into()),
quote_sell_amount: 50000000000000000000u128.into(),
quote_buy_amount: 40000000000000000000u128.into(),
executed: 40000000000000000000u128.into(),
Expand All @@ -93,7 +94,7 @@ async fn surplus_protocol_fee_sell_order_not_capped() {
order_side: order::Side::Sell,
fee_policy,
order_sell_amount: 50000000000000000000u128.into(),
solver_fee: Some(10000000000000000000u128.into()),
network_fee: Some(10000000000000000000u128.into()),
quote_sell_amount: 50000000000000000000u128.into(),
quote_buy_amount: 40000000000000000000u128.into(),
executed: 40000000000000000000u128.into(),
Expand All @@ -116,7 +117,7 @@ async fn surplus_protocol_fee_buy_order_capped() {
order_side: order::Side::Buy,
fee_policy,
order_sell_amount: 50000000000000000000u128.into(),
solver_fee: Some(10000000000000000000u128.into()),
network_fee: Some(10000000000000000000u128.into()),
quote_sell_amount: 50000000000000000000u128.into(),
quote_buy_amount: 40000000000000000000u128.into(),
executed: 40000000000000000000u128.into(),
Expand All @@ -139,7 +140,7 @@ async fn surplus_protocol_fee_sell_order_capped() {
order_side: order::Side::Sell,
fee_policy,
order_sell_amount: 50000000000000000000u128.into(),
solver_fee: Some(10000000000000000000u128.into()),
network_fee: Some(10000000000000000000u128.into()),
quote_sell_amount: 50000000000000000000u128.into(),
quote_buy_amount: 40000000000000000000u128.into(),
executed: 40000000000000000000u128.into(),
Expand All @@ -158,7 +159,7 @@ async fn volume_protocol_fee_buy_order() {
order_side: order::Side::Buy,
fee_policy,
order_sell_amount: 50000000000000000000u128.into(),
solver_fee: Some(10000000000000000000u128.into()),
network_fee: Some(10000000000000000000u128.into()),
quote_sell_amount: 50000000000000000000u128.into(),
quote_buy_amount: 40000000000000000000u128.into(),
executed: 40000000000000000000u128.into(),
Expand All @@ -177,7 +178,7 @@ async fn volume_protocol_fee_sell_order() {
order_side: order::Side::Sell,
fee_policy,
order_sell_amount: 50000000000000000000u128.into(),
solver_fee: Some(10000000000000000000u128.into()),
network_fee: Some(10000000000000000000u128.into()),
quote_sell_amount: 50000000000000000000u128.into(),
quote_buy_amount: 40000000000000000000u128.into(),
executed: 40000000000000000000u128.into(),
Expand All @@ -187,3 +188,121 @@ async fn volume_protocol_fee_sell_order() {

protocol_fee_test_case(test_case).await;
}

#[tokio::test]
#[ignore]
async fn price_improvement_fee_buy_out_of_market_order() {
let fee_policy = FeePolicy::PriceImprovement {
factor: 0.5,
max_volume_factor: 1.0,
quote: PriceImprovementQuote {
sell_amount: 50000000000000000000u128.into(),
buy_amount: 35000000000000000000u128.into(),
network_fee: 1000000000000000000u128.into(),
},
};
let order_sell_amount = 50000000000000000000u128.into();
let order_buy_amount = 40000000000000000000u128.into();
let test_case = TestCase {
order_side: order::Side::Buy,
fee_policy,
order_sell_amount,
network_fee: Some(2000000000000000000u128.into()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely be easier to see with #2472, but can you comment in code how the price improvement is achieved (realized network fee lower than quote, leading to X price improvement). This will help understand the expected executed sell amount (compared to the quoted one), but maybe you can add a comment for this expectation as well.

Should be done for each test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started writing the comments and found out that the results look pretty weird. Found this code which applies a default surplus factor. Not sure if it needs to be eliminated for the price improvement tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, either we need to set it in the test case or make sure it's set to 1. I think your trick with reducing the fee is good enough and allows us to more precisely reason about absolute surplus values (rather than a factor), so I'd slightly prefer setting the factor to 1 (but no strong feelings, as longs as it's clear from the main test how the calculation is carried out)

quote_sell_amount: order_sell_amount,
quote_buy_amount: order_buy_amount,
executed: order_buy_amount,
executed_sell_amount: 54142857142857142857u128.into(),
executed_buy_amount: order_buy_amount,
};

protocol_fee_test_case(test_case).await;
}

#[tokio::test]
#[ignore]
async fn price_improvement_fee_sell_out_of_market_order() {
let fee_policy = FeePolicy::PriceImprovement {
factor: 0.5,
max_volume_factor: 1.0,
quote: PriceImprovementQuote {
sell_amount: 50000000000000000000u128.into(),
buy_amount: 35000000000000000000u128.into(),
network_fee: 1000000000000000000u128.into(),
},
};
let order_sell_amount = 50000000000000000000u128.into();
let order_buy_amount = 40000000000000000000u128.into();
let network_fee = 2000000000000000000u128.into();
let test_case = TestCase {
order_side: order::Side::Sell,
fee_policy,
order_sell_amount,
network_fee: Some(network_fee),
quote_sell_amount: order_sell_amount,
quote_buy_amount: order_buy_amount,
executed: order_sell_amount - network_fee,
executed_sell_amount: order_sell_amount,
executed_buy_amount: 37156862745098039215u128.into(),
};

protocol_fee_test_case(test_case).await;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially fillable orders are missing.

Also can we add a test that shows shifting surplus into network fees doesn't change the protocol fee charged? E.g. the following two executions pay the same fee (assuming same underlying orders & quotes)

  1. Sell 0.1 ETH receive 3500 DAI, pay 0.9 ETH in network fees
  2. Sell 1 ETH receive 3500 DAI, pay 0 in network fees

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially fillable orders require some further refactoring. I will create a separate issue for this.


#[tokio::test]
#[ignore]
async fn price_improvement_fee_buy_in_market_order() {
let fee_policy = FeePolicy::PriceImprovement {
factor: 0.5,
max_volume_factor: 1.0,
quote: PriceImprovementQuote {
sell_amount: 50000000000000000000u128.into(),
buy_amount: 40000000000000000000u128.into(),
network_fee: 1000000000000000000u128.into(),
},
};
let order_sell_amount = 50000000000000000000u128.into();
let order_buy_amount = 35000000000000000000u128.into();
let test_case = TestCase {
order_side: order::Side::Buy,
fee_policy,
order_sell_amount,
network_fee: Some(2000000000000000000u128.into()),
quote_sell_amount: order_sell_amount,
quote_buy_amount: order_buy_amount,
executed: order_buy_amount,
executed_sell_amount: order_sell_amount,
executed_buy_amount: order_buy_amount,
};

protocol_fee_test_case(test_case).await;
}

#[tokio::test]
#[ignore]
async fn price_improvement_fee_sell_in_market_order() {
let fee_policy = FeePolicy::PriceImprovement {
factor: 0.5,
max_volume_factor: 1.0,
quote: PriceImprovementQuote {
sell_amount: 50000000000000000000u128.into(),
buy_amount: 40000000000000000000u128.into(),
network_fee: 1000000000000000000u128.into(),
},
};
let order_sell_amount: eth::U256 = 50000000000000000000u128.into();
let order_buy_amount: eth::U256 = 35000000000000000000u128.into();
let network_fee = 20000000000000000000u128.into();
let test_case = TestCase {
order_side: order::Side::Sell,
fee_policy,
order_sell_amount,
network_fee: Some(network_fee),
quote_sell_amount: order_sell_amount,
quote_buy_amount: order_buy_amount,
executed: order_sell_amount - network_fee,
executed_sell_amount: order_sell_amount,
executed_buy_amount: order_buy_amount,
};

protocol_fee_test_case(test_case).await;
}
30 changes: 30 additions & 0 deletions crates/driver/src/tests/setup/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,28 @@ pub enum Score {
RiskAdjusted { success_probability: f64 },
}

#[serde_as]
#[derive(Debug, Clone, PartialEq, serde::Serialize)]
#[serde(rename_all = "camelCase", tag = "kind")]
pub struct PriceImprovementQuote {
pub buy_amount: eth::U256,
pub sell_amount: eth::U256,
pub network_fee: eth::U256,
}

#[serde_as]
#[derive(Debug, Clone, PartialEq, serde::Serialize)]
#[serde(rename_all = "camelCase", tag = "kind")]
pub enum FeePolicy {
#[serde(rename_all = "camelCase")]
Surplus { factor: f64, max_volume_factor: f64 },
#[serde(rename_all = "camelCase")]
PriceImprovement {
factor: f64,
max_volume_factor: f64,
quote: PriceImprovementQuote,
},
#[serde(rename_all = "camelCase")]
Volume { factor: f64 },
}

Expand All @@ -94,6 +109,21 @@ impl FeePolicy {
"maxVolumeFactor": max_volume_factor
}
}),
FeePolicy::PriceImprovement {
factor,
max_volume_factor,
quote,
} => json!({
"priceImprovement": {
"factor": factor,
"maxVolumeFactor": max_volume_factor,
"quote": {
"sellAmount": quote.sell_amount,
"buyAmount": quote.buy_amount,
"fee": quote.network_fee,
}
}
}),
FeePolicy::Volume { factor } => json!({
"volume": {
"factor": factor
Expand Down
Loading