Fix the Fallback Gas Limiter (#217)

* Add code to disable the fallback gas filler

* Allow benchmarks driver to await tx receipts

* Improve the transaction submission logic

* Update Python Script to process Geth benchmarks

crates/core/src/differential_benchmarks/driver.rs

@@ -1,6 +1,5 @@
 use std::{
     collections::HashMap,
-    ops::ControlFlow,
     sync::{
         Arc,
         atomic::{AtomicUsize, Ordering},
@@ -13,6 +12,7 @@ use alloy::{
     json_abi::JsonAbi,
     network::{Ethereum, TransactionBuilder},
     primitives::{Address, TxHash, U256},
+    providers::Provider,
     rpc::types::{
         TransactionReceipt, TransactionRequest,
         trace::geth::{
@@ -22,12 +22,9 @@ use alloy::{
     },
 };
 use anyhow::{Context as _, Result, bail};
-use futures::TryFutureExt;
+use futures::{FutureExt as _, TryFutureExt};
 use indexmap::IndexMap;
-use revive_dt_common::{
-    futures::{PollingWaitBehavior, poll},
-    types::PrivateKeyAllocator,
-};
+use revive_dt_common::types::PrivateKeyAllocator;
 use revive_dt_format::{
     metadata::{ContractInstance, ContractPathAndIdent},
     steps::{
@@ -37,7 +34,7 @@ use revive_dt_format::{
     traits::{ResolutionContext, ResolverApi},
 };
 use tokio::sync::{Mutex, OnceCell, mpsc::UnboundedSender};
-use tracing::{Instrument, Span, debug, error, field::display, info, info_span, instrument};
+use tracing::{Span, debug, error, field::display, info, instrument};
 
 use crate::{
     differential_benchmarks::{ExecutionState, WatcherEvent},
@@ -73,6 +70,10 @@ pub struct Driver<'a, I> {
     /// The number of steps that were executed on the driver.
     steps_executed: usize,
 
+    /// This function controls if the driver should wait for transactions to be included in a block
+    /// or not before proceeding forward.
+    await_transaction_inclusion: bool,
+
     /// This is the queue of steps that are to be executed by the driver for this test case. Each
     /// time `execute_step` is called one of the steps is executed.
     steps_iterator: I,
@@ -89,6 +90,7 @@ where
         private_key_allocator: Arc<Mutex<PrivateKeyAllocator>>,
         cached_compiler: &CachedCompiler<'a>,
         watcher_tx: UnboundedSender<WatcherEvent>,
+        await_transaction_inclusion: bool,
         steps: I,
     ) -> Result<Self> {
         let mut this = Driver {
@@ -104,6 +106,7 @@ where
             execution_state: ExecutionState::empty(),
             steps_executed: 0,
             steps_iterator: steps,
+            await_transaction_inclusion,
             watcher_tx,
         };
         this.init_execution_state(cached_compiler)
@@ -166,7 +169,7 @@ where
                 code,
             );
             let receipt = self
-                .execute_transaction(tx, None)
+                .execute_transaction(tx, None, Duration::from_secs(5 * 60))
                 .and_then(|(_, receipt_fut)| receipt_fut)
                 .await
                 .inspect_err(|err| {
@@ -365,7 +368,30 @@ where
                 let tx = step
                     .as_transaction(self.resolver.as_ref(), self.default_resolution_context())
                     .await?;
-                Ok(self.execute_transaction(tx, Some(step_path)).await?.0)
+
+                let (tx_hash, receipt_future) = self
+                    .execute_transaction(tx.clone(), Some(step_path), Duration::from_secs(30 * 60))
+                    .await?;
+                if self.await_transaction_inclusion {
+                    let receipt = receipt_future
+                        .await
+                        .context("Failed while waiting for transaction inclusion in block")?;
+
+                    if !receipt.status() {
+                        error!(
+                            ?tx,
+                            tx.hash = %receipt.transaction_hash,
+                            ?receipt,
+                            "Encountered a failing benchmark transaction"
+                        );
+                        bail!(
+                            "Encountered a failing transaction in benchmarks: {}",
+                            receipt.transaction_hash
+                        )
+                    }
+                }
+
+                Ok(tx_hash)
             }
         }
     }
@@ -466,6 +492,7 @@ where
                         .collect::<Vec<_>>();
                     steps.into_iter()
                 },
+                await_transaction_inclusion: self.await_transaction_inclusion,
                 watcher_tx: self.watcher_tx.clone(),
             })
             .map(|driver| driver.execute_all());
@@ -632,7 +659,7 @@ where
         };
 
         let receipt = match self
-            .execute_transaction(tx, step_path)
+            .execute_transaction(tx, step_path, Duration::from_secs(5 * 60))
             .and_then(|(_, receipt_fut)| receipt_fut)
             .await
         {
@@ -677,18 +704,33 @@ where
     #[instrument(
         level = "info",
         skip_all,
-        fields(driver_id = self.driver_id, transaction_hash = tracing::field::Empty)
+        fields(
+            driver_id = self.driver_id,
+            transaction = ?transaction,
+            transaction_hash = tracing::field::Empty
+        ),
+        err(Debug)
     )]
     async fn execute_transaction(
         &self,
         transaction: TransactionRequest,
         step_path: Option<&StepPath>,
+        receipt_wait_duration: Duration,
     ) -> anyhow::Result<(TxHash, impl Future<Output = Result<TransactionReceipt>>)> {
         let node = self.platform_information.node;
-        let transaction_hash = node
-            .submit_transaction(transaction)
+        let provider = node.provider().await.context("Creating provider failed")?;
+
+        let pending_transaction_builder = provider
+            .send_transaction(transaction)
             .await
             .context("Failed to submit transaction")?;
+
+        let transaction_hash = *pending_transaction_builder.tx_hash();
+        let receipt_future = pending_transaction_builder
+            .with_timeout(Some(receipt_wait_duration))
+            .with_required_confirmations(2)
+            .get_receipt()
+            .map(|res| res.context("Failed to get the receipt of the transaction"));
         Span::current().record("transaction_hash", display(transaction_hash));
 
         info!("Submitted transaction");
@@ -701,28 +743,7 @@ where
                 .context("Failed to send the transaction hash to the watcher")?;
         };
 
-        Ok((transaction_hash, async move {
-            info!("Starting to poll for transaction receipt");
-            poll(
-                Duration::from_secs(30 * 60),
-                PollingWaitBehavior::Constant(Duration::from_secs(1)),
-                || {
-                    async move {
-                        match node.get_receipt(transaction_hash).await {
-                            Ok(receipt) => {
-                                info!("Polling succeeded, receipt found");
-                                Ok(ControlFlow::Break(receipt))
-                            }
-                            Err(_) => Ok(ControlFlow::Continue(())),
-                        }
-                    }
-                    .instrument(info_span!("Polling for receipt"))
-                },
-            )
-            .instrument(info_span!("Polling for receipt", %transaction_hash))
-            .await
-            .inspect(|_| info!("Found the transaction receipt"))
-        }))
+        Ok((transaction_hash, receipt_future))
     }
     // endregion:Transaction Execution
 }

