Skip to content

Commit

Permalink
chore(consensus): remove some fields from ConsensusConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
guy-starkware committed Jan 16, 2025
1 parent 14909c5 commit e07b167
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 111 deletions.
28 changes: 9 additions & 19 deletions config/papyrus/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,30 +79,20 @@
"privacy": "TemporaryValue",
"value": true
},
"consensus.chain_id": {
"description": "The chain id of the Starknet chain.",
"pointer_target": "chain_id",
"privacy": "Public"
},
"consensus.consensus_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.",
"consensus.future_height_limit": {
"description": "How many heights in the future should we cache.",
"privacy": "Public",
"value": "consensus"
"value": 10
},
"consensus.num_validators": {
"description": "The number of validators in the consensus.",
"consensus.future_round_limit": {
"description": "How many rounds in the future should we cache.",
"privacy": "Public",
"value": 1
"value": 10
},
"consensus.start_height": {
"description": "The height to start the consensus from.",
"consensus.startup_delay": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"privacy": "Public",
"value": 0
"value": 5
},
"consensus.sync_retry_interval": {
"description": "The duration (seconds) between sync attempts.",
Expand Down
28 changes: 9 additions & 19 deletions config/sequencer/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -514,30 +514,20 @@
"pointer_target": "recorder_url",
"privacy": "Private"
},
"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": {
"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.",
"consensus_manager_config.consensus_config.future_height_limit": {
"description": "How many heights in the future should we cache.",
"privacy": "Public",
"value": "consensus"
"value": 10
},
"consensus_manager_config.consensus_config.num_validators": {
"description": "The number of validators in the consensus.",
"consensus_manager_config.consensus_config.future_round_limit": {
"description": "How many rounds in the future should we cache.",
"privacy": "Public",
"value": 1
"value": 10
},
"consensus_manager_config.consensus_config.start_height": {
"description": "The height to start the consensus from.",
"consensus_manager_config.consensus_config.startup_delay": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"privacy": "Public",
"value": 0
"value": 5
},
"consensus_manager_config.consensus_config.sync_retry_interval": {
"description": "The duration (seconds) between sync attempts.",
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,34 +89,24 @@ 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": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"consensus.future_height_limit": {
"description": "How many heights in the future should we cache.",
"value": {
"$serde_json::private::Number": "5"
"$serde_json::private::Number": "10"
},
"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.",
"consensus.future_round_limit": {
"description": "How many rounds in the future should we cache.",
"value": {
"$serde_json::private::Number": "1"
"$serde_json::private::Number": "10"
},
"privacy": "Public"
},
"consensus.start_height": {
"description": "The height to start the consensus from.",
"consensus.startup_delay": {
"description": "Delay (seconds) before starting consensus to give time for network peering.",
"value": {
"$serde_json::private::Number": "0"
"$serde_json::private::Number": "5"
},
"privacy": "Public"
},
Expand Down
5 changes: 3 additions & 2 deletions crates/papyrus_node/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ const DEFAULT_LEVEL: LevelFilter = LevelFilter::INFO;
// TODO: Consider moving to a more general place.
const GENESIS_HASH: &str = "0x0";

// TODO(guyn): move this to the config.
// TODO(guyn): move this to the config. (in the next PR this will happen!)
pub const NETWORK_VOTES_TOPIC: &str = "consensus_votes";
pub const NETWORK_TOPIC: &str = "consensus_proposals";

// TODO(dvir): add this to config.
Expand Down Expand Up @@ -231,7 +232,7 @@ fn spawn_consensus(
// TODO(Asmaa): replace with the correct value.
papyrus_consensus_config.start_height,
consensus_config.validator_id,
consensus_config.consensus_delay,
consensus_config.startup_delay,
consensus_config.timeouts.clone(),
consensus_config.sync_retry_interval,
network_channels.into(),
Expand Down
71 changes: 29 additions & 42 deletions crates/sequencing/papyrus_consensus/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,82 +13,70 @@ 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 starknet_api::block::BlockNumber;
use validator::Validate;

use crate::types::ValidatorId;

/// Configuration for consensus.
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Validate)]
pub struct ConsensusConfig {
// TODO(guyn): this will be removed in one of the next 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,
// pub start_height: BlockNumber,
/// 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.
#[serde(deserialize_with = "deserialize_float_seconds_to_duration")]
pub sync_retry_interval: Duration,
/// How many heights in the future should we cache.
pub future_height_limit: u32,
/// How many rounds in the future should we cache.
pub future_round_limit: u32,
}

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(
// "start_height",
// &self.start_height,
// "The height to start the consensus from.",
// 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.",
"startup_delay",
&self.startup_delay.as_secs(),
"Delay (seconds) before starting consensus to give time for network peering.",
ParamPrivacyInput::Public,
),
ser_param(
"num_validators",
&self.num_validators,
"The number of validators in the consensus.",
"sync_retry_interval",
&self.sync_retry_interval.as_secs_f64(),
"The duration (seconds) between sync attempts.",
ParamPrivacyInput::Public,
),
ser_param(
"consensus_delay",
&self.consensus_delay.as_secs(),
"Delay (seconds) before starting consensus to give time for network peering.",
"future_height_limit",
&self.future_height_limit,
"How many heights in the future should we cache.",
ParamPrivacyInput::Public,
),
ser_param(
"sync_retry_interval",
&self.sync_retry_interval.as_secs_f64(),
"The duration (seconds) between sync attempts.",
"future_round_limit",
&self.future_round_limit,
"How many rounds in the future should we cache.",
ParamPrivacyInput::Public,
),
]);
Expand All @@ -100,14 +88,13 @@ 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),
// start_height: BlockNumber::default(),
startup_delay: Duration::from_secs(5),
timeouts: TimeoutsConfig::default(),
sync_retry_interval: Duration::from_secs_f64(1.0),
future_height_limit: 10,
future_round_limit: 10,
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions crates/starknet_consensus_manager/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use papyrus_consensus::types::ContextConfig;
use papyrus_consensus_orchestrator::cende::CendeConfig;
use papyrus_network::NetworkConfig;
use serde::{Deserialize, Serialize};
use starknet_api::block::BlockNumber;
use validator::Validate;

/// The consensus manager related configuration.
Expand All @@ -20,7 +21,7 @@ pub struct ConsensusManagerConfig {
pub cende_config: CendeConfig,
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 @@ -67,7 +68,7 @@ impl Default for ConsensusManagerConfig {
network_config: NetworkConfig::default(),
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 @@ -69,7 +69,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 @@ -94,7 +94,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
7 changes: 3 additions & 4 deletions crates/starknet_integration_tests/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,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,
// TODO(Matan, Dan): Set the right amount
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 @@ -51,7 +51,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

0 comments on commit e07b167

Please sign in to comment.