Skip to content

Commit

Permalink
feat(nns): Refresh voting power. (#2320)
Browse files Browse the repository at this point in the history 
This occurs when a neuron is modified in the following ways:

1. vote (directly, not indirectly)
2. change follow

# Background

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

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

# Future Work

This will be used to reduce the voting power in ballots when a neuron
has not refreshed for a long time.

# References

Closes https://dfinity.atlassian.net/browse/NNS1-3405.

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

[prev]: #2268
[next]: #2338
  • Loading branch information
@daniel-wong-dfinity-org
daniel-wong-dfinity-org authored Oct 31, 2024
1 parent 000547c commit a52f966
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 44 deletions.
27 changes: 25 additions & 2 deletions rs/nns/governance/src/governance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5584,6 +5584,8 @@ impl Governance {
// Finally, add this proposal as an open proposal.
self.insert_proposal(proposal_num, proposal_data);

self.refresh_voting_power(proposer_id);

Ok(proposal_id)
}

Expand Down Expand Up @@ -5966,9 +5968,27 @@ impl Governance {

self.process_proposal(proposal_id.id);

self.refresh_voting_power(neuron_id);

Ok(())
}

fn refresh_voting_power(&mut self, neuron_id: &NeuronId) {
let now_seconds = self.env.now();

let result = self.with_neuron_mut(neuron_id, |neuron| {
neuron.refresh_voting_power(now_seconds);
});

if let Err(err) = result {
println!(
"{}WARNING: Tried to refresh the voting power of neuron {}, \
but was unable to find it: {}",
LOG_PREFIX, neuron_id.id, err,
);
}
}

/// Add or remove followees for this neuron for a specified topic.
///
/// If the list of followees is empty, remove the followees for
Expand Down Expand Up @@ -6031,17 +6051,20 @@ impl Governance {
)
})?;

let now_seconds = self.env.now();
self.with_neuron_mut(id, |neuron| {
if follow_request.followees.is_empty() {
neuron.followees.remove(&(topic as i32))
neuron.followees.remove(&(topic as i32));
} else {
neuron.followees.insert(
topic as i32,
Followees {
followees: follow_request.followees.clone(),
},
)
);
}

neuron.refresh_voting_power(now_seconds);
})?;

Ok(())
Expand Down
12 changes: 12 additions & 0 deletions rs/nns/governance/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,18 @@ mod subaccount_index;
/// The value of 10_000 follows the Candid recommendation.
const DEFAULT_SKIPPING_QUOTA: usize = 10_000;

/// Value: one second after midnight, 2024-11-05 (UTC).
///
/// How this value was chosen: This is around the earliest time when
/// "refreshing" a neuron's voting power might be released, (assuming the usual
/// NNS release cycle). Significantly different values could also work, but this
/// seems like a nice "neutral" value.
///
/// How this value is used: when a neuron does not have a value in the
/// voting_power_refreshed_timestamp_seconds field (because it was created before
/// this feature), we pretend as though this value is in that field.
pub const DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS: u64 = 1731628801;

// TODO(NNS1-3248): Delete this once the feature has made it through the
// probation period. At that point, we will not need this "kill switch". We can
// leave this here indefinitely, but it will just be clutter after a modest
Expand Down
17 changes: 5 additions & 12 deletions rs/nns/governance/src/neuron/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
Neuron as NeuronProto, NeuronInfo, NeuronStakeTransfer, NeuronState, NeuronType, Topic,
Visibility, Vote,
},
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
};
use ic_base_types::PrincipalId;
use ic_cdk::println;
Expand All @@ -23,18 +24,6 @@ use ic_nns_common::pb::v1::{NeuronId, ProposalId};
use icp_ledger::Subaccount;
use std::collections::{BTreeSet, HashMap};

/// Value: one second after midnight, 2024-11-05 (UTC).
///
/// How this value was chosen: This is around the earliest time when
/// "refreshing" a neuron's voting power might be released, (assuming the usual
/// NNS release cycle). Significantly different values could also work, but this
/// seems like a nice "neutral" value.
///
/// How this value is used: when a neuron does not have a value in the
/// voting_power_refreshed_timestamp_seconds field (because it was created before
/// this feature), we pretend as though this value is in that field.
const DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS: u64 = 1731628801;

/// A neuron type internal to the governance crate. Currently, this type is identical to the
/// prost-generated Neuron type (except for derivations for prost). Gradually, this type will evolve
/// towards having all private fields while exposing methods for mutations, which allows it to hold
Expand Down Expand Up @@ -480,6 +469,10 @@ impl Neuron {
}
}

pub(crate) fn refresh_voting_power(&mut self, now_seconds: u64) {
self.voting_power_refreshed_timestamp_seconds = now_seconds;
}

