Skip to content

Commit

Permalink
feat(nns): Make distinction between potential and deciding voting pow…
Browse files Browse the repository at this point in the history
…er. (#2339)

Add deciding_voting_power method to Neuron.

Renames the voting_power method of Neuron to potential_voting_power.

Added a flag that controls whether deciding_voting_power applies
adjustment. Per usual, adjustment is off in vanilla builds, but on when
feature = "test".

Voting rewards uses this new method. As long as the the new behavior is
not enabled (via the flag), this should result in no behavior change,
and even initially, when the feature is enabled, since no neuron can be
too unrefreshed, even then, there would be no immediate behavior change.

# Background

This is part of the "periodic confirmation" feature that was recently
approved in proposal [132411][prop].

[prop]: https://dashboard.internetcomputer.org/proposal/132411

# References

Closes https://dfinity.atlassian.net/browse/NNS1-3406,
https://dfinity.atlassian.net/browse/NNS1-3407.

[👈 Previous PR][prev] | [Next PR 👉][next]

[prev]: #2338
[next]: #2375
  • Loading branch information
daniel-wong-dfinity-org authored Nov 5, 2024
1 parent d0b007b commit 7927c34
Show file tree
Hide file tree
Showing 11 changed files with 255 additions and 42 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rs/nns/governance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ DEPENDENCIES = [
"//rs/nervous_system/clients",
"//rs/nervous_system/common",
"//rs/nervous_system/governance",
"//rs/nervous_system/linear_map",
"//rs/nervous_system/neurons_fund",
"//rs/nervous_system/proto",
"//rs/nervous_system/root",
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ ic-nervous-system-clients = { path = "../../nervous_system/clients" }
ic-nervous-system-common = { path = "../../nervous_system/common" }
ic-nervous-system-common-build-metadata = { path = "../../nervous_system/common/build_metadata" }
ic-nervous-system-governance = { path = "../../nervous_system/governance" }
ic-nervous-system-linear-map = { path = "../../nervous_system/linear_map" }
ic-nervous-system-root = { path = "../../nervous_system/root" }
ic-nervous-system-runtime = { path = "../../nervous_system/runtime" }
ic-nervous-system-proto = { path = "../../nervous_system/proto" }
Expand Down
2 changes: 1 addition & 1 deletion rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5671,7 +5671,7 @@ impl Governance {
// No neuron in the stable storage should have maturity.

for neuron in self.neuron_store.voting_eligible_neurons(now_seconds) {
let voting_power = neuron.voting_power(now_seconds);
let voting_power = neuron.deciding_voting_power(now_seconds);

total_power += voting_power as u128;

Expand Down
42 changes: 26 additions & 16 deletions rs/nns/governance/src/governance/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1193,9 +1193,9 @@ mod cast_vote_and_cascade_follow {
followees: Vec<u64>,
vote: Vote| {
let neuron = make_neuron(id, followees);
let voting_power = neuron.voting_power(now);
let deciding_voting_power = neuron.deciding_voting_power(now);
neuron_map.insert(id, neuron);
ballots.insert(id, make_ballot(voting_power, vote));
ballots.insert(id, make_ballot(deciding_voting_power, vote));
};

let add_neuron_without_ballot =
Expand Down Expand Up @@ -1241,15 +1241,20 @@ mod cast_vote_and_cascade_follow {
&mut neuron_store,
);

let deciding_voting_power = |neuron_id| {
neuron_store
.with_neuron(&neuron_id, |n| n.deciding_voting_power(now))
.unwrap()
};
assert_eq!(
ballots,
hashmap! {
1 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 1}, |n| n.voting_power(now)).unwrap(), Vote::Yes),
2 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 2}, |n| n.voting_power(now)).unwrap(), Vote::Unspecified),
3 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 3}, |n| n.voting_power(now)).unwrap(), Vote::Unspecified),
4 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 4}, |n| n.voting_power(now)).unwrap(), Vote::Unspecified),
5 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 5}, |n| n.voting_power(now)).unwrap(), Vote::Unspecified),
6 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 6}, |n| n.voting_power(now)).unwrap(), Vote::Unspecified),
1 => make_ballot(deciding_voting_power(NeuronId { id: 1}), Vote::Yes),
2 => make_ballot(deciding_voting_power(NeuronId { id: 2}), Vote::Unspecified),
3 => make_ballot(deciding_voting_power(NeuronId { id: 3}), Vote::Unspecified),
4 => make_ballot(deciding_voting_power(NeuronId { id: 4}), Vote::Unspecified),
5 => make_ballot(deciding_voting_power(NeuronId { id: 5}), Vote::Unspecified),
6 => make_ballot(deciding_voting_power(NeuronId { id: 6}), Vote::Unspecified),
}
);
}
Expand All @@ -1269,9 +1274,9 @@ mod cast_vote_and_cascade_follow {
followees: Vec<u64>,
vote: Vote| {
let neuron = make_neuron(id, followees);
let voting_power = neuron.voting_power(now);
let deciding_voting_power = neuron.deciding_voting_power(now);
neuron_map.insert(id, neuron);
ballots.insert(id, make_ballot(voting_power, vote));
ballots.insert(id, make_ballot(deciding_voting_power, vote));
};

let add_neuron_without_ballot =
Expand Down Expand Up @@ -1305,15 +1310,20 @@ mod cast_vote_and_cascade_follow {
&mut neuron_store,
);

let deciding_voting_power = |neuron_id| {
neuron_store
.with_neuron(&neuron_id, |n| n.deciding_voting_power(now))
.unwrap()
};
assert_eq!(
ballots,
hashmap! {
1 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 1}, |n| n.voting_power(now)).unwrap(), Vote::Yes),
2 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 2}, |n| n.voting_power(now)).unwrap(), Vote::Yes),
3 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 3}, |n| n.voting_power(now)).unwrap(), Vote::Yes),
4 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 4}, |n| n.voting_power(now)).unwrap(), Vote::Yes),
5 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 5}, |n| n.voting_power(now)).unwrap(), Vote::Yes),
6 => make_ballot(neuron_store.with_neuron(&NeuronId {id: 6}, |n| n.voting_power(now)).unwrap(), Vote::Unspecified),
1 => make_ballot(deciding_voting_power(NeuronId { id: 1 }), Vote::Yes),
2 => make_ballot(deciding_voting_power(NeuronId { id: 2 }), Vote::Yes),
3 => make_ballot(deciding_voting_power(NeuronId { id: 3 }), Vote::Yes),
4 => make_ballot(deciding_voting_power(NeuronId { id: 4 }), Vote::Yes),
5 => make_ballot(deciding_voting_power(NeuronId { id: 5 }), Vote::Yes),
6 => make_ballot(deciding_voting_power(NeuronId { id: 6 }), Vote::Unspecified),
}
);
}
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 @@ -192,6 +192,8 @@ pub const DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS: u64 = 1731628801;
// leave this here indefinitely, but it will just be clutter after a modest
// amount of time.
thread_local! {
static IS_VOTING_POWER_ADJUSTMENT_ENABLED: Cell<bool> = const { Cell::new(cfg!(feature = "test")) };

// TODO(NNS1-3247): To release the feature, set this to true. Do not simply
// delete. That way, if we need to recall the feature, we can do that via a
// 1-line change (by replacing true with `cfg!(feature = "test")`). After
Expand All @@ -204,6 +206,20 @@ thread_local! {
static ACTIVE_NEURONS_IN_STABLE_MEMORY_ENABLED: Cell<bool> = const { Cell::new(false) };
}

pub fn is_voting_power_adjustment_enabled() -> bool {
IS_VOTING_POWER_ADJUSTMENT_ENABLED.with(|ok| ok.get())
}

/// Only integration tests should use this.
pub fn temporarily_enable_voting_power_adjustment() -> Temporary {
Temporary::new(&IS_VOTING_POWER_ADJUSTMENT_ENABLED, true)
}

/// Only integration tests should use this.
pub fn temporarily_disable_voting_power_adjustment() -> Temporary {
Temporary::new(&IS_VOTING_POWER_ADJUSTMENT_ENABLED, false)
}

pub fn is_private_neuron_enforcement_enabled() -> bool {
IS_PRIVATE_NEURON_ENFORCEMENT_ENABLED.with(|ok| ok.get())
}
Expand Down
60 changes: 55 additions & 5 deletions rs/nns/governance/src/neuron/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
LOG_PREFIX, MAX_DISSOLVE_DELAY_SECONDS, MAX_NEURON_AGE_FOR_AGE_BONUS,
MAX_NEURON_RECENT_BALLOTS, MAX_NUM_HOT_KEYS_PER_NEURON,
},
is_private_neuron_enforcement_enabled,
is_private_neuron_enforcement_enabled, is_voting_power_adjustment_enabled,
neuron::{combine_aged_stakes, dissolve_state_and_age::DissolveStateAndAge, neuron_stake_e8s},
neuron_store::NeuronStoreError,
pb::v1::{
Expand All @@ -19,10 +19,15 @@ use crate::{
};
use ic_base_types::PrincipalId;
use ic_cdk::println;
use ic_nervous_system_common::ONE_DAY_SECONDS;
use ic_nervous_system_common::{ONE_DAY_SECONDS, ONE_MONTH_SECONDS};
use ic_nervous_system_linear_map::LinearMap;
use ic_nns_common::pb::v1::{NeuronId, ProposalId};
use icp_ledger::Subaccount;
use std::collections::{BTreeSet, HashMap};
use rust_decimal::Decimal;
use std::{
collections::{BTreeSet, HashMap},
time::Duration,
};

/// A neuron type internal to the governance crate. Gradually, this type will evolve
/// towards having all private fields while exposing methods for mutations, which allows it to hold
Expand Down Expand Up @@ -347,14 +352,57 @@ impl Neuron {
> 0
}

fn deciding_voting_power_adjustment_factor(
duration_since_voting_power_refreshed: Duration,
) -> Decimal {
let linear_map = LinearMap::new(
Decimal::from(6 * ONE_MONTH_SECONDS)..Decimal::from(7 * ONE_MONTH_SECONDS), // from
Decimal::from(1)..Decimal::from(0), // to
);

linear_map
.apply(Decimal::from(
duration_since_voting_power_refreshed.as_secs(),
))
.clamp(Decimal::from(0), Decimal::from(1))
}

/// How much swap this neuron has when it casts its vote on proposals.
pub fn deciding_voting_power(&self, now_seconds: u64) -> u64 {
// Main inputs.
let adjustment_factor: Decimal = if is_voting_power_adjustment_enabled() {
Self::deciding_voting_power_adjustment_factor(Duration::from_secs(
now_seconds.saturating_sub(self.voting_power_refreshed_timestamp_seconds),
))
} else {
Decimal::from(1)
};
let potential_voting_power = self.potential_voting_power(now_seconds);

// Main calculation.
let result = adjustment_factor * Decimal::from(potential_voting_power);

// Convert (back) to u64.
let result = result.round();
u64::try_from(result).unwrap_or_else(|err| {
// Log and fall back to potential voting power. Assuming
// adjustment_factor is in [0, 1], I see no way this can happen.
println!(
"{}ERROR: Unable to convert deciding voting power {} * {} back to u64: {:?}",
LOG_PREFIX, adjustment_factor, potential_voting_power, err,
);
potential_voting_power
})
}

/// Return the voting power of this neuron.
///
/// The voting power is the stake of the neuron modified by a
/// bonus of up to 100% depending on the dissolve delay, with
/// the maximum bonus of 100% received at an 8 year dissolve
/// delay. The voting power is further modified by the age of
/// the neuron giving up to 25% bonus after four years.
pub fn voting_power(&self, now_seconds: u64) -> u64 {
pub fn potential_voting_power(&self, now_seconds: u64) -> u64 {
// We compute the stake adjustments in u128.
let stake = self.stake_e8s() as u128;
// Dissolve delay is capped to eight years, but we cap it
Expand Down Expand Up @@ -833,13 +881,15 @@ impl Neuron {

let visibility = self.visibility().map(|visibility| visibility as i32);

let potential_voting_power = self.potential_voting_power(now_seconds);

NeuronInfo {
retrieved_at_timestamp_seconds: now_seconds,
state: self.state(now_seconds) as i32,
age_seconds: self.age_seconds(now_seconds),
dissolve_delay_seconds: self.dissolve_delay_seconds(now_seconds),
recent_ballots,
voting_power: self.voting_power(now_seconds),
voting_power: potential_voting_power,
created_timestamp_seconds: self.created_timestamp_seconds,
stake_e8s: self.minted_stake_e8s(),
joined_community_fund_timestamp_seconds,
Expand Down
134 changes: 133 additions & 1 deletion rs/nns/governance/src/neuron/types/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use super::*;
use crate::{
neuron::{DissolveStateAndAge, NeuronBuilder},
pb::v1::manage_neuron::{SetDissolveTimestamp, StartDissolving},
temporarily_disable_private_neuron_enforcement, temporarily_enable_private_neuron_enforcement,
temporarily_disable_private_neuron_enforcement, temporarily_disable_voting_power_adjustment,
temporarily_enable_private_neuron_enforcement, temporarily_enable_voting_power_adjustment,
};
use ic_cdk::println;

Expand Down Expand Up @@ -538,3 +539,134 @@ fn test_visibility_when_converting_neuron_to_neuron_info_and_neuron_proto() {
assert_eq!(neuron_proto.visibility, Some(Visibility::Public as i32),);
}
}

#[test]
fn test_adjust_voting_power_enabled() {
let _restore_on_drop = temporarily_enable_voting_power_adjustment();

let principal_id = PrincipalId::new_user_test_id(42);
let created_timestamp_seconds = 1729791574;

let neuron = NeuronBuilder::new(
NeuronId { id: 42 },
Subaccount::try_from(vec![42u8; 32].as_slice()).unwrap(),
principal_id,
DissolveStateAndAge::NotDissolving {
dissolve_delay_seconds: 12 * ONE_MONTH_SECONDS,
aging_since_timestamp_seconds: created_timestamp_seconds + 42,
},
created_timestamp_seconds, // created
)
.with_cached_neuron_stake_e8s(100 * E8)
.build();
let original_potential_voting_power = neuron.potential_voting_power(created_timestamp_seconds);
assert!(original_potential_voting_power > 0);

// At first, there is no difference between deciding and potential voting
// power. The neuron is considered "current".
assert_eq!(
neuron.deciding_voting_power(created_timestamp_seconds),
original_potential_voting_power,
);

// In fact, for the next 6 months, the two remain the same.
let mut previous_potential_voting_power = original_potential_voting_power;
for months in 1..=6 {
let now_seconds = created_timestamp_seconds + months * ONE_MONTH_SECONDS;
let current_potential_voting_power = neuron.potential_voting_power(now_seconds);

assert_eq!(
neuron.deciding_voting_power(now_seconds),
current_potential_voting_power,
);

// This is not verifying the code under test, but is here just as a
// sanity check. The reason we expect potential voting power to keep
// rising is because of age bonus.
assert!(
current_potential_voting_power > previous_potential_voting_power,
"at {} months: {} vs. {}",
months,
original_potential_voting_power,
previous_potential_voting_power,
);

previous_potential_voting_power = current_potential_voting_power;
}

// Now, we are in the adjustment period where the neuron has not been
// updated in "too long" of a time, and as a result, it is now experiencing
// voting power reduction penalties.
for months in [0.0, 0.01, 0.1, 0.25, 0.5, 0.75, 0.9, 0.99] {
let now_seconds =
created_timestamp_seconds + ((6.0 + months) * ONE_MONTH_SECONDS as f64) as u64;

fn relative_error(observed_value: f64, expected_value: f64) -> f64 {
assert!(expected_value.abs() > 1e-9);
(observed_value - expected_value) / expected_value
}

let observed = neuron.deciding_voting_power(now_seconds);
let current_potential_voting_power = neuron.potential_voting_power(now_seconds);
let expected = (1.0 - months) * current_potential_voting_power as f64;
let err = relative_error(
observed as f64,
// Expected value.
expected,
);
assert!(
err < 1e-6, // Relative error is less than 1 ppm (parts per million).
"at {} months: {} vs. {} ({:+0.}% off potential {})",
6.0 + months,
observed,
expected,
100.0 * err,
current_potential_voting_power,
);
}

// Starting at 7 months of no voting power refresh, deciding voting power
// goes all the way down to 0.
for months in 7..=10 {
let now_seconds = created_timestamp_seconds + months * ONE_MONTH_SECONDS;
assert_eq!(neuron.deciding_voting_power(now_seconds), 0,);
}
}

#[test]
fn test_adjust_voting_power_disabled() {
let _restore_on_drop = temporarily_disable_voting_power_adjustment();

let principal_id = PrincipalId::new_user_test_id(42);
let created_timestamp_seconds = 1729791574;

let neuron = NeuronBuilder::new(
NeuronId { id: 42 },
Subaccount::try_from(vec![42u8; 32].as_slice()).unwrap(),
principal_id,
DissolveStateAndAge::NotDissolving {
dissolve_delay_seconds: 12 * ONE_MONTH_SECONDS,
aging_since_timestamp_seconds: created_timestamp_seconds + 42,
},
created_timestamp_seconds, // created
)
.with_cached_neuron_stake_e8s(100 * E8)
.build();
let original_potential_voting_power = neuron.potential_voting_power(created_timestamp_seconds);
assert!(original_potential_voting_power > 0);

// At all times, deciding voting power is exactly the same as potential
// voting power, because adjustment is disabled.
for months in [
0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 6.001, 6.1, 6.25, 6.5, 6.75, 6.9, 6.999, 7.0, 7.001,
7.1, 7.25, 7.5, 8.0, 9.0, 10.0,
] {
let now_seconds = created_timestamp_seconds + (months * ONE_MONTH_SECONDS as f64) as u64;
let current_potential_voting_power = neuron.potential_voting_power(now_seconds);

assert_eq!(
neuron.deciding_voting_power(now_seconds),
current_potential_voting_power,
);
}
}
4 changes: 3 additions & 1 deletion rs/nns/governance/src/neuron_store/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ impl NeuronSubsetMetrics {
neuron.staked_maturity_e8s_equivalent.unwrap_or_default();
let maturity_e8s_equivalent = neuron.maturity_e8s_equivalent;

let voting_power = neuron.voting_power(now_seconds);
// 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 increment = |total: &mut u64, additional_amount| {
*total = total.saturating_add(additional_amount);
Expand Down
Loading

0 comments on commit 7927c34

Please sign in to comment.