Skip to content

Commit

Permalink
Fix scaling factor in driver (#2840)
Browse files Browse the repository at this point in the history
# Description
Furucombo people noticed the scaling of prices in the driver is not
working properly.

Example auction: 9172009
[Example
log](https://production-6de61f.kb.eu-central-1.aws.cloud.es.io/app/discover#/doc/c0e240e0-d9b3-11ed-b0e6-e361adffce0b/cowlogs-prod-2024.07.23?id=cpXk3JABP0v0FZqw807z)
of two solutions that should have been merged.

The `fn scaling_factor` function description is correct:

> /// Given two solutions returns the factors with
> /// which prices of the second solution would have to be multiplied so
that the
> /// given token would have the same price in both solutions.

which means, we need to calculate `factor = first_price / second_price`
so that `factor` represents the number with which `second_price` should
be multiplied to get `first_price`
  • Loading branch information
sunce86 authored Jul 29, 2024
1 parent 74a1271 commit 516719f
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 3 deletions.
2 changes: 1 addition & 1 deletion crates/driver/src/domain/competition/solution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,8 @@ fn scaling_factor(first: &Prices, second: &Prices) -> Option<BigRational> {
let first_price = first[token];
let second_price = second[token];
BigRational::new(
number::conversions::u256_to_big_int(&second_price),
number::conversions::u256_to_big_int(&first_price),
number::conversions::u256_to_big_int(&second_price),
)
})
.collect();
Expand Down
24 changes: 24 additions & 0 deletions crates/driver/src/tests/cases/merge_settlements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ use crate::tests::{
ab_order,
ab_pool,
ab_solution,
ad_order,
ad_pool,
ad_solution,
cd_order,
cd_pool,
cd_solution,
Expand Down Expand Up @@ -45,6 +48,27 @@ async fn possible() {
.await;
}

/// Test that settlements can be merged with two solutions containing a common
/// token.
#[tokio::test]
#[ignore]
async fn possible_common_token() {
let ab_order = ab_order();
let ad_order = ad_order();
let test: Test = setup::setup()
.solvers(vec![test_solver().merge_solutions()])
.pool(ab_pool())
.pool(ad_pool())
.order(ab_order.clone())
.order(ad_order.clone())
.solution(ab_solution())
.solution(ad_solution())
.done()
.await;

test.solve().await.ok().orders(&[ab_order, ad_order]);
}

/// Test that settlements are not merged if the clearing prices don't permit it.
#[tokio::test]
#[ignore]
Expand Down
3 changes: 3 additions & 0 deletions crates/driver/src/tests/cases/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ pub const DEFAULT_POOL_AMOUNT_D: u64 = 6000;
/// The order amount for orders selling token "A" for "B".
pub const AB_ORDER_AMOUNT: u64 = 50;

/// The order amount for orders selling token "A" for "D".
pub const AD_ORDER_AMOUNT: u64 = 48;

/// The order amount for orders selling token "C" for "D".
pub const CD_ORDER_AMOUNT: u64 = 40;

Expand Down
43 changes: 41 additions & 2 deletions crates/driver/src/tests/setup/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ use {
},
tests::{
cases::{
is_approximately_equal,
EtherExt,
AB_ORDER_AMOUNT,
AD_ORDER_AMOUNT,
CD_ORDER_AMOUNT,
DEFAULT_POOL_AMOUNT_A,
DEFAULT_POOL_AMOUNT_B,
Expand Down Expand Up @@ -627,6 +629,37 @@ pub fn ab_solution() -> Solution {
}
}

/// An example order which sells token "A" for token "D".
pub fn ad_order() -> Order {
Order {
name: "A-D order",
sell_amount: AD_ORDER_AMOUNT.ether().into_wei(),
sell_token: "A",
buy_token: "D",
..Default::default()
}
}

/// A pool between tokens "A" and "D".
pub fn ad_pool() -> Pool {
Pool {
token_a: "A",
token_b: "D",
amount_a: DEFAULT_POOL_AMOUNT_A.ether().into_wei(),
amount_b: DEFAULT_POOL_AMOUNT_D.ether().into_wei(),
}
}

/// A solution solving the [`ad_order`].
pub fn ad_solution() -> Solution {
Solution {
calldata: Calldata::Valid {
additional_bytes: 0,
},
orders: vec!["A-D order"],
}
}

/// A pool between tokens "C" and "D".
pub fn cd_pool() -> Pool {
Pool {
Expand Down Expand Up @@ -1175,8 +1208,14 @@ impl<'a> SolveOk<'a> {
Some(executed_amounts) => (executed_amounts.sell, executed_amounts.buy),
None => (quoted_order.sell, quoted_order.buy),
};
assert!(u256(trade.get("sellAmount").unwrap()) == expected_sell);
assert!(u256(trade.get("buyAmount").unwrap()) == expected_buy);
assert!(is_approximately_equal(
u256(trade.get("sellAmount").unwrap()),
expected_sell
));
assert!(is_approximately_equal(
u256(trade.get("buyAmount").unwrap()),
expected_buy
));
}
self
}
Expand Down

0 comments on commit 516719f

Please sign in to comment.