Skip to content

Commit

Permalink
Hopefully fix noisy submission error alert on eden (#481)
Browse files Browse the repository at this point in the history
  • Loading branch information
MartinquaXD authored Sep 1, 2022
1 parent 66299fe commit 2dfc400
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 9 deletions.
12 changes: 11 additions & 1 deletion crates/driver/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ use solver::{
arguments::TransactionStrategyArg, settlement_access_list::AccessListEstimatorType,
solver::ExternalSolverArg,
};
use std::{net::SocketAddr, num::NonZeroU64, time::Duration};
use std::{
net::SocketAddr,
num::{NonZeroU64, NonZeroU8},
time::Duration,
};
use tracing::level_filters::LevelFilter;

#[derive(clap::Parser)]
Expand Down Expand Up @@ -140,6 +144,11 @@ pub struct Arguments {
#[clap(long, env, default_value = "15000000")]
pub simulation_gas_limit: u128,

/// Configures how often the gas price of a transaction may be increased by the minimum amount
/// compared to the previously failing transaction to eventually bring it on chain.
#[clap(long, env, default_value = "1")]
pub max_gas_price_bumps: NonZeroU8,

/// The target confirmation time in seconds for settlement transactions used to estimate gas price.
#[clap(
long,
Expand Down Expand Up @@ -308,6 +317,7 @@ impl std::fmt::Display for Arguments {
display_option(f, "tenderly_url", &self.tenderly_url)?;
display_secret_option(f, "tenderly_api_key", &self.tenderly_api_key)?;
writeln!(f, "simulation_gas_limit: {}", self.simulation_gas_limit)?;
writeln!(f, "max_gas_price_bumps: {}", self.max_gas_price_bumps)?;
writeln!(f, "target_confirm_time: {:?}", self.target_confirm_time)?;
writeln!(
f,
Expand Down
1 change: 1 addition & 0 deletions crates/driver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ async fn build_submitter(common: &CommonComponents, args: &Arguments) -> Arc<Sol
gas_price_cap: args.gas_price_cap,
transaction_strategies,
access_list_estimator: common.access_list_estimator.clone(),
max_gas_price_bumps: args.max_gas_price_bumps,
})
}

Expand Down
3 changes: 2 additions & 1 deletion crates/e2e/tests/e2e/eth_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use solver::{
GlobalTxPool, SolutionSubmitter, StrategyArgs,
},
};
use std::{sync::Arc, time::Duration};
use std::{num::NonZeroU8, sync::Arc, time::Duration};
use web3::signing::SecretKeyRef;

const TRADER_BUY_ETH_A_PK: [u8; 32] = [1; 32];
Expand Down Expand Up @@ -245,6 +245,7 @@ async fn eth_integration(web3: Web3) {
.await
.unwrap(),
),
max_gas_price_bumps: NonZeroU8::new(1).unwrap(),
},
create_orderbook_api(),
create_order_converter(&web3, contracts.weth.address()),
Expand Down
3 changes: 2 additions & 1 deletion crates/e2e/tests/e2e/onchain_settlement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use solver::{
GlobalTxPool, SolutionSubmitter, StrategyArgs,
},
};
use std::{sync::Arc, time::Duration};
use std::{num::NonZeroU8, sync::Arc, time::Duration};
use web3::signing::SecretKeyRef;

const TRADER_A_PK: [u8; 32] =
Expand Down Expand Up @@ -250,6 +250,7 @@ async fn onchain_settlement(web3: Web3) {
.await
.unwrap(),
),
max_gas_price_bumps: NonZeroU8::new(1).unwrap(),
},
create_orderbook_api(),
create_order_converter(&web3, contracts.weth.address()),
Expand Down
3 changes: 2 additions & 1 deletion crates/e2e/tests/e2e/settlement_without_onchain_liquidity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use solver::{
GlobalTxPool, SolutionSubmitter, StrategyArgs,
},
};
use std::{sync::Arc, time::Duration};
use std::{num::NonZeroU8, sync::Arc, time::Duration};
use web3::signing::SecretKeyRef;

