From c9f4a99cc7f7fdb6c7e5620dfac4f799ff65d848 Mon Sep 17 00:00:00 2001 From: Max Summe Date: Thu, 5 Sep 2024 14:30:18 -0700 Subject: [PATCH] chore(nns): reduce logging in tests that is only useful in production --- rs/nervous_system/neurons_fund/src/lib.rs | 5 +- .../src/polynomial_matching_function_tests.rs | 1 + rs/nns/governance/src/governance/test_data.rs | 1 + rs/nns/governance/src/neurons_fund.rs | 77 +++++++++++-------- .../neurons_fund_anonymization_tests.rs | 1 + rs/nns/governance/tests/fixtures/mod.rs | 4 +- rs/nns/governance/tests/governance.rs | 2 +- rs/nns/governance/tests/interleaving_tests.rs | 1 + rs/sns/swap/tests/swap.rs | 15 ++-- 9 files changed, 66 insertions(+), 41 deletions(-) diff --git a/rs/nervous_system/neurons_fund/src/lib.rs b/rs/nervous_system/neurons_fund/src/lib.rs index 15689cb1abd..f817e058a3d 100644 --- a/rs/nervous_system/neurons_fund/src/lib.rs +++ b/rs/nervous_system/neurons_fund/src/lib.rs @@ -845,6 +845,7 @@ impl PolynomialMatchingFunction { pub fn new( total_maturity_equivalent_icp_e8s: u64, neurons_fund_participation_limits: NeuronsFundParticipationLimits, + enable_logging: bool, ) -> Result { // Computations defined in ICP rather than ICP e8s to avoid multiplication overflows for // the `Decimal` type for the range of values that this type is expected to operate on. @@ -882,7 +883,9 @@ impl PolynomialMatchingFunction { cap, }; - persistent_data.log_unreachable_milestones(human_readable_cap_formula); + if enable_logging { + persistent_data.log_unreachable_milestones(human_readable_cap_formula); + } Self::from_persistent_data(persistent_data) } diff --git a/rs/nervous_system/neurons_fund/src/polynomial_matching_function_tests.rs b/rs/nervous_system/neurons_fund/src/polynomial_matching_function_tests.rs index ea449cfb38c..9af8b06a26f 100644 --- a/rs/nervous_system/neurons_fund/src/polynomial_matching_function_tests.rs +++ b/rs/nervous_system/neurons_fund/src/polynomial_matching_function_tests.rs @@ -55,6 +55,7 @@ fn polynomial_matching_function_viability_test() { let f = assert_matches!(PolynomialMatchingFunction::new( *total_maturity_equivalent_icp_e8s, neurons_fund_participation_limits, + true ), Ok(f) => f); // Check that the function can be serialized / deserialized. let f1: Box = assert_matches!( diff --git a/rs/nns/governance/src/governance/test_data.rs b/rs/nns/governance/src/governance/test_data.rs index 88df6162b09..5fc81530e88 100644 --- a/rs/nns/governance/src/governance/test_data.rs +++ b/rs/nns/governance/src/governance/test_data.rs @@ -219,6 +219,7 @@ lazy_static! { one_third_participation_milestone_icp: dec!(100_000.0), full_participation_milestone_icp: dec!(167_000.0), }, + false ).unwrap().serialize(), ), }), diff --git a/rs/nns/governance/src/neurons_fund.rs b/rs/nns/governance/src/neurons_fund.rs index b925fc95017..e62eaa6a9ab 100644 --- a/rs/nns/governance/src/neurons_fund.rs +++ b/rs/nns/governance/src/neurons_fund.rs @@ -1075,26 +1075,32 @@ where // `neurons_fund_reserves.total_amount_icp_e8s()`, the sum of (pre-rounded) `u64` values. let (neurons_fund_reserves, allocated_neurons_fund_participation_icp_e8s) = if total_maturity_equivalent_icp_e8s == 0 { - println!( - "{}WARNING: Neurons' Fund has zero total maturity.", - governance::LOG_PREFIX - ); + #[cfg(not(test))] + { + println!( + "{}WARNING: Neurons' Fund has zero total maturity.", + governance::LOG_PREFIX + ); + } (NeuronsFundSnapshot::empty(), Decimal::ZERO) } else if intended_neurons_fund_participation_icp_e8s == 0 { - println!( - "{}WARNING: intended_neurons_fund_participation_icp_e8s is zero, matching \ + #[cfg(not(test))] + { + println!( + "{}WARNING: intended_neurons_fund_participation_icp_e8s is zero, matching \ direct_participation_icp_e8s = {}. total_maturity_equivalent_icp_e8s = {}. \ ideal_matched_participation_function = {:?}\n \ Plot: \n{:?}", - governance::LOG_PREFIX, - direct_participation_icp_e8s, - total_maturity_equivalent_icp_e8s, - ideal_matched_participation_function, - ideal_matched_participation_function - .plot(NonZeroU64::try_from(50).unwrap()) - .map(|plot| format!("{:?}", plot)) - .unwrap_or_else(|e| e), - ); + governance::LOG_PREFIX, + direct_participation_icp_e8s, + total_maturity_equivalent_icp_e8s, + ideal_matched_participation_function, + ideal_matched_participation_function + .plot(NonZeroU64::try_from(50).unwrap()) + .map(|plot| format!("{:?}", plot)) + .unwrap_or_else(|e| e), + ); + } (NeuronsFundSnapshot::empty(), Decimal::ZERO) } else { // Unlike in most other places, here we keep the ICP values in e8s (even after converting @@ -1140,25 +1146,29 @@ where if ideal_participation_amount_icp_e8s < min_participant_icp_e8s || ideal_participation_amount_icp_e8s < Decimal::ONE { // Do not include neurons that cannot participate under any circumstances. - println!( - "{}INFO: discarding neuron {:?} ({} ICP e8s maturity equivalent) as it \ + #[cfg(not(test))] { + println!( + "{}INFO: discarding neuron {:?} ({} ICP e8s maturity equivalent) as it \ cannot participate in the swap with its proportional participation \ amount ({}) that is less than `min_participant_icp_e8s` ({}) or 1 e8.", - governance::LOG_PREFIX, id, maturity_equivalent_icp_e8s, - ideal_participation_amount_icp_e8s, - min_participant_icp_e8s, - ); + governance::LOG_PREFIX, id, maturity_equivalent_icp_e8s, + ideal_participation_amount_icp_e8s, + min_participant_icp_e8s, + ); + } return Ok((overall_neuron_portions, allocated_neurons_fund_participation_icp_e8s)); } let (amount_icp_e8s, is_capped) = if ideal_participation_amount_icp_e8s > max_participant_icp_e8s { - println!( - "{}INFO: capping neuron {:?} ({} ICP e8s maturity equivalent) as it \ + #[cfg(not(test))] { + println!( + "{}INFO: capping neuron {:?} ({} ICP e8s maturity equivalent) as it \ cannot participate in the swap with all of its proportional \ participation amount ({}) that exceeds `max_participant_icp_e8s` ({}).", - governance::LOG_PREFIX, id, maturity_equivalent_icp_e8s, - ideal_participation_amount_icp_e8s, - max_participant_icp_e8s, - ); + governance::LOG_PREFIX, id, maturity_equivalent_icp_e8s, + ideal_participation_amount_icp_e8s, + max_participant_icp_e8s, + ); + } (max_participant_icp_e8s, true) } else { (ideal_participation_amount_icp_e8s, false) @@ -1550,6 +1560,7 @@ impl PolynomialNeuronsFundParticipation { let ideal_matched_participation_function = Box::from(PolynomialMatchingFunction::new( total_maturity_equivalent_icp_e8s, neurons_fund_participation_limits, + cfg!(not(test)), )?); Self::new_impl( total_maturity_equivalent_icp_e8s, @@ -2335,14 +2346,14 @@ mod test_functions_tests { F: InvertibleFunction + AnalyticallyInvertibleFunction, { let Ok(expected) = function.invert_analytically(target_y) else { - println!( - "Cannot run inverse test as a u64 analytical inverse does not exist for {}.", - target_y, - ); + // println!( + // "Cannot run inverse test as a u64 analytical inverse does not exist for {}.", + // target_y, + // ); return; }; let observed = function.invert(target_y).unwrap(); - println!("{}, target_y = {target_y}", std::any::type_name::(),); + // println!("{}, target_y = {target_y}", std::any::type_name::(),); // Sometimes exact equality cannot be reached with our search strategy. We tolerate errors // up to 1 E8. @@ -2417,7 +2428,7 @@ mod test_functions_tests { let f = LinearFunction { slope, intercept }; for i in generate_potentially_intresting_target_values() { let target_y = u64_to_dec(i).unwrap(); - println!("Inverting linear function {target_y} = f(x) = {slope} * x + {intercept} ..."); + // println!("Inverting linear function {target_y} = f(x) = {slope} * x + {intercept} ..."); run_inverse_function_test(&f, target_y); } } diff --git a/rs/nns/governance/src/neurons_fund/neurons_fund_anonymization_tests.rs b/rs/nns/governance/src/neurons_fund/neurons_fund_anonymization_tests.rs index 1218b92bfed..09a4907c882 100644 --- a/rs/nns/governance/src/neurons_fund/neurons_fund_anonymization_tests.rs +++ b/rs/nns/governance/src/neurons_fund/neurons_fund_anonymization_tests.rs @@ -51,6 +51,7 @@ fn test_neurons_fund_participation_anonymization() { one_third_participation_milestone_icp: dec!(100_000.0), full_participation_milestone_icp: dec!(167_000.0), }, + cfg!(not(test)), ) .unwrap() .serialize(), diff --git a/rs/nns/governance/tests/fixtures/mod.rs b/rs/nns/governance/tests/fixtures/mod.rs index 27e63d68d17..026b561460a 100755 --- a/rs/nns/governance/tests/fixtures/mod.rs +++ b/rs/nns/governance/tests/fixtures/mod.rs @@ -717,7 +717,9 @@ impl NNS { .ledger .accounts .clone(), - governance_proto: self.governance.clone_proto(), + governance_proto: GovernanceProto { + ..Default::default() + }, latest_gc_num_proposals: self.governance.latest_gc_num_proposals, } } diff --git a/rs/nns/governance/tests/governance.rs b/rs/nns/governance/tests/governance.rs index a76d87d3898..2163724a088 100644 --- a/rs/nns/governance/tests/governance.rs +++ b/rs/nns/governance/tests/governance.rs @@ -11229,7 +11229,7 @@ lazy_static! { }; static ref SERIALIZED_IDEAL_MATCHING_FUNCTION_REPR: Option = Some( - PolynomialMatchingFunction::new(2_000_000 * E8, *NEURONS_FUND_PARTICIPATION_LIMITS).unwrap().serialize() + PolynomialMatchingFunction::new(2_000_000 * E8, *NEURONS_FUND_PARTICIPATION_LIMITS, false).unwrap().serialize() ); static ref INITIAL_NEURONS_FUND_PARTICIPATION: Option = diff --git a/rs/nns/governance/tests/interleaving_tests.rs b/rs/nns/governance/tests/interleaving_tests.rs index f9846eee9ed..e6cc6dede94 100644 --- a/rs/nns/governance/tests/interleaving_tests.rs +++ b/rs/nns/governance/tests/interleaving_tests.rs @@ -173,6 +173,7 @@ fn test_cant_interleave_calls_to_settle_neurons_fund() { one_third_participation_milestone_icp: dec!(100_000.0), full_participation_milestone_icp: dec!(167_000.0), }, + false, ) .unwrap(); diff --git a/rs/sns/swap/tests/swap.rs b/rs/sns/swap/tests/swap.rs index 6fa4897f004..9714a179e58 100644 --- a/rs/sns/swap/tests/swap.rs +++ b/rs/sns/swap/tests/swap.rs @@ -32,8 +32,7 @@ use ic_nervous_system_common_test_utils::{ drain_receiver_channel, InterleavingTestLedger, LedgerCall, LedgerControlMessage, LedgerReply, SpyLedger, }; -use ic_nervous_system_proto::pb::v1::Countries; -use ic_nervous_system_proto::pb::v1::Principals; +use ic_nervous_system_proto::pb::v1::{Countries, Principals}; use ic_neurons_fund::{ InvertibleFunction, MatchingFunction, NeuronsFundParticipationLimits, PolynomialMatchingFunction, SerializableFunction, @@ -3133,9 +3132,12 @@ async fn test_finalization_halts_when_set_mode_fails() { #[test] fn test_derived_state() { let total_nf_maturity = 1_000_000 * E8; - let nf_matching_fn = - PolynomialMatchingFunction::new(total_nf_maturity, neurons_fund_participation_limits()) - .unwrap(); + let nf_matching_fn = PolynomialMatchingFunction::new( + total_nf_maturity, + neurons_fund_participation_limits(), + false, + ) + .unwrap(); println!("{}", nf_matching_fn.dbg_plot()); let mut swap = Swap { init: Some(Init { @@ -5265,6 +5267,7 @@ fn test_refresh_buyer_tokens_with_neurons_fund_matched_funding() { (PolynomialMatchingFunction::new( total_nf_maturity_equivalent_icp_e8s, neurons_fund_participation_limits(), + false, ) .unwrap()) .serialize(), @@ -5451,6 +5454,7 @@ fn test_refresh_buyer_tokens_without_neurons_fund_matched_funding() { (PolynomialMatchingFunction::new( total_nf_maturity_equivalent_icp_e8s, neurons_fund_participation_limits(), + false, ) .unwrap()) .serialize(), @@ -5710,6 +5714,7 @@ fn test_swap_cannot_finalize_via_new_participation_if_remaining_lt_minimal_parti PolynomialMatchingFunction::new( total_nf_maturity_equivalent_icp_e8s, neurons_fund_participation_limits(), + false, ) .unwrap() .serialize(),