Skip to content
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.

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

chore(consensus): remove some fields from ConsensusConfig #3379

Merged
merged 1 commit into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 1 addition & 21 deletions config/papyrus/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,31 +79,11 @@
"privacy": "TemporaryValue",
"value": true
},
"consensus.chain_id": {
"description": "The chain id of the Starknet chain.",
"pointer_target": "chain_id",
"privacy": "Public"
},
"consensus.consensus_delay": {
"consensus.startup_delay": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"privacy": "Public",
"value": 5
},
"consensus.network_topic": {
"description": "The network topic of the consensus.",
"privacy": "Public",
"value": "consensus"
},
"consensus.num_validators": {
"description": "The number of validators in the consensus.",
"privacy": "Public",
"value": 1
},
"consensus.start_height": {
"description": "The height to start the consensus from.",
"privacy": "Public",
"value": 0
},
"consensus.sync_retry_interval": {
"description": "The duration (seconds) between sync attempts.",
"privacy": "Public",
Expand Down
22 changes: 1 addition & 21 deletions config/sequencer/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -654,31 +654,11 @@
"privacy": "TemporaryValue",
"value": true
},
"consensus_manager_config.consensus_config.chain_id": {
"description": "The chain id of the Starknet chain.",
"pointer_target": "chain_id",
"privacy": "Public"
},
"consensus_manager_config.consensus_config.consensus_delay": {
"consensus_manager_config.consensus_config.startup_delay": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"privacy": "Public",
"value": 5
},
"consensus_manager_config.consensus_config.network_topic": {
"description": "The network topic of the consensus.",
"privacy": "Public",
"value": "consensus"
},
"consensus_manager_config.consensus_config.num_validators": {
"description": "The number of validators in the consensus.",
"privacy": "Public",
"value": 1
},
"consensus_manager_config.consensus_config.start_height": {
"description": "The height to start the consensus from.",
"privacy": "Public",
"value": 0
},
"consensus_manager_config.consensus_config.sync_retry_interval": {
"description": "The duration (seconds) between sync attempts.",
"privacy": "Public",
Expand Down
1 change: 0 additions & 1 deletion crates/papyrus_node/src/config/pointers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ pub static CONFIG_POINTERS: LazyLock<ConfigPointers> = LazyLock::new(|| {
"The chain to follow. For more details see https://docs.starknet.io/documentation/architecture_and_concepts/Blocks/transactions/#chain-id.",
),
set_pointing_param_paths(&[
"consensus.chain_id",
"context.chain_id",
"network.chain_id",
"rpc.chain_id",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,37 +89,13 @@ expression: dumped_default_config
"value": true,
"privacy": "TemporaryValue"
},
"consensus.chain_id": {
"description": "The chain id of the Starknet chain.",
"value": "0x0",
"privacy": "Public"
},
"consensus.consensus_delay": {
"consensus.startup_delay": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"value": {
"$serde_json::private::Number": "5"
},
"privacy": "Public"
},
"consensus.network_topic": {
"description": "The network topic of the consensus.",
"value": "consensus",
"privacy": "Public"
},
"consensus.num_validators": {
"description": "The number of validators in the consensus.",
"value": {
"$serde_json::private::Number": "1"
},
"privacy": "Public"
},
"consensus.start_height": {
"description": "The height to start the consensus from.",
"value": {
"$serde_json::private::Number": "0"
},
"privacy": "Public"
},
"consensus.sync_retry_interval": {
"description": "The duration (seconds) between sync attempts.",
"value": {
Expand Down
51 changes: 4 additions & 47 deletions crates/starknet_consensus/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,18 @@ use papyrus_config::dumping::{append_sub_config_name, ser_param, SerializeConfig
use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam};
use papyrus_protobuf::consensus::DEFAULT_VALIDATOR_ID;
use serde::{Deserialize, Serialize};
use starknet_api::block::BlockNumber;
use starknet_api::core::ChainId;
use validator::Validate;

use crate::types::ValidatorId;

/// Configuration for consensus.
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Validate)]
pub struct ConsensusConfig {
// TODO(guyn): the chain_id, validator_id, and network_topic are going to be removed in
// following PRs.
/// The chain id of the Starknet chain.
pub chain_id: ChainId,
/// The validator ID of the node.
pub validator_id: ValidatorId,
// TODO(guyn): this will be removed in one of the next PRs.
/// The network topic of the consensus.
pub network_topic: String,
// TODO(guyn): this will be removed in one of the next PRs??
/// The height to start the consensus from.
pub start_height: BlockNumber,
/// The number of validators in the consensus.
// Used for testing in an early milestones.
pub num_validators: u64,
/// The delay (seconds) before starting consensus to give time for network peering.
#[serde(deserialize_with = "deserialize_seconds_to_duration")]
pub consensus_delay: Duration,
pub startup_delay: Duration,
/// Timeouts configuration for consensus.
pub timeouts: TimeoutsConfig,
/// The duration (seconds) between sync attempts.
Expand All @@ -50,39 +35,15 @@ pub struct ConsensusConfig {
impl SerializeConfig for ConsensusConfig {
fn dump(&self) -> BTreeMap<ParamPath, SerializedParam> {
let mut config = BTreeMap::from_iter([
ser_param(
"chain_id",
&self.chain_id,
"The chain id of the Starknet chain.",
ParamPrivacyInput::Public,
),
ser_param(
"validator_id",
&self.validator_id,
"The validator id of the node.",
ParamPrivacyInput::Public,
),
ser_param(
"network_topic",
&self.network_topic,
"The network topic of the consensus.",
ParamPrivacyInput::Public,
),
ser_param(
"start_height",
&self.start_height,
"The height to start the consensus from.",
ParamPrivacyInput::Public,
),
ser_param(
"num_validators",
&self.num_validators,
"The number of validators in the consensus.",
ParamPrivacyInput::Public,
),
ser_param(
"consensus_delay",
&self.consensus_delay.as_secs(),
"startup_delay",
&self.startup_delay.as_secs(),
"Delay (seconds) before starting consensus to give time for network peering.",
ParamPrivacyInput::Public,
),
Expand All @@ -101,12 +62,8 @@ impl SerializeConfig for ConsensusConfig {
impl Default for ConsensusConfig {
fn default() -> Self {
Self {
chain_id: ChainId::Other("0x0".to_string()),
validator_id: ValidatorId::from(DEFAULT_VALIDATOR_ID),
network_topic: "consensus".to_string(),
start_height: BlockNumber::default(),
num_validators: 1,
consensus_delay: Duration::from_secs(5),
startup_delay: Duration::from_secs(5),
timeouts: TimeoutsConfig::default(),
sync_retry_interval: Duration::from_secs_f64(1.0),
}
Expand Down
4 changes: 2 additions & 2 deletions crates/starknet_consensus_manager/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub struct ConsensusManagerConfig {
pub revert_up_to_and_including: Option<BlockNumber>,
pub votes_topic: String,
pub proposals_topic: String,
pub immediate_active_height: u64,
pub immediate_active_height: BlockNumber,
}

impl SerializeConfig for ConsensusManagerConfig {
Expand Down Expand Up @@ -80,7 +80,7 @@ impl Default for ConsensusManagerConfig {
revert_up_to_and_including: None,
votes_topic: "consensus_votes".to_string(),
proposals_topic: "consensus_proposals".to_string(),
immediate_active_height: 0,
immediate_active_height: BlockNumber::default(),
}
}
}
4 changes: 2 additions & 2 deletions crates/starknet_consensus_manager/src/consensus_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl ConsensusManager {
error!("Failed to get height from batcher: {:?}", e);
ConsensusError::Other("Failed to get height from batcher".to_string())
})?;
let active_height = if self.config.consensus_config.start_height == observer_height {
let active_height = if self.config.immediate_active_height == observer_height {
// Setting `start_height` is only used to enable consensus starting immediately without
// observing the first height. This means consensus may return to a height
// it has already voted on, risking equivocation. This is only safe to do if we
Expand All @@ -110,7 +110,7 @@ impl ConsensusManager {
active_height,
observer_height,
self.config.consensus_config.validator_id,
self.config.consensus_config.consensus_delay,
self.config.consensus_config.startup_delay,
self.config.consensus_config.timeouts.clone(),
self.config.consensus_config.sync_retry_interval,
votes_broadcast_channels.into(),
Expand Down
5 changes: 2 additions & 3 deletions crates/starknet_integration_tests/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,10 @@ pub(crate) fn create_consensus_manager_configs_from_network_configs(
// TODO(Matan): Get config from default config file.
.map(|network_config| ConsensusManagerConfig {
network_config,
immediate_active_height: BlockNumber(1),
consensus_config: ConsensusConfig {
start_height: BlockNumber(1),
// TODO(Matan, Dan): Set the right amount
consensus_delay: Duration::from_secs(15),
num_validators,
startup_delay: Duration::from_secs(15),
timeouts: timeouts.clone(),
..Default::default()
},
Expand Down
1 change: 0 additions & 1 deletion crates/starknet_sequencer_node/src/config/node_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ pub static CONFIG_POINTERS: LazyLock<ConfigPointers> = LazyLock::new(|| {
set_pointing_param_paths(&[
"batcher_config.block_builder_config.chain_info.chain_id",
"batcher_config.storage.db_config.chain_id",
"consensus_manager_config.consensus_config.chain_id",
"consensus_manager_config.context_config.chain_id",
"consensus_manager_config.network_config.chain_id",
"gateway_config.chain_info.chain_id",
Expand Down
Loading