From b4285ee0c28e0b6cc68868a0bd0c450e48663baf Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Wed, 28 May 2025 15:59:57 +0000 Subject: [PATCH 01/21] First tests migrated and working --- Cargo.lock | 60 +++++ Cargo.toml | 9 +- crates/op-rbuilder/Cargo.toml | 14 +- .../src/builders/flashblocks/wspub.rs | 2 - crates/op-rbuilder/src/lib.rs | 6 + crates/op-rbuilder/src/main.rs | 79 +++++- crates/op-rbuilder/src/metrics.rs | 54 +--- .../op-rbuilder/src/tests/framework/apis.rs | 167 +++++++----- .../op-rbuilder/src/tests/framework/blocks.rs | 26 +- .../op-rbuilder/src/tests/framework/chain.rs | 234 +++++++++++++++++ .../src/tests/framework/harness.rs | 11 +- .../src/tests/framework/instance.rs | 241 ++++++++++++++++++ crates/op-rbuilder/src/tests/framework/mod.rs | 37 ++- crates/op-rbuilder/src/tests/framework/txs.rs | 22 +- .../op-rbuilder/src/tests/framework/utils.rs | 63 +++++ crates/op-rbuilder/src/tests/mod.rs | 4 +- .../data_availability.rs | 0 crates/op-rbuilder/src/tests/standard/mod.rs | 8 + .../tests/{vanilla => standard}/ordering.rs | 0 .../src/tests/{vanilla => standard}/revert.rs | 0 .../op-rbuilder/src/tests/standard/smoke.rs | 119 +++++++++ .../src/tests/{vanilla => standard}/txpool.rs | 0 crates/op-rbuilder/src/tests/vanilla/mod.rs | 9 - crates/op-rbuilder/src/tests/vanilla/smoke.rs | 130 ---------- 24 files changed, 992 insertions(+), 303 deletions(-) create mode 100644 crates/op-rbuilder/src/tests/framework/chain.rs create mode 100644 crates/op-rbuilder/src/tests/framework/instance.rs create mode 100644 crates/op-rbuilder/src/tests/framework/utils.rs rename crates/op-rbuilder/src/tests/{vanilla => standard}/data_availability.rs (100%) create mode 100644 crates/op-rbuilder/src/tests/standard/mod.rs rename crates/op-rbuilder/src/tests/{vanilla => standard}/ordering.rs (100%) rename crates/op-rbuilder/src/tests/{vanilla => standard}/revert.rs (100%) create mode 100644 crates/op-rbuilder/src/tests/standard/smoke.rs rename crates/op-rbuilder/src/tests/{vanilla => standard}/txpool.rs (100%) delete mode 100644 crates/op-rbuilder/src/tests/vanilla/mod.rs delete mode 100644 crates/op-rbuilder/src/tests/vanilla/smoke.rs diff --git a/Cargo.lock b/Cargo.lock index 7b36dfc4a..6436e3d36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2461,6 +2461,22 @@ dependencies = [ "subtle", ] +[[package]] +name = "ctor" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4735f265ba6a1188052ca32d461028a7d1125868be18e287e756019da7607b5" +dependencies = [ + "ctor-proc-macro", + "dtor", +] + +[[package]] +name = "ctor-proc-macro" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f211af61d8efdd104f96e57adf5e426ba1bc3ed7a4ead616e15e5881fd79c4d" + [[package]] name = "ctr" version = "0.9.2" @@ -2833,6 +2849,21 @@ version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77c90badedccf4105eca100756a0b1289e191f6fcbdadd3cee1d2f614f97da8f" +[[package]] +name = "dtor" +version = "0.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97cbdf2ad6846025e8e25df05171abfb30e3ababa12ee0a0e44b9bbe570633a8" +dependencies = [ + "dtor-proc-macro", +] + +[[package]] +name = "dtor-proc-macro" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7454e41ff9012c00d53cf7f475c5e3afa3b91b7c90568495495e8d9bf47a1055" + [[package]] name = "dunce" version = "1.0.5" @@ -5201,6 +5232,15 @@ dependencies = [ "unsigned-varint", ] +[[package]] +name = "nanoid" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ffa00dec017b5b1a8b7cf5e2c008bfda1aa7e0697ac1508b491fdf2622fb4d8" +dependencies = [ + "rand 0.8.5", +] + [[package]] name = "native-tls" version = "0.2.14" @@ -5544,6 +5584,7 @@ dependencies = [ "chrono", "clap", "clap_builder", + "ctor", "derive_more", "eyre", "futures", @@ -5551,6 +5592,7 @@ dependencies = [ "jsonrpsee 0.25.1", "jsonrpsee-types 0.25.1", "metrics", + "nanoid", "op-alloy-consensus", "op-alloy-flz", "op-alloy-network", @@ -5568,6 +5610,7 @@ dependencies = [ "reth-evm", "reth-execution-types", "reth-exex", + "reth-ipc", "reth-metrics", "reth-network-peers", "reth-node-api", @@ -7142,12 +7185,17 @@ dependencies = [ "rayon", "reth-config", "reth-consensus", + "reth-db", + "reth-db-api", + "reth-ethereum-primitives", "reth-metrics", "reth-network-p2p", "reth-network-peers", "reth-primitives-traits", "reth-storage-api", "reth-tasks", + "reth-testing-utils", + "tempfile", "thiserror 2.0.12", "tokio", "tokio-stream", @@ -7283,6 +7331,7 @@ dependencies = [ "parking_lot", "rayon", "reth-chain-state", + "reth-chainspec", "reth-consensus", "reth-db", "reth-engine-primitives", @@ -7296,9 +7345,13 @@ dependencies = [ "reth-primitives-traits", "reth-provider", "reth-prune", + "reth-prune-types", "reth-revm", + "reth-stages", "reth-stages-api", + "reth-static-file", "reth-tasks", + "reth-tracing", "reth-trie", "reth-trie-db", "reth-trie-parallel", @@ -7941,6 +7994,7 @@ dependencies = [ "auto_impl", "derive_more", "futures", + "parking_lot", "reth-consensus", "reth-eth-wire-types", "reth-ethereum-primitives", @@ -8569,6 +8623,7 @@ version = "1.4.1" source = "git+https://github.com/paradigmxyz/reth?tag=v1.4.1#e6ce41ebba0d8752cef9ed885aae057e09226d05" dependencies = [ "alloy-consensus", + "alloy-primitives", "alloy-rpc-types", "futures-util", "metrics", @@ -9135,11 +9190,13 @@ dependencies = [ "num-traits", "rayon", "reqwest", + "reth-chainspec", "reth-codecs", "reth-config", "reth-consensus", "reth-db", "reth-db-api", + "reth-ethereum-primitives", "reth-etl", "reth-evm", "reth-execution-types", @@ -9154,9 +9211,11 @@ dependencies = [ "reth-stages-api", "reth-static-file-types", "reth-storage-errors", + "reth-testing-utils", "reth-trie", "reth-trie-db", "serde", + "tempfile", "thiserror 2.0.12", "tokio", "tracing", @@ -9349,6 +9408,7 @@ dependencies = [ "futures-util", "metrics", "parking_lot", + "paste", "rand 0.9.0", "reth-chain-state", "reth-chainspec", diff --git a/Cargo.toml b/Cargo.toml index d4a08f6fa..913c4eee0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,7 +76,8 @@ reth-rpc-layer = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-network-peers = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-testing-utils = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-node-builder = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } -reth-rpc-eth-types = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } +reth-rpc-eth-types = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } +reth-ipc = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } # reth optimism reth-optimism-primitives = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } @@ -105,7 +106,11 @@ alloy-primitives = { version = "1.1.0", default-features = false } alloy-rlp = "0.3.10" alloy-chains = "0.2.0" alloy-evm = { version = "0.8.0", default-features = false } -alloy-provider = { version = "1.0.3", features = ["ipc", "pubsub", "txpool-api"] } +alloy-provider = { version = "1.0.3", features = [ + "ipc", + "pubsub", + "txpool-api", +] } alloy-pubsub = { version = "1.0.3" } alloy-eips = { version = "1.0.3" } alloy-rpc-types = { version = "1.0.3" } diff --git a/crates/op-rbuilder/Cargo.toml b/crates/op-rbuilder/Cargo.toml index 994207c4e..58b3cb6f6 100644 --- a/crates/op-rbuilder/Cargo.toml +++ b/crates/op-rbuilder/Cargo.toml @@ -44,6 +44,7 @@ reth-testing-utils.workspace = true reth-optimism-forks.workspace = true reth-node-builder.workspace = true reth-rpc-eth-types.workspace = true +reth-ipc = { workspace = true, optional = true } alloy-primitives.workspace = true alloy-consensus.workspace = true @@ -99,7 +100,7 @@ rand = "0.9.0" tracing-subscriber = { version = "0.3.18", features = ["env-filter", "json"] } shellexpand = "3.1" serde_yaml = { version = "0.9" } - +nanoid = { version = "0.4", optional = true } # `msozin/flashblocks-v1.4.1` branch based on `flashblocks-rebase` rollup-boost = { git = "http://github.com/flashbots/rollup-boost", rev = "8506dfb7d84c65746f7c88d250983658438f59e8" } @@ -112,7 +113,14 @@ 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"]} +alloy-provider = { workspace = true, default-features = true, features = [ + "txpool-api", +] } +nanoid = "0.4" +reth-node-builder = { workspace = true, features = ["test-utils"] } +reth-ipc.workspace = true +reth-rpc-eth-types.workspace = true +ctor = "0.4.2" [features] default = ["jemalloc"] @@ -135,7 +143,7 @@ min-info-logs = ["tracing/release_max_level_info"] min-debug-logs = ["tracing/release_max_level_debug"] min-trace-logs = ["tracing/release_max_level_trace"] -testing = [] +testing = ["nanoid", "reth-node-builder/test-utils", "reth-ipc"] [[bin]] name = "op-rbuilder" diff --git a/crates/op-rbuilder/src/builders/flashblocks/wspub.rs b/crates/op-rbuilder/src/builders/flashblocks/wspub.rs index a31ad3be7..71b045207 100644 --- a/crates/op-rbuilder/src/builders/flashblocks/wspub.rs +++ b/crates/op-rbuilder/src/builders/flashblocks/wspub.rs @@ -182,8 +182,6 @@ async fn broadcast_loop( // Receive payloads from the broadcast channel payload = blocks.recv() => match payload { Ok(payload) => { - // Here you would typically send the payload to the WebSocket clients. - // For this example, we just increment the sent counter. sent.fetch_add(1, Ordering::Relaxed); metrics.messages_sent_count.increment(1); diff --git a/crates/op-rbuilder/src/lib.rs b/crates/op-rbuilder/src/lib.rs index f377643f9..8ab0888cb 100644 --- a/crates/op-rbuilder/src/lib.rs +++ b/crates/op-rbuilder/src/lib.rs @@ -1,5 +1,11 @@ +pub mod args; +pub mod builders; pub mod primitives; pub mod tx_signer; +mod metrics; +mod traits; +mod tx; + #[cfg(any(test, feature = "testing"))] pub mod tests; diff --git a/crates/op-rbuilder/src/main.rs b/crates/op-rbuilder/src/main.rs index f378f961b..f9545974e 100644 --- a/crates/op-rbuilder/src/main.rs +++ b/crates/op-rbuilder/src/main.rs @@ -18,10 +18,6 @@ mod traits; mod tx; mod tx_signer; -use metrics::{ - VersionInfo, BUILD_PROFILE_NAME, CARGO_PKG_VERSION, VERGEN_BUILD_TIMESTAMP, - VERGEN_CARGO_FEATURES, VERGEN_CARGO_TARGET_TRIPLE, VERGEN_GIT_SHA, -}; use monitor_tx_pool::monitor_tx_pool; use revert_protection::{EthApiOverrideServer, RevertProtectionExt}; use tx::FBPooledTransaction; @@ -31,15 +27,6 @@ use tx::FBPooledTransaction; #[global_allocator] static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; -const VERSION: VersionInfo = VersionInfo { - version: CARGO_PKG_VERSION, - build_timestamp: VERGEN_BUILD_TIMESTAMP, - cargo_features: VERGEN_CARGO_FEATURES, - git_sha: VERGEN_GIT_SHA, - target_triple: VERGEN_CARGO_TARGET_TRIPLE, - build_profile: BUILD_PROFILE_NAME, -}; - fn main() { let cli = Cli::parsed(); cli.logs @@ -113,7 +100,7 @@ where Ok(()) }) .on_node_started(move |ctx| { - VERSION.register_version_metrics(); + VersionInfo::from_env().register_version_metrics(); if builder_args.log_pool_transactions { tracing::info!("Logging pool transactions"); ctx.task_executor.spawn_critical( @@ -133,3 +120,67 @@ where }) .unwrap(); } + +/// Contains version information for the application. +#[derive(Debug, Clone)] +pub struct VersionInfo { + /// The version of the application. + pub version: &'static str, + /// The build timestamp of the application. + pub build_timestamp: &'static str, + /// The cargo features enabled for the build. + pub cargo_features: &'static str, + /// The Git SHA of the build. + pub git_sha: &'static str, + /// The target triple for the build. + pub target_triple: &'static str, + /// The build profile (e.g., debug or release). + pub build_profile: &'static str, +} + +impl VersionInfo { + pub const fn from_env() -> Self { + Self { + // The latest version from Cargo.toml. + version: env!("CARGO_PKG_VERSION"), + + // The build timestamp. + build_timestamp: env!("VERGEN_BUILD_TIMESTAMP"), + + // The build features. + cargo_features: env!("VERGEN_CARGO_FEATURES"), + + // The 8 character short SHA of the latest commit. + git_sha: env!("VERGEN_GIT_SHA"), + + // The target triple. + target_triple: env!("VERGEN_CARGO_TARGET_TRIPLE"), + + // The build profile name. + build_profile: env!("OP_RBUILDER_BUILD_PROFILE"), + } + } +} + +impl Default for VersionInfo { + fn default() -> Self { + Self::from_env() + } +} + +impl VersionInfo { + /// This exposes reth's version information over prometheus. + pub fn register_version_metrics(&self) { + let labels: [(&str, &str); 6] = [ + ("version", self.version), + ("build_timestamp", self.build_timestamp), + ("cargo_features", self.cargo_features), + ("git_sha", self.git_sha), + ("target_triple", self.target_triple), + ("build_profile", self.build_profile), + ]; + + let gauge = ::metrics::gauge!("builder_info", &labels); + gauge.set(1); + } +} diff --git a/crates/op-rbuilder/src/metrics.rs b/crates/op-rbuilder/src/metrics.rs index 4b44410bf..2a39ea0d9 100644 --- a/crates/op-rbuilder/src/metrics.rs +++ b/crates/op-rbuilder/src/metrics.rs @@ -1,26 +1,8 @@ use reth_metrics::{ - metrics::{gauge, Counter, Histogram}, + metrics::{Counter, Histogram}, Metrics, }; -/// The latest version from Cargo.toml. -pub const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); - -/// The 8 character short SHA of the latest commit. -pub const VERGEN_GIT_SHA: &str = env!("VERGEN_GIT_SHA_SHORT"); - -/// The build timestamp. -pub const VERGEN_BUILD_TIMESTAMP: &str = env!("VERGEN_BUILD_TIMESTAMP"); - -/// The target triple. -pub const VERGEN_CARGO_TARGET_TRIPLE: &str = env!("VERGEN_CARGO_TARGET_TRIPLE"); - -/// The build features. -pub const VERGEN_CARGO_FEATURES: &str = env!("VERGEN_CARGO_FEATURES"); - -/// The build profile name. -pub const BUILD_PROFILE_NAME: &str = env!("OP_RBUILDER_BUILD_PROFILE"); - /// op-rbuilder metrics #[derive(Metrics, Clone)] #[metrics(scope = "op_rbuilder")] @@ -64,37 +46,3 @@ pub struct OpRBuilderMetrics { /// Byte size of transactions pub tx_byte_size: Histogram, } - -/// Contains version information for the application. -#[derive(Debug, Clone)] -pub struct VersionInfo { - /// The version of the application. - pub version: &'static str, - /// The build timestamp of the application. - pub build_timestamp: &'static str, - /// The cargo features enabled for the build. - pub cargo_features: &'static str, - /// The Git SHA of the build. - pub git_sha: &'static str, - /// The target triple for the build. - pub target_triple: &'static str, - /// The build profile (e.g., debug or release). - pub build_profile: &'static str, -} - -impl VersionInfo { - /// This exposes reth's version information over prometheus. - pub fn register_version_metrics(&self) { - let labels: [(&str, &str); 6] = [ - ("version", self.version), - ("build_timestamp", self.build_timestamp), - ("cargo_features", self.cargo_features), - ("git_sha", self.git_sha), - ("target_triple", self.target_triple), - ("build_profile", self.build_profile), - ]; - - let gauge = gauge!("builder_info", &labels); - gauge.set(1); - } -} diff --git a/crates/op-rbuilder/src/tests/framework/apis.rs b/crates/op-rbuilder/src/tests/framework/apis.rs index cdcba0887..35f4f64f0 100644 --- a/crates/op-rbuilder/src/tests/framework/apis.rs +++ b/crates/op-rbuilder/src/tests/framework/apis.rs @@ -2,6 +2,7 @@ use super::DEFAULT_JWT_TOKEN; use alloy_eips::BlockNumberOrTag; use alloy_primitives::B256; use alloy_rpc_types_engine::{ExecutionPayloadV3, ForkchoiceUpdated, PayloadStatus}; +use core::{future::Future, marker::PhantomData}; use jsonrpsee::{ core::{client::SubscriptionClientT, RpcResult}, proc_macros::rpc, @@ -12,72 +13,131 @@ use reth_optimism_node::OpEngineTypes; use reth_payload_builder::PayloadId; use reth_rpc_layer::{AuthClientLayer, JwtSecret}; use serde_json::Value; -use std::str::FromStr; -/// Helper for engine api operations -pub struct EngineApi { - url: url::Url, - jwt_secret: JwtSecret, +#[derive(Clone, Debug)] +pub enum Address { + Ipc(String), + Http(url::Url), +} + +pub trait Protocol { + fn client( + jwt: JwtSecret, + address: Address, + ) -> impl Future; +} + +pub struct Http; +impl Protocol for Http { + async fn client( + jwt: JwtSecret, + address: Address, + ) -> impl SubscriptionClientT + Send + Sync + Unpin + 'static { + let Address::Http(url) = address else { + unreachable!(); + }; + + let secret_layer = AuthClientLayer::new(jwt); + let middleware = tower::ServiceBuilder::default().layer(secret_layer); + jsonrpsee::http_client::HttpClientBuilder::default() + .set_http_middleware(middleware) + .build(url) + .expect("Failed to create http client") + } +} + +pub struct Ipc; +impl Protocol for Ipc { + async fn client( + _: JwtSecret, // ipc does not use JWT + address: Address, + ) -> impl SubscriptionClientT + Send + Sync + Unpin + 'static { + let Address::Ipc(path) = address else { + unreachable!(); + }; + reth_ipc::client::IpcClientBuilder::default() + .build(&path) + .await + .expect("Failed to create ipc client") + } } -/// Builder for EngineApi configuration -pub struct EngineApiBuilder { - url: String, - jwt_secret: String, +/// Helper for engine api operations +pub struct EngineApi { + address: Address, + jwt_secret: JwtSecret, + _tag: PhantomData