crates/core/src/differential_benchmarks/entry_point.rs

@@ -160,6 +160,7 @@ pub async fn handle_differential_benchmarks(
                 private_key_allocator,
                 cached_compiler.as_ref(),
                 watcher_tx.clone(),
+                context.await_transaction_inclusion,
                 test_definition
                     .case
                     .steps_iterator_for_benchmarks(context.default_repetition_count)

crates/core/src/differential_benchmarks/watcher.rs

@@ -139,23 +139,18 @@ impl Watcher {
                         break;
                     }
 
-                    info!(
-                        block_number = block.ethereum_block_information.block_number,
-                        block_tx_count = block.ethereum_block_information.transaction_hashes.len(),
-                        remaining_transactions = watch_for_transaction_hashes.read().await.len(),
-                        "Observed a block"
-                    );
-
                     // Remove all of the transaction hashes observed in this block from the txs we
                     // are currently watching for.
                     let mut watch_for_transaction_hashes =
                         watch_for_transaction_hashes.write().await;
+                    let mut relevant_transactions_observed = 0;
                     for tx_hash in block.ethereum_block_information.transaction_hashes.iter() {
                         let Some((step_path, submission_time)) =
                             watch_for_transaction_hashes.remove(tx_hash)
                         else {
                             continue;
                         };
+                        relevant_transactions_observed += 1;
                         let transaction_information = TransactionInformation {
                             transaction_hash: *tx_hash,
                             submission_timestamp: submission_time
@@ -172,6 +167,14 @@ impl Watcher {
                             )
                             .expect("Can't fail")
                     }
+
+                    info!(
+                        block_number = block.ethereum_block_information.block_number,
+                        block_tx_count = block.ethereum_block_information.transaction_hashes.len(),
+                        relevant_transactions_observed,
+                        remaining_transactions = watch_for_transaction_hashes.len(),
+                        "Observed a block"
+                    );
                 }
 
                 info!("Watcher's Block Watching Task Finished");

crates/core/src/lib.rs

@@ -91,7 +91,8 @@ impl Platform for GethEvmSolcPlatform {
         let genesis_configuration = AsRef::<GenesisConfiguration>::as_ref(&context);
         let genesis = genesis_configuration.genesis()?.clone();
         Ok(thread::spawn(move || {
-            let node = GethNode::new(context);
+            let use_fallback_gas_filler = matches!(context, Context::Test(..));
+            let node = GethNode::new(context, use_fallback_gas_filler);
             let node = spawn_node::<GethNode>(node, genesis)?;
             Ok(Box::new(node) as Box<_>)
         }))
@@ -145,7 +146,8 @@ impl Platform for LighthouseGethEvmSolcPlatform {
         let genesis_configuration = AsRef::<GenesisConfiguration>::as_ref(&context);
         let genesis = genesis_configuration.genesis()?.clone();
         Ok(thread::spawn(move || {
-            let node = LighthouseGethNode::new(context);
+            let use_fallback_gas_filler = matches!(context, Context::Test(..));
+            let node = LighthouseGethNode::new(context, use_fallback_gas_filler);
             let node = spawn_node::<LighthouseGethNode>(node, genesis)?;
             Ok(Box::new(node) as Box<_>)
         }))
@@ -206,12 +208,14 @@ impl Platform for ReviveDevNodePolkavmResolcPlatform {
 
         let genesis = genesis_configuration.genesis()?.clone();
         Ok(thread::spawn(move || {
+            let use_fallback_gas_filler = matches!(context, Context::Test(..));
             let node = SubstrateNode::new(
                 revive_dev_node_path,
                 SubstrateNode::REVIVE_DEV_NODE_EXPORT_CHAINSPEC_COMMAND,
                 Some(revive_dev_node_consensus),
                 context,
                 &eth_rpc_connection_strings,
+                use_fallback_gas_filler,
             );
             let node = spawn_node(node, genesis)?;
             Ok(Box::new(node) as Box<_>)
@@ -274,12 +278,14 @@ impl Platform for ReviveDevNodeRevmSolcPlatform {
 
         let genesis = genesis_configuration.genesis()?.clone();
         Ok(thread::spawn(move || {
+            let use_fallback_gas_filler = matches!(context, Context::Test(..));
             let node = SubstrateNode::new(
                 revive_dev_node_path,
                 SubstrateNode::REVIVE_DEV_NODE_EXPORT_CHAINSPEC_COMMAND,
                 Some(revive_dev_node_consensus),
                 context,
                 &eth_rpc_connection_strings,
+                use_fallback_gas_filler,
             );
             let node = spawn_node(node, genesis)?;
             Ok(Box::new(node) as Box<_>)
@@ -338,7 +344,9 @@ impl Platform for ZombienetPolkavmResolcPlatform {
             .clone();
         let genesis = genesis_configuration.genesis()?.clone();
         Ok(thread::spawn(move || {
-            let node = ZombienetNode::new(polkadot_parachain_path, context);
+            let use_fallback_gas_filler = matches!(context, Context::Test(..));
+            let node =
+                ZombienetNode::new(polkadot_parachain_path, context, use_fallback_gas_filler);
             let node = spawn_node(node, genesis)?;
             Ok(Box::new(node) as Box<_>)
         }))
@@ -395,7 +403,9 @@ impl Platform for ZombienetRevmSolcPlatform {
             .clone();
         let genesis = genesis_configuration.genesis()?.clone();
         Ok(thread::spawn(move || {
-            let node = ZombienetNode::new(polkadot_parachain_path, context);
+            let use_fallback_gas_filler = matches!(context, Context::Test(..));
+            let node =
+                ZombienetNode::new(polkadot_parachain_path, context, use_fallback_gas_filler);
             let node = spawn_node(node, genesis)?;
             Ok(Box::new(node) as Box<_>)
         }))