fix bugs regarding tx gas limit (#1323)

* fix bugs regarding tx gas limit

* group gas limit capping functionality to a function

crates/evm/src/query.rs

@@ -57,7 +57,7 @@ const ESTIMATE_GAS_ERROR_RATIO: f64 = 0.015;
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 pub(crate) struct EstimatedTxExpenses {
     /// Evm gas used.
-    gas_used: U64,
+    pub gas_used: U64,
     /// Base fee of the L2 block when tx was executed.
     base_fee: U256,
     /// L1 fee.
@@ -684,7 +684,7 @@ impl<C: sov_modules_api::Context> Evm<C> {
 
         Ok(AccessListWithGasUsed {
             access_list,
-            gas_used: estimated.gas_with_l1_overhead(),
+            gas_used: gas_limit_to_return(U64::from(block_env.gas_limit), estimated),
         })
     }
 
@@ -735,7 +735,17 @@ impl<C: sov_modules_api::Context> Evm<C> {
         working_set: &mut WorkingSet<C>,
     ) -> RpcResult<reth_primitives::U256> {
         let estimated = self.estimate_tx_expenses(request, block_number, working_set)?;
-        Ok(estimated.gas_with_l1_overhead())
+
+        // TODO: this assumes all blocks have the same gas limit
+        // if gas limit ever changes this should be updated
+        let last_block = self
+            .blocks
+            .last(&mut working_set.accessory_state())
+            .expect("Head block must be set");
+
+        let block_gas_limit = U64::from(last_block.header.gas_limit);
+
+        Ok(gas_limit_to_return(block_gas_limit, estimated))
     }
 
     /// Handler for: `eth_estimateDiffSize`
@@ -1706,6 +1716,30 @@ fn set_state_to_end_of_evm_block<C: sov_modules_api::Context>(
     working_set.set_archival_version(block_number + 1);
 }
 
+/// We add some kind of L1 fee overhead to the estimated gas
+/// so that the receiver can check if their balance is enough to
+/// cover L1 fees, without ever knowing about L1 fees.
+///
+/// However, there is a chance that the real gas used is not bigger than
+/// block gas limit, but it is bigger with the overhead added. In this
+/// case the mempool will reject the transaction and even if it didn't, the sequencer
+/// wouldn't put it into a block since it's against EVM rules to hava tx that
+/// has more gas limit than the block.
+///
+/// But in the case where the gas used is already bigger than the block gas limit
+/// we want to return the gas estimation as is since the mempool will reject it
+/// anyway.
+#[inline]
+fn gas_limit_to_return(block_gas_limit: U64, estimated_tx_expenses: EstimatedTxExpenses) -> U256 {
+    if estimated_tx_expenses.gas_used > block_gas_limit {
+        estimated_tx_expenses.gas_with_l1_overhead()
+    } else {
+        let with_l1_overhead = estimated_tx_expenses.gas_with_l1_overhead();
+
+        with_l1_overhead.min(U256::from(block_gas_limit))
+    }
+}
+
 /// Creates the next blocks `BlockEnv` based on the latest block
 /// Also updates `Evm::latest_block_hashes` with the new block hash
 fn get_pending_block_env<C: sov_modules_api::Context>(
@@ -1745,3 +1779,32 @@ fn get_pending_block_env<C: sov_modules_api::Context>(
 
     block_env
 }
+
+#[test]
+fn test_gas_limit_to_return() {
+    assert_eq!(
+        gas_limit_to_return(
+            U64::from(8_000_000),
+            EstimatedTxExpenses {
+                gas_used: U64::from(5_000_000),
+                base_fee: U256::from(10000000), // 0.01 gwei
+                l1_fee: U256::from(40_000_000_000_000u128),
+                l1_diff_size: 1 // not relevant to this test
+            }
+        ),
+        U256::from(8_000_000)
+    );
+
+    assert_eq!(
+        gas_limit_to_return(
+            U64::from(8_000_000),
+            EstimatedTxExpenses {
+                gas_used: U64::from(8_000_001),
+                base_fee: U256::from(10000000), // 0.01 gwei
+                l1_fee: U256::from(40_000_000u128),
+                l1_diff_size: 1 // not relevant to this test
+            }
+        ),
+        U256::from(8_000_005)
+    );
+}

crates/sequencer/src/mempool.rs

@@ -85,6 +85,9 @@ impl<C: sov_modules_api::Context> CitreaMempool<C> {
         let validator = TransactionValidationTaskExecutor::eth_builder(Arc::new(chain_spec))
             .no_cancun()
             .no_eip4844()
+            // TODO: if we ever increase block gas limits, we need to pull this from
+            // somewhere else
+            .set_block_gas_limit(evm_config.block_gas_limit)
             .set_shanghai(true)
             .with_additional_tasks(0)
             .build_with_tasks(client, TokioTaskExecutor::default(), blob_store);