From bb8e6999ed4242822f4c762e8c1c16b6fa272456 Mon Sep 17 00:00:00 2001 From: Dan Wiebe Date: Mon, 27 Oct 2025 22:49:04 -0400 Subject: [PATCH 1/3] Functionality and tests. What's the big picture now? --- .gitignore | 1 + masq_lib/src/constants.rs | 2 +- node/src/database/db_initializer.rs | 9 +- .../src/database/db_migrations/db_migrator.rs | 2 + .../migrations/migration_11_to_12.rs | 73 +++++++ .../database/db_migrations/migrations/mod.rs | 3 +- node/src/db_config/config_dao_null.rs | 6 +- .../src/db_config/persistent_configuration.rs | 191 ++++++++++++++++++ .../unprivileged_parse_args_configuration.rs | 76 ++++++- node/src/sub_lib/neighborhood.rs | 16 ++ node/src/test_utils/mod.rs | 6 +- .../persistent_configuration_mock.rs | 11 + 12 files changed, 389 insertions(+), 7 deletions(-) create mode 100644 node/src/database/db_migrations/migrations/migration_11_to_12.rs diff --git a/.gitignore b/.gitignore index 88a2ab5f90..bdf669f129 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ **/*.rs.bk .idea/azure/ .idea/inspectionProfiles/Project_Default.xml +.idea/copilot.* ### Node node_modules diff --git a/masq_lib/src/constants.rs b/masq_lib/src/constants.rs index a67f86128c..0c1a640fb3 100644 --- a/masq_lib/src/constants.rs +++ b/masq_lib/src/constants.rs @@ -5,7 +5,7 @@ use crate::data_version::DataVersion; use const_format::concatcp; pub const DEFAULT_CHAIN: Chain = Chain::BaseMainnet; -pub const CURRENT_SCHEMA_VERSION: usize = 11; +pub const CURRENT_SCHEMA_VERSION: usize = 12; pub const HIGHEST_RANDOM_CLANDESTINE_PORT: u16 = 9999; pub const HTTP_PORT: u16 = 80; diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index cd5c5b1bb5..f741b974f9 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -5,7 +5,7 @@ use crate::database::db_migrations::db_migrator::{DbMigrator, DbMigratorReal}; use crate::db_config::secure_config_layer::EXAMPLE_ENCRYPTED; use crate::neighborhood::DEFAULT_MIN_HOPS; use crate::sub_lib::accountant::{DEFAULT_PAYMENT_THRESHOLDS, DEFAULT_SCAN_INTERVALS}; -use crate::sub_lib::neighborhood::DEFAULT_RATE_PACK; +use crate::sub_lib::neighborhood::{DEFAULT_RATE_PACK, DEFAULT_RATE_PACK_LIMITS}; use crate::sub_lib::utils::db_connection_launch_panic; use masq_lib::blockchains::chains::Chain; use masq_lib::constants::{ @@ -256,6 +256,13 @@ impl DbInitializerReal { false, "rate pack", ); + Self::set_config_value( + conn, + "rate_pack_limits", + Some(DEFAULT_RATE_PACK_LIMITS), + false, + "rate pack limits", + ); Self::set_config_value( conn, "scan_intervals", diff --git a/node/src/database/db_migrations/db_migrator.rs b/node/src/database/db_migrations/db_migrator.rs index 369a78788f..0182875093 100644 --- a/node/src/database/db_migrations/db_migrator.rs +++ b/node/src/database/db_migrations/db_migrator.rs @@ -17,6 +17,7 @@ use crate::database::db_migrations::migrator_utils::{ }; use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use masq_lib::logger::Logger; +use crate::database::db_migrations::migrations::migration_11_to_12::Migrate_11_to_12; pub trait DbMigrator { fn migrate_database( @@ -82,6 +83,7 @@ impl DbMigratorReal { &Migrate_8_to_9, &Migrate_9_to_10, &Migrate_10_to_11, + &Migrate_11_to_12, ] } diff --git a/node/src/database/db_migrations/migrations/migration_11_to_12.rs b/node/src/database/db_migrations/migrations/migration_11_to_12.rs new file mode 100644 index 0000000000..f2ff17b92f --- /dev/null +++ b/node/src/database/db_migrations/migrations/migration_11_to_12.rs @@ -0,0 +1,73 @@ +// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. + +use crate::database::db_migrations::db_migrator::DatabaseMigration; +use crate::database::db_migrations::migrator_utils::DBMigDeclarator; +use crate::sub_lib::neighborhood::DEFAULT_RATE_PACK_LIMITS; + +#[allow(non_camel_case_types)] +pub struct Migrate_11_to_12; + +impl DatabaseMigration for Migrate_11_to_12 { + fn migrate<'a>( + &self, + declaration_utils: Box, + ) -> rusqlite::Result<()> { + declaration_utils.execute_upon_transaction(&[ + &format!( + "INSERT INTO config (name, value, encrypted) VALUES ('rate_pack_limits', '{}', 0)", + DEFAULT_RATE_PACK_LIMITS + ) + ]) + } + + fn old_version(&self) -> usize { + 11 + } +} + +#[cfg(test)] +mod tests { + use crate::database::db_initializer::{ + DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE, + }; + use crate::test_utils::database_utils::{ + bring_db_0_back_to_life_and_return_connection, make_external_data, retrieve_config_row, + }; + use masq_lib::test_utils::utils::ensure_node_home_directory_exists; + use std::fs::create_dir_all; + + #[test] + fn migration_from_11_to_12_is_properly_set() { + let dir_path = ensure_node_home_directory_exists( + "db_migrations", + "migration_from_11_to_12_is_properly_set", + ); + create_dir_all(&dir_path).unwrap(); + let db_path = dir_path.join(DATABASE_FILE); + let _ = bring_db_0_back_to_life_and_return_connection(&db_path); + let subject = DbInitializerReal::default(); + + let result = subject.initialize_to_version( + &dir_path, + 11, + DbInitializationConfig::create_or_migrate(make_external_data()), + ); + + assert!(result.is_ok()); + + let result = subject.initialize_to_version( + &dir_path, + 12, + DbInitializationConfig::create_or_migrate(make_external_data()), + ); + + assert!(result.is_ok()); + let connection = result.unwrap(); + let (lc_value, lc_encrypted) = retrieve_config_row(connection.as_ref(), "rate_pack_limits"); + let (cs_value, cs_encrypted) = retrieve_config_row(connection.as_ref(), "schema_version"); + assert_eq!(lc_value, Some("100-100000000000000|100-100000000000000|100-100000000000000|100-100000000000000".to_string())); + assert_eq!(lc_encrypted, false); + assert_eq!(cs_value, Some("12".to_string())); + assert_eq!(cs_encrypted, false) + } +} diff --git a/node/src/database/db_migrations/migrations/mod.rs b/node/src/database/db_migrations/migrations/mod.rs index 53b7b7bb67..53733f416f 100644 --- a/node/src/database/db_migrations/migrations/mod.rs +++ b/node/src/database/db_migrations/migrations/mod.rs @@ -1,7 +1,6 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. pub mod migration_0_to_1; -pub mod migration_10_to_11; pub mod migration_1_to_2; pub mod migration_2_to_3; pub mod migration_3_to_4; @@ -11,3 +10,5 @@ pub mod migration_6_to_7; pub mod migration_7_to_8; pub mod migration_8_to_9; pub mod migration_9_to_10; +pub mod migration_10_to_11; +pub mod migration_11_to_12; diff --git a/node/src/db_config/config_dao_null.rs b/node/src/db_config/config_dao_null.rs index fd7fb7e059..32a2f249e3 100644 --- a/node/src/db_config/config_dao_null.rs +++ b/node/src/db_config/config_dao_null.rs @@ -5,7 +5,7 @@ use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use crate::db_config::config_dao::{ConfigDao, ConfigDaoError, ConfigDaoRecord}; use crate::neighborhood::DEFAULT_MIN_HOPS; use crate::sub_lib::accountant::{DEFAULT_PAYMENT_THRESHOLDS, DEFAULT_SCAN_INTERVALS}; -use crate::sub_lib::neighborhood::DEFAULT_RATE_PACK; +use crate::sub_lib::neighborhood::{DEFAULT_RATE_PACK, DEFAULT_RATE_PACK_LIMITS}; use itertools::Itertools; use masq_lib::blockchains::chains::Chain; use masq_lib::constants::{CURRENT_SCHEMA_VERSION, DEFAULT_GAS_PRICE}; @@ -138,6 +138,10 @@ impl Default for ConfigDaoNull { "rate_pack".to_string(), (Some(DEFAULT_RATE_PACK.to_string()), false), ); + data.insert( + "rate_pack_limits".to_string(), + (Some(DEFAULT_RATE_PACK_LIMITS.to_string()), false), + ); data.insert( "scan_intervals".to_string(), (Some(DEFAULT_SCAN_INTERVALS.to_string()), false), diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index ba25999ccb..4274b9c46c 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -26,6 +26,12 @@ use std::fmt::Display; use std::net::{Ipv4Addr, SocketAddrV4, TcpListener}; use std::str::FromStr; use websocket::url::Url; +use lazy_static::lazy_static; +use regex::Regex; + +lazy_static! { + static ref RATE_PACK_LIMIT_FORMAT: Regex = Regex::new(r"^(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)$").unwrap(); +} #[derive(Clone, PartialEq, Eq, Debug)] pub enum PersistentConfigError { @@ -159,6 +165,7 @@ pub trait PersistentConfiguration { fn payment_thresholds(&self) -> Result; fn set_payment_thresholds(&mut self, curves: String) -> Result<(), PersistentConfigError>; fn rate_pack(&self) -> Result; + fn rate_pack_limits(&self) -> Result<(RatePack, RatePack), PersistentConfigError>; fn set_rate_pack(&mut self, rate_pack: String) -> Result<(), PersistentConfigError>; fn scan_intervals(&self) -> Result; fn set_scan_intervals(&mut self, intervals: String) -> Result<(), PersistentConfigError>; @@ -570,6 +577,42 @@ impl PersistentConfiguration for PersistentConfigurationReal { self.simple_set_method("rate_pack", rate_pack) } + fn rate_pack_limits(&self) -> Result<(RatePack, RatePack), PersistentConfigError> { + let limits_string = self + .get("rate_pack_limits") + .expect("Required value rate_pack_limits missing from CONFIG table: database is corrupt!") + .expect("Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!"); + let captures = RATE_PACK_LIMIT_FORMAT.captures(limits_string.as_str()) + .expect("Syntax error in rate_pack_limits value 'Booga!': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei."); + let candidate = ( + RatePack { + routing_byte_rate: u64::from_str(captures.get(1).expect("blah").as_str()).expect("blah"), + routing_service_rate: u64::from_str(captures.get(3).expect("blah").as_str()).expect("blah"), + exit_byte_rate: u64::from_str(captures.get(5).expect("blah").as_str()).expect("blah"), + exit_service_rate: u64::from_str(captures.get(7).expect("blah").as_str()).expect("blah"), + }, + RatePack { + routing_byte_rate: u64::from_str(captures.get(2).expect("blah").as_str()).expect("blah"), + routing_service_rate: u64::from_str(captures.get(4).expect("blah").as_str()).expect("blah"), + exit_byte_rate: u64::from_str(captures.get(6).expect("blah").as_str()).expect("blah"), + exit_service_rate: u64::from_str(captures.get(8).expect("blah").as_str()).expect("blah"), + } + ); + let check_rate_pack_limit_order = |low: u64, high: u64, field_name: &str| { + if low >= high { + panic!( + "Rate pack limits should have low limits less than high limits, but {} limits are {}-{}", + field_name, low, high + ); + } + }; + check_rate_pack_limit_order(candidate.0.routing_byte_rate, candidate.1.routing_byte_rate, "routing_byte_rate"); + check_rate_pack_limit_order(candidate.0.routing_service_rate, candidate.1.routing_service_rate, "routing_service_rate"); + check_rate_pack_limit_order(candidate.0.exit_byte_rate, candidate.1.exit_byte_rate, "exit_byte_rate"); + check_rate_pack_limit_order(candidate.0.exit_service_rate, candidate.1.exit_service_rate, "exit_service_rate"); + Ok(candidate) + } + fn scan_intervals(&self) -> Result { self.combined_params_get_method(|str: &str| ScanIntervals::try_from(str), "scan_intervals") } @@ -2284,6 +2327,154 @@ mod tests { getter_method_plain_data_does_not_tolerate_none_value!("rate_pack"); } + #[test] + fn rate_pack_limits_works() { + persistent_config_plain_data_assertions_for_simple_get_method!( + "rate_pack_limits", + "100-200|300-400|500000000000000000-600000000000000000|700-800", + ( + RatePack { + routing_byte_rate: 100, + routing_service_rate: 300, + exit_byte_rate: 500_000_000_000_000_000, + exit_service_rate: 700, + }, + RatePack { + routing_byte_rate: 200, + routing_service_rate: 400, + exit_byte_rate: 600_000_000_000_000_000, + exit_service_rate: 800, + } + ) + ); + } + + #[test] + #[should_panic(expected = "Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!")] + fn rate_pack_limits_panics_at_none_value() { + getter_method_plain_data_does_not_tolerate_none_value!("rate_pack_limits"); + } + + #[test] + #[should_panic(expected = "Syntax error in rate_pack_limits value 'Booga!': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei.")] + fn rate_pack_limits_panics_at_syntax_error_in_value() { + persistent_config_plain_data_assertions_for_simple_get_method!( + "rate_pack_limits", + "Booga!", + // Irrelevant but necessary + ( + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + }, + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + } + ) + ); + } + + #[test] + #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but routing_byte_rate limits are 1-1")] + fn rate_pack_limits_panics_when_routing_byte_rate_limits_are_reversed() { + persistent_config_plain_data_assertions_for_simple_get_method!( + "rate_pack_limits", + "1-1|0-1|0-1|0-1", + // Irrelevant but necessary + ( + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + }, + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + } + ) + ); + } + + #[test] + #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but routing_service_rate limits are 1-1")] + fn rate_pack_limits_panics_when_routing_service_rate_limits_are_reversed() { + persistent_config_plain_data_assertions_for_simple_get_method!( + "rate_pack_limits", + "0-1|1-1|0-1|0-1", + // Irrelevant but necessary + ( + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + }, + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + } + ) + ); + } + + #[test] + #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but exit_byte_rate limits are 1-1")] + fn rate_pack_limits_panics_when_exit_byte_rate_limits_are_reversed() { + persistent_config_plain_data_assertions_for_simple_get_method!( + "rate_pack_limits", + "0-1|0-1|1-1|0-1", + // Irrelevant but necessary + ( + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + }, + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + } + ) + ); + } + + #[test] + #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but exit_service_rate limits are 1-1")] + fn rate_pack_limits_panics_when_exit_service_rate_limits_are_reversed() { + persistent_config_plain_data_assertions_for_simple_get_method!( + "rate_pack_limits", + "0-1|0-1|0-1|1-1", + // Irrelevant but necessary + ( + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + }, + RatePack { + routing_byte_rate: 0, + routing_service_rate: 0, + exit_byte_rate: 0, + exit_service_rate: 0, + } + ) + ); + } + #[test] fn scan_intervals_get_method_works() { persistent_config_plain_data_assertions_for_simple_get_method!( diff --git a/node/src/node_configurator/unprivileged_parse_args_configuration.rs b/node/src/node_configurator/unprivileged_parse_args_configuration.rs index 8ff2ed7c4c..4538c46b47 100644 --- a/node/src/node_configurator/unprivileged_parse_args_configuration.rs +++ b/node/src/node_configurator/unprivileged_parse_args_configuration.rs @@ -556,14 +556,42 @@ fn configure_rate_pack( multi_config: &MultiConfig, persist_config: &mut dyn PersistentConfiguration, ) -> Result { - process_combined_params( + let check_min_and_max = |candidate: u64, min: u64, max: u64, name: &str, error: ConfiguratorError| -> ConfiguratorError { + let mut result = error; + if candidate < min { + result = result.another_required("rate-pack", &format!("Value of {} ({}) is below the minimum allowed ({})", name, candidate, min)); + } else if candidate > max { + result = result.another_required("rate-pack", &format!("Value of {} ({}) is above the maximum allowed ({})", name, candidate, max)); + } + result + }; + match process_combined_params( "rate-pack", multi_config, persist_config, |str: &str| RatePack::try_from(str), |pc: &dyn PersistentConfiguration| pc.rate_pack(), |pc: &mut dyn PersistentConfiguration, rate_pack| pc.set_rate_pack(rate_pack), - ) + ) { + Ok(rate_pack) => { + let (low_limit, high_limit) = match persist_config.rate_pack_limits() { + Ok(pair) => pair, + Err(e) => return Err(e.into_configurator_error("rate-pack")) + }; + let mut error = ConfiguratorError::new(vec![]); + error = check_min_and_max(rate_pack.routing_byte_rate, low_limit.routing_byte_rate, high_limit.routing_byte_rate, "routing_byte_rate", error); + error = check_min_and_max(rate_pack.routing_service_rate, low_limit.routing_service_rate, high_limit.routing_service_rate, "routing_service_rate", error); + error = check_min_and_max(rate_pack.exit_byte_rate, low_limit.exit_byte_rate, high_limit.exit_byte_rate, "exit_byte_rate", error); + error = check_min_and_max(rate_pack.exit_service_rate, low_limit.exit_service_rate, high_limit.exit_service_rate, "exit_service_rate", error); + if error.len() > 0 { + Err(error) + } + else { + Ok(rate_pack) + } + }, + Err(e) => Err(e), + } } fn process_combined_params<'a, T: PartialEq, C1, C2>( @@ -2421,6 +2449,46 @@ mod tests { assert_eq!(result, expected_rate_pack) } + #[test] + fn configure_rate_pack_complains_when_minimums_are_transgressed() { + let mut persistent_config = PersistentConfigurationMock::new() + .rate_pack_result(Ok(RatePack::new(0, 0, 0, 0))) + .set_rate_pack_result(Ok(())) + .rate_pack_limits_result(Ok(( + RatePack::new(5, 5, 5, 5), + RatePack::new(7, 7, 7, 7) + ))); + + let result = configure_rate_pack(&make_simplified_multi_config(["--rate-pack", "4|4|4|4"]), &mut persistent_config); + + let expected_error = ConfiguratorError::new(vec![]) + .another_required("rate-pack", "Value of routing_byte_rate (4) is below the minimum allowed (5)") + .another_required("rate-pack", "Value of routing_service_rate (4) is below the minimum allowed (5)") + .another_required("rate-pack", "Value of exit_byte_rate (4) is below the minimum allowed (5)") + .another_required("rate-pack", "Value of exit_service_rate (4) is below the minimum allowed (5)"); + assert_eq!(result, Err(expected_error)); + } + + #[test] + fn configure_rate_pack_complains_when_maximums_are_transgressed() { + let mut persistent_config = PersistentConfigurationMock::new() + .rate_pack_result(Ok(RatePack::new(0, 0, 0, 0))) + .set_rate_pack_result(Ok(())) + .rate_pack_limits_result(Ok(( + RatePack::new(5, 5, 5, 5), + RatePack::new(7, 7, 7, 7) + ))); + + let result = configure_rate_pack(&make_simplified_multi_config(["--rate-pack", "8|8|8|8"]), &mut persistent_config); + + let expected_error = ConfiguratorError::new(vec![]) + .another_required("rate-pack", "Value of routing_byte_rate (8) is above the maximum allowed (7)") + .another_required("rate-pack", "Value of routing_service_rate (8) is above the maximum allowed (7)") + .another_required("rate-pack", "Value of exit_byte_rate (8) is above the maximum allowed (7)") + .another_required("rate-pack", "Value of exit_service_rate (8) is above the maximum allowed (7)"); + assert_eq!(result, Err(expected_error)); + } + #[test] fn compute_mapping_protocol_returns_saved_value_if_nothing_supplied() { let multi_config = make_new_multi_config( @@ -2661,6 +2729,10 @@ mod tests { .past_neighbors_result(past_neighbors_result) .mapping_protocol_result(Ok(Some(AutomapProtocol::Pcp))) .rate_pack_result(Ok(rate_pack)) + .rate_pack_limits_result(Ok(( + RatePack::new(u64::MIN, u64::MIN, u64::MIN, u64::MIN), + RatePack::new(u64::MAX, u64::MAX, u64::MAX, u64::MAX), + ))) .min_hops_result(Ok(min_hops)) } } diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index f26282aa60..e5179ab127 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -48,6 +48,8 @@ pub const ZERO_RATE_PACK: RatePack = RatePack { exit_service_rate: 0, }; +pub const DEFAULT_RATE_PACK_LIMITS: &str = "100-100000000000000|100-100000000000000|100-100000000000000|100-100000000000000"; + #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] pub struct RatePack { pub routing_byte_rate: u64, @@ -57,6 +59,20 @@ pub struct RatePack { } impl RatePack { + pub fn new( + routing_byte_rate: u64, + routing_service_rate: u64, + exit_byte_rate: u64, + exit_service_rate: u64, + ) -> Self { + Self { + routing_byte_rate, + routing_service_rate, + exit_byte_rate, + exit_service_rate, + } + } + pub fn routing_charge(&self, payload_size: u64) -> u64 { self.routing_service_rate + (self.routing_byte_rate * payload_size) } diff --git a/node/src/test_utils/mod.rs b/node/src/test_utils/mod.rs index 351b27711c..4dffff8c5f 100644 --- a/node/src/test_utils/mod.rs +++ b/node/src/test_utils/mod.rs @@ -507,7 +507,7 @@ pub mod unshared_test_utils { use crate::node_test_utils::DirsWrapperMock; use crate::sub_lib::accountant::{PaymentThresholds, ScanIntervals}; use crate::sub_lib::cryptde::CryptDE; - use crate::sub_lib::neighborhood::{ConnectionProgressMessage, DEFAULT_RATE_PACK}; + use crate::sub_lib::neighborhood::{ConnectionProgressMessage, RatePack, DEFAULT_RATE_PACK}; use crate::sub_lib::proxy_client::ClientResponsePayload_0v1; use crate::sub_lib::proxy_server::{ClientRequestPayload_0v1, ProxyProtocol}; use crate::sub_lib::sequence_buffer::SequencedPacket; @@ -631,6 +631,10 @@ pub mod unshared_test_utils { } else { config }; + let config = config.rate_pack_limits_result(Ok(( + RatePack::new(u64::MIN, u64::MIN, u64::MIN, u64::MIN), + RatePack::new(u64::MAX, u64::MAX, u64::MAX, u64::MAX) + ))); config } diff --git a/node/src/test_utils/persistent_configuration_mock.rs b/node/src/test_utils/persistent_configuration_mock.rs index 138a1deb65..ec70cb3b25 100644 --- a/node/src/test_utils/persistent_configuration_mock.rs +++ b/node/src/test_utils/persistent_configuration_mock.rs @@ -76,6 +76,7 @@ pub struct PersistentConfigurationMock { set_payment_thresholds_params: Arc>>, set_payment_thresholds_results: RefCell>>, rate_pack_results: RefCell>>, + rate_pack_limits_results: RefCell>>, set_rate_pack_params: Arc>>, set_rate_pack_results: RefCell>>, scan_intervals_results: RefCell>>, @@ -152,6 +153,7 @@ impl Clone for PersistentConfigurationMock { set_payment_thresholds_params: self.set_payment_thresholds_params.clone(), set_payment_thresholds_results: self.set_payment_thresholds_results.clone(), rate_pack_results: self.rate_pack_results.clone(), + rate_pack_limits_results: self.rate_pack_limits_results.clone(), set_rate_pack_params: self.set_rate_pack_params.clone(), set_rate_pack_results: self.set_rate_pack_results.clone(), scan_intervals_results: self.scan_intervals_results.clone(), @@ -402,6 +404,10 @@ impl PersistentConfiguration for PersistentConfigurationMock { self.set_rate_pack_results.borrow_mut().remove(0) } + fn rate_pack_limits(&self) -> Result<(RatePack, RatePack), PersistentConfigError> { + self.rate_pack_limits_results.borrow_mut().remove(0) + } + fn scan_intervals(&self) -> Result { self.scan_intervals_results.borrow_mut().remove(0) } @@ -759,6 +765,11 @@ impl PersistentConfigurationMock { self } + pub fn rate_pack_limits_result(self, result: Result<(RatePack, RatePack), PersistentConfigError>) -> Self { + self.rate_pack_limits_results.borrow_mut().push(result); + self + } + pub fn set_rate_pack_params(mut self, params: &Arc>>) -> Self { self.set_rate_pack_params = params.clone(); self From 61d70f53144487216006a5bd238c858e456e07fe Mon Sep 17 00:00:00 2001 From: Dan Wiebe Date: Tue, 28 Oct 2025 08:15:56 -0400 Subject: [PATCH 2/3] I think that might be it --- node/src/daemon/setup_reporter.rs | 26 ++-- node/src/database/db_initializer.rs | 14 +- .../src/database/db_migrations/db_migrator.rs | 2 +- .../migrations/migration_11_to_12.rs | 18 ++- .../database/db_migrations/migrations/mod.rs | 4 +- .../src/db_config/persistent_configuration.rs | 89 +++++++++---- .../unprivileged_parse_args_configuration.rs | 124 +++++++++++++----- node/src/sub_lib/neighborhood.rs | 3 +- node/src/test_utils/mod.rs | 2 +- .../persistent_configuration_mock.rs | 5 +- 10 files changed, 207 insertions(+), 80 deletions(-) diff --git a/node/src/daemon/setup_reporter.rs b/node/src/daemon/setup_reporter.rs index a7448eba44..84502ed367 100644 --- a/node/src/daemon/setup_reporter.rs +++ b/node/src/daemon/setup_reporter.rs @@ -1518,7 +1518,7 @@ mod tests { ("neighborhood-mode", "originate-only", Set), ("neighbors", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678", Set), ("payment-thresholds","1234|50000|1000|1000|20000|20000",Set), - ("rate-pack","1|3|3|8",Set), + ("rate-pack","100|300|300|800",Set), #[cfg(not(target_os = "windows"))] ("real-user", "9999:9999:booga", Set), ("scan-intervals","150|150|150",Set), @@ -1548,7 +1548,7 @@ mod tests { ("neighborhood-mode", "originate-only", Set), ("neighbors", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678", Set), ("payment-thresholds","1234|50000|1000|1000|20000|20000",Set), - ("rate-pack","1|3|3|8",Set), + ("rate-pack","100|300|300|800",Set), #[cfg(not(target_os = "windows"))] ("real-user", "9999:9999:booga", Set), ("scan-intervals","150|150|150",Set), @@ -1588,7 +1588,7 @@ mod tests { ("neighborhood-mode", "originate-only"), ("neighbors", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678"), ("payment-thresholds","1234|50000|1000|1000|15000|15000"), - ("rate-pack","1|3|3|8"), + ("rate-pack","100|300|300|800"), #[cfg(not(target_os = "windows"))] ("real-user", "9999:9999:booga"), ("scan-intervals","140|130|150"), @@ -1623,7 +1623,7 @@ mod tests { ("neighborhood-mode", "originate-only", Set), ("neighbors", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678", Set), ("payment-thresholds","1234|50000|1000|1000|15000|15000",Set), - ("rate-pack","1|3|3|8",Set), + ("rate-pack","100|300|300|800",Set), #[cfg(not(target_os = "windows"))] ("real-user", "9999:9999:booga", Set), ("scan-intervals","140|130|150",Set), @@ -1664,7 +1664,7 @@ mod tests { ("MASQ_NEIGHBORHOOD_MODE", "originate-only"), ("MASQ_NEIGHBORS", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678"), ("MASQ_PAYMENT_THRESHOLDS","12345|50000|1000|1234|19000|20000"), - ("MASQ_RATE_PACK","1|3|3|8"), + ("MASQ_RATE_PACK","100|300|300|800"), #[cfg(not(target_os = "windows"))] ("MASQ_REAL_USER", "9999:9999:booga"), ("MASQ_SCANS", "off"), @@ -1696,7 +1696,7 @@ mod tests { ("neighborhood-mode", "originate-only", Configured), ("neighbors", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678", Configured), ("payment-thresholds","12345|50000|1000|1234|19000|20000",Configured), - ("rate-pack","1|3|3|8",Configured), + ("rate-pack","100|300|300|800",Configured), #[cfg(not(target_os = "windows"))] ("real-user", "9999:9999:booga", Configured), ("scan-intervals","133|133|111",Configured), @@ -1756,7 +1756,9 @@ mod tests { .write_all(b"neighborhood-mode = \"standard\"\n") .unwrap(); config_file.write_all(b"scans = \"off\"\n").unwrap(); - config_file.write_all(b"rate-pack = \"2|2|2|2\"\n").unwrap(); + config_file + .write_all(b"rate-pack = \"200|200|200|200\"\n") + .unwrap(); config_file .write_all(b"payment-thresholds = \"3333|55|33|646|999|999\"\n") .unwrap(); @@ -1799,7 +1801,7 @@ mod tests { .unwrap(); config_file.write_all(b"scans = \"off\"\n").unwrap(); config_file - .write_all(b"rate-pack = \"55|50|60|61\"\n") + .write_all(b"rate-pack = \"5500|5000|6000|6100\"\n") .unwrap(); config_file .write_all(b"payment-thresholds = \"4000|1000|3000|3333|10000|20000\"\n") @@ -1864,7 +1866,7 @@ mod tests { "4000|1000|3000|3333|10000|20000", Configured, ), - ("rate-pack", "55|50|60|61", Configured), + ("rate-pack", "5500|5000|6000|6100", Configured), #[cfg(not(target_os = "windows"))] ( "real-user", @@ -1914,7 +1916,7 @@ mod tests { ("MASQ_NEIGHBORHOOD_MODE", "originate-only"), ("MASQ_NEIGHBORS", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678"), ("MASQ_PAYMENT_THRESHOLDS","1234|50000|1000|1000|20000|20000"), - ("MASQ_RATE_PACK","1|3|3|8"), + ("MASQ_RATE_PACK","100|300|300|800"), #[cfg(not(target_os = "windows"))] ("MASQ_REAL_USER", "9999:9999:booga"), ("MASQ_SCANS", "off"), @@ -1977,7 +1979,7 @@ mod tests { Set, ), ("payment-thresholds", "4321|66666|777|987|123456|124444", Set), - ("rate-pack", "10|30|13|28", Set), + ("rate-pack", "1000|3000|1300|2800", Set), #[cfg(not(target_os = "windows"))] ("real-user", "6666:6666:agoob", Set), ("scan-intervals", "111|111|111", Set), @@ -2011,7 +2013,7 @@ mod tests { ("neighborhood-mode", "originate-only", Configured), ("neighbors", "masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@1.2.3.4:1234,masq://base-sepolia:MTIzNDU2Nzg5MTEyMzQ1Njc4OTIxMjM0NTY3ODkzMTI@5.6.7.8:5678", Configured), ("payment-thresholds","1234|50000|1000|1000|20000|20000",Configured), - ("rate-pack","1|3|3|8",Configured), + ("rate-pack","100|300|300|800",Configured), #[cfg(not(target_os = "windows"))] ("real-user", "9999:9999:booga", Configured), ("scan-intervals","150|150|155",Configured), diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index f741b974f9..0673a3035f 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -668,7 +668,7 @@ mod tests { #[test] fn constants_have_correct_values() { assert_eq!(DATABASE_FILE, "node-data.db"); - assert_eq!(CURRENT_SCHEMA_VERSION, 11); + assert_eq!(CURRENT_SCHEMA_VERSION, 12); } #[test] @@ -966,6 +966,12 @@ mod tests { Some(&DEFAULT_RATE_PACK.to_string()), false, ); + verify( + &mut config_vec, + "rate_pack_limits", + Some(DEFAULT_RATE_PACK_LIMITS), + false, + ); verify( &mut config_vec, "scan_intervals", @@ -1078,6 +1084,12 @@ mod tests { Some(&DEFAULT_RATE_PACK.to_string()), false, ); + verify( + &mut config_vec, + "rate_pack_limits", + Some(DEFAULT_RATE_PACK_LIMITS), + false, + ); verify( &mut config_vec, "scan_intervals", diff --git a/node/src/database/db_migrations/db_migrator.rs b/node/src/database/db_migrations/db_migrator.rs index 0182875093..f3a3252565 100644 --- a/node/src/database/db_migrations/db_migrator.rs +++ b/node/src/database/db_migrations/db_migrator.rs @@ -3,6 +3,7 @@ use crate::database::db_initializer::ExternalData; use crate::database::db_migrations::migrations::migration_0_to_1::Migrate_0_to_1; use crate::database::db_migrations::migrations::migration_10_to_11::Migrate_10_to_11; +use crate::database::db_migrations::migrations::migration_11_to_12::Migrate_11_to_12; use crate::database::db_migrations::migrations::migration_1_to_2::Migrate_1_to_2; use crate::database::db_migrations::migrations::migration_2_to_3::Migrate_2_to_3; use crate::database::db_migrations::migrations::migration_3_to_4::Migrate_3_to_4; @@ -17,7 +18,6 @@ use crate::database::db_migrations::migrator_utils::{ }; use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use masq_lib::logger::Logger; -use crate::database::db_migrations::migrations::migration_11_to_12::Migrate_11_to_12; pub trait DbMigrator { fn migrate_database( diff --git a/node/src/database/db_migrations/migrations/migration_11_to_12.rs b/node/src/database/db_migrations/migrations/migration_11_to_12.rs index f2ff17b92f..87261688c2 100644 --- a/node/src/database/db_migrations/migrations/migration_11_to_12.rs +++ b/node/src/database/db_migrations/migrations/migration_11_to_12.rs @@ -12,12 +12,10 @@ impl DatabaseMigration for Migrate_11_to_12 { &self, declaration_utils: Box, ) -> rusqlite::Result<()> { - declaration_utils.execute_upon_transaction(&[ - &format!( - "INSERT INTO config (name, value, encrypted) VALUES ('rate_pack_limits', '{}', 0)", - DEFAULT_RATE_PACK_LIMITS - ) - ]) + declaration_utils.execute_upon_transaction(&[&format!( + "INSERT INTO config (name, value, encrypted) VALUES ('rate_pack_limits', '{}', 0)", + DEFAULT_RATE_PACK_LIMITS + )]) } fn old_version(&self) -> usize { @@ -65,7 +63,13 @@ mod tests { let connection = result.unwrap(); let (lc_value, lc_encrypted) = retrieve_config_row(connection.as_ref(), "rate_pack_limits"); let (cs_value, cs_encrypted) = retrieve_config_row(connection.as_ref(), "schema_version"); - assert_eq!(lc_value, Some("100-100000000000000|100-100000000000000|100-100000000000000|100-100000000000000".to_string())); + assert_eq!( + lc_value, + Some( + "100-100000000000000|100-100000000000000|100-100000000000000|100-100000000000000" + .to_string() + ) + ); assert_eq!(lc_encrypted, false); assert_eq!(cs_value, Some("12".to_string())); assert_eq!(cs_encrypted, false) diff --git a/node/src/database/db_migrations/migrations/mod.rs b/node/src/database/db_migrations/migrations/mod.rs index 53733f416f..23c395d108 100644 --- a/node/src/database/db_migrations/migrations/mod.rs +++ b/node/src/database/db_migrations/migrations/mod.rs @@ -1,6 +1,8 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. pub mod migration_0_to_1; +pub mod migration_10_to_11; +pub mod migration_11_to_12; pub mod migration_1_to_2; pub mod migration_2_to_3; pub mod migration_3_to_4; @@ -10,5 +12,3 @@ pub mod migration_6_to_7; pub mod migration_7_to_8; pub mod migration_8_to_9; pub mod migration_9_to_10; -pub mod migration_10_to_11; -pub mod migration_11_to_12; diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index 4274b9c46c..1c8acd666c 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -16,21 +16,22 @@ use crate::sub_lib::cryptde_null::CryptDENull; use crate::sub_lib::cryptde_real::CryptDEReal; use crate::sub_lib::neighborhood::{Hops, NodeDescriptor, RatePack}; use crate::sub_lib::wallet::Wallet; +use lazy_static::lazy_static; use masq_lib::blockchains::chains::Chain; use masq_lib::constants::{HIGHEST_USABLE_PORT, LOWEST_USABLE_INSECURE_PORT}; use masq_lib::shared_schema::{ConfiguratorError, ParamError}; use masq_lib::utils::NeighborhoodModeLight; use masq_lib::utils::{to_string, AutomapProtocol}; +use regex::Regex; use rustc_hex::{FromHex, ToHex}; use std::fmt::Display; use std::net::{Ipv4Addr, SocketAddrV4, TcpListener}; use std::str::FromStr; use websocket::url::Url; -use lazy_static::lazy_static; -use regex::Regex; lazy_static! { - static ref RATE_PACK_LIMIT_FORMAT: Regex = Regex::new(r"^(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)$").unwrap(); + static ref RATE_PACK_LIMIT_FORMAT: Regex = + Regex::new(r"^(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)$").unwrap(); } #[derive(Clone, PartialEq, Eq, Debug)] @@ -580,23 +581,35 @@ impl PersistentConfiguration for PersistentConfigurationReal { fn rate_pack_limits(&self) -> Result<(RatePack, RatePack), PersistentConfigError> { let limits_string = self .get("rate_pack_limits") - .expect("Required value rate_pack_limits missing from CONFIG table: database is corrupt!") - .expect("Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!"); + .expect( + "Required value rate_pack_limits missing from CONFIG table: database is corrupt!", + ) + .expect( + "Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!", + ); let captures = RATE_PACK_LIMIT_FORMAT.captures(limits_string.as_str()) .expect("Syntax error in rate_pack_limits value 'Booga!': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei."); let candidate = ( RatePack { - routing_byte_rate: u64::from_str(captures.get(1).expect("blah").as_str()).expect("blah"), - routing_service_rate: u64::from_str(captures.get(3).expect("blah").as_str()).expect("blah"), - exit_byte_rate: u64::from_str(captures.get(5).expect("blah").as_str()).expect("blah"), - exit_service_rate: u64::from_str(captures.get(7).expect("blah").as_str()).expect("blah"), + routing_byte_rate: u64::from_str(captures.get(1).expect("blah").as_str()) + .expect("blah"), + routing_service_rate: u64::from_str(captures.get(3).expect("blah").as_str()) + .expect("blah"), + exit_byte_rate: u64::from_str(captures.get(5).expect("blah").as_str()) + .expect("blah"), + exit_service_rate: u64::from_str(captures.get(7).expect("blah").as_str()) + .expect("blah"), }, RatePack { - routing_byte_rate: u64::from_str(captures.get(2).expect("blah").as_str()).expect("blah"), - routing_service_rate: u64::from_str(captures.get(4).expect("blah").as_str()).expect("blah"), - exit_byte_rate: u64::from_str(captures.get(6).expect("blah").as_str()).expect("blah"), - exit_service_rate: u64::from_str(captures.get(8).expect("blah").as_str()).expect("blah"), - } + routing_byte_rate: u64::from_str(captures.get(2).expect("blah").as_str()) + .expect("blah"), + routing_service_rate: u64::from_str(captures.get(4).expect("blah").as_str()) + .expect("blah"), + exit_byte_rate: u64::from_str(captures.get(6).expect("blah").as_str()) + .expect("blah"), + exit_service_rate: u64::from_str(captures.get(8).expect("blah").as_str()) + .expect("blah"), + }, ); let check_rate_pack_limit_order = |low: u64, high: u64, field_name: &str| { if low >= high { @@ -606,10 +619,26 @@ impl PersistentConfiguration for PersistentConfigurationReal { ); } }; - check_rate_pack_limit_order(candidate.0.routing_byte_rate, candidate.1.routing_byte_rate, "routing_byte_rate"); - check_rate_pack_limit_order(candidate.0.routing_service_rate, candidate.1.routing_service_rate, "routing_service_rate"); - check_rate_pack_limit_order(candidate.0.exit_byte_rate, candidate.1.exit_byte_rate, "exit_byte_rate"); - check_rate_pack_limit_order(candidate.0.exit_service_rate, candidate.1.exit_service_rate, "exit_service_rate"); + check_rate_pack_limit_order( + candidate.0.routing_byte_rate, + candidate.1.routing_byte_rate, + "routing_byte_rate", + ); + check_rate_pack_limit_order( + candidate.0.routing_service_rate, + candidate.1.routing_service_rate, + "routing_service_rate", + ); + check_rate_pack_limit_order( + candidate.0.exit_byte_rate, + candidate.1.exit_byte_rate, + "exit_byte_rate", + ); + check_rate_pack_limit_order( + candidate.0.exit_service_rate, + candidate.1.exit_service_rate, + "exit_service_rate", + ); Ok(candidate) } @@ -2350,13 +2379,17 @@ mod tests { } #[test] - #[should_panic(expected = "Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!")] + #[should_panic( + expected = "Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!" + )] fn rate_pack_limits_panics_at_none_value() { getter_method_plain_data_does_not_tolerate_none_value!("rate_pack_limits"); } #[test] - #[should_panic(expected = "Syntax error in rate_pack_limits value 'Booga!': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei.")] + #[should_panic( + expected = "Syntax error in rate_pack_limits value 'Booga!': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei." + )] fn rate_pack_limits_panics_at_syntax_error_in_value() { persistent_config_plain_data_assertions_for_simple_get_method!( "rate_pack_limits", @@ -2380,7 +2413,9 @@ mod tests { } #[test] - #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but routing_byte_rate limits are 1-1")] + #[should_panic( + expected = "Rate pack limits should have low limits less than high limits, but routing_byte_rate limits are 1-1" + )] fn rate_pack_limits_panics_when_routing_byte_rate_limits_are_reversed() { persistent_config_plain_data_assertions_for_simple_get_method!( "rate_pack_limits", @@ -2404,7 +2439,9 @@ mod tests { } #[test] - #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but routing_service_rate limits are 1-1")] + #[should_panic( + expected = "Rate pack limits should have low limits less than high limits, but routing_service_rate limits are 1-1" + )] fn rate_pack_limits_panics_when_routing_service_rate_limits_are_reversed() { persistent_config_plain_data_assertions_for_simple_get_method!( "rate_pack_limits", @@ -2428,7 +2465,9 @@ mod tests { } #[test] - #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but exit_byte_rate limits are 1-1")] + #[should_panic( + expected = "Rate pack limits should have low limits less than high limits, but exit_byte_rate limits are 1-1" + )] fn rate_pack_limits_panics_when_exit_byte_rate_limits_are_reversed() { persistent_config_plain_data_assertions_for_simple_get_method!( "rate_pack_limits", @@ -2452,7 +2491,9 @@ mod tests { } #[test] - #[should_panic(expected = "Rate pack limits should have low limits less than high limits, but exit_service_rate limits are 1-1")] + #[should_panic( + expected = "Rate pack limits should have low limits less than high limits, but exit_service_rate limits are 1-1" + )] fn rate_pack_limits_panics_when_exit_service_rate_limits_are_reversed() { persistent_config_plain_data_assertions_for_simple_get_method!( "rate_pack_limits", diff --git a/node/src/node_configurator/unprivileged_parse_args_configuration.rs b/node/src/node_configurator/unprivileged_parse_args_configuration.rs index 4538c46b47..7e0f68ef3b 100644 --- a/node/src/node_configurator/unprivileged_parse_args_configuration.rs +++ b/node/src/node_configurator/unprivileged_parse_args_configuration.rs @@ -556,12 +556,29 @@ fn configure_rate_pack( multi_config: &MultiConfig, persist_config: &mut dyn PersistentConfiguration, ) -> Result { - let check_min_and_max = |candidate: u64, min: u64, max: u64, name: &str, error: ConfiguratorError| -> ConfiguratorError { + let check_min_and_max = |candidate: u64, + min: u64, + max: u64, + name: &str, + error: ConfiguratorError| + -> ConfiguratorError { let mut result = error; if candidate < min { - result = result.another_required("rate-pack", &format!("Value of {} ({}) is below the minimum allowed ({})", name, candidate, min)); + result = result.another_required( + "rate-pack", + &format!( + "Value of {} ({}) is below the minimum allowed ({})", + name, candidate, min + ), + ); } else if candidate > max { - result = result.another_required("rate-pack", &format!("Value of {} ({}) is above the maximum allowed ({})", name, candidate, max)); + result = result.another_required( + "rate-pack", + &format!( + "Value of {} ({}) is above the maximum allowed ({})", + name, candidate, max + ), + ); } result }; @@ -576,20 +593,43 @@ fn configure_rate_pack( Ok(rate_pack) => { let (low_limit, high_limit) = match persist_config.rate_pack_limits() { Ok(pair) => pair, - Err(e) => return Err(e.into_configurator_error("rate-pack")) + Err(e) => return Err(e.into_configurator_error("rate-pack")), }; let mut error = ConfiguratorError::new(vec![]); - error = check_min_and_max(rate_pack.routing_byte_rate, low_limit.routing_byte_rate, high_limit.routing_byte_rate, "routing_byte_rate", error); - error = check_min_and_max(rate_pack.routing_service_rate, low_limit.routing_service_rate, high_limit.routing_service_rate, "routing_service_rate", error); - error = check_min_and_max(rate_pack.exit_byte_rate, low_limit.exit_byte_rate, high_limit.exit_byte_rate, "exit_byte_rate", error); - error = check_min_and_max(rate_pack.exit_service_rate, low_limit.exit_service_rate, high_limit.exit_service_rate, "exit_service_rate", error); - if error.len() > 0 { + error = check_min_and_max( + rate_pack.routing_byte_rate, + low_limit.routing_byte_rate, + high_limit.routing_byte_rate, + "routing_byte_rate", + error, + ); + error = check_min_and_max( + rate_pack.routing_service_rate, + low_limit.routing_service_rate, + high_limit.routing_service_rate, + "routing_service_rate", + error, + ); + error = check_min_and_max( + rate_pack.exit_byte_rate, + low_limit.exit_byte_rate, + high_limit.exit_byte_rate, + "exit_byte_rate", + error, + ); + error = check_min_and_max( + rate_pack.exit_service_rate, + low_limit.exit_service_rate, + high_limit.exit_service_rate, + "exit_service_rate", + error, + ); + if !error.is_empty() { Err(error) - } - else { + } else { Ok(rate_pack) } - }, + } Err(e) => Err(e), } } @@ -2454,18 +2494,30 @@ mod tests { let mut persistent_config = PersistentConfigurationMock::new() .rate_pack_result(Ok(RatePack::new(0, 0, 0, 0))) .set_rate_pack_result(Ok(())) - .rate_pack_limits_result(Ok(( - RatePack::new(5, 5, 5, 5), - RatePack::new(7, 7, 7, 7) - ))); + .rate_pack_limits_result(Ok((RatePack::new(5, 5, 5, 5), RatePack::new(7, 7, 7, 7)))); - let result = configure_rate_pack(&make_simplified_multi_config(["--rate-pack", "4|4|4|4"]), &mut persistent_config); + let result = configure_rate_pack( + &make_simplified_multi_config(["--rate-pack", "4|4|4|4"]), + &mut persistent_config, + ); let expected_error = ConfiguratorError::new(vec![]) - .another_required("rate-pack", "Value of routing_byte_rate (4) is below the minimum allowed (5)") - .another_required("rate-pack", "Value of routing_service_rate (4) is below the minimum allowed (5)") - .another_required("rate-pack", "Value of exit_byte_rate (4) is below the minimum allowed (5)") - .another_required("rate-pack", "Value of exit_service_rate (4) is below the minimum allowed (5)"); + .another_required( + "rate-pack", + "Value of routing_byte_rate (4) is below the minimum allowed (5)", + ) + .another_required( + "rate-pack", + "Value of routing_service_rate (4) is below the minimum allowed (5)", + ) + .another_required( + "rate-pack", + "Value of exit_byte_rate (4) is below the minimum allowed (5)", + ) + .another_required( + "rate-pack", + "Value of exit_service_rate (4) is below the minimum allowed (5)", + ); assert_eq!(result, Err(expected_error)); } @@ -2474,18 +2526,30 @@ mod tests { let mut persistent_config = PersistentConfigurationMock::new() .rate_pack_result(Ok(RatePack::new(0, 0, 0, 0))) .set_rate_pack_result(Ok(())) - .rate_pack_limits_result(Ok(( - RatePack::new(5, 5, 5, 5), - RatePack::new(7, 7, 7, 7) - ))); + .rate_pack_limits_result(Ok((RatePack::new(5, 5, 5, 5), RatePack::new(7, 7, 7, 7)))); - let result = configure_rate_pack(&make_simplified_multi_config(["--rate-pack", "8|8|8|8"]), &mut persistent_config); + let result = configure_rate_pack( + &make_simplified_multi_config(["--rate-pack", "8|8|8|8"]), + &mut persistent_config, + ); let expected_error = ConfiguratorError::new(vec![]) - .another_required("rate-pack", "Value of routing_byte_rate (8) is above the maximum allowed (7)") - .another_required("rate-pack", "Value of routing_service_rate (8) is above the maximum allowed (7)") - .another_required("rate-pack", "Value of exit_byte_rate (8) is above the maximum allowed (7)") - .another_required("rate-pack", "Value of exit_service_rate (8) is above the maximum allowed (7)"); + .another_required( + "rate-pack", + "Value of routing_byte_rate (8) is above the maximum allowed (7)", + ) + .another_required( + "rate-pack", + "Value of routing_service_rate (8) is above the maximum allowed (7)", + ) + .another_required( + "rate-pack", + "Value of exit_byte_rate (8) is above the maximum allowed (7)", + ) + .another_required( + "rate-pack", + "Value of exit_service_rate (8) is above the maximum allowed (7)", + ); assert_eq!(result, Err(expected_error)); } diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index e5179ab127..448c3e50ee 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -48,7 +48,8 @@ pub const ZERO_RATE_PACK: RatePack = RatePack { exit_service_rate: 0, }; -pub const DEFAULT_RATE_PACK_LIMITS: &str = "100-100000000000000|100-100000000000000|100-100000000000000|100-100000000000000"; +pub const DEFAULT_RATE_PACK_LIMITS: &str = + "100-100000000000000|100-100000000000000|100-100000000000000|100-100000000000000"; #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] pub struct RatePack { diff --git a/node/src/test_utils/mod.rs b/node/src/test_utils/mod.rs index 4dffff8c5f..7c78d99a93 100644 --- a/node/src/test_utils/mod.rs +++ b/node/src/test_utils/mod.rs @@ -633,7 +633,7 @@ pub mod unshared_test_utils { }; let config = config.rate_pack_limits_result(Ok(( RatePack::new(u64::MIN, u64::MIN, u64::MIN, u64::MIN), - RatePack::new(u64::MAX, u64::MAX, u64::MAX, u64::MAX) + RatePack::new(u64::MAX, u64::MAX, u64::MAX, u64::MAX), ))); config } diff --git a/node/src/test_utils/persistent_configuration_mock.rs b/node/src/test_utils/persistent_configuration_mock.rs index ec70cb3b25..afcb7428d7 100644 --- a/node/src/test_utils/persistent_configuration_mock.rs +++ b/node/src/test_utils/persistent_configuration_mock.rs @@ -765,7 +765,10 @@ impl PersistentConfigurationMock { self } - pub fn rate_pack_limits_result(self, result: Result<(RatePack, RatePack), PersistentConfigError>) -> Self { + pub fn rate_pack_limits_result( + self, + result: Result<(RatePack, RatePack), PersistentConfigError>, + ) -> Self { self.rate_pack_limits_results.borrow_mut().push(result); self } From 6f57d99bc0ddf10e674d6e8b10016d782a50a5c4 Mon Sep 17 00:00:00 2001 From: Dan Wiebe Date: Thu, 30 Oct 2025 07:34:25 -0400 Subject: [PATCH 3/3] Review issues --- .../src/db_config/persistent_configuration.rs | 72 ++++++++++--------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index 1c8acd666c..5119be6d2a 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -22,7 +22,7 @@ use masq_lib::constants::{HIGHEST_USABLE_PORT, LOWEST_USABLE_INSECURE_PORT}; use masq_lib::shared_schema::{ConfiguratorError, ParamError}; use masq_lib::utils::NeighborhoodModeLight; use masq_lib::utils::{to_string, AutomapProtocol}; -use regex::Regex; +use regex::{Captures, Regex}; use rustc_hex::{FromHex, ToHex}; use std::fmt::Display; use std::net::{Ipv4Addr, SocketAddrV4, TcpListener}; @@ -31,7 +31,7 @@ use websocket::url::Url; lazy_static! { static ref RATE_PACK_LIMIT_FORMAT: Regex = - Regex::new(r"^(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)\|(\d+)-(\d+)$").unwrap(); + Regex::new(r"^(\d{1,19})-(\d{1,19})\|(\d{1,19})-(\d{1,19})\|(\d{1,19})-(\d{1,19})\|(\d{1,19})-(\d{1,19})$").unwrap(); } #[derive(Clone, PartialEq, Eq, Debug)] @@ -588,53 +588,27 @@ impl PersistentConfiguration for PersistentConfigurationReal { "Required value rate_pack_limits is NULL in CONFIG table: database is corrupt!", ); let captures = RATE_PACK_LIMIT_FORMAT.captures(limits_string.as_str()) - .expect("Syntax error in rate_pack_limits value 'Booga!': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei."); + .expect(format!("Syntax error in rate_pack_limits value '{}': should be -|-|-|- where L is low, H is high, R is routing, E is exit, BR is byte rate, and SR is service rate. All numbers should be in wei.", limits_string).as_str()); let candidate = ( - RatePack { - routing_byte_rate: u64::from_str(captures.get(1).expect("blah").as_str()) - .expect("blah"), - routing_service_rate: u64::from_str(captures.get(3).expect("blah").as_str()) - .expect("blah"), - exit_byte_rate: u64::from_str(captures.get(5).expect("blah").as_str()) - .expect("blah"), - exit_service_rate: u64::from_str(captures.get(7).expect("blah").as_str()) - .expect("blah"), - }, - RatePack { - routing_byte_rate: u64::from_str(captures.get(2).expect("blah").as_str()) - .expect("blah"), - routing_service_rate: u64::from_str(captures.get(4).expect("blah").as_str()) - .expect("blah"), - exit_byte_rate: u64::from_str(captures.get(6).expect("blah").as_str()) - .expect("blah"), - exit_service_rate: u64::from_str(captures.get(8).expect("blah").as_str()) - .expect("blah"), - }, + Self::extract_candidate(&captures, 1), + Self::extract_candidate(&captures, 2) ); - let check_rate_pack_limit_order = |low: u64, high: u64, field_name: &str| { - if low >= high { - panic!( - "Rate pack limits should have low limits less than high limits, but {} limits are {}-{}", - field_name, low, high - ); - } - }; - check_rate_pack_limit_order( + Self::check_rate_pack_limit_order( candidate.0.routing_byte_rate, candidate.1.routing_byte_rate, "routing_byte_rate", ); - check_rate_pack_limit_order( + Self::check_rate_pack_limit_order( candidate.0.routing_service_rate, candidate.1.routing_service_rate, "routing_service_rate", ); - check_rate_pack_limit_order( + Self::check_rate_pack_limit_order( candidate.0.exit_byte_rate, candidate.1.exit_byte_rate, "exit_byte_rate", ); - check_rate_pack_limit_order( + Self::check_rate_pack_limit_order( candidate.0.exit_service_rate, candidate.1.exit_service_rate, "exit_service_rate", @@ -743,6 +717,34 @@ impl PersistentConfigurationReal { parameter_name ) } + + fn extract_candidate(captures: &Captures, start_index: usize) -> RatePack { + RatePack { + routing_byte_rate: Self::parse_capture(captures, start_index), + routing_service_rate: Self::parse_capture(captures, start_index + 2), + exit_byte_rate: Self::parse_capture(captures, start_index + 4), + exit_service_rate: Self::parse_capture(captures, start_index + 6), + } + } + + fn parse_capture(captures: &Captures, index: usize) -> u64 { + u64::from_str( + captures + .get(index) + .expect("Internal error: regex needs four captures") + .as_str(), + ) + .expect("Internal error: regex must require u64") + } + + fn check_rate_pack_limit_order(low: u64, high: u64, field_name: &str) { + if low >= high { + panic!( + "Rate pack limits should have low limits less than high limits, but {} limits are {}-{}", + field_name, low, high + ); + } + } } #[cfg(test)]