fix: ensure DA transaction throttling uses params that are set

Author: danyalprout

Cargo.lock

@@ -128,6 +128,7 @@ op-alloy-rpc-types-engine = { version = "0.14.1", default-features = false }
 op-alloy-rpc-jsonrpsee = { version = "0.14.1", default-features = false }
 op-alloy-network = { version = "0.14.1", default-features = false }
 op-alloy-consensus = { version = "0.14.1", default-features = false }
+op-alloy-flz = { version = "0.13.0", default-features = false }
 
 async-trait = { version = "0.1.83" }
 clap = { version = "4.4.3", features = ["derive", "env"] }

Cargo.toml

@@ -59,6 +59,7 @@ alloy-op-evm.workspace = true
 op-alloy-consensus.workspace = true
 op-alloy-rpc-types-engine.workspace = true
 op-alloy-network.workspace = true
+op-alloy-flz.workspace = true
 
 revm.workspace = true
 op-revm.workspace = true

crates/op-rbuilder/Cargo.toml

@@ -1097,9 +1097,7 @@ where
                 }
             };
 
-            // add gas used by the transaction to cumulative gas used, before creating the receipt
-            let gas_used = result.gas_used();
-            info.cumulative_gas_used += gas_used;
+            info.track_transaction_resource_usage(sequencer_tx.inner(), &result);
 
             let ctx = ReceiptBuilderCtx {
                 tx: sequencer_tx.inner(),
@@ -1164,7 +1162,7 @@ where
 
             // A sequencer's block should never contain blob or deposit transactions from the pool.
             if tx.is_eip4844() || tx.is_deposit() {
-                println!("B");
+                warn!(target: "payload_builder", ?tx, "Unexpected transaction type.");
                 best_txs.mark_invalid(tx.signer(), tx.nonce());
                 continue;
             }
@@ -1209,9 +1207,8 @@ where
                 trace!(target: "payload_builder", ?tx, "reverted transaction");
             }
 
-            // add gas used by the transaction to cumulative gas used, before creating the receipt
+            info.track_transaction_resource_usage(tx.inner(), &result);
             let gas_used = result.gas_used();
-            info.cumulative_gas_used += gas_used;
 
             let ctx = ReceiptBuilderCtx {
                 tx: tx.inner(),

crates/op-rbuilder/src/payload_builder.rs

@@ -993,9 +993,7 @@ where
                 }
             };
 
-            // add gas used by the transaction to cumulative gas used, before creating the receipt
-            let gas_used = result.gas_used();
-            info.cumulative_gas_used += gas_used;
+            info.track_transaction_resource_usage(sequencer_tx.inner(), &result);
 
             let ctx = ReceiptBuilderCtx {
                 tx: sequencer_tx.inner(),
@@ -1102,10 +1100,8 @@ where
                 continue;
             }
 
-            // add gas used by the transaction to cumulative gas used, before creating the
-            // receipt
+            info.track_transaction_resource_usage(tx.inner(), &result);
             let gas_used = result.gas_used();
