Address code review

crates/fuel-core/src/service/adapters/producer.rs

@@ -150,36 +150,16 @@ impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter {
         }
     }
 
-    async fn get_cost_for_block(&self, height: &DaBlockHeight) -> anyhow::Result<u64> {
-        #[cfg(feature = "relayer")]
-        {
-            if let Some(sync) = self.relayer_synced.as_ref() {
-                get_gas_cost_for_height(**height, sync)
-            } else {
-                Ok(0)
-            }
-        }
-        #[cfg(not(feature = "relayer"))]
-        {
-            anyhow::ensure!(
-                **height == 0,
-                "Cannot have a da height above zero without a relayer"
-            );
-            // If the relayer is not enabled, then all blocks are zero.
-            Ok(0)
-        }
-    }
-
-    async fn get_transactions_number_for_block(
+    async fn get_cost_and_transactions_number_for_block(
         &self,
         height: &DaBlockHeight,
-    ) -> anyhow::Result<u64> {
+    ) -> anyhow::Result<(u64, u64)> {
         #[cfg(feature = "relayer")]
         {
             if let Some(sync) = self.relayer_synced.as_ref() {
-                get_transactions_number_for_height(**height, sync)
+                get_gas_cost_and_transactions_number_for_height(**height, sync)
             } else {
-                Ok(0)
+                Ok((0, 0))
             }
         }
         #[cfg(not(feature = "relayer"))]
@@ -189,53 +169,38 @@ impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter {
                 "Cannot have a da height above zero without a relayer"
             );
             // If the relayer is not enabled, then all blocks are zero.
-            Ok(0)
+            Ok((0, 0))
         }
     }
 }
 
 #[cfg(feature = "relayer")]
