diff --git a/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs b/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs index 5d0ee3b4c2f..cfb2c935f35 100644 --- a/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs @@ -3054,27 +3054,46 @@ pub mod governance { /// example, count = count_buckets.values().sum(). #[prost(uint64, optional, tag = "1")] pub count: Option, + #[prost(uint64, optional, tag = "2")] pub total_staked_e8s: Option, #[prost(uint64, optional, tag = "3")] pub total_staked_maturity_e8s_equivalent: Option, #[prost(uint64, optional, tag = "4")] pub total_maturity_e8s_equivalent: Option, + + /// Deprecated. Use one of the following instead. #[prost(uint64, optional, tag = "5")] pub total_voting_power: Option, + /// Used to decide proposals. If all neurons refresh their voting + /// power/following frequently enough, this will be equal to potential + /// voting power. If not, this will be less. + #[prost(uint64, optional, tag = "11")] + pub total_deciding_voting_power: ::core::option::Option, + /// Used for voting rewards. + #[prost(uint64, optional, tag = "12")] + pub total_potential_voting_power: ::core::option::Option, + /// These fields are keyed by floor(dissolve delay / 0.5 years). These are /// analogous to the (singular) fields above. Here, the usual definition of /// year for the IC is used: exactly 365.25 days. #[prost(map = "uint64, uint64", tag = "6")] pub count_buckets: ::std::collections::HashMap, + #[prost(map = "uint64, uint64", tag = "7")] pub staked_e8s_buckets: ::std::collections::HashMap, #[prost(map = "uint64, uint64", tag = "8")] pub staked_maturity_e8s_equivalent_buckets: ::std::collections::HashMap, #[prost(map = "uint64, uint64", tag = "9")] pub maturity_e8s_equivalent_buckets: ::std::collections::HashMap, + + /// Deprecated. Use one of the following instead. #[prost(map = "uint64, uint64", tag = "10")] pub voting_power_buckets: ::std::collections::HashMap, + #[prost(map = "uint64, uint64", tag = "13")] + pub deciding_voting_power_buckets: ::std::collections::HashMap, + #[prost(map = "uint64, uint64", tag = "14")] + pub potential_voting_power_buckets: ::std::collections::HashMap, } } /// Records that making an OpenSnsTokenSwap (OSTS) or CreateServiceNervousSystem (CSNS) diff --git a/rs/nns/governance/canister/governance.did b/rs/nns/governance/canister/governance.did index 790d00373c5..b56b25faa03 100644 --- a/rs/nns/governance/canister/governance.did +++ b/rs/nns/governance/canister/governance.did @@ -761,16 +761,25 @@ type NeuronStakeTransfer = record { }; type NeuronSubsetMetrics = record { - total_maturity_e8s_equivalent : opt nat64; - maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; - voting_power_buckets : vec record { nat64; nat64 }; - total_staked_e8s : opt nat64; count : opt nat64; + + total_staked_e8s : opt nat64; + total_maturity_e8s_equivalent : opt nat64; total_staked_maturity_e8s_equivalent : opt nat64; - staked_maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; - staked_e8s_buckets : vec record { nat64; nat64 }; + total_voting_power : opt nat64; + total_deciding_voting_power : opt nat64; + total_potential_voting_power : opt nat64; + count_buckets : vec record { nat64; nat64 }; + + staked_e8s_buckets : vec record { nat64; nat64 }; + maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; + staked_maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; + + voting_power_buckets : vec record { nat64; nat64 }; + deciding_voting_power_buckets : vec record { nat64; nat64 }; + potential_voting_power_buckets : vec record { nat64; nat64 }; }; type NeuronsFundAuditInfo = record { diff --git a/rs/nns/governance/canister/governance_test.did b/rs/nns/governance/canister/governance_test.did index 95b0a97aa70..3f625d29dfb 100644 --- a/rs/nns/governance/canister/governance_test.did +++ b/rs/nns/governance/canister/governance_test.did @@ -693,16 +693,25 @@ type NeuronStakeTransfer = record { }; type NeuronSubsetMetrics = record { - total_maturity_e8s_equivalent : opt nat64; - maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; - voting_power_buckets : vec record { nat64; nat64 }; - total_staked_e8s : opt nat64; count : opt nat64; + + total_staked_e8s : opt nat64; + total_maturity_e8s_equivalent : opt nat64; total_staked_maturity_e8s_equivalent : opt nat64; - staked_maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; - staked_e8s_buckets : vec record { nat64; nat64 }; + total_voting_power : opt nat64; + total_deciding_voting_power : opt nat64; + total_potential_voting_power : opt nat64; + count_buckets : vec record { nat64; nat64 }; + + staked_e8s_buckets : vec record { nat64; nat64 }; + maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; + staked_maturity_e8s_equivalent_buckets : vec record { nat64; nat64 }; + + voting_power_buckets : vec record { nat64; nat64 }; + deciding_voting_power_buckets : vec record { nat64; nat64 }; + potential_voting_power_buckets : vec record { nat64; nat64 }; }; type NeuronsFundAuditInfo = record { diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 9def884ba5d..7d91afe41e6 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -2432,19 +2432,33 @@ message Governance { // analogous fields (declared a little lower in this message). For // example, count = count_buckets.values().sum(). optional uint64 count = 1; + optional uint64 total_staked_e8s = 2; optional uint64 total_staked_maturity_e8s_equivalent = 3; optional uint64 total_maturity_e8s_equivalent = 4; + + // Deprecated. Use one of the following instead. optional uint64 total_voting_power = 5; + // Used to decide proposals. If all neurons refresh their voting + // power/following frequently enough, this will be equal to potential + // voting power. If not, this will be less. + optional uint64 total_deciding_voting_power = 11; + // Used for voting rewards. + optional uint64 total_potential_voting_power = 12; // These fields are keyed by floor(dissolve delay / 0.5 years). These are // analogous to the (singular) fields above. Here, the usual definition of // year for the IC is used: exactly 365.25 days. map count_buckets = 6; + map staked_e8s_buckets = 7; map staked_maturity_e8s_equivalent_buckets = 8; map maturity_e8s_equivalent_buckets = 9; + + // Deprecated. Use one of the following instead. map voting_power_buckets = 10; + map deciding_voting_power_buckets = 13; + map potential_voting_power_buckets = 14; } NeuronSubsetMetrics non_self_authenticating_controller_neuron_subset_metrics = 38; diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index 983afdaa993..6e5043a0a9d 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -3629,8 +3629,17 @@ pub mod governance { pub total_staked_maturity_e8s_equivalent: ::core::option::Option, #[prost(uint64, optional, tag = "4")] pub total_maturity_e8s_equivalent: ::core::option::Option, + /// Deprecated. Use one of the following instead. #[prost(uint64, optional, tag = "5")] pub total_voting_power: ::core::option::Option, + /// Used to decide proposals. If all neurons refresh their voting + /// power/following frequently enough, this will be equal to potential + /// voting power. If not, this will be less. + #[prost(uint64, optional, tag = "11")] + pub total_deciding_voting_power: ::core::option::Option, + /// Used for voting rewards. + #[prost(uint64, optional, tag = "12")] + pub total_potential_voting_power: ::core::option::Option, /// These fields are keyed by floor(dissolve delay / 0.5 years). These are /// analogous to the (singular) fields above. Here, the usual definition of /// year for the IC is used: exactly 365.25 days. @@ -3642,8 +3651,13 @@ pub mod governance { pub staked_maturity_e8s_equivalent_buckets: ::std::collections::HashMap, #[prost(map = "uint64, uint64", tag = "9")] pub maturity_e8s_equivalent_buckets: ::std::collections::HashMap, + /// Deprecated. Use one of the following instead. #[prost(map = "uint64, uint64", tag = "10")] pub voting_power_buckets: ::std::collections::HashMap, + #[prost(map = "uint64, uint64", tag = "13")] + pub deciding_voting_power_buckets: ::std::collections::HashMap, + #[prost(map = "uint64, uint64", tag = "14")] + pub potential_voting_power_buckets: ::std::collections::HashMap, } } /// Records that making an OpenSnsTokenSwap (OSTS) or CreateServiceNervousSystem (CSNS) diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index d8185870908..c6ef743733f 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -8090,6 +8090,13 @@ impl Governance { /// Iterate over all neurons and compute `GovernanceCachedMetrics` pub fn compute_cached_metrics(&self, now: u64, icp_supply: Tokens) -> GovernanceCachedMetrics { + let network_economics = self.economics(); + let neuron_minimum_stake_e8s = network_economics.neuron_minimum_stake_e8s; + let voting_power_economics = network_economics + .voting_power_economics + .as_ref() + .unwrap_or(&VotingPowerEconomics::DEFAULT); + let NeuronMetrics { dissolving_neurons_count, dissolving_neurons_e8s_buckets, @@ -8126,9 +8133,11 @@ impl Governance { not_dissolving_neurons_e8s_buckets_ect, non_self_authenticating_controller_neuron_subset_metrics, public_neuron_subset_metrics, - } = self - .neuron_store - .compute_neuron_metrics(now, self.economics().neuron_minimum_stake_e8s); + } = self.neuron_store.compute_neuron_metrics( + neuron_minimum_stake_e8s, + voting_power_economics, + now, + ); let total_staked_e8s_non_self_authenticating_controller = Some(non_self_authenticating_controller_neuron_subset_metrics.total_staked_e8s); @@ -8202,12 +8211,16 @@ impl From for NeuronSubsetMetricsPb { total_staked_maturity_e8s_equivalent, total_maturity_e8s_equivalent, total_voting_power, + total_deciding_voting_power, + total_potential_voting_power, count_buckets, staked_e8s_buckets, staked_maturity_e8s_equivalent_buckets, maturity_e8s_equivalent_buckets, voting_power_buckets, + deciding_voting_power_buckets, + potential_voting_power_buckets, } = src; let count = Some(count); @@ -8215,6 +8228,8 @@ impl From for NeuronSubsetMetricsPb { let total_staked_maturity_e8s_equivalent = Some(total_staked_maturity_e8s_equivalent); let total_maturity_e8s_equivalent = Some(total_maturity_e8s_equivalent); let total_voting_power = Some(total_voting_power); + let total_deciding_voting_power = Some(total_deciding_voting_power); + let total_potential_voting_power = Some(total_potential_voting_power); NeuronSubsetMetricsPb { count, @@ -8222,12 +8237,16 @@ impl From for NeuronSubsetMetricsPb { total_staked_maturity_e8s_equivalent, total_maturity_e8s_equivalent, total_voting_power, + total_deciding_voting_power, + total_potential_voting_power, count_buckets, staked_e8s_buckets, staked_maturity_e8s_equivalent_buckets, maturity_e8s_equivalent_buckets, voting_power_buckets, + deciding_voting_power_buckets, + potential_voting_power_buckets, } } } diff --git a/rs/nns/governance/src/lib.rs b/rs/nns/governance/src/lib.rs index cfddaccc4b4..b33ee78f6e3 100644 --- a/rs/nns/governance/src/lib.rs +++ b/rs/nns/governance/src/lib.rs @@ -214,6 +214,13 @@ thread_local! { static USE_STABLE_MEMORY_FOLLOWING_INDEX: Cell = const { Cell::new(cfg!(feature = "test")) }; } +thread_local! { + // This gets set to the current time when it detects that a new cycle has + // begun. (This occurs in one of the prune_some_following functions.) + static CURRENT_PRUNE_FOLLOWING_FULL_CYCLE_START_TIMESTAMP_SECONDS: Cell = + const { Cell::new(0) }; +} + pub fn is_voting_power_adjustment_enabled() -> bool { IS_VOTING_POWER_ADJUSTMENT_ENABLED.with(|ok| ok.get()) } @@ -415,7 +422,12 @@ fn encode_dissolve_delay_buckets( } } -/// Encodes +/// Outputs many statistics about neurons, proposals, etc in Prometheus format +/// (via the w output parameter). +/// +/// Some of these statistics are useful to operators (these could be called +/// health metrics). Others are of interest to the general public, and help +/// promote transparency of the Internet Computer (especially its governance). pub fn encode_metrics( governance: &Governance, w: &mut ic_metrics_encoder::MetricsEncoder>, @@ -432,11 +444,20 @@ pub fn encode_metrics( ic_nervous_system_common::total_memory_size_bytes() as f64, "Size of the total memory allocated by this canister measured in bytes.", )?; + + // Background Work + w.encode_gauge( "governance_latest_gc_timestamp_seconds", governance.latest_gc_timestamp_seconds as f64, "Timestamp of the last proposal garbage collection event, in seconds since the Unix epoch.", )?; + w.encode_gauge( + "governance_current_prune_following_full_cycle_start_timestamp_seconds", + CURRENT_PRUNE_FOLLOWING_FULL_CYCLE_START_TIMESTAMP_SECONDS + .with(|timestamp_seconds| timestamp_seconds.get()) as f64, + "When the current full cycle of follow pruning started.", + )?; // Proposals (more detailed breakdowns later). @@ -517,30 +538,48 @@ pub fn encode_metrics( // Voting Power - let total_voting_power = match governance + let most_recent_proposal = governance .heap_data .proposals - .iter() - .filter(|(_, proposal_data)| { + .values() + // Exclude ManageNeuron proposals. + .filter(|proposal_data| { proposal_data .proposal .as_ref() .map(|proposal| !proposal.is_manage_neuron()) .unwrap_or_default() }) - .next_back() - { - Some((_, proposal_data)) => match &proposal_data.latest_tally { - Some(tally) => tally.total as f64, - None => 0f64, - }, - None => 0f64, - }; + .next_back(); + let mut total_deciding_voting_power = 0.0; + let mut total_potential_voting_power = 0.0; + if let Some(most_recent_proposal) = &most_recent_proposal { + if let Some(tally) = most_recent_proposal.latest_tally { + total_deciding_voting_power = tally.total as f64; + } + + total_potential_voting_power = most_recent_proposal + .total_potential_voting_power + // Convert to float. + .map(|result| result as f64) + // Fall back to deciding voting power. + .unwrap_or(total_deciding_voting_power); + } w.encode_gauge( "governance_voting_power_total", - total_voting_power, - "The total voting power, according to the most recent proposal.", + total_potential_voting_power, + "Deprecated. Use governance_deciding_voting_power_total instead.", + )?; + w.encode_gauge( + "governance_total_deciding_voting_power", + total_deciding_voting_power, + "The total amount of deciding voting power (in the most recent proposal).", + )?; + w.encode_gauge( + "governance_total_potential_voting_power", + total_potential_voting_power, + "The total amount of potential voting power (in the most recent proposal).", )?; // Neuron Indexes @@ -899,11 +938,7 @@ pub fn encode_metrics( } if let Some(public_neuron_subset_metrics) = public_neuron_subset_metrics { - public_neuron_subset_metrics.encode( - "public", - "that have visibility set to public", - w, - )?; + public_neuron_subset_metrics.encode("public", "have visibility set to public", w)?; } } @@ -934,16 +969,24 @@ impl NeuronSubsetMetricsPb { ) -> std::io::Result<()> { let Self { count, + total_staked_e8s, total_staked_maturity_e8s_equivalent, total_maturity_e8s_equivalent, + total_voting_power, + total_deciding_voting_power, + total_potential_voting_power, count_buckets, + staked_e8s_buckets, staked_maturity_e8s_equivalent_buckets, maturity_e8s_equivalent_buckets, + voting_power_buckets, + deciding_voting_power_buckets, + potential_voting_power_buckets, } = self; w.encode_gauge( @@ -987,7 +1030,30 @@ impl NeuronSubsetMetricsPb { &format!("governance_total_voting_power_{}", neuron_subset_name), total_voting_power.unwrap_or_default() as f64, &format!( - "The total amount of voting power of all neurons that {}.", + "Deprecated. Use governance_total_deciding_voting_power_{} or \ + governance_total_potential_voting_power_{} instead.", + neuron_subset_name, neuron_subset_name, + ), + )?; + w.encode_gauge( + &format!( + "governance_total_deciding_voting_power_{}", + neuron_subset_name + ), + total_deciding_voting_power.unwrap_or_default() as f64, + &format!( + "The total amount of deciding voting power of all neurons that {}.", + neuron_subset_description, + ), + )?; + w.encode_gauge( + &format!( + "governance_total_potential_voting_power_{}", + neuron_subset_name + ), + total_potential_voting_power.unwrap_or_default() as f64, + &format!( + "The total amount of potential voting power of all neurons that {}.", neuron_subset_description, ), )?; @@ -1001,8 +1067,7 @@ impl NeuronSubsetMetricsPb { "Number of neurons that {}, grouped by dissolve delay.", neuron_subset_description, ), - ) - .unwrap(), + )?, count_buckets, ); @@ -1015,11 +1080,10 @@ impl NeuronSubsetMetricsPb { &format!("governance_{}_neurons_e8s_buckets", neuron_subset_name), &format!( "The total amount of staked ICP (in e8s) in neurons that {}, \ - grouped by dissolve delay.", + grouped by dissolve delay.", neuron_subset_description, ), - ) - .unwrap(), + )?, staked_e8s_buckets, ); encode_dissolve_delay_buckets( @@ -1033,8 +1097,7 @@ impl NeuronSubsetMetricsPb { grouped by dissolve delay.", neuron_subset_description, ), - ) - .unwrap(), + )?, staked_maturity_e8s_equivalent_buckets, ); encode_dissolve_delay_buckets( @@ -1046,8 +1109,7 @@ impl NeuronSubsetMetricsPb { grouped by dissolve delay.", neuron_subset_description, ), - ) - .unwrap(), + )?, maturity_e8s_equivalent_buckets, ); @@ -1058,14 +1120,41 @@ impl NeuronSubsetMetricsPb { neuron_subset_name ), &format!( - "The total amount of voting power in neurons that {}, \ - grouped by dissolve delay.", - neuron_subset_description, + "Deprecated. Use either governance_{}_deciding_voting_power_buckets, \ + or governance_{}_potential_voting_power_buckets instead.", + neuron_subset_name, neuron_subset_name, ), - ) - .unwrap(), + )?, voting_power_buckets, ); + encode_dissolve_delay_buckets( + w.gauge_vec( + &format!( + "governance_{}_deciding_voting_power_buckets", + neuron_subset_name + ), + &format!( + "The total amount of deciding voting power (used to decide proposals) \ + in neurons that {}, grouped by dissolve delay.", + neuron_subset_description, + ), + )?, + deciding_voting_power_buckets, + ); + encode_dissolve_delay_buckets( + w.gauge_vec( + &format!( + "governance_{}_potential_voting_power_buckets", + neuron_subset_name + ), + &format!( + "The total amount of potential voting power (used to decide \ + voting rewards) in neurons that {}, grouped by dissolve delay.", + neuron_subset_description, + ), + )?, + potential_voting_power_buckets, + ); Ok(()) } diff --git a/rs/nns/governance/src/neuron/types.rs b/rs/nns/governance/src/neuron/types.rs index 182bcbbd9c8..309bf56cea3 100644 --- a/rs/nns/governance/src/neuron/types.rs +++ b/rs/nns/governance/src/neuron/types.rs @@ -1768,6 +1768,15 @@ impl NeuronBuilder { self } + #[cfg(test)] // To satisfy clippy. Feel free to use in production code. + pub fn with_voting_power_refreshed_timestamp_seconds( + mut self, + voting_power_refreshed_timestamp_seconds: u64, + ) -> Self { + self.voting_power_refreshed_timestamp_seconds = voting_power_refreshed_timestamp_seconds; + self + } + pub fn build(self) -> Neuron { let NeuronBuilder { id, diff --git a/rs/nns/governance/src/neuron_store.rs b/rs/nns/governance/src/neuron_store.rs index c2ff5129e0c..ead85cb62fe 100644 --- a/rs/nns/governance/src/neuron_store.rs +++ b/rs/nns/governance/src/neuron_store.rs @@ -17,6 +17,7 @@ use crate::{ with_stable_neuron_store_mut, }, use_stable_memory_following_index, Clock, IcClock, + CURRENT_PRUNE_FOLLOWING_FULL_CYCLE_START_TIMESTAMP_SECONDS, }; use dyn_clone::DynClone; use ic_base_types::PrincipalId; @@ -1386,6 +1387,15 @@ pub fn prune_some_following( carry_on: impl FnMut() -> bool, ) -> Bound { let now_seconds = neuron_store.now(); + + if next == Bound::Unbounded { + CURRENT_PRUNE_FOLLOWING_FULL_CYCLE_START_TIMESTAMP_SECONDS.with( + |start_timestamp_seconds| { + start_timestamp_seconds.set(now_seconds); + }, + ); + } + groom_some_neurons( neuron_store, |neuron| { diff --git a/rs/nns/governance/src/neuron_store/benches.rs b/rs/nns/governance/src/neuron_store/benches.rs index 6eae0242d91..3a9cb347e5a 100644 --- a/rs/nns/governance/src/neuron_store/benches.rs +++ b/rs/nns/governance/src/neuron_store/benches.rs @@ -242,7 +242,9 @@ fn neuron_metrics_calculation_heap() -> BenchResult { let mut rng = new_rng(); let neuron_store = set_up_neuron_store(&mut rng, 100, 0); - bench_fn(|| neuron_store.compute_neuron_metrics(now_seconds(), E8)) + bench_fn(|| { + neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now_seconds()) + }) } #[bench(raw)] @@ -252,7 +254,9 @@ fn neuron_metrics_calculation_stable() -> BenchResult { let mut rng = new_rng(); let neuron_store = set_up_neuron_store(&mut rng, 100, 0); - bench_fn(|| neuron_store.compute_neuron_metrics(now_seconds(), E8)) + bench_fn(|| { + neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now_seconds()) + }) } fn add_neuron_ready_to_spawn( diff --git a/rs/nns/governance/src/neuron_store/metrics.rs b/rs/nns/governance/src/neuron_store/metrics.rs index 586c2b19cfe..b16e8f84e46 100644 --- a/rs/nns/governance/src/neuron_store/metrics.rs +++ b/rs/nns/governance/src/neuron_store/metrics.rs @@ -1,7 +1,7 @@ use super::NeuronStore; use crate::{ neuron_store::Neuron, - pb::v1::{NeuronState, Visibility}, + pb::v1::{NeuronState, Visibility, VotingPowerEconomics}, storage::{neurons::NeuronSections, with_stable_neuron_store}, }; use ic_base_types::PrincipalId; @@ -65,6 +65,7 @@ pub(crate) struct NeuronMetrics { impl NeuronMetrics { fn increment_non_self_authenticating_controller_neuron_subset_metrics( &mut self, + voting_power_economics: &VotingPowerEconomics, now_seconds: u64, neuron: &Neuron, ) { @@ -80,17 +81,22 @@ impl NeuronMetrics { } self.non_self_authenticating_controller_neuron_subset_metrics - .increment(now_seconds, neuron); + .increment(voting_power_economics, now_seconds, neuron); } - fn increment_public_neuron_subset_metrics(&mut self, now_seconds: u64, neuron: &Neuron) { + fn increment_public_neuron_subset_metrics( + &mut self, + voting_power_economics: &VotingPowerEconomics, + now_seconds: u64, + neuron: &Neuron, + ) { let is_public = neuron.visibility() == Some(Visibility::Public); if !is_public { return; } self.public_neuron_subset_metrics - .increment(now_seconds, neuron); + .increment(voting_power_economics, now_seconds, neuron); } } @@ -104,7 +110,9 @@ pub(crate) struct NeuronSubsetMetrics { pub total_maturity_e8s_equivalent: u64, // Voting power. - pub total_voting_power: u64, + pub total_voting_power: u64, // Deprecated. Use one of the following instead. + pub total_deciding_voting_power: u64, // Used to decide proposals. + pub total_potential_voting_power: u64, // Used for voting rewards. // Broken out by dissolve delay (rounded down to the nearest multiple of 0.5 // years). For example, if the current dissolve delay of a neuron is 7 @@ -120,19 +128,26 @@ pub(crate) struct NeuronSubsetMetrics { pub maturity_e8s_equivalent_buckets: HashMap, // Analogous to total_voting_power. - pub voting_power_buckets: HashMap, + pub voting_power_buckets: HashMap, // Deprecated. Use one of the following instead. + pub deciding_voting_power_buckets: HashMap, // See earlier comments. + pub potential_voting_power_buckets: HashMap, // See earlier comments. } impl NeuronSubsetMetrics { - fn increment(&mut self, now_seconds: u64, neuron: &Neuron) { + fn increment( + &mut self, + voting_power_economics: &VotingPowerEconomics, + now_seconds: u64, + neuron: &Neuron, + ) { let staked_e8s = neuron.minted_stake_e8s(); let staked_maturity_e8s_equivalent = neuron.staked_maturity_e8s_equivalent.unwrap_or_default(); let maturity_e8s_equivalent = neuron.maturity_e8s_equivalent; - // TODO: Also provide deciding voting power. Ideally, we'd rename the metrics to - // potential_voting_power, but that's probably not worth it. - let voting_power = neuron.potential_voting_power(now_seconds); + let potential_voting_power = neuron.potential_voting_power(now_seconds); + let deciding_voting_power = + neuron.deciding_voting_power(voting_power_economics, now_seconds); let increment = |total: &mut u64, additional_amount| { *total = total.saturating_add(additional_amount); @@ -150,7 +165,12 @@ impl NeuronSubsetMetrics { maturity_e8s_equivalent, ); - increment(&mut self.total_voting_power, voting_power); + increment(&mut self.total_voting_power, potential_voting_power); + increment(&mut self.total_deciding_voting_power, deciding_voting_power); + increment( + &mut self.total_potential_voting_power, + potential_voting_power, + ); // Increment metrics broken out by dissolve delay. let dissolve_delay_bucket = neuron @@ -173,7 +193,15 @@ impl NeuronSubsetMetrics { maturity_e8s_equivalent, ); - increment(&mut self.voting_power_buckets, voting_power); + increment(&mut self.voting_power_buckets, potential_voting_power); + increment( + &mut self.deciding_voting_power_buckets, + deciding_voting_power, + ); + increment( + &mut self.potential_voting_power_buckets, + potential_voting_power, + ); } } @@ -181,8 +209,9 @@ impl NeuronStore { /// Computes neuron metrics. pub(crate) fn compute_neuron_metrics( &self, + neuron_minimum_stake_e8s: u64, + voting_power_economics: &VotingPowerEconomics, now_seconds: u64, - minimum_stake_e8s: u64, ) -> NeuronMetrics { let mut metrics = if self.use_stable_memory_for_all_neurons { NeuronMetrics { @@ -202,11 +231,21 @@ impl NeuronStore { }; if self.use_stable_memory_for_all_neurons { - self.compute_neuron_metrics_all_stable(&mut metrics, now_seconds, minimum_stake_e8s); + self.compute_neuron_metrics_all_stable( + &mut metrics, + neuron_minimum_stake_e8s, + voting_power_economics, + now_seconds, + ); } // During migration, some neurons may be in the heap, so we need to compute // metrics for them as well. - self.compute_neuron_metrics_current(&mut metrics, now_seconds, minimum_stake_e8s); + self.compute_neuron_metrics_current( + &mut metrics, + neuron_minimum_stake_e8s, + voting_power_economics, + now_seconds, + ); metrics } @@ -214,25 +253,29 @@ impl NeuronStore { pub(crate) fn compute_neuron_metrics_all_stable( &self, metrics: &mut NeuronMetrics, + neuron_minimum_stake_e8s: u64, + voting_power_economics: &VotingPowerEconomics, now_seconds: u64, - minimum_stake_e8s: u64, ) { with_stable_neuron_store(|stable_neuron_store| { let neuron_sections = NeuronSections { - hot_keys: false, - recent_ballots: false, - followees: false, + // This is needed for `Neuron::visibility`` known_neuron_data: true, - transfer: false, + ..NeuronSections::NONE }; for neuron in stable_neuron_store.range_neurons_sections(.., neuron_sections) { let neuron = &neuron; metrics.increment_non_self_authenticating_controller_neuron_subset_metrics( + voting_power_economics, + now_seconds, + neuron, + ); + metrics.increment_public_neuron_subset_metrics( + voting_power_economics, now_seconds, neuron, ); - metrics.increment_public_neuron_subset_metrics(now_seconds, neuron); metrics.total_staked_e8s += neuron.minted_stake_e8s(); metrics.total_staked_maturity_e8s_equivalent += @@ -254,7 +297,7 @@ impl NeuronStore { } if 0 < neuron.cached_neuron_stake_e8s - && neuron.cached_neuron_stake_e8s < minimum_stake_e8s + && neuron.cached_neuron_stake_e8s < neuron_minimum_stake_e8s { metrics.neurons_with_invalid_stake_count += 1; } @@ -388,15 +431,21 @@ impl NeuronStore { pub(crate) fn compute_neuron_metrics_current( &self, metrics: &mut NeuronMetrics, + neuron_minimum_stake_e8s: u64, + voting_power_economics: &VotingPowerEconomics, now_seconds: u64, - minimum_stake_e8s: u64, ) { for neuron in self.heap_neurons.values() { metrics.increment_non_self_authenticating_controller_neuron_subset_metrics( + voting_power_economics, + now_seconds, + neuron, + ); + metrics.increment_public_neuron_subset_metrics( + voting_power_economics, now_seconds, neuron, ); - metrics.increment_public_neuron_subset_metrics(now_seconds, neuron); metrics.total_staked_e8s += neuron.minted_stake_e8s(); metrics.total_staked_maturity_e8s_equivalent += @@ -414,7 +463,7 @@ impl NeuronStore { } if 0 < neuron.cached_neuron_stake_e8s - && neuron.cached_neuron_stake_e8s < minimum_stake_e8s + && neuron.cached_neuron_stake_e8s < neuron_minimum_stake_e8s { metrics.neurons_with_invalid_stake_count += 1; } diff --git a/rs/nns/governance/src/neuron_store/metrics/tests.rs b/rs/nns/governance/src/neuron_store/metrics/tests.rs index 0bb6a88cb0d..19ca8ced343 100644 --- a/rs/nns/governance/src/neuron_store/metrics/tests.rs +++ b/rs/nns/governance/src/neuron_store/metrics/tests.rs @@ -227,7 +227,7 @@ fn test_compute_metrics() { ) .unwrap(); - let metrics = neuron_store.compute_neuron_metrics(now, 100_000_000); + let metrics = neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now); let expected_metrics = NeuronMetrics { dissolving_neurons_count: 5, @@ -345,12 +345,14 @@ fn test_compute_metrics_inactive_neuron_in_heap() { .unwrap(); // Step 2: verify that 1 neuron (3) are inactive. - let actual_metrics = neuron_store.compute_neuron_metrics(now, 100_000_000); + let actual_metrics = + neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now); assert_eq!(actual_metrics.garbage_collectable_neurons_count, 1); // Step 3: 2 days pass, and now neuron (2) is dissolved 15 days ago, and becomes inactive. let now = now + 2 * ONE_DAY_SECONDS; - let actual_metrics = neuron_store.compute_neuron_metrics(now, 100_000_000); + let actual_metrics = + neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now); assert_eq!(actual_metrics.garbage_collectable_neurons_count, 2); } @@ -482,7 +484,7 @@ fn test_compute_neuron_metrics_non_self_authenticating() { let NeuronMetrics { non_self_authenticating_controller_neuron_subset_metrics, .. - } = neuron_store.compute_neuron_metrics(now_seconds, E8); + } = neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now_seconds); // Step 3: Inspect results. assert_eq!( @@ -496,6 +498,8 @@ fn test_compute_neuron_metrics_non_self_authenticating() { // Voting power. total_voting_power: voting_power_1 + voting_power_3, + total_deciding_voting_power: voting_power_1 + voting_power_3, + total_potential_voting_power: voting_power_1 + voting_power_3, // Broken out by dissolve delay (rounded down to the nearest multiple of 6 // months). @@ -525,6 +529,14 @@ fn test_compute_neuron_metrics_non_self_authenticating() { 8 => voting_power_3, 16 => voting_power_1, }, + deciding_voting_power_buckets: hashmap! { + 8 => voting_power_3, + 16 => voting_power_1, + }, + potential_voting_power_buckets: hashmap! { + 8 => voting_power_3, + 16 => voting_power_1, + }, }, ); } @@ -559,6 +571,7 @@ fn test_compute_neuron_metrics_public_neurons() { .with_staked_maturity_e8s_equivalent(101) .with_maturity_e8s_equivalent(110) .with_visibility(Some(Visibility::Public)) + .with_voting_power_refreshed_timestamp_seconds(now_seconds) .build(); let neuron_2 = NeuronBuilder::new( @@ -575,6 +588,7 @@ fn test_compute_neuron_metrics_public_neurons() { .with_cached_neuron_stake_e8s(200_000) .with_staked_maturity_e8s_equivalent(202_000) .with_maturity_e8s_equivalent(220_000) + .with_voting_power_refreshed_timestamp_seconds(now_seconds) .build(); let neuron_3 = NeuronBuilder::new( @@ -596,6 +610,7 @@ fn test_compute_neuron_metrics_public_neurons() { name: "Daniel Wong".to_string(), description: Some("Best engineer of all time. Of all time.".to_string()), })) + .with_voting_power_refreshed_timestamp_seconds(now_seconds) .build(); let voting_power_1 = neuron_1.potential_voting_power(now_seconds); @@ -613,6 +628,7 @@ fn test_compute_neuron_metrics_public_neurons() { 2 => neuron_2, 3 => neuron_3, }); + neuron_store .with_neuron(&NeuronId { id: 3 }, |neuron| { assert_eq!( @@ -629,7 +645,7 @@ fn test_compute_neuron_metrics_public_neurons() { let NeuronMetrics { public_neuron_subset_metrics, .. - } = neuron_store.compute_neuron_metrics(now_seconds, E8); + } = neuron_store.compute_neuron_metrics(E8, &VotingPowerEconomics::DEFAULT, now_seconds); // Step 3: Inspect results. assert_eq!( @@ -643,6 +659,8 @@ fn test_compute_neuron_metrics_public_neurons() { // Voting power. total_voting_power: voting_power_1 + voting_power_3, + total_deciding_voting_power: voting_power_1 + voting_power_3, + total_potential_voting_power: voting_power_1 + voting_power_3, // Broken out by dissolve delay (rounded down to the nearest multiple of 6 // months). @@ -672,6 +690,14 @@ fn test_compute_neuron_metrics_public_neurons() { 8 => voting_power_3, 16 => voting_power_1, }, + deciding_voting_power_buckets: hashmap! { + 8 => voting_power_3, + 16 => voting_power_1, + }, + potential_voting_power_buckets: hashmap! { + 8 => voting_power_3, + 16 => voting_power_1, + }, }, ); } diff --git a/rs/nns/governance/src/pb/conversions.rs b/rs/nns/governance/src/pb/conversions.rs index f1b8785bd97..22c3a2b84dd 100644 --- a/rs/nns/governance/src/pb/conversions.rs +++ b/rs/nns/governance/src/pb/conversions.rs @@ -3476,15 +3476,24 @@ impl From fn from(item: pb::governance::governance_cached_metrics::NeuronSubsetMetrics) -> Self { Self { count: item.count, + total_staked_e8s: item.total_staked_e8s, total_staked_maturity_e8s_equivalent: item.total_staked_maturity_e8s_equivalent, total_maturity_e8s_equivalent: item.total_maturity_e8s_equivalent, + total_voting_power: item.total_voting_power, + total_deciding_voting_power: item.total_deciding_voting_power, + total_potential_voting_power: item.total_potential_voting_power, + count_buckets: item.count_buckets, + staked_e8s_buckets: item.staked_e8s_buckets, staked_maturity_e8s_equivalent_buckets: item.staked_maturity_e8s_equivalent_buckets, maturity_e8s_equivalent_buckets: item.maturity_e8s_equivalent_buckets, + voting_power_buckets: item.voting_power_buckets, + deciding_voting_power_buckets: item.deciding_voting_power_buckets, + potential_voting_power_buckets: item.potential_voting_power_buckets, } } } @@ -3494,15 +3503,24 @@ impl From fn from(item: pb_api::governance::governance_cached_metrics::NeuronSubsetMetrics) -> Self { Self { count: item.count, + total_staked_e8s: item.total_staked_e8s, total_staked_maturity_e8s_equivalent: item.total_staked_maturity_e8s_equivalent, total_maturity_e8s_equivalent: item.total_maturity_e8s_equivalent, + total_voting_power: item.total_voting_power, + total_deciding_voting_power: item.total_deciding_voting_power, + total_potential_voting_power: item.total_potential_voting_power, + count_buckets: item.count_buckets, + staked_e8s_buckets: item.staked_e8s_buckets, staked_maturity_e8s_equivalent_buckets: item.staked_maturity_e8s_equivalent_buckets, maturity_e8s_equivalent_buckets: item.maturity_e8s_equivalent_buckets, + voting_power_buckets: item.voting_power_buckets, + deciding_voting_power_buckets: item.deciding_voting_power_buckets, + potential_voting_power_buckets: item.potential_voting_power_buckets, } } } diff --git a/rs/nns/governance/src/tests.rs b/rs/nns/governance/src/tests.rs index 97c775fb5ca..d6cd9b76f36 100644 --- a/rs/nns/governance/src/tests.rs +++ b/rs/nns/governance/src/tests.rs @@ -35,14 +35,19 @@ fn test_neuron_subset_metrics_pb_encode() { // Step 2: Call the code under test. let subject = NeuronSubsetMetricsPb { count: Some(42), + total_staked_e8s: Some(43_000), total_staked_maturity_e8s_equivalent: Some(44_000_000), total_maturity_e8s_equivalent: Some(45_000_000_000), + total_voting_power: Some(46_000_000_000_000), + total_deciding_voting_power: Some(47_000_000_000_000_000), + total_potential_voting_power: Some(717_568_738), count_buckets: hashmap! { 3 => 3, }, + staked_e8s_buckets: hashmap! { 4 => 40, }, @@ -52,10 +57,19 @@ fn test_neuron_subset_metrics_pb_encode() { maturity_e8s_equivalent_buckets: hashmap! { 6 => 6_000, }, + voting_power_buckets: hashmap! { 7 => 70_000, 8 => 800_000, }, + deciding_voting_power_buckets: hashmap! { + 9 => 9_000_000, + 10 => 110_000_000, + }, + potential_voting_power_buckets: hashmap! { + 11 => 12_000_000_000, + 12 => 1_300_000_000_000, + }, }; assert_is_ok!(subject.encode("smart", "has IQ > 120", &mut metrics_encoder)); @@ -69,6 +83,7 @@ fn test_neuron_subset_metrics_pb_encode() { let result = prometheus_parse::Scrape::parse(result).unwrap(); assert_eq!(get_gauge(&result, "governance_smart_neurons_count"), 42.0); + assert_eq!( get_gauge(&result, "governance_total_staked_e8s_smart"), 43_000.0 @@ -84,10 +99,19 @@ fn test_neuron_subset_metrics_pb_encode() { get_gauge(&result, "governance_total_maturity_e8s_equivalent_smart"), 45_000_000_000.0 ); + assert_eq!( get_gauge(&result, "governance_total_voting_power_smart"), 46_000_000_000_000.0 ); + assert_eq!( + get_gauge(&result, "governance_total_deciding_voting_power_smart"), + 47_000_000_000_000_000.0 + ); + assert_eq!( + get_gauge(&result, "governance_total_potential_voting_power_smart"), + 717_568_738.0 + ); assert_eq!( get_metric_broken_out_by_dissolve_delay(&result, "governance_smart_neurons_count_buckets",), @@ -111,6 +135,7 @@ fn test_neuron_subset_metrics_pb_encode() { ), hashmap! { "[36, 42)".to_string() => 6_000.0 }, ); + assert_eq!( get_metric_broken_out_by_dissolve_delay( &result, @@ -121,4 +146,24 @@ fn test_neuron_subset_metrics_pb_encode() { "[48, 54)".to_string() => 800_000.0, }, ); + assert_eq!( + get_metric_broken_out_by_dissolve_delay( + &result, + "governance_smart_deciding_voting_power_buckets", + ), + hashmap! { + "[54, 60)".to_string() => 9e6, + "[60, 66)".to_string() => 11e7, + }, + ); + assert_eq!( + get_metric_broken_out_by_dissolve_delay( + &result, + "governance_smart_potential_voting_power_buckets", + ), + hashmap! { + "[66, 72)".to_string() => 12e9, + "[72, 78)".to_string() => 13e11, + }, + ); } diff --git a/rs/nns/governance/tests/governance.rs b/rs/nns/governance/tests/governance.rs index 5893351bf30..1539d4cebbe 100644 --- a/rs/nns/governance/tests/governance.rs +++ b/rs/nns/governance/tests/governance.rs @@ -416,6 +416,12 @@ fn test_single_neuron_proposal_new() { total_voting_power: Some( 1, ), + total_deciding_voting_power: Some( + 1, + ), + total_potential_voting_power: Some( + 1, + ), count_buckets: btreemap! { 1 => 1, }, @@ -431,6 +437,12 @@ fn test_single_neuron_proposal_new() { voting_power_buckets: btreemap! { 1 => 1, }, + deciding_voting_power_buckets: btreemap! { + 1 => 1, + }, + potential_voting_power_buckets: btreemap! { + 1 => 1, + }, }, ), ), @@ -455,11 +467,19 @@ fn test_single_neuron_proposal_new() { total_voting_power: Some( 0, ), + total_deciding_voting_power: Some( + 0, + ), + total_potential_voting_power: Some( + 0, + ), count_buckets: btreemap! {}, staked_e8s_buckets: btreemap! {}, staked_maturity_e8s_equivalent_buckets: btreemap! {}, maturity_e8s_equivalent_buckets: btreemap! {}, voting_power_buckets: btreemap! {}, + deciding_voting_power_buckets: btreemap! {}, + potential_voting_power_buckets: btreemap! {}, }, ), ), diff --git a/rs/sns/governance/BUILD.bazel b/rs/sns/governance/BUILD.bazel index 4f533259cc9..ae30350653b 100644 --- a/rs/sns/governance/BUILD.bazel +++ b/rs/sns/governance/BUILD.bazel @@ -245,13 +245,27 @@ generated_files_check( ], ) -# Usage: -# bazel run //rs/sns/governance:governance-canbench for benchmarking (the output shoudl look like `canbench/canbench_results.yml`). -# bazel run //rs/sns/governance:governance-canbench_update for updating the results file (see `canbench/canbench_results.yml`). -# bazel test //rs/sns/governance:governance_canbench_test for checking if the results file is up to date. +# Usage +# ===== +# +# For benchmarking (see `canbench/canbench_results.yml`): +# ``` +# bazel run //rs/sns/governance:governance-canbench +# ``` +# +# For updating the results file: +# ``` +# bazel run //rs/sns/governance:governance-canbench_update +# ``` +# +# To run the test: +# ``` +# bazel test //rs/sns/governance:governance_canbench_test +# ``` +# # Currently, updating the results file is not automated, and there are no tests to avoid -# regression. For now, we can use it as an example for benchmarking as part of an -# investigation of potential performance issues, or when we make a change that can affect +# regression. For now, we can use it as an example for benchmarking as part +# of an investigation of potential performance issues, or when we make a change that can affect # the performance measured in this benchmark. rust_canbench( name = "governance-canbench", diff --git a/rs/sns/governance/canbench/canbench_results.yml b/rs/sns/governance/canbench/canbench_results.yml index e22abfd8926..a9c567e968f 100644 --- a/rs/sns/governance/canbench/canbench_results.yml +++ b/rs/sns/governance/canbench/canbench_results.yml @@ -1,8 +1,8 @@ benches: bench_example: total: - instructions: 5638 + instructions: 4738 heap_increase: 0 stable_memory_increase: 0 scopes: {} -version: 0.1.7 +version: 0.1.8 diff --git a/rs/sns/governance/src/cached_upgrade_steps.rs b/rs/sns/governance/src/cached_upgrade_steps.rs index 0eedd402b7b..481144592dd 100644 --- a/rs/sns/governance/src/cached_upgrade_steps.rs +++ b/rs/sns/governance/src/cached_upgrade_steps.rs @@ -283,6 +283,9 @@ impl CachedUpgradeSteps { &self.current_version == version || self.subsequent_versions.contains(version) } + /// Returns whether `left` is before or equal to `right` in `self`, in the `Ok` result. + /// + /// Returns `Err` if at least one of the versions `left` or `right` are not in `self`. pub fn contains_in_order(&self, left: &Version, right: &Version) -> Result { if !self.contains(left) { return Err(format!("{:?} does not contain {:?}", self, left)); @@ -361,6 +364,8 @@ impl CachedUpgradeSteps { self.response_timestamp_seconds } + /// Returns `Ok` if `new_target` is in `self` but different from `self.current()`. + /// Otherwise, returns `Err`. pub fn validate_new_target_version(&self, new_target: &Version) -> Result<(), String> { if !self.contains(new_target) { return Err("new_target_version must be among the upgrade steps.".to_string()); diff --git a/rs/sns/governance/src/cached_upgrade_steps/tests.rs b/rs/sns/governance/src/cached_upgrade_steps/tests.rs index 0ab03f006ed..13f54aba7fc 100644 --- a/rs/sns/governance/src/cached_upgrade_steps/tests.rs +++ b/rs/sns/governance/src/cached_upgrade_steps/tests.rs @@ -197,3 +197,161 @@ fn cached_upgrade_steps_without_pending_upgrades() { vec![v] ); } + +#[test] +fn cached_upgrade_steps_with_pending_upgrades() { + let v0 = Version { + root_wasm_hash: vec![0, 0, 0], + governance_wasm_hash: vec![0, 0, 0], + ledger_wasm_hash: vec![0, 0, 0], + swap_wasm_hash: vec![0, 0, 0], + archive_wasm_hash: vec![0, 0, 0], + index_wasm_hash: vec![0, 0, 0], + }; + + let v1 = { + let mut v = v0.clone(); + v.root_wasm_hash = vec![1, 1, 1]; + v + }; + + let v2 = { + let mut v = v0.clone(); + v.root_wasm_hash = vec![2, 2, 2]; + v + }; + + let v3 = { + let mut v = v0.clone(); + v.root_wasm_hash = vec![3, 3, 3]; + v + }; + + let cached_upgrade_steps = CachedUpgradeSteps { + current_version: v0.clone(), + subsequent_versions: vec![v1.clone(), v2.clone()], + response_timestamp_seconds: 0, + requested_timestamp_seconds: 0, + }; + + // .last + assert_eq!(cached_upgrade_steps.last(), &v2); + + // .contains + assert!(cached_upgrade_steps.contains(&v0)); + assert!(cached_upgrade_steps.contains(&v1)); + assert!(cached_upgrade_steps.contains(&v2)); + assert!(!cached_upgrade_steps.contains(&v3)); + + // .current, .is_current + assert_eq!(cached_upgrade_steps.current(), &v0); + assert!(cached_upgrade_steps.is_current(&v0)); + assert!(!cached_upgrade_steps.is_current(&v1)); + assert!(!cached_upgrade_steps.is_current(&v2)); + assert!(!cached_upgrade_steps.is_current(&v3)); + + // .next, .has_pending_upgrades + assert_eq!(cached_upgrade_steps.next(), Some(&v1)); + assert!(cached_upgrade_steps.has_pending_upgrades()); + + // .contains_in_order + let expected_err = Err(format!( + "{:?} does not contain {:?}", + cached_upgrade_steps, v3 + )); + assert_eq!(cached_upgrade_steps.contains_in_order(&v0, &v0), Ok(true)); + assert_eq!(cached_upgrade_steps.contains_in_order(&v0, &v1), Ok(true)); + assert_eq!(cached_upgrade_steps.contains_in_order(&v0, &v2), Ok(true)); + assert_eq!( + cached_upgrade_steps.contains_in_order(&v0, &v3), + expected_err + ); + + assert_eq!(cached_upgrade_steps.contains_in_order(&v1, &v0), Ok(false)); + assert_eq!(cached_upgrade_steps.contains_in_order(&v1, &v1), Ok(true)); + assert_eq!(cached_upgrade_steps.contains_in_order(&v1, &v2), Ok(true)); + assert_eq!( + cached_upgrade_steps.contains_in_order(&v1, &v3), + expected_err + ); + + assert_eq!(cached_upgrade_steps.contains_in_order(&v2, &v0), Ok(false)); + assert_eq!(cached_upgrade_steps.contains_in_order(&v2, &v1), Ok(false)); + assert_eq!(cached_upgrade_steps.contains_in_order(&v2, &v2), Ok(true)); + assert_eq!( + cached_upgrade_steps.contains_in_order(&v2, &v3), + expected_err + ); + + assert_eq!( + cached_upgrade_steps.contains_in_order(&v3, &v0), + expected_err + ); + assert_eq!( + cached_upgrade_steps.contains_in_order(&v3, &v1), + expected_err + ); + assert_eq!( + cached_upgrade_steps.contains_in_order(&v3, &v2), + expected_err + ); + assert_eq!( + cached_upgrade_steps.contains_in_order(&v3, &v3), + expected_err + ); + + // .validate_new_target_version + assert_eq!( + cached_upgrade_steps.validate_new_target_version(&v0), + Err("new_target_version must differ from the current version.".to_string()) + ); + assert_eq!( + cached_upgrade_steps.validate_new_target_version(&v1), + Ok(()) + ); + assert_eq!( + cached_upgrade_steps.validate_new_target_version(&v2), + Ok(()) + ); + assert_eq!( + cached_upgrade_steps.validate_new_target_version(&v3), + Err("new_target_version must be among the upgrade steps.".to_string()) + ); + + // .take_from + assert_eq!( + cached_upgrade_steps.clone().take_from(&v0), + Ok(cached_upgrade_steps.clone()) + ); + assert_eq!( + cached_upgrade_steps.clone().take_from(&v1), + Ok(CachedUpgradeSteps { + current_version: v1.clone(), + subsequent_versions: vec![v2.clone()], + response_timestamp_seconds: 0, + requested_timestamp_seconds: 0, + }) + ); + assert_eq!( + cached_upgrade_steps.clone().take_from(&v2), + Ok(CachedUpgradeSteps { + current_version: v2.clone(), + subsequent_versions: vec![], + response_timestamp_seconds: 0, + requested_timestamp_seconds: 0, + }) + ); + assert_eq!( + cached_upgrade_steps.clone().take_from(&v3), + Err(format!( + "Cannot take_from {} that is not one of the cached upgrade steps.", + v3 + )) + ); + + // .into_iter + assert_eq!( + cached_upgrade_steps.into_iter().collect::>(), + vec![v0, v1, v2] + ); +} diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index 1b219e8e2e4..ad390de1716 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -441,12 +441,6 @@ impl GovernanceProto { } } - /// Gets the current deployed version of the SNS or panics. - pub fn deployed_version_or_panic(&self) -> Version { - self.deployed_version_or_err() - .expect("GovernanceProto.deployed_version must be set.") - } - /// Returns 0 if maturity modulation is disabled (per /// nervous_system_parameters.maturity_modulation_disabled). Otherwise, /// returns the value in self.maturity_modulation.current_basis_points. If @@ -2603,6 +2597,37 @@ impl Governance { Ok(()) } + /// Best effort to return the deployed version of this SNS. + /// + /// Normally, the SNS should always have a deployed version, in which case it is returned. + /// If this is not the case for whatever reason, this function tries to fetch the running + /// version, initialize deployed version with it, and return a copy. + pub async fn get_or_reset_deployed_version(&mut self) -> Result { + if let Some(deployed_version) = self.proto.deployed_version.clone() { + return Ok(deployed_version); + } + + log!( + ERROR, + "The SNS does not have a deployed version. Attempting to reset it ..." + ); + + let root_canister_id = self.proto.root_canister_id_or_panic(); + + let new_deployed_version = get_running_version(&*self.env, root_canister_id).await?; + + // Re-check that a reentrant call to this function did not yet update the state. + if let Some(deployed_version) = self.proto.deployed_version.clone() { + return Ok(deployed_version); + } + + self.proto + .deployed_version + .replace(new_deployed_version.clone()); + + Ok(new_deployed_version) + } + /// Return `Ok(true)` if the upgrade was completed successfully, return `Ok(false)` if an /// upgrade was successfully kicked-off, but its completion is pending. async fn perform_upgrade_to_next_sns_version_legacy( @@ -2611,7 +2636,13 @@ impl Governance { ) -> Result { self.check_no_upgrades_in_progress(Some(proposal_id))?; - let current_version = self.proto.deployed_version_or_panic(); + let current_version = self.get_or_reset_deployed_version().await.map_err(|err| { + GovernanceError::new_with_message( + ErrorType::External, + format!("Could not execute proposal: {}", err), + ) + })?; + let root_canister_id = self.proto.root_canister_id_or_panic(); let UpgradeSnsParams { @@ -2621,10 +2652,10 @@ impl Governance { canister_ids_to_upgrade, } = get_upgrade_params(&*self.env, root_canister_id, ¤t_version) .await - .map_err(|e| { + .map_err(|err| { GovernanceError::new_with_message( ErrorType::InvalidProposal, - format!("Could not execute proposal: {}", e), + format!("Could not execute proposal: {}", err), ) })?; @@ -2864,7 +2895,13 @@ impl Governance { ) -> Result<(), GovernanceError> { self.check_no_upgrades_in_progress(Some(proposal_id))?; - let current_version = self.proto.deployed_version_or_panic(); + let current_version = self.get_or_reset_deployed_version().await.map_err(|err| { + GovernanceError::new_with_message( + ErrorType::External, + format!("Could not execute proposal: {}", err), + ) + })?; + let ledger_canister_id = self.proto.ledger_canister_id_or_panic(); let ledger_canister_info = self.env @@ -4839,9 +4876,17 @@ impl Governance { self.maybe_gc(); } - // Acquires the "upgrade periodic task lock" (a lock shared between all periodic tasks that relate to upgrades) - // if it is currently released or was last acquired over UPGRADE_PERIODIC_TASK_LOCK_TIMEOUT_SECONDS ago. - fn acquire_upgrade_periodic_task_lock(&mut self) -> bool { + /// Attempts to acquire the lock over SNS upgrade-related periodic tasks. + /// + /// Succeeds if the lock is currently released or was last acquired + /// over `UPGRADE_PERIODIC_TASK_LOCK_TIMEOUT_SECONDS` ago. + /// + /// Returns whether the lock was acquired. + /// + /// This function is made public so that it can be called from + /// rs/sns/governance/tests/governance.rs where we need to disable upgrade-related periodic + /// tasks while testing a orthogonal SNS features (e.g., disburse maturity). + pub fn acquire_upgrade_periodic_task_lock(&mut self) -> bool { let now = self.env.now(); match self.upgrade_periodic_task_lock { Some(time_acquired) @@ -4871,10 +4916,12 @@ impl Governance { return; } - let Some(deployed_version) = self.proto.deployed_version.clone() else { - // TODO(NNS1-3445): there should be some way to recover from this state. - log!(ERROR, "No deployed version! This is an internal bug"); - return; + let deployed_version = match self.get_or_reset_deployed_version().await { + Ok(deployed_version) => deployed_version, + Err(err) => { + log!(ERROR, "Cannot get or reset deployed version: {}", err); + return; + } }; let upgrade_steps = self.get_or_reset_upgrade_steps(&deployed_version); diff --git a/rs/sns/governance/src/proposal.rs b/rs/sns/governance/src/proposal.rs index d1cbeb81a78..da437bdd5a5 100644 --- a/rs/sns/governance/src/proposal.rs +++ b/rs/sns/governance/src/proposal.rs @@ -414,15 +414,18 @@ pub(crate) async fn validate_and_render_action( validate_and_render_upgrade_sns_controlled_canister(upgrade) } Action::UpgradeSnsToNextVersion(upgrade_sns) => { - let current_version = governance_proto.deployed_version_or_panic(); - - validate_and_render_upgrade_sns_to_next_version( - upgrade_sns, - env, - root_canister_id, - current_version, - ) - .await + match governance_proto.deployed_version_or_err() { + Ok(current_version) => { + validate_and_render_upgrade_sns_to_next_version( + upgrade_sns, + env, + root_canister_id, + current_version, + ) + .await + } + Err(err) => Err(err), + } } proposal::Action::AddGenericNervousSystemFunction(function_to_add) => { validate_and_render_add_generic_nervous_system_function( diff --git a/rs/sns/governance/tests/fixtures/mod.rs b/rs/sns/governance/tests/fixtures/mod.rs index dac15391fdd..ef790ceab1a 100755 --- a/rs/sns/governance/tests/fixtures/mod.rs +++ b/rs/sns/governance/tests/fixtures/mod.rs @@ -9,10 +9,9 @@ use ic_sns_governance::{ governance::{Governance, ValidGovernanceProto}, pb::v1::{ get_neuron_response, get_proposal_response, - governance::{MaturityModulation, Mode, SnsMetadata}, - manage_neuron, + governance::{MaturityModulation, Mode, SnsMetadata, Version}, manage_neuron::{ - AddNeuronPermissions, MergeMaturity, RegisterVote, RemoveNeuronPermissions, + self, AddNeuronPermissions, MergeMaturity, RegisterVote, RemoveNeuronPermissions, }, manage_neuron_response::{ self, AddNeuronPermissionsResponse, FollowResponse, MergeMaturityResponse, @@ -433,6 +432,13 @@ impl GovernanceCanisterFixture { self } + /// Ensures that SNS upgrade features are not going to produce any external calls that are + /// orthogonal to this test scenario, as they would require setting up mock responses. + pub fn temporarily_disable_sns_upgrades(&mut self) -> &mut Self { + assert!(self.governance.acquire_upgrade_periodic_task_lock()); + self + } + pub fn capture_state(&mut self) -> &mut Self { self.initial_state = Some(self.get_state()); self @@ -854,6 +860,7 @@ impl Default for GovernanceCanisterFixtureBuilder { current_basis_points: Some(0), updated_at_timestamp_seconds: Some(1), }), + deployed_version: Some(Version::default()), ..Default::default() }, sns_ledger_transforms: Vec::default(), diff --git a/rs/sns/governance/tests/governance.rs b/rs/sns/governance/tests/governance.rs index 3c87b4410c0..61a39d698b0 100644 --- a/rs/sns/governance/tests/governance.rs +++ b/rs/sns/governance/tests/governance.rs @@ -304,14 +304,20 @@ fn test_disburse_maturity_succeeds_to_self() { assert_eq!(account_balance_before_disbursal, account_balance); // Advance time by a few days, but without triggering disbursal finalization. - env.gov_fixture.advance_time_by(6 * ONE_DAY_SECONDS); - env.gov_fixture.run_periodic_tasks_now(); + env.gov_fixture + .advance_time_by(6 * ONE_DAY_SECONDS) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); + let neuron = env.gov_fixture.get_neuron(&env.neuron_id); assert_eq!(neuron.disburse_maturity_in_progress.len(), 1); // Advance more, to hit 7-day period, and to trigger disbursal finalization. - env.gov_fixture.advance_time_by(ONE_DAY_SECONDS + 10); - env.gov_fixture.run_periodic_tasks_now(); + env.gov_fixture + .advance_time_by(ONE_DAY_SECONDS + 10) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); + let neuron = env.gov_fixture.get_neuron(&env.neuron_id); assert_eq!(neuron.disburse_maturity_in_progress.len(), 0); @@ -409,14 +415,20 @@ fn test_disburse_maturity_succeeds_to_other() { assert_eq!(receiver_balance_before_disbursal, account_balance); // Advance time by a few days, but without triggering disbursal finalization. - env.gov_fixture.advance_time_by(6 * ONE_DAY_SECONDS); - env.gov_fixture.run_periodic_tasks_now(); + env.gov_fixture + .advance_time_by(6 * ONE_DAY_SECONDS) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); + let neuron = env.gov_fixture.get_neuron(&env.neuron_id); assert_eq!(neuron.disburse_maturity_in_progress.len(), 1); // Advance more, to hit 7-day period, and to trigger disbursal finalization. - env.gov_fixture.advance_time_by(ONE_DAY_SECONDS + 10); - env.gov_fixture.run_periodic_tasks_now(); + env.gov_fixture + .advance_time_by(ONE_DAY_SECONDS + 10) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); + let neuron = env.gov_fixture.get_neuron(&env.neuron_id); assert_eq!(neuron.disburse_maturity_in_progress.len(), 0); @@ -498,7 +510,10 @@ fn test_disburse_maturity_succeeds_with_multiple_operations() { } // Advance time, to trigger disbursal finalization. - env.gov_fixture.advance_time_by(7 * ONE_DAY_SECONDS + 10); + env.gov_fixture + .advance_time_by(7 * ONE_DAY_SECONDS + 10) + .temporarily_disable_sns_upgrades(); + let mut remaining_maturity_e8s = earned_maturity_e8s; for (i, (percentage, destination)) in percentage_and_destination.iter().enumerate() { let destination_account = icrc_ledger_types::icrc1::account::Account { @@ -2628,7 +2643,9 @@ async fn assert_disburse_maturity_with_modulation_disburses_correctly( .create(); // This is supposed to cause Governance to poll CMC for the maturity modulation. - canister_fixture.run_periodic_tasks_now(); + canister_fixture + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); // Get the Neuron and assert its maturity is set as expected let neuron = canister_fixture.get_neuron(&neuron_id); @@ -2675,8 +2692,10 @@ async fn assert_disburse_maturity_with_modulation_disburses_correctly( let neuron = canister_fixture.get_neuron(&neuron_id); assert_eq!(neuron.maturity_e8s_equivalent, 0); - canister_fixture.advance_time_by(7 * ONE_DAY_SECONDS + 1); - canister_fixture.run_periodic_tasks_now(); + canister_fixture + .advance_time_by(7 * ONE_DAY_SECONDS + 1) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); // Assert that the Neuron owner's account balance has increased the expected amount let account_balance_after_disbursal = @@ -2714,7 +2733,9 @@ async fn test_disburse_maturity_applied_modulation_at_end_of_window() { .create(); // This is supposed to cause Governance to poll CMC for the maturity modulation. - canister_fixture.run_periodic_tasks_now(); + canister_fixture + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); let current_basis_points = canister_fixture .get_maturity_modulation() @@ -2782,8 +2803,11 @@ async fn test_disburse_maturity_applied_modulation_at_end_of_window() { .unwrap() = time_of_disbursement_maturity_modulation_basis_points; // Advancing time and triggering periodic tasks should force a query of the new modulation. - canister_fixture.advance_time_by(2 * ONE_DAY_SECONDS); - canister_fixture.run_periodic_tasks_now(); + canister_fixture + .advance_time_by(2 * ONE_DAY_SECONDS) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); + let current_basis_points = canister_fixture .get_maturity_modulation() .maturity_modulation @@ -2802,8 +2826,10 @@ async fn test_disburse_maturity_applied_modulation_at_end_of_window() { assert_eq!(account_balance_before_disbursal, 0); // Advancing time and triggering periodic tasks should trigger the final disbursal. - canister_fixture.advance_time_by(5 * ONE_DAY_SECONDS + 1); - canister_fixture.run_periodic_tasks_now(); + canister_fixture + .advance_time_by(5 * ONE_DAY_SECONDS + 1) + .temporarily_disable_sns_upgrades() + .run_periodic_tasks_now(); // Assert that the Neuron owner's account balance has increased the expected amount let account_balance_after_disbursal =