Skip to content

Commit

Permalink
chore(nns): reduce logging in tests that is only useful in production
Browse files Browse the repository at this point in the history
  • Loading branch information
max-dfinity committed Sep 5, 2024
1 parent 39d2681 commit c9f4a99
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 41 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
77 changes: 44 additions & 33 deletions rs/nns/governance/src/neurons_fund.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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::<F>(),);
// 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.
Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 3 additions & 1 deletion rs/nns/governance/tests/fixtures/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
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 @@ -11229,7 +11229,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 @@ -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 {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down

0 comments on commit c9f4a99

Please sign in to comment.