From 06847fb6bc3388b93a38f9f982255e8b6982eade Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Thu, 1 May 2025 10:45:48 +0100 Subject: [PATCH 1/3] Finish block building process even when cancel request is foud --- Cargo.lock | 1 + crates/op-rbuilder/Cargo.toml | 1 + .../src/integration/integration_test.rs | 28 +++- crates/op-rbuilder/src/integration/mod.rs | 137 ++++++++++++++++++ .../src/payload_builder_vanilla.rs | 19 +-- crates/op-rbuilder/src/tester/main.rs | 2 +- crates/op-rbuilder/src/tester/mod.rs | 49 +++++-- 7 files changed, 203 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f40bff4f..1c7502694 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7469,6 +7469,7 @@ dependencies = [ "op-alloy-network", "op-alloy-rpc-types-engine", "op-revm", + "parking_lot", "rand 0.9.0", "reth", "reth-basic-payload-builder", diff --git a/crates/op-rbuilder/Cargo.toml b/crates/op-rbuilder/Cargo.toml index 2159c04ea..413fb3585 100644 --- a/crates/op-rbuilder/Cargo.toml +++ b/crates/op-rbuilder/Cargo.toml @@ -80,6 +80,7 @@ metrics.workspace = true serde_json.workspace = true tokio-util.workspace = true thiserror.workspace = true +parking_lot.workspace = true time = { version = "0.3.36", features = ["macros", "formatting", "parsing"] } chrono = "0.4" diff --git a/crates/op-rbuilder/src/integration/integration_test.rs b/crates/op-rbuilder/src/integration/integration_test.rs index 187d49a21..dafc86175 100644 --- a/crates/op-rbuilder/src/integration/integration_test.rs +++ b/crates/op-rbuilder/src/integration/integration_test.rs @@ -1,5 +1,6 @@ #[cfg(all(test, feature = "integration"))] mod tests { + use crate::integration::TestHarness; use crate::{ integration::{op_rbuilder::OpRbuilderConfig, op_reth::OpRethConfig, IntegrationFramework}, tester::{BlockGenerator, EngineApi}, @@ -9,8 +10,6 @@ mod tests { use alloy_eips::{eip1559::MIN_PROTOCOL_BASE_FEE, eip2718::Encodable2718}; use alloy_primitives::hex; use alloy_provider::{Identity, Provider, ProviderBuilder}; - use alloy_rpc_types_eth::BlockTransactionsKind; - use futures_util::StreamExt; use op_alloy_consensus::OpTypedTransaction; use op_alloy_network::Optimism; use std::{ @@ -66,7 +65,7 @@ mod tests { let engine_api = EngineApi::new("http://localhost:1234").unwrap(); let validation_api = EngineApi::new("http://localhost:1236").unwrap(); - let mut generator = BlockGenerator::new(&engine_api, Some(&validation_api), false, 1, None); + let mut generator = BlockGenerator::new(engine_api, Some(validation_api), false, 1, None); generator.init().await?; let provider = ProviderBuilder::::default() @@ -138,7 +137,7 @@ mod tests { let engine_api = EngineApi::new("http://localhost:1244").unwrap(); let validation_api = EngineApi::new("http://localhost:1246").unwrap(); - let mut generator = BlockGenerator::new(&engine_api, Some(&validation_api), false, 1, None); + let mut generator = BlockGenerator::new(engine_api, Some(validation_api), false, 1, None); let latest_block = generator.init().await?; let provider = ProviderBuilder::::default() @@ -263,7 +262,7 @@ mod tests { let engine_api = EngineApi::new("http://localhost:1264").unwrap(); let validation_api = EngineApi::new("http://localhost:1266").unwrap(); - let mut generator = BlockGenerator::new(&engine_api, Some(&validation_api), false, 1, None); + let mut generator = BlockGenerator::new(engine_api, Some(validation_api), false, 1, None); let latest_block = generator.init().await?; let provider = ProviderBuilder::::default() @@ -349,6 +348,25 @@ mod tests { Ok(()) } + #[tokio::test] + #[cfg(not(feature = "flashblocks"))] + async fn integration_test_get_payload_close_to_fcu() -> eyre::Result<()> { + let test_harness = TestHarness::new("integration_test_get_payload_close_to_fcu").await; + let mut block_generator = test_harness.block_generator().await?; + + // add some transactions to the pool so that the builder is busy when we send the fcu/getPayload requests + for _ in 0..10 { + // Note, for this test it is okay if they are not valid + test_harness.send_valid_transaction().await?; + } + + // TODO: In the fail case scenario, this hangs forever, but it should return an error + // Figure out how to do timeout (i.e. 1s) on the engine api. + block_generator.submit_payload(None, 0, true).await?; + + Ok(()) + } + #[tokio::test] #[cfg(feature = "flashblocks")] async fn integration_test_chain_produces_blocks() -> eyre::Result<()> { diff --git a/crates/op-rbuilder/src/integration/mod.rs b/crates/op-rbuilder/src/integration/mod.rs index 1bc7748a9..695b33bf2 100644 --- a/crates/op-rbuilder/src/integration/mod.rs +++ b/crates/op-rbuilder/src/integration/mod.rs @@ -1,5 +1,19 @@ +use alloy_consensus::{Transaction, TxEip1559}; +use alloy_eips::eip4844::builder; +use alloy_eips::BlockNumberOrTag; +use alloy_eips::{eip1559::MIN_PROTOCOL_BASE_FEE, eip2718::Encodable2718}; +use alloy_provider::{Identity, Provider, ProviderBuilder}; +use op_alloy_consensus::OpTypedTransaction; +use op_alloy_network::Optimism; +use op_rbuilder::OpRbuilderConfig; +use op_reth::OpRethConfig; +use parking_lot::Mutex; +use std::cmp::max; +use std::collections::HashSet; use std::future::Future; +use std::net::TcpListener; use std::path::Path; +use std::sync::LazyLock; use std::{ fs::{File, OpenOptions}, io, @@ -10,6 +24,10 @@ use std::{ }; use time::{format_description, OffsetDateTime}; use tokio::time::sleep; +use uuid::Uuid; + +use crate::tester::{BlockGenerator, EngineApi}; +use crate::tx_signer::Signer; /// Default JWT token for testing purposes pub const DEFAULT_JWT_TOKEN: &str = @@ -195,3 +213,122 @@ impl Drop for IntegrationFramework { } } } + +const BUILDER_PRIVATE_KEY: &str = + "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; + +struct TestHarness { + framework: IntegrationFramework, + builder_auth_rpc_port: u16, + builder_http_port: u16, + validator_auth_rpc_port: u16, +} + +impl TestHarness { + pub async fn new(name: &str) -> Self { + let mut framework = IntegrationFramework::new(&name).unwrap(); + + // we are going to use a genesis file pre-generated before the test + let mut genesis_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + genesis_path.push("../../genesis.json"); + assert!(genesis_path.exists()); + + // create the builder + let builder_data_dir = 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 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); + + // 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 _ = framework + .start("op-rbuilder", &op_rbuilder_config) + .await + .unwrap(); + + Self { + framework, + builder_auth_rpc_port, + builder_http_port, + validator_auth_rpc_port, + } + } + + pub async fn send_valid_transaction(&self) -> eyre::Result<()> { + // Get builder's address + let known_wallet = Signer::try_from_secret(BUILDER_PRIVATE_KEY.parse()?)?; + let builder_address = known_wallet.address; + + let url = format!("http://localhost:{}", self.builder_http_port); + let provider = + ProviderBuilder::::default().on_http(url.parse()?); + + // Get current nonce includeing the ones from the txpool + let nonce = provider + .get_transaction_count(builder_address) + .pending() + .await?; + + let latest_block = provider + .get_block_by_number(BlockNumberOrTag::Latest) + .await? + .unwrap(); + + let base_fee = max( + latest_block.header.base_fee_per_gas.unwrap(), + MIN_PROTOCOL_BASE_FEE, + ); + + // Transaction from builder should succeed + let tx_request = OpTypedTransaction::Eip1559(TxEip1559 { + chain_id: 901, + nonce, + gas_limit: 210000, + max_fee_per_gas: base_fee.into(), + ..Default::default() + }); + let signed_tx = known_wallet.sign_tx(tx_request)?; + let _ = provider + .send_raw_transaction(signed_tx.encoded_2718().as_slice()) + .await?; + + Ok(()) + } + + 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); + generator.init().await?; + + Ok(generator) + } +} + +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; + } + } +} diff --git a/crates/op-rbuilder/src/payload_builder_vanilla.rs b/crates/op-rbuilder/src/payload_builder_vanilla.rs index 3dcb0c17f..38b8d67b6 100644 --- a/crates/op-rbuilder/src/payload_builder_vanilla.rs +++ b/crates/op-rbuilder/src/payload_builder_vanilla.rs @@ -514,18 +514,13 @@ impl OpBuilder<'_, Txs> { ctx.metrics .transaction_pool_fetch_duration .record(best_txs_start_time.elapsed()); - if ctx - .execute_best_transactions( - &mut info, - state, - best_txs, - block_gas_limit, - block_da_limit, - )? - .is_some() - { - return Ok(BuildOutcomeKind::Cancelled); - } + ctx.execute_best_transactions( + &mut info, + state, + best_txs, + block_gas_limit, + block_da_limit, + )?; } // Add builder tx to the block diff --git a/crates/op-rbuilder/src/tester/main.rs b/crates/op-rbuilder/src/tester/main.rs index 8946c4c86..f5094490e 100644 --- a/crates/op-rbuilder/src/tester/main.rs +++ b/crates/op-rbuilder/src/tester/main.rs @@ -62,7 +62,7 @@ async fn main() -> eyre::Result<()> { } Commands::Deposit { address, amount } => { let engine_api = EngineApi::builder().build().unwrap(); - let mut generator = BlockGenerator::new(&engine_api, None, false, 1, None); + let mut generator = BlockGenerator::new(engine_api, None, false, 1, None); generator.init().await?; diff --git a/crates/op-rbuilder/src/tester/mod.rs b/crates/op-rbuilder/src/tester/mod.rs index a107f845b..5fb183520 100644 --- a/crates/op-rbuilder/src/tester/mod.rs +++ b/crates/op-rbuilder/src/tester/mod.rs @@ -80,10 +80,22 @@ impl EngineApi { Self::builder().with_url(url).build() } + pub fn new_with_port(port: u16) -> Result> { + Self::builder() + .with_url(&format!("http://localhost:{}", port)) + .build() + } + pub async fn get_payload_v3( &self, payload_id: PayloadId, ) -> eyre::Result<::ExecutionPayloadEnvelopeV3> { + println!( + "Fetching payload with id: {} at {}", + payload_id, + chrono::Utc::now() + ); + Ok( EngineApiClient::::get_payload_v3(&self.engine_api_client, payload_id) .await?, @@ -96,6 +108,8 @@ impl EngineApi { versioned_hashes: Vec, parent_beacon_block_root: B256, ) -> eyre::Result { + println!("Submitting new payload at {}...", chrono::Utc::now()); + Ok(EngineApiClient::::new_payload_v3( &self.engine_api_client, payload, @@ -111,6 +125,8 @@ impl EngineApi { new_head: B256, payload_attributes: Option<::PayloadAttributes>, ) -> eyre::Result { + println!("Updating forkchoice at {}...", chrono::Utc::now()); + Ok(EngineApiClient::::fork_choice_updated_v3( &self.engine_api_client, ForkchoiceState { @@ -183,9 +199,9 @@ pub async fn generate_genesis(output: Option) -> eyre::Result<()> { const FJORD_DATA: &[u8] = &hex!("440a5e200000146b000f79c500000000000000040000000066d052e700000000013ad8a3000000000000000000000000000000000000000000000000000000003ef1278700000000000000000000000000000000000000000000000000000000000000012fdf87b89884a61e74b322bbcf60386f543bfae7827725efaaf0ab1de2294a590000000000000000000000006887246668a3b87f54deb3b94ba47a6f63f32985"); /// A system that continuously generates blocks using the engine API -pub struct BlockGenerator<'a> { - engine_api: &'a EngineApi, - validation_api: Option<&'a EngineApi>, +pub struct BlockGenerator { + engine_api: EngineApi, + validation_api: Option, latest_hash: B256, no_tx_pool: bool, block_time_secs: u64, @@ -194,10 +210,10 @@ pub struct BlockGenerator<'a> { flashblocks_service: Option, } -impl<'a> BlockGenerator<'a> { +impl BlockGenerator { pub fn new( - engine_api: &'a EngineApi, - validation_api: Option<&'a EngineApi>, + engine_api: EngineApi, + validation_api: Option, no_tx_pool: bool, block_time_secs: u64, flashblocks_endpoint: Option, @@ -219,7 +235,7 @@ impl<'a> BlockGenerator<'a> { self.latest_hash = latest_block.header.hash; // Sync validation node if it exists - if let Some(validation_api) = self.validation_api { + if let Some(validation_api) = &self.validation_api { self.sync_validation_node(validation_api).await?; } @@ -321,10 +337,11 @@ impl<'a> BlockGenerator<'a> { } /// Helper function to submit a payload and update chain state - async fn submit_payload( + 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 = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -398,7 +415,7 @@ impl<'a> BlockGenerator<'a> { flashblocks_service.set_current_payload_id(payload_id).await; } - if !self.no_tx_pool { + 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; } @@ -420,7 +437,7 @@ impl<'a> BlockGenerator<'a> { } // Validate with validation node if present - if let Some(validation_api) = self.validation_api { + if let Some(validation_api) = &self.validation_api { let validation_status = validation_api .new_payload(payload.execution_payload.clone(), vec![], B256::ZERO) .await?; @@ -442,7 +459,7 @@ impl<'a> BlockGenerator<'a> { .await?; // Update forkchoice on validator if present - if let Some(validation_api) = self.validation_api { + if let Some(validation_api) = &self.validation_api { validation_api .update_forkchoice(self.latest_hash, new_block_hash, None) .await?; @@ -455,11 +472,11 @@ impl<'a> BlockGenerator<'a> { /// Generate a single new block and return its hash pub async fn generate_block(&mut self) -> eyre::Result { - self.submit_payload(None, 0).await + 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).await + self.submit_payload(None, delay, false).await } /// Submit a deposit transaction to seed an account with ETH @@ -482,7 +499,7 @@ impl<'a> BlockGenerator<'a> { 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) + self.submit_payload(Some(vec![signed_tx_rlp.into()]), 0, false) .await } } @@ -505,8 +522,8 @@ pub async fn run_system( }; let mut generator = BlockGenerator::new( - &engine_api, - validation_api.as_ref(), + engine_api, + validation_api, no_tx_pool, block_time_secs, flashblocks_endpoint, From 49a492021c150cdba2f01d3cc579a80512d369bc Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Thu, 1 May 2025 12:01:26 +0100 Subject: [PATCH 2/3] Fix lint --- .../op-rbuilder/src/integration/integration_test.rs | 13 ++++--------- crates/op-rbuilder/src/integration/mod.rs | 11 +++++------ 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/crates/op-rbuilder/src/integration/integration_test.rs b/crates/op-rbuilder/src/integration/integration_test.rs index dafc86175..6c8f61e93 100644 --- a/crates/op-rbuilder/src/integration/integration_test.rs +++ b/crates/op-rbuilder/src/integration/integration_test.rs @@ -1,8 +1,9 @@ #[cfg(all(test, feature = "integration"))] mod tests { - use crate::integration::TestHarness; use crate::{ - integration::{op_rbuilder::OpRbuilderConfig, op_reth::OpRethConfig, IntegrationFramework}, + integration::{ + op_rbuilder::OpRbuilderConfig, op_reth::OpRethConfig, IntegrationFramework, TestHarness, + }, tester::{BlockGenerator, EngineApi}, tx_signer::Signer, }; @@ -12,13 +13,7 @@ mod tests { use alloy_provider::{Identity, Provider, ProviderBuilder}; use op_alloy_consensus::OpTypedTransaction; use op_alloy_network::Optimism; - use std::{ - cmp::max, - path::PathBuf, - sync::{Arc, Mutex}, - time::Duration, - }; - use tokio_tungstenite::connect_async; + use std::{cmp::max, path::PathBuf}; use uuid::Uuid; const BUILDER_PRIVATE_KEY: &str = diff --git a/crates/op-rbuilder/src/integration/mod.rs b/crates/op-rbuilder/src/integration/mod.rs index 695b33bf2..d62b0d485 100644 --- a/crates/op-rbuilder/src/integration/mod.rs +++ b/crates/op-rbuilder/src/integration/mod.rs @@ -1,5 +1,4 @@ -use alloy_consensus::{Transaction, TxEip1559}; -use alloy_eips::eip4844::builder; +use alloy_consensus::TxEip1559; use alloy_eips::BlockNumberOrTag; use alloy_eips::{eip1559::MIN_PROTOCOL_BASE_FEE, eip2718::Encodable2718}; use alloy_provider::{Identity, Provider, ProviderBuilder}; @@ -217,8 +216,8 @@ impl Drop for IntegrationFramework { const BUILDER_PRIVATE_KEY: &str = "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; -struct TestHarness { - framework: IntegrationFramework, +pub struct TestHarness { + _framework: IntegrationFramework, builder_auth_rpc_port: u16, builder_http_port: u16, validator_auth_rpc_port: u16, @@ -226,7 +225,7 @@ struct TestHarness { impl TestHarness { pub async fn new(name: &str) -> Self { - let mut framework = IntegrationFramework::new(&name).unwrap(); + let mut framework = IntegrationFramework::new(name).unwrap(); // we are going to use a genesis file pre-generated before the test let mut genesis_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); @@ -263,7 +262,7 @@ impl TestHarness { .unwrap(); Self { - framework, + _framework: framework, builder_auth_rpc_port, builder_http_port, validator_auth_rpc_port, From 782d20b108452cfb759937ee91ff4aa69e65b939 Mon Sep 17 00:00:00 2001 From: Ferran Borreguero Date: Thu, 1 May 2025 12:12:20 +0100 Subject: [PATCH 3/3] Fix flashblocks tests --- .../src/integration/integration_test.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/op-rbuilder/src/integration/integration_test.rs b/crates/op-rbuilder/src/integration/integration_test.rs index 6c8f61e93..86c2b033e 100644 --- a/crates/op-rbuilder/src/integration/integration_test.rs +++ b/crates/op-rbuilder/src/integration/integration_test.rs @@ -11,9 +11,17 @@ mod tests { use alloy_eips::{eip1559::MIN_PROTOCOL_BASE_FEE, eip2718::Encodable2718}; use alloy_primitives::hex; use alloy_provider::{Identity, Provider, ProviderBuilder}; + use alloy_rpc_types_eth::BlockTransactionsKind; + use futures_util::StreamExt; use op_alloy_consensus::OpTypedTransaction; use op_alloy_network::Optimism; - use std::{cmp::max, path::PathBuf}; + use std::{ + cmp::max, + path::PathBuf, + sync::{Arc, Mutex}, + time::Duration, + }; + use tokio_tungstenite::connect_async; use uuid::Uuid; const BUILDER_PRIVATE_KEY: &str = @@ -423,7 +431,7 @@ mod tests { let engine_api = EngineApi::new("http://localhost:1234").unwrap(); let validation_api = EngineApi::new("http://localhost:1236").unwrap(); - let mut generator = BlockGenerator::new(&engine_api, Some(&validation_api), false, 2, None); + let mut generator = BlockGenerator::new(engine_api, Some(validation_api), false, 2, None); generator.init().await?; let provider = ProviderBuilder::::default() @@ -552,8 +560,8 @@ mod tests { let validation_api = EngineApi::new("http://localhost:1246").unwrap(); let mut generator = BlockGenerator::new( - &engine_api, - Some(&validation_api), + engine_api, + Some(validation_api), false, block_time_ms / 1000, None,