Skip to content

Commit

Permalink
Merge branch 'main' into solve-delay-debug-logs
Browse files Browse the repository at this point in the history
# Conflicts:
#	crates/driver/src/infra/observe/mod.rs
  • Loading branch information
squadgazzz committed Dec 1, 2023
2 parents 35b9160 + 0c1fcb0 commit bec8b2e
Show file tree
Hide file tree
Showing 16 changed files with 263 additions and 305 deletions.
2 changes: 1 addition & 1 deletion crates/autopilot/src/database/onchain_order_events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ async fn get_quote(
quoter,
&parameters.clone(),
Some(*quote_id),
order_data.fee_amount,
Some(order_data.fee_amount),
)
.await
.map_err(onchain_order_placement_error_from)
Expand Down
6 changes: 1 addition & 5 deletions crates/autopilot/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,7 @@ pub async fn run(args: Arguments) {
block_retriever.clone(),
skip_event_sync_start,
));
let mut maintainers: Vec<Arc<dyn Maintaining>> =
vec![pool_fetcher.clone(), event_updater, Arc::new(db.clone())];
let mut maintainers: Vec<Arc<dyn Maintaining>> = vec![event_updater, Arc::new(db.clone())];

let gas_price_estimator = Arc::new(InstrumentedGasEstimator::new(
shared::gas_price_estimation::create_priority_estimator(
Expand Down Expand Up @@ -538,9 +537,6 @@ pub async fn run(args: Arguments) {
);
maintainers.push(broadcaster_event_updater);
}
if let Some(balancer) = balancer_pool_fetcher {
maintainers.push(balancer);
}
if let Some(uniswap_v3) = uniswap_v3_pool_fetcher {
maintainers.push(uniswap_v3);
}
Expand Down
15 changes: 5 additions & 10 deletions crates/contracts/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,16 +564,11 @@ fn generate_contract_with_config(

println!("cargo:rerun-if-changed={}", path.display());

config(
ContractBuilder::new()
// for some reason formatting the generate code is broken on nightly
.rustfmt(false)
.visibility_modifier("pub"),
)
.generate(&contract)
.unwrap()
.write_to_file(Path::new(&dest).join(format!("{name}.rs")))
.unwrap();
config(ContractBuilder::new().visibility_modifier("pub"))
.generate(&contract)
.unwrap()
.write_to_file(Path::new(&dest).join(format!("{name}.rs")))
.unwrap();
}

fn addr(s: &str) -> Address {
Expand Down
2 changes: 2 additions & 0 deletions crates/contracts/rustfmt.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Applying our custom formatting to the generated code is broken.
# That's why we specifically unset those rules for this directory.
48 changes: 4 additions & 44 deletions crates/driver/src/boundary/liquidity/uniswap/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,9 @@ use {
},
async_trait::async_trait,
contracts::{GPv2Settlement, IUniswapLikeRouter},
ethrpc::{
current_block::{self, CurrentBlockStream},
Web3,
},
futures::StreamExt,
ethrpc::{current_block::CurrentBlockStream, Web3},
shared::{
http_solver::model::TokenAmount,
maintenance::Maintaining,
sources::uniswap_v2::{
pair_provider::PairProvider,
pool_cache::PoolCache,
Expand All @@ -30,9 +25,8 @@ use {
},
std::{
collections::HashSet,
sync::{self, Arc, Mutex},
sync::{Arc, Mutex},
},
tracing::Instrument,
};

/// Median gas used per UniswapInteraction (v2).
Expand Down Expand Up @@ -150,18 +144,11 @@ where
config.missing_pool_cache_time,
);

let pool_cache = Arc::new(PoolCache::new(
Arc::new(PoolCache::new(
boundary::liquidity::cache_config(),
Arc::new(pool_fetcher),
blocks.clone(),
)?);

tokio::task::spawn(
cache_update(blocks.clone(), Arc::downgrade(&pool_cache))
.instrument(tracing::info_span!("uniswap_v2_cache")),
);

pool_cache
)?)
};

Ok(Box::new(UniswapLikeLiquidity::with_allowances(
Expand All @@ -172,33 +159,6 @@ where
)))
}

async fn cache_update(blocks: CurrentBlockStream, pool_cache: sync::Weak<PoolCache>) {
let mut blocks = current_block::into_stream(blocks);
loop {
let block = blocks
.next()
.await
.expect("block stream unexpectedly ended")
.number;

let pool_cache = match pool_cache.upgrade() {
Some(value) => value,
None => {
tracing::debug!("pool cache dropped; stopping update task");
break;
}
};

tracing::info_span!("maintenance", block)
.in_scope(|| async move {
if let Err(err) = pool_cache.run_maintenance().await {
tracing::warn!(?err, "error updating pool cache");
}
})
.await;
}
}

/// An allowance manager that always reports no allowances.
struct NoAllowanceManaging;

Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/infra/observe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn init(log: &str) {

/// Observe a received auction.
pub fn auction(auction: &dto::Auction) {
tracing::debug!(?auction, "received auction");
tracing::debug!(id=?auction.id(), "received auction");
}

/// Observe that liquidity fetching is about to start.
Expand Down
2 changes: 2 additions & 0 deletions crates/e2e/tests/e2e/colocation_quoting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ async fn uses_stale_liquidity(web3: Web3) {

tracing::info!("waiting for liquidity state to update");
wait_for_condition(TIMEOUT, || async {
// Mint blocks until we evict the cached liquidty and fetch the new state.
onchain.mint_block().await;
let next = services.submit_quote(&quote).await.unwrap();
next.quote.buy_amount != first.quote.buy_amount
})
Expand Down
12 changes: 3 additions & 9 deletions crates/orderbook/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use {
fee_subsidy::{config::FeeSubsidyConfiguration, FeeSubsidizing},
gas_price::InstrumentedGasEstimator,
http_client::HttpClientFactory,
maintenance::{Maintaining, ServiceMaintenance},
maintenance::ServiceMaintenance,
metrics::{serve_metrics, DEFAULT_METRICS_PORT},
network::network_name,
oneinch_api::OneInchClientImpl,
Expand Down Expand Up @@ -518,12 +518,9 @@ pub async fn run(args: Arguments) {
ipfs,
));

let mut maintainers = vec![pool_fetcher as Arc<dyn Maintaining>];
if let Some(balancer) = balancer_pool_fetcher {
maintainers.push(balancer);
}
if let Some(uniswap_v3) = uniswap_v3_pool_fetcher {
maintainers.push(uniswap_v3);
let service_maintainer = ServiceMaintenance::new(vec![uniswap_v3]);
task::spawn(service_maintainer.run_maintenance_on_new_block(current_block_stream));
}

check_database_connection(orderbook.as_ref()).await;
Expand All @@ -548,9 +545,6 @@ pub async fn run(args: Arguments) {
native_price_estimator,
);

let service_maintainer = ServiceMaintenance::new(maintainers);
task::spawn(service_maintainer.run_maintenance_on_new_block(current_block_stream));

let mut metrics_address = args.bind_address;
metrics_address.set_port(DEFAULT_METRICS_PORT);
tracing::info!(%metrics_address, "serving metrics");
Expand Down
104 changes: 61 additions & 43 deletions crates/shared/src/order_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,31 +659,32 @@ impl OrderValidating for OrderValidator {
additional_gas: app_data.inner.protocol.hooks.gas_limit(),
verification,
};
let quote = if class == OrderClass::Market {
let quote = get_quote_and_check_fee(
&*self.quoter,
&quote_parameters,
order.quote_id,
data.fee_amount,
)
.await?;
Some(quote)
} else {
// We don't try to get quotes for liquidity and limit orders
// for two reasons:
// 1. They don't pay fees, meaning we don't need to know what the min fee amount
// is.
// 2. We don't really care about the equivalent quote since they aren't expected
// to follow regular order creation flow.
None
let quote = match class {
OrderClass::Market => {
let fee = Some(data.fee_amount);
let quote =
get_quote_and_check_fee(&*self.quoter, &quote_parameters, order.quote_id, fee)
.await?;
Some(quote)
}
OrderClass::Limit(_) => {
let quote =
get_quote_and_check_fee(&*self.quoter, &quote_parameters, order.quote_id, None)
.await?;
Some(quote)
}
OrderClass::Liquidity => None,
};

let full_fee_amount = quote
.as_ref()
.map(|quote| quote.full_fee_amount)
// The `full_fee_amount` should never be lower than the `fee_amount` (which may include
// subsidies). This only makes a difference for liquidity orders.
.unwrap_or(data.fee_amount);
let full_fee_amount = match class {
OrderClass::Market | OrderClass::Liquidity => quote
.as_ref()
.map(|quote| quote.full_fee_amount)
// The `full_fee_amount` should never be lower than the `fee_amount` (which may include
// subsidies). This only makes a difference for liquidity orders.
.unwrap_or(data.fee_amount),
OrderClass::Limit(_) => 0.into(), // limit orders have a solver determined fee
};

let min_balance = minimum_balance(&data).ok_or(ValidationError::SellAmountOverflow)?;

Expand Down Expand Up @@ -733,11 +734,11 @@ impl OrderValidating for OrderValidator {
},
}

// Check if we need to re-classify the order if it is outside the market
// Check if we need to re-classify the market order if it is outside the market
// price. We consider out-of-price orders as liquidity orders. See
// <https://github.com/cowprotocol/services/pull/301>.
let class = match &quote {
Some(quote)
let class = match (class, &quote) {
(OrderClass::Market, Some(quote))
if is_order_outside_market_price(
&quote_parameters.sell_amount,
&quote_parameters.buy_amount,
Expand All @@ -747,7 +748,7 @@ impl OrderValidating for OrderValidator {
tracing::debug!(%uid, ?owner, ?class, "order being flagged as outside market price");
OrderClass::Liquidity
}
_ => class,
(_, _) => class,
};

self.check_max_limit_orders(owner, &class).await?;
Expand Down Expand Up @@ -892,14 +893,31 @@ fn minimum_balance(order: &OrderData) -> Option<U256> {
/// Retrieves the quote for an order that is being created and verify that its
/// fee is sufficient.
///
/// The fee is checked only if `fee_amount` is specified.
pub async fn get_quote_and_check_fee(
quoter: &dyn OrderQuoting,
quote_search_parameters: &QuoteSearchParameters,
quote_id: Option<i64>,
fee_amount: Option<U256>,
) -> Result<Quote, ValidationError> {
let quote = get_or_create_quote(quoter, quote_search_parameters, quote_id).await?;

if fee_amount.is_some_and(|fee| fee < quote.fee_amount) {
return Err(ValidationError::InsufficientFee);
}

Ok(quote)
}

/// Retrieves the quote for an order that is being created
///
/// This works by first trying to find an existing quote, and then falling back
/// to calculating a brand new one if none can be found and a quote ID was not
/// specified.
pub async fn get_quote_and_check_fee(
async fn get_or_create_quote(
quoter: &dyn OrderQuoting,
quote_search_parameters: &QuoteSearchParameters,
quote_id: Option<i64>,
fee_amount: U256,
) -> Result<Quote, ValidationError> {
let quote = match quoter
.find_quote(quote_id, quote_search_parameters.clone())
Expand Down Expand Up @@ -948,10 +966,6 @@ pub async fn get_quote_and_check_fee(
Err(err) => return Err(err.into()),
};

if fee_amount < quote.fee_amount {
return Err(ValidationError::InsufficientFee);
}

Ok(quote)
}

Expand Down Expand Up @@ -1481,7 +1495,7 @@ mod tests {
.validate_and_construct_order(creation_, &domain_separator, Default::default(), None)
.await
.unwrap();
assert_eq!(quote, None);
assert!(quote.is_some());
assert!(order.metadata.class.is_limit());

let creation_ = OrderCreation {
Expand All @@ -1495,7 +1509,7 @@ mod tests {
.validate_and_construct_order(creation_, &domain_separator, Default::default(), None)
.await
.unwrap();
assert_eq!(quote, None);
assert!(quote.is_some());
assert!(order.metadata.class.is_limit());
}

Expand Down Expand Up @@ -2171,7 +2185,7 @@ mod tests {
&order_quoter,
&quote_search_parameters,
quote_id,
fee_amount,
Some(fee_amount),
)
.await
.unwrap();
Expand Down Expand Up @@ -2238,10 +2252,14 @@ mod tests {
})
});

let quote =
get_quote_and_check_fee(&order_quoter, &quote_search_parameters, None, fee_amount)
.await
.unwrap();
let quote = get_quote_and_check_fee(
&order_quoter,
&quote_search_parameters,
None,
Some(fee_amount),
)
.await
.unwrap();

assert_eq!(
quote,
Expand All @@ -2268,7 +2286,7 @@ mod tests {
&order_quoter,
&quote_search_parameters,
Some(0),
U256::zero(),
Some(U256::zero()),
)
.await
.unwrap_err();
Expand All @@ -2290,7 +2308,7 @@ mod tests {
&order_quoter,
&Default::default(),
Default::default(),
U256::one(),
Some(U256::one()),
)
.await
.unwrap_err();
Expand Down Expand Up @@ -2350,7 +2368,7 @@ mod tests {
..Default::default()
},
Default::default(),
U256::zero(),
Some(U256::zero()),
)
.await
.unwrap_err();
Expand Down
Loading

0 comments on commit bec8b2e

Please sign in to comment.