remove the option for standard building (#38)

Author: akundaz

src/args.rs

@@ -86,9 +86,9 @@ pub struct FlashblocksArgs {
     	name = "WS_ADDRESS",
 		env = "FLASHBLOCKS_WS_ADDRESS",
     	num_args = 0..=1,
-		default_missing_value = "0.0.0.0:10111"
+		default_value = "0.0.0.0:10111"
 	)]
-	enabled: Option<SocketAddr>,
+	pub ws_address: SocketAddr,
 
 	/// Should we calculate the state root for each flashblock
 	#[arg(
@@ -109,18 +109,6 @@ impl Default for BuilderArgs {
 	}
 }
 
-impl FlashblocksArgs {
-	/// Returns true if flashblocks are enabled.
-	pub fn enabled(&self) -> bool {
-		self.enabled.is_some()
-	}
-
-	/// Returns the WebSocket address for flashblocks publishing socket.
-	pub fn ws_address(&self) -> Option<SocketAddr> {
-		self.enabled
-	}
-}
-
 /// This trait is used to extend Reth's CLI with additional functionality that
 /// are specific to the OP builder, such as populating default values for CLI
 /// arguments when running in the playground mode or checking the builder mode.
@@ -209,10 +197,12 @@ impl FlashblocksArgs {
 		Self {
 			interval: Duration::from_millis(250),
 			leeway_time: Duration::from_millis(75),
-			enabled: Some(
-				SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, Self::get_available_port())
-					.into(),
-			),
+			ws_address: SocketAddrV4::new(
+				Ipv4Addr::UNSPECIFIED,
+				Self::get_available_port(),
+			)
+			.into(),
+
 			calculate_state_root: true,
 		}
 	}
@@ -226,10 +216,12 @@ impl FlashblocksArgs {
 		Self {
 			interval,
 			leeway_time,
-			enabled: Some(
-				SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, Self::get_available_port())
-					.into(),
-			),
+			ws_address: SocketAddrV4::new(
+				Ipv4Addr::UNSPECIFIED,
+				Self::get_available_port(),
+			)
+			.into(),
+
 			calculate_state_root: true,
 		}
 	}

src/main.rs

@@ -83,64 +83,6 @@ fn build_pipeline(
 	cli_args: &BuilderArgs,
 	pool: &OrderPool<Flashblocks>,
 ) -> eyre::Result<Pipeline<Flashblocks>> {
-	let pipeline = if cli_args.flashblocks_args.enabled() {
-		build_flashblocks_pipeline(cli_args, pool)?
-	} else {
-		build_classic_pipeline(cli_args, pool)
-	};
-	pool.attach_pipeline(&pipeline);
-
-	Ok(pipeline)
-}
-
-/// Classic block builder
-///
-/// Block building strategy that builds blocks using the classic approach by
-/// producing one block payload per CL payload job.
-fn build_classic_pipeline(
-	cli_args: &BuilderArgs,
-	pool: &OrderPool<Flashblocks>,
-) -> Pipeline<Flashblocks> {
-	let pipeline = if cli_args.revert_protection {
-		Pipeline::<Flashblocks>::named("classic")
-			.with_prologue(OptimismPrologue)
-			.with_pipeline(
-				Loop,
-				(
-					AppendOrders::from_pool(pool),
-					OrderByPriorityFee::default(),
-					RemoveRevertedTransactions::default(),
-				),
-			)
-	} else {
-		Pipeline::<Flashblocks>::named("classic")
-			.with_prologue(OptimismPrologue)
-			.with_pipeline(
-				Loop,
-				(AppendOrders::from_pool(pool), OrderByPriorityFee::default()),
-			)
-	};
-
-	if let Some(ref signer) = cli_args.builder_signer {
-		pipeline.with_epilogue(
-			BuilderEpilogue::with_signer(signer.clone().into())
-				.with_message(|block| format!("Block Number: {}", block.number())),
-		)
-	} else {
-		warn!("BUILDER_SECRET_KEY is not specified, skipping builder transactions");
-		pipeline
-	}
-}
-
-fn build_flashblocks_pipeline(
-	cli_args: &BuilderArgs,
-	pool: &OrderPool<Flashblocks>,
-) -> eyre::Result<Pipeline<Flashblocks>> {
-	let socket_address = cli_args
-		.flashblocks_args
-		.ws_address()
-		.expect("WebSocket address must be set for Flashblocks");
-
 	// how often a flashblock is published
 	let interval = cli_args.flashblocks_args.interval;
 
@@ -152,7 +94,7 @@ fn build_flashblocks_pipeline(
 	// building and the payload job deadline that is given by the CL.
 	let total_building_time = Minus(leeway_time);
 
-	let ws = Arc::new(WebSocketSink::new(socket_address)?);
+	let ws = Arc::new(WebSocketSink::new(cli_args.flashblocks_args.ws_address)?);
 
 	let flashblock_building_pipeline_steps = (
 		AppendOrders::from_pool(pool).with_ok_on_limit(),
@@ -208,7 +150,9 @@ fn build_flashblocks_pipeline(
 				.with_step(BreakAfterMaxFlashblocks::new(flashblock_number)),
 		)
 		.with_limits(Scaled::default().deadline(total_building_time));
+
 	ws.watch_shutdown(&pipeline);
+	pool.attach_pipeline(&pipeline);
 
 	Ok(pipeline)
 }

src/tests/args.rs

@@ -56,28 +56,3 @@ fn test_leeway_time_invalid_format() {
 		);
 	}
 }