, } -impl Default for EngineApiBuilder { - fn default() -> Self { - Self::new() +impl EngineApi

{ + async fn client(&self) -> impl SubscriptionClientT + Send + Sync + Unpin + 'static { + P::client(self.jwt_secret, self.address.clone()).await } } -impl EngineApiBuilder { - pub fn new() -> Self { - Self { - url: String::from("http://localhost:8551"), - jwt_secret: String::from(DEFAULT_JWT_TOKEN), +// http specific +impl EngineApi { + pub fn with_http(url: &str) -> EngineApi { + EngineApi:: { + address: Address::Http(url.parse().expect("Invalid URL")), + jwt_secret: DEFAULT_JWT_TOKEN.parse().expect("Invalid JWT"), + _tag: PhantomData, } } - pub fn with_url(mut self, url: &str) -> Self { - self.url = url.to_string(); - self + pub fn with_localhost_port(port: u16) -> EngineApi { + EngineApi:: { + address: Address::Http( + format!("http://localhost:{}", port) + .parse() + .expect("Invalid URL"), + ), + jwt_secret: DEFAULT_JWT_TOKEN.parse().expect("Invalid JWT"), + _tag: PhantomData, + } } - pub fn build(self) -> Result> { - Ok(EngineApi { - url: self.url.parse()?, - jwt_secret: JwtSecret::from_str(&self.jwt_secret)?, - }) + pub fn with_port(mut self, port: u16) -> Self { + let Address::Http(url) = &mut self.address else { + unreachable!(); + }; + + url.set_port(Some(port)).expect("Invalid port"); + self } -} -impl EngineApi { - pub fn builder() -> EngineApiBuilder { - EngineApiBuilder::new() + pub fn with_jwt_secret(mut self, jwt_secret: &str) -> Self { + self.jwt_secret = jwt_secret.parse().expect("Invalid JWT"); + self } - pub fn new(url: &str) -> Result> { - Self::builder().with_url(url).build() + pub fn url(&self) -> &url::Url { + let Address::Http(url) = &self.address else { + unreachable!(); + }; + url } +} - pub fn new_with_port(port: u16) -> Result> { - Self::builder() - .with_url(&format!("http://localhost:{port}")) - .build() +// ipc specific +impl EngineApi { + pub fn with_ipc(path: &str) -> EngineApi { + EngineApi:: { + address: Address::Ipc(path.into()), + jwt_secret: DEFAULT_JWT_TOKEN.parse().expect("Invalid JWT"), + _tag: PhantomData, + } } - pub fn http_client(&self) -> impl SubscriptionClientT + Clone + Send + Sync + Unpin + 'static { - // Create a middleware that adds a new JWT token to every request. - let secret_layer = AuthClientLayer::new(self.jwt_secret); - let middleware = tower::ServiceBuilder::default().layer(secret_layer); - jsonrpsee::http_client::HttpClientBuilder::default() - .set_http_middleware(middleware) - .build(&self.url) - .expect("Failed to create http client") + pub fn path(&self) -> &str { + let Address::Ipc(path) = &self.address else { + unreachable!(); + }; + path } +} +impl EngineApi

{ pub async fn get_payload_v3( &self, payload_id: PayloadId, @@ -89,7 +149,7 @@ impl EngineApi { ); Ok( - EngineApiClient::::get_payload_v3(&self.http_client(), payload_id) + EngineApiClient::::get_payload_v3(&self.client().await, payload_id) .await?, ) } @@ -101,9 +161,8 @@ impl EngineApi { parent_beacon_block_root: B256, ) -> eyre::Result { println!("Submitting new payload at {}...", chrono::Utc::now()); - Ok(EngineApiClient::::new_payload_v3( - &self.http_client(), + &self.client().await, payload, versioned_hashes, parent_beacon_block_root, @@ -118,9 +177,8 @@ impl EngineApi { payload_attributes: Option<::PayloadAttributes>, ) -> eyre::Result { println!("Updating forkchoice at {}...", chrono::Utc::now()); - Ok(EngineApiClient::::fork_choice_updated_v3( - &self.http_client(), + &self.client().await, ForkchoiceState { head_block_hash: new_head, safe_block_hash: current_head, @@ -130,19 +188,6 @@ impl EngineApi { ) .await?) } - - pub async fn latest(&self) -> eyre::Result> { - self.get_block_by_number(BlockNumberOrTag::Latest, false) - .await - } - - pub async fn get_block_by_number( - &self, - number: BlockNumberOrTag, - include_txs: bool, - ) -> eyre::Result> { - Ok(BlockApiClient::get_block_by_number(&self.http_client(), number, include_txs).await?) - } } #[rpc(server, client, namespace = "eth")] diff --git a/crates/op-rbuilder/src/tests/framework/blocks.rs b/crates/op-rbuilder/src/tests/framework/blocks.rs index a9fd7ba14..e71f77e6e 100644 --- a/crates/op-rbuilder/src/tests/framework/blocks.rs +++ b/crates/op-rbuilder/src/tests/framework/blocks.rs @@ -10,7 +10,7 @@ use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; use op_alloy_rpc_types_engine::OpPayloadAttributes; use rollup_boost::{Flashblocks, FlashblocksService, OpExecutionPayloadEnvelope, Version}; -use super::apis::EngineApi; +use super::{apis::EngineApi, Http, Ipc, Protocol}; // L1 block info for OP mainnet block 124665056 (stored in input of tx at index 0) // @@ -18,9 +18,9 @@ use super::apis::EngineApi; const FJORD_DATA: &[u8] = &hex!("440a5e200000146b000f79c500000000000000040000000066d052e700000000013ad8a3000000000000000000000000000000000000000000000000000000003ef1278700000000000000000000000000000000000000000000000000000000000000012fdf87b89884a61e74b322bbcf60386f543bfae7827725efaaf0ab1de2294a590000000000000000000000006887246668a3b87f54deb3b94ba47a6f63f32985"); /// A system that continuously generates blocks using the engine API -pub struct BlockGenerator { - engine_api: EngineApi, - validation_api: Option, +pub struct BlockGenerator { + engine_api: EngineApi, + validation_api: Option>, latest_hash: B256, no_tx_pool: bool, block_time_secs: u64, @@ -30,10 +30,12 @@ pub struct BlockGenerator { flashblocks_service: Option, } -impl BlockGenerator { +impl + BlockGenerator +{ pub fn new( - engine_api: EngineApi, - validation_api: Option, + engine_api: EngineApi, + validation_api: Option>, no_tx_pool: bool, block_time_secs: u64, flashblocks_endpoint: Option, @@ -57,9 +59,7 @@ impl BlockGenerator { self.timestamp = latest_block.header.timestamp; // Sync validation node if it exists - if let Some(validation_api) = &self.validation_api { - self.sync_validation_node(validation_api).await?; - } + self.sync_validation_node().await?; // Initialize flashblocks service if let Some(flashblocks_endpoint) = &self.flashblocks_endpoint { @@ -75,7 +75,11 @@ impl BlockGenerator { } /// Sync the validation node to the current state - async fn sync_validation_node(&self, validation_api: &EngineApi) -> eyre::Result<()> { + async fn sync_validation_node(&self) -> eyre::Result<()> { + let Some(validation_api) = &self.validation_api else { + return Ok(()); // No validation node to sync + }; + let latest_validation_block = validation_api.latest().await?.expect("block not found"); let latest_block = self.engine_api.latest().await?.expect("block not found"); diff --git a/crates/op-rbuilder/src/tests/framework/chain.rs b/crates/op-rbuilder/src/tests/framework/chain.rs new file mode 100644 index 000000000..7e7ff242a --- /dev/null +++ b/crates/op-rbuilder/src/tests/framework/chain.rs @@ -0,0 +1,234 @@ +use core::time::Duration; +use std::time::SystemTime; + +use alloy_eips::{BlockNumberOrTag, Encodable2718}; +use alloy_primitives::{address, hex, Bytes, TxKind, B256, U256}; +use alloy_provider::{Provider, RootProvider}; +use alloy_rpc_types_engine::{ + ExecutionPayload, ForkchoiceUpdated, PayloadAttributes, PayloadStatusEnum, +}; +use alloy_rpc_types_eth::Block; +use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; +use op_alloy_network::Optimism; +use op_alloy_rpc_types::Transaction; +use reth_optimism_node::OpPayloadAttributes; +use rollup_boost::OpExecutionPayloadEnvelope; + +use crate::tx_signer::Signer; + +use super::{EngineApi, Ipc, LocalInstance, TransactionBuilder}; + +const DEFAULT_GAS_LIMIT: u64 = 10_000_000; + +/// The ChainDriver is a type that allows driving the op builder node to build new blocks manually +/// by calling the `build_new_block` method. It uses the Engine API to interact with the node +/// and the provider to fetch blocks and transactions. +pub struct ChainDriver { + engine_api: EngineApi, + provider: RootProvider, + signer: Option, + gas_limit: Option, +} + +// instantiation and configuration +impl ChainDriver { + const MIN_BLOCK_TIME: Duration = Duration::from_secs(1); + + pub async fn new(instance: &LocalInstance) -> eyre::Result { + Ok(Self { + engine_api: instance.engine_api(), + provider: instance.provider().await?, + signer: Default::default(), + gas_limit: None, + }) + } + + pub fn with_signer(mut self, signer: Signer) -> Self { + self.signer = Some(signer); + self + } + + pub fn with_gas_limit(mut self, gas_limit: u64) -> Self { + self.gas_limit = Some(gas_limit); + self + } +} + +// public test api +impl ChainDriver { + /// Builds a new block using the current state of the chain and the transactions in the pool. + pub async fn build_new_block(&self) -> eyre::Result> { + self.build_new_block_with_txs(vec![]).await + } + + /// Builds a new block using the current state of the chain and the transactions in the pool with a list + /// of mandatory builder transactions. Those are usually deposit transactions. + pub async fn build_new_block_with_txs( + &self, + txs: Vec, + ) -> eyre::Result> { + let latest = self.latest().await?; + let latest_timestamp = Duration::from_secs(latest.header.timestamp); + let actual_timestamp = SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_err(|_| eyre::eyre!("Failed to get current system time"))?; + + // block timestamp will be the max of the actual timestamp and the latest block + // timestamp plus the minimum block time. This ensures that blocks don't break any + // assumptions, but also gives the test author the ability to control the block time + // in the test. + let block_timestamp = actual_timestamp.max(latest_timestamp + Self::MIN_BLOCK_TIME); + + // Add L1 block info as the first transaction in every L2 block + // This deposit transaction contains L1 block metadata required by the L2 chain + // Currently using hardcoded data from L1 block 124665056 + // If this info is not provided, Reth cannot decode the receipt for any transaction + // in the block since it also includes this info as part of the result. + // It does not matter if the to address (4200000000000000000000000000000000000015) is + // not deployed on the L2 chain since Reth queries the block to get the info and not the contract. + let block_info_tx: Bytes = { + let deposit_tx = TxDeposit { + source_hash: B256::default(), + from: address!("DeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAd0001"), + to: TxKind::Call(address!("4200000000000000000000000000000000000015")), + mint: None, + value: U256::default(), + gas_limit: 210000, + is_system_transaction: false, + input: FJORD_DATA.into(), + }; + + // Create a temporary signer for the deposit + let signer = self.signer.unwrap_or_else(Signer::random); + let signed_tx = signer.sign_tx(OpTypedTransaction::Deposit(deposit_tx))?; + signed_tx.encoded_2718().into() + }; + + let fcu_result = self + .fcu(OpPayloadAttributes { + payload_attributes: PayloadAttributes { + timestamp: block_timestamp.as_secs(), + parent_beacon_block_root: Some(B256::ZERO), + withdrawals: Some(vec![]), + ..Default::default() + }, + transactions: Some(vec![block_info_tx].into_iter().chain(txs).collect()), + gas_limit: Some(self.gas_limit.unwrap_or(DEFAULT_GAS_LIMIT)), + ..Default::default() + }) + .await?; + + if fcu_result.payload_status.is_invalid() { + return Err(eyre::eyre!("Forkchoice update failed: {fcu_result:?}")); + } + + let payload_id = fcu_result + .payload_id + .ok_or_else(|| eyre::eyre!("Forkchoice update did not return a payload ID"))?; + + let payload = + OpExecutionPayloadEnvelope::V3(self.engine_api.get_payload_v3(payload_id).await?); + + let ExecutionPayload::V3(payload) = payload.into() else { + return Err(eyre::eyre!("Expected V3 payload, got something else")); + }; + + if self + .engine_api + .new_payload(payload.clone(), vec![], B256::ZERO) + .await? + .status + != PayloadStatusEnum::Valid + { + return Err(eyre::eyre!("Invalid validation status from builder")); + } + + let new_block_hash = payload.payload_inner.payload_inner.block_hash; + self.engine_api + .update_forkchoice(latest.header.hash, new_block_hash, None) + .await?; + + let block = self + .provider + .get_block_by_number(alloy_eips::BlockNumberOrTag::Latest) + .full() + .await? + .ok_or_else(|| eyre::eyre!("Failed to get latest block after building new block"))?; + + assert_eq!( + block.header.hash, new_block_hash, + "New block hash does not match expected hash" + ); + + Ok(block) + } + + /// Retreives the latest built block and returns only a list of transaction + /// hashes from its body. + pub async fn latest(&self) -> eyre::Result> { + self.provider + .get_block_by_number(alloy_eips::BlockNumberOrTag::Latest) + .await? + .ok_or_else(|| eyre::eyre!("Failed to get latest block")) + } + + /// Retreives the latest built block and returns a list of full transaction + /// contents in its body. + pub async fn latest_full(&self) -> eyre::Result> { + self.provider + .get_block_by_number(alloy_eips::BlockNumberOrTag::Latest) + .full() + .await? + .ok_or_else(|| eyre::eyre!("Failed to get latest full block")) + } + + /// retreives a specific block by its number or tag and returns a list of transaction + /// hashes from its body. + pub async fn get_block( + &self, + number: BlockNumberOrTag, + ) -> eyre::Result>> { + Ok(self.provider.get_block_by_number(number).await?) + } + + /// retreives a specific block by its number or tag and returns a list of full transaction + /// contents in its body. + pub async fn get_block_full( + &self, + number: BlockNumberOrTag, + ) -> eyre::Result>> { + Ok(self.provider.get_block_by_number(number).full().await?) + } + + /// Returns a transaction builder that can be used to create and send transactions. + pub fn transaction(&self) -> TransactionBuilder { + TransactionBuilder::new(self.provider.clone()) + } + + /// Returns a reference to the underlying alloy provider that is used to + /// interact with the chain. + pub const fn provider(&self) -> &RootProvider { + &self.provider + } +} + +// internal methods +impl ChainDriver { + async fn fcu(&self, attribs: OpPayloadAttributes) -> eyre::Result { + let latest = self.latest().await?.header.hash; + self.engine_api + .update_forkchoice(latest, latest, Some(attribs)) + .await + } +} + +// L1 block info for OP mainnet block 124665056 (stored in input of tx at index 0) +// +// https://optimistic.etherscan.io/tx/0x312e290cf36df704a2217b015d6455396830b0ce678b860ebfcc30f41403d7b1 +const FJORD_DATA: &[u8] = &hex!( + "440a5e200000146b000f79c500000000000000040000000066d052e700000000013ad8a + 3000000000000000000000000000000000000000000000000000000003ef12787000000 + 00000000000000000000000000000000000000000000000000000000012fdf87b89884a + 61e74b322bbcf60386f543bfae7827725efaaf0ab1de2294a5900000000000000000000 + 00006887246668a3b87f54deb3b94ba47a6f63f32985" +); diff --git a/crates/op-rbuilder/src/tests/framework/harness.rs b/crates/op-rbuilder/src/tests/framework/harness.rs index f55da367c..70641ee5e 100644 --- a/crates/op-rbuilder/src/tests/framework/harness.rs +++ b/crates/op-rbuilder/src/tests/framework/harness.rs @@ -3,7 +3,7 @@ use super::{ blocks::BlockGenerator, op::{OpRbuilderConfig, OpRethConfig}, service::{self, Service, ServiceInstance}, - TransactionBuilder, BUILDER_PRIVATE_KEY, + LocalInstance, TransactionBuilder, BUILDER_PRIVATE_KEY, }; use alloy_eips::BlockNumberOrTag; use alloy_network::Network; @@ -128,7 +128,7 @@ impl TestHarnessBuilder { let builder_log_path = builder.log_path.clone(); Ok(TestHarness { - framework: framework, + framework, builder_auth_rpc_port, builder_http_port, validator_auth_rpc_port, @@ -139,6 +139,7 @@ impl TestHarnessBuilder { pub struct TestHarness { framework: IntegrationFramework, + local_instance: LocalInstance, builder_auth_rpc_port: u16, builder_http_port: u16, validator_auth_rpc_port: u16, @@ -170,10 +171,8 @@ impl TestHarness { } pub async fn block_generator(&self) -> eyre::Result { - let engine_api = EngineApi::new_with_port(self.builder_auth_rpc_port).unwrap(); - let validation_api = Some(EngineApi::new_with_port(self.validator_auth_rpc_port).unwrap()); - - let mut generator = BlockGenerator::new(engine_api, validation_api, false, 1, None); + let engine_api = self.local_instance.engine_api(); + let mut generator = BlockGenerator::new(engine_api, None, false, 1, None); generator.init().await?; Ok(generator) diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs new file mode 100644 index 000000000..a490c9aa7 --- /dev/null +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -0,0 +1,241 @@ +use core::{ + any::Any, + future::Future, + pin::Pin, + task::{Context, Poll}, + time::Duration, +}; +use std::sync::{Arc, LazyLock}; + +use alloy_provider::{Identity, ProviderBuilder, RootProvider}; +use futures::FutureExt; +use nanoid::nanoid; +use op_alloy_network::Optimism; +use tokio::sync::oneshot; + +use reth::{ + args::{DatadirArgs, NetworkArgs, RpcServerArgs}, + core::exit::NodeExitFuture, + tasks::TaskManager, +}; +use reth_node_builder::{NodeBuilder, NodeConfig}; +use reth_optimism_chainspec::OpChainSpec; +use reth_optimism_node::{ + node::{OpAddOnsBuilder, OpPoolBuilder}, + OpNode, +}; + +use crate::{ + args::OpRbuilderArgs, + builders::{BuilderConfig, FlashblocksBuilder, PayloadBuilder, StandardBuilder}, + tx::FBPooledTransaction, + tx_signer::Signer, +}; + +use super::{ChainDriver, ChainDriverExt, EngineApi, Ipc, BUILDER_PRIVATE_KEY}; + +/// Represents a type that emulates a local in-process instance of the OP builder node. +/// This node uses IPC as the communication channel for the RPC server Engine API. +pub struct LocalInstance { + signer: Signer, + config: NodeConfig, + task_manager: Option, + exit_future: NodeExitFuture, + _node_handle: Box, +} + +impl LocalInstance { + /// Creates a new local instance of the OP builder node with the given arguments. + /// This method does not prefund any accounts, so before sending any transactions + /// make sure that sender accounts are funded. + pub async fn new(args: OpRbuilderArgs) -> eyre::Result { + let mut args = args; + let task_manager = task_manager(); + let config = node_config(); + let op_node = OpNode::new(args.rollup_args.clone()); + let (ready_tx, ready_rx) = oneshot::channel::<()>(); + let signer = args.builder_signer.clone().unwrap_or_else(|| { + Signer::try_from_secret( + BUILDER_PRIVATE_KEY + .parse() + .expect("Invalid builder private key"), + ) + .expect("Failed to create signer from private key") + }); + args.builder_signer = Some(signer.clone()); + let builder_config = BuilderConfig::::try_from(args.clone()) + .expect("Failed to convert rollup args to builder config"); + + let node_builder = NodeBuilder::<_, OpChainSpec>::new(config.clone()) + .testing_node(task_manager.executor()) + .with_types::() + .with_components( + op_node + .components() + .pool(pool_component(&args)) + .payload(P::new_service(builder_config)?), + ) + .with_add_ons( + OpAddOnsBuilder::default() + .with_sequencer(op_node.args.sequencer.clone()) + .with_enable_tx_conditional(op_node.args.enable_tx_conditional) + .build(), + ) + .on_rpc_started(move |_, _| { + let _ = ready_tx.send(()); + Ok(()) + }); + + let node_handle = node_builder.launch().await?; + let exit_future = node_handle.node_exit_future; + let node_handle: Box = Box::new(node_handle.node); + + // don't give the user an instance before we have a functioning RPC server + ready_rx.await.expect("Failed to receive ready signal"); + + Ok(Self { + signer, + config, + exit_future, + _node_handle: node_handle, + task_manager: Some(task_manager), + }) + } + + /// Creates new local instance of the OP builder node with the standard builder configuration. + /// This method prefunds the default accounts with 1 ETH each. + pub async fn standard() -> eyre::Result { + let instance = Self::new::(Default::default()).await?; + let driver = ChainDriver::new(&instance).await?; + driver.fund_default_accounts().await?; + Ok(instance) + } + + /// Creates new local instance of the OP builder node with the flashblocks builder configuration. + /// This method prefunds the default accounts with 1 ETH each. + pub async fn flashblocks() -> eyre::Result { + let instance = Self::new::(Default::default()).await?; + let driver = ChainDriver::new(&instance).await?; + driver.fund_default_accounts().await?; + Ok(instance) + } + + pub const fn config(&self) -> &NodeConfig { + &self.config + } + + pub const fn signer(&self) -> &Signer { + &self.signer + } + + pub fn rpc_ipc(&self) -> &str { + &self.config.rpc.ipcpath + } + + pub fn auth_ipc(&self) -> &str { + &self.config.rpc.auth_ipc_path + } + + pub fn engine_api(&self) -> EngineApi { + EngineApi::::with_ipc(self.auth_ipc()) + } + + pub async fn provider(&self) -> eyre::Result> { + ProviderBuilder::::default() + .connect_ipc(self.rpc_ipc().to_string().into()) + .await + .map_err(|e| eyre::eyre!("Failed to connect to provider: {e}")) + } +} + +impl Drop for LocalInstance { + fn drop(&mut self) { + if let Some(task_manager) = self.task_manager.take() { + task_manager.graceful_shutdown_with_timeout(Duration::from_secs(3)); + std::fs::remove_dir_all(self.config().datadir().to_string()).unwrap_or_else(|e| { + panic!( + "Failed to remove temporary data directory {}: {e}", + self.config().datadir() + ) + }); + } + } +} + +impl Future for LocalInstance { + type Output = eyre::Result<()>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + self.get_mut().exit_future.poll_unpin(cx) + } +} + +fn node_config() -> NodeConfig { + let tempdir = std::env::temp_dir(); + + let data_path = tempdir + .join(format!("rbuilder.{}.datadir", nanoid!())) + .to_path_buf(); + + std::fs::create_dir_all(&data_path).expect("Failed to create temporary data directory"); + + let rpc_ipc_path = tempdir + .join(format!("rbuilder.{}.rpc-ipc", nanoid!())) + .to_path_buf(); + + let auth_ipc_path = tempdir + .join(format!("rbuilder.{}.auth-ipc", nanoid!())) + .to_path_buf(); + + let mut rpc = RpcServerArgs::default().with_auth_ipc(); + rpc.ws = false; + rpc.http = false; + rpc.auth_port = 0; + rpc.ipcpath = rpc_ipc_path.to_string_lossy().into(); + rpc.auth_ipc_path = auth_ipc_path.to_string_lossy().into(); + + let mut network = NetworkArgs::default().with_unused_ports(); + network.discovery.disable_discovery = true; + + let datadir = DatadirArgs { + datadir: data_path + .to_string_lossy() + .parse() + .expect("Failed to parse data dir path"), + static_files_path: None, + }; + + NodeConfig::::new(chain_spec()) + .with_datadir_args(datadir) + .with_rpc(rpc) + .with_network(network) +} + +fn chain_spec() -> Arc { + static CHAIN_SPEC: LazyLock> = LazyLock::new(|| { + let genesis = include_str!("./artifacts/genesis.json.tmpl"); + let genesis = serde_json::from_str(genesis).expect("invalid genesis JSON"); + let chain_spec = OpChainSpec::from_genesis(genesis); + Arc::new(chain_spec) + }); + + CHAIN_SPEC.clone() +} + +fn task_manager() -> TaskManager { + TaskManager::new(tokio::runtime::Handle::current()) +} + +fn pool_component(args: &OpRbuilderArgs) -> OpPoolBuilder { + let rollup_args = &args.rollup_args; + OpPoolBuilder::::default() + .with_enable_tx_conditional( + // Revert protection uses the same internal pool logic as conditional transactions + // to garbage collect transactions out of the bundle range. + rollup_args.enable_tx_conditional || args.enable_revert_protection, + ) + .with_supervisor( + rollup_args.supervisor_http.clone(), + rollup_args.supervisor_safety_level, + ) +} diff --git a/crates/op-rbuilder/src/tests/framework/mod.rs b/crates/op-rbuilder/src/tests/framework/mod.rs index d8ffff3c0..2c68e645e 100644 --- a/crates/op-rbuilder/src/tests/framework/mod.rs +++ b/crates/op-rbuilder/src/tests/framework/mod.rs @@ -1,16 +1,23 @@ mod apis; -mod blocks; -mod harness; +//mod blocks; +mod chain; +//mod harness; +mod instance; mod op; mod service; mod txs; +mod utils; pub use apis::*; -pub use blocks::*; -pub use harness::*; +//pub use blocks::*; +pub use chain::*; + +//pub use harness::*; +pub use instance::*; pub use op::*; pub use service::*; pub use txs::*; +pub use utils::*; const BUILDER_PRIVATE_KEY: &str = "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; @@ -22,3 +29,25 @@ pub const DEFAULT_JWT_TOKEN: &str = "688f5d737bad920bdfb2fc2f488d6b6209eebda1dae949a8de91398d932c517a"; pub const ONE_ETH: u128 = 1_000_000_000_000_000_000; + +/// This gets invoked before any tests, when the cargo test framework loads the test library. +/// It injects itself into +#[ctor::ctor] +fn init_tests_logging() { + if let Ok(v) = std::env::var("TEST_TRACE") { + let level = match v.as_str() { + "false" | "off" => return, + "true" | "debug" | "on" => tracing::Level::DEBUG, + "trace" => tracing::Level::TRACE, + "info" => tracing::Level::INFO, + "warn" => tracing::Level::WARN, + "error" => tracing::Level::ERROR, + _ => return, + }; + + tracing_subscriber::fmt() + .with_max_level(level) + .with_test_writer() + .init(); + } +} diff --git a/crates/op-rbuilder/src/tests/framework/txs.rs b/crates/op-rbuilder/src/tests/framework/txs.rs index 70c8d3bd8..1721534a1 100644 --- a/crates/op-rbuilder/src/tests/framework/txs.rs +++ b/crates/op-rbuilder/src/tests/framework/txs.rs @@ -4,7 +4,7 @@ use crate::{ }; use alloy_consensus::TxEip1559; use alloy_eips::{eip2718::Encodable2718, BlockNumberOrTag}; -use alloy_primitives::{hex, Bytes}; +use alloy_primitives::{Address, Bytes, TxKind, U256}; use alloy_provider::{PendingTransactionBuilder, Provider, RootProvider}; use core::cmp::max; use op_alloy_consensus::{OpTxEnvelope, OpTypedTransaction}; @@ -48,11 +48,26 @@ impl TransactionBuilder { } } + pub fn with_to(mut self, to: Address) -> Self { + self.tx.to = TxKind::Call(to); + self + } + + pub fn with_create(mut self) -> Self { + self.tx.to = TxKind::Create; + self + } + pub fn with_key(mut self, key: u64) -> Self { self.key = Some(key); self } + pub fn with_value(mut self, value: u128) -> Self { + self.tx.value = U256::from(value); + self + } + pub fn with_signer(mut self, signer: Signer) -> Self { self.signer = Some(signer); self @@ -93,11 +108,6 @@ impl TransactionBuilder { self } - pub fn with_revert(mut self) -> Self { - self.tx.input = hex!("60006000fd").into(); - self - } - pub async fn build(mut self) -> Recovered { let signer = match self.signer { Some(signer) => signer, diff --git a/crates/op-rbuilder/src/tests/framework/utils.rs b/crates/op-rbuilder/src/tests/framework/utils.rs new file mode 100644 index 000000000..4b9ba9835 --- /dev/null +++ b/crates/op-rbuilder/src/tests/framework/utils.rs @@ -0,0 +1,63 @@ +use alloy_eips::Encodable2718; +use alloy_primitives::{hex, Address, TxKind, B256, U256}; +use core::future::Future; +use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; + +use crate::{tests::ONE_ETH, tx_signer::Signer}; + +use super::{TransactionBuilder, FUNDED_PRIVATE_KEYS}; + +pub trait TransactionBuilderExt { + fn random_valid_transfer(self) -> Self; + fn random_reverting_transaction(self) -> Self; +} + +impl TransactionBuilderExt for TransactionBuilder { + fn random_valid_transfer(self) -> Self { + self.with_to(rand::random::

()).with_value(1) + } + + fn random_reverting_transaction(self) -> Self { + self.with_input(hex!("60006000fd").into()) + } +} + +pub trait ChainDriverExt { + fn fund_default_accounts(&self) -> impl Future>; + fn fund(&self, address: Address, amount: u128) -> impl Future>; + fn first_funded_address(&self) -> Address { + FUNDED_PRIVATE_KEYS[0] + .parse() + .expect("Invalid funded private key") + } +} + +impl ChainDriverExt for super::ChainDriver { + async fn fund_default_accounts(&self) -> eyre::Result<()> { + for key in FUNDED_PRIVATE_KEYS { + let signer: Signer = key.parse()?; + self.fund(signer.address, ONE_ETH).await?; + } + Ok(()) + } + + async fn fund(&self, address: Address, amount: u128) -> eyre::Result<()> { + let deposit = TxDeposit { + source_hash: B256::default(), + from: address, // Set the sender to the address of the account to seed + to: TxKind::Create, + mint: Some(amount), // Amount to deposit + value: U256::default(), + gas_limit: 210000, + is_system_transaction: false, + input: Default::default(), // No input data for the deposit + }; + + let signer = Signer::random(); + let signed_tx = signer.sign_tx(OpTypedTransaction::Deposit(deposit))?; + let signed_tx_rlp = signed_tx.encoded_2718(); + self.build_new_block_with_txs(vec![signed_tx_rlp.into()]) + .await?; + Ok(()) + } +} diff --git a/crates/op-rbuilder/src/tests/mod.rs b/crates/op-rbuilder/src/tests/mod.rs index e3f96187c..33bf3ae84 100644 --- a/crates/op-rbuilder/src/tests/mod.rs +++ b/crates/op-rbuilder/src/tests/mod.rs @@ -2,5 +2,5 @@ mod framework; pub use framework::*; -mod flashblocks; -mod vanilla; +//mod flashblocks; +mod standard; diff --git a/crates/op-rbuilder/src/tests/vanilla/data_availability.rs b/crates/op-rbuilder/src/tests/standard/data_availability.rs similarity index 100% rename from crates/op-rbuilder/src/tests/vanilla/data_availability.rs rename to crates/op-rbuilder/src/tests/standard/data_availability.rs diff --git a/crates/op-rbuilder/src/tests/standard/mod.rs b/crates/op-rbuilder/src/tests/standard/mod.rs new file mode 100644 index 000000000..e64baaca5 --- /dev/null +++ b/crates/op-rbuilder/src/tests/standard/mod.rs @@ -0,0 +1,8 @@ +#![cfg(test)] + +// mod data_availability; +// mod ordering; +// mod revert; +//mod smoke; +mod smoke; +//mod txpool; diff --git a/crates/op-rbuilder/src/tests/vanilla/ordering.rs b/crates/op-rbuilder/src/tests/standard/ordering.rs similarity index 100% rename from crates/op-rbuilder/src/tests/vanilla/ordering.rs rename to crates/op-rbuilder/src/tests/standard/ordering.rs diff --git a/crates/op-rbuilder/src/tests/vanilla/revert.rs b/crates/op-rbuilder/src/tests/standard/revert.rs similarity index 100% rename from crates/op-rbuilder/src/tests/vanilla/revert.rs rename to crates/op-rbuilder/src/tests/standard/revert.rs diff --git a/crates/op-rbuilder/src/tests/standard/smoke.rs b/crates/op-rbuilder/src/tests/standard/smoke.rs new file mode 100644 index 000000000..96bf48a6a --- /dev/null +++ b/crates/op-rbuilder/src/tests/standard/smoke.rs @@ -0,0 +1,119 @@ +use core::sync::atomic::AtomicBool; +use std::collections::HashSet; + +use alloy_primitives::TxHash; +use tokio::join; + +use super::super::{ChainDriver, LocalInstance, TransactionBuilderExt}; + +#[tokio::test] + +/// This is a smoke test that ensures that transactions are included in blocks +/// and that the block generator is functioning correctly. +async fn chain_produces_blocks() -> eyre::Result<()> { + let rbuilder = LocalInstance::standard().await?; + let driver = ChainDriver::new(&rbuilder).await?; + + const SAMPLE_SIZE: usize = 10; + + // ensure that each block has at least two transactions when + // no user transactions are sent. + // the deposit transaction and the block generator's transaction + for _ in 0..SAMPLE_SIZE { + let block = driver.build_new_block().await?; + let transactions = block.transactions; + + assert_eq!( + transactions.len(), + 2, + "Empty blocks should have exactly two transactions" + ); + } + + // ensure that transactions are included in blocks and each block has all the transactions + // sent to it during its block time + the two mandatory transactions + for _ in 0..SAMPLE_SIZE { + let count = rand::random_range(1..8); + let mut tx_hashes = HashSet::::default(); + + for _ in 0..count { + let tx = driver + .transaction() + .random_valid_transfer() + .send() + .await + .expect("Failed to send transaction"); + tx_hashes.insert(*tx.tx_hash()); + } + + let block = driver.build_new_block().await?; + let txs = block.transactions; + + assert_eq!( + txs.len(), + 2 + count, + "Block should have {} transactions", + 2 + count + ); + + for tx_hash in tx_hashes { + assert!( + txs.hashes().any(|hash| hash == tx_hash), + "Transaction {} should be included in the block", + tx_hash + ); + } + } + + Ok(()) +} + +/// Ensures that payloads are generated correctly even when the builder is busy +/// with other requests, such as fcu or getPayload. +#[tokio::test] +async fn produces_blocks_under_load_within_deadline() -> eyre::Result<()> { + let rbuilder = LocalInstance::standard().await?; + let driver = ChainDriver::new(&rbuilder).await?; + + let done = AtomicBool::new(false); + + let (populate, produce) = join!( + async { + // Keep the builder busy with new transactions. + loop { + let _ = driver.transaction().random_valid_transfer().send().await?; + if done.load(std::sync::atomic::Ordering::Relaxed) { + break; + } + } + Ok::<(), eyre::Error>(()) + }, + async { + // give it some time for the transactions queue to fill up + // and the producer to send many txs. + tokio::time::sleep(std::time::Duration::from_secs(2)).await; + + for _ in 0..10 { + // Ensure that the builder can still produce blocks under + // heavy load of incoming transactions. + let _ = tokio::time::timeout( + std::time::Duration::from_secs(1), + driver.build_new_block(), + ) + .await + .map_err(|_| eyre::eyre!("Timeout while waiting for block"))?; + } + + // we're happy with one block produced under load + // set the done flag to true to stop the transaction population + done.store(true, std::sync::atomic::Ordering::Relaxed); + + Ok::<(), eyre::Error>(()) + } + ); + + assert!(populate.is_ok(), "Failed to populate transactions"); + assert!(produce.is_ok(), "Failed to produce block under load"); + + Ok(()) +} diff --git a/crates/op-rbuilder/src/tests/vanilla/txpool.rs b/crates/op-rbuilder/src/tests/standard/txpool.rs similarity index 100% rename from crates/op-rbuilder/src/tests/vanilla/txpool.rs rename to crates/op-rbuilder/src/tests/standard/txpool.rs diff --git a/crates/op-rbuilder/src/tests/vanilla/mod.rs b/crates/op-rbuilder/src/tests/vanilla/mod.rs deleted file mode 100644 index 976a3dc33..000000000 --- a/crates/op-rbuilder/src/tests/vanilla/mod.rs +++ /dev/null @@ -1,9 +0,0 @@ -#![cfg(test)] - -mod data_availability; -mod ordering; -mod revert; -mod smoke; -mod txpool; - -use super::*; diff --git a/crates/op-rbuilder/src/tests/vanilla/smoke.rs b/crates/op-rbuilder/src/tests/vanilla/smoke.rs deleted file mode 100644 index 33109c4a3..000000000 --- a/crates/op-rbuilder/src/tests/vanilla/smoke.rs +++ /dev/null @@ -1,130 +0,0 @@ -use super::framework::TestHarnessBuilder; -use alloy_provider::Provider; -use std::collections::HashSet; - -/// This is a smoke test that ensures that transactions are included in blocks -/// and that the block generator is functioning correctly. -#[tokio::test] -async fn chain_produces_blocks() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("chain_produces_blocks") - .build() - .await?; - - let mut generator = harness.block_generator().await?; - - const SAMPLE_SIZE: usize = 10; - - // ensure that each block has at least two transactions when - // no user transactions are sent. - // the deposit transaction and the block generator's transaction - for _ in 0..SAMPLE_SIZE { - generator - .generate_block() - .await - .expect("Failed to generate block"); - let transactions = harness.latest_block().await.transactions; - assert!( - transactions.len() == 2, - "Block should have exactly two transactions" - ); - } - - // ensure that transactions are included in blocks and each block has all the transactions - // sent to it during its block time + the two mandatory transactions - for _ in 0..SAMPLE_SIZE { - let count = rand::random_range(1..8); - let mut tx_hashes = HashSet::new(); - for _ in 0..count { - let tx = harness - .send_valid_transaction() - .await - .expect("Failed to send transaction"); - let tx_hash = *tx.tx_hash(); - tx_hashes.insert(tx_hash); - } - generator - .generate_block() - .await - .expect("Failed to generate block"); - let transactions = harness.latest_block().await.transactions; - - assert!( - transactions.len() == 2 + count, - "Block should have {} transactions", - 2 + count - ); - - for tx_hash in tx_hashes { - assert!( - transactions.hashes().any(|hash| hash == *tx_hash), - "Transaction {} should be included in the block", - tx_hash - ); - } - } - - Ok(()) -} - -/// Ensures that payloads are generated correctly even when the builder is busy -/// with other requests, such as fcu or getPayload. -#[tokio::test] -async fn get_payload_close_to_fcu() -> eyre::Result<()> { - let test_harness = TestHarnessBuilder::new("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 - let _ = test_harness.send_valid_transaction().await?; - } - - let result = tokio::time::timeout( - std::time::Duration::from_secs(1), - block_generator.submit_payload(None, 0, true), - ) - .await; - - // ensure we didn't timeout - let result = result.expect("Submit payload timed out"); - - // ensure we got a payload - assert!(result.is_ok(), "Failed to get payload: {:?}", result); - - Ok(()) -} - -/// This test validates that if we flood the builder with many transactions -/// and we request short block times, the builder can still eventually resolve all the transactions -#[tokio::test] -async fn transaction_flood_no_sleep() -> eyre::Result<()> { - let test_harness = TestHarnessBuilder::new("transaction_flood_no_sleep") - .build() - .await?; - - let mut block_generator = test_harness.block_generator().await?; - let provider = test_harness.provider()?; - - // Send 200 valid transactions to the builder - // More than this and there is an issue with the RPC endpoint not being able to handle the load - let mut transactions = vec![]; - for _ in 0..200 { - let tx = test_harness.send_valid_transaction().await?; - let tx_hash = *tx.tx_hash(); - transactions.push(tx_hash); - } - - // After a 10 blocks all the transactions should be included in a block - for _ in 0..10 { - block_generator.submit_payload(None, 0, true).await.unwrap(); - } - - for tx in transactions { - provider.get_transaction_receipt(tx).await?; - } - - Ok(()) -} From 0bc263616e53fd467a4966e45f55a51654b72326 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Wed, 28 May 2025 23:20:52 +0000 Subject: [PATCH 02/21] checkpoint --- Cargo.lock | 2 + Cargo.toml | 2 + crates/op-rbuilder/Cargo.toml | 2 + crates/op-rbuilder/src/lib.rs | 1 + .../src/tests/framework/instance.rs | 39 +- crates/op-rbuilder/src/tests/framework/txs.rs | 46 +- .../op-rbuilder/src/tests/framework/utils.rs | 18 +- crates/op-rbuilder/src/tests/standard/mod.rs | 7 +- .../src/tests/standard/ordering.rs | 33 +- .../op-rbuilder/src/tests/standard/revert.rs | 413 +++++++++--------- .../op-rbuilder/src/tests/standard/smoke.rs | 6 +- 11 files changed, 339 insertions(+), 230 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6436e3d36..24e581493 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5607,6 +5607,7 @@ dependencies = [ "reth-chainspec", "reth-cli", "reth-cli-util", + "reth-db-api", "reth-evm", "reth-execution-types", "reth-exex", @@ -5624,6 +5625,7 @@ dependencies = [ "reth-optimism-node", "reth-optimism-payload-builder", "reth-optimism-primitives", + "reth-optimism-rpc", "reth-optimism-txpool", "reth-payload-builder", "reth-payload-builder-primitives", diff --git a/Cargo.toml b/Cargo.toml index 913c4eee0..fcc464bde 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ reth-chain-state = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" reth-cli = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-cli-util = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-db = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } +reth-db-api = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-db-common = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-errors = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-payload-builder = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } @@ -89,6 +90,7 @@ reth-optimism-node = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4. reth-optimism-payload-builder = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-optimism-chainspec = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } reth-optimism-txpool = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } +reth-optimism-rpc = { git = "https://github.com/paradigmxyz/reth", tag = "v1.4.1" } # compatible with reth "v1.4.1 dependencies revm = { version = "23.1.0", features = [ diff --git a/crates/op-rbuilder/Cargo.toml b/crates/op-rbuilder/Cargo.toml index 58b3cb6f6..b20824c7d 100644 --- a/crates/op-rbuilder/Cargo.toml +++ b/crates/op-rbuilder/Cargo.toml @@ -17,6 +17,7 @@ reth-optimism-evm.workspace = true reth-optimism-consensus.workspace = true reth-optimism-primitives.workspace = true reth-optimism-txpool.workspace = true +reth-optimism-rpc.workspace = true reth-cli.workspace = true reth-cli-util.workspace = true reth-payload-primitives.workspace = true @@ -45,6 +46,7 @@ reth-optimism-forks.workspace = true reth-node-builder.workspace = true reth-rpc-eth-types.workspace = true reth-ipc = { workspace = true, optional = true } +reth-db-api.workspace = true alloy-primitives.workspace = true alloy-consensus.workspace = true diff --git a/crates/op-rbuilder/src/lib.rs b/crates/op-rbuilder/src/lib.rs index 8ab0888cb..d38fe4e18 100644 --- a/crates/op-rbuilder/src/lib.rs +++ b/crates/op-rbuilder/src/lib.rs @@ -4,6 +4,7 @@ pub mod primitives; pub mod tx_signer; mod metrics; +mod revert_protection; mod traits; mod tx; diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index a490c9aa7..476a63b4d 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -6,7 +6,7 @@ use core::{ time::Duration, }; use std::sync::{Arc, LazyLock}; - +use reth_transaction_pool::{AllTransactionsEvents, TransactionPool}; use alloy_provider::{Identity, ProviderBuilder, RootProvider}; use futures::FutureExt; use nanoid::nanoid; @@ -26,13 +26,10 @@ use reth_optimism_node::{ }; use crate::{ - args::OpRbuilderArgs, - builders::{BuilderConfig, FlashblocksBuilder, PayloadBuilder, StandardBuilder}, - tx::FBPooledTransaction, - tx_signer::Signer, + args::OpRbuilderArgs, builders::{BuilderConfig, FlashblocksBuilder, PayloadBuilder, StandardBuilder}, revert_protection::{EthApiOverrideServer, RevertProtectionExt}, tx::FBPooledTransaction, tx_signer::Signer }; -use super::{ChainDriver, ChainDriverExt, EngineApi, Ipc, BUILDER_PRIVATE_KEY}; +use super::{ChainDriver, ChainDriverExt, EngineApi, Ipc, TransactionPoolObserver, BUILDER_PRIVATE_KEY}; /// Represents a type that emulates a local in-process instance of the OP builder node. /// This node uses IPC as the communication channel for the RPC server Engine API. @@ -42,6 +39,7 @@ pub struct LocalInstance { task_manager: Option, exit_future: NodeExitFuture, _node_handle: Box, + pool_observer: TransactionPoolObserver } impl LocalInstance { @@ -65,7 +63,7 @@ impl LocalInstance { args.builder_signer = Some(signer.clone()); let builder_config = BuilderConfig::::try_from(args.clone()) .expect("Failed to convert rollup args to builder config"); - + println!("Builder config: {builder_config:#?}"); let node_builder = NodeBuilder::<_, OpChainSpec>::new(config.clone()) .testing_node(task_manager.executor()) .with_types::() @@ -81,6 +79,20 @@ impl LocalInstance { .with_enable_tx_conditional(op_node.args.enable_tx_conditional) .build(), ) + .extend_rpc_modules(move |ctx| { + if args.enable_revert_protection { + tracing::info!("Revert protection enabled"); + + let pool = ctx.pool().clone(); + let provider = ctx.provider().clone(); + let revert_protection_ext = RevertProtectionExt::new(pool, provider); + + ctx.modules + .merge_configured(revert_protection_ext.into_rpc())?; + } + + Ok(()) + }) .on_rpc_started(move |_, _| { let _ = ready_tx.send(()); Ok(()) @@ -88,7 +100,9 @@ impl LocalInstance { let node_handle = node_builder.launch().await?; let exit_future = node_handle.node_exit_future; - let node_handle: Box = Box::new(node_handle.node); + let boxed_handle = Box::new(node_handle.node); + let pool_monitor: AllTransactionsEvents = boxed_handle.pool.all_transactions_event_listener(); + let node_handle: Box = boxed_handle; // don't give the user an instance before we have a functioning RPC server ready_rx.await.expect("Failed to receive ready signal"); @@ -99,6 +113,7 @@ impl LocalInstance { exit_future, _node_handle: node_handle, task_manager: Some(task_manager), + pool_observer: TransactionPoolObserver::new(pool_monitor), }) } @@ -140,6 +155,14 @@ impl LocalInstance { EngineApi::::with_ipc(self.auth_ipc()) } + pub const fn pool(&self) -> &TransactionPoolObserver { + &self.pool_observer + } + + pub async fn driver(&self) -> eyre::Result { + ChainDriver::new(self).await + } + pub async fn provider(&self) -> eyre::Result> { ProviderBuilder::::default() .connect_ipc(self.rpc_ipc().to_string().into()) diff --git a/crates/op-rbuilder/src/tests/framework/txs.rs b/crates/op-rbuilder/src/tests/framework/txs.rs index 1721534a1..7de55185e 100644 --- a/crates/op-rbuilder/src/tests/framework/txs.rs +++ b/crates/op-rbuilder/src/tests/framework/txs.rs @@ -1,11 +1,11 @@ use crate::{ - primitives::bundle::{Bundle, BundleResult}, - tx_signer::Signer, + primitives::bundle::{Bundle, BundleResult}, tx::FBPooledTransaction, tx_signer::Signer }; use alloy_consensus::TxEip1559; use alloy_eips::{eip2718::Encodable2718, BlockNumberOrTag}; -use alloy_primitives::{Address, Bytes, TxKind, U256}; +use alloy_primitives::{Address, Bytes, TxHash, TxKind, U256}; use alloy_provider::{PendingTransactionBuilder, Provider, RootProvider}; +use reth_transaction_pool::AllTransactionsEvents; use core::cmp::max; use op_alloy_consensus::{OpTxEnvelope, OpTypedTransaction}; use op_alloy_network::Optimism; @@ -183,3 +183,43 @@ impl TransactionBuilder { .await?) } } + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TransactionStatus { + NotFound, + Pending, + Queued, + Dropped, +} + + +pub struct TransactionPoolObserver; + +impl TransactionPoolObserver { + pub fn new(stream: AllTransactionsEvents) -> Self { + Self + } + + pub fn current_status(&self, _txhash: TxHash) -> TransactionStatus { + TransactionStatus::NotFound + } + + pub fn is_pending(&self, txhash: TxHash) -> bool { + matches!(self.current_status(txhash), TransactionStatus::Pending) + } + + pub fn is_queued(&self, txhash: TxHash) -> bool { + matches!(self.current_status(txhash), TransactionStatus::Queued) + } + + pub fn is_dropped(&self, txhash: TxHash) -> bool { + matches!(self.current_status(txhash), TransactionStatus::Dropped) + } + + pub fn exists(&self, txhash: TxHash) -> bool { + matches!( + self.current_status(txhash), + TransactionStatus::Pending | TransactionStatus::Queued + ) + } +} \ No newline at end of file diff --git a/crates/op-rbuilder/src/tests/framework/utils.rs b/crates/op-rbuilder/src/tests/framework/utils.rs index 4b9ba9835..51c7b9665 100644 --- a/crates/op-rbuilder/src/tests/framework/utils.rs +++ b/crates/op-rbuilder/src/tests/framework/utils.rs @@ -18,7 +18,7 @@ impl TransactionBuilderExt for TransactionBuilder { } fn random_reverting_transaction(self) -> Self { - self.with_input(hex!("60006000fd").into()) + self.with_create().with_input(hex!("60006000fd").into()) // PUSH1 0x00 PUSH1 0x00 REVERT } } @@ -30,6 +30,22 @@ pub trait ChainDriverExt { .parse() .expect("Invalid funded private key") } + + fn fund_accounts( + &self, + count: usize, + amount: u128, + ) -> impl Future>> { + async move { + let accounts = (0..count).map(|_| Signer::random()).collect::>(); + + for account in &accounts { + self.fund(account.address, amount).await?; + } + + Ok(accounts) + } + } } impl ChainDriverExt for super::ChainDriver { diff --git a/crates/op-rbuilder/src/tests/standard/mod.rs b/crates/op-rbuilder/src/tests/standard/mod.rs index e64baaca5..11f920cee 100644 --- a/crates/op-rbuilder/src/tests/standard/mod.rs +++ b/crates/op-rbuilder/src/tests/standard/mod.rs @@ -1,8 +1,9 @@ #![cfg(test)] // mod data_availability; -// mod ordering; -// mod revert; -//mod smoke; +mod ordering; +mod revert; mod smoke; //mod txpool; + +use super::*; diff --git a/crates/op-rbuilder/src/tests/standard/ordering.rs b/crates/op-rbuilder/src/tests/standard/ordering.rs index f524e57d4..b244d4757 100644 --- a/crates/op-rbuilder/src/tests/standard/ordering.rs +++ b/crates/op-rbuilder/src/tests/standard/ordering.rs @@ -1,22 +1,24 @@ -use crate::tests::framework::{TestHarnessBuilder, ONE_ETH}; +use crate::tests::{ChainDriverExt, LocalInstance, ONE_ETH}; use alloy_consensus::Transaction; use futures::{future::join_all, stream, StreamExt}; /// This test ensures that the transactions are ordered by fee priority in the block. #[tokio::test] async fn fee_priority_ordering() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("integration_test_fee_priority_ordering") - .build() - .await?; + let rbuilder = LocalInstance::standard().await?; + let driver = rbuilder.driver().await?; + let accounts = driver.fund_accounts(10, ONE_ETH).await?; - let mut generator = harness.block_generator().await?; - let accounts = generator.create_funded_accounts(10, ONE_ETH).await?; - let base_fee = harness.latest_base_fee().await; + let latest_block = driver.latest().await?; + let base_fee = latest_block + .header + .base_fee_per_gas + .expect("Base fee should be present in the latest block"); // generate transactions with randomized tips let txs = join_all(accounts.iter().map(|signer| { - harness - .create_transaction() + driver + .transaction() .with_signer(*signer) .with_max_priority_fee_per_gas(rand::random_range(1..50)) .send() @@ -28,15 +30,16 @@ async fn fee_priority_ordering() -> eyre::Result<()> { .map(|tx| *tx.tx_hash()) .collect::>(); - generator.generate_block().await?; + driver.build_new_block().await?; // verify all transactions are included in the block assert!( stream::iter(txs.iter()) .all(|tx_hash| async { - harness - .latest_block() + driver + .latest() .await + .expect("Failed to fetch latest block") .transactions .hashes() .any(|hash| hash == *tx_hash) @@ -46,9 +49,9 @@ async fn fee_priority_ordering() -> eyre::Result<()> { ); // verify all transactions are ordered by fee priority - let txs_tips = harness - .latest_block() - .await + let txs_tips = driver + .latest() + .await? .into_transactions_vec() .into_iter() .skip(1) // skip the deposit transaction diff --git a/crates/op-rbuilder/src/tests/standard/revert.rs b/crates/op-rbuilder/src/tests/standard/revert.rs index 037ab032f..1c888f392 100644 --- a/crates/op-rbuilder/src/tests/standard/revert.rs +++ b/crates/op-rbuilder/src/tests/standard/revert.rs @@ -1,77 +1,49 @@ use alloy_provider::PendingTransactionBuilder; use op_alloy_network::Optimism; -use crate::{ - primitives::bundle::MAX_BLOCK_RANGE_BLOCKS, - tests::{BundleOpts, TestHarness, TestHarnessBuilder}, -}; - -/// This test ensures that the transactions that get reverted and not included in the block, -/// are eventually dropped from the pool once their block range is reached. -/// This test creates N transactions with different block ranges. +use crate::{args::OpRbuilderArgs, builders::StandardBuilder, tests::{BundleOpts, LocalInstance, TransactionBuilderExt}}; + +async fn local_instance_with_revert_protection() -> eyre::Result { + LocalInstance::new::(OpRbuilderArgs { + enable_revert_protection: true, + ..Default::default() + }).await +} + +/// If revert protection is disabled, the transactions that revert are included in the block. #[tokio::test] -async fn revert_protection_monitor_transaction_gc() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("revert_protection_monitor_transaction_gc") - .with_revert_protection() - .with_namespaces("eth,web3,txpool") - .build() - .await?; - - let mut generator = harness.block_generator().await?; - - // send 10 bundles with different block ranges - let mut pending_txn = Vec::new(); - for i in 1..=10 { - let txn = harness - .create_transaction() - .with_revert() - .with_bundle(BundleOpts { - block_number_max: Some(i), - }) - .send() - .await?; - pending_txn.push(txn); - } +async fn revert_protection_disabled() -> eyre::Result<()> { + let rbuilder = LocalInstance::standard().await?; + let driver = rbuilder.driver().await?; - // generate 10 blocks - for i in 0..10 { - let generated_block = generator.generate_block().await?; - - // blocks should only include two transactions (deposit + builder) - assert_eq!(generated_block.block.transactions.len(), 2); - - // since we created the 10 transactions with increasing block ranges, as we generate blocks - // one transaction will be gc on each block. - // transactions from [0, i] should be dropped, transactions from [i+1, 10] should be queued - for j in 0..=i { - let status = harness.check_tx_in_pool(*pending_txn[j].tx_hash()).await?; - assert!(status.is_dropped()); - } - for j in i + 1..10 { - let status = harness.check_tx_in_pool(*pending_txn[j].tx_hash()).await?; - assert!(status.is_queued()); - } + for _ in 0..10 { + let valid_tx = driver.transaction().random_valid_transfer().send().await?; + let reverting_tx = driver.transaction().random_reverting_transaction().send().await?; + let block = driver.build_new_block().await?; + + assert!(block.transactions.hashes().any(|hash| hash == *valid_tx.tx_hash())); + assert!(block.transactions.hashes().any(|hash| hash == *reverting_tx.tx_hash())); } Ok(()) } -/// If revert protection is disabled, the transactions that revert are included in the block. +/// If revert protection is enabled, the transactions that revert are not included in the block. #[tokio::test] -async fn revert_protection_disabled() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("revert_protection_disabled") - .build() - .await?; - - let mut generator = harness.block_generator().await?; +async fn revert_protection_enabled() -> eyre::Result<()> { + let rbuilder = LocalInstance::new::(OpRbuilderArgs { + enable_revert_protection: true, + ..Default::default() + }).await?; + let driver = rbuilder.driver().await?; for _ in 0..10 { - let valid_tx = harness.send_valid_transaction().await?; - let reverting_tx = harness.send_revert_transaction().await?; - let block_generated = generator.generate_block().await?; + let valid_tx = driver.transaction().random_valid_transfer().send().await?; + let reverting_tx = driver.transaction().random_reverting_transaction().send().await?; + let block = driver.build_new_block().await?; - assert!(block_generated.includes(*valid_tx.tx_hash())); - assert!(block_generated.includes(*reverting_tx.tx_hash())); + assert!(block.transactions.hashes().any(|hash| hash == *valid_tx.tx_hash())); + assert!(!block.transactions.hashes().any(|hash| hash == *reverting_tx.tx_hash())); } Ok(()) @@ -81,12 +53,11 @@ async fn revert_protection_disabled() -> eyre::Result<()> { /// since the revert RPC endpoint is not available. #[tokio::test] async fn revert_protection_disabled_bundle_endpoint_error() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("revert_protection_disabled_bundle_endpoint_error") - .build() - .await?; + let rbuilder = LocalInstance::standard().await?; + let driver = rbuilder.driver().await?; - let res = harness - .create_transaction() + let res = driver + .transaction() .with_bundle(BundleOpts::default()) .send() .await; @@ -98,135 +69,183 @@ async fn revert_protection_disabled_bundle_endpoint_error() -> eyre::Result<()> Ok(()) } -/// Test the behaviour of the revert protection bundle, if the bundle **does not** revert -/// the transaction is included in the block. If the bundle reverts, the transaction -/// is not included in the block and tried again for the next bundle range blocks -/// when it will be dropped from the pool. -#[tokio::test] -async fn revert_protection_bundle() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("revert_protection_bundle") - .with_revert_protection() - .with_namespaces("eth,web3,txpool") - .build() - .await?; - - let mut generator = harness.block_generator().await?; // Block 1 - - // Test 1: Bundle does not revert - let valid_bundle = harness - .create_transaction() - .with_bundle(BundleOpts::default()) - .send() - .await?; - - let block_generated = generator.generate_block().await?; // Block 2 - assert!(block_generated.includes(*valid_bundle.tx_hash())); - - let bundle_opts = BundleOpts { - block_number_max: Some(4), - }; - - let reverted_bundle = harness - .create_transaction() - .with_revert() - .with_bundle(bundle_opts) - .send() - .await?; - - // Test 2: Bundle reverts. It is not included in the block - let block_generated = generator.generate_block().await?; // Block 3 - assert!(block_generated.not_includes(*reverted_bundle.tx_hash())); - - // After the block the transaction is still pending in the pool - assert!(harness - .check_tx_in_pool(*reverted_bundle.tx_hash()) - .await? - .is_pending()); - - // Test 3: Chain progresses beyond the bundle range. The transaction is dropped from the pool - generator.generate_block().await?; // Block 4 - assert!(harness - .check_tx_in_pool(*reverted_bundle.tx_hash()) - .await? - .is_pending()); - - generator.generate_block().await?; // Block 5 - assert!(harness - .check_tx_in_pool(*reverted_bundle.tx_hash()) - .await? - .is_dropped()); - - Ok(()) -} - -/// Test the range limits for the revert protection bundle. -#[tokio::test] -async fn revert_protection_bundle_range_limits() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("revert_protection_bundle_range_limits") - .with_revert_protection() - .build() - .await?; - - let mut generator = harness.block_generator().await?; - - // Advance two blocks and try to send a bundle with max block = 1 - generator.generate_block().await?; // Block 1 - generator.generate_block().await?; // Block 2 - - async fn send_bundle( - harness: &TestHarness, - block_number_max: u64, - ) -> eyre::Result> { - harness - .create_transaction() - .with_bundle(BundleOpts { - block_number_max: Some(block_number_max), - }) - .send() - .await - } - - // Max block cannot be a past block - assert!(send_bundle(&harness, 1).await.is_err()); - - // Bundles are valid if their max block in in between the current block and the max block range - let next_valid_block = 3; - - for i in next_valid_block..next_valid_block + MAX_BLOCK_RANGE_BLOCKS { - assert!(send_bundle(&harness, i).await.is_ok()); - } - - // A bundle with a block out of range is invalid - assert!( - send_bundle(&harness, next_valid_block + MAX_BLOCK_RANGE_BLOCKS + 1) - .await - .is_err() - ); - - Ok(()) -} - -/// If a transaction reverts and was sent as a normal transaction through the eth_sendRawTransaction -/// bundle, the transaction should be included in the block. -/// This behaviour is the same as the 'revert_protection_disabled' test. -#[tokio::test] -async fn revert_protection_allow_reverted_transactions_without_bundle() -> eyre::Result<()> { - let harness = - TestHarnessBuilder::new("revert_protection_allow_reverted_transactions_without_bundle") - .with_revert_protection() - .build() - .await?; - - let mut generator = harness.block_generator().await?; - - for _ in 0..10 { - let valid_tx = harness.send_valid_transaction().await?; - let reverting_tx = harness.send_revert_transaction().await?; - let block_generated = generator.generate_block().await?; - - assert!(block_generated.includes(*valid_tx.tx_hash())); - assert!(block_generated.includes(*reverting_tx.tx_hash())); - } - - Ok(()) -} +// /// This test ensures that the transactions that get reverted and not included in the block, +// /// are eventually dropped from the pool once their block range is reached. +// /// This test creates N transactions with different block ranges. +// #[tokio::test] +// async fn revert_protection_monitor_transaction_gc() -> eyre::Result<()> { +// let rbuilder = LocalInstance::standard().await?; +// let driver = rbuilder.driver().await?; + +// let latest_block_number = driver.latest().await?.header.number; + +// // send 10 bundles with different block ranges +// let mut pending_txn = Vec::new(); +// for i in 1..=10 { +// let txn = driver +// .transaction() +// .random_reverting_transaction() +// .with_bundle(BundleOpts { +// block_number_max: Some(latest_block_number + i), +// }) +// .send() +// .await?; +// pending_txn.push(txn); +// } + +// // generate 10 blocks +// for i in 0..10 { +// let block = driver.build_new_block().await?; + +// // blocks should only include two transactions (deposit + builder) +// assert_eq!(block.transactions.len(), 2); + +// // since we created the 10 transactions with increasing block ranges, as we generate blocks +// // one transaction will be gc on each block. +// // transactions from [0, i] should be dropped, transactions from [i+1, 10] should be queued +// for j in 0..=i { +// let status = harness.check_tx_in_pool(*pending_txn[j].tx_hash()).await?; +// assert!(status.is_dropped()); +// } +// for j in i + 1..10 { +// let status = harness.check_tx_in_pool(*pending_txn[j].tx_hash()).await?; +// assert!(status.is_queued()); +// } +// } + +// Ok(()) +// } + + +// /// Test the behaviour of the revert protection bundle, if the bundle **does not** revert +// /// the transaction is included in the block. If the bundle reverts, the transaction +// /// is not included in the block and tried again for the next bundle range blocks +// /// when it will be dropped from the pool. +// #[tokio::test] +// async fn revert_protection_bundle() -> eyre::Result<()> { +// let harness = TestHarnessBuilder::new("revert_protection_bundle") +// .with_revert_protection() +// .with_namespaces("eth,web3,txpool") +// .build() +// .await?; + +// let mut generator = harness.block_generator().await?; // Block 1 + +// // Test 1: Bundle does not revert +// let valid_bundle = harness +// .create_transaction() +// .with_bundle(BundleOpts::default()) +// .send() +// .await?; + +// let block_generated = generator.generate_block().await?; // Block 2 +// assert!(block_generated.includes(*valid_bundle.tx_hash())); + +// let bundle_opts = BundleOpts { +// block_number_max: Some(4), +// }; + +// let reverted_bundle = harness +// .create_transaction() +// .with_revert() +// .with_bundle(bundle_opts) +// .send() +// .await?; + +// // Test 2: Bundle reverts. It is not included in the block +// let block_generated = generator.generate_block().await?; // Block 3 +// assert!(block_generated.not_includes(*reverted_bundle.tx_hash())); + +// // After the block the transaction is still pending in the pool +// assert!(harness +// .check_tx_in_pool(*reverted_bundle.tx_hash()) +// .await? +// .is_pending()); + +// // Test 3: Chain progresses beyond the bundle range. The transaction is dropped from the pool +// generator.generate_block().await?; // Block 4 +// assert!(harness +// .check_tx_in_pool(*reverted_bundle.tx_hash()) +// .await? +// .is_pending()); + +// generator.generate_block().await?; // Block 5 +// assert!(harness +// .check_tx_in_pool(*reverted_bundle.tx_hash()) +// .await? +// .is_dropped()); + +// Ok(()) +// } + +// /// Test the range limits for the revert protection bundle. +// #[tokio::test] +// async fn revert_protection_bundle_range_limits() -> eyre::Result<()> { +// let harness = TestHarnessBuilder::new("revert_protection_bundle_range_limits") +// .with_revert_protection() +// .build() +// .await?; + +// let mut generator = harness.block_generator().await?; + +// // Advance two blocks and try to send a bundle with max block = 1 +// generator.generate_block().await?; // Block 1 +// generator.generate_block().await?; // Block 2 + +// async fn send_bundle( +// harness: &TestHarness, +// block_number_max: u64, +// ) -> eyre::Result> { +// harness +// .create_transaction() +// .with_bundle(BundleOpts { +// block_number_max: Some(block_number_max), +// }) +// .send() +// .await +// } + +// // Max block cannot be a past block +// assert!(send_bundle(&harness, 1).await.is_err()); + +// // Bundles are valid if their max block in in between the current block and the max block range +// let next_valid_block = 3; + +// for i in next_valid_block..next_valid_block + MAX_BLOCK_RANGE_BLOCKS { +// assert!(send_bundle(&harness, i).await.is_ok()); +// } + +// // A bundle with a block out of range is invalid +// assert!( +// send_bundle(&harness, next_valid_block + MAX_BLOCK_RANGE_BLOCKS + 1) +// .await +// .is_err() +// ); + +// Ok(()) +// } + +// /// If a transaction reverts and was sent as a normal transaction through the eth_sendRawTransaction +// /// bundle, the transaction should be included in the block. +// /// This behaviour is the same as the 'revert_protection_disabled' test. +// #[tokio::test] +// async fn revert_protection_allow_reverted_transactions_without_bundle() -> eyre::Result<()> { +// let harness = +// TestHarnessBuilder::new("revert_protection_allow_reverted_transactions_without_bundle") +// .with_revert_protection() +// .build() +// .await?; + +// let mut generator = harness.block_generator().await?; + +// for _ in 0..10 { +// let valid_tx = harness.send_valid_transaction().await?; +// let reverting_tx = harness.send_revert_transaction().await?; +// let block_generated = generator.generate_block().await?; + +// assert!(block_generated.includes(*valid_tx.tx_hash())); +// assert!(block_generated.includes(*reverting_tx.tx_hash())); +// } + +// Ok(()) +// } diff --git a/crates/op-rbuilder/src/tests/standard/smoke.rs b/crates/op-rbuilder/src/tests/standard/smoke.rs index 96bf48a6a..0a585ddc9 100644 --- a/crates/op-rbuilder/src/tests/standard/smoke.rs +++ b/crates/op-rbuilder/src/tests/standard/smoke.rs @@ -4,7 +4,7 @@ use std::collections::HashSet; use alloy_primitives::TxHash; use tokio::join; -use super::super::{ChainDriver, LocalInstance, TransactionBuilderExt}; +use crate::tests::{ChainDriver, LocalInstance, TransactionBuilderExt}; #[tokio::test] @@ -103,11 +103,11 @@ async fn produces_blocks_under_load_within_deadline() -> eyre::Result<()> { .await .map_err(|_| eyre::eyre!("Timeout while waiting for block"))?; } - + // we're happy with one block produced under load // set the done flag to true to stop the transaction population done.store(true, std::sync::atomic::Ordering::Relaxed); - + Ok::<(), eyre::Error>(()) } ); From 5207e43a1b2ae82ba4cf00229465929c61dc252c Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 10:14:30 +0000 Subject: [PATCH 03/21] More tests are passing --- Cargo.lock | 1 + crates/op-rbuilder/Cargo.toml | 4 +- crates/op-rbuilder/src/lib.rs | 4 +- .../src/tests/framework/instance.rs | 49 +++- crates/op-rbuilder/src/tests/framework/mod.rs | 15 +- crates/op-rbuilder/src/tests/framework/txs.rs | 129 ++++++++- .../op-rbuilder/src/tests/framework/utils.rs | 20 ++ crates/op-rbuilder/src/tests/standard/mod.rs | 2 - .../op-rbuilder/src/tests/standard/revert.rs | 257 ++++++++++-------- 9 files changed, 334 insertions(+), 147 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 24e581493..0c87744d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5585,6 +5585,7 @@ dependencies = [ "clap", "clap_builder", "ctor", + "dashmap 6.1.0", "derive_more", "eyre", "futures", diff --git a/crates/op-rbuilder/Cargo.toml b/crates/op-rbuilder/Cargo.toml index b20824c7d..3fcb470c7 100644 --- a/crates/op-rbuilder/Cargo.toml +++ b/crates/op-rbuilder/Cargo.toml @@ -103,6 +103,7 @@ tracing-subscriber = { version = "0.3.18", features = ["env-filter", "json"] } shellexpand = "3.1" serde_yaml = { version = "0.9" } nanoid = { version = "0.4", optional = true } +dashmap = { version = "6.1", optional = true } # `msozin/flashblocks-v1.4.1` branch based on `flashblocks-rebase` rollup-boost = { git = "http://github.com/flashbots/rollup-boost", rev = "8506dfb7d84c65746f7c88d250983658438f59e8" } @@ -123,6 +124,7 @@ reth-node-builder = { workspace = true, features = ["test-utils"] } reth-ipc.workspace = true reth-rpc-eth-types.workspace = true ctor = "0.4.2" +dashmap = "6.1" [features] default = ["jemalloc"] @@ -145,7 +147,7 @@ min-info-logs = ["tracing/release_max_level_info"] min-debug-logs = ["tracing/release_max_level_debug"] min-trace-logs = ["tracing/release_max_level_trace"] -testing = ["nanoid", "reth-node-builder/test-utils", "reth-ipc"] +testing = ["nanoid", "reth-node-builder/test-utils", "reth-ipc", "dashmap"] [[bin]] name = "op-rbuilder" diff --git a/crates/op-rbuilder/src/lib.rs b/crates/op-rbuilder/src/lib.rs index d38fe4e18..9862b4de6 100644 --- a/crates/op-rbuilder/src/lib.rs +++ b/crates/op-rbuilder/src/lib.rs @@ -4,9 +4,11 @@ pub mod primitives; pub mod tx_signer; mod metrics; -mod revert_protection; mod traits; mod tx; #[cfg(any(test, feature = "testing"))] pub mod tests; + +#[cfg(any(test, feature = "testing"))] +mod revert_protection; diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 476a63b4d..9866525f8 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -1,3 +1,4 @@ +use alloy_provider::{Identity, ProviderBuilder, RootProvider}; use core::{ any::Any, future::Future, @@ -5,12 +6,11 @@ use core::{ task::{Context, Poll}, time::Duration, }; -use std::sync::{Arc, LazyLock}; -use reth_transaction_pool::{AllTransactionsEvents, TransactionPool}; -use alloy_provider::{Identity, ProviderBuilder, RootProvider}; use futures::FutureExt; use nanoid::nanoid; use op_alloy_network::Optimism; +use reth_transaction_pool::{AllTransactionsEvents, TransactionPool}; +use std::sync::{Arc, LazyLock}; use tokio::sync::oneshot; use reth::{ @@ -26,10 +26,16 @@ use reth_optimism_node::{ }; use crate::{ - args::OpRbuilderArgs, builders::{BuilderConfig, FlashblocksBuilder, PayloadBuilder, StandardBuilder}, revert_protection::{EthApiOverrideServer, RevertProtectionExt}, tx::FBPooledTransaction, tx_signer::Signer + args::OpRbuilderArgs, + builders::{BuilderConfig, FlashblocksBuilder, PayloadBuilder, StandardBuilder}, + revert_protection::{EthApiOverrideServer, RevertProtectionExt}, + tx::FBPooledTransaction, + tx_signer::Signer, }; -use super::{ChainDriver, ChainDriverExt, EngineApi, Ipc, TransactionPoolObserver, BUILDER_PRIVATE_KEY}; +use super::{ + ChainDriver, ChainDriverExt, EngineApi, Ipc, TransactionPoolObserver, BUILDER_PRIVATE_KEY, +}; /// Represents a type that emulates a local in-process instance of the OP builder node. /// This node uses IPC as the communication channel for the RPC server Engine API. @@ -39,7 +45,7 @@ pub struct LocalInstance { task_manager: Option, exit_future: NodeExitFuture, _node_handle: Box, - pool_observer: TransactionPoolObserver + pool_observer: TransactionPoolObserver, } impl LocalInstance { @@ -51,7 +57,11 @@ impl LocalInstance { let task_manager = task_manager(); let config = node_config(); let op_node = OpNode::new(args.rollup_args.clone()); - let (ready_tx, ready_rx) = oneshot::channel::<()>(); + + let (rpc_ready_tx, rpc_ready_rx) = oneshot::channel::<()>(); + let (txpool_ready_tx, txpool_ready_rx) = + oneshot::channel::>(); + let signer = args.builder_signer.clone().unwrap_or_else(|| { Signer::try_from_secret( BUILDER_PRIVATE_KEY @@ -61,6 +71,7 @@ impl LocalInstance { .expect("Failed to create signer from private key") }); args.builder_signer = Some(signer.clone()); + args.rollup_args.enable_tx_conditional = true; let builder_config = BuilderConfig::::try_from(args.clone()) .expect("Failed to convert rollup args to builder config"); println!("Builder config: {builder_config:#?}"); @@ -94,18 +105,27 @@ impl LocalInstance { Ok(()) }) .on_rpc_started(move |_, _| { - let _ = ready_tx.send(()); + let _ = rpc_ready_tx.send(()); + Ok(()) + }) + .on_node_started(move |ctx| { + let _ = txpool_ready_tx + .send(ctx.pool.all_transactions_event_listener()) + .expect("Failed to send txpool ready signal"); + Ok(()) }); let node_handle = node_builder.launch().await?; let exit_future = node_handle.node_exit_future; let boxed_handle = Box::new(node_handle.node); - let pool_monitor: AllTransactionsEvents = boxed_handle.pool.all_transactions_event_listener(); let node_handle: Box = boxed_handle; - // don't give the user an instance before we have a functioning RPC server - ready_rx.await.expect("Failed to receive ready signal"); + // Wait for all required components to be ready + rpc_ready_rx.await.expect("Failed to receive ready signal"); + let pool_monitor = txpool_ready_rx + .await + .expect("Failed to receive txpool ready signal"); Ok(Self { signer, @@ -195,19 +215,20 @@ impl Future for LocalInstance { fn node_config() -> NodeConfig { let tempdir = std::env::temp_dir(); + let random_id = nanoid!(); let data_path = tempdir - .join(format!("rbuilder.{}.datadir", nanoid!())) + .join(format!("rbuilder.{random_id}.datadir")) .to_path_buf(); std::fs::create_dir_all(&data_path).expect("Failed to create temporary data directory"); let rpc_ipc_path = tempdir - .join(format!("rbuilder.{}.rpc-ipc", nanoid!())) + .join(format!("rbuilder.{random_id}.rpc-ipc")) .to_path_buf(); let auth_ipc_path = tempdir - .join(format!("rbuilder.{}.auth-ipc", nanoid!())) + .join(format!("rbuilder.{random_id}.auth-ipc")) .to_path_buf(); let mut rpc = RpcServerArgs::default().with_auth_ipc(); diff --git a/crates/op-rbuilder/src/tests/framework/mod.rs b/crates/op-rbuilder/src/tests/framework/mod.rs index 2c68e645e..377478f94 100644 --- a/crates/op-rbuilder/src/tests/framework/mod.rs +++ b/crates/op-rbuilder/src/tests/framework/mod.rs @@ -16,6 +16,7 @@ pub use chain::*; pub use instance::*; pub use op::*; pub use service::*; +use tracing_subscriber::{filter::filter_fn, prelude::*}; pub use txs::*; pub use utils::*; @@ -45,9 +46,17 @@ fn init_tests_logging() { _ => return, }; - tracing_subscriber::fmt() - .with_max_level(level) - .with_test_writer() + // let prefix_blacklist = &["alloy_transport_ipc", "storage::db::mdbx"]; + let prefix_blacklist = &["storage::db::mdbx"]; + + tracing_subscriber::registry() + .with(tracing_subscriber::fmt::layer()) + .with(filter_fn(move |metadata| { + metadata.level() <= &level + && !prefix_blacklist + .iter() + .any(|prefix| metadata.target().starts_with(prefix)) + })) .init(); } } diff --git a/crates/op-rbuilder/src/tests/framework/txs.rs b/crates/op-rbuilder/src/tests/framework/txs.rs index 7de55185e..517c76b00 100644 --- a/crates/op-rbuilder/src/tests/framework/txs.rs +++ b/crates/op-rbuilder/src/tests/framework/txs.rs @@ -1,15 +1,22 @@ use crate::{ - primitives::bundle::{Bundle, BundleResult}, tx::FBPooledTransaction, tx_signer::Signer + primitives::bundle::{Bundle, BundleResult}, + tx::FBPooledTransaction, + tx_signer::Signer, }; use alloy_consensus::TxEip1559; use alloy_eips::{eip2718::Encodable2718, BlockNumberOrTag}; use alloy_primitives::{Address, Bytes, TxHash, TxKind, U256}; use alloy_provider::{PendingTransactionBuilder, Provider, RootProvider}; -use reth_transaction_pool::AllTransactionsEvents; use core::cmp::max; +use dashmap::DashMap; +use futures::StreamExt; use op_alloy_consensus::{OpTxEnvelope, OpTypedTransaction}; use op_alloy_network::Optimism; use reth_primitives::Recovered; +use reth_transaction_pool::{AllTransactionsEvents, FullTransactionEvent, TransactionEvent}; +use std::{collections::VecDeque, sync::Arc}; +use tokio::sync::watch; +use tracing::debug; use alloy_eips::eip1559::MIN_PROTOCOL_BASE_FEE; @@ -192,34 +199,132 @@ pub enum TransactionStatus { Dropped, } +impl TransactionStatus { + pub fn is_pending(&self) -> bool { + matches!(self, TransactionStatus::Pending) + } + + pub fn is_queued(&self) -> bool { + matches!(self, TransactionStatus::Queued) + } + + pub fn is_dropped(&self) -> bool { + matches!(self, TransactionStatus::Dropped) + } +} -pub struct TransactionPoolObserver; +type ObservationsMap = DashMap>; + +pub struct TransactionPoolObserver { + /// Stores a mapping of all observed transactions to their history of events. + observations: Arc, + + /// Fired when this type is dropped, giving a signal to the listener loop + /// to stop listening for events. + term: Option>, +} + +impl Drop for TransactionPoolObserver { + fn drop(&mut self) { + // Signal the listener loop to stop listening for events + if let Some(term) = self.term.take() { + let _ = term.send(true); + } + } +} impl TransactionPoolObserver { pub fn new(stream: AllTransactionsEvents) -> Self { - Self + let mut stream = stream; + let observations = Arc::new(ObservationsMap::new()); + let observations_clone = Arc::clone(&observations); + let (term, mut term_rx) = watch::channel(false); + + tokio::spawn(async move { + let observations = observations_clone; + + loop { + tokio::select! { + _ = term_rx.changed() => { + if *term_rx.borrow() { + debug!("Transaction pool observer terminated."); + return; + } + } + tx_event = stream.next() => { + match tx_event { + Some(FullTransactionEvent::Pending(hash)) => { + tracing::debug!("Transaction pending: {hash}"); + observations.entry(hash).or_default().push_back(TransactionEvent::Pending); + }, + Some(FullTransactionEvent::Queued(hash)) => { + tracing::debug!("Transaction queued: {hash}"); + observations.entry(hash).or_default().push_back(TransactionEvent::Queued); + }, + Some(FullTransactionEvent::Mined { tx_hash, block_hash }) => { + tracing::debug!("Transaction mined: {tx_hash} in block {block_hash}"); + observations.entry(tx_hash).or_default().push_back(TransactionEvent::Mined(block_hash)); + }, + Some(FullTransactionEvent::Replaced { transaction, replaced_by }) => { + tracing::debug!("Transaction replaced: {transaction:?} by {replaced_by}"); + observations.entry(*transaction.hash()).or_default().push_back(TransactionEvent::Replaced(replaced_by)); + }, + Some(FullTransactionEvent::Discarded(hash)) => { + tracing::debug!("Transaction discarded: {hash}"); + observations.entry(hash).or_default().push_back(TransactionEvent::Discarded); + }, + Some(FullTransactionEvent::Invalid(hash)) => { + tracing::debug!("Transaction invalid: {hash}"); + observations.entry(hash).or_default().push_back(TransactionEvent::Invalid); + }, + Some(FullTransactionEvent::Propagated(_)) => {}, + None => {}, + } + } + } + } + }); + + Self { + observations, + term: Some(term), + } } - pub fn current_status(&self, _txhash: TxHash) -> TransactionStatus { - TransactionStatus::NotFound + pub fn tx_status(&self, txhash: TxHash) -> Option { + self.observations + .get(&txhash) + .map(|history| history.back().cloned()) + .flatten() } pub fn is_pending(&self, txhash: TxHash) -> bool { - matches!(self.current_status(txhash), TransactionStatus::Pending) + matches!(self.tx_status(txhash), Some(TransactionEvent::Pending)) } pub fn is_queued(&self, txhash: TxHash) -> bool { - matches!(self.current_status(txhash), TransactionStatus::Queued) + matches!(self.tx_status(txhash), Some(TransactionEvent::Queued)) } pub fn is_dropped(&self, txhash: TxHash) -> bool { - matches!(self.current_status(txhash), TransactionStatus::Dropped) + matches!(self.tx_status(txhash), Some(TransactionEvent::Discarded)) + } + + /// Returns the history of pool events for a transaction. + pub fn history(&self, txhash: TxHash) -> Option> { + self.observations + .get(&txhash) + .map(|history| history.iter().cloned().collect()) + } + + pub fn print_all(&self) { + tracing::debug!("TxPool {:#?}", self.observations); } pub fn exists(&self, txhash: TxHash) -> bool { matches!( - self.current_status(txhash), - TransactionStatus::Pending | TransactionStatus::Queued + self.tx_status(txhash), + Some(TransactionEvent::Pending) | Some(TransactionEvent::Queued) ) } -} \ No newline at end of file +} diff --git a/crates/op-rbuilder/src/tests/framework/utils.rs b/crates/op-rbuilder/src/tests/framework/utils.rs index 51c7b9665..18fb2432a 100644 --- a/crates/op-rbuilder/src/tests/framework/utils.rs +++ b/crates/op-rbuilder/src/tests/framework/utils.rs @@ -1,5 +1,7 @@ use alloy_eips::Encodable2718; use alloy_primitives::{hex, Address, TxKind, B256, U256}; +use alloy_rpc_types_eth::{Block, BlockTransactionHashes}; +use op_alloy_rpc_types::Transaction; use core::future::Future; use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; @@ -77,3 +79,21 @@ impl ChainDriverExt for super::ChainDriver { Ok(()) } } + + +pub trait BlockTransactionsExt { + fn includes(&self, tx_hash: &B256) -> bool; +} + +impl BlockTransactionsExt for Block { + fn includes(&self, tx_hash: &B256) -> bool { + self.transactions.hashes().any(|tx| tx == *tx_hash) + } +} + +impl<'a> BlockTransactionsExt for BlockTransactionHashes<'a, Transaction> { + fn includes(&self, tx_hash: &B256) -> bool { + let mut iter = self.clone(); + iter.any(|tx| tx == *tx_hash) + } +} \ No newline at end of file diff --git a/crates/op-rbuilder/src/tests/standard/mod.rs b/crates/op-rbuilder/src/tests/standard/mod.rs index 11f920cee..d55a20380 100644 --- a/crates/op-rbuilder/src/tests/standard/mod.rs +++ b/crates/op-rbuilder/src/tests/standard/mod.rs @@ -5,5 +5,3 @@ mod ordering; mod revert; mod smoke; //mod txpool; - -use super::*; diff --git a/crates/op-rbuilder/src/tests/standard/revert.rs b/crates/op-rbuilder/src/tests/standard/revert.rs index 1c888f392..8853ea4ee 100644 --- a/crates/op-rbuilder/src/tests/standard/revert.rs +++ b/crates/op-rbuilder/src/tests/standard/revert.rs @@ -1,14 +1,10 @@ -use alloy_provider::PendingTransactionBuilder; -use op_alloy_network::Optimism; +use reth_transaction_pool::TransactionEvent; -use crate::{args::OpRbuilderArgs, builders::StandardBuilder, tests::{BundleOpts, LocalInstance, TransactionBuilderExt}}; - -async fn local_instance_with_revert_protection() -> eyre::Result { - LocalInstance::new::(OpRbuilderArgs { - enable_revert_protection: true, - ..Default::default() - }).await -} +use crate::{ + args::OpRbuilderArgs, + builders::StandardBuilder, + tests::{BlockTransactionsExt, BundleOpts, ChainDriverExt, LocalInstance, TransactionBuilderExt, ONE_ETH}, +}; /// If revert protection is disabled, the transactions that revert are included in the block. #[tokio::test] @@ -18,11 +14,21 @@ async fn revert_protection_disabled() -> eyre::Result<()> { for _ in 0..10 { let valid_tx = driver.transaction().random_valid_transfer().send().await?; - let reverting_tx = driver.transaction().random_reverting_transaction().send().await?; + let reverting_tx = driver + .transaction() + .random_reverting_transaction() + .send() + .await?; let block = driver.build_new_block().await?; - assert!(block.transactions.hashes().any(|hash| hash == *valid_tx.tx_hash())); - assert!(block.transactions.hashes().any(|hash| hash == *reverting_tx.tx_hash())); + assert!(block + .transactions + .hashes() + .any(|hash| hash == *valid_tx.tx_hash())); + assert!(block + .transactions + .hashes() + .any(|hash| hash == *reverting_tx.tx_hash())); } Ok(()) @@ -30,20 +36,32 @@ async fn revert_protection_disabled() -> eyre::Result<()> { /// If revert protection is enabled, the transactions that revert are not included in the block. #[tokio::test] +#[ignore = "This test is ignored for now because the revert protection logic has changed"] async fn revert_protection_enabled() -> eyre::Result<()> { let rbuilder = LocalInstance::new::(OpRbuilderArgs { enable_revert_protection: true, ..Default::default() - }).await?; + }) + .await?; let driver = rbuilder.driver().await?; for _ in 0..10 { let valid_tx = driver.transaction().random_valid_transfer().send().await?; - let reverting_tx = driver.transaction().random_reverting_transaction().send().await?; + let reverting_tx = driver + .transaction() + .random_reverting_transaction() + .send() + .await?; let block = driver.build_new_block().await?; - assert!(block.transactions.hashes().any(|hash| hash == *valid_tx.tx_hash())); - assert!(!block.transactions.hashes().any(|hash| hash == *reverting_tx.tx_hash())); + assert!(block + .transactions + .hashes() + .any(|hash| hash == *valid_tx.tx_hash())); + assert!(!block + .transactions + .hashes() + .any(|hash| hash == *reverting_tx.tx_hash())); } Ok(()) @@ -69,114 +87,125 @@ async fn revert_protection_disabled_bundle_endpoint_error() -> eyre::Result<()> Ok(()) } -// /// This test ensures that the transactions that get reverted and not included in the block, -// /// are eventually dropped from the pool once their block range is reached. -// /// This test creates N transactions with different block ranges. -// #[tokio::test] -// async fn revert_protection_monitor_transaction_gc() -> eyre::Result<()> { -// let rbuilder = LocalInstance::standard().await?; -// let driver = rbuilder.driver().await?; - -// let latest_block_number = driver.latest().await?.header.number; - -// // send 10 bundles with different block ranges -// let mut pending_txn = Vec::new(); -// for i in 1..=10 { -// let txn = driver -// .transaction() -// .random_reverting_transaction() -// .with_bundle(BundleOpts { -// block_number_max: Some(latest_block_number + i), -// }) -// .send() -// .await?; -// pending_txn.push(txn); -// } +/// This test ensures that the transactions that get reverted and not included in the block, +/// are eventually dropped from the pool once their block range is reached. +/// This test creates N transactions with different block ranges. +#[tokio::test] +async fn revert_protection_monitor_transaction_gc() -> eyre::Result<()> { + let rbuilder = LocalInstance::new::(OpRbuilderArgs { + enable_revert_protection: true, + ..Default::default() + }) + .await?; -// // generate 10 blocks -// for i in 0..10 { -// let block = driver.build_new_block().await?; - -// // blocks should only include two transactions (deposit + builder) -// assert_eq!(block.transactions.len(), 2); - -// // since we created the 10 transactions with increasing block ranges, as we generate blocks -// // one transaction will be gc on each block. -// // transactions from [0, i] should be dropped, transactions from [i+1, 10] should be queued -// for j in 0..=i { -// let status = harness.check_tx_in_pool(*pending_txn[j].tx_hash()).await?; -// assert!(status.is_dropped()); -// } -// for j in i + 1..10 { -// let status = harness.check_tx_in_pool(*pending_txn[j].tx_hash()).await?; -// assert!(status.is_queued()); -// } -// } + let driver = rbuilder.driver().await?; + let accounts = driver.fund_accounts(10, ONE_ETH).await?; + let latest_block_number = driver.latest().await?.header.number; + + // send 10 bundles with different block ranges + let mut pending_txn = Vec::new(); + + for i in 0..accounts.len() { + let txn = driver + .transaction() + .random_reverting_transaction() + .with_signer(accounts[i].clone()) + .with_bundle(BundleOpts { + block_number_max: Some(latest_block_number + i as u64 + 1), + }) + .send() + .await?; + pending_txn.push(txn); + } -// Ok(()) -// } + rbuilder.pool().print_all(); + // generate 10 blocks + for i in 0..accounts.len() { + let block = driver.build_new_block().await?; -// /// Test the behaviour of the revert protection bundle, if the bundle **does not** revert -// /// the transaction is included in the block. If the bundle reverts, the transaction -// /// is not included in the block and tried again for the next bundle range blocks -// /// when it will be dropped from the pool. -// #[tokio::test] -// async fn revert_protection_bundle() -> eyre::Result<()> { -// let harness = TestHarnessBuilder::new("revert_protection_bundle") -// .with_revert_protection() -// .with_namespaces("eth,web3,txpool") -// .build() -// .await?; + // blocks should only include two transactions (deposit + builder) + assert_eq!(block.transactions.len(), 2); + + // since we created the 10 transactions with increasing block ranges, as we generate blocks + // one transaction will be gc on each block. + // transactions from [0, i] should be dropped, transactions from [i+1, 10] should be queued + for j in 0..=i { + assert_eq!( + rbuilder + .pool() + .tx_status(*pending_txn[j].tx_hash()) + .expect("tx not found in pool"), + TransactionEvent::Discarded + ); + } + for j in i + 1..10 { + assert_eq!( + rbuilder + .pool() + .tx_status(*pending_txn[j].tx_hash()) + .expect("tx not found in pool"), + TransactionEvent::Pending + ); + } + } -// let mut generator = harness.block_generator().await?; // Block 1 + Ok(()) +} -// // Test 1: Bundle does not revert -// let valid_bundle = harness -// .create_transaction() -// .with_bundle(BundleOpts::default()) -// .send() -// .await?; +/// Test the behaviour of the revert protection bundle, if the bundle **does not** revert +/// the transaction is included in the block. If the bundle reverts, the transaction +/// is not included in the block and tried again for the next bundle range blocks +/// when it will be dropped from the pool. +#[tokio::test] +async fn revert_protection_bundle() -> eyre::Result<()> { + let rbuilder = LocalInstance::new::(OpRbuilderArgs { + enable_revert_protection: true, + ..Default::default() + }) + .await?; -// let block_generated = generator.generate_block().await?; // Block 2 -// assert!(block_generated.includes(*valid_bundle.tx_hash())); + let driver = rbuilder.driver().await?; + let _ = driver.build_new_block().await?; // Block 1 -// let bundle_opts = BundleOpts { -// block_number_max: Some(4), -// }; + // Test 1: Bundle does not revert + let valid_bundle = driver + .transaction() + .random_valid_transfer() + .with_bundle(BundleOpts::default()) + .send() + .await?; -// let reverted_bundle = harness -// .create_transaction() -// .with_revert() -// .with_bundle(bundle_opts) -// .send() -// .await?; + let block2 = driver.build_new_block().await?; // Block 2 + assert!(block2.transactions.hashes().includes(valid_bundle.tx_hash())); -// // Test 2: Bundle reverts. It is not included in the block -// let block_generated = generator.generate_block().await?; // Block 3 -// assert!(block_generated.not_includes(*reverted_bundle.tx_hash())); - -// // After the block the transaction is still pending in the pool -// assert!(harness -// .check_tx_in_pool(*reverted_bundle.tx_hash()) -// .await? -// .is_pending()); - -// // Test 3: Chain progresses beyond the bundle range. The transaction is dropped from the pool -// generator.generate_block().await?; // Block 4 -// assert!(harness -// .check_tx_in_pool(*reverted_bundle.tx_hash()) -// .await? -// .is_pending()); - -// generator.generate_block().await?; // Block 5 -// assert!(harness -// .check_tx_in_pool(*reverted_bundle.tx_hash()) -// .await? -// .is_dropped()); + let bundle_opts = BundleOpts { + block_number_max: Some(4), + }; -// Ok(()) -// } + let reverted_bundle = driver + .transaction() + .random_reverting_transaction() + .with_bundle(bundle_opts) + .send() + .await?; + + // Test 2: Bundle reverts. It is not included in the block + let block3 = driver.build_new_block().await?; // Block 3 + assert!(!block3.includes(reverted_bundle.tx_hash())); + + // After the block the transaction is still pending in the pool + assert!(rbuilder.pool().is_pending(*reverted_bundle.tx_hash())); + + // Test 3: Chain progresses beyond the bundle range. The transaction is dropped from the pool + driver.build_new_block().await?; // Block 4 + assert!(rbuilder.pool().is_dropped(*reverted_bundle.tx_hash())); + + driver.build_new_block().await?; // Block 5 + assert!(rbuilder.pool().is_dropped(*reverted_bundle.tx_hash())); + + Ok(()) +} // /// Test the range limits for the revert protection bundle. // #[tokio::test] From 2745a217864ccc681874372c0cebb7d2547f324e Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 10:40:58 +0000 Subject: [PATCH 04/21] all revert tests are passing --- .../op-rbuilder/src/tests/framework/apis.rs | 11 +- .../src/tests/framework/instance.rs | 2 +- .../op-rbuilder/src/tests/standard/revert.rs | 180 +++++++----------- 3 files changed, 80 insertions(+), 113 deletions(-) diff --git a/crates/op-rbuilder/src/tests/framework/apis.rs b/crates/op-rbuilder/src/tests/framework/apis.rs index 35f4f64f0..dce0cd002 100644 --- a/crates/op-rbuilder/src/tests/framework/apis.rs +++ b/crates/op-rbuilder/src/tests/framework/apis.rs @@ -2,6 +2,7 @@ use super::DEFAULT_JWT_TOKEN; use alloy_eips::BlockNumberOrTag; use alloy_primitives::B256; use alloy_rpc_types_engine::{ExecutionPayloadV3, ForkchoiceUpdated, PayloadStatus}; +use tracing::debug; use core::{future::Future, marker::PhantomData}; use jsonrpsee::{ core::{client::SubscriptionClientT, RpcResult}, @@ -142,7 +143,7 @@ impl EngineApi

{ &self, payload_id: PayloadId, ) -> eyre::Result<::ExecutionPayloadEnvelopeV3> { - println!( + debug!( "Fetching payload with id: {} at {}", payload_id, chrono::Utc::now() @@ -160,7 +161,7 @@ impl EngineApi

{ versioned_hashes: Vec, parent_beacon_block_root: B256, ) -> eyre::Result { - println!("Submitting new payload at {}...", chrono::Utc::now()); + debug!("Submitting new payload at {}...", chrono::Utc::now()); Ok(EngineApiClient::::new_payload_v3( &self.client().await, payload, @@ -176,7 +177,7 @@ impl EngineApi

{ new_head: B256, payload_attributes: Option<::PayloadAttributes>, ) -> eyre::Result { - println!("Updating forkchoice at {}...", chrono::Utc::now()); + debug!("Updating forkchoice at {}...", chrono::Utc::now()); Ok(EngineApiClient::::fork_choice_updated_v3( &self.client().await, ForkchoiceState { @@ -217,9 +218,9 @@ pub async fn generate_genesis(output: Option) -> eyre::Result<()> { // Write the result to the output file if let Some(output) = output { std::fs::write(&output, serde_json::to_string_pretty(&genesis)?)?; - println!("Generated genesis file at: {output}"); + debug!("Generated genesis file at: {output}"); } else { - println!("{}", serde_json::to_string_pretty(&genesis)?); + debug!("{}", serde_json::to_string_pretty(&genesis)?); } Ok(()) diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 9866525f8..1c0c17643 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -74,7 +74,7 @@ impl LocalInstance { args.rollup_args.enable_tx_conditional = true; let builder_config = BuilderConfig::::try_from(args.clone()) .expect("Failed to convert rollup args to builder config"); - println!("Builder config: {builder_config:#?}"); + let node_builder = NodeBuilder::<_, OpChainSpec>::new(config.clone()) .testing_node(task_manager.executor()) .with_types::() diff --git a/crates/op-rbuilder/src/tests/standard/revert.rs b/crates/op-rbuilder/src/tests/standard/revert.rs index 8853ea4ee..ff2fe0a44 100644 --- a/crates/op-rbuilder/src/tests/standard/revert.rs +++ b/crates/op-rbuilder/src/tests/standard/revert.rs @@ -1,9 +1,9 @@ +use alloy_provider::PendingTransactionBuilder; +use op_alloy_network::Optimism; use reth_transaction_pool::TransactionEvent; use crate::{ - args::OpRbuilderArgs, - builders::StandardBuilder, - tests::{BlockTransactionsExt, BundleOpts, ChainDriverExt, LocalInstance, TransactionBuilderExt, ONE_ETH}, + args::OpRbuilderArgs, builders::StandardBuilder, primitives::bundle::MAX_BLOCK_RANGE_BLOCKS, tests::{BlockTransactionsExt, BundleOpts, ChainDriver, ChainDriverExt, LocalInstance, TransactionBuilderExt, ONE_ETH} }; /// If revert protection is disabled, the transactions that revert are included in the block. @@ -34,39 +34,6 @@ async fn revert_protection_disabled() -> eyre::Result<()> { Ok(()) } -/// If revert protection is enabled, the transactions that revert are not included in the block. -#[tokio::test] -#[ignore = "This test is ignored for now because the revert protection logic has changed"] -async fn revert_protection_enabled() -> eyre::Result<()> { - let rbuilder = LocalInstance::new::(OpRbuilderArgs { - enable_revert_protection: true, - ..Default::default() - }) - .await?; - let driver = rbuilder.driver().await?; - - for _ in 0..10 { - let valid_tx = driver.transaction().random_valid_transfer().send().await?; - let reverting_tx = driver - .transaction() - .random_reverting_transaction() - .send() - .await?; - let block = driver.build_new_block().await?; - - assert!(block - .transactions - .hashes() - .any(|hash| hash == *valid_tx.tx_hash())); - assert!(!block - .transactions - .hashes() - .any(|hash| hash == *reverting_tx.tx_hash())); - } - - Ok(()) -} - /// If revert protection is disabled, it should not be possible to send a revert bundle /// since the revert RPC endpoint is not available. #[tokio::test] @@ -207,74 +174,73 @@ async fn revert_protection_bundle() -> eyre::Result<()> { Ok(()) } -// /// Test the range limits for the revert protection bundle. -// #[tokio::test] -// async fn revert_protection_bundle_range_limits() -> eyre::Result<()> { -// let harness = TestHarnessBuilder::new("revert_protection_bundle_range_limits") -// .with_revert_protection() -// .build() -// .await?; - -// let mut generator = harness.block_generator().await?; - -// // Advance two blocks and try to send a bundle with max block = 1 -// generator.generate_block().await?; // Block 1 -// generator.generate_block().await?; // Block 2 - -// async fn send_bundle( -// harness: &TestHarness, -// block_number_max: u64, -// ) -> eyre::Result> { -// harness -// .create_transaction() -// .with_bundle(BundleOpts { -// block_number_max: Some(block_number_max), -// }) -// .send() -// .await -// } - -// // Max block cannot be a past block -// assert!(send_bundle(&harness, 1).await.is_err()); - -// // Bundles are valid if their max block in in between the current block and the max block range -// let next_valid_block = 3; - -// for i in next_valid_block..next_valid_block + MAX_BLOCK_RANGE_BLOCKS { -// assert!(send_bundle(&harness, i).await.is_ok()); -// } - -// // A bundle with a block out of range is invalid -// assert!( -// send_bundle(&harness, next_valid_block + MAX_BLOCK_RANGE_BLOCKS + 1) -// .await -// .is_err() -// ); - -// Ok(()) -// } - -// /// If a transaction reverts and was sent as a normal transaction through the eth_sendRawTransaction -// /// bundle, the transaction should be included in the block. -// /// This behaviour is the same as the 'revert_protection_disabled' test. -// #[tokio::test] -// async fn revert_protection_allow_reverted_transactions_without_bundle() -> eyre::Result<()> { -// let harness = -// TestHarnessBuilder::new("revert_protection_allow_reverted_transactions_without_bundle") -// .with_revert_protection() -// .build() -// .await?; - -// let mut generator = harness.block_generator().await?; - -// for _ in 0..10 { -// let valid_tx = harness.send_valid_transaction().await?; -// let reverting_tx = harness.send_revert_transaction().await?; -// let block_generated = generator.generate_block().await?; - -// assert!(block_generated.includes(*valid_tx.tx_hash())); -// assert!(block_generated.includes(*reverting_tx.tx_hash())); -// } - -// Ok(()) -// } +/// Test the range limits for the revert protection bundle. +#[tokio::test] +async fn revert_protection_bundle_range_limits() -> eyre::Result<()> { + let rbuilder = LocalInstance::new::(OpRbuilderArgs { + enable_revert_protection: true, + ..Default::default() + }) + .await?; + + let driver = rbuilder.driver().await?; + let _ = driver.build_new_block().await?; // Block 1 + let _ = driver.build_new_block().await?; // Block 2 + + async fn send_bundle( + driver: &ChainDriver, + block_number_max: u64, + ) -> eyre::Result> { + driver + .transaction() + .with_bundle(BundleOpts { + block_number_max: Some(block_number_max), + }) + .send() + .await + } + + // Max block cannot be a past block + assert!(send_bundle(&driver, 1).await.is_err()); + + // Bundles are valid if their max block in in between the current block and the max block range + let next_valid_block = 3; + + for i in next_valid_block..next_valid_block + MAX_BLOCK_RANGE_BLOCKS { + assert!(send_bundle(&driver, i).await.is_ok()); + } + + // A bundle with a block out of range is invalid + assert!( + send_bundle(&driver, next_valid_block + MAX_BLOCK_RANGE_BLOCKS + 1) + .await + .is_err() + ); + + Ok(()) +} + +/// If a transaction reverts and was sent as a normal transaction through the eth_sendRawTransaction +/// bundle, the transaction should be included in the block. +/// This behaviour is the same as the 'revert_protection_disabled' test. +#[tokio::test] +async fn revert_protection_allow_reverted_transactions_without_bundle() -> eyre::Result<()> { + let rbuilder = LocalInstance::new::(OpRbuilderArgs { + enable_revert_protection: true, + ..Default::default() + }) + .await?; + + let driver = rbuilder.driver().await?; + + for _ in 0..10 { + let valid_tx = driver.transaction().random_valid_transfer().send().await?; + let reverting_tx = driver.transaction().random_reverting_transaction().send().await?; + let block = driver.build_new_block().await?; + + assert!(block.includes(valid_tx.tx_hash())); + assert!(block.includes(reverting_tx.tx_hash())); + } + + Ok(()) +} From 18fbebf2f503e990612de37323cf7dbad971af98 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 11:02:09 +0000 Subject: [PATCH 05/21] txpool tests are passing --- .../op-rbuilder/src/tests/framework/apis.rs | 2 +- .../src/tests/framework/instance.rs | 21 +++++-- crates/op-rbuilder/src/tests/framework/txs.rs | 19 ++++++ .../op-rbuilder/src/tests/framework/utils.rs | 5 +- crates/op-rbuilder/src/tests/standard/mod.rs | 2 +- .../op-rbuilder/src/tests/standard/revert.rs | 21 +++++-- .../op-rbuilder/src/tests/standard/txpool.rs | 63 ++++++++++--------- 7 files changed, 91 insertions(+), 42 deletions(-) diff --git a/crates/op-rbuilder/src/tests/framework/apis.rs b/crates/op-rbuilder/src/tests/framework/apis.rs index dce0cd002..918f8608e 100644 --- a/crates/op-rbuilder/src/tests/framework/apis.rs +++ b/crates/op-rbuilder/src/tests/framework/apis.rs @@ -2,7 +2,6 @@ use super::DEFAULT_JWT_TOKEN; use alloy_eips::BlockNumberOrTag; use alloy_primitives::B256; use alloy_rpc_types_engine::{ExecutionPayloadV3, ForkchoiceUpdated, PayloadStatus}; -use tracing::debug; use core::{future::Future, marker::PhantomData}; use jsonrpsee::{ core::{client::SubscriptionClientT, RpcResult}, @@ -14,6 +13,7 @@ use reth_optimism_node::OpEngineTypes; use reth_payload_builder::PayloadId; use reth_rpc_layer::{AuthClientLayer, JwtSecret}; use serde_json::Value; +use tracing::debug; #[derive(Clone, Debug)] pub enum Address { diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 1c0c17643..2078c5314 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -49,13 +49,26 @@ pub struct LocalInstance { } impl LocalInstance { - /// Creates a new local instance of the OP builder node with the given arguments. + /// Creates a new local instance of the OP builder node with the given arguments, + /// with the default Reth node configuration. + /// /// This method does not prefund any accounts, so before sending any transactions /// make sure that sender accounts are funded. pub async fn new(args: OpRbuilderArgs) -> eyre::Result { + Self::new_with_config::

(args, default_node_config()).await + } + + /// Creates a new local instance of the OP builder node with the given arguments, + /// with a given Reth node configuration. + /// + /// This method does not prefund any accounts, so before sending any transactions + /// make sure that sender accounts are funded. + pub async fn new_with_config( + args: OpRbuilderArgs, + config: NodeConfig, + ) -> eyre::Result { let mut args = args; let task_manager = task_manager(); - let config = node_config(); let op_node = OpNode::new(args.rollup_args.clone()); let (rpc_ready_tx, rpc_ready_rx) = oneshot::channel::<()>(); @@ -74,7 +87,7 @@ impl LocalInstance { args.rollup_args.enable_tx_conditional = true; let builder_config = BuilderConfig::::try_from(args.clone()) .expect("Failed to convert rollup args to builder config"); - + let node_builder = NodeBuilder::<_, OpChainSpec>::new(config.clone()) .testing_node(task_manager.executor()) .with_types::() @@ -213,7 +226,7 @@ impl Future for LocalInstance { } } -fn node_config() -> NodeConfig { +pub fn default_node_config() -> NodeConfig { let tempdir = std::env::temp_dir(); let random_id = nanoid!(); diff --git a/crates/op-rbuilder/src/tests/framework/txs.rs b/crates/op-rbuilder/src/tests/framework/txs.rs index 517c76b00..123d949aa 100644 --- a/crates/op-rbuilder/src/tests/framework/txs.rs +++ b/crates/op-rbuilder/src/tests/framework/txs.rs @@ -310,6 +310,25 @@ impl TransactionPoolObserver { matches!(self.tx_status(txhash), Some(TransactionEvent::Discarded)) } + pub fn count(&self, status: TransactionEvent) -> usize { + self.observations + .iter() + .filter(|tx| tx.value().back() == Some(&status)) + .count() + } + + pub fn pending_count(&self) -> usize { + self.count(TransactionEvent::Pending) + } + + pub fn queued_count(&self) -> usize { + self.count(TransactionEvent::Queued) + } + + pub fn dropped_count(&self) -> usize { + self.count(TransactionEvent::Discarded) + } + /// Returns the history of pool events for a transaction. pub fn history(&self, txhash: TxHash) -> Option> { self.observations diff --git a/crates/op-rbuilder/src/tests/framework/utils.rs b/crates/op-rbuilder/src/tests/framework/utils.rs index 18fb2432a..6bee49327 100644 --- a/crates/op-rbuilder/src/tests/framework/utils.rs +++ b/crates/op-rbuilder/src/tests/framework/utils.rs @@ -1,9 +1,9 @@ use alloy_eips::Encodable2718; use alloy_primitives::{hex, Address, TxKind, B256, U256}; use alloy_rpc_types_eth::{Block, BlockTransactionHashes}; -use op_alloy_rpc_types::Transaction; use core::future::Future; use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; +use op_alloy_rpc_types::Transaction; use crate::{tests::ONE_ETH, tx_signer::Signer}; @@ -80,7 +80,6 @@ impl ChainDriverExt for super::ChainDriver { } } - pub trait BlockTransactionsExt { fn includes(&self, tx_hash: &B256) -> bool; } @@ -96,4 +95,4 @@ impl<'a> BlockTransactionsExt for BlockTransactionHashes<'a, Transaction> { let mut iter = self.clone(); iter.any(|tx| tx == *tx_hash) } -} \ No newline at end of file +} diff --git a/crates/op-rbuilder/src/tests/standard/mod.rs b/crates/op-rbuilder/src/tests/standard/mod.rs index d55a20380..40c980101 100644 --- a/crates/op-rbuilder/src/tests/standard/mod.rs +++ b/crates/op-rbuilder/src/tests/standard/mod.rs @@ -4,4 +4,4 @@ mod ordering; mod revert; mod smoke; -//mod txpool; +mod txpool; diff --git a/crates/op-rbuilder/src/tests/standard/revert.rs b/crates/op-rbuilder/src/tests/standard/revert.rs index ff2fe0a44..bbd9f1c92 100644 --- a/crates/op-rbuilder/src/tests/standard/revert.rs +++ b/crates/op-rbuilder/src/tests/standard/revert.rs @@ -3,7 +3,13 @@ use op_alloy_network::Optimism; use reth_transaction_pool::TransactionEvent; use crate::{ - args::OpRbuilderArgs, builders::StandardBuilder, primitives::bundle::MAX_BLOCK_RANGE_BLOCKS, tests::{BlockTransactionsExt, BundleOpts, ChainDriver, ChainDriverExt, LocalInstance, TransactionBuilderExt, ONE_ETH} + args::OpRbuilderArgs, + builders::StandardBuilder, + primitives::bundle::MAX_BLOCK_RANGE_BLOCKS, + tests::{ + BlockTransactionsExt, BundleOpts, ChainDriver, ChainDriverExt, LocalInstance, + TransactionBuilderExt, ONE_ETH, + }, }; /// If revert protection is disabled, the transactions that revert are included in the block. @@ -144,7 +150,10 @@ async fn revert_protection_bundle() -> eyre::Result<()> { .await?; let block2 = driver.build_new_block().await?; // Block 2 - assert!(block2.transactions.hashes().includes(valid_bundle.tx_hash())); + assert!(block2 + .transactions + .hashes() + .includes(valid_bundle.tx_hash())); let bundle_opts = BundleOpts { block_number_max: Some(4), @@ -170,7 +179,7 @@ async fn revert_protection_bundle() -> eyre::Result<()> { driver.build_new_block().await?; // Block 5 assert!(rbuilder.pool().is_dropped(*reverted_bundle.tx_hash())); - + Ok(()) } @@ -235,7 +244,11 @@ async fn revert_protection_allow_reverted_transactions_without_bundle() -> eyre: for _ in 0..10 { let valid_tx = driver.transaction().random_valid_transfer().send().await?; - let reverting_tx = driver.transaction().random_reverting_transaction().send().await?; + let reverting_tx = driver + .transaction() + .random_reverting_transaction() + .send() + .await?; let block = driver.build_new_block().await?; assert!(block.includes(valid_tx.tx_hash())); diff --git a/crates/op-rbuilder/src/tests/standard/txpool.rs b/crates/op-rbuilder/src/tests/standard/txpool.rs index cbf15512d..82c344c9a 100644 --- a/crates/op-rbuilder/src/tests/standard/txpool.rs +++ b/crates/op-rbuilder/src/tests/standard/txpool.rs @@ -1,46 +1,55 @@ -use crate::tests::{framework::TestHarnessBuilder, ONE_ETH}; -use alloy_provider::ext::TxPoolApi; +use crate::{builders::StandardBuilder, tests::{ + default_node_config, BlockTransactionsExt, ChainDriverExt, LocalInstance, + TransactionBuilderExt, ONE_ETH, +}}; +use reth::args::TxPoolArgs; +use reth_node_builder::NodeConfig; +use reth_optimism_chainspec::OpChainSpec; /// 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("pending_pool_limit") - .with_namespaces("txpool,eth,debug,admin") - .with_extra_params("--txpool.pending-max-count 50") - .build() - .await?; + let rbuilder = LocalInstance::new_with_config::( + Default::default(), + NodeConfig:: { + txpool: TxPoolArgs { + pending_max_count: 50, + ..Default::default() + }, + ..default_node_config() + }, + ) + .await?; - let mut generator = harness.block_generator().await?; + let driver = rbuilder.driver().await?; + let accounts = driver.fund_accounts(2, ONE_ETH).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() + let _ = driver + .transaction() + .random_valid_transfer() .with_signer(*acc_no_priority) .send() .await?; } - let pool = harness - .provider() - .expect("provider not available") - .txpool_status() - .await?; assert_eq!( - pool.pending, 50, + rbuilder.pool().pending_count(), + 50, "Pending pool must contain at max 50 txs {:?}", - pool + rbuilder.pool().pending_count() ); // Send 10 txs that should be included in the block let mut txs = Vec::new(); for _ in 0..10 { - let tx = harness - .create_transaction() + let tx = driver + .transaction() + .random_valid_transfer() .with_signer(*acc_with_priority) .with_max_priority_fee_per_gas(10) .send() @@ -48,22 +57,18 @@ async fn pending_pool_limit() -> eyre::Result<()> { txs.push(*tx.tx_hash()); } - let pool = harness - .provider() - .expect("provider not available") - .txpool_status() - .await?; assert_eq!( - pool.pending, 50, + rbuilder.pool().pending_count(), + 50, "Pending pool must contain at max 50 txs {:?}", - pool + rbuilder.pool().pending_count() ); // 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?; + let block = driver.build_new_block().await?; // Ensure that 10 extra txs got included - assert!(block.includes_vec(txs)); + assert!(txs.iter().all(|tx| block.includes(tx))); Ok(()) } From cb3b1320b24a108b75eec3fd92e4ecd362d7249f Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 11:17:31 +0000 Subject: [PATCH 06/21] All standard tests are passing --- .../src/tests/framework/instance.rs | 2 + .../src/tests/standard/data_availability.rs | 144 +++++++++--------- crates/op-rbuilder/src/tests/standard/mod.rs | 2 +- .../op-rbuilder/src/tests/standard/txpool.rs | 11 +- 4 files changed, 82 insertions(+), 77 deletions(-) diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 2078c5314..822731194 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -87,6 +87,7 @@ impl LocalInstance { args.rollup_args.enable_tx_conditional = true; let builder_config = BuilderConfig::::try_from(args.clone()) .expect("Failed to convert rollup args to builder config"); + let da_config = builder_config.da_config.clone(); let node_builder = NodeBuilder::<_, OpChainSpec>::new(config.clone()) .testing_node(task_manager.executor()) @@ -101,6 +102,7 @@ impl LocalInstance { OpAddOnsBuilder::default() .with_sequencer(op_node.args.sequencer.clone()) .with_enable_tx_conditional(op_node.args.enable_tx_conditional) + .with_da_config(da_config) .build(), ) .extend_rpc_modules(move |ctx| { diff --git a/crates/op-rbuilder/src/tests/standard/data_availability.rs b/crates/op-rbuilder/src/tests/standard/data_availability.rs index f18385c78..1ee2d5bda 100644 --- a/crates/op-rbuilder/src/tests/standard/data_availability.rs +++ b/crates/op-rbuilder/src/tests/standard/data_availability.rs @@ -1,33 +1,31 @@ -use crate::tests::framework::TestHarnessBuilder; +use crate::tests::{BlockTransactionsExt, LocalInstance, TransactionBuilderExt}; 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_namespaces("admin,eth,miner") - .build() - .await?; - - let mut generator = harness.block_generator().await?; + let rbuilder = LocalInstance::standard().await?; + let driver = rbuilder.driver().await?; // Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction - let call = harness - .provider()? + let call = rbuilder + .provider().await? .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() + let unfit_tx = driver + .transaction() + .random_valid_transfer() .with_max_priority_fee_per_gas(50) .send() .await?; - let block = generator.generate_block().await?; + let block = driver.build_new_block().await?; + // tx should not be included because we set the tx_size_limit to 1 assert!( - block.not_includes(*unfit_tx.tx_hash()), + !block.includes(unfit_tx.tx_hash()), "transaction should not be included in the block" ); @@ -38,77 +36,79 @@ async fn data_availability_tx_size_limit() -> eyre::Result<()> { /// We will set limit to 1 byte and see that the builder will not include any transactions. #[tokio::test] async fn data_availability_block_size_limit() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("data_availability_block_size_limit") - .with_namespaces("admin,eth,miner") - .build() - .await?; - - let mut generator = harness.block_generator().await?; + let rbuilder = LocalInstance::standard().await?; + let driver = rbuilder.driver().await?; // Set block da size to be small, so it won't include tx - let call = harness - .provider()? + let call = rbuilder + .provider().await? .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(*unfit_tx.tx_hash()), - "transaction should not be included in the block" - ); - - Ok(()) -} - -/// This test ensures that block will fill up to the limit. -/// Size of each transaction is 100000000 -/// We will set limit to 3 txs and see that the builder will include 3 transactions. -/// 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_namespaces("admin,eth,miner") - .build() - .await?; - - let mut generator = harness.block_generator().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() + let unfit_tx = driver + .transaction() + .random_valid_transfer() .with_max_priority_fee_per_gas(50) .send() .await?; - let unfit_tx_3 = harness.create_transaction().send().await?; + let block = driver.build_new_block().await?; - let block = generator.generate_block().await?; - // 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" - ); + // tx should not be included because we set the tx_size_limit to 1 assert!( - harness.latest_block().await.transactions.len() == 4, - "builder + deposit + 2 valid txs should be in the block" + !block.includes(unfit_tx.tx_hash()), + "transaction should not be included in the block" ); Ok(()) } + +// /// This test ensures that block will fill up to the limit. +// /// Size of each transaction is 100000000 +// /// We will set limit to 3 txs and see that the builder will include 3 transactions. +// /// 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_namespaces("admin,eth,miner") +// .build() +// .await?; + +// let mut generator = harness.block_generator().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?; +// // 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" +// ); +// assert!( +// harness.latest_block().await.transactions.len() == 4, +// "builder + deposit + 2 valid txs should be in the block" +// ); + +// Ok(()) +// } diff --git a/crates/op-rbuilder/src/tests/standard/mod.rs b/crates/op-rbuilder/src/tests/standard/mod.rs index 40c980101..5eb1364e1 100644 --- a/crates/op-rbuilder/src/tests/standard/mod.rs +++ b/crates/op-rbuilder/src/tests/standard/mod.rs @@ -1,6 +1,6 @@ #![cfg(test)] -// mod data_availability; +mod data_availability; mod ordering; mod revert; mod smoke; diff --git a/crates/op-rbuilder/src/tests/standard/txpool.rs b/crates/op-rbuilder/src/tests/standard/txpool.rs index 82c344c9a..8137a8680 100644 --- a/crates/op-rbuilder/src/tests/standard/txpool.rs +++ b/crates/op-rbuilder/src/tests/standard/txpool.rs @@ -1,7 +1,10 @@ -use crate::{builders::StandardBuilder, tests::{ - default_node_config, BlockTransactionsExt, ChainDriverExt, LocalInstance, - TransactionBuilderExt, ONE_ETH, -}}; +use crate::{ + builders::StandardBuilder, + tests::{ + default_node_config, BlockTransactionsExt, ChainDriverExt, LocalInstance, + TransactionBuilderExt, ONE_ETH, + }, +}; use reth::args::TxPoolArgs; use reth_node_builder::NodeConfig; use reth_optimism_chainspec::OpChainSpec; From a807fcb657e135c3d340ce55f9235707149eaf00 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 11:19:38 +0000 Subject: [PATCH 07/21] All standard tests are passing --- .../src/tests/standard/data_availability.rs | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/crates/op-rbuilder/src/tests/standard/data_availability.rs b/crates/op-rbuilder/src/tests/standard/data_availability.rs index 1ee2d5bda..1ac482605 100644 --- a/crates/op-rbuilder/src/tests/standard/data_availability.rs +++ b/crates/op-rbuilder/src/tests/standard/data_availability.rs @@ -63,52 +63,52 @@ async fn data_availability_block_size_limit() -> eyre::Result<()> { Ok(()) } -// /// This test ensures that block will fill up to the limit. -// /// Size of each transaction is 100000000 -// /// We will set limit to 3 txs and see that the builder will include 3 transactions. -// /// 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_namespaces("admin,eth,miner") -// .build() -// .await?; - -// let mut generator = harness.block_generator().await?; +/// This test ensures that block will fill up to the limit. +/// Size of each transaction is 100000000 +/// We will set limit to 3 txs and see that the builder will include 3 transactions. +/// 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 rbuilder = LocalInstance::standard().await?; + let driver = rbuilder.driver().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"); + // Set block big enough so it could fit 3 transactions without tx size limit + let call = rbuilder + .provider().await? + .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?; + // 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 = driver + .transaction() + .with_max_priority_fee_per_gas(50) + .send() + .await?; + let fit_tx_2 = driver + .transaction() + .with_max_priority_fee_per_gas(50) + .send() + .await?; + let unfit_tx_3 = driver + .transaction() + .random_valid_transfer() + .send() + .await?; -// let block = generator.generate_block().await?; -// // 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" -// ); -// assert!( -// harness.latest_block().await.transactions.len() == 4, -// "builder + deposit + 2 valid txs should be in the block" -// ); + let block = driver.build_new_block().await?; + // 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.includes(unfit_tx_3.tx_hash()), + "unfit tx should not be in block" + ); + assert!( + driver.latest().await?.transactions.len() == 4, + "builder + deposit + 2 valid txs should be in the block" + ); -// Ok(()) -// } + Ok(()) +} From 03475cb7b2fe2cd48c029e0bd3900450a44ad1a3 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 11:52:57 +0000 Subject: [PATCH 08/21] All tests are passing --- .../src/tests/flashblocks/smoke.rs | 32 +++++++++++-------- .../src/tests/framework/instance.rs | 6 ++++ crates/op-rbuilder/src/tests/mod.rs | 2 +- .../src/tests/standard/data_availability.rs | 15 ++++----- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs b/crates/op-rbuilder/src/tests/flashblocks/smoke.rs index 63e71cb3b..7f90cea98 100644 --- a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs +++ b/crates/op-rbuilder/src/tests/flashblocks/smoke.rs @@ -1,22 +1,28 @@ use std::sync::Arc; - use futures::StreamExt; use parking_lot::Mutex; use tokio::task::JoinHandle; use tokio_tungstenite::{connect_async, tungstenite::Message}; use tokio_util::sync::CancellationToken; - -use crate::tests::TestHarnessBuilder; +use crate::{ + args::OpRbuilderArgs, + builders::FlashblocksBuilder, + tests::{LocalInstance, TransactionBuilderExt}, + tx_signer::Signer, +}; #[tokio::test] -#[ignore = "Flashblocks tests need more work"] async fn chain_produces_blocks() -> eyre::Result<()> { - let harness = TestHarnessBuilder::new("flashbots_chain_produces_blocks") - .with_flashblocks_ws_url("ws://localhost:1239") - .with_chain_block_time(2000) - .with_flashbots_block_time(200) - .build() - .await?; + let rbuilder = LocalInstance::new::(OpRbuilderArgs { + builder_signer: Some(Signer::random()), + enable_flashblocks: true, + flashblocks_ws_url: "0.0.0.0:1239".to_string(), + chain_block_time: 2000, + flashblock_block_time: 200, + ..Default::default() + }) + .await?; + let driver = rbuilder.driver().await?; // Create a struct to hold received messages let received_messages = Arc::new(Mutex::new(Vec::new())); @@ -41,15 +47,13 @@ async fn chain_produces_blocks() -> eyre::Result<()> { } }); - let mut generator = harness.block_generator().await?; - for _ in 0..10 { for _ in 0..5 { // send a valid transaction - let _ = harness.send_valid_transaction().await?; + let _ = driver.transaction().random_valid_transfer().send().await?; } - generator.generate_block().await?; + driver.build_new_block().await?; tokio::time::sleep(std::time::Duration::from_secs(1)).await; } diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 822731194..b05d23a57 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -42,6 +42,7 @@ use super::{ pub struct LocalInstance { signer: Signer, config: NodeConfig, + args: OpRbuilderArgs, task_manager: Option, exit_future: NodeExitFuture, _node_handle: Box, @@ -143,6 +144,7 @@ impl LocalInstance { .expect("Failed to receive txpool ready signal"); Ok(Self { + args, signer, config, exit_future, @@ -174,6 +176,10 @@ impl LocalInstance { &self.config } + pub const fn args(&self) -> &OpRbuilderArgs { + &self.args + } + pub const fn signer(&self) -> &Signer { &self.signer } diff --git a/crates/op-rbuilder/src/tests/mod.rs b/crates/op-rbuilder/src/tests/mod.rs index 33bf3ae84..a5134ca81 100644 --- a/crates/op-rbuilder/src/tests/mod.rs +++ b/crates/op-rbuilder/src/tests/mod.rs @@ -2,5 +2,5 @@ mod framework; pub use framework::*; -//mod flashblocks; +mod flashblocks; mod standard; diff --git a/crates/op-rbuilder/src/tests/standard/data_availability.rs b/crates/op-rbuilder/src/tests/standard/data_availability.rs index 1ac482605..58e326607 100644 --- a/crates/op-rbuilder/src/tests/standard/data_availability.rs +++ b/crates/op-rbuilder/src/tests/standard/data_availability.rs @@ -10,7 +10,8 @@ async fn data_availability_tx_size_limit() -> eyre::Result<()> { // Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction let call = rbuilder - .provider().await? + .provider() + .await? .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (1, 0)) .await?; assert!(call, "miner_setMaxDASize should be executed successfully"); @@ -41,7 +42,8 @@ async fn data_availability_block_size_limit() -> eyre::Result<()> { // Set block da size to be small, so it won't include tx let call = rbuilder - .provider().await? + .provider() + .await? .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (0, 1)) .await?; assert!(call, "miner_setMaxDASize should be executed successfully"); @@ -74,7 +76,8 @@ async fn data_availability_block_fill() -> eyre::Result<()> { // Set block big enough so it could fit 3 transactions without tx size limit let call = rbuilder - .provider().await? + .provider() + .await? .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (0, 100000000 * 3)) .await?; assert!(call, "miner_setMaxDASize should be executed successfully"); @@ -91,11 +94,7 @@ async fn data_availability_block_fill() -> eyre::Result<()> { .with_max_priority_fee_per_gas(50) .send() .await?; - let unfit_tx_3 = driver - .transaction() - .random_valid_transfer() - .send() - .await?; + let unfit_tx_3 = driver.transaction().random_valid_transfer().send().await?; let block = driver.build_new_block().await?; // Now the first 2 txs will fit into the block From 1b77e79900be3f200ae766e830dec5462a8acde5 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 12:42:47 +0000 Subject: [PATCH 09/21] Default values in local instance use cli defaults --- .../src/tests/framework/instance.rs | 20 +++++++++--- .../src/tests/standard/data_availability.rs | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index b05d23a57..47d604389 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -1,4 +1,5 @@ use alloy_provider::{Identity, ProviderBuilder, RootProvider}; +use reth_optimism_cli::chainspec::OpChainSpecParser; use core::{ any::Any, future::Future, @@ -14,10 +15,9 @@ use std::sync::{Arc, LazyLock}; use tokio::sync::oneshot; use reth::{ - args::{DatadirArgs, NetworkArgs, RpcServerArgs}, - core::exit::NodeExitFuture, - tasks::TaskManager, + args::{DatadirArgs, NetworkArgs, RpcServerArgs}, core::exit::NodeExitFuture, tasks::TaskManager }; +use reth_optimism_cli::commands::Commands; use reth_node_builder::{NodeBuilder, NodeConfig}; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_node::{ @@ -33,6 +33,8 @@ use crate::{ tx_signer::Signer, }; +use clap::Parser; + use super::{ ChainDriver, ChainDriverExt, EngineApi, Ipc, TransactionPoolObserver, BUILDER_PRIVATE_KEY, }; @@ -157,7 +159,11 @@ impl LocalInstance { /// Creates new local instance of the OP builder node with the standard builder configuration. /// This method prefunds the default accounts with 1 ETH each. pub async fn standard() -> eyre::Result { - let instance = Self::new::(Default::default()).await?; + let args = crate::args::Cli::parse_from(["dummy", "node"]); + let Commands::Node(ref node_command) = args.command else { + unreachable!() + }; + let instance = Self::new::(node_command.ext.clone()).await?; let driver = ChainDriver::new(&instance).await?; driver.fund_default_accounts().await?; Ok(instance) @@ -166,7 +172,11 @@ impl LocalInstance { /// Creates new local instance of the OP builder node with the flashblocks builder configuration. /// This method prefunds the default accounts with 1 ETH each. pub async fn flashblocks() -> eyre::Result { - let instance = Self::new::(Default::default()).await?; + let args = crate::args::Cli::parse_from(["dummy", "node"]); + let Commands::Node(ref node_command) = args.command else { + unreachable!() + }; + let instance = Self::new::(node_command.ext.clone()).await?; let driver = ChainDriver::new(&instance).await?; driver.fund_default_accounts().await?; Ok(instance) diff --git a/crates/op-rbuilder/src/tests/standard/data_availability.rs b/crates/op-rbuilder/src/tests/standard/data_availability.rs index 58e326607..a96f849cb 100644 --- a/crates/op-rbuilder/src/tests/standard/data_availability.rs +++ b/crates/op-rbuilder/src/tests/standard/data_availability.rs @@ -33,6 +33,38 @@ async fn data_availability_tx_size_limit() -> eyre::Result<()> { Ok(()) } +/// 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_fb() -> eyre::Result<()> { + let rbuilder = LocalInstance::flashblocks().await?; + let driver = rbuilder.driver().await?; + + // Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction + let call = rbuilder + .provider() + .await? + .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (1, 0)) + .await?; + assert!(call, "miner_setMaxDASize should be executed successfully"); + + let unfit_tx = driver + .transaction() + .random_valid_transfer() + .with_max_priority_fee_per_gas(50) + .send() + .await?; + let block = driver.build_new_block().await?; + + // tx should not be included because we set the tx_size_limit to 1 + assert!( + !block.includes(unfit_tx.tx_hash()), + "transaction should not be included in the block" + ); + + Ok(()) +} + /// This test ensures that the block size limit is respected. /// We will set limit to 1 byte and see that the builder will not include any transactions. #[tokio::test] From 7bbea2179eb4b229c196bb76ec767a6d15f8eaad Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 12:49:08 +0000 Subject: [PATCH 10/21] Checkpoint before adding proc macros --- crates/op-rbuilder/src/tests/flashblocks/smoke.rs | 12 ++++++------ crates/op-rbuilder/src/tests/framework/instance.rs | 7 ++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs b/crates/op-rbuilder/src/tests/flashblocks/smoke.rs index 7f90cea98..0def5f2d1 100644 --- a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs +++ b/crates/op-rbuilder/src/tests/flashblocks/smoke.rs @@ -1,15 +1,15 @@ -use std::sync::Arc; -use futures::StreamExt; -use parking_lot::Mutex; -use tokio::task::JoinHandle; -use tokio_tungstenite::{connect_async, tungstenite::Message}; -use tokio_util::sync::CancellationToken; use crate::{ args::OpRbuilderArgs, builders::FlashblocksBuilder, tests::{LocalInstance, TransactionBuilderExt}, tx_signer::Signer, }; +use futures::StreamExt; +use parking_lot::Mutex; +use std::sync::Arc; +use tokio::task::JoinHandle; +use tokio_tungstenite::{connect_async, tungstenite::Message}; +use tokio_util::sync::CancellationToken; #[tokio::test] async fn chain_produces_blocks() -> eyre::Result<()> { diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 47d604389..5c572fb7b 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -1,5 +1,4 @@ use alloy_provider::{Identity, ProviderBuilder, RootProvider}; -use reth_optimism_cli::chainspec::OpChainSpecParser; use core::{ any::Any, future::Future, @@ -15,11 +14,13 @@ use std::sync::{Arc, LazyLock}; use tokio::sync::oneshot; use reth::{ - args::{DatadirArgs, NetworkArgs, RpcServerArgs}, core::exit::NodeExitFuture, tasks::TaskManager + args::{DatadirArgs, NetworkArgs, RpcServerArgs}, + core::exit::NodeExitFuture, + tasks::TaskManager, }; -use reth_optimism_cli::commands::Commands; use reth_node_builder::{NodeBuilder, NodeConfig}; use reth_optimism_chainspec::OpChainSpec; +use reth_optimism_cli::commands::Commands; use reth_optimism_node::{ node::{OpAddOnsBuilder, OpPoolBuilder}, OpNode, From 4615e25168f5792ae177460db0d7c3cdf6425040 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Thu, 29 May 2025 12:51:38 +0000 Subject: [PATCH 11/21] Removed old test machinery --- .../op-rbuilder/src/tests/framework/blocks.rs | 387 ------------------ .../src/tests/framework/harness.rs | 348 ---------------- crates/op-rbuilder/src/tests/framework/mod.rs | 6 - crates/op-rbuilder/src/tests/framework/op.rs | 207 +--------- .../src/tests/framework/service.rs | 111 ----- 5 files changed, 1 insertion(+), 1058 deletions(-) delete mode 100644 crates/op-rbuilder/src/tests/framework/blocks.rs delete mode 100644 crates/op-rbuilder/src/tests/framework/harness.rs delete mode 100644 crates/op-rbuilder/src/tests/framework/service.rs diff --git a/crates/op-rbuilder/src/tests/framework/blocks.rs b/crates/op-rbuilder/src/tests/framework/blocks.rs deleted file mode 100644 index e71f77e6e..000000000 --- a/crates/op-rbuilder/src/tests/framework/blocks.rs +++ /dev/null @@ -1,387 +0,0 @@ -use crate::tx_signer::Signer; -use alloy_eips::{eip2718::Encodable2718, BlockNumberOrTag}; -use alloy_primitives::{address, hex, Address, Bytes, TxKind, B256, U256}; -use alloy_rpc_types_engine::{ - ExecutionPayload, ExecutionPayloadV1, ExecutionPayloadV2, ExecutionPayloadV3, - PayloadAttributes, PayloadStatusEnum, -}; -use alloy_rpc_types_eth::Block; -use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; -use op_alloy_rpc_types_engine::OpPayloadAttributes; -use rollup_boost::{Flashblocks, FlashblocksService, OpExecutionPayloadEnvelope, Version}; - -use super::{apis::EngineApi, Http, Ipc, Protocol}; - -// L1 block info for OP mainnet block 124665056 (stored in input of tx at index 0) -// -// https://optimistic.etherscan.io/tx/0x312e290cf36df704a2217b015d6455396830b0ce678b860ebfcc30f41403d7b1 -const FJORD_DATA: &[u8] = &hex!("440a5e200000146b000f79c500000000000000040000000066d052e700000000013ad8a3000000000000000000000000000000000000000000000000000000003ef1278700000000000000000000000000000000000000000000000000000000000000012fdf87b89884a61e74b322bbcf60386f543bfae7827725efaaf0ab1de2294a590000000000000000000000006887246668a3b87f54deb3b94ba47a6f63f32985"); - -/// A system that continuously generates blocks using the engine API -pub struct BlockGenerator { - engine_api: EngineApi, - validation_api: Option>, - latest_hash: B256, - no_tx_pool: bool, - block_time_secs: u64, - timestamp: u64, - // flashblocks service - flashblocks_endpoint: Option, - flashblocks_service: Option, -} - -impl - BlockGenerator -{ - pub fn new( - engine_api: EngineApi, - validation_api: Option>, - no_tx_pool: bool, - block_time_secs: u64, - flashblocks_endpoint: Option, - ) -> Self { - Self { - engine_api, - validation_api, - latest_hash: B256::ZERO, // temporary value - no_tx_pool, - timestamp: 0, - block_time_secs, - flashblocks_endpoint, - flashblocks_service: None, - } - } - - /// Initialize the block generator by fetching the latest block - pub async fn init(&mut self) -> eyre::Result { - let latest_block = self.engine_api.latest().await?.expect("block not found"); - self.latest_hash = latest_block.header.hash; - self.timestamp = latest_block.header.timestamp; - - // Sync validation node if it exists - self.sync_validation_node().await?; - - // Initialize flashblocks service - if let Some(flashblocks_endpoint) = &self.flashblocks_endpoint { - println!("Initializing flashblocks service at {flashblocks_endpoint}"); - - self.flashblocks_service = Some(Flashblocks::run( - flashblocks_endpoint.to_string(), - "127.0.0.1:1112".to_string(), // output address for the preconfirmations from rb - )?); - } - - Ok(latest_block) - } - - /// Sync the validation node to the current state - async fn sync_validation_node(&self) -> eyre::Result<()> { - let Some(validation_api) = &self.validation_api else { - return Ok(()); // No validation node to sync - }; - - let latest_validation_block = validation_api.latest().await?.expect("block not found"); - let latest_block = self.engine_api.latest().await?.expect("block not found"); - - if latest_validation_block.header.number > latest_block.header.number { - return Err(eyre::eyre!("validation node is ahead of the builder")); - } - - if latest_validation_block.header.number < latest_block.header.number { - println!( - "validation node {} is behind the builder {}, syncing up", - latest_validation_block.header.number, latest_block.header.number - ); - - let mut latest_hash = latest_validation_block.header.hash; - - for i in (latest_validation_block.header.number + 1)..=latest_block.header.number { - println!("syncing block {i}"); - - let block = self - .engine_api - .get_block_by_number(BlockNumberOrTag::Number(i), true) - .await? - .expect("block not found"); - - if block.header.parent_hash != latest_hash { - return Err(eyre::eyre!("unexpected parent hash during sync")); - } - - let payload_request = ExecutionPayloadV3 { - payload_inner: ExecutionPayloadV2 { - payload_inner: ExecutionPayloadV1 { - parent_hash: block.header.parent_hash, - fee_recipient: block.header.beneficiary, - state_root: block.header.state_root, - receipts_root: block.header.receipts_root, - logs_bloom: block.header.logs_bloom, - prev_randao: B256::ZERO, - block_number: block.header.number, - gas_limit: block.header.gas_limit, - gas_used: block.header.gas_used, - timestamp: block.header.timestamp, - extra_data: block.header.extra_data.clone(), - base_fee_per_gas: U256::from(block.header.base_fee_per_gas.unwrap()), - block_hash: block.header.hash, - transactions: vec![], // there are no txns yet - }, - withdrawals: block.withdrawals.unwrap().to_vec(), - }, - blob_gas_used: block.header.inner.blob_gas_used.unwrap(), - excess_blob_gas: block.header.inner.excess_blob_gas.unwrap(), - }; - - let validation_status = validation_api - .new_payload(payload_request, vec![], B256::ZERO) - .await?; - - if validation_status.status != PayloadStatusEnum::Valid { - return Err(eyre::eyre!("invalid payload status during sync")); - } - - let new_chain_hash = validation_status - .latest_valid_hash - .ok_or_else(|| eyre::eyre!("missing latest valid hash"))?; - - if new_chain_hash != block.header.hash { - return Err(eyre::eyre!("hash mismatch during sync")); - } - - validation_api - .update_forkchoice(latest_hash, new_chain_hash, None) - .await?; - - latest_hash = new_chain_hash; - } - } - - Ok(()) - } - - /// Helper function to submit a payload and update chain state - pub async fn submit_payload( - &mut self, - transactions: Option>, - block_building_delay_secs: u64, - no_sleep: bool, // TODO: Change this, too many parameters we can tweak here to put as a function arguments - ) -> eyre::Result { - let timestamp = self.timestamp + self.block_time_secs; - - // Add L1 block info as the first transaction in every L2 block - // This deposit transaction contains L1 block metadata required by the L2 chain - // Currently using hardcoded data from L1 block 124665056 - // If this info is not provided, Reth cannot decode the receipt for any transaction - // in the block since it also includes this info as part of the result. - // It does not matter if the to address (4200000000000000000000000000000000000015) is - // not deployed on the L2 chain since Reth queries the block to get the info and not the contract. - let block_info_tx: Bytes = { - let deposit_tx = TxDeposit { - source_hash: B256::default(), - from: address!("DeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAd0001"), - to: TxKind::Call(address!("4200000000000000000000000000000000000015")), - mint: None, - value: U256::default(), - gas_limit: 210000, - is_system_transaction: false, - input: FJORD_DATA.into(), - }; - - // Create a temporary signer for the deposit - let signer = Signer::random(); - let signed_tx = signer.sign_tx(OpTypedTransaction::Deposit(deposit_tx))?; - signed_tx.encoded_2718().into() - }; - - let transactions = if let Some(transactions) = transactions { - // prepend the block info transaction - let mut all_transactions = vec![block_info_tx]; - all_transactions.extend(transactions.into_iter()); - all_transactions - } else { - vec![block_info_tx] - }; - - let result = self - .engine_api - .update_forkchoice( - self.latest_hash, - self.latest_hash, - Some(OpPayloadAttributes { - payload_attributes: PayloadAttributes { - withdrawals: Some(vec![]), - parent_beacon_block_root: Some(B256::ZERO), - timestamp, - prev_randao: B256::ZERO, - suggested_fee_recipient: Default::default(), - }, - transactions: Some(transactions), - no_tx_pool: Some(self.no_tx_pool), - gas_limit: Some(10000000), - eip_1559_params: None, - }), - ) - .await?; - - if result.payload_status.status != PayloadStatusEnum::Valid { - return Err(eyre::eyre!("Invalid payload status")); - } - - let payload_id = result.payload_id.unwrap(); - - // update the payload id in the flashblocks service if present - if let Some(flashblocks_service) = &self.flashblocks_service { - flashblocks_service.set_current_payload_id(payload_id).await; - } - - if !self.no_tx_pool && !no_sleep { - let sleep_time = self.block_time_secs + block_building_delay_secs; - tokio::time::sleep(tokio::time::Duration::from_secs(sleep_time)).await; - } - - let payload = if let Some(flashblocks_service) = &self.flashblocks_service { - flashblocks_service - .get_best_payload(Version::V3) - .await? - .unwrap() - } else { - OpExecutionPayloadEnvelope::V3(self.engine_api.get_payload_v3(payload_id).await?) - }; - - let execution_payload = if let ExecutionPayload::V3(execution_payload) = payload.into() { - execution_payload - } else { - return Err(eyre::eyre!("execution_payload should be V3")); - }; - - // Validate with builder node - let validation_status = self - .engine_api - .new_payload(execution_payload.clone(), vec![], B256::ZERO) - .await?; - - if validation_status.status != PayloadStatusEnum::Valid { - return Err(eyre::eyre!("Invalid validation status from builder")); - } - - // Validate with validation node if present - if let Some(validation_api) = &self.validation_api { - let validation_status = validation_api - .new_payload(execution_payload.clone(), vec![], B256::ZERO) - .await?; - - if validation_status.status != PayloadStatusEnum::Valid { - return Err(eyre::eyre!("Invalid validation status from validator")); - } - } - - let new_block_hash = execution_payload.payload_inner.payload_inner.block_hash; - - // Update forkchoice on builder - self.engine_api - .update_forkchoice(self.latest_hash, new_block_hash, None) - .await?; - - // Update forkchoice on validator if present - if let Some(validation_api) = &self.validation_api { - validation_api - .update_forkchoice(self.latest_hash, new_block_hash, None) - .await?; - } - - // Update internal state - self.latest_hash = new_block_hash; - self.timestamp = execution_payload.timestamp(); - - let block = self - .engine_api - .get_block_by_number(BlockNumberOrTag::Latest, false) - .await? - .expect("block not found"); - - assert_eq!(block.header.hash, new_block_hash); - - let generated_block = BlockGenerated { block }; - Ok(generated_block) - } - - /// Generate a single new block and return its hash - pub async fn generate_block(&mut self) -> eyre::Result { - self.submit_payload(None, 0, false).await - } - - pub async fn generate_block_with_delay(&mut self, delay: u64) -> eyre::Result { - self.submit_payload(None, delay, false).await - } - - /// Submit a deposit transaction to seed an account with ETH - pub async fn deposit(&mut self, address: Address, value: u128) -> eyre::Result { - // Create deposit transaction - let deposit_tx = TxDeposit { - source_hash: B256::default(), - from: address, // Set the sender to the address of the account to seed - to: TxKind::Create, - mint: Some(value), // Amount to deposit - value: U256::default(), - gas_limit: 210000, - is_system_transaction: false, - input: Bytes::default(), - }; - - // Create a temporary signer for the deposit - let signer = Signer::random(); - let signed_tx = signer.sign_tx(OpTypedTransaction::Deposit(deposit_tx))?; - let signed_tx_rlp = signed_tx.encoded_2718(); - - self.submit_payload(Some(vec![signed_tx_rlp.into()]), 0, false) - .await - } - - pub async fn create_funded_accounts( - &mut self, - count: usize, - amount: u128, - ) -> eyre::Result> { - let mut signers = Vec::with_capacity(count); - - for _ in 0..count { - // Create a new signer - let signer = Signer::random(); - let address = signer.address; - - // Deposit funds to the new account - self.deposit(address, amount).await?; - - signers.push(signer); - } - - Ok(signers) - } -} - -#[derive(Debug)] -pub struct BlockGenerated { - pub block: Block, -} - -impl BlockGenerated { - pub fn block_hash(&self) -> B256 { - self.block.header.hash - } - - pub fn not_includes(&self, tx_hash: B256) -> bool { - !self.includes(tx_hash) - } - - 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) -> bool { - tx_hashes.iter().all(|hash| self.includes(*hash)) - } - - pub fn not_includes_vec(&self, tx_hashes: Vec) -> bool { - tx_hashes.iter().all(|hash| self.not_includes(*hash)) - } -} diff --git a/crates/op-rbuilder/src/tests/framework/harness.rs b/crates/op-rbuilder/src/tests/framework/harness.rs deleted file mode 100644 index 70641ee5e..000000000 --- a/crates/op-rbuilder/src/tests/framework/harness.rs +++ /dev/null @@ -1,348 +0,0 @@ -use super::{ - apis::EngineApi, - blocks::BlockGenerator, - op::{OpRbuilderConfig, OpRethConfig}, - service::{self, Service, ServiceInstance}, - LocalInstance, TransactionBuilder, BUILDER_PRIVATE_KEY, -}; -use alloy_eips::BlockNumberOrTag; -use alloy_network::Network; -use alloy_primitives::{hex, B256}; -use alloy_provider::{ - ext::TxPoolApi, Identity, PendingTransactionBuilder, Provider, ProviderBuilder, RootProvider, -}; -use op_alloy_network::Optimism; -use parking_lot::Mutex; -use std::{ - collections::HashSet, net::TcpListener, path::PathBuf, sync::LazyLock, time::SystemTime, -}; -use time::{format_description, OffsetDateTime}; -use uuid::Uuid; - -pub struct TestHarnessBuilder { - name: String, - use_revert_protection: bool, - flashblocks_ws_url: Option, - chain_block_time: Option, - flashbots_block_time: Option, - namespaces: Option, - extra_params: Option, -} - -impl TestHarnessBuilder { - pub fn new(name: &str) -> Self { - Self { - name: name.to_string(), - use_revert_protection: false, - flashblocks_ws_url: None, - chain_block_time: None, - flashbots_block_time: None, - namespaces: None, - extra_params: None, - } - } - - pub fn with_revert_protection(mut self) -> Self { - self.use_revert_protection = true; - self - } - - pub fn with_flashblocks_ws_url(mut self, url: &str) -> Self { - self.flashblocks_ws_url = Some(url.to_string()); - self - } - - pub fn with_chain_block_time(mut self, block_time: u64) -> Self { - self.chain_block_time = Some(block_time); - self - } - - pub fn with_flashbots_block_time(mut self, block_time: u64) -> Self { - self.flashbots_block_time = Some(block_time); - self - } - - pub fn with_namespaces(mut self, namespaces: &str) -> Self { - self.namespaces = Some(namespaces.to_string()); - self - } - - pub fn with_extra_params(mut self, extra_params: &str) -> Self { - self.extra_params = Some(extra_params.to_string()); - self - } - - pub async fn build(self) -> eyre::Result { - let mut framework = IntegrationFramework::new(&self.name).unwrap(); - - // we are going to use the fixture genesis and copy it to each test folder - let genesis = include_str!("artifacts/genesis.json.tmpl"); - - let mut genesis_path = framework.test_dir.clone(); - genesis_path.push("genesis.json"); - std::fs::write(&genesis_path, genesis)?; - - // create the builder - 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() - .chain_config_path(genesis_path.clone()) - .data_dir(builder_data_dir) - .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_revert_protection(self.use_revert_protection) - .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); - } - - if let Some(chain_block_time) = self.chain_block_time { - op_rbuilder_config = op_rbuilder_config.with_chain_block_time(chain_block_time); - } - - if let Some(flashbots_block_time) = self.flashbots_block_time { - op_rbuilder_config = op_rbuilder_config.with_flashbots_block_time(flashbots_block_time); - } - - // create the validation reth node - - let reth_data_dir = std::env::temp_dir().join(Uuid::new_v4().to_string()); - let validator_auth_rpc_port = get_available_port(); - let reth = OpRethConfig::new() - .chain_config_path(genesis_path) - .data_dir(reth_data_dir) - .auth_rpc_port(validator_auth_rpc_port) - .network_port(get_available_port()); - - framework.start("op-reth", &reth).await.unwrap(); - - let builder = framework - .start("op-rbuilder", &op_rbuilder_config) - .await - .unwrap(); - - let builder_log_path = builder.log_path.clone(); - - Ok(TestHarness { - framework, - builder_auth_rpc_port, - builder_http_port, - validator_auth_rpc_port, - builder_log_path, - }) - } -} - -pub struct TestHarness { - framework: IntegrationFramework, - local_instance: LocalInstance, - builder_auth_rpc_port: u16, - builder_http_port: u16, - validator_auth_rpc_port: u16, - builder_log_path: PathBuf, -} - -impl TestHarness { - pub async fn send_valid_transaction( - &self, - ) -> eyre::Result> { - self.create_transaction().send().await - } - - pub async fn send_revert_transaction( - &self, - ) -> eyre::Result> { - self.create_transaction() - .with_input(hex!("60006000fd").into()) // PUSH1 0x00 PUSH1 0x00 REVERT - .send() - .await - } - - pub fn provider(&self) -> eyre::Result> { - let url = format!("http://localhost:{}", self.builder_http_port); - let provider = - ProviderBuilder::::default().connect_http(url.parse()?); - - Ok(provider) - } - - pub async fn block_generator(&self) -> eyre::Result { - let engine_api = self.local_instance.engine_api(); - let mut generator = BlockGenerator::new(engine_api, None, false, 1, None); - generator.init().await?; - - Ok(generator) - } - - pub fn create_transaction(&self) -> TransactionBuilder { - TransactionBuilder::new(self.provider().expect("provider not available")) - } - - pub async fn latest_block(&self) -> ::BlockResponse { - self.provider() - .expect("provider not available") - .get_block_by_number(BlockNumberOrTag::Latest) - .full() - .await - .expect("failed to get latest block by hash") - .expect("latest block should exist") - } - - pub async fn latest_base_fee(&self) -> u128 { - self.latest_block() - .await - .header - .base_fee_per_gas - .expect("Base fee per gas not found in the latest block header") as u128 - } - - pub const fn builder_private_key() -> &'static str { - BUILDER_PRIVATE_KEY - } - - pub async fn check_tx_in_pool(&self, tx_hash: B256) -> eyre::Result { - let pool_inspect = self - .provider() - .expect("provider not available") - .txpool_content() - .await?; - - let is_pending = pool_inspect.pending.iter().any(|pending_account_map| { - pending_account_map - .1 - .iter() - .any(|(_, tx)| tx.as_recovered().hash() == *tx_hash) - }); - if is_pending { - return Ok(TransactionStatus::Pending); - } - - let is_queued = pool_inspect.queued.iter().any(|queued_account_map| { - queued_account_map - .1 - .iter() - .any(|(_, tx)| tx.as_recovered().hash() == *tx_hash) - }); - if is_queued { - return Ok(TransactionStatus::Queued); - } - - // check that the builder emitted logs for the reverted transactions with the monitoring logic - // this will tell us whether the builder dropped the transaction - // 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 - // Note that this does not tell us the reason why the transaction was dropped. Ideally - // we should know it at this point. - // 'Transaction event received target="monitoring" tx_hash="" kind="discarded"' - let builder_logs = std::fs::read_to_string(&self.builder_log_path)?; - let txn_log = format!( - "Transaction event received target=\"monitoring\" tx_hash=\"{}\" kind=\"discarded\"", - tx_hash, - ); - if builder_logs.contains(txn_log.as_str()) { - return Ok(TransactionStatus::Dropped); - } - - Ok(TransactionStatus::NotFound) - } -} - -impl Drop for TestHarness { - fn drop(&mut self) { - for service in &mut self.framework.services { - let res = service.stop(); - if let Err(e) = res { - println!("Failed to stop service: {}", e); - } - } - } -} -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum TransactionStatus { - NotFound, - Pending, - Queued, - Dropped, -} - -impl TransactionStatus { - pub fn is_pending(&self) -> bool { - matches!(self, TransactionStatus::Pending) - } - - pub fn is_queued(&self) -> bool { - matches!(self, TransactionStatus::Queued) - } - - pub fn is_dropped(&self) -> bool { - matches!(self, TransactionStatus::Dropped) - } -} - -pub fn get_available_port() -> u16 { - static CLAIMED_PORTS: LazyLock>> = - LazyLock::new(|| Mutex::new(HashSet::new())); - loop { - let port: u16 = rand::random_range(1000..20000); - if TcpListener::bind(("127.0.0.1", port)).is_ok() && CLAIMED_PORTS.lock().insert(port) { - return port; - } - } -} - -#[derive(Debug)] -pub enum IntegrationError { - SpawnError, - BinaryNotFound, - SetupError, - LogError, - ServiceAlreadyRunning, -} - -struct IntegrationFramework { - test_dir: PathBuf, - services: Vec, -} - -impl IntegrationFramework { - pub fn new(test_name: &str) -> Result { - let dt: OffsetDateTime = SystemTime::now().into(); - let format = format_description::parse("[year]_[month]_[day]_[hour]_[minute]_[second]") - .map_err(|_| IntegrationError::SetupError)?; - - let date_format = dt - .format(&format) - .map_err(|_| IntegrationError::SetupError)?; - - let mut test_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - test_dir.push("../../integration_logs"); - test_dir.push(format!("{date_format}_{test_name}")); - - std::fs::create_dir_all(&test_dir).map_err(|_| IntegrationError::SetupError)?; - - Ok(Self { - test_dir, - services: Vec::new(), - }) - } - - pub async fn start( - &mut self, - name: &str, - config: &T, - ) -> Result<&mut ServiceInstance, service::Error> { - let service = self.create_service(name)?; - service.start_with_config(config).await?; - Ok(service) - } - - pub fn create_service(&mut self, name: &str) -> Result<&mut ServiceInstance, service::Error> { - let service = ServiceInstance::new(name.to_string(), self.test_dir.clone()); - self.services.push(service); - Ok(self.services.last_mut().unwrap()) - } -} diff --git a/crates/op-rbuilder/src/tests/framework/mod.rs b/crates/op-rbuilder/src/tests/framework/mod.rs index 377478f94..901af1ba4 100644 --- a/crates/op-rbuilder/src/tests/framework/mod.rs +++ b/crates/op-rbuilder/src/tests/framework/mod.rs @@ -1,21 +1,15 @@ mod apis; -//mod blocks; mod chain; -//mod harness; mod instance; mod op; -mod service; mod txs; mod utils; pub use apis::*; -//pub use blocks::*; pub use chain::*; -//pub use harness::*; pub use instance::*; pub use op::*; -pub use service::*; use tracing_subscriber::{filter::filter_fn, prelude::*}; pub use txs::*; pub use utils::*; diff --git a/crates/op-rbuilder/src/tests/framework/op.rs b/crates/op-rbuilder/src/tests/framework/op.rs index 3dea0513b..2f75774db 100644 --- a/crates/op-rbuilder/src/tests/framework/op.rs +++ b/crates/op-rbuilder/src/tests/framework/op.rs @@ -1,18 +1,4 @@ -use std::{ - fs::File, - future::Future, - io::{ErrorKind, Read}, - path::{Path, PathBuf}, - process::Command, -}; - -use std::time::Duration; -use tokio::time::sleep; - -use super::{ - service::{self, Service}, - DEFAULT_JWT_TOKEN, -}; +use std::path::PathBuf; #[derive(Default, Debug)] pub struct OpRbuilderConfig { @@ -97,103 +83,6 @@ impl OpRbuilderConfig { } } -impl Service for OpRbuilderConfig { - fn command(&self) -> Command { - let mut bin_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - bin_path.push("../../target/debug/op-rbuilder"); - - let mut cmd = Command::new(bin_path); - let jwt_path = get_or_create_jwt_path(self.jwt_secret_path.as_ref()); - - cmd.arg("node") - .arg("--authrpc.port") - .arg( - self.auth_rpc_port - .expect("auth_rpc_port not set") - .to_string(), - ) - .arg("--authrpc.jwtsecret") - .arg( - jwt_path - .to_str() - .expect("Failed to convert jwt_path to string"), - ) - .arg("--chain") - .arg( - self.chain_config_path - .as_ref() - .expect("chain_config_path not set"), - ) - .arg("--datadir") - .arg(self.data_dir.as_ref().expect("data_dir not set")) - .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") - .arg("-vvvv"); - - if let Some(revert_protection) = self.with_revert_protection { - if revert_protection { - cmd.arg("--builder.enable-revert-protection"); - } - } - - if let Some(builder_private_key) = &self.builder_private_key { - cmd.arg("--rollup.builder-secret-key") - .arg(builder_private_key); - } - - if let Some(http_port) = self.http_port { - cmd.arg("--http") - .arg("--http.port") - .arg(http_port.to_string()); - } - - if let Some(flashblocks_ws_url) = &self.flashblocks_ws_url { - cmd.arg("--rollup.enable-flashblocks").arg("true"); - - cmd.arg("--rollup.flashblocks-ws-url") - .arg(flashblocks_ws_url); - } - - if let Some(chain_block_time) = self.chain_block_time { - cmd.arg("--rollup.chain-block-time") - .arg(chain_block_time.to_string()); - } - - if let Some(flashbots_block_time) = self.flashbots_block_time { - cmd.arg("--rollup.flashblock-block-time") - .arg(flashbots_block_time.to_string()); - } - - if let Some(namespaces) = &self.namespaces { - cmd.arg("--http.api").arg(namespaces); - } - - if let Some(extra_params) = &self.extra_params { - cmd.args(extra_params.split_ascii_whitespace()); - } - - cmd - } - - #[allow(clippy::manual_async_fn)] - fn ready(&self, log_path: &Path) -> impl Future> + Send { - async move { - poll_logs( - log_path, - "Starting consensus engine", - Duration::from_millis(100), - Duration::from_secs(60), - ) - .await - } - } -} - #[derive(Default, Debug)] pub struct OpRethConfig { auth_rpc_port: Option, @@ -229,97 +118,3 @@ impl OpRethConfig { self } } - -impl Service for OpRethConfig { - fn command(&self) -> Command { - let bin_path = PathBuf::from("op-reth"); - - let mut cmd = Command::new(bin_path); - let jwt_path = get_or_create_jwt_path(self.jwt_secret_path.as_ref()); - - cmd.arg("node") - .arg("--authrpc.port") - .arg( - self.auth_rpc_port - .expect("auth_rpc_port not set") - .to_string(), - ) - .arg("--authrpc.jwtsecret") - .arg( - jwt_path - .to_str() - .expect("Failed to convert jwt_path to string"), - ) - .arg("--chain") - .arg( - self.chain_config_path - .as_ref() - .expect("chain_config_path not set"), - ) - .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"); - - if let Some(http_port) = self.http_port { - cmd.arg("--http") - .arg("--http.port") - .arg(http_port.to_string()); - } - - cmd - } - - #[allow(clippy::manual_async_fn)] - fn ready(&self, log_path: &Path) -> impl Future> + Send { - async move { - poll_logs( - log_path, - "Starting consensus engine", - Duration::from_millis(100), - Duration::from_secs(60), - ) - .await - } - } -} - -fn get_or_create_jwt_path(jwt_path: Option<&PathBuf>) -> PathBuf { - jwt_path.cloned().unwrap_or_else(|| { - let tmp_dir = std::env::temp_dir(); - let jwt_path = tmp_dir.join("jwt.hex"); - std::fs::write(&jwt_path, DEFAULT_JWT_TOKEN).expect("Failed to write JWT secret file"); - jwt_path - }) -} - -/// Helper function to poll logs periodically -pub async fn poll_logs( - log_path: &Path, - pattern: &str, - interval: Duration, - timeout: Duration, -) -> Result<(), service::Error> { - let start = std::time::Instant::now(); - - loop { - if start.elapsed() > timeout { - return Err(service::Error::Spawn(ErrorKind::TimedOut)); - } - - let mut file = File::open(log_path).map_err(|_| service::Error::Logs)?; - let mut contents = String::new(); - file.read_to_string(&mut contents) - .map_err(|_| service::Error::Logs)?; - - if contents.contains(pattern) { - return Ok(()); - } - - sleep(interval).await; - } -} diff --git a/crates/op-rbuilder/src/tests/framework/service.rs b/crates/op-rbuilder/src/tests/framework/service.rs deleted file mode 100644 index 6bc587c4c..000000000 --- a/crates/op-rbuilder/src/tests/framework/service.rs +++ /dev/null @@ -1,111 +0,0 @@ -use std::{ - fs::{File, OpenOptions}, - future::Future, - io::{ErrorKind, Read}, - path::{Path, PathBuf}, - process::{Child, Command}, -}; -use thiserror::Error; - -#[derive(Debug, Error)] -pub enum Error { - #[error("Binary not found")] - BinaryNotFound, - - #[error("Failed to spawn process")] - Spawn(ErrorKind), - - #[error("Failed initialize log streams")] - Logs, - - #[error("Service is already running")] - ServiceAlreadyRunning, -} - -pub struct ServiceInstance { - process: Option, - pub log_path: PathBuf, -} - -impl ServiceInstance { - pub fn new(name: String, test_dir: PathBuf) -> Self { - let log_path = test_dir.join(format!("{name}.log")); - Self { - process: None, - log_path, - } - } - - pub fn start(&mut self, command: Command) -> Result<(), Error> { - if self.process.is_some() { - return Err(Error::ServiceAlreadyRunning); - } - - let log = open_log_file(&self.log_path)?; - let stdout = log.try_clone().map_err(|_| Error::Logs)?; - let stderr = log.try_clone().map_err(|_| Error::Logs)?; - - let mut cmd = command; - cmd.stdout(stdout).stderr(stderr); - - let child = match cmd.spawn() { - Ok(child) => Ok(child), - Err(e) => match e.kind() { - ErrorKind::NotFound => Err(Error::BinaryNotFound), - e => Err(Error::Spawn(e)), - }, - }?; - - self.process = Some(child); - Ok(()) - } - - pub fn stop(&mut self) -> Result<(), Error> { - if let Some(mut process) = self.process.take() { - return process.kill().map_err(|e| Error::Spawn(e.kind())); - } - Ok(()) - } - - /// Start a service using its configuration and wait for it to be ready - pub async fn start_with_config(&mut self, config: &T) -> Result<(), Error> { - self.start(config.command())?; - config.ready(&self.log_path).await?; - Ok(()) - } - - pub async fn find_log_line(&self, pattern: &str) -> eyre::Result<()> { - let mut file = - File::open(&self.log_path).map_err(|_| eyre::eyre!("Failed to open log file"))?; - let mut contents = String::new(); - file.read_to_string(&mut contents) - .map_err(|_| eyre::eyre!("Failed to read log file"))?; - - if contents.contains(pattern) { - Ok(()) - } else { - Err(eyre::eyre!("Pattern not found in log file: {}", pattern)) - } - } -} - -pub struct IntegrationFramework; - -pub trait Service { - /// Configure and return the command to run the service - fn command(&self) -> Command; - - /// Return a future that resolves when the service is ready - fn ready(&self, log_path: &Path) -> impl Future> + Send; -} - -fn open_log_file(path: &PathBuf) -> Result { - let prefix = path.parent().unwrap(); - std::fs::create_dir_all(prefix).map_err(|_| Error::Logs)?; - - OpenOptions::new() - .append(true) - .create(true) - .open(path) - .map_err(|_| Error::Logs) -} From 402fe1963f155125a859fcccc4a1327bff9efc2b Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Fri, 30 May 2025 11:59:05 +0000 Subject: [PATCH 12/21] rb-test macro works --- Cargo.lock | 10 + crates/op-rbuilder/Cargo.toml | 10 +- crates/op-rbuilder/src/args/op.rs | 17 +- .../src/builders/flashblocks/payload.rs | 1 + .../src/tests/framework/instance.rs | 6 +- crates/op-rbuilder/src/tests/framework/mod.rs | 4 +- crates/op-rbuilder/src/tests/framework/op.rs | 120 ------------ .../op-rbuilder/src/tests/macros/Cargo.toml | 13 ++ .../op-rbuilder/src/tests/macros/src/lib.rs | 173 ++++++++++++++++++ .../src/tests/standard/data_availability.rs | 47 +---- .../src/tests/standard/ordering.rs | 5 +- .../op-rbuilder/src/tests/standard/revert.rs | 19 +- .../op-rbuilder/src/tests/standard/smoke.rs | 11 +- .../op-rbuilder/src/tests/standard/txpool.rs | 25 ++- 14 files changed, 266 insertions(+), 195 deletions(-) delete mode 100644 crates/op-rbuilder/src/tests/framework/op.rs create mode 100644 crates/op-rbuilder/src/tests/macros/Cargo.toml create mode 100644 crates/op-rbuilder/src/tests/macros/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 0c87744d3..8681517c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4967,6 +4967,15 @@ dependencies = [ "syn 2.0.100", ] +[[package]] +name = "macros" +version = "0.1.0" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.100", +] + [[package]] name = "matchers" version = "0.1.0" @@ -5592,6 +5601,7 @@ dependencies = [ "futures-util", "jsonrpsee 0.25.1", "jsonrpsee-types 0.25.1", + "macros", "metrics", "nanoid", "op-alloy-consensus", diff --git a/crates/op-rbuilder/Cargo.toml b/crates/op-rbuilder/Cargo.toml index 3fcb470c7..dad36a59f 100644 --- a/crates/op-rbuilder/Cargo.toml +++ b/crates/op-rbuilder/Cargo.toml @@ -104,6 +104,7 @@ shellexpand = "3.1" serde_yaml = { version = "0.9" } nanoid = { version = "0.4", optional = true } dashmap = { version = "6.1", optional = true } +macros = { path = "src/tests/macros", optional = true } # `msozin/flashblocks-v1.4.1` branch based on `flashblocks-rebase` rollup-boost = { git = "http://github.com/flashbots/rollup-boost", rev = "8506dfb7d84c65746f7c88d250983658438f59e8" } @@ -125,6 +126,7 @@ reth-ipc.workspace = true reth-rpc-eth-types.workspace = true ctor = "0.4.2" dashmap = "6.1" +macros = { path = "src/tests/macros" } [features] default = ["jemalloc"] @@ -147,7 +149,13 @@ min-info-logs = ["tracing/release_max_level_info"] min-debug-logs = ["tracing/release_max_level_debug"] min-trace-logs = ["tracing/release_max_level_trace"] -testing = ["nanoid", "reth-node-builder/test-utils", "reth-ipc", "dashmap"] +testing = [ + "nanoid", + "reth-node-builder/test-utils", + "reth-ipc", + "dashmap", + "macros", +] [[bin]] name = "op-rbuilder" diff --git a/crates/op-rbuilder/src/args/op.rs b/crates/op-rbuilder/src/args/op.rs index caf25cf4f..7580311c4 100644 --- a/crates/op-rbuilder/src/args/op.rs +++ b/crates/op-rbuilder/src/args/op.rs @@ -3,13 +3,14 @@ //! Copied from OptimismNode to allow easy extension. //! clap [Args](clap::Args) for optimism rollup configuration -use std::path::PathBuf; - use crate::tx_signer::Signer; +use clap::Parser; +use reth_optimism_cli::commands::Commands; use reth_optimism_node::args::RollupArgs; +use std::path::PathBuf; /// Parameters for rollup configuration -#[derive(Debug, Clone, Default, PartialEq, Eq, clap::Args)] +#[derive(Debug, Clone, PartialEq, Eq, clap::Args)] #[command(next_help_heading = "Rollup")] pub struct OpRbuilderArgs { /// Rollup configuration @@ -73,6 +74,16 @@ pub struct OpRbuilderArgs { pub playground: Option, } +impl Default for OpRbuilderArgs { + fn default() -> Self { + let args = crate::args::Cli::parse_from(["dummy", "node"]); + let Commands::Node(node_command) = args.command else { + unreachable!() + }; + node_command.ext + } +} + fn expand_path(s: &str) -> Result { shellexpand::full(s) .map_err(|e| format!("expansion error for `{s}`: {e}"))? diff --git a/crates/op-rbuilder/src/builders/flashblocks/payload.rs b/crates/op-rbuilder/src/builders/flashblocks/payload.rs index 128c9a68d..9f7eef0d2 100644 --- a/crates/op-rbuilder/src/builders/flashblocks/payload.rs +++ b/crates/op-rbuilder/src/builders/flashblocks/payload.rs @@ -235,6 +235,7 @@ where // Spawn the timer task that signals when to build a new flashblock let cancel_clone = ctx.cancel.clone(); let interval = self.config.specific.interval; + tokio::spawn(async move { let mut interval = tokio::time::interval(interval); loop { diff --git a/crates/op-rbuilder/src/tests/framework/instance.rs b/crates/op-rbuilder/src/tests/framework/instance.rs index 5c572fb7b..868553953 100644 --- a/crates/op-rbuilder/src/tests/framework/instance.rs +++ b/crates/op-rbuilder/src/tests/framework/instance.rs @@ -173,10 +173,12 @@ impl LocalInstance { /// Creates new local instance of the OP builder node with the flashblocks builder configuration. /// This method prefunds the default accounts with 1 ETH each. pub async fn flashblocks() -> eyre::Result { - let args = crate::args::Cli::parse_from(["dummy", "node"]); - let Commands::Node(ref node_command) = args.command else { + let mut args = crate::args::Cli::parse_from(["dummy", "node"]); + let Commands::Node(ref mut node_command) = args.command else { unreachable!() }; + node_command.ext.enable_flashblocks = true; + node_command.ext.flashblocks_ws_url = "0.0.0.0:0".to_string(); let instance = Self::new::(node_command.ext.clone()).await?; let driver = ChainDriver::new(&instance).await?; driver.fund_default_accounts().await?; diff --git a/crates/op-rbuilder/src/tests/framework/mod.rs b/crates/op-rbuilder/src/tests/framework/mod.rs index 901af1ba4..2ee13436a 100644 --- a/crates/op-rbuilder/src/tests/framework/mod.rs +++ b/crates/op-rbuilder/src/tests/framework/mod.rs @@ -1,7 +1,6 @@ mod apis; mod chain; mod instance; -mod op; mod txs; mod utils; @@ -9,8 +8,6 @@ pub use apis::*; pub use chain::*; pub use instance::*; -pub use op::*; -use tracing_subscriber::{filter::filter_fn, prelude::*}; pub use txs::*; pub use utils::*; @@ -29,6 +26,7 @@ pub const ONE_ETH: u128 = 1_000_000_000_000_000_000; /// It injects itself into #[ctor::ctor] fn init_tests_logging() { + use tracing_subscriber::{filter::filter_fn, prelude::*}; if let Ok(v) = std::env::var("TEST_TRACE") { let level = match v.as_str() { "false" | "off" => return, diff --git a/crates/op-rbuilder/src/tests/framework/op.rs b/crates/op-rbuilder/src/tests/framework/op.rs deleted file mode 100644 index 2f75774db..000000000 --- a/crates/op-rbuilder/src/tests/framework/op.rs +++ /dev/null @@ -1,120 +0,0 @@ -use std::path::PathBuf; - -#[derive(Default, Debug)] -pub struct OpRbuilderConfig { - auth_rpc_port: Option, - jwt_secret_path: Option, - chain_config_path: Option, - data_dir: Option, - http_port: Option, - network_port: Option, - builder_private_key: Option, - flashblocks_ws_url: Option, - chain_block_time: Option, - flashbots_block_time: Option, - with_revert_protection: Option, - namespaces: Option, - extra_params: Option, -} - -impl OpRbuilderConfig { - pub fn new() -> Self { - Self::default() - } - - pub fn auth_rpc_port(mut self, port: u16) -> Self { - self.auth_rpc_port = Some(port); - self - } - - pub fn chain_config_path>(mut self, path: P) -> Self { - self.chain_config_path = Some(path.into()); - self - } - - pub fn data_dir>(mut self, path: P) -> Self { - self.data_dir = Some(path.into()); - self - } - - pub fn network_port(mut self, port: u16) -> Self { - self.network_port = Some(port); - self - } - - pub fn http_port(mut self, port: u16) -> Self { - self.http_port = Some(port); - self - } - - pub fn with_builder_private_key(mut self, private_key: &str) -> Self { - self.builder_private_key = Some(private_key.to_string()); - self - } - - pub fn with_revert_protection(mut self, revert_protection: bool) -> Self { - self.with_revert_protection = Some(revert_protection); - self - } - - pub fn with_flashblocks_ws_url(mut self, url: &str) -> Self { - self.flashblocks_ws_url = Some(url.to_string()); - self - } - - pub fn with_chain_block_time(mut self, time: u64) -> Self { - self.chain_block_time = Some(time); - self - } - - pub fn with_flashbots_block_time(mut self, time: u64) -> Self { - self.flashbots_block_time = Some(time); - self - } - - pub fn with_namespaces(mut self, namespaces: Option) -> Self { - self.namespaces = namespaces; - self - } - - pub fn with_extra_params(mut self, extra_params: Option) -> Self { - self.extra_params = extra_params; - self - } -} - -#[derive(Default, Debug)] -pub struct OpRethConfig { - auth_rpc_port: Option, - jwt_secret_path: Option, - chain_config_path: Option, - data_dir: Option, - http_port: Option, - network_port: Option, -} - -impl OpRethConfig { - pub fn new() -> Self { - Self::default() - } - - pub fn auth_rpc_port(mut self, port: u16) -> Self { - self.auth_rpc_port = Some(port); - self - } - - pub fn chain_config_path>(mut self, path: P) -> Self { - self.chain_config_path = Some(path.into()); - self - } - - pub fn data_dir>(mut self, path: P) -> Self { - self.data_dir = Some(path.into()); - self - } - - pub fn network_port(mut self, port: u16) -> Self { - self.network_port = Some(port); - self - } -} diff --git a/crates/op-rbuilder/src/tests/macros/Cargo.toml b/crates/op-rbuilder/src/tests/macros/Cargo.toml new file mode 100644 index 000000000..eb800c50c --- /dev/null +++ b/crates/op-rbuilder/src/tests/macros/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "macros" +version = "0.1.0" +edition = "2024" +description = "Macros supporting the tests infrastructure for op-rbuilder" + +[lib] +proc-macro = true + +[dependencies] +syn = "2.0" +quote = "1.0" +proc-macro2 = "1.0" diff --git a/crates/op-rbuilder/src/tests/macros/src/lib.rs b/crates/op-rbuilder/src/tests/macros/src/lib.rs new file mode 100644 index 000000000..bcdd4095e --- /dev/null +++ b/crates/op-rbuilder/src/tests/macros/src/lib.rs @@ -0,0 +1,173 @@ +use proc_macro::TokenStream; +use quote::{quote, ToTokens}; +use syn::{parse_macro_input, punctuated::Punctuated, Expr, ItemFn, Meta, Token}; + +struct TestConfig { + standard: Option>, // None = not specified, Some(None) = default, Some(Some(expr)) = custom + flashblocks: Option>, // Same as above + args: Option, // Expression to pass to LocalInstance::new() +} + +impl syn::parse::Parse for TestConfig { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let mut config = TestConfig { + standard: None, + flashblocks: None, + args: None, + }; + + if input.is_empty() { + // No arguments provided, generate both with defaults + config.standard = Some(None); + config.flashblocks = Some(None); + return Ok(config); + } + + let args: Punctuated = input.parse_terminated(Meta::parse, Token![,])?; + + for arg in args { + match arg { + Meta::Path(path) if path.is_ident("standard") => { + config.standard = Some(None); + } + Meta::Path(path) if path.is_ident("flashblocks") => { + config.flashblocks = Some(None); + } + Meta::NameValue(nv) if nv.path.is_ident("standard") => { + config.standard = Some(Some(nv.value)); + } + Meta::NameValue(nv) if nv.path.is_ident("flashblocks") => { + config.flashblocks = Some(Some(nv.value)); + } + Meta::NameValue(nv) if nv.path.is_ident("args") => { + config.args = Some(nv.value); + } + _ => { + return Err(syn::Error::new_spanned( + arg, + "Unknown attribute. Use 'standard', 'flashblocks', or 'args'", + )); + } + } + } + + // Validate that custom expressions and args are not used together + if let Some(Some(_)) = &config.standard { + if config.args.is_some() { + return Err(syn::Error::new_spanned( + config.args.as_ref().unwrap(), + "Cannot use 'args' with custom 'standard' expression. Use either 'standard = expression' or 'args = expression', not both.", + )); + } + } + + if let Some(Some(_)) = &config.flashblocks { + if config.args.is_some() { + return Err(syn::Error::new_spanned( + config.args.as_ref().unwrap(), + "Cannot use 'args' with custom 'flashblocks' expression. Use either 'flashblocks = expression' or 'args = expression', not both.", + )); + } + } + + // If only args is specified, generate both standard and flashblocks tests + if config.standard.is_none() && config.flashblocks.is_none() && config.args.is_some() { + config.standard = Some(None); + config.flashblocks = Some(None); + } + + Ok(config) + } +} + +#[proc_macro_attribute] +pub fn rb_test(args: TokenStream, input: TokenStream) -> TokenStream { + let input_fn = parse_macro_input!(input as ItemFn); + let config = parse_macro_input!(args as TestConfig); + + validate_signature(&input_fn); + + // Create the original function without test attributes (helper function) + let mut helper_fn = input_fn.clone(); + // Remove any existing test attributes + helper_fn + .attrs + .retain(|attr| !attr.path().is_ident("test") && !attr.path().is_ident("tokio")); + + let original_name = &input_fn.sig.ident; + + let mut generated_functions = vec![quote! { #helper_fn }]; + + // Generate standard test if requested + if let Some(standard_init) = config.standard { + let standard_test_name = + syn::Ident::new(&format!("{}_standard", original_name), original_name.span()); + + let instance_init = match (standard_init, &config.args) { + (None, None) => quote! { crate::tests::LocalInstance::standard().await? }, + (None, Some(args_expr)) => { + quote! { crate::tests::LocalInstance::new::(#args_expr).await? } + } + (Some(expr), _) => quote! { #expr }, + }; + + generated_functions.push(quote! { + #[tokio::test] + async fn #standard_test_name() -> eyre::Result<()> { + let instance = #instance_init; + #original_name(instance).await + } + }); + } + + // Generate flashblocks test if requested + if let Some(flashblocks_init) = config.flashblocks { + let flashblocks_test_name = syn::Ident::new( + &format!("{}_flashblocks", original_name), + original_name.span(), + ); + + let instance_init = match (flashblocks_init, &config.args) { + (None, None) => quote! { crate::tests::LocalInstance::flashblocks().await? }, + (None, Some(args_expr)) => { + quote! { crate::tests::LocalInstance::new::(#args_expr).await? } + } + (Some(expr), _) => quote! { #expr }, + }; + + generated_functions.push(quote! { + #[tokio::test] + async fn #flashblocks_test_name() -> eyre::Result<()> { + let instance = #instance_init; + #original_name(instance).await + } + }); + } + + TokenStream::from(quote! { + #(#generated_functions)* + }) +} + +fn validate_signature(item_fn: &ItemFn) { + if item_fn.sig.asyncness.is_none() { + panic!("Function must be async."); + } + if item_fn.sig.inputs.len() != 1 { + panic!("Function must have exactly one parameter of type LocalInstance."); + } + + let output_types = item_fn + .sig + .output + .to_token_stream() + .to_string() + .replace(" ", ""); + + if output_types != "->eyre::Result<()>" { + panic!( + "Function must return Result<(), eyre::Error>. Actual: {}", + output_types + ); + } +} diff --git a/crates/op-rbuilder/src/tests/standard/data_availability.rs b/crates/op-rbuilder/src/tests/standard/data_availability.rs index a96f849cb..7a8f452bb 100644 --- a/crates/op-rbuilder/src/tests/standard/data_availability.rs +++ b/crates/op-rbuilder/src/tests/standard/data_availability.rs @@ -3,9 +3,8 @@ 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 rbuilder = LocalInstance::standard().await?; +#[macros::rb_test] +async fn data_availability_tx_size_limit(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; // Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction @@ -33,43 +32,11 @@ async fn data_availability_tx_size_limit() -> eyre::Result<()> { Ok(()) } -/// 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_fb() -> eyre::Result<()> { - let rbuilder = LocalInstance::flashblocks().await?; - let driver = rbuilder.driver().await?; - - // Set (max_tx_da_size, max_block_da_size), with this case block won't fit any transaction - let call = rbuilder - .provider() - .await? - .raw_request::<(i32, i32), bool>("miner_setMaxDASize".into(), (1, 0)) - .await?; - assert!(call, "miner_setMaxDASize should be executed successfully"); - - let unfit_tx = driver - .transaction() - .random_valid_transfer() - .with_max_priority_fee_per_gas(50) - .send() - .await?; - let block = driver.build_new_block().await?; - - // tx should not be included because we set the tx_size_limit to 1 - assert!( - !block.includes(unfit_tx.tx_hash()), - "transaction should not be included in the block" - ); - - Ok(()) -} /// This test ensures that the block 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_block_size_limit() -> eyre::Result<()> { - let rbuilder = LocalInstance::standard().await?; +#[macros::rb_test] +async fn data_availability_block_size_limit(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; // Set block da size to be small, so it won't include tx @@ -101,9 +68,8 @@ async fn data_availability_block_size_limit() -> eyre::Result<()> { /// Size of each transaction is 100000000 /// We will set limit to 3 txs and see that the builder will include 3 transactions. /// 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 rbuilder = LocalInstance::standard().await?; +#[macros::rb_test] +async fn data_availability_block_fill(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; // Set block big enough so it could fit 3 transactions without tx size limit @@ -129,6 +95,7 @@ async fn data_availability_block_fill() -> eyre::Result<()> { let unfit_tx_3 = driver.transaction().random_valid_transfer().send().await?; let block = driver.build_new_block().await?; + // 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"); diff --git a/crates/op-rbuilder/src/tests/standard/ordering.rs b/crates/op-rbuilder/src/tests/standard/ordering.rs index b244d4757..634fe2eed 100644 --- a/crates/op-rbuilder/src/tests/standard/ordering.rs +++ b/crates/op-rbuilder/src/tests/standard/ordering.rs @@ -3,9 +3,8 @@ use alloy_consensus::Transaction; use futures::{future::join_all, stream, StreamExt}; /// This test ensures that the transactions are ordered by fee priority in the block. -#[tokio::test] -async fn fee_priority_ordering() -> eyre::Result<()> { - let rbuilder = LocalInstance::standard().await?; +#[macros::rb_test(standard)] +async fn fee_priority_ordering(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; let accounts = driver.fund_accounts(10, ONE_ETH).await?; diff --git a/crates/op-rbuilder/src/tests/standard/revert.rs b/crates/op-rbuilder/src/tests/standard/revert.rs index bbd9f1c92..9f90669c6 100644 --- a/crates/op-rbuilder/src/tests/standard/revert.rs +++ b/crates/op-rbuilder/src/tests/standard/revert.rs @@ -13,9 +13,8 @@ use crate::{ }; /// If revert protection is disabled, the transactions that revert are included in the block. -#[tokio::test] -async fn revert_protection_disabled() -> eyre::Result<()> { - let rbuilder = LocalInstance::standard().await?; +#[macros::rb_test] +async fn revert_protection_disabled(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; for _ in 0..10 { @@ -63,14 +62,12 @@ async fn revert_protection_disabled_bundle_endpoint_error() -> eyre::Result<()> /// This test ensures that the transactions that get reverted and not included in the block, /// are eventually dropped from the pool once their block range is reached. /// This test creates N transactions with different block ranges. -#[tokio::test] -async fn revert_protection_monitor_transaction_gc() -> eyre::Result<()> { - let rbuilder = LocalInstance::new::(OpRbuilderArgs { - enable_revert_protection: true, - ..Default::default() - }) - .await?; - +#[macros::rb_test(args = OpRbuilderArgs { + enable_flashblocks: true, + enable_revert_protection: true, + ..Default::default() +})] +async fn revert_protection_monitor_transaction_gc(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; let accounts = driver.fund_accounts(10, ONE_ETH).await?; let latest_block_number = driver.latest().await?.header.number; diff --git a/crates/op-rbuilder/src/tests/standard/smoke.rs b/crates/op-rbuilder/src/tests/standard/smoke.rs index 0a585ddc9..85520a0f2 100644 --- a/crates/op-rbuilder/src/tests/standard/smoke.rs +++ b/crates/op-rbuilder/src/tests/standard/smoke.rs @@ -6,12 +6,10 @@ use tokio::join; use crate::tests::{ChainDriver, LocalInstance, TransactionBuilderExt}; -#[tokio::test] - /// This is a smoke test that ensures that transactions are included in blocks /// and that the block generator is functioning correctly. -async fn chain_produces_blocks() -> eyre::Result<()> { - let rbuilder = LocalInstance::standard().await?; +#[macros::rb_test] +async fn chain_produces_blocks(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = ChainDriver::new(&rbuilder).await?; const SAMPLE_SIZE: usize = 10; @@ -70,9 +68,8 @@ async fn chain_produces_blocks() -> eyre::Result<()> { /// Ensures that payloads are generated correctly even when the builder is busy /// with other requests, such as fcu or getPayload. -#[tokio::test] -async fn produces_blocks_under_load_within_deadline() -> eyre::Result<()> { - let rbuilder = LocalInstance::standard().await?; +#[macros::rb_test] +async fn produces_blocks_under_load_within_deadline(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = ChainDriver::new(&rbuilder).await?; let done = AtomicBool::new(false); diff --git a/crates/op-rbuilder/src/tests/standard/txpool.rs b/crates/op-rbuilder/src/tests/standard/txpool.rs index 8137a8680..64ef4cb56 100644 --- a/crates/op-rbuilder/src/tests/standard/txpool.rs +++ b/crates/op-rbuilder/src/tests/standard/txpool.rs @@ -1,5 +1,5 @@ use crate::{ - builders::StandardBuilder, + builders::{StandardBuilder, FlashblocksBuilder}, tests::{ default_node_config, BlockTransactionsExt, ChainDriverExt, LocalInstance, TransactionBuilderExt, ONE_ETH, @@ -8,11 +8,11 @@ use crate::{ use reth::args::TxPoolArgs; use reth_node_builder::NodeConfig; use reth_optimism_chainspec::OpChainSpec; +use crate::args::OpRbuilderArgs; /// 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 rbuilder = LocalInstance::new_with_config::( +#[macros::rb_test( + standard = LocalInstance::new_with_config::( Default::default(), NodeConfig:: { txpool: TxPoolArgs { @@ -22,8 +22,23 @@ async fn pending_pool_limit() -> eyre::Result<()> { ..default_node_config() }, ) - .await?; + .await?, + flashblocks = LocalInstance::new_with_config::( + OpRbuilderArgs { + enable_flashblocks: true, + ..Default::default() + }, + NodeConfig:: { + txpool: TxPoolArgs { + pending_max_count: 50, + ..Default::default() + }, + ..default_node_config() + }, + ).await? +)] +async fn pending_pool_limit(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; let accounts = driver.fund_accounts(2, ONE_ETH).await?; From 729c4ff053a6c39f7ae53d8e72a4b47fc45a7ae8 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Fri, 30 May 2025 12:15:01 +0000 Subject: [PATCH 13/21] flattened tests directory tree --- .../src/tests/flashblocks/smoke.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs b/crates/op-rbuilder/src/tests/flashblocks/smoke.rs index 0def5f2d1..78c958bb9 100644 --- a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs +++ b/crates/op-rbuilder/src/tests/flashblocks/smoke.rs @@ -1,6 +1,5 @@ use crate::{ args::OpRbuilderArgs, - builders::FlashblocksBuilder, tests::{LocalInstance, TransactionBuilderExt}, tx_signer::Signer, }; @@ -11,17 +10,15 @@ use tokio::task::JoinHandle; use tokio_tungstenite::{connect_async, tungstenite::Message}; use tokio_util::sync::CancellationToken; -#[tokio::test] -async fn chain_produces_blocks() -> eyre::Result<()> { - let rbuilder = LocalInstance::new::(OpRbuilderArgs { - builder_signer: Some(Signer::random()), - enable_flashblocks: true, - flashblocks_ws_url: "0.0.0.0:1239".to_string(), - chain_block_time: 2000, - flashblock_block_time: 200, - ..Default::default() - }) - .await?; +#[macros::rb_test(flashblocks, args = OpRbuilderArgs { + builder_signer: Some(Signer::random()), + enable_flashblocks: true, + flashblocks_ws_url: "0.0.0.0:1239".to_string(), + chain_block_time: 2000, + flashblock_block_time: 200, + ..Default::default() +})] +async fn chain_produces_blocks(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; // Create a struct to hold received messages @@ -59,6 +56,7 @@ async fn chain_produces_blocks() -> eyre::Result<()> { cancellation_token.cancel(); assert!(ws_handle.await.is_ok(), "WebSocket listener task failed"); + assert!( !received_messages .lock() From da0d6029dc58a3832369815d93dc3b7389991f87 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Fri, 30 May 2025 12:15:19 +0000 Subject: [PATCH 14/21] flattened tests directory tree --- .../tests/{standard => }/data_availability.rs | 0 .../{flashblocks/smoke.rs => flashblocks.rs} | 0 .../op-rbuilder/src/tests/flashblocks/mod.rs | 3 --- crates/op-rbuilder/src/tests/mod.rs | 19 ++++++++++++++++++- .../src/tests/{standard => }/ordering.rs | 0 .../src/tests/{standard => }/revert.rs | 0 .../src/tests/{standard => }/smoke.rs | 0 crates/op-rbuilder/src/tests/standard/mod.rs | 7 ------- .../src/tests/{standard => }/txpool.rs | 0 9 files changed, 18 insertions(+), 11 deletions(-) rename crates/op-rbuilder/src/tests/{standard => }/data_availability.rs (100%) rename crates/op-rbuilder/src/tests/{flashblocks/smoke.rs => flashblocks.rs} (100%) delete mode 100644 crates/op-rbuilder/src/tests/flashblocks/mod.rs rename crates/op-rbuilder/src/tests/{standard => }/ordering.rs (100%) rename crates/op-rbuilder/src/tests/{standard => }/revert.rs (100%) rename crates/op-rbuilder/src/tests/{standard => }/smoke.rs (100%) delete mode 100644 crates/op-rbuilder/src/tests/standard/mod.rs rename crates/op-rbuilder/src/tests/{standard => }/txpool.rs (100%) diff --git a/crates/op-rbuilder/src/tests/standard/data_availability.rs b/crates/op-rbuilder/src/tests/data_availability.rs similarity index 100% rename from crates/op-rbuilder/src/tests/standard/data_availability.rs rename to crates/op-rbuilder/src/tests/data_availability.rs diff --git a/crates/op-rbuilder/src/tests/flashblocks/smoke.rs b/crates/op-rbuilder/src/tests/flashblocks.rs similarity index 100% rename from crates/op-rbuilder/src/tests/flashblocks/smoke.rs rename to crates/op-rbuilder/src/tests/flashblocks.rs diff --git a/crates/op-rbuilder/src/tests/flashblocks/mod.rs b/crates/op-rbuilder/src/tests/flashblocks/mod.rs deleted file mode 100644 index 6ca676337..000000000 --- a/crates/op-rbuilder/src/tests/flashblocks/mod.rs +++ /dev/null @@ -1,3 +0,0 @@ -#![cfg(test)] - -mod smoke; diff --git a/crates/op-rbuilder/src/tests/mod.rs b/crates/op-rbuilder/src/tests/mod.rs index a5134ca81..12844a3fb 100644 --- a/crates/op-rbuilder/src/tests/mod.rs +++ b/crates/op-rbuilder/src/tests/mod.rs @@ -1,6 +1,23 @@ +#![cfg(any(test, feature = "testing"))] + // base mod framework; pub use framework::*; +#[cfg(test)] mod flashblocks; -mod standard; + +#[cfg(test)] +mod data_availability; + +#[cfg(test)] +mod ordering; + +#[cfg(test)] +mod revert; + +#[cfg(test)] +mod smoke; + +#[cfg(test)] +mod txpool; diff --git a/crates/op-rbuilder/src/tests/standard/ordering.rs b/crates/op-rbuilder/src/tests/ordering.rs similarity index 100% rename from crates/op-rbuilder/src/tests/standard/ordering.rs rename to crates/op-rbuilder/src/tests/ordering.rs diff --git a/crates/op-rbuilder/src/tests/standard/revert.rs b/crates/op-rbuilder/src/tests/revert.rs similarity index 100% rename from crates/op-rbuilder/src/tests/standard/revert.rs rename to crates/op-rbuilder/src/tests/revert.rs diff --git a/crates/op-rbuilder/src/tests/standard/smoke.rs b/crates/op-rbuilder/src/tests/smoke.rs similarity index 100% rename from crates/op-rbuilder/src/tests/standard/smoke.rs rename to crates/op-rbuilder/src/tests/smoke.rs diff --git a/crates/op-rbuilder/src/tests/standard/mod.rs b/crates/op-rbuilder/src/tests/standard/mod.rs deleted file mode 100644 index 5eb1364e1..000000000 --- a/crates/op-rbuilder/src/tests/standard/mod.rs +++ /dev/null @@ -1,7 +0,0 @@ -#![cfg(test)] - -mod data_availability; -mod ordering; -mod revert; -mod smoke; -mod txpool; diff --git a/crates/op-rbuilder/src/tests/standard/txpool.rs b/crates/op-rbuilder/src/tests/txpool.rs similarity index 100% rename from crates/op-rbuilder/src/tests/standard/txpool.rs rename to crates/op-rbuilder/src/tests/txpool.rs From 8c342fa340470b3ea3bf09c606c16cd5266cbc36 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Fri, 30 May 2025 20:40:30 +0000 Subject: [PATCH 15/21] if_flashblocks macro --- .../src/tests/data_availability.rs | 7 +++--- crates/op-rbuilder/src/tests/flashblocks.rs | 3 ++- .../op-rbuilder/src/tests/macros/src/lib.rs | 12 ++++++++++ crates/op-rbuilder/src/tests/ordering.rs | 3 ++- crates/op-rbuilder/src/tests/revert.rs | 23 ++++++++++++------- crates/op-rbuilder/src/tests/smoke.rs | 5 ++-- crates/op-rbuilder/src/tests/txpool.rs | 3 ++- 7 files changed, 40 insertions(+), 16 deletions(-) diff --git a/crates/op-rbuilder/src/tests/data_availability.rs b/crates/op-rbuilder/src/tests/data_availability.rs index 36f089d44..48c1a241e 100644 --- a/crates/op-rbuilder/src/tests/data_availability.rs +++ b/crates/op-rbuilder/src/tests/data_availability.rs @@ -1,9 +1,10 @@ use crate::tests::{BlockTransactionsExt, LocalInstance, TransactionBuilderExt}; use alloy_provider::Provider; +use macros::*; /// 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. -#[macros::rb_test] +#[rb_test] async fn data_availability_tx_size_limit(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; @@ -34,7 +35,7 @@ async fn data_availability_tx_size_limit(rbuilder: LocalInstance) -> eyre::Resul /// This test ensures that the block size limit is respected. /// We will set limit to 1 byte and see that the builder will not include any transactions. -#[macros::rb_test] +#[rb_test] async fn data_availability_block_size_limit(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; @@ -67,7 +68,7 @@ async fn data_availability_block_size_limit(rbuilder: LocalInstance) -> eyre::Re /// Size of each transaction is 100000000 /// We will set limit to 3 txs and see that the builder will include 3 transactions. /// We should not forget about builder transaction so we will spawn only 2 regular txs. -#[macros::rb_test] +#[rb_test] async fn data_availability_block_fill(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; diff --git a/crates/op-rbuilder/src/tests/flashblocks.rs b/crates/op-rbuilder/src/tests/flashblocks.rs index 8aea9214d..4e9ff3b5c 100644 --- a/crates/op-rbuilder/src/tests/flashblocks.rs +++ b/crates/op-rbuilder/src/tests/flashblocks.rs @@ -4,13 +4,14 @@ use crate::{ tx_signer::Signer, }; use futures::StreamExt; +use macros::*; use parking_lot::Mutex; use std::sync::Arc; use tokio::task::JoinHandle; use tokio_tungstenite::{connect_async, tungstenite::Message}; use tokio_util::sync::CancellationToken; -#[macros::rb_test(flashblocks, args = OpRbuilderArgs { +#[rb_test(flashblocks, args = OpRbuilderArgs { builder_signer: Some(Signer::random()), chain_block_time: 2000, ..Default::default() diff --git a/crates/op-rbuilder/src/tests/macros/src/lib.rs b/crates/op-rbuilder/src/tests/macros/src/lib.rs index de409bc8c..02b45fc56 100644 --- a/crates/op-rbuilder/src/tests/macros/src/lib.rs +++ b/crates/op-rbuilder/src/tests/macros/src/lib.rs @@ -212,3 +212,15 @@ fn validate_signature(item_fn: &ItemFn) { ); } } +// in cargo tests threads are named after the test function that +// is running, so we can check if the current thread is a flashblocks test +#[proc_macro] +pub fn if_flashblocks(input: TokenStream) -> TokenStream { + let input = proc_macro2::TokenStream::from(input); + + TokenStream::from(quote! { + if std::thread::current().name().unwrap_or("").ends_with("_flashblocks") { + #input + } + }) +} diff --git a/crates/op-rbuilder/src/tests/ordering.rs b/crates/op-rbuilder/src/tests/ordering.rs index 634fe2eed..1f3d2a123 100644 --- a/crates/op-rbuilder/src/tests/ordering.rs +++ b/crates/op-rbuilder/src/tests/ordering.rs @@ -1,9 +1,10 @@ use crate::tests::{ChainDriverExt, LocalInstance, ONE_ETH}; use alloy_consensus::Transaction; use futures::{future::join_all, stream, StreamExt}; +use macros::*; /// This test ensures that the transactions are ordered by fee priority in the block. -#[macros::rb_test(standard)] +#[rb_test(standard)] async fn fee_priority_ordering(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; let accounts = driver.fund_accounts(10, ONE_ETH).await?; diff --git a/crates/op-rbuilder/src/tests/revert.rs b/crates/op-rbuilder/src/tests/revert.rs index ad9cef6e3..460c473f2 100644 --- a/crates/op-rbuilder/src/tests/revert.rs +++ b/crates/op-rbuilder/src/tests/revert.rs @@ -1,4 +1,6 @@ use alloy_provider::{PendingTransactionBuilder, Provider}; +use macros::if_flashblocks; +use macros::*; use op_alloy_network::Optimism; use reth_transaction_pool::TransactionEvent; @@ -12,7 +14,7 @@ use crate::{ }; /// If revert protection is disabled, the transactions that revert are included in the block. -#[macros::rb_test] +#[rb_test] async fn revert_protection_disabled(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; @@ -40,7 +42,7 @@ async fn revert_protection_disabled(rbuilder: LocalInstance) -> eyre::Result<()> /// If revert protection is disabled, it should not be possible to send a revert bundle /// since the revert RPC endpoint is not available. -#[macros::rb_test] +#[rb_test] async fn revert_protection_disabled_bundle_endpoint_error( rbuilder: LocalInstance, ) -> eyre::Result<()> { @@ -56,13 +58,18 @@ async fn revert_protection_disabled_bundle_endpoint_error( res.is_err(), "Expected error because method is not available" ); + + if_flashblocks! { + println!("I am in flashblocks mode"); + } + Ok(()) } /// This test ensures that the transactions that get reverted and not included in the block, /// are eventually dropped from the pool once their block range is reached. /// This test creates N transactions with different block ranges. -#[macros::rb_test(args = OpRbuilderArgs { +#[rb_test(args = OpRbuilderArgs { enable_revert_protection: true, ..Default::default() })] @@ -126,7 +133,7 @@ async fn revert_protection_monitor_transaction_gc(rbuilder: LocalInstance) -> ey /// the transaction is included in the block. If the bundle reverts, the transaction /// is not included in the block and tried again for the next bundle range blocks /// when it will be dropped from the pool. -#[macros::rb_test(args = OpRbuilderArgs { +#[rb_test(args = OpRbuilderArgs { enable_revert_protection: true, ..Default::default() })] @@ -177,7 +184,7 @@ async fn revert_protection_bundle(rbuilder: LocalInstance) -> eyre::Result<()> { } /// Test the range limits for the revert protection bundle. -#[macros::rb_test(args = OpRbuilderArgs { +#[rb_test(args = OpRbuilderArgs { enable_revert_protection: true, ..Default::default() })] @@ -222,7 +229,7 @@ async fn revert_protection_bundle_range_limits(rbuilder: LocalInstance) -> eyre: /// If a transaction reverts and was sent as a normal transaction through the eth_sendRawTransaction /// bundle, the transaction should be included in the block. /// This behaviour is the same as the 'revert_protection_disabled' test. -#[macros::rb_test(args = OpRbuilderArgs { +#[rb_test(args = OpRbuilderArgs { enable_revert_protection: true, ..Default::default() })] @@ -249,7 +256,7 @@ async fn revert_protection_allow_reverted_transactions_without_bundle( /// If a transaction reverts and gets dropped it, the eth_getTransactionReceipt should return /// an error message that it was dropped. -#[macros::rb_test(args = OpRbuilderArgs { +#[rb_test(args = OpRbuilderArgs { enable_revert_protection: true, ..OpRbuilderArgs::test_default() })] @@ -280,7 +287,7 @@ async fn revert_protection_check_transaction_receipt_status_message( // Dropped let _ = driver.build_new_block().await?; let receipt = provider.get_transaction_receipt(*tx_hash).await; - + assert!(receipt.is_err()); Ok(()) diff --git a/crates/op-rbuilder/src/tests/smoke.rs b/crates/op-rbuilder/src/tests/smoke.rs index 85520a0f2..fa0698c2c 100644 --- a/crates/op-rbuilder/src/tests/smoke.rs +++ b/crates/op-rbuilder/src/tests/smoke.rs @@ -2,13 +2,14 @@ use core::sync::atomic::AtomicBool; use std::collections::HashSet; use alloy_primitives::TxHash; +use macros::*; use tokio::join; use crate::tests::{ChainDriver, LocalInstance, TransactionBuilderExt}; /// This is a smoke test that ensures that transactions are included in blocks /// and that the block generator is functioning correctly. -#[macros::rb_test] +#[rb_test] async fn chain_produces_blocks(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = ChainDriver::new(&rbuilder).await?; @@ -68,7 +69,7 @@ async fn chain_produces_blocks(rbuilder: LocalInstance) -> eyre::Result<()> { /// Ensures that payloads are generated correctly even when the builder is busy /// with other requests, such as fcu or getPayload. -#[macros::rb_test] +#[rb_test] async fn produces_blocks_under_load_within_deadline(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = ChainDriver::new(&rbuilder).await?; diff --git a/crates/op-rbuilder/src/tests/txpool.rs b/crates/op-rbuilder/src/tests/txpool.rs index b5b6b2c00..92ac7dfae 100644 --- a/crates/op-rbuilder/src/tests/txpool.rs +++ b/crates/op-rbuilder/src/tests/txpool.rs @@ -2,12 +2,13 @@ use crate::tests::{ default_node_config, BlockTransactionsExt, ChainDriverExt, LocalInstance, TransactionBuilderExt, ONE_ETH, }; +use macros::*; use reth::args::TxPoolArgs; use reth_node_builder::NodeConfig; use reth_optimism_chainspec::OpChainSpec; /// This test ensures that pending pool custom limit is respected and priority tx would be included even when pool if full. -#[macros::rb_test( +#[rb_test( config = NodeConfig:: { txpool: TxPoolArgs { pending_max_count: 50, From 13605abf076bf34152a0f1fe144e05c15b87dd9a Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Tue, 3 Jun 2025 13:47:16 +0000 Subject: [PATCH 16/21] Giving the block builder time to build a block between FCU and GetPayload --- crates/op-rbuilder/src/tests/framework/chain.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/op-rbuilder/src/tests/framework/chain.rs b/crates/op-rbuilder/src/tests/framework/chain.rs index 9f0e04668..2308a8929 100644 --- a/crates/op-rbuilder/src/tests/framework/chain.rs +++ b/crates/op-rbuilder/src/tests/framework/chain.rs @@ -12,7 +12,7 @@ use op_alloy_rpc_types::Transaction; use reth_optimism_node::OpPayloadAttributes; use rollup_boost::OpExecutionPayloadEnvelope; -use crate::tx_signer::Signer; +use crate::{args::OpRbuilderArgs, tx_signer::Signer}; use super::{EngineApi, Ipc, LocalInstance, TransactionBuilder}; @@ -26,6 +26,7 @@ pub struct ChainDriver { provider: RootProvider, signer: Option, gas_limit: Option, + args: OpRbuilderArgs, } // instantiation and configuration @@ -38,6 +39,7 @@ impl ChainDriver { provider: instance.provider().await?, signer: Default::default(), gas_limit: None, + args: instance.args().clone(), }) } @@ -124,6 +126,9 @@ impl ChainDriver { .payload_id .ok_or_else(|| eyre::eyre!("Forkchoice update did not return a payload ID"))?; + // give the builder some time to build the block + tokio::time::sleep(Duration::from_millis(self.args.chain_block_time)).await; + let payload = OpExecutionPayloadEnvelope::V4(self.engine_api.get_payload(payload_id).await?); let OpExecutionPayloadEnvelope::V4(payload) = payload else { From 2654a3fc5dc3a46222133c5b22a56c2dd492fd22 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Tue, 3 Jun 2025 13:55:34 +0000 Subject: [PATCH 17/21] merge conflicts resolved --- crates/op-rbuilder/src/args/mod.rs | 6 --- crates/op-rbuilder/src/main.rs | 66 +----------------------------- crates/op-rbuilder/src/metrics.rs | 34 +++++++++++++++ 3 files changed, 35 insertions(+), 71 deletions(-) diff --git a/crates/op-rbuilder/src/args/mod.rs b/crates/op-rbuilder/src/args/mod.rs index 293ca9794..3d30296f9 100644 --- a/crates/op-rbuilder/src/args/mod.rs +++ b/crates/op-rbuilder/src/args/mod.rs @@ -1,14 +1,8 @@ -<<<<<<< HEAD -use crate::builders::BuilderMode; -use clap::Parser; -======= use crate::{ builders::BuilderMode, metrics::{CARGO_PKG_VERSION, VERGEN_GIT_SHA}, }; use clap_builder::{CommandFactory, FromArgMatches}; -pub use op::OpRbuilderArgs; ->>>>>>> 0df5b0873925773662576f96218b17b35a8f7063 use playground::PlaygroundOptions; use reth_optimism_cli::{chainspec::OpChainSpecParser, commands::Commands}; diff --git a/crates/op-rbuilder/src/main.rs b/crates/op-rbuilder/src/main.rs index 4cf4c1520..488ad019d 100644 --- a/crates/op-rbuilder/src/main.rs +++ b/crates/op-rbuilder/src/main.rs @@ -114,7 +114,7 @@ where Ok(()) }) .on_node_started(move |ctx| { - VersionInfo::from_env().register_version_metrics(); + VERSION.register_version_metrics(); if builder_args.log_pool_transactions { tracing::info!("Logging pool transactions"); ctx.task_executor.spawn_critical( @@ -138,67 +138,3 @@ where }) .unwrap(); } - -/// Contains version information for the application. -#[derive(Debug, Clone)] -pub struct VersionInfo { - /// The version of the application. - pub version: &'static str, - /// The build timestamp of the application. - pub build_timestamp: &'static str, - /// The cargo features enabled for the build. - pub cargo_features: &'static str, - /// The Git SHA of the build. - pub git_sha: &'static str, - /// The target triple for the build. - pub target_triple: &'static str, - /// The build profile (e.g., debug or release). - pub build_profile: &'static str, -} - -impl VersionInfo { - pub const fn from_env() -> Self { - Self { - // The latest version from Cargo.toml. - version: env!("CARGO_PKG_VERSION"), - - // The build timestamp. - build_timestamp: env!("VERGEN_BUILD_TIMESTAMP"), - - // The build features. - cargo_features: env!("VERGEN_CARGO_FEATURES"), - - // The 8 character short SHA of the latest commit. - git_sha: env!("VERGEN_GIT_SHA"), - - // The target triple. - target_triple: env!("VERGEN_CARGO_TARGET_TRIPLE"), - - // The build profile name. - build_profile: env!("OP_RBUILDER_BUILD_PROFILE"), - } - } -} - -impl Default for VersionInfo { - fn default() -> Self { - Self::from_env() - } -} - -impl VersionInfo { - /// This exposes reth's version information over prometheus. - pub fn register_version_metrics(&self) { - let labels: [(&str, &str); 6] = [ - ("version", self.version), - ("build_timestamp", self.build_timestamp), - ("cargo_features", self.cargo_features), - ("git_sha", self.git_sha), - ("target_triple", self.target_triple), - ("build_profile", self.build_profile), - ]; - - let gauge = ::metrics::gauge!("builder_info", &labels); - gauge.set(1); - } -} diff --git a/crates/op-rbuilder/src/metrics.rs b/crates/op-rbuilder/src/metrics.rs index 0c3c00c07..f39787cd5 100644 --- a/crates/op-rbuilder/src/metrics.rs +++ b/crates/op-rbuilder/src/metrics.rs @@ -77,3 +77,37 @@ pub struct OpRBuilderMetrics { /// Da tx size limit pub da_tx_size_limit: Gauge, } + +/// Contains version information for the application. +#[derive(Debug, Clone)] +pub struct VersionInfo { + /// The version of the application. + pub version: &'static str, + /// The build timestamp of the application. + pub build_timestamp: &'static str, + /// The cargo features enabled for the build. + pub cargo_features: &'static str, + /// The Git SHA of the build. + pub git_sha: &'static str, + /// The target triple for the build. + pub target_triple: &'static str, + /// The build profile (e.g., debug or release). + pub build_profile: &'static str, +} + +impl VersionInfo { + /// This exposes reth's version information over prometheus. + pub fn register_version_metrics(&self) { + let labels: [(&str, &str); 6] = [ + ("version", self.version), + ("build_timestamp", self.build_timestamp), + ("cargo_features", self.cargo_features), + ("git_sha", self.git_sha), + ("target_triple", self.target_triple), + ("build_profile", self.build_profile), + ]; + + let gauge = gauge!("builder_info", &labels); + gauge.set(1); + } +} From fb4a7c9749fb70a31873ebf142ce76c18b9502f7 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Tue, 3 Jun 2025 14:01:08 +0000 Subject: [PATCH 18/21] Most tests passing --- crates/op-rbuilder/src/tests/smoke.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/op-rbuilder/src/tests/smoke.rs b/crates/op-rbuilder/src/tests/smoke.rs index fa0698c2c..984542321 100644 --- a/crates/op-rbuilder/src/tests/smoke.rs +++ b/crates/op-rbuilder/src/tests/smoke.rs @@ -95,7 +95,7 @@ async fn produces_blocks_under_load_within_deadline(rbuilder: LocalInstance) -> // Ensure that the builder can still produce blocks under // heavy load of incoming transactions. let _ = tokio::time::timeout( - std::time::Duration::from_secs(1), + std::time::Duration::from_secs(rbuilder.args().chain_block_time + 1), driver.build_new_block(), ) .await From 44939ef1f42247e0294205da60ca4bc8d28c2fbc Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Tue, 3 Jun 2025 14:13:20 +0000 Subject: [PATCH 19/21] all tests are passing except 1 --- crates/op-rbuilder/src/tests/flashblocks.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/op-rbuilder/src/tests/flashblocks.rs b/crates/op-rbuilder/src/tests/flashblocks.rs index 4e9ff3b5c..46e79e410 100644 --- a/crates/op-rbuilder/src/tests/flashblocks.rs +++ b/crates/op-rbuilder/src/tests/flashblocks.rs @@ -12,7 +12,6 @@ use tokio_tungstenite::{connect_async, tungstenite::Message}; use tokio_util::sync::CancellationToken; #[rb_test(flashblocks, args = OpRbuilderArgs { - builder_signer: Some(Signer::random()), chain_block_time: 2000, ..Default::default() })] @@ -23,11 +22,12 @@ async fn chain_produces_blocks(rbuilder: LocalInstance) -> eyre::Result<()> { let received_messages = Arc::new(Mutex::new(Vec::new())); let messages_clone = received_messages.clone(); let cancellation_token = CancellationToken::new(); + let ws_url = rbuilder.flashblocks_ws_url(); // Spawn WebSocket listener task let cancellation_token_clone = cancellation_token.clone(); let ws_handle: JoinHandle> = tokio::spawn(async move { - let (ws_stream, _) = connect_async(rbuilder.flashblocks_ws_url()).await?; + let (ws_stream, _) = connect_async(ws_url).await?; let (_, mut read) = ws_stream.split(); loop { @@ -49,6 +49,7 @@ async fn chain_produces_blocks(rbuilder: LocalInstance) -> eyre::Result<()> { } let block = driver.build_new_block().await?; + println!("Block built with hash: {block:#?}"); assert_eq!(block.transactions.len(), 7); // 5 normal txn + deposit + builder txn tokio::time::sleep(std::time::Duration::from_secs(1)).await; From 3a53fc96f581e3d493e9f333958e305211d20ffb Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Tue, 3 Jun 2025 14:17:38 +0000 Subject: [PATCH 20/21] all tests are passing --- crates/op-rbuilder/src/tests/data_availability.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/op-rbuilder/src/tests/data_availability.rs b/crates/op-rbuilder/src/tests/data_availability.rs index 1d640c9a5..10cf8a775 100644 --- a/crates/op-rbuilder/src/tests/data_availability.rs +++ b/crates/op-rbuilder/src/tests/data_availability.rs @@ -68,7 +68,7 @@ async fn data_availability_block_size_limit(rbuilder: LocalInstance) -> eyre::Re /// Size of each transaction is 100000000 /// We will set limit to 3 txs and see that the builder will include 3 transactions. /// We should not forget about builder transaction so we will spawn only 2 regular txs. -#[rb_test] +#[rb_test(standard)] async fn data_availability_block_fill(rbuilder: LocalInstance) -> eyre::Result<()> { let driver = rbuilder.driver().await?; From b1501fac9b6d872f5da8dfed9cbd508f3f298519 Mon Sep 17 00:00:00 2001 From: Karim Agha Date: Tue, 3 Jun 2025 14:28:27 +0000 Subject: [PATCH 21/21] lint --- crates/op-rbuilder/src/metrics.rs | 2 ++ crates/op-rbuilder/src/tests/revert.rs | 5 ----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/op-rbuilder/src/metrics.rs b/crates/op-rbuilder/src/metrics.rs index f39787cd5..37a6deb58 100644 --- a/crates/op-rbuilder/src/metrics.rs +++ b/crates/op-rbuilder/src/metrics.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] + use reth_metrics::{ metrics::{gauge, Counter, Gauge, Histogram}, Metrics, diff --git a/crates/op-rbuilder/src/tests/revert.rs b/crates/op-rbuilder/src/tests/revert.rs index 460c473f2..898ae2ab2 100644 --- a/crates/op-rbuilder/src/tests/revert.rs +++ b/crates/op-rbuilder/src/tests/revert.rs @@ -1,5 +1,4 @@ use alloy_provider::{PendingTransactionBuilder, Provider}; -use macros::if_flashblocks; use macros::*; use op_alloy_network::Optimism; use reth_transaction_pool::TransactionEvent; @@ -59,10 +58,6 @@ async fn revert_protection_disabled_bundle_endpoint_error( "Expected error because method is not available" ); - if_flashblocks! { - println!("I am in flashblocks mode"); - } - Ok(()) }