From a9bd63a159b11df705c5a704897f28f61f09151a Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Wed, 17 Jan 2024 19:23:16 +0100 Subject: [PATCH] comments --- .../domain/competition/solution/settlement.rs | 4 +- crates/driver/src/domain/eth/mod.rs | 9 +++ crates/driver/src/domain/mempools.rs | 71 +++++++++++-------- crates/driver/src/infra/blockchain/mod.rs | 34 ++++++++- crates/driver/src/infra/mempool/mod.rs | 3 + crates/driver/src/infra/simulator/mod.rs | 14 +--- crates/e2e/src/setup/colocation.rs | 1 + crates/e2e/tests/e2e/univ2.rs | 1 + 8 files changed, 90 insertions(+), 47 deletions(-) diff --git a/crates/driver/src/domain/competition/solution/settlement.rs b/crates/driver/src/domain/competition/solution/settlement.rs index abf36987f9..8a97b9f4a7 100644 --- a/crates/driver/src/domain/competition/solution/settlement.rs +++ b/crates/driver/src/domain/competition/solution/settlement.rs @@ -399,9 +399,9 @@ impl Gas { self.limit * self.fee_per_gas() } + // Compute an upper bound for `max_fee_per_gas` for the given settlement. fn fee_per_gas(&self) -> eth::FeePerGas { - // Compute an upper bound for `max_fee_per_gas` for the given - // settlement. We multiply a fixed factor of the current base fee per + // We multiply a fixed factor of the current base fee per // gas, which is chosen to be the maximum possible increase to the base // fee per gas over 12 blocks, also including the "tip". // diff --git a/crates/driver/src/domain/eth/mod.rs b/crates/driver/src/domain/eth/mod.rs index 1800a035b7..86ea183208 100644 --- a/crates/driver/src/domain/eth/mod.rs +++ b/crates/driver/src/domain/eth/mod.rs @@ -337,6 +337,15 @@ impl From for TxId { } } +pub enum TxStatus { + /// The transaction has been included and executed successfully. + Executed, + /// The transaction has been included but execution failed. + Reverted, + /// The transaction has not been included yet. + Pending, +} + /// An onchain transaction. #[derive(Clone)] pub struct Tx { diff --git a/crates/driver/src/domain/mempools.rs b/crates/driver/src/domain/mempools.rs index 4ef5027e43..8d3df8d782 100644 --- a/crates/driver/src/domain/mempools.rs +++ b/crates/driver/src/domain/mempools.rs @@ -1,7 +1,10 @@ use { super::{competition, eth}, crate::{ - domain::{competition::solution::Settlement, eth::FeePerGas}, + domain::{ + competition::solution::Settlement, + eth::{FeePerGas, TxStatus}, + }, infra::{self, observe, solver::Solver, Ethereum}, }, ethrpc::current_block::into_stream, @@ -79,44 +82,52 @@ impl Mempools { settlement: &Settlement, ) -> Result { let eth = &self.1; - let start = std::time::Instant::now(); - let tx = settlement.boundary.tx( - settlement.auction_id, - self.1.contracts().settlement(), - competition::solution::settlement::Internalization::Enable, - ); + let deadline = tokio::time::Instant::now() + mempool.config().max_confirm_time; + let tx = eth::Tx { + // boundary.tx() does not populate the access list + access_list: settlement.access_list.clone(), + ..settlement.boundary.tx( + settlement.auction_id, + self.1.contracts().settlement(), + competition::solution::settlement::Internalization::Enable, + ) + }; let hash = mempool.submit(tx.clone(), settlement.gas, solver).await?; let mut block_stream = into_stream(eth.current_block().clone()); loop { - match eth.transaction_receipt(&hash).await.unwrap_or_else(|err| { - tracing::warn!( - "failed to get transaction receipt for tx {:?}: {:?}", - hash, - err - ); - None - }) { - Some(true) => return Ok(hash), - Some(false) => return Err(Error::Revert(hash)), - None => { - // Check if too late - if start.elapsed() >= mempool.config().max_confirm_time { - tracing::warn!("tx {:?} not confirmed in time, cancelling", hash,); - self.cancel(mempool, settlement.gas.price, solver).await?; - return Err(Error::Expired); - } + // Wait for the next block to be mined or we time out. Block stream immediately + // yields the latest block, thus the first iteration starts immediately. + if let Err(_) = tokio::time::timeout_at(deadline, block_stream.next()).await { + tracing::info!(?hash, "tx not confirmed in time, cancelling"); + self.cancel(mempool, settlement.gas.price, solver).await?; + return Err(Error::Expired); + } + tracing::debug!(?hash, "checking if tx is confirmed"); + let receipt = eth.transaction_receipt(&hash).await.unwrap_or_else(|err| { + tracing::warn!(?hash, ?err, "failed to get transaction receipt",); + TxStatus::Pending + }); + match receipt { + TxStatus::Executed => return Ok(hash), + TxStatus::Reverted => return Err(Error::Revert(hash)), + TxStatus::Pending => { // Check if transaction still simulates if let Err(err) = eth.estimate_gas(tx.clone()).await { - tracing::warn!("tx started failing in mempool {:?}: {:?}", hash, err); - self.cancel(mempool, settlement.gas.price, solver).await?; - return Err(Error::SimulationRevert); + if err.is_revert() { + tracing::info!( + ?hash, + ?err, + "tx started failing in mempool, cancelling" + ); + self.cancel(mempool, settlement.gas.price, solver).await?; + return Err(Error::SimulationRevert); + } else { + tracing::warn!(?hash, ?err, "couldn't re-simulate tx"); + } } } } - - // Wait for the next block to be mined. - block_stream.next().await.expect("blockchains never end"); } } diff --git a/crates/driver/src/infra/blockchain/mod.rs b/crates/driver/src/infra/blockchain/mod.rs index 13b8bf15ec..c3e8ded5cc 100644 --- a/crates/driver/src/infra/blockchain/mod.rs +++ b/crates/driver/src/infra/blockchain/mod.rs @@ -12,7 +12,7 @@ pub mod contracts; pub mod gas; pub mod token; -use gas_estimation::GasPriceEstimating; +use {ethcontract::errors::ExecutionError, gas_estimation::GasPriceEstimating}; pub use self::{contracts::Contracts, gas::GasPriceEstimator}; @@ -194,12 +194,24 @@ impl Ethereum { /// If the transaction has been confirmed returns its execution status /// (success or failure) of None if the transaction is not yet confirmed. - pub async fn transaction_receipt(&self, tx_hash: ð::TxId) -> Result, Error> { + pub async fn transaction_receipt(&self, tx_hash: ð::TxId) -> Result { self.web3 .eth() .transaction_receipt(tx_hash.0) .await - .map(|result| result.map(|receipt| receipt.status == Some(1.into()))) + .map(|result| match result { + Some(web3::types::TransactionReceipt { + status: Some(status), + .. + }) => { + if status.is_zero() { + eth::TxStatus::Reverted + } else { + eth::TxStatus::Executed + } + } + _ => eth::TxStatus::Pending, + }) .map_err(Into::into) } } @@ -227,6 +239,22 @@ pub enum Error { AccessList(serde_json::Value), } +impl Error { + // Returns whether the error indicates that the original transaction reverted. + pub fn is_revert(&self) -> bool { + // This behavior is node dependent + match self { + Error::Method(error) => matches!(error.inner, ExecutionError::Revert(_)), + Error::Web3(inner) => { + let error = ExecutionError::from(inner.clone()); + matches!(error, ExecutionError::Revert(_)) + } + Error::GasPrice(_) => false, + Error::AccessList(_) => true, + } + } +} + impl From for Error { fn from(err: contracts::Error) -> Self { match err { diff --git a/crates/driver/src/infra/mempool/mod.rs b/crates/driver/src/infra/mempool/mod.rs index 7e25ebd53c..031f03aff8 100644 --- a/crates/driver/src/infra/mempool/mod.rs +++ b/crates/driver/src/infra/mempool/mod.rs @@ -69,7 +69,10 @@ impl Inner { max_fee_per_gas: gas.price.max.into(), max_priority_fee_per_gas: gas.price.tip.into(), }) + .data(tx.input.into()) + .value(tx.value.0) .gas(gas.limit.0) + .access_list(web3::types::AccessList::from(tx.access_list)) .send() .await .map(|result| eth::TxId(result.hash())) diff --git a/crates/driver/src/infra/simulator/mod.rs b/crates/driver/src/infra/simulator/mod.rs index 9a4d345e74..88f38b4cef 100644 --- a/crates/driver/src/infra/simulator/mod.rs +++ b/crates/driver/src/infra/simulator/mod.rs @@ -3,7 +3,6 @@ use { domain::eth, infra::blockchain::{self, Ethereum}, }, - ethcontract::errors::ExecutionError, observe::future::Measure, }; @@ -181,22 +180,13 @@ where let tx = match &err { SimulatorError::Tenderly(tenderly::Error::Http(_)) => None, SimulatorError::Tenderly(tenderly::Error::Revert(_)) => Some(tx), - SimulatorError::Blockchain(blockchain::Error::Method(error)) - if matches!(error.inner, ExecutionError::Revert(_)) => - { - Some(tx) - } - SimulatorError::Blockchain(blockchain::Error::Method(_)) => None, - SimulatorError::Blockchain(blockchain::Error::Web3(inner)) => { - let error = ExecutionError::from(inner.clone()); - if matches!(error, ExecutionError::Revert(_)) { + SimulatorError::Blockchain(error) => { + if error.is_revert() { Some(tx) } else { None } } - SimulatorError::Blockchain(blockchain::Error::GasPrice(_)) => None, - SimulatorError::Blockchain(blockchain::Error::AccessList(_)) => Some(tx), SimulatorError::Enso(enso::Error::Http(_)) => None, SimulatorError::Enso(enso::Error::Revert(_)) => Some(tx), }; diff --git a/crates/e2e/src/setup/colocation.rs b/crates/e2e/src/setup/colocation.rs index 845f1b4eb9..800af69249 100644 --- a/crates/e2e/src/setup/colocation.rs +++ b/crates/e2e/src/setup/colocation.rs @@ -92,6 +92,7 @@ missing-pool-cache-time = "1h" [submission] gas-price-cap = 1000000000000 +logic = "native" [[submission.mempool]] mempool = "public" diff --git a/crates/e2e/tests/e2e/univ2.rs b/crates/e2e/tests/e2e/univ2.rs index 58335da635..c4d31ef8e4 100644 --- a/crates/e2e/tests/e2e/univ2.rs +++ b/crates/e2e/tests/e2e/univ2.rs @@ -92,6 +92,7 @@ async fn test(web3: Web3) { onchain.mint_blocks_past_reorg_threshold().await; let cip_20_data_updated = || async { + onchain.mint_block().await; let data = match crate::database::most_recent_cip_20_data(services.db()).await { Some(data) => data, None => return false,