Skip to content

Commit

Permalink
Refresh voting power when vote directly. Also, when change following.
Browse files Browse the repository at this point in the history
  • Loading branch information
@daniel-wong-dfinity-org
daniel-wong-dfinity-org committed Oct 29, 2024
1 parent 46f3e2d commit 8f2e696
Show file tree
Hide file tree
Showing 4 changed files with 113 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 update the voting power of {}, \
but where neuron? {}",
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
101 changes: 71 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 @@ -262,16 +263,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 @@ -929,6 +944,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 @@ -1061,16 +1083,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 @@ -1164,16 +1195,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

0 comments on commit 8f2e696

Please sign in to comment.