const TRADER_A_PK: [u8; 32] =
Expand Down Expand Up @@ -239,6 +239,7 @@ async fn onchain_settlement_without_liquidity(web3: Web3) {
.await
.unwrap(),
),
max_gas_price_bumps: NonZeroU8::new(1).unwrap(),
},
create_orderbook_api(),
create_order_converter(&web3, contracts.weth.address()),
Expand Down
3 changes: 2 additions & 1 deletion crates/e2e/tests/e2e/smart_contract_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use solver::{
GlobalTxPool, SolutionSubmitter, StrategyArgs,
},
};
use std::{sync::Arc, time::Duration};
use std::{num::NonZeroU8, sync::Arc, time::Duration};
use web3::signing::SecretKeyRef;

const TRADER: [u8; 32] = [1; 32];
Expand Down Expand Up @@ -276,6 +276,7 @@ async fn smart_contract_orders(web3: Web3) {
.await
.unwrap(),
),
max_gas_price_bumps: NonZeroU8::new(1).unwrap(),
},
create_orderbook_api(),
create_order_converter(&web3, contracts.weth.address()),
Expand Down
3 changes: 2 additions & 1 deletion crates/e2e/tests/e2e/vault_balances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use solver::{
GlobalTxPool, SolutionSubmitter, StrategyArgs,
},
};
use std::{sync::Arc, time::Duration};
use std::{num::NonZeroU8, sync::Arc, time::Duration};
use web3::signing::SecretKeyRef;

const TRADER: [u8; 32] = [1; 32];
Expand Down Expand Up @@ -189,6 +189,7 @@ async fn vault_balances(web3: Web3) {
.await
.unwrap(),
),
max_gas_price_bumps: NonZeroU8::new(1).unwrap(),
},
create_orderbook_api(),
create_order_converter(&web3, contracts.weth.address()),
Expand Down
8 changes: 7 additions & 1 deletion crates/solver/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
use primitive_types::H160;
use reqwest::Url;
use shared::arguments::{display_list, display_option, display_secret_option};
use std::time::Duration;
use std::{num::NonZeroU8, time::Duration};

