From fef7b1e16498218ffc6b9a40b8d687da2ea412bc Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Tue, 13 May 2025 19:49:15 +0100 Subject: [PATCH 1/4] Add e2e test for monitor txn --- .../src/integration/integration_test.rs | 49 +++++++++++++++++-- crates/op-rbuilder/src/integration/mod.rs | 42 +++++++++++----- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/crates/op-rbuilder/src/integration/integration_test.rs b/crates/op-rbuilder/src/integration/integration_test.rs index 32aa15e9d..d23f786fe 100644 --- a/crates/op-rbuilder/src/integration/integration_test.rs +++ b/crates/op-rbuilder/src/integration/integration_test.rs @@ -2,7 +2,8 @@ mod tests { use crate::{ integration::{ - op_rbuilder::OpRbuilderConfig, op_reth::OpRethConfig, IntegrationFramework, TestHarness, + op_rbuilder::OpRbuilderConfig, op_reth::OpRethConfig, IntegrationFramework, + TestHarness, TestHarnessBuilder, }, tester::{BlockGenerator, EngineApi}, tx_signer::Signer, @@ -94,10 +95,48 @@ mod tests { Ok(()) } + #[tokio::test] + #[cfg(not(feature = "flashblocks"))] + async fn integration_test_monitor_transaction_drops() -> eyre::Result<()> { + let harness = TestHarnessBuilder::new("integration_test_monitor_transaction_drops") + .with_revert_protection() + .build() + .await?; + + let mut generator = harness.block_generator().await?; + + // send 10 reverting transactions + let mut pending_txn = Vec::new(); + for _ in 0..10 { + let txn = harness.send_revert_transaction().await?; + pending_txn.push(txn); + } + + // generate 10 blocks + for _ in 0..10 { + let block_hash = generator.generate_block().await?; + + // query the block and the transactions inside the block + let block = harness + .provider()? + .get_block_by_hash(block_hash) + .await? + .expect("block"); + + // blocks should only include two + println!("block: {:?}", block.transactions.len()); + } + + Ok(()) + } + #[tokio::test] #[cfg(not(feature = "flashblocks"))] async fn integration_test_revert_protection_disabled() -> eyre::Result<()> { - let harness = TestHarness::new("integration_test_revert_protection_disabled").await; + let harness = TestHarnessBuilder::new("integration_test_revert_protection_disabled") + .build() + .await?; + let mut generator = harness.block_generator().await?; let txn1 = harness.send_valid_transaction().await?; @@ -386,13 +425,15 @@ mod tests { #[tokio::test] #[cfg(not(feature = "flashblocks"))] async fn integration_test_get_payload_close_to_fcu() -> eyre::Result<()> { - let test_harness = TestHarness::new("integration_test_get_payload_close_to_fcu").await; + let test_harness = TestHarnessBuilder::new("integration_test_get_payload_close_to_fcu") + .build() + .await?; let mut block_generator = test_harness.block_generator().await?; // add some transactions to the pool so that the builder is busy when we send the fcu/getPayload requests for _ in 0..10 { // Note, for this test it is okay if they are not valid - test_harness.send_valid_transaction().await?; + let _ = test_harness.send_valid_transaction().await?; } // TODO: In the fail case scenario, this hangs forever, but it should return an error diff --git a/crates/op-rbuilder/src/integration/mod.rs b/crates/op-rbuilder/src/integration/mod.rs index 28d55eede..fa3658000 100644 --- a/crates/op-rbuilder/src/integration/mod.rs +++ b/crates/op-rbuilder/src/integration/mod.rs @@ -219,16 +219,26 @@ impl Drop for IntegrationFramework { const BUILDER_PRIVATE_KEY: &str = "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; -pub struct TestHarness { - _framework: IntegrationFramework, - builder_auth_rpc_port: u16, - builder_http_port: u16, - validator_auth_rpc_port: u16, +pub struct TestHarnessBuilder { + name: String, + use_revert_protection: bool, } -impl TestHarness { - pub async fn new(name: &str) -> Self { - let mut framework = IntegrationFramework::new(name).unwrap(); +impl TestHarnessBuilder { + pub fn new(name: &str) -> Self { + Self { + name: name.to_string(), + use_revert_protection: false, + } + } + + pub fn with_revert_protection(mut self) -> Self { + self.use_revert_protection = true; + self + } + + pub async fn build(self) -> eyre::Result { + let mut framework = IntegrationFramework::new(&self.name).unwrap(); // we are going to use a genesis file pre-generated before the test let mut genesis_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); @@ -245,7 +255,8 @@ impl TestHarness { .auth_rpc_port(builder_auth_rpc_port) .network_port(get_available_port()) .http_port(builder_http_port) - .with_builder_private_key(BUILDER_PRIVATE_KEY); + .with_builder_private_key(BUILDER_PRIVATE_KEY) + .with_revert_protection(self.use_revert_protection); // create the validation reth node @@ -264,14 +275,23 @@ impl TestHarness { .await .unwrap(); - Self { + Ok(TestHarness { _framework: framework, builder_auth_rpc_port, builder_http_port, validator_auth_rpc_port, - } + }) } +} + +pub struct TestHarness { + _framework: IntegrationFramework, + builder_auth_rpc_port: u16, + builder_http_port: u16, + validator_auth_rpc_port: u16, +} +impl TestHarness { pub async fn send_valid_transaction( &self, ) -> eyre::Result> { From 6a7252ef4e2263d490a331e1c9cb58869dbbffc8 Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Tue, 13 May 2025 19:55:42 +0100 Subject: [PATCH 2/4] Partial --- crates/op-rbuilder/src/integration/integration_test.rs | 2 ++ crates/op-rbuilder/src/integration/op_rbuilder.rs | 2 ++ crates/op-rbuilder/src/integration/op_reth.rs | 2 ++ 3 files changed, 6 insertions(+) diff --git a/crates/op-rbuilder/src/integration/integration_test.rs b/crates/op-rbuilder/src/integration/integration_test.rs index d23f786fe..e23dced45 100644 --- a/crates/op-rbuilder/src/integration/integration_test.rs +++ b/crates/op-rbuilder/src/integration/integration_test.rs @@ -98,6 +98,8 @@ mod tests { #[tokio::test] #[cfg(not(feature = "flashblocks"))] async fn integration_test_monitor_transaction_drops() -> eyre::Result<()> { + // This test ensures that the transactions that get reverted an not included in the block + // are emitted as a log on the builder. let harness = TestHarnessBuilder::new("integration_test_monitor_transaction_drops") .with_revert_protection() .build() diff --git a/crates/op-rbuilder/src/integration/op_rbuilder.rs b/crates/op-rbuilder/src/integration/op_rbuilder.rs index 8ac6b938e..3bc99f71d 100644 --- a/crates/op-rbuilder/src/integration/op_rbuilder.rs +++ b/crates/op-rbuilder/src/integration/op_rbuilder.rs @@ -116,6 +116,8 @@ impl Service for OpRbuilderConfig { .arg("--datadir") .arg(self.data_dir.as_ref().expect("data_dir not set")) .arg("--disable-discovery") + .arg("--color") + .arg("never") .arg("--port") .arg(self.network_port.expect("network_port not set").to_string()) .arg("--ipcdisable"); diff --git a/crates/op-rbuilder/src/integration/op_reth.rs b/crates/op-rbuilder/src/integration/op_reth.rs index b809f4794..154ba1190 100644 --- a/crates/op-rbuilder/src/integration/op_reth.rs +++ b/crates/op-rbuilder/src/integration/op_reth.rs @@ -80,6 +80,8 @@ impl Service for OpRethConfig { .arg("--datadir") .arg(self.data_dir.as_ref().expect("data_dir not set")) .arg("--disable-discovery") + .arg("--color") + .arg("never") .arg("--port") .arg(self.network_port.expect("network_port not set").to_string()) .arg("--ipcdisable"); From 5f49111f6e4c4f643ba0ebc908b2c18fbba51f7e Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Tue, 13 May 2025 20:06:48 +0100 Subject: [PATCH 3/4] Add test --- .../src/integration/integration_test.rs | 20 +++++++++++++++++-- crates/op-rbuilder/src/integration/mod.rs | 6 +++++- .../src/integration/op_rbuilder.rs | 1 + 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/crates/op-rbuilder/src/integration/integration_test.rs b/crates/op-rbuilder/src/integration/integration_test.rs index e23dced45..dffc7d800 100644 --- a/crates/op-rbuilder/src/integration/integration_test.rs +++ b/crates/op-rbuilder/src/integration/integration_test.rs @@ -125,8 +125,24 @@ mod tests { .await? .expect("block"); - // blocks should only include two - println!("block: {:?}", block.transactions.len()); + // blocks should only include two transactions (deposit + builder) + assert_eq!(block.transactions.len(), 2); + } + + // check that the builder emitted logs for the reverted transactions + // with the monitoring logic + // TODO: this is not ideal, lets find a different way to detect this + // Each time a transaction is dropped, it emits a log like this + // 'Transaction event received target="monitoring" tx_hash="" kind="discarded"' + let builder_logs = std::fs::read_to_string(harness.builder_log_path)?; + + for txn in pending_txn { + let txn_log = format!( + "Transaction event received target=\"monitoring\" tx_hash=\"{}\" kind=\"discarded\"", + txn.tx_hash() + ); + + assert!(builder_logs.contains(txn_log.as_str())); } Ok(()) diff --git a/crates/op-rbuilder/src/integration/mod.rs b/crates/op-rbuilder/src/integration/mod.rs index fa3658000..d901cd815 100644 --- a/crates/op-rbuilder/src/integration/mod.rs +++ b/crates/op-rbuilder/src/integration/mod.rs @@ -270,16 +270,19 @@ impl TestHarnessBuilder { framework.start("op-reth", &reth).await.unwrap(); - let _ = framework + let builder = framework .start("op-rbuilder", &op_rbuilder_config) .await .unwrap(); + let builder_log_path = builder.log_path.clone(); + Ok(TestHarness { _framework: framework, builder_auth_rpc_port, builder_http_port, validator_auth_rpc_port, + builder_log_path, }) } } @@ -289,6 +292,7 @@ pub struct TestHarness { builder_auth_rpc_port: u16, builder_http_port: u16, validator_auth_rpc_port: u16, + builder_log_path: PathBuf, } impl TestHarness { diff --git a/crates/op-rbuilder/src/integration/op_rbuilder.rs b/crates/op-rbuilder/src/integration/op_rbuilder.rs index 3bc99f71d..4170a89cc 100644 --- a/crates/op-rbuilder/src/integration/op_rbuilder.rs +++ b/crates/op-rbuilder/src/integration/op_rbuilder.rs @@ -118,6 +118,7 @@ impl Service for OpRbuilderConfig { .arg("--disable-discovery") .arg("--color") .arg("never") + .arg("--builder.log-pool-transactions") .arg("--port") .arg(self.network_port.expect("network_port not set").to_string()) .arg("--ipcdisable"); From 5544143928134bfad3938ffdb91497fbd40c5307 Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Tue, 13 May 2025 20:32:06 +0100 Subject: [PATCH 4/4] Fix lint --- crates/op-rbuilder/src/integration/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/op-rbuilder/src/integration/mod.rs b/crates/op-rbuilder/src/integration/mod.rs index d901cd815..11cd6d75c 100644 --- a/crates/op-rbuilder/src/integration/mod.rs +++ b/crates/op-rbuilder/src/integration/mod.rs @@ -292,6 +292,7 @@ pub struct TestHarness { builder_auth_rpc_port: u16, builder_http_port: u16, validator_auth_rpc_port: u16, + #[allow(dead_code)] // I think this is due to some feature flag conflicts builder_log_path: PathBuf, }