Skip to content

Commit 6233430

Browse files
frisitanofrisitano
committed
update tests
1 parent 3feb460 commit 6233430

File tree

12 files changed

+276
-59
lines changed

12 files changed

+276
-59
lines changed

‎Cargo.lock‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/manager/src/consensus.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub trait Consensus: Send + Debug {
1515
block: &ScrollBlock,
1616
signature: &Signature,
1717
) -> Result<(), ConsensusError>;
18+
/// Returns a boolean indicating whether the sequencer should sequence a block.
19+
fn should_sequence_block(&self, sequencer: &Address) -> bool;
1820
}
1921

2022
/// A no-op consensus instance.
@@ -32,6 +34,10 @@ impl Consensus for NoopConsensus {
3234
) -> Result<(), ConsensusError> {
3335
Ok(())
3436
}
37+
38+
fn should_sequence_block(&self, _sequencer: &Address) -> bool {
39+
true
40+
}
3541
}
3642

3743
/// The system contract consensus.
@@ -71,6 +77,10 @@ impl Consensus for SystemContractConsensus {
7177
}
7278
Ok(())
7379
}
80+
81+
fn should_sequence_block(&self, sequencer: &Address) -> bool {
82+
sequencer == &self.authorized_signer
83+
}
7484
}
7585

7686
#[cfg(test)]

‎crates/manager/src/manager/mod.rs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,8 @@ where
317317
let _ = signer.sign_block(payload.clone()).inspect_err(|err| error!(target: "scroll::node::manager", ?err, "Failed to send new payload to signer"));
318318
}
319319

