Skip to content

Commit

Permalink
chore(nns): Deleted backfilling voting_power_refreshed_timestamp. (#3780
Browse files Browse the repository at this point in the history 
)

Because it has already run once, and after that, it has no effect.

# References

This is the denouement of [PR 2972].

[PR 2972]: #2972
  • Loading branch information
@daniel-wong-dfinity-org
daniel-wong-dfinity-org authored Feb 5, 2025
1 parent 2608017 commit 8c3920c
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 216 deletions.
40 changes: 0 additions & 40 deletions rs/nns/governance/canister/canister.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,13 @@ fn schedule_timers() {
schedule_unstake_maturity_of_dissolved_neurons();
schedule_neuron_data_validation();
schedule_vote_processing();

// TODO(NNS1-3446): Delete. (This only needs to be run once, but can safely be run multiple times).
schedule_backfill_voting_power_refreshed_timestamps(Duration::from_secs(0));
}

// Seeding interval seeks to find a balance between the need for rng secrecy, and
// avoiding the overhead of frequent reseeding.
const SEEDING_INTERVAL: Duration = Duration::from_secs(3600);
const RETRY_SEEDING_INTERVAL: Duration = Duration::from_secs(30);
const PRUNE_FOLLOWING_INTERVAL: Duration = Duration::from_secs(10);
const BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_INTERVAL: Duration = Duration::from_secs(60);

// Once this amount of instructions is used by the
// Governance::prune_some_following, it stops, saves where it is, schedules more
Expand All @@ -195,8 +191,6 @@ const BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_INTERVAL: Duration = Duration::
// prune_some_following uses a couple of bucks worth of instructions each day.
const MAX_PRUNE_SOME_FOLLOWING_INSTRUCTIONS: u64 = 50_000_000;

const MAX_BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_INSTRUCTIONS: u64 = 50_000_000;