-fn get_transactions_number_for_height(
-    height: u64,
-    sync: &fuel_core_relayer::SharedState<
-        crate::database::Database<
-            crate::database::database_description::relayer::Relayer,
-        >,
-    >,
-) -> anyhow::Result<u64> {
-    let da_height = DaBlockHeight(height);
-    let transactions = sync
-        .database()
-        .storage::<fuel_core_relayer::storage::EventsHistory>()
-        .get(&da_height)?
-        .unwrap_or_default()
-        .iter()
-        .filter(|event| match event {
-            fuel_core_types::services::relayer::Event::Message(_) => false,
-            fuel_core_types::services::relayer::Event::Transaction(_) => true,
-        })
-        .count();
-    Ok(transactions as u64)
-}
-
-#[cfg(feature = "relayer")]
-fn get_gas_cost_for_height(
+fn get_gas_cost_and_transactions_number_for_height(
     height: u64,
     sync: &fuel_core_relayer::SharedState<
         crate::database::Database<
             crate::database::database_description::relayer::Relayer,
         >,
     >,
-) -> anyhow::Result<u64> {
+) -> anyhow::Result<(u64, u64)> {
     let da_height = DaBlockHeight(height);
-    let cost = sync
+    let cost_with_transactions = sync
         .database()
         .storage::<fuel_core_relayer::storage::EventsHistory>()
         .get(&da_height)?
         .unwrap_or_default()
         .iter()
-        .fold(0u64, |acc, event| acc.saturating_add(event.cost()));
-    Ok(cost)
+        .fold((0u64, 0u64), |(cost, transactions), event| {
+            let cost = cost.saturating_add(event.cost());
+            let transactions = match event {
+                fuel_core_types::services::relayer::Event::Message(_) => transactions,
+                fuel_core_types::services::relayer::Event::Transaction(_) => {
+                    transactions.saturating_add(1)
+                }
+            };
+            (cost, transactions)
+        });
+    Ok(cost_with_transactions)
 }
 
 impl fuel_core_producer::ports::BlockProducerDatabase for OnChainIterableKeyValueView {

crates/services/executor/src/executor.rs

@@ -576,7 +576,7 @@ where
                 if transaction.max_gas(&self.consensus_params)? > remaining_gas_limit {
                     data.skipped_transactions
                         .push((tx_id, ExecutorError::GasOverflow));
-                    continue
+                    continue;
                 }
                 match self.execute_transaction_and_commit(
                     block,
@@ -858,7 +858,7 @@ where
                             .insert(message.nonce(), &message)?;
                         execution_data
                             .events
-                            .push(ExecutorEvent::MessageImported(message))
+                            .push(ExecutorEvent::MessageImported(message));
                     }
                     Event::Transaction(relayed_tx) => {
                         let id = relayed_tx.id();

crates/services/producer/src/block_producer.rs

@@ -392,34 +392,25 @@ where
                 best: highest,
                 previous_block: previous_da_height,
             }
-            .into())
+            .into());
         }
 
         if highest == previous_da_height {
-            return Ok(highest)
+            return Ok(highest);
         }
 
         let next_da_height = previous_da_height.saturating_add(1);
         for height in next_da_height..=highest.0 {
-            let cost = self
+            let (cost, transactions_number) = self
                 .relayer
-                .get_cost_for_block(&DaBlockHeight(height))
+                .get_cost_and_transactions_number_for_block(&DaBlockHeight(height))
                 .await?;
             total_cost = total_cost.saturating_add(cost);
-            if total_cost > gas_limit {
+            total_transactions = total_transactions.saturating_add(transactions_number);
+            if total_cost > gas_limit || total_transactions > transactions_limit {
                 break;
             } else {
-                let transactions_number = self
-                    .relayer
-                    .get_transactions_number_for_block(&DaBlockHeight(height))
-                    .await?;
-                total_transactions =
-                    total_transactions.saturating_add(transactions_number);
-                if total_transactions > transactions_limit {
-                    break;
-                } else {
-                    new_best = DaBlockHeight(height);
-                }
+                new_best = DaBlockHeight(height);
             }
         }
         if new_best == previous_da_height {

crates/services/producer/src/block_producer/tests.rs

@@ -846,8 +846,7 @@ impl<Executor> TestContext<Executor> {
 
 struct TestContextBuilder {
     latest_block_height: DaBlockHeight,
-    blocks_with_gas_costs: HashMap<DaBlockHeight, u64>,
-    blocks_with_transactions: HashMap<DaBlockHeight, u64>,
+    blocks_with_gas_costs_and_transactions_number: HashMap<DaBlockHeight, (u64, u64)>,
     prev_da_height: DaBlockHeight,
     block_gas_limit: Option<u64>,
     prev_height: BlockHeight,
@@ -857,8 +856,7 @@ impl TestContextBuilder {
     fn new() -> Self {
         Self {
             latest_block_height: 0u64.into(),
-            blocks_with_gas_costs: HashMap::new(),
-            blocks_with_transactions: HashMap::new(),
+            blocks_with_gas_costs_and_transactions_number: HashMap::new(),
             prev_da_height: 1u64.into(),
             block_gas_limit: None,
             prev_height: 0u32.into(),
@@ -870,21 +868,47 @@ impl TestContextBuilder {
         self
     }
 
+    fn with_latest_blocks_with_gas_costs_and_transactions_number(
+        mut self,
+        latest_blocks_with_gas_costs_and_transactions: impl Iterator<
+            Item = (DaBlockHeight, (u64, u64)),
+        >,
+    ) -> Self {
+        self.blocks_with_gas_costs_and_transactions_number
+            .extend(latest_blocks_with_gas_costs_and_transactions);
+        self
+    }
+
+    // Helper function that can be used in tests where transaction numbers in a da block are irrelevant
     fn with_latest_blocks_with_gas_costs(
         mut self,
         latest_blocks_with_gas_costs: impl Iterator<Item = (DaBlockHeight, u64)>,
     ) -> Self {
-        self.blocks_with_gas_costs
-            .extend(latest_blocks_with_gas_costs);
+        let latest_blocks_with_gas_costs_and_transactions_number =
+            latest_blocks_with_gas_costs
+                .into_iter()
+                .map(|(da_block_height, gas_costs)| (da_block_height, (gas_costs, 0)));
+        // Assigning `self` necessary to avoid the compiler complaining about the mutability of `self`
+        self = self.with_latest_blocks_with_gas_costs_and_transactions_number(
+            latest_blocks_with_gas_costs_and_transactions_number,
+        );
         self
     }
 
+    // Helper function that can be used in tests where gas costs in a da block are irrelevant
     fn with_latest_blocks_with_transactions(
         mut self,
         latest_blocks_with_transactions: impl Iterator<Item = (DaBlockHeight, u64)>,
     ) -> Self {
-        self.blocks_with_transactions
-            .extend(latest_blocks_with_transactions);
+        let latest_blocks_with_gas_costs_and_transactions_number =
+            latest_blocks_with_transactions.into_iter().map(
+                |(da_block_height, transactions)| (da_block_height, (0, transactions)),
+            );
+
+        // Assigning `self` necessary to avoid the compiler complaining about the mutability of `self`
+        self = self.with_latest_blocks_with_gas_costs_and_transactions_number(
+            latest_blocks_with_gas_costs_and_transactions_number,
+        );
         self
     }
 
@@ -934,8 +958,9 @@ impl TestContextBuilder {
 
         let mock_relayer = MockRelayer {
             latest_block_height: self.latest_block_height,
-            latest_da_blocks_with_costs: self.blocks_with_gas_costs.clone(),
-            latest_da_blocks_with_transactions: self.blocks_with_transactions.clone(),
+            latest_da_blocks_with_costs_and_transactions_number: self
+                .blocks_with_gas_costs_and_transactions_number
+                .clone(),
             ..Default::default()
         };
 
@@ -977,8 +1002,9 @@ impl TestContextBuilder {
 
         let mock_relayer = MockRelayer {
             latest_block_height: self.latest_block_height,
-            latest_da_blocks_with_costs: self.blocks_with_gas_costs.clone(),
-            latest_da_blocks_with_transactions: self.blocks_with_transactions.clone(),
+            latest_da_blocks_with_costs_and_transactions_number: self
+                .blocks_with_gas_costs_and_transactions_number
+                .clone(),
             ..Default::default()
         };
 

crates/services/producer/src/mocks.rs

@@ -57,8 +57,8 @@ use std::{
 pub struct MockRelayer {
     pub block_production_key: Address,
     pub latest_block_height: DaBlockHeight,
-    pub latest_da_blocks_with_costs: HashMap<DaBlockHeight, u64>,
-    pub latest_da_blocks_with_transactions: HashMap<DaBlockHeight, u64>,
+    pub latest_da_blocks_with_costs_and_transactions_number:
+        HashMap<DaBlockHeight, (u64, u64)>,
 }
 
 #[async_trait::async_trait]
@@ -71,25 +71,16 @@ impl Relayer for MockRelayer {
         Ok(highest)
     }
 
-    async fn get_cost_for_block(&self, height: &DaBlockHeight) -> anyhow::Result<u64> {
-        let cost = self
-            .latest_da_blocks_with_costs
-            .get(height)
-            .cloned()
-            .unwrap_or_default();
-        Ok(cost)
-    }
-
-    async fn get_transactions_number_for_block(
+    async fn get_cost_and_transactions_number_for_block(
         &self,
         height: &DaBlockHeight,
-    ) -> anyhow::Result<u64> {
-        let cost = self
-            .latest_da_blocks_with_transactions
+    ) -> anyhow::Result<(u64, u64)> {
+        let cost_with_transactions_number = self
+            .latest_da_blocks_with_costs_and_transactions_number
             .get(height)
             .cloned()
             .unwrap_or_default();
-        Ok(cost)
+        Ok(cost_with_transactions_number)
     }
 }
 

crates/services/producer/src/ports.rs

@@ -71,13 +71,10 @@ pub trait Relayer: Send + Sync {
     ) -> anyhow::Result<DaBlockHeight>;
 
     /// Get the total Forced Transaction gas cost for the block at the given height.
-    async fn get_cost_for_block(&self, height: &DaBlockHeight) -> anyhow::Result<u64>;
-
-    /// Get the total number of Forced Transactions for the block at the given height.
-    async fn get_transactions_number_for_block(
+    async fn get_cost_and_transactions_number_for_block(
         &self,
         height: &DaBlockHeight,
-    ) -> anyhow::Result<u64>;
+    ) -> anyhow::Result<(u64, u64)>;
 }
 
 pub trait BlockProducer<TxSource>: Send + Sync {

crates/services/upgradable-executor/src/executor.rs

@@ -662,7 +662,7 @@ where
     ) -> ExecutorResult<wasmtime::Module> {
         let guard = self.cached_modules.lock();
         if let Some(module) = guard.get(&version) {
-            return Ok(module.clone())
+            return Ok(module.clone());
         }
         drop(guard);