Skip to content

feat(consensus): Add consensus check for block sequencing #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(consensus): Add consensus check for block sequencing #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6233430
update tests
frisitano Aug 4, 2025
d3d5477
remove println statement
frisitano Aug 4, 2025
53908c1
update consensus algorithm to noop in integration tests
frisitano Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions crates/manager/src/consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -32,6 +34,10 @@ impl Consensus for NoopConsensus {
) -> Result<(), ConsensusError> {
Ok(())
}

fn should_sequence_block(&self, _sequencer: &Address) -> bool {
true
}
}

/// The system contract consensus.
Expand Down Expand Up @@ -71,6 +77,10 @@ impl Consensus for SystemContractConsensus {
}
Ok(())
}

fn should_sequence_block(&self, sequencer: &Address) -> bool {
sequencer == &self.authorized_signer
}
}

#[cfg(test)]
Expand Down
25 changes: 13 additions & 12 deletions crates/manager/src/manager/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,21 +510,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();
}
_ => {}
}
);

Expand Down
3 changes: 3 additions & 0 deletions crates/node/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
]
150 changes: 113 additions & 37 deletions crates/node/src/args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(())
}
}
Expand Down Expand Up @@ -202,13 +223,23 @@ impl ScrollRollupNodeConfig {
);

// Create the consensus.
let consensus: Box<dyn Consensus> = 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<dyn Consensus> = 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<Sender<Arc<L1Notification>>>, _) =
Expand Down Expand Up @@ -297,6 +328,45 @@ pub struct DatabaseArgs {
pub path: Option<PathBuf>,
}

/// 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<Address>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be purer to only fetch authorized_signer based on the system contract's slot? But let me ask the rationale of adding this config first.

Copy link
Collaborator Author

@frisitano frisitano Aug 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is primarily for testing purposes, such that we can run unit tests and set the initial signer without an L1 provider. I agree with the general sentiment. How about we put this behind a test-utils feature flag?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea.

}

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 {
Expand Down Expand Up @@ -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<String>,

/// The private key signer, if any.
pub private_key: Option<PrivateKeySigner>,
}

impl SignerArgs {
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand All @@ -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"
));
}

Expand All @@ -529,20 +611,23 @@ 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(),
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("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]
Expand All @@ -553,13 +638,15 @@ 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(),
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());
Expand All @@ -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());
Expand All @@ -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());
Expand Down
Loading