From 3cb71394591ce6d4adc9918398b3d76a9c40cbed Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 17 Mar 2025 16:23:47 +0100 Subject: [PATCH 01/15] refacto: Create macro to register config parameters --- Cargo.lock | 1 + .../src/commands/serve_command.rs | 277 +++++++++++++----- mithril-common/Cargo.toml | 1 + mithril-common/src/lib.rs | 1 + mithril-common/src/source_config.rs | 248 ++++++++++++++++ 5 files changed, 461 insertions(+), 67 deletions(-) create mode 100644 mithril-common/src/source_config.rs diff --git a/Cargo.lock b/Cargo.lock index adc327b1567..201bc3909da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3801,6 +3801,7 @@ dependencies = [ "chrono", "ciborium", "ckb-merkle-mountain-range", + "config", "criterion", "digest 0.10.7", "ed25519-dalek", diff --git a/mithril-aggregator/src/commands/serve_command.rs b/mithril-aggregator/src/commands/serve_command.rs index 1f07ba462f0..f07da78f9f1 100644 --- a/mithril-aggregator/src/commands/serve_command.rs +++ b/mithril-aggregator/src/commands/serve_command.rs @@ -11,7 +11,7 @@ use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind use slog::{crit, debug, info, warn, Logger}; use tokio::{sync::oneshot, task::JoinSet}; -use mithril_common::StdResult; +use mithril_common::{register, register_parameter_bool, register_parameter_opt, StdResult}; use mithril_metric::MetricsServer; use crate::{dependency_injection::DependenciesBuilder, tools::VacuumTracker, Configuration}; @@ -80,72 +80,27 @@ impl Source for ServeCommand { let mut result = Map::new(); let namespace = "clap arguments".to_string(); - if let Some(server_ip) = self.server_ip.clone() { - result.insert( - "server_ip".to_string(), - Value::new(Some(&namespace), ValueKind::from(server_ip)), - ); - } - if let Some(server_port) = self.server_port { - result.insert( - "server_port".to_string(), - Value::new(Some(&namespace), ValueKind::from(server_port)), - ); - } - if let Some(snapshot_directory) = self.snapshot_directory.clone() { - result.insert( - "snapshot_directory".to_string(), - Value::new( - Some(&namespace), - ValueKind::from(format!("{}", snapshot_directory.to_string_lossy())), - ), - ); - } - if self.disable_digests_cache { - result.insert( - "disable_digests_cache".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - }; - if self.reset_digests_cache { - result.insert( - "reset_digests_cache".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - } - if self.allow_unparsable_block { - result.insert( - "allow_unparsable_block".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - }; - if self.enable_metrics_server { - result.insert( - "enable_metrics_server".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - }; - if let Some(metrics_server_ip) = self.metrics_server_ip.clone() { - result.insert( - "metrics_server_ip".to_string(), - Value::new(Some(&namespace), ValueKind::from(metrics_server_ip)), - ); - } - if let Some(metrics_server_port) = self.metrics_server_port { - result.insert( - "metrics_server_port".to_string(), - Value::new(Some(&namespace), ValueKind::from(metrics_server_port)), - ); - } - if let Some(master_aggregator_endpoint) = self.master_aggregator_endpoint.clone() { - result.insert( - "master_aggregator_endpoint".to_string(), - Value::new( - Some(&namespace), - ValueKind::from(Some(master_aggregator_endpoint)), - ), - ); - } + register_parameter_opt!(result, &namespace, self.server_ip); + register_parameter_opt!(result, &namespace, self.server_port); + register_parameter_opt!( + result, + &namespace, + self.snapshot_directory, + |v: PathBuf| format!("{}", v.to_string_lossy()) + ); + register_parameter_bool!(result, &namespace, self.disable_digests_cache); + register_parameter_bool!(result, &namespace, self.reset_digests_cache); + register_parameter_bool!(result, &namespace, self.allow_unparsable_block); + register_parameter_bool!(result, &namespace, self.enable_metrics_server); + register_parameter_opt!(result, &namespace, self.metrics_server_ip); + register_parameter_opt!(result, &namespace, self.metrics_server_port); + // TODO is it normal to pass a Some(v) and not only v when value is present ? + register_parameter_opt!( + result, + &namespace, + self.master_aggregator_endpoint, + |v: String| { Some(v) } + ); Ok(result) } @@ -362,3 +317,191 @@ impl ServeCommand { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + + // TODO : just here to check there is no regression with the old configuration. + // We may remove it and probably all tests in this file when macros are finished + impl ServeCommand { + fn collect_legacy(&self) -> Result, config::ConfigError> { + let mut result = Map::new(); + let namespace = "clap arguments".to_string(); + + if let Some(server_ip) = self.server_ip.clone() { + result.insert( + "server_ip".to_string(), + Value::new(Some(&namespace), ValueKind::from(server_ip)), + ); + } + if let Some(server_port) = self.server_port { + result.insert( + "server_port".to_string(), + Value::new(Some(&namespace), ValueKind::from(server_port)), + ); + } + if let Some(snapshot_directory) = self.snapshot_directory.clone() { + result.insert( + "snapshot_directory".to_string(), + Value::new( + Some(&namespace), + ValueKind::from(format!("{}", snapshot_directory.to_string_lossy())), + ), + ); + } + if self.disable_digests_cache { + result.insert( + "disable_digests_cache".to_string(), + Value::new(Some(&namespace), ValueKind::from(true)), + ); + }; + if self.reset_digests_cache { + result.insert( + "reset_digests_cache".to_string(), + Value::new(Some(&namespace), ValueKind::from(true)), + ); + } + if self.allow_unparsable_block { + result.insert( + "allow_unparsable_block".to_string(), + Value::new(Some(&namespace), ValueKind::from(true)), + ); + }; + if self.enable_metrics_server { + result.insert( + "enable_metrics_server".to_string(), + Value::new(Some(&namespace), ValueKind::from(true)), + ); + }; + if let Some(metrics_server_ip) = self.metrics_server_ip.clone() { + result.insert( + "metrics_server_ip".to_string(), + Value::new(Some(&namespace), ValueKind::from(metrics_server_ip)), + ); + } + if let Some(metrics_server_port) = self.metrics_server_port { + result.insert( + "metrics_server_port".to_string(), + Value::new(Some(&namespace), ValueKind::from(metrics_server_port)), + ); + } + if let Some(master_aggregator_endpoint) = self.master_aggregator_endpoint.clone() { + result.insert( + "master_aggregator_endpoint".to_string(), + Value::new( + Some(&namespace), + ValueKind::from(Some(master_aggregator_endpoint)), + ), + ); + } + + Ok(result) + } + } + + #[test] + fn test_serve_command_collect() { + let serve_command = ServeCommand { + server_ip: Some("value_server_ip".to_string()), + server_port: Some(8000), + snapshot_directory: Some(PathBuf::from("/mithril/aggregator/")), + disable_digests_cache: true, + reset_digests_cache: true, + allow_unparsable_block: true, + enable_metrics_server: true, + metrics_server_ip: Some("value_metrics_server_ip".to_string()), + metrics_server_port: Some(8080), + master_aggregator_endpoint: Some("value_master_aggregator_endpoint".to_string()), + }; + + let result = serve_command.collect().unwrap().clone(); + let mut expected = HashMap::new(); + expected.insert( + "server_ip".to_string(), + Value::new( + Some(&"clap arguments".to_string()), + ValueKind::from("value_server_ip"), + ), + ); + expected.insert( + "server_port".to_string(), + Value::new( + Some(&"clap arguments".to_string()), + ValueKind::from(8000 as u64), + ), + ); + expected.insert( + "snapshot_directory".to_string(), + Value::new( + Some(&"clap arguments".to_string()), + ValueKind::from("/mithril/aggregator/"), + ), + ); + expected.insert( + "disable_digests_cache".to_string(), + Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), + ); + expected.insert( + "reset_digests_cache".to_string(), + Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), + ); + expected.insert( + "allow_unparsable_block".to_string(), + Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), + ); + expected.insert( + "enable_metrics_server".to_string(), + Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), + ); + expected.insert( + "metrics_server_ip".to_string(), + Value::new( + Some(&"clap arguments".to_string()), + ValueKind::from("value_metrics_server_ip"), + ), + ); + expected.insert( + "metrics_server_port".to_string(), + Value::new( + Some(&"clap arguments".to_string()), + ValueKind::from(Some(8080 as u64)), + ), + ); + expected.insert( + "master_aggregator_endpoint".to_string(), + Value::new( + Some(&"clap arguments".to_string()), + ValueKind::from("value_master_aggregator_endpoint"), + ), + ); + + assert_eq!(expected, result); + + assert_eq!(serve_command.collect_legacy().unwrap(), result); + } + + #[test] + fn test_serve_command_collect_when_empty_values() { + let serve_command = ServeCommand { + server_ip: None, + server_port: None, + snapshot_directory: None, + disable_digests_cache: false, + reset_digests_cache: false, + allow_unparsable_block: false, + enable_metrics_server: false, + metrics_server_ip: None, + metrics_server_port: None, + master_aggregator_endpoint: None, + }; + + let result = serve_command.collect().unwrap().clone(); + let expected = HashMap::new(); + + assert_eq!(expected, result); + + assert_eq!(serve_command.collect_legacy().unwrap(), result); + } +} diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index 0a8890c862e..6ebeabff689 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -36,6 +36,7 @@ bech32 = "0.11.0" blake2 = "0.10.6" chrono = { version = "0.4.39", features = ["serde"] } ciborium = "0.2.2" +config = "0.15.7" ckb-merkle-mountain-range = "0.6.0" digest = "0.10.7" ed25519-dalek = { version = "2.1.1", features = ["rand_core", "serde"] } diff --git a/mithril-common/src/lib.rs b/mithril-common/src/lib.rs index 353f4082cc2..3227516e66c 100644 --- a/mithril-common/src/lib.rs +++ b/mithril-common/src/lib.rs @@ -41,6 +41,7 @@ pub mod logging; pub mod messages; pub mod protocol; pub mod signable_builder; +pub mod source_config; cfg_test_tools! { pub mod test_utils; diff --git a/mithril-common/src/source_config.rs b/mithril-common/src/source_config.rs new file mode 100644 index 00000000000..f6c91bc940e --- /dev/null +++ b/mithril-common/src/source_config.rs @@ -0,0 +1,248 @@ +//! Utilities to register config parameters. + +/// Register a parameter in the config map. +#[macro_export] +macro_rules! register { + ( $map:ident, $namespace:expr, $command: ident, $value:expr ) => {{ + $map.insert( + stringify!($command).to_string(), + config::Value::new(Some($namespace), $value), + ); + }}; +} + +/// Register a optional parameter in the config map. +#[macro_export] +macro_rules! register_parameter_opt { + ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ + if let Some(value) = $self.$command.clone() { + register!($map, $namespace, $command, value); + } + }}; + ( $map:ident, $namespace:expr, $self:ident.$command:ident, $mapping:expr ) => {{ + if let Some(value) = $self.$command.clone() { + register!($map, $namespace, $command, $mapping(value)); + } + }}; +} + +/// Register a boolean parameter in the config map. +#[macro_export] +macro_rules! register_parameter_bool { + ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ + if $self.$command { + register!($map, $namespace, $command, true); + } + }}; +} + +/// Register a parameter in the config map. +#[macro_export] +macro_rules! register_parameter { + ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ + register!($map, $namespace, $command, $self.$command); + }}; +} + +#[cfg(test)] +mod tests { + use config::{ConfigError, Map, Source, Value, ValueKind}; + use std::collections::HashMap; + + #[test] + fn test_register_macro() { + let mut map = HashMap::new(); + register!( + map, + &"namespace".to_string(), + server_ip, + Some("value_server_ip".to_string()) + ); + + let expected = HashMap::from([( + "server_ip".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("value_server_ip"), + ), + )]); + + assert_eq!(expected, map); + } + + #[test] + fn test_register_parameter_macro_add_value() { + struct Fake { + string_value: String, + u64_value: u64, + } + + let fake = Fake { + string_value: "a string value".to_string(), + u64_value: 124, + }; + + let mut map = HashMap::new(); + register_parameter!(map, &"namespace".to_string(), fake.string_value); + register_parameter!(map, &"namespace".to_string(), fake.u64_value); + + let expected = HashMap::from([ + ( + "string_value".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("a string value"), + ), + ), + ( + "u64_value".to_string(), + Value::new(Some(&"namespace".to_string()), ValueKind::from(124 as u64)), + ), + ]); + + assert_eq!(expected, map); + } + + #[test] + fn test_register_parameter_option_macro_not_add_none_value() { + struct Fake { + option_with_value: Option, + option_none: Option, + } + + let fake = Fake { + option_with_value: Some("value_of_option".to_string()), + option_none: None, + }; + + let mut map = HashMap::new(); + register_parameter_opt!(map, &"namespace".to_string(), fake.option_with_value); + register_parameter_opt!(map, &"namespace".to_string(), fake.option_none); + + let expected = HashMap::from([( + "option_with_value".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("value_of_option"), + ), + )]); + + assert_eq!(expected, map); + } + + #[test] + fn test_register_parameter_option_macro_with_mapping_transform_value_before_adding_it() { + struct Fake { + option_with_value: Option, + option_none: Option, + } + + let fake = Fake { + option_with_value: Some("value_of_option".to_string()), + option_none: None, + }; + + let mut map = HashMap::new(); + register_parameter_opt!( + map, + &"namespace".to_string(), + fake.option_with_value, + |v: String| format!("mapped_value from {}", v) + ); + register_parameter_opt!(map, &"namespace".to_string(), fake.option_none); + + let expected = HashMap::from([( + "option_with_value".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("mapped_value from value_of_option"), + ), + )]); + + assert_eq!(expected, map); + } + + #[test] + fn test_register_parameter_bool_macro_add_only_true_values() { + struct Fake { + bool_true: bool, + bool_false: bool, + } + + let fake = Fake { + bool_true: true, + bool_false: false, + }; + + let mut map = HashMap::new(); + register_parameter_bool!(map, &"namespace".to_string(), fake.bool_true); + register_parameter_bool!(map, &"namespace".to_string(), fake.bool_false); + + let expected = HashMap::from([( + "bool_true".to_string(), + Value::new(Some(&"namespace".to_string()), ValueKind::from(true)), + )]); + + assert_eq!(expected, map); + } + + #[test] + fn test_register_parameter_macro() { + struct Fake { + server_ip: Option, + } + + let fake = Fake { + server_ip: Some("value_server_ip".to_string()), + }; + + let mut map = HashMap::new(); + register_parameter!(map, &"namespace".to_string(), fake.server_ip); + + let expected = HashMap::from([( + "server_ip".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("value_server_ip"), + ), + )]); + + assert_eq!(expected, map); + } + + #[test] + fn test_collect_source_values() { + #[derive(Debug, Clone)] + struct Fake { + server_ip: Option, + } + + let fake = Fake { + server_ip: Some("value_server_ip".to_string()), + }; + + impl Source for Fake { + fn collect(&self) -> Result, ConfigError> { + let mut map = Map::new(); + register_parameter_opt!(map, &"namespace".to_string(), self.server_ip); + Ok(map) + } + + fn clone_into_box(&self) -> Box { + Box::new(self.clone()) + } + } + + let result = fake.collect().unwrap().clone(); + + let expected = HashMap::from([( + "server_ip".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("value_server_ip"), + ), + )]); + + assert_eq!(expected, result); + } +} From 5751d079d94c14f810383fd39541c226812eec52 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 09:06:20 +0100 Subject: [PATCH 02/15] refacto: extend register_parameter with mapping --- mithril-common/src/source_config.rs | 100 +++++++++++++++++++++++++--- 1 file changed, 91 insertions(+), 9 deletions(-) diff --git a/mithril-common/src/source_config.rs b/mithril-common/src/source_config.rs index f6c91bc940e..5c07531eb1b 100644 --- a/mithril-common/src/source_config.rs +++ b/mithril-common/src/source_config.rs @@ -42,6 +42,9 @@ macro_rules! register_parameter { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ register!($map, $namespace, $command, $self.$command); }}; + ( $map:ident, $namespace:expr, $self:ident.$command:ident, $mapping:expr ) => {{ + register!($map, $namespace, $command, $mapping($self.$command)); + }}; } #[cfg(test)] @@ -70,21 +73,38 @@ mod tests { assert_eq!(expected, map); } + // #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] + + #[derive(Debug, Clone)] + pub enum EnumValue { + Test, + } + impl From for ValueKind { + fn from(value: EnumValue) -> Self { + match value { + EnumValue::Test => ValueKind::String("Test".to_string()), + } + } + } + #[test] fn test_register_parameter_macro_add_value() { struct Fake { string_value: String, u64_value: u64, + enum_value: EnumValue, } let fake = Fake { string_value: "a string value".to_string(), u64_value: 124, + enum_value: EnumValue::Test, }; let mut map = HashMap::new(); register_parameter!(map, &"namespace".to_string(), fake.string_value); register_parameter!(map, &"namespace".to_string(), fake.u64_value); + register_parameter!(map, &"namespace".to_string(), fake.enum_value); let expected = HashMap::from([ ( @@ -98,11 +118,44 @@ mod tests { "u64_value".to_string(), Value::new(Some(&"namespace".to_string()), ValueKind::from(124 as u64)), ), + ( + "enum_value".to_string(), + Value::new(Some(&"namespace".to_string()), ValueKind::from("Test")), + ), ]); assert_eq!(expected, map); } + #[test] + fn test_register_parameter_macro_with_mapping_transform_value_before_adding_it() { + struct Fake { + string_value: String, + } + + let fake = Fake { + string_value: "a string value".to_string(), + }; + + let mut map = HashMap::new(); + register_parameter!( + map, + &"namespace".to_string(), + fake.string_value, + |v: String| { format!("mapped_value from {}", v) } + ); + + let expected = HashMap::from([( + "string_value".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("mapped_value from a string value"), + ), + )]); + + assert_eq!(expected, map); + } + #[test] fn test_register_parameter_option_macro_not_add_none_value() { struct Fake { @@ -214,17 +267,29 @@ mod tests { fn test_collect_source_values() { #[derive(Debug, Clone)] struct Fake { - server_ip: Option, + option_value: Option, + string_value: String, + u64_value: u64, + enum_value: EnumValue, } let fake = Fake { - server_ip: Some("value_server_ip".to_string()), + option_value: Some("option value".to_string()), + string_value: "a string value".to_string(), + u64_value: 124, + enum_value: EnumValue::Test, }; impl Source for Fake { fn collect(&self) -> Result, ConfigError> { let mut map = Map::new(); - register_parameter_opt!(map, &"namespace".to_string(), self.server_ip); + + let myself = self.clone(); + register_parameter_opt!(map, &"namespace".to_string(), myself.option_value); + register_parameter!(map, &"namespace".to_string(), myself.string_value); + register_parameter!(map, &"namespace".to_string(), myself.u64_value); + register_parameter!(map, &"namespace".to_string(), myself.enum_value); + Ok(map) } @@ -235,13 +300,30 @@ mod tests { let result = fake.collect().unwrap().clone(); - let expected = HashMap::from([( - "server_ip".to_string(), - Value::new( - Some(&"namespace".to_string()), - ValueKind::from("value_server_ip"), + let expected = HashMap::from([ + ( + "option_value".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("option value"), + ), ), - )]); + ( + "string_value".to_string(), + Value::new( + Some(&"namespace".to_string()), + ValueKind::from("a string value"), + ), + ), + ( + "u64_value".to_string(), + Value::new(Some(&"namespace".to_string()), ValueKind::from(124 as u64)), + ), + ( + "enum_value".to_string(), + Value::new(Some(&"namespace".to_string()), ValueKind::from("Test")), + ), + ]); assert_eq!(expected, result); } From a20721eabd3cc37abb71579d075125911e13568b Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 09:07:04 +0100 Subject: [PATCH 03/15] refacto: use `register_parameter`on configuration and mod aggregator --- mithril-aggregator/src/commands/mod.rs | 80 +++++++- mithril-aggregator/src/configuration.rs | 259 +++++++++++++++++++----- 2 files changed, 281 insertions(+), 58 deletions(-) diff --git a/mithril-aggregator/src/commands/mod.rs b/mithril-aggregator/src/commands/mod.rs index 864cd16f9f0..48c55e7fe6e 100644 --- a/mithril-aggregator/src/commands/mod.rs +++ b/mithril-aggregator/src/commands/mod.rs @@ -7,7 +7,7 @@ mod tools_command; use anyhow::anyhow; use clap::{CommandFactory, Parser, Subcommand}; use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; -use mithril_common::StdResult; +use mithril_common::{register, register_parameter_opt, StdResult}; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use slog::{debug, Level, Logger}; use std::path::PathBuf; @@ -102,15 +102,11 @@ impl Source for MainOpts { let mut result = Map::new(); let namespace = "clap arguments".to_string(); - if let Some(db_directory) = self.db_directory.clone() { - result.insert( - "db_directory".to_string(), - Value::new( - Some(&namespace), - ValueKind::from(format!("{}", db_directory.to_string_lossy())), - ), - ); - } + // TODO Is it normal to only have db_directory ? + register_parameter_opt!(result, &namespace, self.db_directory, |v: PathBuf| format!( + "{}", + v.to_string_lossy() + )); Ok(result) } @@ -145,3 +141,67 @@ impl MainOpts { } } } + +#[cfg(test)] +mod tests { + use crate::commands::tools_command::{ + RecomputeCertificatesHashCommand, ToolsCommand, ToolsSubCommand::RecomputeCertificatesHash, + }; + + use super::*; + + // TODO : just here to check there is no regression with the old configuration. + // We may remove it and probably all tests in this file when macros are finished + impl MainOpts { + fn collect_legacy(&self) -> Result, config::ConfigError> { + let mut result = Map::new(); + let namespace = "clap arguments".to_string(); + + if let Some(db_directory) = self.db_directory.clone() { + result.insert( + "db_directory".to_string(), + Value::new( + Some(&namespace), + ValueKind::from(format!("{}", db_directory.to_string_lossy())), + ), + ); + } + + Ok(result) + } + } + + #[test] + fn test_main_opts_collect() { + let serve_command = MainOpts { + db_directory: Some(PathBuf::from("/db_directory")), + command: MainCommand::Tools(ToolsCommand { + genesis_subcommand: RecomputeCertificatesHash(RecomputeCertificatesHashCommand {}), + }), + run_mode: "mode".to_string(), + verbose: 1, + config_directory: PathBuf::from(""), + }; + + let result = serve_command.collect().unwrap().clone(); + + assert_eq!(serve_command.collect_legacy().unwrap(), result); + } + + #[test] + fn test_main_opts_collect_when_empty_values() { + let serve_command = MainOpts { + db_directory: None, + command: MainCommand::Tools(ToolsCommand { + genesis_subcommand: RecomputeCertificatesHash(RecomputeCertificatesHashCommand {}), + }), + run_mode: "mode".to_string(), + verbose: 1, + config_directory: PathBuf::from(""), + }; + + let result = serve_command.collect().unwrap().clone(); + + assert_eq!(serve_command.collect_legacy().unwrap(), result); + } +} diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 466d7addf94..d0a6716ebe1 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -13,7 +13,7 @@ use mithril_common::entities::{ SignedEntityTypeDiscriminants, }; use mithril_common::era::adapters::EraReaderAdapterType; -use mithril_common::{CardanoNetwork, StdResult}; +use mithril_common::{register, register_parameter, CardanoNetwork, StdResult}; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use crate::entities::AggregatorEpochSettings; @@ -525,67 +525,64 @@ impl Source for DefaultConfiguration { } fn collect(&self) -> Result, ConfigError> { - macro_rules! insert_default_configuration { - ( $map:ident, $config:ident.$parameter:ident ) => {{ - $map.insert( - stringify!($parameter).to_string(), - into_value($config.$parameter), - ); - }}; - } - - fn into_value>(value: V) -> Value { - Value::new(Some(&DefaultConfiguration::namespace()), value.into()) - } let mut result = Map::new(); + + let namespace = DefaultConfiguration::namespace(); + let myself = self.clone(); - insert_default_configuration!(result, myself.environment); - insert_default_configuration!(result, myself.server_ip); - insert_default_configuration!(result, myself.server_port); - insert_default_configuration!(result, myself.db_directory); - insert_default_configuration!(result, myself.snapshot_directory); - insert_default_configuration!(result, myself.snapshot_store_type); - insert_default_configuration!(result, myself.snapshot_uploader_type); - insert_default_configuration!(result, myself.era_reader_adapter_type); - insert_default_configuration!(result, myself.reset_digests_cache); - insert_default_configuration!(result, myself.disable_digests_cache); - insert_default_configuration!(result, myself.snapshot_compression_algorithm); - insert_default_configuration!(result, myself.snapshot_use_cdn_domain); - insert_default_configuration!(result, myself.signer_importer_run_interval); - insert_default_configuration!(result, myself.allow_unparsable_block); - insert_default_configuration!(result, myself.cardano_transactions_prover_cache_pool_size); - insert_default_configuration!( + register_parameter!(result, &namespace, myself.environment); + register_parameter!(result, &namespace, myself.server_ip); + register_parameter!(result, &namespace, myself.server_port); + register_parameter!(result, &namespace, myself.db_directory); + register_parameter!(result, &namespace, myself.snapshot_directory); + register_parameter!(result, &namespace, myself.snapshot_store_type); + register_parameter!(result, &namespace, myself.snapshot_uploader_type); + register_parameter!(result, &namespace, myself.era_reader_adapter_type); + register_parameter!(result, &namespace, myself.reset_digests_cache); + register_parameter!(result, &namespace, myself.disable_digests_cache); + register_parameter!(result, &namespace, myself.snapshot_compression_algorithm); + register_parameter!(result, &namespace, myself.snapshot_use_cdn_domain); + register_parameter!(result, &namespace, myself.signer_importer_run_interval); + register_parameter!(result, &namespace, myself.allow_unparsable_block); + register_parameter!( result, + &namespace, + myself.cardano_transactions_prover_cache_pool_size + ); + register_parameter!( + result, + &namespace, myself.cardano_transactions_database_connection_pool_size ); - insert_default_configuration!( + register_parameter!( result, + &namespace, myself.cardano_transactions_prover_max_hashes_allowed_by_request ); - insert_default_configuration!( + register_parameter!( result, + &namespace, myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll ); - insert_default_configuration!(result, myself.enable_metrics_server); - insert_default_configuration!(result, myself.metrics_server_ip); - insert_default_configuration!(result, myself.metrics_server_port); - insert_default_configuration!(result, myself.persist_usage_report_interval_in_seconds); - result.insert( - "cardano_transactions_signing_config".to_string(), - into_value(HashMap::from([ + register_parameter!(result, &namespace, myself.enable_metrics_server); + register_parameter!(result, &namespace, myself.metrics_server_ip); + register_parameter!(result, &namespace, myself.metrics_server_port); + register_parameter!( + result, + &namespace, + myself.persist_usage_report_interval_in_seconds + ); + register_parameter!( + result, + &namespace, + myself.cardano_transactions_signing_config, + |v: CardanoTransactionsSigningConfig| HashMap::from([ ( "security_parameter".to_string(), - ValueKind::from( - *myself - .cardano_transactions_signing_config - .security_parameter, - ), + ValueKind::from(*v.security_parameter,), ), - ( - "step".to_string(), - ValueKind::from(*myself.cardano_transactions_signing_config.step), - ), - ])), + ("step".to_string(), ValueKind::from(*v.step),) + ]) ); Ok(result) } @@ -597,6 +594,172 @@ mod test { use super::*; + // TODO : just here to check there is no regression with the old configuration. + // We may remove it and probably all tests in this file when macros are finished + impl DefaultConfiguration { + fn collect_legacy(&self) -> Result, ConfigError> { + macro_rules! insert_default_configuration { + ( $map:ident, $config:ident.$parameter:ident ) => {{ + $map.insert( + stringify!($parameter).to_string(), + into_value($config.$parameter), + ); + }}; + } + + fn into_value>(value: V) -> Value { + Value::new(Some(&DefaultConfiguration::namespace()), value.into()) + } + let mut result = Map::new(); + let myself = self.clone(); + + result.insert( + "cardano_transactions_signing_config".to_string(), + into_value(HashMap::from([ + ( + "security_parameter".to_string(), + ValueKind::from( + *myself + .cardano_transactions_signing_config + .security_parameter, + ), + ), + ( + "step".to_string(), + ValueKind::from(*myself.cardano_transactions_signing_config.step), + ), + ])), + ); + + insert_default_configuration!(result, myself.environment); + insert_default_configuration!(result, myself.server_ip); + insert_default_configuration!(result, myself.server_port); + insert_default_configuration!(result, myself.db_directory); + insert_default_configuration!(result, myself.snapshot_directory); + insert_default_configuration!(result, myself.snapshot_store_type); + insert_default_configuration!(result, myself.snapshot_uploader_type); + insert_default_configuration!(result, myself.era_reader_adapter_type); + insert_default_configuration!(result, myself.reset_digests_cache); + insert_default_configuration!(result, myself.disable_digests_cache); + insert_default_configuration!(result, myself.snapshot_compression_algorithm); + insert_default_configuration!(result, myself.snapshot_use_cdn_domain); + insert_default_configuration!(result, myself.signer_importer_run_interval); + insert_default_configuration!(result, myself.allow_unparsable_block); + insert_default_configuration!( + result, + myself.cardano_transactions_prover_cache_pool_size + ); + insert_default_configuration!( + result, + myself.cardano_transactions_database_connection_pool_size + ); + insert_default_configuration!( + result, + myself.cardano_transactions_prover_max_hashes_allowed_by_request + ); + insert_default_configuration!( + result, + myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll + ); + insert_default_configuration!(result, myself.enable_metrics_server); + insert_default_configuration!(result, myself.metrics_server_ip); + insert_default_configuration!(result, myself.metrics_server_port); + insert_default_configuration!(result, myself.persist_usage_report_interval_in_seconds); + result.insert( + "cardano_transactions_signing_config".to_string(), + into_value(HashMap::from([ + ( + "security_parameter".to_string(), + ValueKind::from( + *myself + .cardano_transactions_signing_config + .security_parameter, + ), + ), + ( + "step".to_string(), + ValueKind::from(*myself.cardano_transactions_signing_config.step), + ), + ])), + ); + Ok(result) + } + } + + #[test] + fn test_default_configuration_collect() { + let default_configuration = DefaultConfiguration { + environment: ExecutionEnvironment::Production, + server_ip: "0.0.0.0".to_string(), + server_port: "8080".to_string(), + db_directory: "/db".to_string(), + snapshot_directory: ".".to_string(), + snapshot_store_type: "local".to_string(), + snapshot_uploader_type: "gcp".to_string(), + era_reader_adapter_type: "bootstrap".to_string(), + chain_observer_type: "pallas".to_string(), + reset_digests_cache: "true".to_string(), + disable_digests_cache: "true".to_string(), + snapshot_compression_algorithm: "zstandard".to_string(), + snapshot_use_cdn_domain: "true".to_string(), + signer_importer_run_interval: 720, + allow_unparsable_block: "true".to_string(), + cardano_transactions_prover_cache_pool_size: 10, + cardano_transactions_database_connection_pool_size: 10, + cardano_transactions_signing_config: CardanoTransactionsSigningConfig { + security_parameter: BlockNumber(3000), + step: BlockNumber(120), + }, + cardano_transactions_prover_max_hashes_allowed_by_request: 100, + cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, + enable_metrics_server: "true".to_string(), + metrics_server_ip: "0.0.0.0".to_string(), + metrics_server_port: 9090, + persist_usage_report_interval_in_seconds: 10, + }; + + let result = default_configuration.collect().unwrap().clone(); + + assert_eq!(default_configuration.collect_legacy().unwrap(), result); + } + + #[test] + fn test_default_configuration_collect_when_empty_values() { + let default_configuration = DefaultConfiguration { + environment: ExecutionEnvironment::Production, + server_ip: "0.0.0.0".to_string(), + server_port: "8080".to_string(), + db_directory: "/db".to_string(), + snapshot_directory: ".".to_string(), + snapshot_store_type: "local".to_string(), + snapshot_uploader_type: "gcp".to_string(), + era_reader_adapter_type: "bootstrap".to_string(), + chain_observer_type: "pallas".to_string(), + reset_digests_cache: "false".to_string(), + disable_digests_cache: "false".to_string(), + snapshot_compression_algorithm: "zstandard".to_string(), + snapshot_use_cdn_domain: "false".to_string(), + signer_importer_run_interval: 720, + allow_unparsable_block: "false".to_string(), + cardano_transactions_prover_cache_pool_size: 10, + cardano_transactions_database_connection_pool_size: 10, + cardano_transactions_signing_config: CardanoTransactionsSigningConfig { + security_parameter: BlockNumber(3000), + step: BlockNumber(120), + }, + cardano_transactions_prover_max_hashes_allowed_by_request: 100, + cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, + enable_metrics_server: "false".to_string(), + metrics_server_ip: "0.0.0.0".to_string(), + metrics_server_port: 9090, + persist_usage_report_interval_in_seconds: 10, + }; + + let result = default_configuration.collect().unwrap().clone(); + + assert_eq!(default_configuration.collect_legacy().unwrap(), result); + } + #[test] fn safe_epoch_retention_limit_wont_change_a_value_higher_than_three() { for limit in 4..=10u64 { From 4550a516e1873b2b15737b541ce68597813c766c Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 09:19:52 +0100 Subject: [PATCH 04/15] refacto: use `register_parameter` for signer configuration --- mithril-signer/src/configuration.rs | 111 ++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 21 deletions(-) diff --git a/mithril-signer/src/configuration.rs b/mithril-signer/src/configuration.rs index 47fe3040e54..0a21d132178 100644 --- a/mithril-signer/src/configuration.rs +++ b/mithril-signer/src/configuration.rs @@ -12,7 +12,7 @@ use mithril_common::{ adapters::{EraReaderAdapterBuilder, EraReaderAdapterType}, EraReaderAdapter, }, - CardanoNetwork, StdResult, + register, register_parameter, CardanoNetwork, StdResult, }; /// Client configuration @@ -262,33 +262,102 @@ impl Source for DefaultConfiguration { } fn collect(&self) -> Result, ConfigError> { - macro_rules! insert_default_configuration { - ( $map:ident, $config:ident.$parameter:ident ) => {{ - $map.insert( - stringify!($parameter).to_string(), - into_value($config.$parameter), - ); - }}; - } - - fn into_value>(value: V) -> Value { - Value::new(Some(&DefaultConfiguration::namespace()), value.into()) - } let mut result = Map::new(); + let namespace = DefaultConfiguration::namespace(); let myself = self.clone(); - insert_default_configuration!(result, myself.era_reader_adapter_type); - insert_default_configuration!(result, myself.metrics_server_ip); - insert_default_configuration!(result, myself.metrics_server_port); - insert_default_configuration!(result, myself.network_security_parameter); - insert_default_configuration!(result, myself.preload_security_parameter); - insert_default_configuration!(result, myself.enable_transaction_pruning); - insert_default_configuration!(result, myself.transactions_import_block_chunk_size); - insert_default_configuration!( + register_parameter!(result, &namespace, myself.era_reader_adapter_type); + register_parameter!(result, &namespace, myself.metrics_server_ip); + register_parameter!(result, &namespace, myself.metrics_server_port); + register_parameter!(result, &namespace, myself.network_security_parameter); + register_parameter!(result, &namespace, myself.preload_security_parameter); + register_parameter!(result, &namespace, myself.enable_transaction_pruning); + register_parameter!( + result, + &namespace, + myself.transactions_import_block_chunk_size + ); + register_parameter!( result, + &namespace, myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll ); Ok(result) } } + +#[cfg(test)] +mod test { + + use super::*; + + // TODO : just here to check there is no regression with the old configuration. + // We may remove it and probably all tests in this file when macros are finished + impl DefaultConfiguration { + fn collect_legacy(&self) -> Result, ConfigError> { + macro_rules! insert_default_configuration { + ( $map:ident, $config:ident.$parameter:ident ) => {{ + $map.insert( + stringify!($parameter).to_string(), + into_value($config.$parameter), + ); + }}; + } + + fn into_value>(value: V) -> Value { + Value::new(Some(&DefaultConfiguration::namespace()), value.into()) + } + let mut result = Map::new(); + let myself = self.clone(); + + insert_default_configuration!(result, myself.era_reader_adapter_type); + insert_default_configuration!(result, myself.metrics_server_ip); + insert_default_configuration!(result, myself.metrics_server_port); + insert_default_configuration!(result, myself.network_security_parameter); + insert_default_configuration!(result, myself.preload_security_parameter); + insert_default_configuration!(result, myself.enable_transaction_pruning); + insert_default_configuration!(result, myself.transactions_import_block_chunk_size); + insert_default_configuration!( + result, + myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll + ); + + Ok(result) + } + } + + #[test] + fn test_default_configuration_collect() { + let default_configuration = DefaultConfiguration { + era_reader_adapter_type: "bootstrap".to_string(), + metrics_server_ip: "0.0.0.0".to_string(), + metrics_server_port: 9090, + network_security_parameter: 2160, + preload_security_parameter: 1000, + enable_transaction_pruning: true, + transactions_import_block_chunk_size: 1500, + cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, + }; + let result = default_configuration.collect().unwrap().clone(); + + assert_eq!(default_configuration.collect_legacy().unwrap(), result); + } + + #[test] + fn test_default_configuration_collect_with_empty_values() { + let default_configuration = DefaultConfiguration { + era_reader_adapter_type: "bootstrap".to_string(), + metrics_server_ip: "0.0.0.0".to_string(), + metrics_server_port: 9090, + network_security_parameter: 2160, + preload_security_parameter: 1000, + enable_transaction_pruning: false, + transactions_import_block_chunk_size: 1500, + cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, + }; + let result = default_configuration.collect().unwrap().clone(); + + assert_eq!(default_configuration.collect_legacy().unwrap(), result); + } +} From 4a090db22db23998b2c2e84cbf241f9185480a9d Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 10:20:26 +0100 Subject: [PATCH 05/15] refacto: Update source_config documentation --- mithril-common/src/source_config.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mithril-common/src/source_config.rs b/mithril-common/src/source_config.rs index 5c07531eb1b..dda6ded7c41 100644 --- a/mithril-common/src/source_config.rs +++ b/mithril-common/src/source_config.rs @@ -11,7 +11,7 @@ macro_rules! register { }}; } -/// Register a optional parameter in the config map. +/// Register a optional parameter in the config map when it's not None. #[macro_export] macro_rules! register_parameter_opt { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ @@ -26,7 +26,7 @@ macro_rules! register_parameter_opt { }}; } -/// Register a boolean parameter in the config map. +/// Register a boolean parameter in the config map only when it's true. #[macro_export] macro_rules! register_parameter_bool { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ @@ -73,8 +73,6 @@ mod tests { assert_eq!(expected, map); } - // #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] - #[derive(Debug, Clone)] pub enum EnumValue { Test, From c77326ac5de03a83ab5f1978b4b9b454abd25de2 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 11:04:41 +0100 Subject: [PATCH 06/15] refacto: Create a `mithril-cli-helper` crate and move macros to register config parameter within it --- Cargo.lock | 10 +++++++++- Cargo.toml | 1 + internal/mithril-cli-helper/Cargo.toml | 16 ++++++++++++++++ internal/mithril-cli-helper/Makefile | 19 +++++++++++++++++++ internal/mithril-cli-helper/README.md | 3 +++ internal/mithril-cli-helper/src/lib.rs | 1 + .../mithril-cli-helper}/src/source_config.rs | 4 ++-- mithril-aggregator/Cargo.toml | 1 + mithril-aggregator/src/commands/mod.rs | 7 +++++-- .../src/commands/serve_command.rs | 13 +++++++++---- mithril-aggregator/src/configuration.rs | 3 ++- mithril-common/Cargo.toml | 1 - mithril-common/src/lib.rs | 1 - mithril-signer/Cargo.toml | 1 + mithril-signer/src/configuration.rs | 7 +++++-- 15 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 internal/mithril-cli-helper/Cargo.toml create mode 100644 internal/mithril-cli-helper/Makefile create mode 100644 internal/mithril-cli-helper/README.md create mode 100644 internal/mithril-cli-helper/src/lib.rs rename {mithril-common => internal/mithril-cli-helper}/src/source_config.rs (99%) diff --git a/Cargo.lock b/Cargo.lock index 201bc3909da..554277a140a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3652,6 +3652,7 @@ dependencies = [ "hex", "http 1.2.0", "httpmock", + "mithril-cli-helper", "mithril-common", "mithril-doc", "mithril-metric", @@ -3714,6 +3715,13 @@ dependencies = [ "serde_yaml", ] +[[package]] +name = "mithril-cli-helper" +version = "0.0.1" +dependencies = [ + "config", +] + [[package]] name = "mithril-client" version = "0.11.16" @@ -3801,7 +3809,6 @@ dependencies = [ "chrono", "ciborium", "ckb-merkle-mountain-range", - "config", "criterion", "digest 0.10.7", "ed25519-dalek", @@ -3988,6 +3995,7 @@ dependencies = [ "hex", "http 1.2.0", "httpmock", + "mithril-cli-helper", "mithril-common", "mithril-doc", "mithril-metric", diff --git a/Cargo.toml b/Cargo.toml index 9dfcc551c62..86fec229814 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ members = [ "examples/client-cardano-transaction", "examples/client-mithril-stake-distribution", "internal/mithril-build-script", + "internal/mithril-cli-helper", "internal/mithril-doc", "internal/mithril-doc-derive", "internal/mithril-metric", diff --git a/internal/mithril-cli-helper/Cargo.toml b/internal/mithril-cli-helper/Cargo.toml new file mode 100644 index 00000000000..3cff971637e --- /dev/null +++ b/internal/mithril-cli-helper/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "mithril-cli-helper" +version = "0.0.1" +description = "An internal crate to provide tools for cli." +authors = { workspace = true } +edition = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +repository = { workspace = true } +include = ["**/*.rs", "Cargo.toml", "README.md", ".gitignore"] + +[dependencies] +config = "0.15.7" + +[features] +default = [] diff --git a/internal/mithril-cli-helper/Makefile b/internal/mithril-cli-helper/Makefile new file mode 100644 index 00000000000..adc3e3d5c11 --- /dev/null +++ b/internal/mithril-cli-helper/Makefile @@ -0,0 +1,19 @@ +.PHONY: all build test check doc + +CARGO = cargo + +all: test build + +build: + ${CARGO} build --release + +test: + ${CARGO} test + +check: + ${CARGO} check --release --all-features --all-targets + ${CARGO} clippy --release --all-features --all-targets + ${CARGO} fmt --check + +doc: + ${CARGO} doc --no-deps --open --features full diff --git a/internal/mithril-cli-helper/README.md b/internal/mithril-cli-helper/README.md new file mode 100644 index 00000000000..e38e4f34079 --- /dev/null +++ b/internal/mithril-cli-helper/README.md @@ -0,0 +1,3 @@ +# Mithril-cli-helper + +An internal crate to provide tools for Mithril clients. diff --git a/internal/mithril-cli-helper/src/lib.rs b/internal/mithril-cli-helper/src/lib.rs new file mode 100644 index 00000000000..7b487ff7e1a --- /dev/null +++ b/internal/mithril-cli-helper/src/lib.rs @@ -0,0 +1 @@ +pub mod source_config; diff --git a/mithril-common/src/source_config.rs b/internal/mithril-cli-helper/src/source_config.rs similarity index 99% rename from mithril-common/src/source_config.rs rename to internal/mithril-cli-helper/src/source_config.rs index dda6ded7c41..8bbf2087db8 100644 --- a/mithril-common/src/source_config.rs +++ b/internal/mithril-cli-helper/src/source_config.rs @@ -114,7 +114,7 @@ mod tests { ), ( "u64_value".to_string(), - Value::new(Some(&"namespace".to_string()), ValueKind::from(124 as u64)), + Value::new(Some(&"namespace".to_string()), ValueKind::from(124_u64)), ), ( "enum_value".to_string(), @@ -315,7 +315,7 @@ mod tests { ), ( "u64_value".to_string(), - Value::new(Some(&"namespace".to_string()), ValueKind::from(124 as u64)), + Value::new(Some(&"namespace".to_string()), ValueKind::from(124_u64)), ), ( "enum_value".to_string(), diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index d07b902fe7d..2a2d4936748 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -26,6 +26,7 @@ cloud-storage = "0.11.1" config = "0.15.7" flate2 = "1.0.35" hex = "0.4.3" +mithril-cli-helper = { path = "../internal/mithril-cli-helper" } mithril-common = { path = "../mithril-common", features = ["full"] } mithril-doc = { path = "../internal/mithril-doc" } mithril-metric = { path = "../internal/mithril-metric" } diff --git a/mithril-aggregator/src/commands/mod.rs b/mithril-aggregator/src/commands/mod.rs index 48c55e7fe6e..e306c1c782d 100644 --- a/mithril-aggregator/src/commands/mod.rs +++ b/mithril-aggregator/src/commands/mod.rs @@ -6,8 +6,9 @@ mod tools_command; use anyhow::anyhow; use clap::{CommandFactory, Parser, Subcommand}; -use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; -use mithril_common::{register, register_parameter_opt, StdResult}; +use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; +use mithril_cli_helper::{register, register_parameter_opt}; +use mithril_common::StdResult; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use slog::{debug, Level, Logger}; use std::path::PathBuf; @@ -144,6 +145,8 @@ impl MainOpts { #[cfg(test)] mod tests { + use config::ValueKind; + use crate::commands::tools_command::{ RecomputeCertificatesHashCommand, ToolsCommand, ToolsSubCommand::RecomputeCertificatesHash, }; diff --git a/mithril-aggregator/src/commands/serve_command.rs b/mithril-aggregator/src/commands/serve_command.rs index f07da78f9f1..e9227322c1d 100644 --- a/mithril-aggregator/src/commands/serve_command.rs +++ b/mithril-aggregator/src/commands/serve_command.rs @@ -7,11 +7,14 @@ use std::{ use anyhow::{anyhow, Context}; use chrono::TimeDelta; use clap::Parser; -use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; + +use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; + use slog::{crit, debug, info, warn, Logger}; use tokio::{sync::oneshot, task::JoinSet}; -use mithril_common::{register, register_parameter_bool, register_parameter_opt, StdResult}; +use mithril_cli_helper::{register, register_parameter_bool, register_parameter_opt}; +use mithril_common::StdResult; use mithril_metric::MetricsServer; use crate::{dependency_injection::DependenciesBuilder, tools::VacuumTracker, Configuration}; @@ -320,6 +323,8 @@ impl ServeCommand { #[cfg(test)] mod tests { + use config::ValueKind; + use super::*; use std::collections::HashMap; @@ -429,7 +434,7 @@ mod tests { "server_port".to_string(), Value::new( Some(&"clap arguments".to_string()), - ValueKind::from(8000 as u64), + ValueKind::from(8000_u64), ), ); expected.insert( @@ -466,7 +471,7 @@ mod tests { "metrics_server_port".to_string(), Value::new( Some(&"clap arguments".to_string()), - ValueKind::from(Some(8080 as u64)), + ValueKind::from(Some(8080_u64)), ), ); expected.insert( diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index d0a6716ebe1..d8269896175 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -5,6 +5,7 @@ use std::collections::{BTreeSet, HashMap}; use std::path::PathBuf; use std::str::FromStr; +use mithril_cli_helper::{register, register_parameter}; use mithril_common::chain_observer::ChainObserverType; use mithril_common::crypto_helper::ProtocolGenesisSigner; use mithril_common::entities::{ @@ -13,7 +14,7 @@ use mithril_common::entities::{ SignedEntityTypeDiscriminants, }; use mithril_common::era::adapters::EraReaderAdapterType; -use mithril_common::{register, register_parameter, CardanoNetwork, StdResult}; +use mithril_common::{CardanoNetwork, StdResult}; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use crate::entities::AggregatorEpochSettings; diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index 6ebeabff689..0a8890c862e 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -36,7 +36,6 @@ bech32 = "0.11.0" blake2 = "0.10.6" chrono = { version = "0.4.39", features = ["serde"] } ciborium = "0.2.2" -config = "0.15.7" ckb-merkle-mountain-range = "0.6.0" digest = "0.10.7" ed25519-dalek = { version = "2.1.1", features = ["rand_core", "serde"] } diff --git a/mithril-common/src/lib.rs b/mithril-common/src/lib.rs index 3227516e66c..353f4082cc2 100644 --- a/mithril-common/src/lib.rs +++ b/mithril-common/src/lib.rs @@ -41,7 +41,6 @@ pub mod logging; pub mod messages; pub mod protocol; pub mod signable_builder; -pub mod source_config; cfg_test_tools! { pub mod test_utils; diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 17882f14eb1..6457b4bbb3f 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -20,6 +20,7 @@ chrono = { version = "0.4.39", features = ["serde"] } clap = { version = "4.5.28", features = ["derive", "env"] } config = "0.15.7" hex = "0.4.3" +mithril-cli-helper = { path = "../internal/mithril-cli-helper" } mithril-common = { path = "../mithril-common", features = ["full"] } mithril-doc = { path = "../internal/mithril-doc" } mithril-metric = { path = "../internal/mithril-metric" } diff --git a/mithril-signer/src/configuration.rs b/mithril-signer/src/configuration.rs index 0a21d132178..88c2733af00 100644 --- a/mithril-signer/src/configuration.rs +++ b/mithril-signer/src/configuration.rs @@ -1,9 +1,10 @@ use anyhow::Context; -use config::{ConfigError, Map, Source, Value, ValueKind}; +use config::{ConfigError, Map, Source, Value}; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; +use mithril_cli_helper::{register, register_parameter}; use mithril_common::{ chain_observer::ChainObserver, crypto_helper::tests_setup, @@ -12,7 +13,7 @@ use mithril_common::{ adapters::{EraReaderAdapterBuilder, EraReaderAdapterType}, EraReaderAdapter, }, - register, register_parameter, CardanoNetwork, StdResult, + CardanoNetwork, StdResult, }; /// Client configuration @@ -290,6 +291,8 @@ impl Source for DefaultConfiguration { #[cfg(test)] mod test { + use config::ValueKind; + use super::*; // TODO : just here to check there is no regression with the old configuration. From 20433079e50760c5bdd11e3840840176f95fc2d6 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 11:45:15 +0100 Subject: [PATCH 07/15] refacto: Rename macros from `register_parameter` to `register_config_value` --- .../mithril-cli-helper/src/source_config.rs | 48 +++++++++---------- mithril-aggregator/src/commands/mod.rs | 4 +- .../src/commands/serve_command.rs | 22 ++++----- mithril-aggregator/src/configuration.rs | 48 +++++++++---------- mithril-signer/src/configuration.rs | 18 +++---- 5 files changed, 70 insertions(+), 70 deletions(-) diff --git a/internal/mithril-cli-helper/src/source_config.rs b/internal/mithril-cli-helper/src/source_config.rs index 8bbf2087db8..25ce46d41d2 100644 --- a/internal/mithril-cli-helper/src/source_config.rs +++ b/internal/mithril-cli-helper/src/source_config.rs @@ -13,7 +13,7 @@ macro_rules! register { /// Register a optional parameter in the config map when it's not None. #[macro_export] -macro_rules! register_parameter_opt { +macro_rules! register_config_value_option { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ if let Some(value) = $self.$command.clone() { register!($map, $namespace, $command, value); @@ -28,7 +28,7 @@ macro_rules! register_parameter_opt { /// Register a boolean parameter in the config map only when it's true. #[macro_export] -macro_rules! register_parameter_bool { +macro_rules! register_config_value_bool { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ if $self.$command { register!($map, $namespace, $command, true); @@ -38,7 +38,7 @@ macro_rules! register_parameter_bool { /// Register a parameter in the config map. #[macro_export] -macro_rules! register_parameter { +macro_rules! register_config_value { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ register!($map, $namespace, $command, $self.$command); }}; @@ -86,7 +86,7 @@ mod tests { } #[test] - fn test_register_parameter_macro_add_value() { + fn test_register_config_value_macro_add_value() { struct Fake { string_value: String, u64_value: u64, @@ -100,9 +100,9 @@ mod tests { }; let mut map = HashMap::new(); - register_parameter!(map, &"namespace".to_string(), fake.string_value); - register_parameter!(map, &"namespace".to_string(), fake.u64_value); - register_parameter!(map, &"namespace".to_string(), fake.enum_value); + register_config_value!(map, &"namespace".to_string(), fake.string_value); + register_config_value!(map, &"namespace".to_string(), fake.u64_value); + register_config_value!(map, &"namespace".to_string(), fake.enum_value); let expected = HashMap::from([ ( @@ -126,7 +126,7 @@ mod tests { } #[test] - fn test_register_parameter_macro_with_mapping_transform_value_before_adding_it() { + fn test_register_config_value_macro_with_mapping_transform_value_before_adding_it() { struct Fake { string_value: String, } @@ -136,7 +136,7 @@ mod tests { }; let mut map = HashMap::new(); - register_parameter!( + register_config_value!( map, &"namespace".to_string(), fake.string_value, @@ -155,7 +155,7 @@ mod tests { } #[test] - fn test_register_parameter_option_macro_not_add_none_value() { + fn test_register_config_value_optionion_macro_not_add_none_value() { struct Fake { option_with_value: Option, option_none: Option, @@ -167,8 +167,8 @@ mod tests { }; let mut map = HashMap::new(); - register_parameter_opt!(map, &"namespace".to_string(), fake.option_with_value); - register_parameter_opt!(map, &"namespace".to_string(), fake.option_none); + register_config_value_option!(map, &"namespace".to_string(), fake.option_with_value); + register_config_value_option!(map, &"namespace".to_string(), fake.option_none); let expected = HashMap::from([( "option_with_value".to_string(), @@ -182,7 +182,7 @@ mod tests { } #[test] - fn test_register_parameter_option_macro_with_mapping_transform_value_before_adding_it() { + fn test_register_config_value_optionion_macro_with_mapping_transform_value_before_adding_it() { struct Fake { option_with_value: Option, option_none: Option, @@ -194,13 +194,13 @@ mod tests { }; let mut map = HashMap::new(); - register_parameter_opt!( + register_config_value_option!( map, &"namespace".to_string(), fake.option_with_value, |v: String| format!("mapped_value from {}", v) ); - register_parameter_opt!(map, &"namespace".to_string(), fake.option_none); + register_config_value_option!(map, &"namespace".to_string(), fake.option_none); let expected = HashMap::from([( "option_with_value".to_string(), @@ -214,7 +214,7 @@ mod tests { } #[test] - fn test_register_parameter_bool_macro_add_only_true_values() { + fn test_register_config_value_bool_macro_add_only_true_values() { struct Fake { bool_true: bool, bool_false: bool, @@ -226,8 +226,8 @@ mod tests { }; let mut map = HashMap::new(); - register_parameter_bool!(map, &"namespace".to_string(), fake.bool_true); - register_parameter_bool!(map, &"namespace".to_string(), fake.bool_false); + register_config_value_bool!(map, &"namespace".to_string(), fake.bool_true); + register_config_value_bool!(map, &"namespace".to_string(), fake.bool_false); let expected = HashMap::from([( "bool_true".to_string(), @@ -238,7 +238,7 @@ mod tests { } #[test] - fn test_register_parameter_macro() { + fn test_register_config_value_macro() { struct Fake { server_ip: Option, } @@ -248,7 +248,7 @@ mod tests { }; let mut map = HashMap::new(); - register_parameter!(map, &"namespace".to_string(), fake.server_ip); + register_config_value!(map, &"namespace".to_string(), fake.server_ip); let expected = HashMap::from([( "server_ip".to_string(), @@ -283,10 +283,10 @@ mod tests { let mut map = Map::new(); let myself = self.clone(); - register_parameter_opt!(map, &"namespace".to_string(), myself.option_value); - register_parameter!(map, &"namespace".to_string(), myself.string_value); - register_parameter!(map, &"namespace".to_string(), myself.u64_value); - register_parameter!(map, &"namespace".to_string(), myself.enum_value); + register_config_value_option!(map, &"namespace".to_string(), myself.option_value); + register_config_value!(map, &"namespace".to_string(), myself.string_value); + register_config_value!(map, &"namespace".to_string(), myself.u64_value); + register_config_value!(map, &"namespace".to_string(), myself.enum_value); Ok(map) } diff --git a/mithril-aggregator/src/commands/mod.rs b/mithril-aggregator/src/commands/mod.rs index e306c1c782d..1b485fd1a06 100644 --- a/mithril-aggregator/src/commands/mod.rs +++ b/mithril-aggregator/src/commands/mod.rs @@ -7,7 +7,7 @@ mod tools_command; use anyhow::anyhow; use clap::{CommandFactory, Parser, Subcommand}; use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; -use mithril_cli_helper::{register, register_parameter_opt}; +use mithril_cli_helper::{register, register_config_value_option}; use mithril_common::StdResult; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use slog::{debug, Level, Logger}; @@ -104,7 +104,7 @@ impl Source for MainOpts { let namespace = "clap arguments".to_string(); // TODO Is it normal to only have db_directory ? - register_parameter_opt!(result, &namespace, self.db_directory, |v: PathBuf| format!( + register_config_value_option!(result, &namespace, self.db_directory, |v: PathBuf| format!( "{}", v.to_string_lossy() )); diff --git a/mithril-aggregator/src/commands/serve_command.rs b/mithril-aggregator/src/commands/serve_command.rs index e9227322c1d..5839030c2a9 100644 --- a/mithril-aggregator/src/commands/serve_command.rs +++ b/mithril-aggregator/src/commands/serve_command.rs @@ -13,7 +13,7 @@ use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; use slog::{crit, debug, info, warn, Logger}; use tokio::{sync::oneshot, task::JoinSet}; -use mithril_cli_helper::{register, register_parameter_bool, register_parameter_opt}; +use mithril_cli_helper::{register, register_config_value_bool, register_config_value_option}; use mithril_common::StdResult; use mithril_metric::MetricsServer; @@ -83,22 +83,22 @@ impl Source for ServeCommand { let mut result = Map::new(); let namespace = "clap arguments".to_string(); - register_parameter_opt!(result, &namespace, self.server_ip); - register_parameter_opt!(result, &namespace, self.server_port); - register_parameter_opt!( + register_config_value_option!(result, &namespace, self.server_ip); + register_config_value_option!(result, &namespace, self.server_port); + register_config_value_option!( result, &namespace, self.snapshot_directory, |v: PathBuf| format!("{}", v.to_string_lossy()) ); - register_parameter_bool!(result, &namespace, self.disable_digests_cache); - register_parameter_bool!(result, &namespace, self.reset_digests_cache); - register_parameter_bool!(result, &namespace, self.allow_unparsable_block); - register_parameter_bool!(result, &namespace, self.enable_metrics_server); - register_parameter_opt!(result, &namespace, self.metrics_server_ip); - register_parameter_opt!(result, &namespace, self.metrics_server_port); + register_config_value_bool!(result, &namespace, self.disable_digests_cache); + register_config_value_bool!(result, &namespace, self.reset_digests_cache); + register_config_value_bool!(result, &namespace, self.allow_unparsable_block); + register_config_value_bool!(result, &namespace, self.enable_metrics_server); + register_config_value_option!(result, &namespace, self.metrics_server_ip); + register_config_value_option!(result, &namespace, self.metrics_server_port); // TODO is it normal to pass a Some(v) and not only v when value is present ? - register_parameter_opt!( + register_config_value_option!( result, &namespace, self.master_aggregator_endpoint, diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index d8269896175..6aa450425bc 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -5,7 +5,7 @@ use std::collections::{BTreeSet, HashMap}; use std::path::PathBuf; use std::str::FromStr; -use mithril_cli_helper::{register, register_parameter}; +use mithril_cli_helper::{register, register_config_value}; use mithril_common::chain_observer::ChainObserverType; use mithril_common::crypto_helper::ProtocolGenesisSigner; use mithril_common::entities::{ @@ -531,49 +531,49 @@ impl Source for DefaultConfiguration { let namespace = DefaultConfiguration::namespace(); let myself = self.clone(); - register_parameter!(result, &namespace, myself.environment); - register_parameter!(result, &namespace, myself.server_ip); - register_parameter!(result, &namespace, myself.server_port); - register_parameter!(result, &namespace, myself.db_directory); - register_parameter!(result, &namespace, myself.snapshot_directory); - register_parameter!(result, &namespace, myself.snapshot_store_type); - register_parameter!(result, &namespace, myself.snapshot_uploader_type); - register_parameter!(result, &namespace, myself.era_reader_adapter_type); - register_parameter!(result, &namespace, myself.reset_digests_cache); - register_parameter!(result, &namespace, myself.disable_digests_cache); - register_parameter!(result, &namespace, myself.snapshot_compression_algorithm); - register_parameter!(result, &namespace, myself.snapshot_use_cdn_domain); - register_parameter!(result, &namespace, myself.signer_importer_run_interval); - register_parameter!(result, &namespace, myself.allow_unparsable_block); - register_parameter!( + register_config_value!(result, &namespace, myself.environment); + register_config_value!(result, &namespace, myself.server_ip); + register_config_value!(result, &namespace, myself.server_port); + register_config_value!(result, &namespace, myself.db_directory); + register_config_value!(result, &namespace, myself.snapshot_directory); + register_config_value!(result, &namespace, myself.snapshot_store_type); + register_config_value!(result, &namespace, myself.snapshot_uploader_type); + register_config_value!(result, &namespace, myself.era_reader_adapter_type); + register_config_value!(result, &namespace, myself.reset_digests_cache); + register_config_value!(result, &namespace, myself.disable_digests_cache); + register_config_value!(result, &namespace, myself.snapshot_compression_algorithm); + register_config_value!(result, &namespace, myself.snapshot_use_cdn_domain); + register_config_value!(result, &namespace, myself.signer_importer_run_interval); + register_config_value!(result, &namespace, myself.allow_unparsable_block); + register_config_value!( result, &namespace, myself.cardano_transactions_prover_cache_pool_size ); - register_parameter!( + register_config_value!( result, &namespace, myself.cardano_transactions_database_connection_pool_size ); - register_parameter!( + register_config_value!( result, &namespace, myself.cardano_transactions_prover_max_hashes_allowed_by_request ); - register_parameter!( + register_config_value!( result, &namespace, myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll ); - register_parameter!(result, &namespace, myself.enable_metrics_server); - register_parameter!(result, &namespace, myself.metrics_server_ip); - register_parameter!(result, &namespace, myself.metrics_server_port); - register_parameter!( + register_config_value!(result, &namespace, myself.enable_metrics_server); + register_config_value!(result, &namespace, myself.metrics_server_ip); + register_config_value!(result, &namespace, myself.metrics_server_port); + register_config_value!( result, &namespace, myself.persist_usage_report_interval_in_seconds ); - register_parameter!( + register_config_value!( result, &namespace, myself.cardano_transactions_signing_config, diff --git a/mithril-signer/src/configuration.rs b/mithril-signer/src/configuration.rs index 88c2733af00..49de25c4bbe 100644 --- a/mithril-signer/src/configuration.rs +++ b/mithril-signer/src/configuration.rs @@ -4,7 +4,7 @@ use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; -use mithril_cli_helper::{register, register_parameter}; +use mithril_cli_helper::{register, register_config_value}; use mithril_common::{ chain_observer::ChainObserver, crypto_helper::tests_setup, @@ -267,18 +267,18 @@ impl Source for DefaultConfiguration { let namespace = DefaultConfiguration::namespace(); let myself = self.clone(); - register_parameter!(result, &namespace, myself.era_reader_adapter_type); - register_parameter!(result, &namespace, myself.metrics_server_ip); - register_parameter!(result, &namespace, myself.metrics_server_port); - register_parameter!(result, &namespace, myself.network_security_parameter); - register_parameter!(result, &namespace, myself.preload_security_parameter); - register_parameter!(result, &namespace, myself.enable_transaction_pruning); - register_parameter!( + register_config_value!(result, &namespace, myself.era_reader_adapter_type); + register_config_value!(result, &namespace, myself.metrics_server_ip); + register_config_value!(result, &namespace, myself.metrics_server_port); + register_config_value!(result, &namespace, myself.network_security_parameter); + register_config_value!(result, &namespace, myself.preload_security_parameter); + register_config_value!(result, &namespace, myself.enable_transaction_pruning); + register_config_value!( result, &namespace, myself.transactions_import_block_chunk_size ); - register_parameter!( + register_config_value!( result, &namespace, myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll From d53a86182280796d8cc400d581f7bb5f0dd0df32 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 12:16:54 +0100 Subject: [PATCH 08/15] refacto: simplify macros --- .../mithril-cli-helper/src/source_config.rs | 44 ++++++++++--------- mithril-aggregator/src/commands/mod.rs | 2 +- .../src/commands/serve_command.rs | 4 +- mithril-aggregator/src/configuration.rs | 2 +- mithril-signer/src/configuration.rs | 2 +- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/internal/mithril-cli-helper/src/source_config.rs b/internal/mithril-cli-helper/src/source_config.rs index 25ce46d41d2..b79ee39513f 100644 --- a/internal/mithril-cli-helper/src/source_config.rs +++ b/internal/mithril-cli-helper/src/source_config.rs @@ -1,27 +1,16 @@ //! Utilities to register config parameters. -/// Register a parameter in the config map. -#[macro_export] -macro_rules! register { - ( $map:ident, $namespace:expr, $command: ident, $value:expr ) => {{ - $map.insert( - stringify!($command).to_string(), - config::Value::new(Some($namespace), $value), - ); - }}; -} - /// Register a optional parameter in the config map when it's not None. #[macro_export] macro_rules! register_config_value_option { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ if let Some(value) = $self.$command.clone() { - register!($map, $namespace, $command, value); + register_config_value!($map, $namespace, $command = value); } }}; ( $map:ident, $namespace:expr, $self:ident.$command:ident, $mapping:expr ) => {{ if let Some(value) = $self.$command.clone() { - register!($map, $namespace, $command, $mapping(value)); + register_config_value!($map, $namespace, $command = $mapping(value)); } }}; } @@ -31,19 +20,33 @@ macro_rules! register_config_value_option { macro_rules! register_config_value_bool { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ if $self.$command { - register!($map, $namespace, $command, true); + register_config_value!($map, $namespace, $command = true); } }}; } -/// Register a parameter in the config map. +/// Register a parameter in the config map using the identifier as key. +/// Example: +/// register_config_value(map, namespace, self.identifier) +/// +/// The same macro, with a different syntax, is used to insert the given value without transformation. +/// Iit is designed to be used by other macros. +/// Example: +/// register_config_value!(map, namespace, identifier = value) #[macro_export] macro_rules! register_config_value { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ - register!($map, $namespace, $command, $self.$command); + register_config_value!($map, $namespace, $command = $self.$command); }}; ( $map:ident, $namespace:expr, $self:ident.$command:ident, $mapping:expr ) => {{ - register!($map, $namespace, $command, $mapping($self.$command)); + register_config_value!($map, $namespace, $command = $mapping($self.$command)); + }}; + + ( $map:ident, $namespace:expr, $command:ident = $value:expr ) => {{ + $map.insert( + stringify!($command).to_string(), + config::Value::new(Some($namespace), $value), + ); }}; } @@ -53,13 +56,12 @@ mod tests { use std::collections::HashMap; #[test] - fn test_register_macro() { + fn test_register_config_value_macro_with_the_value() { let mut map = HashMap::new(); - register!( + register_config_value!( map, &"namespace".to_string(), - server_ip, - Some("value_server_ip".to_string()) + server_ip = Some("value_server_ip".to_string()) ); let expected = HashMap::from([( diff --git a/mithril-aggregator/src/commands/mod.rs b/mithril-aggregator/src/commands/mod.rs index 1b485fd1a06..93d511992d3 100644 --- a/mithril-aggregator/src/commands/mod.rs +++ b/mithril-aggregator/src/commands/mod.rs @@ -7,7 +7,7 @@ mod tools_command; use anyhow::anyhow; use clap::{CommandFactory, Parser, Subcommand}; use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; -use mithril_cli_helper::{register, register_config_value_option}; +use mithril_cli_helper::{register_config_value, register_config_value_option}; use mithril_common::StdResult; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use slog::{debug, Level, Logger}; diff --git a/mithril-aggregator/src/commands/serve_command.rs b/mithril-aggregator/src/commands/serve_command.rs index 5839030c2a9..1e55f894161 100644 --- a/mithril-aggregator/src/commands/serve_command.rs +++ b/mithril-aggregator/src/commands/serve_command.rs @@ -13,7 +13,9 @@ use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; use slog::{crit, debug, info, warn, Logger}; use tokio::{sync::oneshot, task::JoinSet}; -use mithril_cli_helper::{register, register_config_value_bool, register_config_value_option}; +use mithril_cli_helper::{ + register_config_value, register_config_value_bool, register_config_value_option, +}; use mithril_common::StdResult; use mithril_metric::MetricsServer; diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 6aa450425bc..676e21b1e9e 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -5,7 +5,7 @@ use std::collections::{BTreeSet, HashMap}; use std::path::PathBuf; use std::str::FromStr; -use mithril_cli_helper::{register, register_config_value}; +use mithril_cli_helper::register_config_value; use mithril_common::chain_observer::ChainObserverType; use mithril_common::crypto_helper::ProtocolGenesisSigner; use mithril_common::entities::{ diff --git a/mithril-signer/src/configuration.rs b/mithril-signer/src/configuration.rs index 49de25c4bbe..ffe6a61bd8d 100644 --- a/mithril-signer/src/configuration.rs +++ b/mithril-signer/src/configuration.rs @@ -4,7 +4,7 @@ use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; -use mithril_cli_helper::{register, register_config_value}; +use mithril_cli_helper::register_config_value; use mithril_common::{ chain_observer::ChainObserver, crypto_helper::tests_setup, From 6d4267b2f98a12f15f72a506221bf9d029bae30b Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 12:51:32 +0100 Subject: [PATCH 09/15] refacto: Use `register_config_value` in mithril-client-cli --- Cargo.lock | 1 + mithril-client-cli/Cargo.toml | 1 + mithril-client-cli/src/main.rs | 11 ++++------- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 554277a140a..69bff890926 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3767,6 +3767,7 @@ dependencies = [ "futures", "human_bytes", "indicatif", + "mithril-cli-helper", "mithril-client", "mithril-common", "mithril-doc", diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml index d3e3329ca23..eda9c339a2f 100644 --- a/mithril-client-cli/Cargo.toml +++ b/mithril-client-cli/Cargo.toml @@ -32,6 +32,7 @@ fs2 = "0.4.3" futures = "0.3.31" human_bytes = { version = "0.4.3", features = ["fast"] } indicatif = { version = "0.17.11", features = ["tokio"] } +mithril-cli-helper = { path = "../internal/mithril-cli-helper" } mithril-client = { path = "../mithril-client", features = ["fs", "unstable"] } mithril-doc = { path = "../internal/mithril-doc" } serde = { version = "1.0.217", features = ["derive"] } diff --git a/mithril-client-cli/src/main.rs b/mithril-client-cli/src/main.rs index cb60b241eb6..875f88f1b82 100644 --- a/mithril-client-cli/src/main.rs +++ b/mithril-client-cli/src/main.rs @@ -2,7 +2,8 @@ use anyhow::{anyhow, Context}; use clap::{CommandFactory, Parser, Subcommand}; -use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; +use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value}; +use mithril_cli_helper::{register_config_value, register_config_value_option}; use slog::{debug, Drain, Fuse, Level, Logger}; use slog_term::Decorator; use std::io::Write; @@ -177,12 +178,8 @@ impl Source for Args { let mut map = Map::new(); let namespace = "clap arguments".to_string(); - if let Some(aggregator_endpoint) = self.aggregator_endpoint.clone() { - map.insert( - "aggregator_endpoint".to_string(), - Value::new(Some(&namespace), ValueKind::from(aggregator_endpoint)), - ); - } + let myself = self.clone(); + register_config_value_option!(map, &namespace, myself.aggregator_endpoint); Ok(map) } From 0c1683808b5e496d0225c1aa7e1dc1bf1061dc46 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 14:19:50 +0100 Subject: [PATCH 10/15] refacto: Use `register_config_value` in mithril-doc crate --- Cargo.lock | 1 + internal/mithril-doc/Cargo.toml | 1 + internal/mithril-doc/src/test_doc_macro.rs | 10 +++++----- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 69bff890926..d07daa5f9ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3857,6 +3857,7 @@ version = "0.1.17" dependencies = [ "clap", "config", + "mithril-cli-helper", "mithril-doc-derive", "regex", ] diff --git a/internal/mithril-doc/Cargo.toml b/internal/mithril-doc/Cargo.toml index e5324d6a057..d0d5ed1a2fa 100644 --- a/internal/mithril-doc/Cargo.toml +++ b/internal/mithril-doc/Cargo.toml @@ -15,6 +15,7 @@ config = "0.15.7" mithril-doc-derive = { path = "../mithril-doc-derive" } [dev-dependencies] +mithril-cli-helper = { path = "../mithril-cli-helper" } regex = "1.11.1" [features] diff --git a/internal/mithril-doc/src/test_doc_macro.rs b/internal/mithril-doc/src/test_doc_macro.rs index b4c6b8493f8..14a5a8fb2b8 100644 --- a/internal/mithril-doc/src/test_doc_macro.rs +++ b/internal/mithril-doc/src/test_doc_macro.rs @@ -1,7 +1,8 @@ #[cfg(test)] mod tests { use crate::{Documenter, DocumenterDefault, StructDoc}; - use config::{Map, Source, Value, ValueKind}; + use config::{Map, Source, Value}; + use mithril_cli_helper::register_config_value; #[allow(dead_code)] #[derive(Debug, Clone, mithril_doc_derive::Documenter)] @@ -32,11 +33,10 @@ mod tests { fn collect(&self) -> Result, config::ConfigError> { let mut result = Map::new(); let namespace = "default configuration".to_string(); + let myself = self.clone(); - result.insert( - "environment".to_string(), - Value::new(Some(&namespace), ValueKind::from(myself.environment)), - ); + register_config_value!(result, &namespace, myself.environment); + Ok(result) } } From a656b4ae43f35e8902de4e9d6da7768b3ee17254 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 14:29:38 +0100 Subject: [PATCH 11/15] refacto: remove tests that checks there is no regression in configuration --- mithril-aggregator/src/commands/mod.rs | 66 ------ .../src/commands/serve_command.rs | 190 ------------------ mithril-aggregator/src/configuration.rs | 166 --------------- mithril-signer/src/configuration.rs | 77 ------- 4 files changed, 499 deletions(-) diff --git a/mithril-aggregator/src/commands/mod.rs b/mithril-aggregator/src/commands/mod.rs index 93d511992d3..d5b5cbeb8cf 100644 --- a/mithril-aggregator/src/commands/mod.rs +++ b/mithril-aggregator/src/commands/mod.rs @@ -142,69 +142,3 @@ impl MainOpts { } } } - -#[cfg(test)] -mod tests { - use config::ValueKind; - - use crate::commands::tools_command::{ - RecomputeCertificatesHashCommand, ToolsCommand, ToolsSubCommand::RecomputeCertificatesHash, - }; - - use super::*; - - // TODO : just here to check there is no regression with the old configuration. - // We may remove it and probably all tests in this file when macros are finished - impl MainOpts { - fn collect_legacy(&self) -> Result, config::ConfigError> { - let mut result = Map::new(); - let namespace = "clap arguments".to_string(); - - if let Some(db_directory) = self.db_directory.clone() { - result.insert( - "db_directory".to_string(), - Value::new( - Some(&namespace), - ValueKind::from(format!("{}", db_directory.to_string_lossy())), - ), - ); - } - - Ok(result) - } - } - - #[test] - fn test_main_opts_collect() { - let serve_command = MainOpts { - db_directory: Some(PathBuf::from("/db_directory")), - command: MainCommand::Tools(ToolsCommand { - genesis_subcommand: RecomputeCertificatesHash(RecomputeCertificatesHashCommand {}), - }), - run_mode: "mode".to_string(), - verbose: 1, - config_directory: PathBuf::from(""), - }; - - let result = serve_command.collect().unwrap().clone(); - - assert_eq!(serve_command.collect_legacy().unwrap(), result); - } - - #[test] - fn test_main_opts_collect_when_empty_values() { - let serve_command = MainOpts { - db_directory: None, - command: MainCommand::Tools(ToolsCommand { - genesis_subcommand: RecomputeCertificatesHash(RecomputeCertificatesHashCommand {}), - }), - run_mode: "mode".to_string(), - verbose: 1, - config_directory: PathBuf::from(""), - }; - - let result = serve_command.collect().unwrap().clone(); - - assert_eq!(serve_command.collect_legacy().unwrap(), result); - } -} diff --git a/mithril-aggregator/src/commands/serve_command.rs b/mithril-aggregator/src/commands/serve_command.rs index 1e55f894161..e0fb1bd3689 100644 --- a/mithril-aggregator/src/commands/serve_command.rs +++ b/mithril-aggregator/src/commands/serve_command.rs @@ -322,193 +322,3 @@ impl ServeCommand { Ok(()) } } - -#[cfg(test)] -mod tests { - use config::ValueKind; - - use super::*; - use std::collections::HashMap; - - // TODO : just here to check there is no regression with the old configuration. - // We may remove it and probably all tests in this file when macros are finished - impl ServeCommand { - fn collect_legacy(&self) -> Result, config::ConfigError> { - let mut result = Map::new(); - let namespace = "clap arguments".to_string(); - - if let Some(server_ip) = self.server_ip.clone() { - result.insert( - "server_ip".to_string(), - Value::new(Some(&namespace), ValueKind::from(server_ip)), - ); - } - if let Some(server_port) = self.server_port { - result.insert( - "server_port".to_string(), - Value::new(Some(&namespace), ValueKind::from(server_port)), - ); - } - if let Some(snapshot_directory) = self.snapshot_directory.clone() { - result.insert( - "snapshot_directory".to_string(), - Value::new( - Some(&namespace), - ValueKind::from(format!("{}", snapshot_directory.to_string_lossy())), - ), - ); - } - if self.disable_digests_cache { - result.insert( - "disable_digests_cache".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - }; - if self.reset_digests_cache { - result.insert( - "reset_digests_cache".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - } - if self.allow_unparsable_block { - result.insert( - "allow_unparsable_block".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - }; - if self.enable_metrics_server { - result.insert( - "enable_metrics_server".to_string(), - Value::new(Some(&namespace), ValueKind::from(true)), - ); - }; - if let Some(metrics_server_ip) = self.metrics_server_ip.clone() { - result.insert( - "metrics_server_ip".to_string(), - Value::new(Some(&namespace), ValueKind::from(metrics_server_ip)), - ); - } - if let Some(metrics_server_port) = self.metrics_server_port { - result.insert( - "metrics_server_port".to_string(), - Value::new(Some(&namespace), ValueKind::from(metrics_server_port)), - ); - } - if let Some(master_aggregator_endpoint) = self.master_aggregator_endpoint.clone() { - result.insert( - "master_aggregator_endpoint".to_string(), - Value::new( - Some(&namespace), - ValueKind::from(Some(master_aggregator_endpoint)), - ), - ); - } - - Ok(result) - } - } - - #[test] - fn test_serve_command_collect() { - let serve_command = ServeCommand { - server_ip: Some("value_server_ip".to_string()), - server_port: Some(8000), - snapshot_directory: Some(PathBuf::from("/mithril/aggregator/")), - disable_digests_cache: true, - reset_digests_cache: true, - allow_unparsable_block: true, - enable_metrics_server: true, - metrics_server_ip: Some("value_metrics_server_ip".to_string()), - metrics_server_port: Some(8080), - master_aggregator_endpoint: Some("value_master_aggregator_endpoint".to_string()), - }; - - let result = serve_command.collect().unwrap().clone(); - let mut expected = HashMap::new(); - expected.insert( - "server_ip".to_string(), - Value::new( - Some(&"clap arguments".to_string()), - ValueKind::from("value_server_ip"), - ), - ); - expected.insert( - "server_port".to_string(), - Value::new( - Some(&"clap arguments".to_string()), - ValueKind::from(8000_u64), - ), - ); - expected.insert( - "snapshot_directory".to_string(), - Value::new( - Some(&"clap arguments".to_string()), - ValueKind::from("/mithril/aggregator/"), - ), - ); - expected.insert( - "disable_digests_cache".to_string(), - Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), - ); - expected.insert( - "reset_digests_cache".to_string(), - Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), - ); - expected.insert( - "allow_unparsable_block".to_string(), - Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), - ); - expected.insert( - "enable_metrics_server".to_string(), - Value::new(Some(&"clap arguments".to_string()), ValueKind::from(true)), - ); - expected.insert( - "metrics_server_ip".to_string(), - Value::new( - Some(&"clap arguments".to_string()), - ValueKind::from("value_metrics_server_ip"), - ), - ); - expected.insert( - "metrics_server_port".to_string(), - Value::new( - Some(&"clap arguments".to_string()), - ValueKind::from(Some(8080_u64)), - ), - ); - expected.insert( - "master_aggregator_endpoint".to_string(), - Value::new( - Some(&"clap arguments".to_string()), - ValueKind::from("value_master_aggregator_endpoint"), - ), - ); - - assert_eq!(expected, result); - - assert_eq!(serve_command.collect_legacy().unwrap(), result); - } - - #[test] - fn test_serve_command_collect_when_empty_values() { - let serve_command = ServeCommand { - server_ip: None, - server_port: None, - snapshot_directory: None, - disable_digests_cache: false, - reset_digests_cache: false, - allow_unparsable_block: false, - enable_metrics_server: false, - metrics_server_ip: None, - metrics_server_port: None, - master_aggregator_endpoint: None, - }; - - let result = serve_command.collect().unwrap().clone(); - let expected = HashMap::new(); - - assert_eq!(expected, result); - - assert_eq!(serve_command.collect_legacy().unwrap(), result); - } -} diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 676e21b1e9e..6e0315d6998 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -595,172 +595,6 @@ mod test { use super::*; - // TODO : just here to check there is no regression with the old configuration. - // We may remove it and probably all tests in this file when macros are finished - impl DefaultConfiguration { - fn collect_legacy(&self) -> Result, ConfigError> { - macro_rules! insert_default_configuration { - ( $map:ident, $config:ident.$parameter:ident ) => {{ - $map.insert( - stringify!($parameter).to_string(), - into_value($config.$parameter), - ); - }}; - } - - fn into_value>(value: V) -> Value { - Value::new(Some(&DefaultConfiguration::namespace()), value.into()) - } - let mut result = Map::new(); - let myself = self.clone(); - - result.insert( - "cardano_transactions_signing_config".to_string(), - into_value(HashMap::from([ - ( - "security_parameter".to_string(), - ValueKind::from( - *myself - .cardano_transactions_signing_config - .security_parameter, - ), - ), - ( - "step".to_string(), - ValueKind::from(*myself.cardano_transactions_signing_config.step), - ), - ])), - ); - - insert_default_configuration!(result, myself.environment); - insert_default_configuration!(result, myself.server_ip); - insert_default_configuration!(result, myself.server_port); - insert_default_configuration!(result, myself.db_directory); - insert_default_configuration!(result, myself.snapshot_directory); - insert_default_configuration!(result, myself.snapshot_store_type); - insert_default_configuration!(result, myself.snapshot_uploader_type); - insert_default_configuration!(result, myself.era_reader_adapter_type); - insert_default_configuration!(result, myself.reset_digests_cache); - insert_default_configuration!(result, myself.disable_digests_cache); - insert_default_configuration!(result, myself.snapshot_compression_algorithm); - insert_default_configuration!(result, myself.snapshot_use_cdn_domain); - insert_default_configuration!(result, myself.signer_importer_run_interval); - insert_default_configuration!(result, myself.allow_unparsable_block); - insert_default_configuration!( - result, - myself.cardano_transactions_prover_cache_pool_size - ); - insert_default_configuration!( - result, - myself.cardano_transactions_database_connection_pool_size - ); - insert_default_configuration!( - result, - myself.cardano_transactions_prover_max_hashes_allowed_by_request - ); - insert_default_configuration!( - result, - myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll - ); - insert_default_configuration!(result, myself.enable_metrics_server); - insert_default_configuration!(result, myself.metrics_server_ip); - insert_default_configuration!(result, myself.metrics_server_port); - insert_default_configuration!(result, myself.persist_usage_report_interval_in_seconds); - result.insert( - "cardano_transactions_signing_config".to_string(), - into_value(HashMap::from([ - ( - "security_parameter".to_string(), - ValueKind::from( - *myself - .cardano_transactions_signing_config - .security_parameter, - ), - ), - ( - "step".to_string(), - ValueKind::from(*myself.cardano_transactions_signing_config.step), - ), - ])), - ); - Ok(result) - } - } - - #[test] - fn test_default_configuration_collect() { - let default_configuration = DefaultConfiguration { - environment: ExecutionEnvironment::Production, - server_ip: "0.0.0.0".to_string(), - server_port: "8080".to_string(), - db_directory: "/db".to_string(), - snapshot_directory: ".".to_string(), - snapshot_store_type: "local".to_string(), - snapshot_uploader_type: "gcp".to_string(), - era_reader_adapter_type: "bootstrap".to_string(), - chain_observer_type: "pallas".to_string(), - reset_digests_cache: "true".to_string(), - disable_digests_cache: "true".to_string(), - snapshot_compression_algorithm: "zstandard".to_string(), - snapshot_use_cdn_domain: "true".to_string(), - signer_importer_run_interval: 720, - allow_unparsable_block: "true".to_string(), - cardano_transactions_prover_cache_pool_size: 10, - cardano_transactions_database_connection_pool_size: 10, - cardano_transactions_signing_config: CardanoTransactionsSigningConfig { - security_parameter: BlockNumber(3000), - step: BlockNumber(120), - }, - cardano_transactions_prover_max_hashes_allowed_by_request: 100, - cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, - enable_metrics_server: "true".to_string(), - metrics_server_ip: "0.0.0.0".to_string(), - metrics_server_port: 9090, - persist_usage_report_interval_in_seconds: 10, - }; - - let result = default_configuration.collect().unwrap().clone(); - - assert_eq!(default_configuration.collect_legacy().unwrap(), result); - } - - #[test] - fn test_default_configuration_collect_when_empty_values() { - let default_configuration = DefaultConfiguration { - environment: ExecutionEnvironment::Production, - server_ip: "0.0.0.0".to_string(), - server_port: "8080".to_string(), - db_directory: "/db".to_string(), - snapshot_directory: ".".to_string(), - snapshot_store_type: "local".to_string(), - snapshot_uploader_type: "gcp".to_string(), - era_reader_adapter_type: "bootstrap".to_string(), - chain_observer_type: "pallas".to_string(), - reset_digests_cache: "false".to_string(), - disable_digests_cache: "false".to_string(), - snapshot_compression_algorithm: "zstandard".to_string(), - snapshot_use_cdn_domain: "false".to_string(), - signer_importer_run_interval: 720, - allow_unparsable_block: "false".to_string(), - cardano_transactions_prover_cache_pool_size: 10, - cardano_transactions_database_connection_pool_size: 10, - cardano_transactions_signing_config: CardanoTransactionsSigningConfig { - security_parameter: BlockNumber(3000), - step: BlockNumber(120), - }, - cardano_transactions_prover_max_hashes_allowed_by_request: 100, - cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, - enable_metrics_server: "false".to_string(), - metrics_server_ip: "0.0.0.0".to_string(), - metrics_server_port: 9090, - persist_usage_report_interval_in_seconds: 10, - }; - - let result = default_configuration.collect().unwrap().clone(); - - assert_eq!(default_configuration.collect_legacy().unwrap(), result); - } - #[test] fn safe_epoch_retention_limit_wont_change_a_value_higher_than_three() { for limit in 4..=10u64 { diff --git a/mithril-signer/src/configuration.rs b/mithril-signer/src/configuration.rs index ffe6a61bd8d..79ac12a6a75 100644 --- a/mithril-signer/src/configuration.rs +++ b/mithril-signer/src/configuration.rs @@ -287,80 +287,3 @@ impl Source for DefaultConfiguration { Ok(result) } } - -#[cfg(test)] -mod test { - - use config::ValueKind; - - use super::*; - - // TODO : just here to check there is no regression with the old configuration. - // We may remove it and probably all tests in this file when macros are finished - impl DefaultConfiguration { - fn collect_legacy(&self) -> Result, ConfigError> { - macro_rules! insert_default_configuration { - ( $map:ident, $config:ident.$parameter:ident ) => {{ - $map.insert( - stringify!($parameter).to_string(), - into_value($config.$parameter), - ); - }}; - } - - fn into_value>(value: V) -> Value { - Value::new(Some(&DefaultConfiguration::namespace()), value.into()) - } - let mut result = Map::new(); - let myself = self.clone(); - - insert_default_configuration!(result, myself.era_reader_adapter_type); - insert_default_configuration!(result, myself.metrics_server_ip); - insert_default_configuration!(result, myself.metrics_server_port); - insert_default_configuration!(result, myself.network_security_parameter); - insert_default_configuration!(result, myself.preload_security_parameter); - insert_default_configuration!(result, myself.enable_transaction_pruning); - insert_default_configuration!(result, myself.transactions_import_block_chunk_size); - insert_default_configuration!( - result, - myself.cardano_transactions_block_streamer_max_roll_forwards_per_poll - ); - - Ok(result) - } - } - - #[test] - fn test_default_configuration_collect() { - let default_configuration = DefaultConfiguration { - era_reader_adapter_type: "bootstrap".to_string(), - metrics_server_ip: "0.0.0.0".to_string(), - metrics_server_port: 9090, - network_security_parameter: 2160, - preload_security_parameter: 1000, - enable_transaction_pruning: true, - transactions_import_block_chunk_size: 1500, - cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, - }; - let result = default_configuration.collect().unwrap().clone(); - - assert_eq!(default_configuration.collect_legacy().unwrap(), result); - } - - #[test] - fn test_default_configuration_collect_with_empty_values() { - let default_configuration = DefaultConfiguration { - era_reader_adapter_type: "bootstrap".to_string(), - metrics_server_ip: "0.0.0.0".to_string(), - metrics_server_port: 9090, - network_security_parameter: 2160, - preload_security_parameter: 1000, - enable_transaction_pruning: false, - transactions_import_block_chunk_size: 1500, - cardano_transactions_block_streamer_max_roll_forwards_per_poll: 10000, - }; - let result = default_configuration.collect().unwrap().clone(); - - assert_eq!(default_configuration.collect_legacy().unwrap(), result); - } -} From 40ced9176f272f830a4401a76b04e7e38b29f9bd Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 18 Mar 2025 15:32:05 +0100 Subject: [PATCH 12/15] refacto: remove todo --- mithril-aggregator/src/commands/mod.rs | 1 - mithril-aggregator/src/commands/serve_command.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/mithril-aggregator/src/commands/mod.rs b/mithril-aggregator/src/commands/mod.rs index d5b5cbeb8cf..53eb1e440f6 100644 --- a/mithril-aggregator/src/commands/mod.rs +++ b/mithril-aggregator/src/commands/mod.rs @@ -103,7 +103,6 @@ impl Source for MainOpts { let mut result = Map::new(); let namespace = "clap arguments".to_string(); - // TODO Is it normal to only have db_directory ? register_config_value_option!(result, &namespace, self.db_directory, |v: PathBuf| format!( "{}", v.to_string_lossy() diff --git a/mithril-aggregator/src/commands/serve_command.rs b/mithril-aggregator/src/commands/serve_command.rs index e0fb1bd3689..67c5f38557e 100644 --- a/mithril-aggregator/src/commands/serve_command.rs +++ b/mithril-aggregator/src/commands/serve_command.rs @@ -99,7 +99,6 @@ impl Source for ServeCommand { register_config_value_bool!(result, &namespace, self.enable_metrics_server); register_config_value_option!(result, &namespace, self.metrics_server_ip); register_config_value_option!(result, &namespace, self.metrics_server_port); - // TODO is it normal to pass a Some(v) and not only v when value is present ? register_config_value_option!( result, &namespace, From 4eced3b064932214bb2a8691678d866ae381d59d Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 19 Mar 2025 09:58:13 +0100 Subject: [PATCH 13/15] refacto: Syntax correction in comments --- internal/mithril-cli-helper/README.md | 2 +- internal/mithril-cli-helper/src/source_config.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/mithril-cli-helper/README.md b/internal/mithril-cli-helper/README.md index e38e4f34079..8927a45ce97 100644 --- a/internal/mithril-cli-helper/README.md +++ b/internal/mithril-cli-helper/README.md @@ -1,3 +1,3 @@ # Mithril-cli-helper -An internal crate to provide tools for Mithril clients. +An internal crate to provide tools for Mithril CLIs. diff --git a/internal/mithril-cli-helper/src/source_config.rs b/internal/mithril-cli-helper/src/source_config.rs index b79ee39513f..ef219f68fd0 100644 --- a/internal/mithril-cli-helper/src/source_config.rs +++ b/internal/mithril-cli-helper/src/source_config.rs @@ -1,6 +1,6 @@ //! Utilities to register config parameters. -/// Register a optional parameter in the config map when it's not None. +/// Register an optional parameter in the config map when it's not None. #[macro_export] macro_rules! register_config_value_option { ( $map:ident, $namespace:expr, $self:ident.$command:ident ) => {{ @@ -27,7 +27,7 @@ macro_rules! register_config_value_bool { /// Register a parameter in the config map using the identifier as key. /// Example: -/// register_config_value(map, namespace, self.identifier) +/// register_config_value!(map, namespace, self.identifier) /// /// The same macro, with a different syntax, is used to insert the given value without transformation. /// Iit is designed to be used by other macros. @@ -157,7 +157,7 @@ mod tests { } #[test] - fn test_register_config_value_optionion_macro_not_add_none_value() { + fn test_register_config_value_option_macro_not_add_none_value() { struct Fake { option_with_value: Option, option_none: Option, @@ -184,7 +184,7 @@ mod tests { } #[test] - fn test_register_config_value_optionion_macro_with_mapping_transform_value_before_adding_it() { + fn test_register_config_value_option_macro_with_mapping_transform_value_before_adding_it() { struct Fake { option_with_value: Option, option_none: Option, From 2aeaf3811a7e92469032cc03e309e8abb053d5da Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 19 Mar 2025 10:21:17 +0100 Subject: [PATCH 14/15] Update components in the Makefile --- Makefile | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 154a89d2a3a..e2030cf3934 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,11 @@ -COMPONENTS = mithril-common mithril-stm mithril-aggregator mithril-client mithril-client-cli mithril-signer \ - internal/mithril-persistence internal/mithril-doc-derive internal/mithril-doc internal/mithril-build-script \ - demo/protocol-demo mithril-test-lab/mithril-end-to-end +COMPONENTS = mithril-aggregator mithril-client mithril-client-cli mithril-client-wasm \ + mithril-common mithril-relay mithril-signer mithril-stm \ + internal/mithril-build-script internal/mithril-cli-helper internal/mithril-doc \ + internal/mithril-doc-derive internal/mithril-metric internal/mithril-persistence \ + internal/mithril-resource-pool internal/signed-entity/mithril-signed-entity-lock \ + internal/signed-entity/mithril-signed-entity-preloader \ + demo/protocol-demo \ + mithril-test-lab/mithril-aggregator-fake mithril-test-lab/mithril-end-to-end GOALS := $(or $(MAKECMDGOALS),all) NON_COMPONENT_GOALS := check-format format From e8bf4be980eb4b025a7157b5697c31dc0370d383 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 19 Mar 2025 10:57:32 +0100 Subject: [PATCH 15/15] chore: upgrade crate versions * mithril-doc from `0.1.17` to `0.1.18` * mithril-aggregator from `0.7.17` to `0.7.18` * mithril-client-cli from `0.11.9` to `0.11.10` * mithril-signer from `0.2.234` to `0.2.235` * mithril-stm from `0.3.39` to `0.3.40` --- Cargo.lock | 10 +++++----- internal/mithril-doc/Cargo.toml | 2 +- mithril-aggregator/Cargo.toml | 2 +- mithril-client-cli/Cargo.toml | 2 +- mithril-signer/Cargo.toml | 2 +- mithril-stm/Cargo.toml | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d07daa5f9ce..0015cd14190 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3639,7 +3639,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.7.17" +version = "0.7.18" dependencies = [ "anyhow", "async-trait", @@ -3755,7 +3755,7 @@ dependencies = [ [[package]] name = "mithril-client-cli" -version = "0.11.9" +version = "0.11.10" dependencies = [ "anyhow", "async-trait", @@ -3853,7 +3853,7 @@ dependencies = [ [[package]] name = "mithril-doc" -version = "0.1.17" +version = "0.1.18" dependencies = [ "clap", "config", @@ -3986,7 +3986,7 @@ dependencies = [ [[package]] name = "mithril-signer" -version = "0.2.234" +version = "0.2.235" dependencies = [ "anyhow", "async-trait", @@ -4023,7 +4023,7 @@ dependencies = [ [[package]] name = "mithril-stm" -version = "0.3.39" +version = "0.3.40" dependencies = [ "bincode", "blake2 0.10.6", diff --git a/internal/mithril-doc/Cargo.toml b/internal/mithril-doc/Cargo.toml index d0d5ed1a2fa..19ece0b1d32 100644 --- a/internal/mithril-doc/Cargo.toml +++ b/internal/mithril-doc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-doc" -version = "0.1.17" +version = "0.1.18" description = "An internal crate to generate documentation." authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 2a2d4936748..c216ff5ee52 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.7.17" +version = "0.7.18" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml index eda9c339a2f..1d3c3396e3c 100644 --- a/mithril-client-cli/Cargo.toml +++ b/mithril-client-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-client-cli" -version = "0.11.9" +version = "0.11.10" description = "A Mithril Client" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 6457b4bbb3f..4d6f8fc37c1 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-signer" -version = "0.2.234" +version = "0.2.235" description = "A Mithril Signer" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-stm/Cargo.toml b/mithril-stm/Cargo.toml index 3f9734aec38..a41dd28183d 100644 --- a/mithril-stm/Cargo.toml +++ b/mithril-stm/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-stm" -version = "0.3.39" +version = "0.3.40" edition = { workspace = true } authors = { workspace = true } homepage = { workspace = true }