Skip to content

Commit

Permalink
Merge branch 'canister-controlled-neuron-metrics-daniel-wong' into 'm…
Browse files Browse the repository at this point in the history
…aster'

feat(nns): Metrics for neurons controlled by non-self-authenticating principals.

Closes NNS1-3121.

[\<\< Previous MR](https://gitlab.com/dfinity-lab/public/ic/-/merge_requests/19801) 

Closes NNS1-3121

See merge request dfinity-lab/public/ic!19824
  • Loading branch information
daniel-wong-dfinity-org committed Jun 18, 2024
2 parents a9a6799 + 97a9c21 commit 2ebdd6f
Show file tree
Hide file tree
Showing 8 changed files with 197 additions and 8 deletions.
2 changes: 2 additions & 0 deletions rs/nns/governance/canister/governance.did
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,13 @@ type GovernanceCachedMetrics = record {
not_dissolving_neurons_count : nat64;
total_locked_e8s : nat64;
neurons_fund_total_active_neurons : nat64;
total_voting_power_non_self_authenticating_controller : opt nat64;
total_staked_maturity_e8s_equivalent : nat64;
not_dissolving_neurons_e8s_buckets_ect : vec record { nat64; float64 };
total_staked_e8s_ect : nat64;
not_dissolving_neurons_staked_maturity_e8s_equivalent_sum : nat64;
dissolved_neurons_e8s : nat64;
total_staked_e8s_non_self_authenticating_controller : opt nat64;
dissolving_neurons_e8s_buckets_seed : vec record { nat64; float64 };
neurons_with_less_than_6_months_dissolve_delay_e8s : nat64;
not_dissolving_neurons_staked_maturity_e8s_equivalent_buckets : vec record {
Expand Down
2 changes: 2 additions & 0 deletions rs/nns/governance/canister/governance_test.did
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,13 @@ type GovernanceCachedMetrics = record {
not_dissolving_neurons_count : nat64;
total_locked_e8s : nat64;
neurons_fund_total_active_neurons : nat64;
total_voting_power_non_self_authenticating_controller : opt nat64;
total_staked_maturity_e8s_equivalent : nat64;
not_dissolving_neurons_e8s_buckets_ect : vec record { nat64; float64 };
total_staked_e8s_ect : nat64;
not_dissolving_neurons_staked_maturity_e8s_equivalent_sum : nat64;
dissolved_neurons_e8s : nat64;
total_staked_e8s_non_self_authenticating_controller : opt nat64;
dissolving_neurons_e8s_buckets_seed : vec record { nat64; float64 };
neurons_with_less_than_6_months_dissolve_delay_e8s : nat64;
not_dissolving_neurons_staked_maturity_e8s_equivalent_buckets : vec record {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,8 @@ message Governance {
map<uint64, double> dissolving_neurons_e8s_buckets_ect = 33;
map<uint64, double> not_dissolving_neurons_e8s_buckets_seed = 34;
map<uint64, double> not_dissolving_neurons_e8s_buckets_ect = 35;
optional uint64 total_voting_power_non_self_authenticating_controller = 36;
optional uint64 total_staked_e8s_non_self_authenticating_controller = 37;
}

GovernanceCachedMetrics metrics = 15;
Expand Down
4 changes: 4 additions & 0 deletions rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2537,6 +2537,10 @@ pub mod governance {
pub not_dissolving_neurons_e8s_buckets_seed: ::std::collections::HashMap<u64, f64>,
#[prost(map = "uint64, double", tag = "35")]
pub not_dissolving_neurons_e8s_buckets_ect: ::std::collections::HashMap<u64, f64>,
#[prost(uint64, optional, tag = "36")]
pub total_voting_power_non_self_authenticating_controller: ::core::option::Option<u64>,
#[prost(uint64, optional, tag = "37")]
pub total_staked_e8s_non_self_authenticating_controller: ::core::option::Option<u64>,
}
/// Records that making an OpenSnsTokenSwap (OSTS) or CreateServiceNervousSystem (CSNS)
/// proposal is in progress. We only want one of these to be happening at the same time,
Expand Down
9 changes: 9 additions & 0 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7542,10 +7542,17 @@ impl Governance {
dissolving_neurons_e8s_buckets_ect,
not_dissolving_neurons_e8s_buckets_seed,
not_dissolving_neurons_e8s_buckets_ect,
total_voting_power_non_self_authenticating_controller,
total_staked_e8s_non_self_authenticating_controller,
} = self
.neuron_store
.compute_neuron_metrics(now, self.economics().neuron_minimum_stake_e8s);

let total_voting_power_non_self_authenticating_controller =
Some(total_voting_power_non_self_authenticating_controller);
let total_staked_e8s_non_self_authenticating_controller =
Some(total_staked_e8s_non_self_authenticating_controller);

GovernanceCachedMetrics {
timestamp_seconds: now,
total_supply_icp: icp_supply.get_tokens(),
Expand Down Expand Up @@ -7582,6 +7589,8 @@ impl Governance {
dissolving_neurons_e8s_buckets_ect,
not_dissolving_neurons_e8s_buckets_seed,
not_dissolving_neurons_e8s_buckets_ect,
total_voting_power_non_self_authenticating_controller,
total_staked_e8s_non_self_authenticating_controller,
}
}

Expand Down
16 changes: 16 additions & 0 deletions rs/nns/governance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,8 @@ pub fn encode_metrics(
dissolving_neurons_e8s_buckets_ect,
not_dissolving_neurons_e8s_buckets_seed,
not_dissolving_neurons_e8s_buckets_ect,
total_voting_power_non_self_authenticating_controller,
total_staked_e8s_non_self_authenticating_controller,
} = metrics;

w.encode_gauge(
Expand Down Expand Up @@ -645,6 +647,20 @@ pub fn encode_metrics(
"The total amount of Neurons' staked maturity in ECT Neurons",
)?;

w.encode_gauge(
"governance_total_voting_power_non_self_authenticating_controller",
total_voting_power_non_self_authenticating_controller.unwrap_or_default() as f64,
"The total amount of voting power of all neurons with a \
controller that is not self-authenticating",
)?;

w.encode_gauge(
"governance_total_staked_e8s_non_self_authenticating_controller",
total_staked_e8s_non_self_authenticating_controller.unwrap_or_default() as f64,
"The total amount of staked ICP in neurons with a \
controller that is not self-authenticating",
)?;

encode_dissolve_delay_buckets(
w.gauge_vec(
"governance_dissolving_neurons_e8s_seed",
Expand Down
131 changes: 126 additions & 5 deletions rs/nns/governance/src/neuron_store/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,17 @@ use std::collections::HashMap;
#[derive(Default, Debug, Clone, PartialEq)]
pub struct NeuronMetrics {
pub dissolving_neurons_count: u64,
pub dissolving_neurons_e8s_buckets: HashMap<u64, f64>,
// This maps floor(dissolve delay / 6 months) to total e8s.
//
// The keys used by Other fields (with names of the form `*_buckets`) are
// also like this.
//
// Also, notice that the value type is f64. Presumably, the reasoning there
// is that these are eventually turned into Prometheus metrics.
pub dissolving_neurons_e8s_buckets: HashMap<
u64, // floor(dissolve delay / 6 months)
f64, // total e8s
>,
pub dissolving_neurons_count_buckets: HashMap<u64, u64>,
pub not_dissolving_neurons_count: u64,
pub not_dissolving_neurons_e8s_buckets: HashMap<u64, f64>,
Expand Down Expand Up @@ -39,6 +49,8 @@ pub struct NeuronMetrics {
pub dissolving_neurons_e8s_buckets_ect: HashMap<u64, f64>,
pub not_dissolving_neurons_e8s_buckets_seed: HashMap<u64, f64>,
pub not_dissolving_neurons_e8s_buckets_ect: HashMap<u64, f64>,
pub total_voting_power_non_self_authenticating_controller: u64,
pub total_staked_e8s_non_self_authenticating_controller: u64,
}

impl NeuronStore {
Expand All @@ -57,6 +69,14 @@ impl NeuronStore {
};

for neuron in self.heap_neurons.values() {
let voting_power = neuron.voting_power(now_seconds);
let minted_stake_e8s = neuron.minted_stake_e8s();

if !neuron.controller().is_self_authenticating() {
metrics.total_voting_power_non_self_authenticating_controller += voting_power;
metrics.total_staked_e8s_non_self_authenticating_controller += minted_stake_e8s;
}

metrics.total_staked_e8s += neuron.minted_stake_e8s();
metrics.total_staked_maturity_e8s_equivalent +=
neuron.staked_maturity_e8s_equivalent.unwrap_or(0);
Expand Down Expand Up @@ -222,11 +242,11 @@ mod tests {
pb::v1::NeuronType,
};
use ic_base_types::PrincipalId;
use ic_nervous_system_common::{ONE_DAY_SECONDS, ONE_YEAR_SECONDS};
use ic_nervous_system_common::{E8, ONE_DAY_SECONDS, ONE_YEAR_SECONDS};
use ic_nns_common::pb::v1::NeuronId;
use icp_ledger::Subaccount;
use maplit::hashmap;
use std::collections::BTreeMap;
use maplit::{btreemap, hashmap};
use std::{collections::BTreeMap, str::FromStr};

fn create_test_neuron_builder(
id: u64,
Expand Down Expand Up @@ -500,8 +520,18 @@ mod tests {
dissolving_neurons_e8s_buckets_ect: hashmap! { 6 => 568000000.0 },
not_dissolving_neurons_e8s_buckets_seed: hashmap! { 0 => 100000000.0 },
not_dissolving_neurons_e8s_buckets_ect: hashmap! { 2 => 234000000.0 },
// Some garbage values, because this test was written before this feature.
total_voting_power_non_self_authenticating_controller: 42,
total_staked_e8s_non_self_authenticating_controller: 43,
};
assert_eq!(metrics, expected_metrics);
assert_eq!(
NeuronMetrics {
total_voting_power_non_self_authenticating_controller: 42,
total_staked_e8s_non_self_authenticating_controller: 43,
..metrics
},
expected_metrics,
);
}

#[test]
Expand Down Expand Up @@ -557,4 +587,95 @@ mod tests {
let actual_metrics = neuron_store.compute_neuron_metrics(now, 100_000_000);
assert_eq!(actual_metrics.garbage_collectable_neurons_count, 2);
}

/// In this test, the NeuronStore has 3 neurons. Neurons 1 and 3 have (different)
/// non-self-authenticating controllers (weird). Whereas, 2 has a self-authenticating controller
/// (normal).
///
/// They have radically different staked amounts, to make it clearer what
/// bug(s) might exist if/when this test fails.
#[test]
fn test_compute_neuron_metrics_non_self_authenticating() {
// Step 1: Prepare the world.

// Step 1.1: Construct controllers (per test dcostring).

let controller_of_neuron_1 = PrincipalId::new_user_test_id(0x1337_CAFE);
assert!(!controller_of_neuron_1.is_self_authenticating());

let controller_of_neuron_2 = PrincipalId::from_str(
"ubktz-haghv-fqsdh-23fhi-3urex-bykoz-pvpfd-5rs6w-qpo3t-nf2dv-oae",
)
.unwrap();
assert!(controller_of_neuron_2.is_self_authenticating());

let controller_of_neuron_3 = PrincipalId::from_str(
// This is the NNS root canister's principal (canister) ID.
"r7inp-6aaaa-aaaaa-aaabq-cai",
)
.unwrap();
assert!(!controller_of_neuron_3.is_self_authenticating());

// Step 1.1 Misc shared values.

let now_seconds = 1718213756;
// This gives 2x dissolve delay bonus.
let dissolve_state_and_age = DissolveStateAndAge::NotDissolving {
dissolve_delay_seconds: 8 * ONE_YEAR_SECONDS,
aging_since_timestamp_seconds: now_seconds,
};

// Step 1.2: Finally, construct the main object, NeuronStore.

let neuron_store = NeuronStore::new(btreemap! {
1 => NeuronBuilder::new(
NeuronId { id: 1 },
Subaccount::try_from([1_u8; 32].as_ref()).unwrap(),
controller_of_neuron_1,
dissolve_state_and_age,
now_seconds,
)
.with_cached_neuron_stake_e8s(100)
.build(),

2 => NeuronBuilder::new(
NeuronId { id: 2 },
Subaccount::try_from([2_u8; 32].as_ref()).unwrap(),
controller_of_neuron_2,
dissolve_state_and_age,
now_seconds,
)
.with_cached_neuron_stake_e8s(200_000)
.build(),

3 => NeuronBuilder::new(
NeuronId { id: 3 },
Subaccount::try_from([3_u8; 32].as_ref()).unwrap(),
controller_of_neuron_3,
dissolve_state_and_age,
now_seconds,
)
.with_cached_neuron_stake_e8s(300_000_000)
.build(),
});

// Step 2: Call code under test.

let NeuronMetrics {
total_voting_power_non_self_authenticating_controller,
total_staked_e8s_non_self_authenticating_controller,
..
} = neuron_store.compute_neuron_metrics(now_seconds, E8);

// The expected value for voting power is 2x e8s, because the dissolve
// delay bonus is 2x (and no age bonus).
assert_eq!(
total_voting_power_non_self_authenticating_controller,
600_000_200
);
assert_eq!(
total_staked_e8s_non_self_authenticating_controller,
300_000_100
);
}
}
39 changes: 36 additions & 3 deletions rs/nns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,12 @@ fn test_single_neuron_proposal_new() {
0.0,
),
]),
GovernanceCachedMetricsChange::TotalVotingPowerNonSelfAuthenticatingController(
comparable::OptionChange::Different(None, Some(1)),
),
GovernanceCachedMetricsChange::TotalStakedE8SNonSelfAuthenticatingController(
comparable::OptionChange::Different(None, Some(1)),
),
]),
)),
GovernanceChange::CachedDailyMaturityModulationBasisPoints(
Expand Down Expand Up @@ -12897,6 +12903,9 @@ async fn test_metrics() {
dissolving_neurons_e8s_buckets_ect: Default::default(),
not_dissolving_neurons_e8s_buckets_seed: hashmap! {0 => 100000000.0},
not_dissolving_neurons_e8s_buckets_ect: hashmap! {2 => 200000000.0},
// Garbage values, because this test was written before this feature.
total_voting_power_non_self_authenticating_controller: Some(0xDEAD),
total_staked_e8s_non_self_authenticating_controller: Some(0xBEEF),
};

let driver = fake::FakeDriver::default().at(60 * 60 * 24 * 30);
Expand All @@ -12909,7 +12918,14 @@ async fn test_metrics() {

let actual_metrics = gov.compute_cached_metrics(now, Tokens::new(0, 0).unwrap());
assert_eq!(
expected_metrics, actual_metrics,
expected_metrics,
GovernanceCachedMetrics {
// Garbage values, because this test was written before this feature.
total_voting_power_non_self_authenticating_controller: Some(0xDEAD),
total_staked_e8s_non_self_authenticating_controller: Some(0xBEEF),

..actual_metrics
},
"Cached metrics don't match expected metrics."
);

Expand All @@ -12918,7 +12934,14 @@ async fn test_metrics() {
// Check again after periodic task.
let actual_metrics = gov.compute_cached_metrics(now, Tokens::new(0, 0).unwrap());
assert_eq!(
expected_metrics, actual_metrics,
expected_metrics,
GovernanceCachedMetrics {
// Garbage values, because this test was written before this feature.
total_voting_power_non_self_authenticating_controller: Some(0xDEAD),
total_staked_e8s_non_self_authenticating_controller: Some(0xBEEF),

..actual_metrics
},
"Invalid metrics after period tasks execution."
);

Expand Down Expand Up @@ -12966,10 +12989,20 @@ async fn test_metrics() {
dissolving_neurons_e8s_buckets_ect: Default::default(),
not_dissolving_neurons_e8s_buckets_seed: hashmap! { 0 => 100000000.0 },
not_dissolving_neurons_e8s_buckets_ect: hashmap! { 2 => 200000000.0 },
// Garbage values, because this test was written before this feature.
total_voting_power_non_self_authenticating_controller: Some(0xDEAD),
total_staked_e8s_non_self_authenticating_controller: Some(0xBEEF),
};
let metrics = gov.get_metrics().expect("Error while querying metrics.");
assert_eq!(
expected_metrics, metrics,
expected_metrics,
GovernanceCachedMetrics {
// Garbage values, because this test was written before this feature.
total_voting_power_non_self_authenticating_controller: Some(0xDEAD),
total_staked_e8s_non_self_authenticating_controller: Some(0xBEEF),

..metrics
},
"Queried metrics don't match expected metrics."
);

Expand Down

0 comments on commit 2ebdd6f

Please sign in to comment.