320+
println!("new payload sequenced by {:?}", self.signer.as_ref().map(|s| s.address));
321+
320322
if let Some(event_sender) = self.event_sender.as_ref() {
321323
event_sender.notify(RollupManagerEvent::BlockSequenced(payload.clone()));
322324
}
@@ -510,21 +512,22 @@ where
510512
proceed_if!(
511513
en_synced,
512514
// Check if we need to trigger the build of a new payload.
513-
match (
514-
this.block_building_trigger.as_mut().map(|x| x.poll_tick(cx)),
515-
this.engine.is_payload_building_in_progress(),
515+
if let (Some(Poll::Ready(_)), Some(sequencer)) = (
516+
this.block_building_trigger.as_mut().map(|se| se.poll_tick(cx)),
517+
this.sequencer.as_mut()
516518
) {
517-
(Some(Poll::Ready(_)), false) => {
518-
if let Some(sequencer) = this.sequencer.as_mut() {
519-
sequencer.build_payload_attributes();
520-
}
521-
}
522-
(Some(Poll::Ready(_)), true) => {
523-
// If the sequencer is already building a payload, we don't need to trigger it
524-
// again.
519+
if !this.consensus.should_sequence_block(
520+
this.signer
521+
.as_ref()
522+
.map(|s| &s.address)
523+
.expect("signer must be set if sequencer is present"),
524+
) {
525+
trace!(target: "scroll::node::manager", "Signer is not authorized to sequence block for this slot");
526+
} else if this.engine.is_payload_building_in_progress() {
525527
warn!(target: "scroll::node::manager", "Payload building is already in progress skipping slot");
528+
} else {
529+
sequencer.build_payload_attributes();
526530
}
527-
_ => {}
528531
}
529532
);
530533

‎crates/node/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ futures.workspace = true
9595
reth-e2e-test-utils.workspace = true
9696
reth-node-core.workspace = true
9797
reth-provider.workspace = true
98+
reth-primitives-traits.workspace = true
9899
reth-rpc-server-types.workspace = true
99100
reth-scroll-node = { workspace = true, features = ["test-utils"] }
100101
reth-tasks.workspace = true
@@ -140,4 +141,6 @@ test-utils = [
140141
"scroll-alloy-rpc-types-engine",
141142
"alloy-rpc-types-engine",
142143
"scroll-derivation-pipeline",
144+
"reth-primitives-traits/test-utils",
145+
"reth-primitives-traits/test-utils",
143146
]

‎crates/node/src/args.rs‎

Lines changed: 113 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ pub struct ScrollRollupNodeConfig {
4545
/// Whether the rollup node should be run in test mode.
4646
#[arg(long)]
4747
pub test: bool,
48+
/// Consensus args
49+
#[command(flatten)]
50+
pub consensus_args: ConsensusArgs,
4851
/// Database args
4952
#[command(flatten)]
5053
pub database_args: DatabaseArgs,
@@ -74,14 +77,32 @@ pub struct ScrollRollupNodeConfig {
7477
impl ScrollRollupNodeConfig {
7578
/// Validate that either signer key file or AWS KMS key ID is provided when sequencer is enabled
7679
pub fn validate(&self) -> Result<(), String> {
77-
if !self.test && self.sequencer_args.sequencer_enabled {
78-
if self.signer_args.key_file.is_none() && self.signer_args.aws_kms_key_id.is_none() {
79-
return Err("Either signer key file or AWS KMS key ID is required when sequencer is enabled".to_string());
80+
if self.sequencer_args.sequencer_enabled &
81+
!matches!(self.consensus_args.algorithm, ConsensusAlgorithm::Noop)
82+
{
83+
if self.signer_args.key_file.is_none() &&
84+
self.signer_args.aws_kms_key_id.is_none() &&
85+
self.signer_args.private_key.is_none()
86+
{
87+
return Err("Either signer key file, AWS KMS key ID or private key is required when sequencer is enabled".to_string());
8088
}
81-
if self.signer_args.key_file.is_some() && self.signer_args.aws_kms_key_id.is_some() {
82-
return Err("Cannot specify both signer key file and AWS KMS key ID".to_string());
89+
90+
if (self.signer_args.key_file.is_some() as u8 +
91+
self.signer_args.aws_kms_key_id.is_some() as u8 +
92+
self.signer_args.private_key.is_some() as u8) >
93+
1
94+
{
95+
return Err("Cannot specify more than one signer key source".to_string());
8396
}
8497
}
98+
99+
if self.consensus_args.algorithm == ConsensusAlgorithm::SystemContract &&
100+
self.consensus_args.authorized_signer.is_none() &&
101+
self.l1_provider_args.url.is_none()
102+
{
103+
return Err("System contract consensus requires either an authorized signer or a L1 provider URL".to_string());
104+
}
105+
85106
Ok(())
86107
}
87108
}
@@ -202,13 +223,23 @@ impl ScrollRollupNodeConfig {
202223
);
203224

204225
// Create the consensus.
205-
let consensus: Box<dyn Consensus> = if let Some(ref provider) = l1_provider {
206-
let signer = provider
207-
.authorized_signer(node_config.address_book.system_contract_address)
208-
.await?;
209-
Box::new(SystemContractConsensus::new(signer))
210-
} else {
211-
Box::new(NoopConsensus::default())
226+
let consensus: Box<dyn Consensus> = match self.consensus_args.algorithm {
227+
ConsensusAlgorithm::Noop => Box::new(NoopConsensus::default()),
228+
ConsensusAlgorithm::SystemContract => {
229+
let authorized_signer = if let Some(address) = self.consensus_args.authorized_signer
230+
{
231+
address
232+
} else if let Some(provider) = l1_provider.as_ref() {
233+
provider
234+
.authorized_signer(node_config.address_book.system_contract_address)
235+
.await?
236+
} else {
237+
return Err(eyre::eyre!(
238+
"System contract consensus requires either an authorized signer or a L1 provider URL"
239+
));
240+
};
241+
Box::new(SystemContractConsensus::new(authorized_signer))
242+
}
212243
};
213244

214245
let (l1_notification_tx, l1_notification_rx): (Option<Sender<Arc<L1Notification>>>, _) =
@@ -297,6 +328,45 @@ pub struct DatabaseArgs {
297328
pub path: Option<PathBuf>,
298329
}
299330

331+
/// The database arguments.
332+
#[derive(Debug, Default, Clone, clap::Args)]
333+
pub struct ConsensusArgs {
334+
/// The type of consensus to use.
335+
#[arg(
336+
long = "consensus.algorithm",
337+
value_name = "CONSENSUS_ALGORITHM",
338+
default_value = "system-contract"
339+
)]
340+
pub algorithm: ConsensusAlgorithm,
341+
342+
/// The optional authorized signer for system contract consensus.
343+
#[arg(long = "consensus.authorized-signer", value_name = "ADDRESS")]
344+
pub authorized_signer: Option<Address>,
345+
}
346+
347+
impl ConsensusArgs {
348+
/// Create a new [`ConsensusArgs`] with the no-op consensus algorithm.
349+
pub const fn noop() -> Self {
350+
Self { algorithm: ConsensusAlgorithm::Noop, authorized_signer: None }
351+
}
352+
}
353+
354+
/// The consensus algorithm to use.
355+
#[derive(Debug, clap::ValueEnum, Clone, PartialEq, Eq)]
356+
pub enum ConsensusAlgorithm {
357+
/// System contract consensus with an optional authorized signer. If the authorized signer is
358+
/// not provided the system will use the L1 provider to query the authorized signer from L1.
359+
SystemContract,
360+
/// No-op consensus that does not validate blocks.
361+
Noop,
362+
}
363+
364+
impl Default for ConsensusAlgorithm {
365+
fn default() -> Self {
366+
Self::SystemContract
367+
}
368+
}
369+
300370
/// The engine driver args.
301371
#[derive(Debug, Default, Clone, clap::Args)]
302372
pub struct EngineDriverArgs {
@@ -424,6 +494,9 @@ pub struct SignerArgs {
424494
help = "AWS KMS Key ID for signing transactions. Mutually exclusive with --signer.key-file"
425495
)]
426496
pub aws_kms_key_id: Option<String>,
497+
498+
/// The private key signer, if any.
499+
pub private_key: Option<PrivateKeySigner>,
427500
}
428501

429502
impl SignerArgs {
@@ -455,7 +528,7 @@ impl SignerArgs {
455528
.map_err(|e| eyre::eyre!("Failed to create signer from key file: {}", e))?
456529
.with_chain_id(Some(chain_id));
457530

458-
tracing::info!(
531+
tracing::info!(target: "scroll::node::args",
459532
"Created private key signer with address: {} for chain ID: {}",
460533
private_key_signer.address(),
461534
chain_id
@@ -474,12 +547,17 @@ impl SignerArgs {
474547
.map_err(|e| eyre::eyre!("Failed to initialize AWS KMS signer: {}", e))?;
475548

476549
tracing::info!(
550+
target: "scroll::node::args",
477551
"Created AWS KMS signer with address: {} for chain ID: {}",
478552
aws_signer.address(),
479553
chain_id
480554
);
481555

482556
Ok(Some(Box::new(aws_signer)))
557+
} else if let Some(private_key) = &self.private_key {
558+
tracing::info!(target: "scroll::node::args", "Created private key signer with address: {} for chain ID: {}", private_key.address(), chain_id);
559+
let signer = private_key.clone().with_chain_id(Some(chain_id));
560+
Ok(Some(Box::new(signer)))
483561
} else {
484562
Ok(None)
485563
}
@@ -505,19 +583,23 @@ mod tests {
505583
let config = ScrollRollupNodeConfig {
506584
test: false,
507585
sequencer_args: SequencerArgs { sequencer_enabled: true, ..Default::default() },
508-
signer_args: SignerArgs { key_file: None, aws_kms_key_id: None },
586+
signer_args: SignerArgs { key_file: None, aws_kms_key_id: None, private_key: None },
509587
database_args: DatabaseArgs::default(),
510588
engine_driver_args: EngineDriverArgs::default(),
511589
l1_provider_args: L1ProviderArgs::default(),
512590
beacon_provider_args: BeaconProviderArgs::default(),
513591
network_args: NetworkArgs::default(),
514592
gas_price_oracle_args: GasPriceOracleArgs::default(),
593+
consensus_args: ConsensusArgs {
594+
algorithm: ConsensusAlgorithm::SystemContract,
595+
authorized_signer: None,
596+
},
515597
};
516598

517599
let result = config.validate();
518600
assert!(result.is_err());
519601
assert!(result.unwrap_err().contains(
520-
"Either signer key file or AWS KMS key ID is required when sequencer is enabled"
602+
"Either signer key file, AWS KMS key ID or private key is required when sequencer is enabled"
521603
));
522604
}
523605

@@ -529,20 +611,23 @@ mod tests {
529611
signer_args: SignerArgs {
530612
key_file: Some(PathBuf::from("/path/to/key")),
531613
aws_kms_key_id: Some("key-id".to_string()),
614+
private_key: None,
532615
},
533616
database_args: DatabaseArgs::default(),
534617
engine_driver_args: EngineDriverArgs::default(),
535618
l1_provider_args: L1ProviderArgs::default(),
536619
beacon_provider_args: BeaconProviderArgs::default(),
537620
network_args: NetworkArgs::default(),
538621
gas_price_oracle_args: GasPriceOracleArgs::default(),
622+
consensus_args: ConsensusArgs {
623+
algorithm: ConsensusAlgorithm::SystemContract,
624+
authorized_signer: None,
625+
},
539626
};
540627

541628
let result = config.validate();
542629
assert!(result.is_err());
543-
assert!(result
544-
.unwrap_err()
545-
.contains("Cannot specify both signer key file and AWS KMS key ID"));
630+
assert!(result.unwrap_err().contains("Cannot specify more than one signer key source"));
546631
}
547632

548633
#[test]
@@ -553,13 +638,15 @@ mod tests {
553638
signer_args: SignerArgs {
554639
key_file: Some(PathBuf::from("/path/to/key")),
555640
aws_kms_key_id: None,
641+
private_key: None,
556642
},
557643
database_args: DatabaseArgs::default(),
558644
engine_driver_args: EngineDriverArgs::default(),
559645
l1_provider_args: L1ProviderArgs::default(),
560646
beacon_provider_args: BeaconProviderArgs::default(),
561647
network_args: NetworkArgs::default(),
562648
gas_price_oracle_args: GasPriceOracleArgs::default(),
649+
consensus_args: ConsensusArgs::noop(),
563650
};
564651

565652
assert!(config.validate().is_ok());
@@ -570,30 +657,18 @@ mod tests {
570657
let config = ScrollRollupNodeConfig {
571658
test: false,
572659
sequencer_args: SequencerArgs { sequencer_enabled: true, ..Default::default() },
573-
signer_args: SignerArgs { key_file: None, aws_kms_key_id: Some("key-id".to_string()) },
574-
database_args: DatabaseArgs::default(),
575-
engine_driver_args: EngineDriverArgs::default(),
576-
l1_provider_args: L1ProviderArgs::default(),
577-
beacon_provider_args: BeaconProviderArgs::default(),
578-
network_args: NetworkArgs::default(),
579-
gas_price_oracle_args: GasPriceOracleArgs::default(),
580-
};
581-
582-
assert!(config.validate().is_ok());
583-
}
584-
585-
#[test]
586-
fn test_validate_test_mode_without_any_signer_succeeds() {
587-
let config = ScrollRollupNodeConfig {
588-
test: true,
589-
sequencer_args: SequencerArgs { sequencer_enabled: true, ..Default::default() },
590-
signer_args: SignerArgs { key_file: None, aws_kms_key_id: None },
660+
signer_args: SignerArgs {
661+
key_file: None,
662+
aws_kms_key_id: Some("key-id".to_string()),
663+
private_key: None,
664+
},
591665
database_args: DatabaseArgs::default(),
592666
engine_driver_args: EngineDriverArgs::default(),
593667
l1_provider_args: L1ProviderArgs::default(),
594668
beacon_provider_args: BeaconProviderArgs::default(),
595669
network_args: NetworkArgs::default(),
596670
gas_price_oracle_args: GasPriceOracleArgs::default(),
671+
consensus_args: ConsensusArgs::noop(),
597672
};
598673

599674
assert!(config.validate().is_ok());
@@ -604,13 +679,14 @@ mod tests {
604679
let config = ScrollRollupNodeConfig {
605680
test: false,
606681
sequencer_args: SequencerArgs { sequencer_enabled: false, ..Default::default() },
607-
signer_args: SignerArgs { key_file: None, aws_kms_key_id: None },
682+
signer_args: SignerArgs { key_file: None, aws_kms_key_id: None, private_key: None },
608683
database_args: DatabaseArgs::default(),
609684
engine_driver_args: EngineDriverArgs::default(),
610685
l1_provider_args: L1ProviderArgs::default(),
611686
beacon_provider_args: BeaconProviderArgs::default(),
612687
network_args: NetworkArgs::default(),
613688
gas_price_oracle_args: GasPriceOracleArgs::default(),
689+
consensus_args: ConsensusArgs::noop(),
614690
};
615691

616692
assert!(config.validate().is_ok());

0 commit comments

Comments
 (0)