-
-#[test]
-fn test_leeway_time_with_flashblocks_enabled() {
-	let args = Cli::try_parse_from([
-		"flashblocks",
-		"node",
-		"--flashblocks.leeway-time",
-		"150ms",
-		"--flashblocks",
-		"127.0.0.1:8080",
-	])
-	.expect("Should parse successfully");
-
-	if let Commands::Node(node_command) = args.command {
-		let flashblocks_args = &node_command.ext.flashblocks_args;
-		assert_eq!(flashblocks_args.leeway_time, Duration::from_millis(150));
-		assert!(flashblocks_args.enabled());
-		assert_eq!(
-			flashblocks_args.ws_address().unwrap().to_string(),
-			"127.0.0.1:8080"
-		);
-	} else {
-		panic!("Expected Node command");
-	}
-}

src/tests/bundle.rs

@@ -14,7 +14,7 @@ use {
 
 #[tokio::test]
 async fn one_valid_tx_included() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let bundle_with_one_tx = random_valid_bundle(1);
 	let bundle_hash = bundle_with_one_tx.hash();
@@ -40,7 +40,7 @@ async fn one_valid_tx_included() -> eyre::Result<()> {
 
 #[tokio::test]
 async fn two_valid_txs_included() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let bundle_with_two_txs = random_valid_bundle(2);
 	let bundle_hash = bundle_with_two_txs.hash();
@@ -67,7 +67,7 @@ async fn two_valid_txs_included() -> eyre::Result<()> {
 
 #[tokio::test]
 async fn min_block_timestamp_constraint() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let mut bundle_with_one_tx = random_valid_bundle(1);
 	bundle_with_one_tx.min_block_number = Some(3);

src/tests/flashblocks.rs

@@ -10,7 +10,7 @@ use {
 
 #[tokio::test]
 async fn empty_blocks_smoke() -> eyre::Result<()> {
-	let (node, ws_addr) = Flashblocks::test_node_with_flashblocks_on().await?;
+	let (node, ws_addr) = Flashblocks::test_node().await?;
 	let ws = WebSocketObserver::new(ws_addr).await?;
 
 	for i in 1..=5 {
@@ -41,7 +41,7 @@ async fn blocks_with_txs_smoke() -> eyre::Result<()> {
 	const BLOCKS: usize = 5;
 	const TXS_PER_BLOCK: usize = 60;
 
-	let (node, ws_addr) = Flashblocks::test_node_with_flashblocks_on().await?;
+	let (node, ws_addr) = Flashblocks::test_node().await?;
 	let ws = WebSocketObserver::new(ws_addr).await?;
 
 	for i in 1..=BLOCKS {
@@ -103,7 +103,7 @@ async fn flashblock_timings_2000ms_block_time_0ms_leeway_time()
 	const TXS_PER_BLOCK: usize = 60;
 
 	let (node, ws_addr) =
-		Flashblocks::test_node_with_flashblocks_on_and_custom_leeway_time_and_interval(
+		Flashblocks::test_node_with_custom_leeway_time_and_interval(
 			Duration::from_millis(0),
 			Duration::from_millis(250),
 		)
@@ -154,7 +154,7 @@ async fn flashblock_timings_2000ms_block_time_75ms_leeway_time()
 -> eyre::Result<()> {
 	const TXS_PER_BLOCK: usize = 60;
 
-	let (node, ws_addr) = Flashblocks::test_node_with_flashblocks_on().await?;
+	let (node, ws_addr) = Flashblocks::test_node().await?;
 	let ws = WebSocketObserver::new(ws_addr).await?;
 
 	// Create a block at the top of the timestamp second
@@ -202,7 +202,7 @@ async fn flashblock_timings_2000ms_block_time_500ms_leeway_time()
 	const TXS_PER_BLOCK: usize = 60;
 
 	let (node, ws_addr) =
-		Flashblocks::test_node_with_flashblocks_on_and_custom_leeway_time_and_interval(
+		Flashblocks::test_node_with_custom_leeway_time_and_interval(
 			Duration::from_millis(500),
 			Duration::from_millis(500),
 		)

src/tests/limits.rs

@@ -31,7 +31,7 @@ async fn run_test(
 	const TXS_PER_BLOCK: usize = 60;
 
 	let (node, ws_addr) =
-		Flashblocks::test_node_with_flashblocks_on_and_custom_leeway_time_and_interval(
+		Flashblocks::test_node_with_custom_leeway_time_and_interval(
 			leeway_time,
 			flashblocks_interval,
 		)

src/tests/mod.rs

@@ -1,5 +1,5 @@
 use {
-	crate::{args::BuilderArgs, bundle::FlashblocksBundle},
+	crate::bundle::FlashblocksBundle,
 	rand::{Rng, rng},
 	rblib::{
 		alloy::{

src/tests/ordering.rs

@@ -15,7 +15,7 @@ use {
 /// flashblock, but not the entire block.
 #[tokio::test]
 async fn txs_ordered_by_priority_fee() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let tx_tips = vec![100, 300, 200, 500, 400];
 	let mut sent_txs = Vec::new();

src/tests/revert.rs

@@ -19,7 +19,7 @@ use {
 
 #[tokio::test]
 async fn critical_reverted_tx_not_included() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let bundle_without_reverts = random_valid_bundle(1);
 	let bundle_with_reverts = random_bundle_with_reverts(0, 1);
@@ -54,7 +54,7 @@ async fn critical_reverted_tx_not_included() -> eyre::Result<()> {
 
 #[tokio::test]
 async fn fallible_reverted_included() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	// create a bundle with one valid tx
 	let bundle_without_reverts = random_valid_bundle(1);
@@ -96,7 +96,7 @@ async fn fallible_reverted_included() -> eyre::Result<()> {
 
 #[tokio::test]
 async fn fallible_optional_reverted_not_included() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	// create a bundle with one valid and one reverting tx
 	let mut bundle_with_reverts = random_bundle_with_reverts(1, 1);
@@ -127,11 +127,7 @@ async fn fallible_optional_reverted_not_included() -> eyre::Result<()> {
 
 #[tokio::test]
 async fn when_disabled_reverted_txs_are_included() -> eyre::Result<()> {
-	let node = Flashblocks::test_node_with_cli_args(BuilderArgs {
-		revert_protection: false,
-		..Default::default()
-	})
-	.await?;
+	let (node, _) = Flashblocks::test_node_with_revert_protection_off().await?;
 
 	// create a bundle with one valid and one reverting tx
 	let mut bundle_with_reverts = random_bundle_with_reverts(1, 1);

src/tests/rpc.rs

@@ -29,7 +29,7 @@ macro_rules! assert_ineligible {
 /// should be rejected by the RPC before making it to the orders pool.
 #[tokio::test]
 async fn max_block_number_in_past() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let block = node.next_block().await?;
 	assert_eq!(block.number(), 1);
@@ -63,7 +63,7 @@ async fn max_block_number_in_past() -> eyre::Result<()> {
 #[tokio::test]
 async fn max_block_timestamp_in_past() -> eyre::Result<()> {
 	// node at genesis, block 0
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 	let genesis_timestamp = node.config().chain.genesis_timestamp();
 	let mut bundle = random_valid_bundle(1);
 	bundle.max_timestamp = Some(genesis_timestamp.saturating_sub(1));
@@ -81,7 +81,7 @@ async fn max_block_timestamp_in_past() -> eyre::Result<()> {
 
 #[tokio::test]
 async fn empty_bundle_rejected_by_rpc() -> eyre::Result<()> {
-	let node = Flashblocks::test_node().await?;
+	let (node, _) = Flashblocks::test_node().await?;
 
 	let empty_bundle = FlashblocksBundle::with_transactions(vec![]);
 	let result = BundlesApiClient::<Flashblocks>::send_bundle(