Add txpool test

Fix DA config, set it via miner
Extend tests

Author: Solar Mithril

Cargo.lock

@@ -108,6 +108,9 @@ tikv-jemallocator = { version = "0.6", optional = true }
 vergen = { workspace = true, features = ["build", "cargo", "emit_and_set"] }
 vergen-git2.workspace = true
 
+[dev-dependencies]
+alloy-provider = {workspace = true, default-features = true, features = ["txpool-api"]}
+
 [features]
 default = ["jemalloc"]
 

crates/op-rbuilder/Cargo.toml

@@ -58,10 +58,6 @@ pub struct OpRbuilderArgs {
         env = "PLAYGROUND_DIR",
     )]
     pub playground: Option<PathBuf>,
-    #[arg(long = "builder.max-da-tx-size", default_value = "0")]
-    pub max_da_tx_size: u64,
-    #[arg(long = "builder.max-da-block-size", default_value = "0")]
-    pub max_da_block_size: u64,
 }
 
 fn expand_path(s: &str) -> Result<PathBuf, String> {

crates/op-rbuilder/src/args/op.rs

@@ -51,16 +51,15 @@ fn main() {
         .run(|builder, builder_args| async move {
             let rollup_args = builder_args.rollup_args;
 
-            let da_config =
-                OpDAConfig::new(builder_args.max_da_tx_size, builder_args.max_da_block_size);
+            let da_config = OpDAConfig::default();
             let op_node = OpNode::new(rollup_args.clone());
             let handle = builder
                 .with_types::<OpNode>()
                 .with_components(op_node.components().payload(CustomOpPayloadBuilder::new(
                     builder_args.builder_signer,
                     std::time::Duration::from_secs(builder_args.extra_block_deadline_secs),
                     builder_args.enable_revert_protection,
-                    da_config,
+                    da_config.clone(),
                     builder_args.flashblocks_ws_url,
                     builder_args.chain_block_time,
                     builder_args.flashblock_block_time,
@@ -69,6 +68,7 @@ fn main() {
                     OpAddOnsBuilder::default()
                         .with_sequencer(rollup_args.sequencer.clone())
                         .with_enable_tx_conditional(rollup_args.enable_tx_conditional)
+                        .with_da_config(da_config)
                         .build(),
                 )
                 .on_node_started(move |ctx| {

crates/op-rbuilder/src/main.rs

@@ -372,4 +372,12 @@ impl BlockGenerated {
     pub fn includes(&self, tx_hash: B256) -> bool {
         self.block.transactions.hashes().any(|hash| hash == tx_hash)
     }
+
+    pub fn includes_vec(&self, tx_hashes: Vec<B256>) -> bool {
+        tx_hashes.iter().all(|hash| self.includes(*hash))
+    }
+
+    pub fn not_includes_vec(&self, tx_hashes: Vec<B256>) -> bool {
+        tx_hashes.iter().all(|hash| self.not_includes(*hash))
+    }
 }

crates/op-rbuilder/src/tests/framework/blocks.rs

@@ -25,8 +25,8 @@ pub struct TestHarnessBuilder {
     flashblocks_ws_url: Option<String>,
     chain_block_time: Option<u64>,
     flashbots_block_time: Option<u64>,
-    max_da_tx_size: Option<u64>,
-    max_da_block_size: Option<u64>,
+    namespaces: Option<String>,
+    extra_params: Option<String>,
 }
 
 impl TestHarnessBuilder {
@@ -37,8 +37,8 @@ impl TestHarnessBuilder {
             flashblocks_ws_url: None,
             chain_block_time: None,
             flashbots_block_time: None,
-            max_da_tx_size: None,
-            max_da_block_size: None,
+            namespaces: None,
+            extra_params: None,
         }
     }
 
@@ -62,13 +62,13 @@ impl TestHarnessBuilder {
         self
     }
 
-    pub fn with_max_da_tx_size(mut self, max_da_tx_size: u64) -> Self {
-        self.max_da_tx_size = Some(max_da_tx_size);
+    pub fn with_namespaces(mut self, namespaces: &str) -> Self {
+        self.namespaces = Some(namespaces.to_string());
         self
     }
 
-    pub fn with_max_da_block_size(mut self, max_da_block_size: u64) -> Self {
-        self.max_da_block_size = Some(max_da_block_size);
+    pub fn with_extra_params(mut self, extra_params: &str) -> Self {
+        self.extra_params = Some(extra_params.to_string());
         self
     }
 
@@ -83,7 +83,7 @@ impl TestHarnessBuilder {
         std::fs::write(&genesis_path, genesis)?;
 
         // create the builder
-        let builder_data_dir = std::env::temp_dir().join(Uuid::new_v4().to_string());
+        let builder_data_dir: PathBuf = std::env::temp_dir().join(Uuid::new_v4().to_string());
         let builder_auth_rpc_port = get_available_port();
         let builder_http_port = get_available_port();
         let mut op_rbuilder_config = OpRbuilderConfig::new()
@@ -94,8 +94,8 @@ impl TestHarnessBuilder {
             .http_port(builder_http_port)
             .with_builder_private_key(BUILDER_PRIVATE_KEY)
             .with_revert_protection(self.use_revert_protection)
-            .with_max_da_block_size(self.max_da_block_size)
-            .with_max_da_tx_size(self.max_da_tx_size);
+            .with_namespaces(self.namespaces)
+            .with_extra_params(self.extra_params);
         if let Some(flashblocks_ws_url) = self.flashblocks_ws_url {
             op_rbuilder_config = op_rbuilder_config.with_flashblocks_ws_url(&flashblocks_ws_url);
         }

crates/op-rbuilder/src/tests/framework/harness.rs

@@ -27,8 +27,8 @@ pub struct OpRbuilderConfig {
     chain_block_time: Option<u64>,
     flashbots_block_time: Option<u64>,
     with_revert_protection: Option<bool>,
-    max_da_tx_size: Option<u64>,
-    max_da_block_size: Option<u64>,
+    namespaces: Option<String>,
+    extra_params: Option<String>,
 }
 
 impl OpRbuilderConfig {
@@ -86,13 +86,13 @@ impl OpRbuilderConfig {
         self
     }
 
-    pub fn with_max_da_tx_size(mut self, size: Option<u64>) -> Self {
-        self.max_da_tx_size = size;
+    pub fn with_namespaces(mut self, namespaces: Option<String>) -> Self {
+        self.namespaces = namespaces;
         self
     }
 
-    pub fn with_max_da_block_size(mut self, size: Option<u64>) -> Self {
-        self.max_da_block_size = size;
+    pub fn with_extra_params(mut self, extra_params: Option<String>) -> Self {
+        self.extra_params = extra_params;
         self
     }
 }
@@ -167,14 +167,12 @@ impl Service for OpRbuilderConfig {
                 .arg(flashbots_block_time.to_string());
         }
 
-        if let Some(max_da_tx_size) = self.max_da_tx_size {
-            cmd.arg("--builder.max-da-tx-size")
-                .arg(max_da_tx_size.to_string());
+        if let Some(namespaces) = &self.namespaces {
+            cmd.arg("--http.api").arg(namespaces);
         }
 
-        if let Some(max_da_block_size) = self.max_da_block_size {
-            cmd.arg("--builder.max-da-block-size")
-                .arg(max_da_block_size.to_string());
+        if let Some(extra_params) = &self.extra_params {
+            cmd.args(extra_params.split_ascii_whitespace());
         }
 
         cmd
@@ -232,7 +230,7 @@ impl OpRethConfig {
 
 impl Service for OpRethConfig {
     fn command(&self) -> Command {
-        let bin_path = PathBuf::from("op-reth");
+        let bin_path = PathBuf::from("/home/msozin/neth/op-rbuilder/op-reth");
 
         let mut cmd = Command::new(bin_path);
         let jwt_path = get_or_create_jwt_path(self.jwt_secret_path.as_ref());

crates/op-rbuilder/src/tests/framework/op.rs

@@ -1,24 +1,33 @@
 use crate::tests::framework::TestHarnessBuilder;
+use alloy_provider::Provider;
 
 /// This test ensures that the transaction size limit is respected.
 /// We will set limit to 1 byte and see that the builder will not include any transactions.
 #[tokio::test]
 async fn data_availability_tx_size_limit() -> eyre::Result<()> {
     let harness = TestHarnessBuilder::new("data_availability_tx_size_limit")
-        .with_max_da_tx_size(1)
+        .with_namespaces("admin,eth,miner")
         .build()
         .await?;
 
     let mut generator = harness.block_generator().await?;
 
-    // generate regualr tx
-    let invalid_tx = harness.send_valid_transaction().await?;
+    // Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction
+    let call = harness
+        .provider()?
+        .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (1, 0))
+        .await?;
+    assert!(call, "miner_setMaxDASize should be executed successfully");
 
+    let unfit_tx = harness
+        .create_transaction()
+        .with_max_priority_fee_per_gas(50)
+        .send()
+        .await?;
     let block = generator.generate_block().await?;
-
     // tx should not be included because we set the tx_size_limit to 1
     assert!(
-        block.not_includes(*invalid_tx.tx_hash()),
+        block.not_includes(*unfit_tx.tx_hash()),
         "transaction should not be included in the block"
     );
 
@@ -30,20 +39,24 @@ async fn data_availability_tx_size_limit() -> eyre::Result<()> {
 #[tokio::test]
 async fn data_availability_block_size_limit() -> eyre::Result<()> {
     let harness = TestHarnessBuilder::new("data_availability_block_size_limit")
-        .with_max_da_block_size(1)
+        .with_namespaces("admin,eth,miner")
         .build()
         .await?;
 
     let mut generator = harness.block_generator().await?;
 
-    // generate regualr tx
-    let invalid_tx = harness.send_valid_transaction().await?;
+    // Set block da size to be small, so it won't include tx
+    let call = harness
+        .provider()?
+        .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (0, 1))
+        .await?;
+    assert!(call, "miner_setMaxDASize should be executed successfully");
 
+    let unfit_tx = harness.send_valid_transaction().await?;
     let block = generator.generate_block().await?;
-
     // tx should not be included because we set the tx_size_limit to 1
     assert!(
-        block.not_includes(*invalid_tx.tx_hash()),
+        block.not_includes(*unfit_tx.tx_hash()),
         "transaction should not be included in the block"
     );
 
@@ -56,29 +69,38 @@ async fn data_availability_block_size_limit() -> eyre::Result<()> {
 /// We should not forget about builder transaction so we will spawn only 2 regular txs.
 #[tokio::test]
 async fn data_availability_block_fill() -> eyre::Result<()> {
-    let harness = TestHarnessBuilder::new("data_availability_block_fill")
-        .with_max_da_block_size(100000000 * 3)
+    let harness = TestHarnessBuilder::new("data_availability_tx_size_limit")
+        .with_namespaces("admin,eth,miner")
         .build()
         .await?;
 
     let mut generator = harness.block_generator().await?;
 
-    // generate regualr tx
-    let valid_tx_1 = harness.send_valid_transaction().await?;
-    let valid_tx_2 = harness.send_valid_transaction().await?;
-    let unfit_tx_3 = harness.send_valid_transaction().await?;
+    // Set block big enough so it could fit 3 transactions without tx size limit
+    let call = harness
+        .provider()?
+        .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (0, 100000000 * 3))
+        .await?;
+    assert!(call, "miner_setMaxDASize should be executed successfully");
+
+    // We already have 2 so we will spawn one more to check that it won't be included (it won't fit
+    // because of builder tx)
+    let fit_tx_1 = harness
+        .create_transaction()
+        .with_max_priority_fee_per_gas(50)
+        .send()
+        .await?;
+    let fit_tx_2 = harness
+        .create_transaction()
+        .with_max_priority_fee_per_gas(50)
+        .send()
+        .await?;
+    let unfit_tx_3 = harness.create_transaction().send().await?;
 
     let block = generator.generate_block().await?;
-
-    // tx should not be included because we set the tx_size_limit to 1
-    assert!(
-        block.includes(*valid_tx_1.tx_hash()),
-        "tx should be in block"
-    );
-    assert!(
-        block.includes(*valid_tx_2.tx_hash()),
-        "tx should be in block"
-    );
+    // Now the first 2 txs will fit into the block
+    assert!(block.includes(*fit_tx_1.tx_hash()), "tx should be in block");
+    assert!(block.includes(*fit_tx_2.tx_hash()), "tx should be in block");
     assert!(
         block.not_includes(*unfit_tx_3.tx_hash()),
         "unfit tx should not be in block"
@@ -87,5 +109,6 @@ async fn data_availability_block_fill() -> eyre::Result<()> {
         harness.latest_block().await.transactions.len() == 4,
         "builder + deposit + 2 valid txs should be in the block"
     );
+
     Ok(())
 }

crates/op-rbuilder/src/tests/vanilla/data_availability.rs

@@ -4,5 +4,6 @@ mod data_availability;
 mod ordering;
 mod revert;
 mod smoke;
+mod txpool;
 
 use super::*;

crates/op-rbuilder/src/tests/vanilla/mod.rs

@@ -0,0 +1,69 @@
+use crate::tests::{framework::TestHarnessBuilder, ONE_ETH};
+use alloy_provider::ext::TxPoolApi;
+
+/// This test ensures that pending pool custom limit is respected and priority tx would be included even when pool if full.
+#[tokio::test]
+async fn pending_pool_limit() -> eyre::Result<()> {
+    let harness = TestHarnessBuilder::new("data_availability_tx_size_limit")
+        .with_namespaces("txpool,eth,debug,admin")
+        .with_extra_params("--txpool.pending-max-count 50")
+        .build()
+        .await?;
+
+    let mut generator = harness.block_generator().await?;
+
+    // Send 50 txs from different addrs
+    let accounts = generator.create_funded_accounts(2, ONE_ETH).await?;
+    let acc_no_priority = accounts.first().unwrap();
+    let acc_with_priority = accounts.last().unwrap();
+
+    for _ in 0..50 {
+        let _ = harness
+            .create_transaction()
+            .with_signer(*acc_no_priority)
+            .send()
+            .await?;
+    }
+
+    let pool = harness
+        .provider()
+        .expect("provider not available")
+        .txpool_status()
+        .await?;
+    assert_eq!(
+        pool.pending, 50,
+        "Pending pool must contain at max 50 txs {:?}",
+        pool
+    );
+
+    // Send 10 txs that should be included in the block
+    let mut txs = Vec::new();
+    for _ in 0..10 {
+        let tx = harness
+            .create_transaction()
+            .with_signer(*acc_with_priority)
+            .with_max_priority_fee_per_gas(10)
+            .send()
+            .await?;
+        txs.push(*tx.tx_hash());
+    }
+
+    let pool = harness
+        .provider()
+        .expect("provider not available")
+        .txpool_status()
+        .await?;
+    assert_eq!(
+        pool.pending, 50,
+        "Pending pool must contain at max 50 txs {:?}",
+        pool
+    );
+
+    // After we try building block our reverting tx would be removed and other tx will move to queue pool
+    let block = generator.generate_block().await?;
+
+    // Ensure that 10 extra txs got included
+    assert!(block.includes_vec(txs));
+
+    Ok(())
+}