Skip to content

Commit

Permalink
chore(consensus): remove config default for chain_id config parameter (
Browse files Browse the repository at this point in the history
  • Loading branch information
guy-starkware authored Jan 16, 2025
1 parent 453e3f1 commit be32bfa
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 19 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/papyrus_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
- name: Run executable
run: >
target/debug/papyrus_node --base_layer.node_url ${{ secrets.CI_BASE_LAYER_NODE_URL }}
target/debug/papyrus_node --chain_id SN_SEPOLIA --base_layer.node_url ${{ secrets.CI_BASE_LAYER_NODE_URL }}
& sleep 30 ; kill $!
executable-run-no-rpc:
Expand All @@ -55,7 +55,7 @@ jobs:
- name: Run executable
run: >
target/debug/papyrus_node --base_layer.node_url ${{ secrets.CI_BASE_LAYER_NODE_URL }}
target/debug/papyrus_node --chain_id SN_SEPOLIA --base_layer.node_url ${{ secrets.CI_BASE_LAYER_NODE_URL }}
& sleep 30 ; kill $!
# FIXME: Job is currently running out of disk space, error is hidden inside the `Annoatations`
Expand Down
6 changes: 3 additions & 3 deletions config/papyrus/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@
"privacy": "Public"
},
"chain_id": {
"description": "The chain to follow. For more details see https://docs.starknet.io/documentation/architecture_and_concepts/Blocks/transactions/#chain-id.",
"privacy": "TemporaryValue",
"value": "SN_MAIN"
"description": "A required param! The chain to follow. For more details see https://docs.starknet.io/documentation/architecture_and_concepts/Blocks/transactions/#chain-id.",
"param_type": "String",
"privacy": "TemporaryValue"
},
"collect_metrics": {
"description": "If true, collect metrics for the node.",
Expand Down
1 change: 1 addition & 0 deletions crates/blockifier/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ impl ChainInfo {
impl Default for ChainInfo {
fn default() -> Self {
ChainInfo {
// TODO(guyn): should we remove the default value for chain_id?
chain_id: ChainId::Other("0x0".to_string()),
fee_token_addresses: FeeTokenAddresses::default(),
}
Expand Down
1 change: 0 additions & 1 deletion crates/papyrus_config/src/loading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ pub fn load_and_process_config<T: for<'a> Deserialize<'a>>(
) -> Result<T, ConfigError> {
let deserialized_default_config: Map<String, Value> =
serde_json::from_reader(default_config_file)?;

// Store the pointers separately from the default values. The pointers will receive a value
// only at the end of the process.
let (default_config_map, pointers_map) = split_pointers_map(deserialized_default_config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ async fn main() {
let _ = fs::remove_dir_all(path.clone());
fs::create_dir_all(path.clone()).expect("Should make a temporary `data` directory");
let config = NodeConfig::load_and_process(vec![
"Placeholder-binary-name".to_owned(),
"--chain_id=SN_SEPOLIA".to_owned(),
"--starknet_url=https://alpha-sepolia.starknet.io/".to_owned(),
"--base_layer.node_url=https://mainnet.infura.io/v3/1234".to_owned(),
Expand Down
25 changes: 18 additions & 7 deletions crates/papyrus_node/src/config/config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,29 @@ fn required_args() -> Vec<String> {
args
}

fn get_args(additional_args: Vec<&str>) -> Vec<String> {
fn get_args(additional_args: impl IntoIterator<Item = impl ToString>) -> Vec<String> {
let mut args = vec!["Papyrus".to_owned()];
args.append(&mut required_args());
args.append(&mut additional_args.into_iter().map(|s| s.to_owned()).collect());
args.extend(required_args());
args.extend(additional_args.into_iter().map(|s| s.to_string()));
args
}

#[test]
fn load_default_config() {
env::set_current_dir(resolve_project_relative_path("").unwrap())
.expect("Couldn't set working dir.");
NodeConfig::load_and_process(get_args(vec![])).expect("Failed to load the config.");
NodeConfig::load_and_process(get_args(["--chain_id", "SN_MAIN"]))
.expect("Failed to load the config.");
}

#[test]
fn load_http_headers() {
let args = get_args(vec!["--central.http_headers", "NAME_1:VALUE_1 NAME_2:VALUE_2"]);
let args = get_args([
"--central.http_headers",
"NAME_1:VALUE_1 NAME_2:VALUE_2",
"--chain_id",
"SN_MAIN",
]);
env::set_current_dir(resolve_project_relative_path("").unwrap())
.expect("Couldn't set working dir.");
let config = NodeConfig::load_and_process(args).unwrap();
Expand Down Expand Up @@ -113,12 +119,17 @@ fn test_dump_default_config() {
fn test_default_config_process() {
env::set_current_dir(resolve_project_relative_path("").unwrap())
.expect("Couldn't set working dir.");
assert_eq!(NodeConfig::load_and_process(get_args(vec![])).unwrap(), NodeConfig::default());
assert_eq!(
NodeConfig::load_and_process(get_args(["--chain_id", "SN_MAIN"])).unwrap(),
NodeConfig::default()
);
}

#[test]
fn test_update_dumped_config_by_command() {
let args = get_args(vec![
let args = get_args([
"--chain_id",
"SN_MAIN",
"--central.retry_config.retry_max_delay_millis",
"1234",
"--storage.db_config.path_prefix",
Expand Down
7 changes: 4 additions & 3 deletions crates/papyrus_node/src/config/pointers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use papyrus_config::dumping::{
append_sub_config_name,
ser_optional_sub_config,
ser_pointer_target_param,
ser_pointer_target_required_param,
set_pointing_param_paths,
ConfigPointers,
Pointers,
Expand All @@ -26,7 +27,7 @@ use papyrus_config::dumping::{
use papyrus_config::loading::load_and_process_config;
#[cfg(not(feature = "rpc"))]
use papyrus_config::ParamPrivacyInput;
use papyrus_config::{ConfigError, ParamPath, SerializedParam};
use papyrus_config::{ConfigError, ParamPath, SerializationType, SerializedParam};
use papyrus_monitoring_gateway::MonitoringGatewayConfig;
use papyrus_network::NetworkConfig;
use papyrus_p2p_sync::client::{P2pSyncClient, P2pSyncClientConfig};
Expand All @@ -51,9 +52,9 @@ use crate::version::VERSION_FULL;
pub static CONFIG_POINTERS: LazyLock<ConfigPointers> = LazyLock::new(|| {
vec![
(
ser_pointer_target_param(
ser_pointer_target_required_param(
"chain_id",
&ChainId::Mainnet,
SerializationType::String,
"The chain to follow. For more details see https://docs.starknet.io/documentation/architecture_and_concepts/Blocks/transactions/#chain-id.",
),
set_pointing_param_paths(&[
Expand Down
1 change: 1 addition & 0 deletions crates/papyrus_storage/src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl Default for DbConfig {
fn default() -> Self {
DbConfig {
path_prefix: PathBuf::from("./data"),
// TODO(guyn): should we remove the default for chain_id?
chain_id: ChainId::Mainnet,
enforce_file_exists: false,
min_size: 1 << 20, // 1MB
Expand Down
5 changes: 2 additions & 3 deletions crates/starknet_integration_tests/src/flow_test_setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use papyrus_storage::StorageConfig;
use starknet_api::rpc_transaction::RpcTransaction;
use starknet_api::transaction::TransactionHash;
use starknet_consensus_manager::config::ConsensusManagerConfig;
use starknet_consensus_manager::consensus_manager::CONSENSUS_PROPOSALS_TOPIC;
use starknet_gateway_types::errors::GatewaySpecError;
use starknet_http_server::config::HttpServerConfig;
use starknet_http_server::test_utils::HttpTestClient;
Expand Down Expand Up @@ -202,9 +203,7 @@ pub fn create_consensus_manager_configs_and_channels(
let channels_network_config = network_configs.pop().unwrap();
let broadcast_channels = network_config_into_broadcast_channels(
channels_network_config,
papyrus_network::gossipsub_impl::Topic::new(
starknet_consensus_manager::consensus_manager::CONSENSUS_PROPOSALS_TOPIC,
),
papyrus_network::gossipsub_impl::Topic::new(CONSENSUS_PROPOSALS_TOPIC),
);

let n_network_configs = network_configs.len();
Expand Down

0 comments on commit be32bfa

Please sign in to comment.