Skip to content

Commit

Permalink
Set the correct order uid for foreign liquidity orders
Browse files Browse the repository at this point in the history
These orders do not need a correct uid because it doesn't matter for
encoding the settlement for the contract which is why we are currently
storing a default 0 uid.
It is still nice to have the correct uid for logging and in the
competition info.
  • Loading branch information
vkgnosis committed Dec 2, 2022
1 parent 6988d10 commit 41ded83
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 6 deletions.
3 changes: 3 additions & 0 deletions crates/driver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use driver::{
commit_reveal::CommitRevealSolver, driver::Driver,
};
use gas_estimation::GasPriceEstimating;
use model::DomainSeparator;
use shared::{
baseline_solver::BaseTokens,
code_fetching::{CachedCodeFetcher, CodeFetching},
Expand Down Expand Up @@ -148,6 +149,7 @@ async fn init_common_components(args: &Arguments) -> CommonComponents {
}

async fn build_solvers(common: &CommonComponents, args: &Arguments) -> Vec<Arc<dyn Solver>> {
let domain = DomainSeparator::new(common.chain_id, common.settlement_contract.address());
let buffer_retriever = Arc::new(BufferRetriever::new(
common.web3.clone(),
common.settlement_contract.address(),
Expand Down Expand Up @@ -183,6 +185,7 @@ async fn build_solvers(common: &CommonComponents, args: &Arguments) -> Vec<Arc<d
false,
args.slippage.get_global_calculator(),
Default::default(),
domain,
)) as Arc<dyn Solver>
})
.collect()
Expand Down
3 changes: 3 additions & 0 deletions crates/solver/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use anyhow::Context;
use clap::Parser;
use contracts::{BalancerV2Vault, IUniswapLikeRouter, UniswapV3SwapRouter, WETH9};
use model::DomainSeparator;
use num::rational::Ratio;
use shared::{
baseline_solver::BaseTokens,
Expand Down Expand Up @@ -239,6 +240,7 @@ async fn main() -> ! {
market_makable_token_list.clone(),
));