-            info.cumulative_gas_used += gas_used;
 
             // Push transaction changeset and calculate header bloom filter for receipt.
             let ctx = ReceiptBuilderCtx {
@@ -1174,9 +1170,7 @@ where
                     .transact(&builder_tx)
                     .map_err(|err| PayloadBuilderError::EvmExecutionError(Box::new(err)))?;
 
-                // Add gas used by the transaction to cumulative gas used, before creating the receipt
-                let gas_used = result.gas_used();
-                info.cumulative_gas_used += gas_used;
+                info.track_transaction_resource_usage(builder_tx.inner(), &result);
 
                 let ctx = ReceiptBuilderCtx {
                     tx: builder_tx.inner(),

crates/op-rbuilder/src/payload_builder_vanilla.rs

@@ -1,8 +1,11 @@
 //! Heavily influenced by [reth](https://github.com/paradigmxyz/reth/blob/1e965caf5fa176f244a31c0d2662ba1b590938db/crates/optimism/payload/src/builder.rs#L570)
 use alloy_consensus::Transaction;
-use alloy_primitives::{private::alloy_rlp::Encodable, Address, TxHash, U256};
+use alloy_eips::Encodable2718;
+use alloy_primitives::{Address, TxHash, U256};
+use op_revm::OpHaltReason;
 use reth_node_api::NodePrimitives;
 use reth_optimism_primitives::OpReceipt;
+use revm::context::result::ExecutionResult;
 use std::collections::HashSet;
 
 /// Holds the state after execution
@@ -62,16 +65,242 @@ impl<N: NodePrimitives> ExecutionInfo<N> {
         tx_data_limit: Option<u64>,
         block_data_limit: Option<u64>,
     ) -> bool {
-        if tx_data_limit.is_some_and(|da_limit| tx.length() as u64 > da_limit) {
+        if self.cumulative_gas_used + tx.gas_limit() > block_gas_limit {
             return true;
         }
 
-        if block_data_limit
-            .is_some_and(|da_limit| self.cumulative_da_bytes_used + (tx.length() as u64) > da_limit)
-        {
+        if tx_data_limit.is_none() && block_data_limit.is_none() {
+            return false;
+        }
+
+        let tx_compressed_size = op_alloy_flz::flz_compress_len(tx.encoded_2718().as_slice());
+        if tx_data_limit.is_some_and(|da_limit| tx_compressed_size as u64 > da_limit) {
             return true;
         }
 
-        self.cumulative_gas_used + tx.gas_limit() > block_gas_limit
+        if block_data_limit.is_some_and(|da_limit| {
+            self.cumulative_da_bytes_used + (tx_compressed_size as u64) > da_limit
+        }) {
+            return true;
+        }
+
+        false
+    }
+
+    /// Increments the current usage trackers (gas, DA) for a transaction that has been included.
+    pub fn track_transaction_resource_usage(
+        &mut self,
+        tx: &N::SignedTx,
+        result: &ExecutionResult<OpHaltReason>,
+    ) {
+        self.cumulative_gas_used += result.gas_used();
+        self.cumulative_da_bytes_used +=
+            op_alloy_flz::flz_compress_len(tx.encoded_2718().as_slice()) as u64;
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use alloy_consensus::{SignableTransaction, TxEip1559};
+    use alloy_eips::Encodable2718;
+    use alloy_primitives::private::alloy_rlp::Encodable;
+    use alloy_primitives::{Bytes, Signature};
+    use rand::RngCore;
+    use reth::revm::context::result::SuccessReason;
+    use reth_optimism_primitives::{OpPrimitives, OpTransactionSigned};
+    use revm::context::result::Output;
+
+    #[test]
+    fn test_block_gas_limit() {
+        let gas_limit = 100;
+        let info = ExecutionInfo::<OpPrimitives>::with_capacity(10);
+
+        let allowable_transaction: OpTransactionSigned = TxEip1559 {
+            gas_limit: gas_limit - 10,
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        assert_eq!(
+            false,
+            info.is_tx_over_limits(&allowable_transaction, gas_limit, None, None)
+        );
+
+        let too_much_gas: OpTransactionSigned = TxEip1559 {
+            gas_limit: gas_limit + 10,
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        assert_eq!(
+            true,
+            info.is_tx_over_limits(&too_much_gas, gas_limit, None, None)
+        );
+    }
+
+    fn gen_random_bytes(size: usize) -> alloy_primitives::bytes::Bytes {
+        let mut rng = rand::thread_rng();
+        let mut vec = vec![0u8; size];
+        rng.fill_bytes(&mut vec);
+        vec.into()
+    }
+
+    #[test]
+    fn test_tx_da_size() {
+        let allowable_transaction: OpTransactionSigned = TxEip1559 {
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        let over_tx_da_limits: OpTransactionSigned = TxEip1559 {
+            input: gen_random_bytes(2000).into(),
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        let large_but_compressable: OpTransactionSigned = TxEip1559 {
+            input: vec![0u8; 2000].into(),
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        let max_tx_size: u64 = 1000;
+
+        // Sanity check compressed and uncompressed transaction sizes
+        // Uncompressed, the large transactions are the same size, but the all zero one compresses
+        // 90+% and should be included when DA throttling is active
+        assert_eq!(
+            78,
+            op_alloy_flz::flz_compress_len(allowable_transaction.encoded_2718().as_slice())
+        );
+        assert_eq!(81, allowable_transaction.length());
+        assert_eq!(
+            108,
+            op_alloy_flz::flz_compress_len(large_but_compressable.encoded_2718().as_slice())
+        );
+        assert_eq!(2085, large_but_compressable.length());
+        // Relative check as the randomness of the data may change the compressed size
+        assert_eq!(
+            true,
+            max_tx_size
+                < op_alloy_flz::flz_compress_len(over_tx_da_limits.encoded_2718().as_slice())
+                    as u64
+        );
+        assert_eq!(2085, over_tx_da_limits.length());
+
+        let info = ExecutionInfo::<OpPrimitives>::with_capacity(10);
+
+        assert_eq!(
+            false,
+            info.is_tx_over_limits(&allowable_transaction, 10000, Some(max_tx_size), None)
+        );
+        assert_eq!(
+            false,
+            info.is_tx_over_limits(&large_but_compressable, 10000, Some(max_tx_size), None)
+        );
+        assert_eq!(
+            true,
+            info.is_tx_over_limits(&over_tx_da_limits, 10000, Some(max_tx_size), None)
+        );
+
+        // When no DA specific limits are set, large transactions are allowable
+        assert_eq!(
+            false,
+            info.is_tx_over_limits(&over_tx_da_limits, 10000, None, None)
+        );
+    }
+
+    #[test]
+    fn test_block_da_limit() {
+        let block_data_limit = 1000;
+        let mut info = ExecutionInfo::<OpPrimitives>::with_capacity(10);
+
+        let large_transaction: OpTransactionSigned = TxEip1559 {
+            input: gen_random_bytes(2000).into(),
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        let large_but_compressable: OpTransactionSigned = TxEip1559 {
+            input: vec![0u8; 2000].into(),
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        // Sanity check compressed and uncompressed transaction sizes
+        assert_eq!(
+            108,
+            op_alloy_flz::flz_compress_len(large_but_compressable.encoded_2718().as_slice())
+        );
+        assert_eq!(2085, large_but_compressable.length());
+        // Relative check as the randomness of the data may change the compressed size
+        assert_eq!(
+            true,
+            block_data_limit
+                < op_alloy_flz::flz_compress_len(large_transaction.encoded_2718().as_slice())
+                    as u64
+        );
+        assert_eq!(2085, large_transaction.length());
+
+        assert_eq!(
+            true,
+            info.is_tx_over_limits(&large_transaction, 1000, None, Some(block_data_limit))
+        );
+        assert_eq!(
+            false,
+            info.is_tx_over_limits(&large_but_compressable, 1000, None, Some(block_data_limit))
+        );
+
+        // Block level DA inclusion should take into account the current amount of DA bytes used in the block
+        info.cumulative_da_bytes_used += 990;
+        assert_eq!(
+            true,
+            info.is_tx_over_limits(&large_but_compressable, 1000, None, Some(block_data_limit))
+        );
+    }
+
+    #[test]
+    pub fn test_track_resource_usage() {
+        let txn_gas_limit = 250;
+
+        let txn: OpTransactionSigned = TxEip1559 {
+            input: vec![0u8; 2000].into(),
+            gas_limit: txn_gas_limit,
+            ..TxEip1559::default()
+        }
+        .into_signed(Signature::test_signature())
+        .into();
+
+        let expected_compressed_size: u64 = 112;
+        assert_eq!(
+            expected_compressed_size,
+            op_alloy_flz::flz_compress_len(txn.encoded_2718().as_slice()) as u64
+        );
+
+        let mut info = ExecutionInfo::<OpPrimitives>::with_capacity(10);
+
+        let result = &ExecutionResult::<OpHaltReason>::Success {
+            reason: SuccessReason::Return,
+            gas_used: 100,
+            gas_refunded: 0,
+            logs: vec![],
+            output: Output::Call(Bytes(vec![].into())),
+        };
+
+        assert_eq!(0, info.cumulative_gas_used);
+        assert_eq!(0, info.cumulative_da_bytes_used);
+
+        info.track_transaction_resource_usage(&txn, &result);
+
+        assert_eq!(100, info.cumulative_gas_used);
+        assert_eq!(expected_compressed_size, info.cumulative_da_bytes_used);
     }
 }