pub(crate) fn ready_to_unstake_maturity(&self, now_seconds: u64) -> bool {
self.state(now_seconds) == NeuronState::Dissolved
&& self.staked_maturity_e8s_equivalent.unwrap_or(0) > 0
Expand Down
129 changes: 99 additions & 30 deletions rs/nns/governance/tests/governance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ use ic_nns_governance::{
},
temporarily_disable_private_neuron_enforcement, temporarily_disable_set_visibility_proposals,
temporarily_enable_private_neuron_enforcement, temporarily_enable_set_visibility_proposals,
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
};
use ic_nns_governance_api::{
pb::v1::CreateServiceNervousSystem as ApiCreateServiceNervousSystem,
Expand Down Expand Up @@ -265,16 +266,30 @@ fn test_single_neuron_proposal_new() {
NNSStateChange::GovernanceProto(vec![
GovernanceChange::Neurons(vec![MapChange::Changed(
0,
vec![NeuronChange::RecentBallots(vec![VecChange::Added(
0,
vec![
BallotInfoChange::ProposalId(OptionChange::Different(
None,
Some(ProposalId { id: 1 }),
)),
BallotInfoChange::Vote(I32Change(0, 1)),
],
)])],
vec![
NeuronChange::RecentBallots(vec![VecChange::Added(
0,
vec![
BallotInfoChange::ProposalId(OptionChange::Different(
None,
Some(ProposalId { id: 1 }),
)),
BallotInfoChange::Vote(I32Change(0, 1)),
],
)]),
// Neuron's voting power was refreshed, because it directly votes (i.e. not
// as a result of following), but implicitly voted (by virtue of being the
// proposer). (Normally, the amount would increase, but for testing
// purposes, it is enough to show that the value changed.)
NeuronChange::VotingPowerRefreshedTimestampSeconds(
OptionChange::BothSome(
U64Change(
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
DEFAULT_TEST_START_TIMESTAMP_SECONDS,
),
),
),
],
)]),
GovernanceChange::Proposals(vec![MapChange::Added(
1,
Expand Down Expand Up @@ -932,6 +947,13 @@ async fn test_cascade_following_new() {
BallotInfoChange::Vote(I32Change(0, 1)),
],
)]),
// As in an earlier test, the proposer's voting power gets refreshed.
NeuronChange::VotingPowerRefreshedTimestampSeconds(OptionChange::BothSome(
U64Change(
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
DEFAULT_TEST_START_TIMESTAMP_SECONDS,
),
),),
],
)]),
GovernanceChange::Proposals(vec![MapChange::Added(
Expand Down Expand Up @@ -1064,16 +1086,25 @@ async fn test_cascade_following_new() {
Changed::Changed(vec![NNSStateChange::GovernanceProto(vec![
GovernanceChange::Neurons(vec![MapChange::Changed(
5,
vec![NeuronChange::RecentBallots(vec![VecChange::Added(
0,
vec![
BallotInfoChange::ProposalId(OptionChange::Different(
None,
Some(ProposalId { id: 1 }),
)),
BallotInfoChange::Vote(I32Change(0, 1)),
],
)])],
vec![
NeuronChange::RecentBallots(vec![VecChange::Added(
0,
vec![
BallotInfoChange::ProposalId(OptionChange::Different(
None,
Some(ProposalId { id: 1 }),
)),
BallotInfoChange::Vote(I32Change(0, 1)),
],
)]),
// Neuron's voting power was refreshed, because it voted directly.
NeuronChange::VotingPowerRefreshedTimestampSeconds(OptionChange::BothSome(
U64Change(
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
DEFAULT_TEST_START_TIMESTAMP_SECONDS,
),
),),
],
)]),
GovernanceChange::Proposals(vec![MapChange::Changed(
1,
Expand Down Expand Up @@ -1167,16 +1198,26 @@ async fn test_cascade_following_new() {
),
MapChange::Changed(
6,
vec![NeuronChange::RecentBallots(vec![VecChange::Added(
0,
vec![
BallotInfoChange::ProposalId(OptionChange::Different(
None,
Some(ProposalId { id: 1 }),
)),
BallotInfoChange::Vote(I32Change(0, 1)),
],
)])],
vec![
NeuronChange::RecentBallots(vec![VecChange::Added(
0,
vec![
BallotInfoChange::ProposalId(OptionChange::Different(
None,
Some(ProposalId { id: 1 }),
)),
BallotInfoChange::Vote(I32Change(0, 1)),
],
)]),
// Only this neuron voted directly; therefore, even though other neurons
// voted (via following) only this neuron's voting power was refreshed.
NeuronChange::VotingPowerRefreshedTimestampSeconds(OptionChange::BothSome(
U64Change(
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
DEFAULT_TEST_START_TIMESTAMP_SECONDS,
),
),),
],
),
]),
GovernanceChange::Proposals(vec![MapChange::Changed(
Expand Down Expand Up @@ -1699,6 +1740,18 @@ async fn test_all_follow_proposer() {
driver.get_fake_ledger(),
driver.get_fake_cmc(),
);

// Later, we'll inspect the same field again to verify that voting power
// refresh actually took place. This is just to make sure that the later
// value is different from this earlier one.
assert_eq!(
gov.with_neuron(&NeuronId { id: 5 }, |neuron| {
neuron.voting_power_refreshed_timestamp_seconds()
})
.unwrap(),
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
);

// Add following for 5 and 6 for 1.
gov.manage_neuron(
// Must match neuron 5's serialized_id.
Expand All @@ -1716,6 +1769,22 @@ async fn test_all_follow_proposer() {
.unwrap()
.panic_if_error("Manage neuron failed");

// Assert that neuron 5's voting power was refreshed. More concretely,
// verifying that neuron 5's voting_power_refreshed_timestamp_seconds
// changed from before. (Earlier, we saw that this field had a different
// value, to wit, DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS)
assert_ne!(
DEFAULT_TEST_START_TIMESTAMP_SECONDS,
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS,
);
assert_eq!(
gov.with_neuron(&NeuronId { id: 5 }, |neuron| {
neuron.voting_power_refreshed_timestamp_seconds()
})
.unwrap(),
DEFAULT_TEST_START_TIMESTAMP_SECONDS,
);

gov.manage_neuron(
// Must match neuron 6's serialized_id.
&principal(6),
Expand Down

0 comments on commit a52f966

Please sign in to comment.