let domain = DomainSeparator::new(chain_id, settlement_contract.address());
let solver = solver::solver::create(
web3.clone(),
solvers,
Expand Down Expand Up @@ -272,6 +274,7 @@ async fn main() -> ! {
market_makable_token_list,
&args.order_prioritization,
post_processing_pipeline,
&domain,
)
.expect("failure creating solvers");

Expand Down
1 change: 1 addition & 0 deletions crates/solver/src/settlement_simulation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ mod tests {
Arc::new(MockAllowanceManaging::new()),
Arc::new(OrderConverter::test(H160([0x42; 20]))),
SlippageContext::default(),
&Default::default(),
)
.await
.map(|settlement| vec![settlement])
Expand Down
4 changes: 3 additions & 1 deletion crates/solver/src/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
use anyhow::{anyhow, Context, Result};
use contracts::{BalancerV2Vault, GPv2Settlement};
use ethcontract::{errors::ExecutionError, Account, PrivateKey, H160, U256};
use model::auction::AuctionId;
use model::{auction::AuctionId, DomainSeparator};
use num::BigRational;
use reqwest::Url;
use shared::{
Expand Down Expand Up @@ -282,6 +282,7 @@ pub fn create(
market_makable_token_list: AutoUpdatingTokenList,
order_prioritization_config: &single_order_solver::Arguments,
post_processing_pipeline: Arc<dyn PostProcessing>,
domain: &DomainSeparator,
) -> Result<Solvers> {
// Tiny helper function to help out with type inference. Otherwise, all
// `Box::new(...)` expressions would have to be cast `as Box<dyn Solver>`.
Expand Down Expand Up @@ -336,6 +337,7 @@ pub fn create(
filter_non_fee_connected_orders,
slippage_calculator,
market_makable_token_list.clone(),
*domain,
)
};

Expand Down
8 changes: 7 additions & 1 deletion crates/solver/src/solver/http_solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use ethcontract::{errors::ExecutionError, Account, U256};
use futures::{join, lock::Mutex};
use itertools::{Either, Itertools as _};
use maplit::{btreemap, hashset};
use model::{auction::AuctionId, order::OrderKind};
use model::{auction::AuctionId, order::OrderKind, DomainSeparator};
use num::{BigInt, BigRational};
use primitive_types::H160;
use shared::{
Expand Down Expand Up @@ -68,6 +68,7 @@ pub struct HttpSolver {
filter_non_fee_connected_orders: bool,
slippage_calculator: SlippageCalculator,
market_makable_token_list: AutoUpdatingTokenList,
domain: DomainSeparator,
}

impl HttpSolver {
Expand All @@ -84,6 +85,7 @@ impl HttpSolver {
filter_non_fee_connected_orders: bool,
slippage_calculator: SlippageCalculator,
market_makable_token_list: AutoUpdatingTokenList,
domain: DomainSeparator,
) -> Self {
Self {
solver,
Expand All @@ -97,6 +99,7 @@ impl HttpSolver {
filter_non_fee_connected_orders,
slippage_calculator,
market_makable_token_list,
domain,
}
}

Expand Down Expand Up @@ -564,6 +567,7 @@ impl Solver for HttpSolver {
self.allowance_manager.clone(),
self.order_converter.clone(),
slippage,
&self.domain,
)
.await
{
Expand Down Expand Up @@ -666,6 +670,7 @@ mod tests {
true,
SlippageCalculator::default(),
Default::default(),
Default::default(),
);
let base = |x: u128| x * 10u128.pow(18);
let limit_orders = vec![LimitOrder {
Expand Down Expand Up @@ -904,6 +909,7 @@ mod tests {
false, // filter_non_fee_connected_orders
SlippageCalculator::default(),
Default::default(),
Default::default(),
);

let (model, context) = solver
Expand Down
25 changes: 21 additions & 4 deletions crates/solver/src/solver/http_solver/settlement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ use crate::{
settlement::Settlement,
};
use anyhow::{anyhow, Context as _, Result};
use model::order::{Interactions, Order, OrderClass, OrderKind, OrderMetadata};
use model::{
order::{Interactions, Order, OrderClass, OrderKind, OrderMetadata},
DomainSeparator,
};
use primitive_types::{H160, U256};
use shared::http_solver::model::*;
use std::{
Expand All @@ -29,13 +32,15 @@ pub async fn convert_settlement(
allowance_manager: Arc<dyn AllowanceManaging>,
order_converter: Arc<OrderConverter>,
slippage: SlippageContext<'_>,
domain: &DomainSeparator,
) -> Result<Settlement> {
IntermediateSettlement::new(
settled,
context,
allowance_manager,
order_converter,
slippage,
domain,
)
.await?
.into_settlement()
Expand Down Expand Up @@ -153,11 +158,15 @@ impl<'a> IntermediateSettlement<'a> {
allowance_manager: Arc<dyn AllowanceManaging>,
order_converter: Arc<OrderConverter>,
slippage: SlippageContext<'a>,
domain: &DomainSeparator,
) -> Result<IntermediateSettlement<'a>> {
let executed_limit_orders =
match_prepared_and_settled_orders(context.orders, settled.orders)?;
let foreign_liquidity_orders =
convert_foreign_liquidity_orders(order_converter, settled.foreign_liquidity_orders)?;
let foreign_liquidity_orders = convert_foreign_liquidity_orders(
order_converter,
settled.foreign_liquidity_orders,
domain,
)?;
let prices = match_settled_prices(executed_limit_orders.as_slice(), settled.prices)?;
let approvals = compute_approvals(allowance_manager, settled.approvals).await?;
let executions_amm = match_prepared_and_settled_amms(context.liquidity, settled.amms)?;
Expand Down Expand Up @@ -224,6 +233,7 @@ fn match_prepared_and_settled_orders(
fn convert_foreign_liquidity_orders(
order_converter: Arc<OrderConverter>,
foreign_liquidity_orders: Vec<ExecutedLiquidityOrderModel>,
domain: &DomainSeparator,
) -> Result<Vec<ExecutedLimitOrder>> {
foreign_liquidity_orders
.into_iter()
Expand All @@ -235,9 +245,10 @@ fn convert_foreign_liquidity_orders(
// All foreign orders **MUST** be liquidity, this is
// important so they cannot be used to affect the objective.
class: OrderClass::Liquidity,
// Not needed for encoding but nice to have for logs and competition info.
uid: liquidity.order.data.uid(domain, &liquidity.order.from),
// These fields do not seem to be used at all for order
// encoding, so we just use the default values.
uid: Default::default(),
settlement_contract: Default::default(),
// For other metdata fields, the default value is correct.
..Default::default()
Expand Down Expand Up @@ -487,6 +498,10 @@ mod tests {
exec_sell_amount: 101.into(),
exec_buy_amount: 102.into(),
};
let foreign_liquidity_order_uid = foreign_liquidity_order
.order
.data
.uid(&Default::default(), &foreign_liquidity_order.order.from);
let updated_uniswap = UpdatedAmmModel {
execution: vec![ExecutedAmmModel {
sell_token: t1,
Expand Down Expand Up @@ -565,6 +580,7 @@ mod tests {
Arc::new(MockAllowanceManaging::new()),
Arc::new(OrderConverter::test(weth)),
SlippageContext::default(),
&Default::default(),
)
.await
.unwrap();
Expand All @@ -582,6 +598,7 @@ mod tests {
owner: H160([99; 20]),
full_fee_amount: 42.into(),
class: OrderClass::Liquidity,
uid: foreign_liquidity_order_uid,
..Default::default()
},
data: OrderData {
Expand Down

0 comments on commit 41ded83

Please sign in to comment.