#[derive(clap::Parser)]
pub struct Arguments {
Expand Down Expand Up @@ -271,6 +271,11 @@ pub struct Arguments {
#[clap(long, env, default_value = "15000000")]
pub simulation_gas_limit: u128,

/// Configures how often the gas price of a transaction may be increased by the minimum amount
/// compared to the previously failing transaction to eventually bring it on chain.
#[clap(long, env, default_value = "1")]
pub max_gas_price_bumps: NonZeroU8,

/// In order to protect against malicious solvers, the driver will check that settlements prices do not
/// exceed a max price deviation compared to the external prices of the driver, if this optional value is set.
/// The max deviation value should be provided as a float percentage value. E.g. for a max price deviation
Expand Down Expand Up @@ -383,6 +388,7 @@ impl std::fmt::Display for Arguments {
)?;
writeln!(f, "weth_unwrap_factor: {}", self.weth_unwrap_factor)?;
writeln!(f, "simulation_gas_limit: {}", self.simulation_gas_limit)?;
writeln!(f, "max_gas_price_bumps: {}", self.max_gas_price_bumps)?;
display_option(
f,
"max_settlement_price_deviation",
Expand Down
1 change: 1 addition & 0 deletions crates/solver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ async fn main() {
gas_price_cap: args.gas_price_cap,
transaction_strategies,
access_list_estimator,
max_gas_price_bumps: args.max_gas_price_bumps,
};
let api = OrderBookApi::new(
args.orderbook_url,
Expand Down
3 changes: 3 additions & 0 deletions crates/solver/src/settlement_submission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use primitive_types::{H256, U256};
use shared::Web3;
use std::{
collections::HashMap,
num::NonZeroU8,
sync::{Arc, Mutex},
time::{Duration, Instant},
};
Expand Down Expand Up @@ -106,6 +107,7 @@ pub struct SolutionSubmitter {
pub retry_interval: Duration,
pub gas_price_cap: f64,
pub transaction_strategies: Vec<TransactionStrategy>,
pub max_gas_price_bumps: NonZeroU8,
}

pub struct StrategyArgs {
Expand Down Expand Up @@ -197,6 +199,7 @@ impl SolutionSubmitter {
&gas_price_estimator,
self.access_list_estimator.as_ref(),
strategy_args.sub_tx_pool.clone(),
self.max_gas_price_bumps
)?;
submitter.submit(settlement.clone(), params).await
}
Expand Down
35 changes: 33 additions & 2 deletions crates/solver/src/settlement_submission/submitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use primitive_types::{H256, U256};
use shared::{Web3, Web3Transport};
use std::{
fmt,
num::NonZeroU8,
time::{Duration, Instant},
};
use web3::types::{AccessList, TransactionReceipt, U64};
Expand Down Expand Up @@ -192,6 +193,7 @@ pub struct Submitter<'a> {
gas_price_estimator: &'a SubmitterGasPriceEstimator<'a>,
access_list_estimator: &'a dyn AccessListEstimating,
submitted_transactions: SubTxPoolRef,
max_gas_price_bumps: NonZeroU8,
}

impl<'a> Submitter<'a> {
Expand All @@ -202,6 +204,7 @@ impl<'a> Submitter<'a> {
gas_price_estimator: &'a SubmitterGasPriceEstimator<'a>,
access_list_estimator: &'a dyn AccessListEstimating,
submitted_transactions: SubTxPoolRef,
max_gas_price_bumps: NonZeroU8,
) -> Result<Self> {
Ok(Self {
contract,
Expand All @@ -210,6 +213,7 @@ impl<'a> Submitter<'a> {
gas_price_estimator,
access_list_estimator,
submitted_transactions,
max_gas_price_bumps,
})
}
}
Expand Down Expand Up @@ -381,6 +385,8 @@ impl<'a> Submitter<'a> {
let submitter_name = self.submit_api.name();
let target_confirm_time = Instant::now() + params.target_confirm_time;

let mut allowed_gas_price_bumps = 1i32;

tracing::debug!(
"submit_with_increasing_gas_prices_until_simulation_fails entered with submitter: {}",
submitter_name
Expand Down Expand Up @@ -453,7 +459,9 @@ impl<'a> Submitter<'a> {

if let Err(err) = method.clone().view().call().await {
if let Some((_, previous_gas_price)) = transactions.last() {
let gas_price = previous_gas_price.bump(GAS_PRICE_BUMP).ceil();
let gas_price = previous_gas_price
.bump(GAS_PRICE_BUMP.powi(allowed_gas_price_bumps))
.ceil();
match self.cancel_transaction(&gas_price, nonce).await {
Ok(handle) => transactions.push((handle, gas_price)),
Err(err) => tracing::warn!("cancellation failed: {:?}", err),
Expand All @@ -467,7 +475,18 @@ impl<'a> Submitter<'a> {
.last()
.map(|(_, previous_gas_price)| previous_gas_price)
{
let previous_gas_price = previous_gas_price.bump(GAS_PRICE_BUMP).ceil();
// Sometimes a tx gets successfully submitted but the API returns an error. When that
// happens the gas price computation will return a gas price which is not big enough to
// replace the supposedly not submitted tx. To get out of that issue the new gas price
// has to be bumped by at least `GAS_PRICE_BUMP ^ 2` in order to replace the stuck tx.
let previous_gas_price = previous_gas_price
.bump(GAS_PRICE_BUMP.powi(allowed_gas_price_bumps))
.ceil();
tracing::debug!(
?previous_gas_price,
allowed_gas_price_bumps,
"minimum gas price"
);
if gas_price.max_priority_fee_per_gas < previous_gas_price.max_priority_fee_per_gas
|| gas_price.max_fee_per_gas < previous_gas_price.max_fee_per_gas
{
Expand All @@ -494,12 +513,23 @@ impl<'a> Submitter<'a> {
"submitted transaction",
);
transactions.push((handle, gas_price));
allowed_gas_price_bumps = 1;
}
Err(err) => {
tracing::warn!(
submitter = %submitter_name, ?err,
"submission failed",
);
let err = err.to_string();
if err.contains("underpriced") || err.contains("already known") {
allowed_gas_price_bumps = std::cmp::min(
allowed_gas_price_bumps + 1,
self.max_gas_price_bumps.get() as i32,
);
tracing::debug!(allowed_gas_price_bumps, "bump gas price exponent");
} else {
allowed_gas_price_bumps = 1;
}
}
}
tokio::time::sleep(params.retry_interval).await;
Expand Down Expand Up @@ -728,6 +758,7 @@ mod tests {
&gas_price_estimator,
access_list_estimator.as_ref(),
submitted_transactions,
NonZeroU8::new(1).unwrap(),
)
.unwrap();

Expand Down

0 comments on commit 2dfc400

Please sign in to comment.