fn schedule_seeding(delay: Duration) {
ic_cdk_timers::set_timer(delay, || {
spawn(async {
Expand Down Expand Up @@ -236,40 +230,6 @@ fn schedule_prune_following(delay: Duration, original_begin: Bound<NeuronIdProto
});
}

thread_local! {
static BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_CHECKPOINT: RefCell<Bound<NeuronIdProto>> =
const { RefCell::new(Bound::Unbounded) };
}

fn schedule_backfill_voting_power_refreshed_timestamps(delay: Duration) {
ic_cdk_timers::set_timer(delay, || {
let original_checkpoint =
BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_CHECKPOINT.with(|p| *p.borrow());

let carry_on = || {
call_context_instruction_counter()
< MAX_BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_INSTRUCTIONS
};
let new_checkpoint = governance_mut()
.backfill_some_voting_power_refreshed_timestamps(original_checkpoint, carry_on);

BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_CHECKPOINT.with(|p| {
let mut borrow = p.borrow_mut();
*borrow = new_checkpoint;
});

let is_done = new_checkpoint == Bound::Unbounded;
if is_done {
return;
}

// Otherwise, continue later.
schedule_backfill_voting_power_refreshed_timestamps(
BACKFILL_VOTING_POWER_REFRESHED_TIMESTAMPS_INTERVAL,
);
});
}

// The interval before adjusting neuron storage for the next batch of neurons starting from last
// neuron id scanned in the last batch.
const ADJUST_NEURON_STORAGE_BATCH_INTERVAL: Duration = Duration::from_secs(5);
Expand Down
11 changes: 1 addition & 10 deletions rs/nns/governance/src/governance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ use crate::{
neuron::{DissolveStateAndAge, Neuron, NeuronBuilder, Visibility},
neuron_data_validation::{NeuronDataValidationSummary, NeuronDataValidator},
neuron_store::{
backfill_some_voting_power_refreshed_timestamps, metrics::NeuronSubsetMetrics,
prune_some_following, NeuronMetrics, NeuronStore,
metrics::NeuronSubsetMetrics, prune_some_following, NeuronMetrics, NeuronStore,
},
neurons_fund::{
NeuronsFund, NeuronsFundNeuronPortion, NeuronsFundSnapshot,
Expand Down Expand Up @@ -5993,14 +5992,6 @@ impl Governance {
)
}

pub fn backfill_some_voting_power_refreshed_timestamps(
&mut self,
begin: std::ops::Bound<NeuronId>,
carry_on: impl FnMut() -> bool,
) -> std::ops::Bound<NeuronId> {
backfill_some_voting_power_refreshed_timestamps(&mut self.neuron_store, begin, carry_on)
}

pub fn backfill_install_code_hashes(&mut self) {
for proposal in self.heap_data.proposals.values_mut() {
let Some(proposal) = proposal.proposal.as_mut() else {
Expand Down
11 changes: 0 additions & 11 deletions rs/nns/governance/src/neuron/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,17 +525,6 @@ impl Neuron {
u64::try_from(result).unwrap()
}

pub(crate) fn backfill_voting_power_refreshed_timestamp(&mut self) {
// This used to be the default, but we later changed our minds.
// The old definition:
// https://sourcegraph.com/github.com/dfinity/ic@1956e438af82a5b4aa9713bcbbe385684bf0704f/-/blob/rs/nns/governance/src/lib.rs?L189
const EVIL_TIMESTAMP_SECONDS: u64 = 1731628801;
if self.voting_power_refreshed_timestamp_seconds == EVIL_TIMESTAMP_SECONDS {
self.voting_power_refreshed_timestamp_seconds =
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_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
13 changes: 0 additions & 13 deletions rs/nns/governance/src/neuron_store.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1398,19 +1398,6 @@ pub fn groom_some_neurons(
}
}

pub fn backfill_some_voting_power_refreshed_timestamps(
neuron_store: &mut NeuronStore,
next: Bound<NeuronId>,
carry_on: impl FnMut() -> bool,
) -> Bound<NeuronId> {
groom_some_neurons(
neuron_store,
|neuron| neuron.backfill_voting_power_refreshed_timestamp(),
next,
carry_on,
)
}

/// Number of entries for each neuron indexes (in stable storage)
pub struct NeuronIndexesLens {
pub subaccount: usize,
Expand Down
144 changes: 2 additions & 142 deletions rs/nns/integration_tests/src/neuron_following.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use assert_matches::assert_matches;
use ic_base_types::PrincipalId;
use ic_nervous_system_common::{E8, ONE_DAY_SECONDS, ONE_MONTH_SECONDS};
use ic_nervous_system_common::{E8, ONE_MONTH_SECONDS};
use ic_nervous_system_integration_tests::pocket_ic_helpers::{install_canister, nns};
use ic_nns_common::{pb::v1::NeuronId, types::ProposalId};
use ic_nns_constants::{GOVERNANCE_CANISTER_ID, ROOT_CANISTER_ID};
use ic_nns_governance::DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS;
use ic_nns_governance_api::pb::v1::{
governance_error::ErrorType,
manage_neuron_response::{Command, FollowResponse},
neuron::{DissolveState, Followees},
Neuron, Tally, Topic, Visibility, Vote,
Neuron, Tally, Topic, Vote,
};
use ic_nns_governance_init::GovernanceCanisterInitPayloadBuilder;
use ic_nns_test_utils::{
Expand Down Expand Up @@ -539,145 +538,6 @@ async fn test_prune_some_following() {
}
}

#[tokio::test]
async fn test_backfill_voting_power_refreshed_timestamps() {
// Step 1: Prepare the world. (This mainly consists of initializing NNS
// governance canister with some specially crafted neurons.)

let pocket_ic = PocketIcBuilder::new().with_nns_subnet().build_async().await;

let now_seconds = pocket_ic
.get_time()
.await
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap()
.as_secs();

// This is used in dissolve_state to avoid making the neuron "active" (for
// the purposes of storage location). To control whether a neuron is active,
// we instead use the cached_neuron_stake_e8s field.
let earlier_timestamp_seconds = now_seconds - ONE_MONTH_SECONDS;

let neuron_base = Neuron {
dissolve_state: Some(DissolveState::WhenDissolvedTimestampSeconds(
earlier_timestamp_seconds,
)),
created_timestamp_seconds: earlier_timestamp_seconds,
aging_since_timestamp_seconds: u64::MAX,

// This is so that original_neurons will match what gets
// returned by the governance canister.
visibility: Some(Visibility::Private as i32),

..Default::default()
};

// When this occurs in the voting_power_refreshed_timestamp_seconds field,
// that field is supposed to get set to (the new)
// DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS.
const EVIL_TIMESTAMP_SECONDS: u64 = 1731628801;
// Assert that evil timestamp is "much greater" than the proper value.
{
let seconds = EVIL_TIMESTAMP_SECONDS - DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS;
assert!(
seconds as f64 / ONE_DAY_SECONDS as f64 > 60.0,
"{} vs. {}",
EVIL_TIMESTAMP_SECONDS as f64 / ONE_DAY_SECONDS as f64,
DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS as f64 / ONE_DAY_SECONDS as f64,
);
}

let voting_power_refreshed_timestamp_seconds_neuron_values = vec![
EVIL_TIMESTAMP_SECONDS - 1,
EVIL_TIMESTAMP_SECONDS,
EVIL_TIMESTAMP_SECONDS + 1,
now_seconds,
];

let cached_neuron_stake_e8s_neuron_values = vec![
0, // inactive -> stable memory
42 * E8, // active -> heap
];

let original_neurons = voting_power_refreshed_timestamp_seconds_neuron_values
.into_iter()
.cartesian_product(cached_neuron_stake_e8s_neuron_values.into_iter())
.collect::<Vec<_>>();

let controller = PrincipalId::new_user_test_id(482_783_461);

let ids = 100..;
let original_neurons = ids
.zip(original_neurons.into_iter())
.map(
|(id, (voting_power_refreshed_timestamp_seconds, cached_neuron_stake_e8s))| Neuron {
id: Some(NeuronId { id }),
controller: Some(controller),
account: vec![id as u8; 32],
cached_neuron_stake_e8s,
voting_power_refreshed_timestamp_seconds: Some(
voting_power_refreshed_timestamp_seconds,
),

..neuron_base.clone()
},
)
.collect::<Vec<_>>();

assert_eq!(original_neurons.len(), 8);

let governance_proto = GovernanceCanisterInitPayloadBuilder::new()
.with_additional_neurons(original_neurons.clone())
.build();

install_canister(
&pocket_ic,
"NNS Governance",
GOVERNANCE_CANISTER_ID,
governance_proto.encode_to_vec(),
build_test_governance_wasm(),
Some(ROOT_CANISTER_ID.get()),
)
.await;

// Step 2: Call the code under test.

// Wait for backfilling to complete.
for _ in 0..15 {
pocket_ic.advance_time(Duration::from_secs(1)).await;
pocket_ic.tick().await;
}

// Step 3: Inspect results

let mut observed_neurons = nns::governance::list_neurons(&pocket_ic, controller)
.await
.full_neurons;
assert_eq!(observed_neurons.len(), 8);
observed_neurons.sort_by_key(|neuron| neuron.id.as_ref().unwrap().id);
// Do not worry about verifying derived/automatically populated fields.
for observed_neuron in &mut observed_neurons {
observed_neuron.deciding_voting_power = None;
observed_neuron.potential_voting_power = None;
}

let mut expected_neurons = original_neurons.clone();
assert_eq!(
expected_neurons[2].voting_power_refreshed_timestamp_seconds,
Some(EVIL_TIMESTAMP_SECONDS),
);
assert_eq!(
expected_neurons[3].voting_power_refreshed_timestamp_seconds,
Some(EVIL_TIMESTAMP_SECONDS),
);
expected_neurons[2].voting_power_refreshed_timestamp_seconds =
Some(DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS);
expected_neurons[3].voting_power_refreshed_timestamp_seconds =
Some(DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS);

assert_eq!(observed_neurons, expected_neurons);
}

fn split_neuron(state_machine: &StateMachine, neuron: &TestNeuronOwner, amount: u64) -> NeuronId {
let response = nns_split_neuron(state_machine, neuron.principal_id, neuron.neuron_id, amount);
if let Command::Split(resp) = response.command.unwrap() {
Expand Down

0 comments on commit 8c3920c

Please sign in to comment.