Skip to content

Commit

Permalink
chore(nns): reduce logging in tests (#1367)
Browse files Browse the repository at this point in the history
There are many logs that are helpful in mainnet that are creating too
much noise in tests, causing in some cases the testing infrastructure to
get overwhelmed and hang during cleanup.

Additionally, removing the extra logs reduces the test run time from
~40s to ~5s.
  • Loading branch information
max-dfinity authored Sep 10, 2024
1 parent 38b7b5a commit 7b8d005
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 51 deletions.
5 changes: 4 additions & 1 deletion rs/nervous_system/neurons_fund/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,7 @@ impl PolynomialMatchingFunction {
pub fn new(
total_maturity_equivalent_icp_e8s: u64,
neurons_fund_participation_limits: NeuronsFundParticipationLimits,
enable_logging: bool,
) -> Result<Self, String> {
// 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.
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<PolynomialMatchingFunction> = assert_matches!(
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/src/governance/test_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
),
}),
Expand Down
49 changes: 5 additions & 44 deletions rs/nns/governance/src/neurons_fund.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,27 +1075,9 @@ where
// `u64`, so we keep the sum as precise as possible. This gives us more precision than
// `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
);
(NeuronsFundSnapshot::empty(), Decimal::ZERO)
} else if intended_neurons_fund_participation_icp_e8s == 0 {
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),
);
if total_maturity_equivalent_icp_e8s == 0
|| intended_neurons_fund_participation_icp_e8s == 0
{
(NeuronsFundSnapshot::empty(), Decimal::ZERO)
} else {
// Unlike in most other places, here we keep the ICP values in e8s (even after converting
Expand Down Expand Up @@ -1141,25 +1123,9 @@ 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 \
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,
);
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 \
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,
);
(max_participant_icp_e8s, true)
} else {
(ideal_participation_amount_icp_e8s, false)
Expand Down Expand Up @@ -1551,6 +1517,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,
Expand Down Expand Up @@ -2334,15 +2301,9 @@ 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,
);
return;
};
let observed = function.invert(target_y).unwrap();
println!("{}, target_y = {target_y}", std::any::type_name::<F>(),);

// Sometimes exact equality cannot be reached with our search strategy. We tolerate errors
// up to 1 E8.
assert!(
Expand Down Expand Up @@ -2416,7 +2377,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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,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(),
Expand Down
2 changes: 1 addition & 1 deletion rs/nns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11228,7 +11228,7 @@ lazy_static! {
};

static ref SERIALIZED_IDEAL_MATCHING_FUNCTION_REPR: Option<String> = 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<NeuronsFundParticipation> =
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/tests/interleaving_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
15 changes: 10 additions & 5 deletions rs/sns/swap/tests/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -3125,9 +3124,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 {
Expand Down Expand Up @@ -5261,6 +5263,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(),
Expand Down Expand Up @@ -5447,6 +5450,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(),
Expand Down Expand Up @@ -5706,6 +5710,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(),
Expand Down

0 comments on commit 7b8d005

Please sign in to comment.