From 623343063c429fbb70e69f5bf31538001b713e28 Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 5 Aug 2025 01:52:03 +0800 Subject: [PATCH 1/4] update tests --- Cargo.lock | 2 + crates/manager/src/consensus.rs | 10 ++ crates/manager/src/manager/mod.rs | 27 +++--- crates/node/Cargo.toml | 3 + crates/node/src/args.rs | 150 ++++++++++++++++++++++-------- crates/node/src/test_utils.rs | 4 +- crates/node/tests/e2e.rs | 116 ++++++++++++++++++++++- crates/node/tests/sync.rs | 5 +- crates/sequencer/tests/e2e.rs | 8 +- crates/signer/Cargo.toml | 1 + crates/signer/src/handle.rs | 6 +- crates/signer/src/lib.rs | 3 +- 12 files changed, 276 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8fde7c9e..e32b07c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10336,6 +10336,7 @@ dependencies = [ "reth-node-builder", "reth-node-core", "reth-node-types", + "reth-primitives-traits", "reth-provider", "reth-revm", "reth-rpc-api", @@ -10524,6 +10525,7 @@ dependencies = [ name = "rollup-node-signer" version = "0.0.1" dependencies = [ + "alloy-primitives", "alloy-signer", "alloy-signer-local", "futures", diff --git a/crates/manager/src/consensus.rs b/crates/manager/src/consensus.rs index 06c3246f..d07f9292 100644 --- a/crates/manager/src/consensus.rs +++ b/crates/manager/src/consensus.rs @@ -15,6 +15,8 @@ pub trait Consensus: Send + Debug { block: &ScrollBlock, signature: &Signature, ) -> Result<(), ConsensusError>; + /// Returns a boolean indicating whether the sequencer should sequence a block. + fn should_sequence_block(&self, sequencer: &Address) -> bool; } /// A no-op consensus instance. @@ -32,6 +34,10 @@ impl Consensus for NoopConsensus { ) -> Result<(), ConsensusError> { Ok(()) } + + fn should_sequence_block(&self, _sequencer: &Address) -> bool { + true + } } /// The system contract consensus. @@ -71,6 +77,10 @@ impl Consensus for SystemContractConsensus { } Ok(()) } + + fn should_sequence_block(&self, sequencer: &Address) -> bool { + sequencer == &self.authorized_signer + } } #[cfg(test)] diff --git a/crates/manager/src/manager/mod.rs b/crates/manager/src/manager/mod.rs index ddd301e3..148ad7cb 100644 --- a/crates/manager/src/manager/mod.rs +++ b/crates/manager/src/manager/mod.rs @@ -317,6 +317,8 @@ where let _ = signer.sign_block(payload.clone()).inspect_err(|err| error!(target: "scroll::node::manager", ?err, "Failed to send new payload to signer")); } + println!("new payload sequenced by {:?}", self.signer.as_ref().map(|s| s.address)); + if let Some(event_sender) = self.event_sender.as_ref() { event_sender.notify(RollupManagerEvent::BlockSequenced(payload.clone())); } @@ -510,21 +512,22 @@ where proceed_if!( en_synced, // Check if we need to trigger the build of a new payload. - match ( - this.block_building_trigger.as_mut().map(|x| x.poll_tick(cx)), - this.engine.is_payload_building_in_progress(), + if let (Some(Poll::Ready(_)), Some(sequencer)) = ( + this.block_building_trigger.as_mut().map(|se| se.poll_tick(cx)), + this.sequencer.as_mut() ) { - (Some(Poll::Ready(_)), false) => { - if let Some(sequencer) = this.sequencer.as_mut() { - sequencer.build_payload_attributes(); - } - } - (Some(Poll::Ready(_)), true) => { - // If the sequencer is already building a payload, we don't need to trigger it - // again. + if !this.consensus.should_sequence_block( + this.signer + .as_ref() + .map(|s| &s.address) + .expect("signer must be set if sequencer is present"), + ) { + trace!(target: "scroll::node::manager", "Signer is not authorized to sequence block for this slot"); + } else if this.engine.is_payload_building_in_progress() { warn!(target: "scroll::node::manager", "Payload building is already in progress skipping slot"); + } else { + sequencer.build_payload_attributes(); } - _ => {} } ); diff --git a/crates/node/Cargo.toml b/crates/node/Cargo.toml index 7a41729d..548f147e 100644 --- a/crates/node/Cargo.toml +++ b/crates/node/Cargo.toml @@ -95,6 +95,7 @@ futures.workspace = true reth-e2e-test-utils.workspace = true reth-node-core.workspace = true reth-provider.workspace = true +reth-primitives-traits.workspace = true reth-rpc-server-types.workspace = true reth-scroll-node = { workspace = true, features = ["test-utils"] } reth-tasks.workspace = true @@ -140,4 +141,6 @@ test-utils = [ "scroll-alloy-rpc-types-engine", "alloy-rpc-types-engine", "scroll-derivation-pipeline", + "reth-primitives-traits/test-utils", + "reth-primitives-traits/test-utils", ] diff --git a/crates/node/src/args.rs b/crates/node/src/args.rs index 981f4e2e..336cdd8a 100644 --- a/crates/node/src/args.rs +++ b/crates/node/src/args.rs @@ -45,6 +45,9 @@ pub struct ScrollRollupNodeConfig { /// Whether the rollup node should be run in test mode. #[arg(long)] pub test: bool, + /// Consensus args + #[command(flatten)] + pub consensus_args: ConsensusArgs, /// Database args #[command(flatten)] pub database_args: DatabaseArgs, @@ -74,14 +77,32 @@ pub struct ScrollRollupNodeConfig { impl ScrollRollupNodeConfig { /// Validate that either signer key file or AWS KMS key ID is provided when sequencer is enabled pub fn validate(&self) -> Result<(), String> { - if !self.test && self.sequencer_args.sequencer_enabled { - if self.signer_args.key_file.is_none() && self.signer_args.aws_kms_key_id.is_none() { - return Err("Either signer key file or AWS KMS key ID is required when sequencer is enabled".to_string()); + if self.sequencer_args.sequencer_enabled & + !matches!(self.consensus_args.algorithm, ConsensusAlgorithm::Noop) + { + if self.signer_args.key_file.is_none() && + self.signer_args.aws_kms_key_id.is_none() && + self.signer_args.private_key.is_none() + { + return Err("Either signer key file, AWS KMS key ID or private key is required when sequencer is enabled".to_string()); } - if self.signer_args.key_file.is_some() && self.signer_args.aws_kms_key_id.is_some() { - return Err("Cannot specify both signer key file and AWS KMS key ID".to_string()); + + if (self.signer_args.key_file.is_some() as u8 + + self.signer_args.aws_kms_key_id.is_some() as u8 + + self.signer_args.private_key.is_some() as u8) > + 1 + { + return Err("Cannot specify more than one signer key source".to_string()); } } + + if self.consensus_args.algorithm == ConsensusAlgorithm::SystemContract && + self.consensus_args.authorized_signer.is_none() && + self.l1_provider_args.url.is_none() + { + return Err("System contract consensus requires either an authorized signer or a L1 provider URL".to_string()); + } + Ok(()) } } @@ -202,13 +223,23 @@ impl ScrollRollupNodeConfig { ); // Create the consensus. - let consensus: Box = if let Some(ref provider) = l1_provider { - let signer = provider - .authorized_signer(node_config.address_book.system_contract_address) - .await?; - Box::new(SystemContractConsensus::new(signer)) - } else { - Box::new(NoopConsensus::default()) + let consensus: Box = match self.consensus_args.algorithm { + ConsensusAlgorithm::Noop => Box::new(NoopConsensus::default()), + ConsensusAlgorithm::SystemContract => { + let authorized_signer = if let Some(address) = self.consensus_args.authorized_signer + { + address + } else if let Some(provider) = l1_provider.as_ref() { + provider + .authorized_signer(node_config.address_book.system_contract_address) + .await? + } else { + return Err(eyre::eyre!( + "System contract consensus requires either an authorized signer or a L1 provider URL" + )); + }; + Box::new(SystemContractConsensus::new(authorized_signer)) + } }; let (l1_notification_tx, l1_notification_rx): (Option>>, _) = @@ -297,6 +328,45 @@ pub struct DatabaseArgs { pub path: Option, } +/// The database arguments. +#[derive(Debug, Default, Clone, clap::Args)] +pub struct ConsensusArgs { + /// The type of consensus to use. + #[arg( + long = "consensus.algorithm", + value_name = "CONSENSUS_ALGORITHM", + default_value = "system-contract" + )] + pub algorithm: ConsensusAlgorithm, + + /// The optional authorized signer for system contract consensus. + #[arg(long = "consensus.authorized-signer", value_name = "ADDRESS")] + pub authorized_signer: Option
, +} + +impl ConsensusArgs { + /// Create a new [`ConsensusArgs`] with the no-op consensus algorithm. + pub const fn noop() -> Self { + Self { algorithm: ConsensusAlgorithm::Noop, authorized_signer: None } + } +} + +/// The consensus algorithm to use. +#[derive(Debug, clap::ValueEnum, Clone, PartialEq, Eq)] +pub enum ConsensusAlgorithm { + /// System contract consensus with an optional authorized signer. If the authorized signer is + /// not provided the system will use the L1 provider to query the authorized signer from L1. + SystemContract, + /// No-op consensus that does not validate blocks. + Noop, +} + +impl Default for ConsensusAlgorithm { + fn default() -> Self { + Self::SystemContract + } +} + /// The engine driver args. #[derive(Debug, Default, Clone, clap::Args)] pub struct EngineDriverArgs { @@ -424,6 +494,9 @@ pub struct SignerArgs { help = "AWS KMS Key ID for signing transactions. Mutually exclusive with --signer.key-file" )] pub aws_kms_key_id: Option, + + /// The private key signer, if any. + pub private_key: Option, } impl SignerArgs { @@ -455,7 +528,7 @@ impl SignerArgs { .map_err(|e| eyre::eyre!("Failed to create signer from key file: {}", e))? .with_chain_id(Some(chain_id)); - tracing::info!( + tracing::info!(target: "scroll::node::args", "Created private key signer with address: {} for chain ID: {}", private_key_signer.address(), chain_id @@ -474,12 +547,17 @@ impl SignerArgs { .map_err(|e| eyre::eyre!("Failed to initialize AWS KMS signer: {}", e))?; tracing::info!( + target: "scroll::node::args", "Created AWS KMS signer with address: {} for chain ID: {}", aws_signer.address(), chain_id ); Ok(Some(Box::new(aws_signer))) + } else if let Some(private_key) = &self.private_key { + tracing::info!(target: "scroll::node::args", "Created private key signer with address: {} for chain ID: {}", private_key.address(), chain_id); + let signer = private_key.clone().with_chain_id(Some(chain_id)); + Ok(Some(Box::new(signer))) } else { Ok(None) } @@ -505,19 +583,23 @@ mod tests { let config = ScrollRollupNodeConfig { test: false, sequencer_args: SequencerArgs { sequencer_enabled: true, ..Default::default() }, - signer_args: SignerArgs { key_file: None, aws_kms_key_id: None }, + signer_args: SignerArgs { key_file: None, aws_kms_key_id: None, private_key: None }, database_args: DatabaseArgs::default(), engine_driver_args: EngineDriverArgs::default(), l1_provider_args: L1ProviderArgs::default(), beacon_provider_args: BeaconProviderArgs::default(), network_args: NetworkArgs::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs { + algorithm: ConsensusAlgorithm::SystemContract, + authorized_signer: None, + }, }; let result = config.validate(); assert!(result.is_err()); assert!(result.unwrap_err().contains( - "Either signer key file or AWS KMS key ID is required when sequencer is enabled" + "Either signer key file, AWS KMS key ID or private key is required when sequencer is enabled" )); } @@ -529,6 +611,7 @@ mod tests { signer_args: SignerArgs { key_file: Some(PathBuf::from("/path/to/key")), aws_kms_key_id: Some("key-id".to_string()), + private_key: None, }, database_args: DatabaseArgs::default(), engine_driver_args: EngineDriverArgs::default(), @@ -536,13 +619,15 @@ mod tests { beacon_provider_args: BeaconProviderArgs::default(), network_args: NetworkArgs::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs { + algorithm: ConsensusAlgorithm::SystemContract, + authorized_signer: None, + }, }; let result = config.validate(); assert!(result.is_err()); - assert!(result - .unwrap_err() - .contains("Cannot specify both signer key file and AWS KMS key ID")); + assert!(result.unwrap_err().contains("Cannot specify more than one signer key source")); } #[test] @@ -553,6 +638,7 @@ mod tests { signer_args: SignerArgs { key_file: Some(PathBuf::from("/path/to/key")), aws_kms_key_id: None, + private_key: None, }, database_args: DatabaseArgs::default(), engine_driver_args: EngineDriverArgs::default(), @@ -560,6 +646,7 @@ mod tests { beacon_provider_args: BeaconProviderArgs::default(), network_args: NetworkArgs::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; assert!(config.validate().is_ok()); @@ -570,30 +657,18 @@ mod tests { let config = ScrollRollupNodeConfig { test: false, sequencer_args: SequencerArgs { sequencer_enabled: true, ..Default::default() }, - signer_args: SignerArgs { key_file: None, aws_kms_key_id: Some("key-id".to_string()) }, - database_args: DatabaseArgs::default(), - engine_driver_args: EngineDriverArgs::default(), - l1_provider_args: L1ProviderArgs::default(), - beacon_provider_args: BeaconProviderArgs::default(), - network_args: NetworkArgs::default(), - gas_price_oracle_args: GasPriceOracleArgs::default(), - }; - - assert!(config.validate().is_ok()); - } - - #[test] - fn test_validate_test_mode_without_any_signer_succeeds() { - let config = ScrollRollupNodeConfig { - test: true, - sequencer_args: SequencerArgs { sequencer_enabled: true, ..Default::default() }, - signer_args: SignerArgs { key_file: None, aws_kms_key_id: None }, + signer_args: SignerArgs { + key_file: None, + aws_kms_key_id: Some("key-id".to_string()), + private_key: None, + }, database_args: DatabaseArgs::default(), engine_driver_args: EngineDriverArgs::default(), l1_provider_args: L1ProviderArgs::default(), beacon_provider_args: BeaconProviderArgs::default(), network_args: NetworkArgs::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; assert!(config.validate().is_ok()); @@ -604,13 +679,14 @@ mod tests { let config = ScrollRollupNodeConfig { test: false, sequencer_args: SequencerArgs { sequencer_enabled: false, ..Default::default() }, - signer_args: SignerArgs { key_file: None, aws_kms_key_id: None }, + signer_args: SignerArgs { key_file: None, aws_kms_key_id: None, private_key: None }, database_args: DatabaseArgs::default(), engine_driver_args: EngineDriverArgs::default(), l1_provider_args: L1ProviderArgs::default(), beacon_provider_args: BeaconProviderArgs::default(), network_args: NetworkArgs::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; assert!(config.validate().is_ok()); diff --git a/crates/node/src/test_utils.rs b/crates/node/src/test_utils.rs index 4c3bdc69..d6212212 100644 --- a/crates/node/src/test_utils.rs +++ b/crates/node/src/test_utils.rs @@ -1,6 +1,6 @@ //! This crate contains utilities for running end-to-end tests for the scroll reth node. -use crate::GasPriceOracleArgs; +use crate::{ConsensusArgs, GasPriceOracleArgs}; use super::{ BeaconProviderArgs, DatabaseArgs, EngineDriverArgs, L1ProviderArgs, ScrollRollupNode, @@ -152,6 +152,7 @@ pub fn default_test_scroll_rollup_node_config() -> ScrollRollupNodeConfig { }, signer_args: Default::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), } } @@ -177,5 +178,6 @@ pub fn default_sequencer_test_scroll_rollup_node_config() -> ScrollRollupNodeCon }, signer_args: Default::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), } } diff --git a/crates/node/tests/e2e.rs b/crates/node/tests/e2e.rs index 3b36d250..8c7a8fe2 100644 --- a/crates/node/tests/e2e.rs +++ b/crates/node/tests/e2e.rs @@ -2,7 +2,10 @@ use alloy_eips::BlockNumberOrTag; use alloy_primitives::{address, b256, Address, Bytes, Signature, B256, U256}; +use alloy_signer::Signer; +use alloy_signer_local::PrivateKeySigner; use futures::StreamExt; +use reth_chainspec::EthChainSpec; use reth_network::{NetworkConfigBuilder, PeersInfo}; use reth_network_api::block::EthWireProvider; use reth_rpc_api::EthApiServer; @@ -14,11 +17,12 @@ use rollup_node::{ default_sequencer_test_scroll_rollup_node_config, default_test_scroll_rollup_node_config, generate_tx, setup_engine, }, - BeaconProviderArgs, DatabaseArgs, EngineDriverArgs, GasPriceOracleArgs, L1ProviderArgs, - NetworkArgs as ScrollNetworkArgs, ScrollRollupNodeConfig, SequencerArgs, + BeaconProviderArgs, ConsensusAlgorithm, ConsensusArgs, DatabaseArgs, EngineDriverArgs, + GasPriceOracleArgs, L1ProviderArgs, NetworkArgs as ScrollNetworkArgs, ScrollRollupNodeConfig, + SequencerArgs, }; use rollup_node_manager::{RollupManagerCommand, RollupManagerEvent, RollupManagerHandle}; -use rollup_node_primitives::BatchCommitData; +use rollup_node_primitives::{sig_encode_hash, BatchCommitData, ConsensusUpdate}; use rollup_node_providers::BlobSource; use rollup_node_sequencer::L1MessageInclusionMode; use rollup_node_watcher::L1Notification; @@ -54,6 +58,7 @@ async fn can_bridge_l1_messages() -> eyre::Result<()> { }, signer_args: Default::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; let (mut nodes, _tasks, _wallet) = setup_engine(node_args, 1, chain_spec, false, false).await?; let node = nodes.pop().unwrap(); @@ -124,6 +129,7 @@ async fn can_sequence_and_gossip_blocks() { }, signer_args: Default::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; let (nodes, _tasks, wallet) = @@ -937,6 +943,110 @@ async fn can_gossip_over_eth_wire() -> eyre::Result<()> { Ok(()) } +#[allow(clippy::large_stack_frames)] +#[tokio::test] +async fn signer_rotation() -> eyre::Result<()> { + reth_tracing::init_test_tracing(); + + // Create the chain spec for scroll dev with Feynman activated and a test genesis. + let chain_spec = (*SCROLL_DEV).clone(); + + // Create two signers. + let signer_1 = PrivateKeySigner::random().with_chain_id(Some(chain_spec.chain().id())); + let signer_1_address = signer_1.address(); + let signer_2 = PrivateKeySigner::random().with_chain_id(Some(chain_spec.chain().id())); + let signer_2_address = signer_2.address(); + + let mut sequencer_1_config = default_sequencer_test_scroll_rollup_node_config(); + + sequencer_1_config.test = false; + sequencer_1_config.consensus_args.algorithm = ConsensusAlgorithm::SystemContract; + sequencer_1_config.consensus_args.authorized_signer = Some(signer_1_address); + sequencer_1_config.signer_args.private_key = Some(signer_1); + + let mut sequencer_2_config = default_sequencer_test_scroll_rollup_node_config(); + sequencer_2_config.test = false; + sequencer_2_config.consensus_args.algorithm = ConsensusAlgorithm::SystemContract; + sequencer_2_config.consensus_args.authorized_signer = Some(signer_1_address); + sequencer_2_config.signer_args.private_key = Some(signer_2); + + // Setup two sequencer nodes. + let (mut nodes, _tasks, _) = + setup_engine(sequencer_1_config, 2, chain_spec.clone(), false, false).await.unwrap(); + let mut sequencer_1 = nodes.pop().unwrap(); + let follower = nodes.pop().unwrap(); + let (mut nodes, _tasks, _) = + setup_engine(sequencer_2_config, 1, chain_spec.clone(), false, false).await.unwrap(); + let mut sequencer_2 = nodes.pop().unwrap(); + + // Create an L1 + let follower_l1_notification_tx = follower.inner.add_ons_handle.l1_watcher_tx.clone().unwrap(); + let sequencer_1_l1_notification_tx = + sequencer_1.inner.add_ons_handle.l1_watcher_tx.clone().unwrap(); + let sequencer_2_l1_notification_tx = + sequencer_2.inner.add_ons_handle.l1_watcher_tx.clone().unwrap(); + + // Create a follower event stream. + let mut follower_events = + follower.inner.add_ons_handle.rollup_manager_handle.get_event_listener().await.unwrap(); + + // connect the two sequencers + sequencer_1.connect(&mut sequencer_2).await; + + wait_n_events( + &mut follower_events, + |event| { + if let RollupManagerEvent::NewBlockReceived(block) = event { + let signature = block.signature; + let hash = sig_encode_hash(&block.block); + // Verify that the block is signed by the first sequencer. + let recovered_address = signature.recover_address_from_prehash(&hash).unwrap(); + recovered_address == signer_1_address + } else { + false + } + }, + 5, + ) + .await; + + // now update the authorized signer to sequencer 2 + follower_l1_notification_tx + .send(Arc::new(L1Notification::Consensus(ConsensusUpdate::AuthorizedSigner( + signer_2_address, + )))) + .await?; + sequencer_1_l1_notification_tx + .send(Arc::new(L1Notification::Consensus(ConsensusUpdate::AuthorizedSigner( + signer_2_address, + )))) + .await?; + sequencer_2_l1_notification_tx + .send(Arc::new(L1Notification::Consensus(ConsensusUpdate::AuthorizedSigner( + signer_2_address, + )))) + .await?; + + wait_n_events( + &mut follower_events, + |event| { + if let RollupManagerEvent::NewBlockReceived(block) = event { + let signature = block.signature; + let hash = sig_encode_hash(&block.block); + let recovered_address = signature.recover_address_from_prehash(&hash).unwrap(); + // Verify that the block is signed by the second sequencer. + recovered_address == signer_2_address + } else { + false + } + }, + 5, + ) + .await; + + Ok(()) +} + /// Read the file provided at `path` as a [`Bytes`]. pub fn read_to_bytes>(path: P) -> eyre::Result { use std::str::FromStr; diff --git a/crates/node/tests/sync.rs b/crates/node/tests/sync.rs index 99cc55a7..48b4a281 100644 --- a/crates/node/tests/sync.rs +++ b/crates/node/tests/sync.rs @@ -12,8 +12,8 @@ use rollup_node::{ default_sequencer_test_scroll_rollup_node_config, default_test_scroll_rollup_node_config, setup_engine, }, - BeaconProviderArgs, DatabaseArgs, EngineDriverArgs, GasPriceOracleArgs, L1ProviderArgs, - NetworkArgs, ScrollRollupNode, ScrollRollupNodeConfig, SequencerArgs, + BeaconProviderArgs, ConsensusArgs, DatabaseArgs, EngineDriverArgs, GasPriceOracleArgs, + L1ProviderArgs, NetworkArgs, ScrollRollupNode, ScrollRollupNodeConfig, SequencerArgs, }; use rollup_node_manager::{RollupManagerCommand, RollupManagerEvent}; use rollup_node_providers::BlobSource; @@ -59,6 +59,7 @@ async fn test_should_consolidate_to_block_15k() -> eyre::Result<()> { }, signer_args: Default::default(), gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; let chain_spec = (*SCROLL_SEPOLIA).clone(); diff --git a/crates/sequencer/tests/e2e.rs b/crates/sequencer/tests/e2e.rs index 5d192597..6b3cb292 100644 --- a/crates/sequencer/tests/e2e.rs +++ b/crates/sequencer/tests/e2e.rs @@ -10,8 +10,8 @@ use reth_scroll_chainspec::SCROLL_DEV; use reth_scroll_node::test_utils::setup; use rollup_node::{ test_utils::{default_test_scroll_rollup_node_config, setup_engine}, - BeaconProviderArgs, DatabaseArgs, EngineDriverArgs, GasPriceOracleArgs, L1ProviderArgs, - NetworkArgs, ScrollRollupNodeConfig, SequencerArgs, SignerArgs, + BeaconProviderArgs, ConsensusArgs, DatabaseArgs, EngineDriverArgs, GasPriceOracleArgs, + L1ProviderArgs, NetworkArgs, ScrollRollupNodeConfig, SequencerArgs, SignerArgs, }; use rollup_node_manager::RollupManagerEvent; use rollup_node_primitives::{sig_encode_hash, BlockInfo, L1MessageEnvelope}; @@ -453,8 +453,10 @@ async fn can_sequence_blocks_with_private_key_file() -> eyre::Result<()> { signer_args: SignerArgs { key_file: Some(temp_file.path().to_path_buf()), aws_kms_key_id: None, + private_key: None, }, gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; let (nodes, _tasks, wallet) = @@ -542,8 +544,10 @@ async fn can_sequence_blocks_with_hex_key_file_without_prefix() -> eyre::Result< signer_args: SignerArgs { key_file: Some(temp_file.path().to_path_buf()), aws_kms_key_id: None, + private_key: None, }, gas_price_oracle_args: GasPriceOracleArgs::default(), + consensus_args: ConsensusArgs::noop(), }; let (nodes, _tasks, wallet) = diff --git a/crates/signer/Cargo.toml b/crates/signer/Cargo.toml index 120d0a97..1e60b79e 100644 --- a/crates/signer/Cargo.toml +++ b/crates/signer/Cargo.toml @@ -11,6 +11,7 @@ workspace = true [dependencies] # alloy +alloy-primitives.workspace = true alloy-signer.workspace = true # reth-scroll diff --git a/crates/signer/src/handle.rs b/crates/signer/src/handle.rs index 8dece5ce..1143eab0 100644 --- a/crates/signer/src/handle.rs +++ b/crates/signer/src/handle.rs @@ -1,3 +1,4 @@ +use alloy_primitives::Address; use futures::{stream::StreamExt, Stream}; use tokio::sync::mpsc::UnboundedSender; use tokio_stream::wrappers::UnboundedReceiverStream; @@ -11,6 +12,8 @@ pub struct SignerHandle { pub request_tx: UnboundedSender, /// A channel to receive events from the signer. pub event_rx: UnboundedReceiverStream, + /// The signer address. + pub address: Address, } impl SignerHandle { @@ -18,8 +21,9 @@ impl SignerHandle { pub const fn new( request_tx: UnboundedSender, event_rx: UnboundedReceiverStream, + address: Address, ) -> Self { - Self { request_tx, event_rx } + Self { request_tx, event_rx, address } } /// Sends a request to sign a block. diff --git a/crates/signer/src/lib.rs b/crates/signer/src/lib.rs index c625d5ce..ed22fd81 100644 --- a/crates/signer/src/lib.rs +++ b/crates/signer/src/lib.rs @@ -50,6 +50,7 @@ impl Signer { fn new(signer: impl alloy_signer::Signer + Send + Sync + 'static) -> (Self, SignerHandle) { let (req_tx, req_rx) = tokio::sync::mpsc::unbounded_channel(); let (event_tx, event_rx) = tokio::sync::mpsc::unbounded_channel(); + let address = signer.address(); let signer = Self { signer: Arc::new(signer), requests: req_rx.into(), @@ -57,7 +58,7 @@ impl Signer { sender: event_tx, metrics: SignerMetrics::default(), }; - (signer, SignerHandle::new(req_tx, event_rx.into())) + (signer, SignerHandle::new(req_tx, event_rx.into(), address)) } /// Spawns a new `Signer` instance onto the tokio runtime. From d3d54772e4c9ac8a6c2087db337aec7488ca8299 Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 5 Aug 2025 02:01:39 +0800 Subject: [PATCH 2/4] remove println statement --- crates/manager/src/manager/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/manager/src/manager/mod.rs b/crates/manager/src/manager/mod.rs index 148ad7cb..d4c9d3a9 100644 --- a/crates/manager/src/manager/mod.rs +++ b/crates/manager/src/manager/mod.rs @@ -317,8 +317,6 @@ where let _ = signer.sign_block(payload.clone()).inspect_err(|err| error!(target: "scroll::node::manager", ?err, "Failed to send new payload to signer")); } - println!("new payload sequenced by {:?}", self.signer.as_ref().map(|s| s.address)); - if let Some(event_sender) = self.event_sender.as_ref() { event_sender.notify(RollupManagerEvent::BlockSequenced(payload.clone())); } From 53908c1a1f866b5e0c2910121ecb07a14a720383 Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 5 Aug 2025 02:34:42 +0800 Subject: [PATCH 3/4] update consensus algorithm to noop in integration tests --- tests/launch_rollup_node_follower.bash | 3 ++- tests/launch_rollup_node_sequencer.bash | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/launch_rollup_node_follower.bash b/tests/launch_rollup_node_follower.bash index b2ec2a55..117608f7 100644 --- a/tests/launch_rollup_node_follower.bash +++ b/tests/launch_rollup_node_follower.bash @@ -9,4 +9,5 @@ exec rollup-node node --chain dev --datadir=/l2reth --metrics=0.0.0.0:6060 --net --txpool.pending-max-count=1000 \ --builder.gaslimit=20000000 \ --rpc.max-connections=5000 \ - --trusted-peers enode://3983278a7cab48862d9ab3187278edf376a0736a7deb55472a5650592f6922ce626a1ea7d74b77b9a679694b343f5e93ea97d5d60a9db4e4b51bb0c23a36d01b@rollup-node-sequencer:30303 + --trusted-peers enode://3983278a7cab48862d9ab3187278edf376a0736a7deb55472a5650592f6922ce626a1ea7d74b77b9a679694b343f5e93ea97d5d60a9db4e4b51bb0c23a36d01b@rollup-node-sequencer:30303 \ + --consensus.algorithm=noop diff --git a/tests/launch_rollup_node_sequencer.bash b/tests/launch_rollup_node_sequencer.bash index 57ab5abe..fa032efb 100644 --- a/tests/launch_rollup_node_sequencer.bash +++ b/tests/launch_rollup_node_sequencer.bash @@ -11,4 +11,5 @@ exec rollup-node node --chain dev --datadir=/l2reth --metrics=0.0.0.0:6060 --net --sequencer.payload-building-duration 230 \ --txpool.pending-max-count=1000 \ --builder.gaslimit=20000000 \ - --rpc.max-connections=5000 + --rpc.max-connections=5000 \ + --consensus.algorithm=noop From fbc8e41bf33b0db18ded614b22a2b98b022153bb Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 5 Aug 2025 18:38:40 +0800 Subject: [PATCH 4/4] address comments --- Makefile | 5 ++-- crates/node/src/args.rs | 55 ++++++++++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index 3862b725..424c952b 100644 --- a/Makefile +++ b/Makefile @@ -64,11 +64,12 @@ lint: fmt lint-toml clippy udeps codespell zepter .PHONY: test test: - cargo test \ + cargo nextest run \ --workspace \ --locked \ --all-features \ - --no-fail-fast + --no-fail-fast \ + -E 'not test(docker)' # Used to update the mainnet-sample.sql data. Provide the path to the sqlite database that should be read from # using `DB_PATH`. diff --git a/crates/node/src/args.rs b/crates/node/src/args.rs index 336cdd8a..719193e3 100644 --- a/crates/node/src/args.rs +++ b/crates/node/src/args.rs @@ -137,6 +137,10 @@ impl ScrollRollupNodeConfig { RollupManagerHandle, Option>>, )> { + tracing::info!(target: "rollup_node::args", + "Building rollup node with config:\n{:#?}", + self + ); // Instantiate the network manager let scroll_network_manager = ScrollNetworkManager::from_parts(network.clone(), events); @@ -223,24 +227,16 @@ impl ScrollRollupNodeConfig { ); // Create the consensus. - let consensus: Box = match self.consensus_args.algorithm { - ConsensusAlgorithm::Noop => Box::new(NoopConsensus::default()), - ConsensusAlgorithm::SystemContract => { - let authorized_signer = if let Some(address) = self.consensus_args.authorized_signer - { - address - } else if let Some(provider) = l1_provider.as_ref() { - provider - .authorized_signer(node_config.address_book.system_contract_address) - .await? - } else { - return Err(eyre::eyre!( - "System contract consensus requires either an authorized signer or a L1 provider URL" - )); - }; - Box::new(SystemContractConsensus::new(authorized_signer)) - } + let authorized_signer = if let Some(provider) = l1_provider.as_ref() { + Some( + provider + .authorized_signer(node_config.address_book.system_contract_address) + .await?, + ) + } else { + None }; + let consensus = self.consensus_args.consensus(authorized_signer)?; let (l1_notification_tx, l1_notification_rx): (Option>>, _) = if let Some(provider) = l1_provider.filter(|_| !self.test) { @@ -349,6 +345,31 @@ impl ConsensusArgs { pub const fn noop() -> Self { Self { algorithm: ConsensusAlgorithm::Noop, authorized_signer: None } } + + /// Creates a consensus instance based on the configured algorithm and authorized signer. + /// + /// The `authorized_signer` field of `ConsensusArgs` takes precedence over the + /// `authorized_signer` parameter passed to this method. + pub fn consensus( + &self, + authorized_signer: Option
, + ) -> eyre::Result> { + match self.algorithm { + ConsensusAlgorithm::Noop => Ok(Box::new(NoopConsensus::default())), + ConsensusAlgorithm::SystemContract => { + let authorized_signer = if let Some(address) = self.authorized_signer { + address + } else if let Some(address) = authorized_signer { + address + } else { + return Err(eyre::eyre!( + "System contract consensus requires either an authorized signer or a L1 provider URL" + )); + }; + Ok(Box::new(SystemContractConsensus::new(authorized_signer))) + } + } + } } /// The consensus algorithm to use.