diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 9263eceb6f2..a5b24e6a9fb 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -2234,8 +2234,6 @@ message Governance { // Migrates neuron indexes to stable storage. Migration neuron_indexes_migration = 1; Migration copy_inactive_neurons_to_stable_memory_migration = 2; - - // TODO(NNS1-2533): Migration delete_inactive_neurons_from_heap = 3; } // Migration related data. @@ -2555,10 +2553,12 @@ message AuditEvent { uint64 timestamp_seconds = 1; oneof payload { - // Reset aging timestamps for https://forum.dfinity.org/t/icp-neuron-age-is-52-years/21261/26 + // Reset aging timestamps (https://forum.dfinity.org/t/icp-neuron-age-is-52-years/21261/26). ResetAging reset_aging = 2; - // Restore aging timestamp that were incorrectly reset. + // Restore aging timestamp that were incorrectly reset (https://forum.dfinity.org/t/restore-neuron-age-in-proposal-129394/29840). RestoreAging restore_aging = 3; + // Normalize neuron dissolve state and age (https://forum.dfinity.org/t/simplify-neuron-state-age/30527) + NormalizeDissolveStateAndAge normalize_dissolve_state_and_age = 4; } message ResetAging { @@ -2600,6 +2600,30 @@ message AuditEvent { // Neuron's stake at the time of restore. optional uint64 neuron_stake_e8s = 6; } + + enum NeuronLegacyCase { + NEURON_LEGACY_CASE_UNSPECIFIED = 0; + // Neuron is dissolving or dissolved but with a non-zero age. + NEURON_LEGACY_CASE_DISSOLVING_OR_DISSOLVED = 1; + // Neuron is dissolved with DissolveDelaySeconds(0). + NEURON_LEGACY_CASE_DISSOLVED = 2; + // Neuron has a None dissolve state. + NEURON_LEGACY_CASE_NONE_DISSOLVE_STATE = 3; + } + + message NormalizeDissolveStateAndAge { + // The neuron id whose dissolve state and age were normalized. + optional uint64 neuron_id = 1; + + // Which legacy case the neuron falls into. + NeuronLegacyCase neuron_legacy_case = 2; + + // Previous when_dissolved_timestamp_seconds if the neuron was dissolving or dissolved. + optional uint64 previous_when_dissolved_timestamp_seconds = 3; + + // Previous aging_since_timestamp_seconds. + optional uint64 previous_aging_since_timestamp_seconds = 4; + } } // The summary of the restore aging event. diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index b3ff5b30204..84ee2048126 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -3080,7 +3080,7 @@ pub struct AuditEvent { /// The timestamp of the event. #[prost(uint64, tag = "1")] pub timestamp_seconds: u64, - #[prost(oneof = "audit_event::Payload", tags = "2, 3")] + #[prost(oneof = "audit_event::Payload", tags = "2, 3, 4")] pub payload: ::core::option::Option, } /// Nested message and enum types in `AuditEvent`. @@ -3157,14 +3157,85 @@ pub mod audit_event { } #[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)] #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Message)] + pub struct NormalizeDissolveStateAndAge { + /// The neuron id whose dissolve state and age were normalized. + #[prost(uint64, optional, tag = "1")] + pub neuron_id: ::core::option::Option, + /// Which legacy case the neuron falls into. + #[prost(enumeration = "NeuronLegacyCase", tag = "2")] + pub neuron_legacy_case: i32, + /// Previous when_dissolved_timestamp_seconds if the neuron was dissolving or dissolved. + #[prost(uint64, optional, tag = "3")] + pub previous_when_dissolved_timestamp_seconds: ::core::option::Option, + /// Previous aging_since_timestamp_seconds. + #[prost(uint64, optional, tag = "4")] + pub previous_aging_since_timestamp_seconds: ::core::option::Option, + } + #[derive( + candid::CandidType, + candid::Deserialize, + serde::Serialize, + comparable::Comparable, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + ::prost::Enumeration, + )] + #[repr(i32)] + pub enum NeuronLegacyCase { + Unspecified = 0, + /// Neuron is dissolving or dissolved but with a non-zero age. + DissolvingOrDissolved = 1, + /// Neuron is dissolved with DissolveDelaySeconds(0). + Dissolved = 2, + /// Neuron has a None dissolve state. + NoneDissolveState = 3, + } + impl NeuronLegacyCase { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + NeuronLegacyCase::Unspecified => "NEURON_LEGACY_CASE_UNSPECIFIED", + NeuronLegacyCase::DissolvingOrDissolved => { + "NEURON_LEGACY_CASE_DISSOLVING_OR_DISSOLVED" + } + NeuronLegacyCase::Dissolved => "NEURON_LEGACY_CASE_DISSOLVED", + NeuronLegacyCase::NoneDissolveState => "NEURON_LEGACY_CASE_NONE_DISSOLVE_STATE", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "NEURON_LEGACY_CASE_UNSPECIFIED" => Some(Self::Unspecified), + "NEURON_LEGACY_CASE_DISSOLVING_OR_DISSOLVED" => Some(Self::DissolvingOrDissolved), + "NEURON_LEGACY_CASE_DISSOLVED" => Some(Self::Dissolved), + "NEURON_LEGACY_CASE_NONE_DISSOLVE_STATE" => Some(Self::NoneDissolveState), + _ => None, + } + } + } + #[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)] + #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum Payload { - /// Reset aging timestamps for + /// Reset aging timestamps (). #[prost(message, tag = "2")] ResetAging(ResetAging), - /// Restore aging timestamp that were incorrectly reset. + /// Restore aging timestamp that were incorrectly reset (). #[prost(message, tag = "3")] RestoreAging(RestoreAging), + /// Normalize neuron dissolve state and age () + #[prost(message, tag = "4")] + NormalizeDissolveStateAndAge(NormalizeDissolveStateAndAge), } } /// The summary of the restore aging event. diff --git a/rs/nns/governance/src/governance/merge_neurons.rs b/rs/nns/governance/src/governance/merge_neurons.rs index 6a3e61ce2a5..4c10f81c01f 100644 --- a/rs/nns/governance/src/governance/merge_neurons.rs +++ b/rs/nns/governance/src/governance/merge_neurons.rs @@ -1605,11 +1605,11 @@ mod tests { } else { panic!("Source neuron should not stop dissolving after merging"); } - let target_state_and_age = effect.target_neuron_dissolve_state_and_age; + let target_dissolve_state_and_age = effect.target_neuron_dissolve_state_and_age; if let DissolveStateAndAge::NotDissolving { dissolve_delay_seconds, aging_since_timestamp_seconds, - } = target_state_and_age + } = target_dissolve_state_and_age { prop_assert!(dissolve_delay_seconds >= target_dissolve_delay_seconds); // The resulted age should be between the source and target ages. diff --git a/rs/nns/governance/src/neuron/dissolve_state_and_age.rs b/rs/nns/governance/src/neuron/dissolve_state_and_age.rs index f4c2a43adef..70dea692209 100644 --- a/rs/nns/governance/src/neuron/dissolve_state_and_age.rs +++ b/rs/nns/governance/src/neuron/dissolve_state_and_age.rs @@ -1,4 +1,7 @@ -use crate::{governance::MAX_DISSOLVE_DELAY_SECONDS, pb::v1::NeuronState}; +use crate::{ + governance::MAX_DISSOLVE_DELAY_SECONDS, + pb::v1::{audit_event::NeuronLegacyCase, NeuronState}, +}; /// An enum to represent different combinations of a neurons dissolve_state and /// aging_since_timestamp_seconds. Currently, the back-and-forth conversions should make sure the @@ -38,6 +41,16 @@ pub enum DissolveStateAndAge { } impl DissolveStateAndAge { + /// Returns true if the neuron is in a legacy state. + pub fn is_legacy(self) -> bool { + match self { + Self::NotDissolving { .. } | Self::DissolvingOrDissolved { .. } => false, + Self::LegacyDissolvingOrDissolved { .. } + | Self::LegacyDissolved { .. } + | Self::LegacyNoneDissolveState { .. } => true, + } + } + /// Returns the current state given the current time. Mainly for differentiating between /// dissolving and dissolved neurons. pub fn current_state(self, now_seconds: u64) -> NeuronState { @@ -323,6 +336,41 @@ impl DissolveStateAndAge { }, } } + + /// Returns the normalized neuron dissolve state and age. If the neuron is in a legacy state, it + /// returns the valid state, as well as the audit event log for logging purposes. Otherwise it + /// returns the existing state and None. + pub fn normalize(self, created_timestamp_seconds: u64) -> Option<(Self, NeuronLegacyCase)> { + match self { + Self::NotDissolving { .. } | Self::DissolvingOrDissolved { .. } => None, + + Self::LegacyDissolvingOrDissolved { + when_dissolved_timestamp_seconds, + aging_since_timestamp_seconds: _, + } => Some(( + Self::DissolvingOrDissolved { + when_dissolved_timestamp_seconds, + }, + NeuronLegacyCase::DissolvingOrDissolved, + )), + + Self::LegacyDissolved { .. } => Some(( + Self::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: created_timestamp_seconds, + }, + NeuronLegacyCase::Dissolved, + )), + + // This case should be impossible, but treating it the same way as LegacyDissolved is + // also reasonable. + Self::LegacyNoneDissolveState { .. } => Some(( + Self::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: created_timestamp_seconds, + }, + NeuronLegacyCase::NoneDissolveState, + )), + } + } } #[cfg(test)] @@ -568,7 +616,7 @@ mod tests { #[test] fn test_increase_dissolve_delay_for_dissolved_neurons() { - for dissolved_state_and_age in [ + for dissolve_state_and_age in [ // Valid cases DissolveStateAndAge::DissolvingOrDissolved { when_dissolved_timestamp_seconds: NOW, @@ -624,7 +672,7 @@ mod tests { }, ] { assert_increase_dissolve_delay( - dissolved_state_and_age, + dissolve_state_and_age, 1, DissolveStateAndAge::NotDissolving { dissolve_delay_seconds: 1, @@ -634,7 +682,7 @@ mod tests { // Test that the dissolve delay is capped at MAX_DISSOLVE_DELAY_SECONDS. assert_increase_dissolve_delay( - dissolved_state_and_age, + dissolve_state_and_age, MAX_DISSOLVE_DELAY_SECONDS as u32 + 1000, DissolveStateAndAge::NotDissolving { dissolve_delay_seconds: MAX_DISSOLVE_DELAY_SECONDS, @@ -910,4 +958,75 @@ mod tests { } ); } + + #[test] + fn test_normalize() { + let created_timestamp_seconds = 123_456_789; + + { + let normal_cases = vec![ + DissolveStateAndAge::NotDissolving { + dissolve_delay_seconds: 1000, + aging_since_timestamp_seconds: NOW - 100, + }, + DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: NOW + 1000, + }, + DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: NOW - 1000, + }, + ]; + for normal_case in normal_cases { + assert_eq!(normal_case.normalize(created_timestamp_seconds), None); + } + } + + // In the legacy dissolving/dissolved case, we remove the age bonus. + let legacy_dissolving_or_dissolved = DissolveStateAndAge::LegacyDissolvingOrDissolved { + when_dissolved_timestamp_seconds: NOW + 1000, + aging_since_timestamp_seconds: NOW - 1000, + }; + let (normalized, legacy_case) = legacy_dissolving_or_dissolved + .normalize(created_timestamp_seconds) + .unwrap(); + assert_eq!( + normalized, + DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: NOW + 1000 + } + ); + assert_eq!(legacy_case, NeuronLegacyCase::DissolvingOrDissolved); + + // In the legacy dissolved case, we normalize it to dissolving/dissolved case while setting + // the dissolved timestamp to the creation timestamp. + let legacy_dissolved = DissolveStateAndAge::LegacyDissolved { + aging_since_timestamp_seconds: NOW - 1000, + }; + let (normalized, legacy_case) = legacy_dissolved + .normalize(created_timestamp_seconds) + .unwrap(); + assert_eq!( + normalized, + DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: created_timestamp_seconds + } + ); + assert_eq!(legacy_case, NeuronLegacyCase::Dissolved); + + // In the legacy none-dissolve state case, we normalize it to dissolving/dissolved case while setting + // the dissolved timestamp to the creation timestamp. + let legacy_none_dissolve_state = DissolveStateAndAge::LegacyNoneDissolveState { + aging_since_timestamp_seconds: NOW - 1000, + }; + let (normalized, legacy_case) = legacy_none_dissolve_state + .normalize(created_timestamp_seconds) + .unwrap(); + assert_eq!( + normalized, + DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: created_timestamp_seconds + } + ); + assert_eq!(legacy_case, NeuronLegacyCase::NoneDissolveState); + } } diff --git a/rs/nns/governance/src/neuron/mod.rs b/rs/nns/governance/src/neuron/mod.rs index c489c5179ea..be9a2ee69d8 100644 --- a/rs/nns/governance/src/neuron/mod.rs +++ b/rs/nns/governance/src/neuron/mod.rs @@ -356,8 +356,9 @@ impl Neuron { fn start_dissolving(&mut self, now_seconds: u64) -> Result<(), GovernanceError> { let dissolve_state_and_age = self.dissolve_state_and_age(); if let DissolveStateAndAge::NotDissolving { .. } = dissolve_state_and_age { - let new_disolved_state_and_age = dissolve_state_and_age.start_dissolving(now_seconds); - self.set_dissolve_state_and_age(new_disolved_state_and_age); + let new_disolved_dissolve_state_and_age = + dissolve_state_and_age.start_dissolving(now_seconds); + self.set_dissolve_state_and_age(new_disolved_dissolve_state_and_age); Ok(()) } else { Err(GovernanceError::new(ErrorType::RequiresNotDissolving)) @@ -369,11 +370,12 @@ impl Neuron { /// If the neuron is not dissolving, an error is returned. fn stop_dissolving(&mut self, now_seconds: u64) -> Result<(), GovernanceError> { let dissolve_state_and_age = self.dissolve_state_and_age(); - let new_disolved_state_and_age = dissolve_state_and_age.stop_dissolving(now_seconds); - if new_disolved_state_and_age == dissolve_state_and_age { + let new_disolved_dissolve_state_and_age = + dissolve_state_and_age.stop_dissolving(now_seconds); + if new_disolved_dissolve_state_and_age == dissolve_state_and_age { Err(GovernanceError::new(ErrorType::RequiresDissolving)) } else { - self.set_dissolve_state_and_age(new_disolved_state_and_age); + self.set_dissolve_state_and_age(new_disolved_dissolve_state_and_age); Ok(()) } } @@ -669,10 +671,10 @@ impl Neuron { self.cached_neuron_stake_e8s = new_stake_e8s; let new_aging_since_timestamp_seconds = now.saturating_sub(new_age_seconds); - let new_disolved_state_and_age = self + let new_disolved_dissolve_state_and_age = self .dissolve_state_and_age() .adjust_age(new_aging_since_timestamp_seconds); - self.set_dissolve_state_and_age(new_disolved_state_and_age); + self.set_dissolve_state_and_age(new_disolved_dissolve_state_and_age); } } diff --git a/rs/nns/governance/src/neuron/types.rs b/rs/nns/governance/src/neuron/types.rs index b3828712bc7..f19bca5645a 100644 --- a/rs/nns/governance/src/neuron/types.rs +++ b/rs/nns/governance/src/neuron/types.rs @@ -1,12 +1,18 @@ use crate::{ + audit_event::add_audit_event, + governance::LOG_PREFIX, neuron::dissolve_state_and_age::DissolveStateAndAge, neuron_store::NeuronStoreError, pb::v1::{ abridged_neuron::DissolveState as AbridgedNeuronDissolveState, + audit_event::{NormalizeDissolveStateAndAge, Payload}, neuron::{DissolveState as NeuronDissolveState, Followees}, - AbridgedNeuron, BallotInfo, KnownNeuronData, Neuron as NeuronProto, NeuronStakeTransfer, + AbridgedNeuron, AuditEvent, BallotInfo, KnownNeuronData, Neuron as NeuronProto, + NeuronStakeTransfer, }, }; +#[cfg(target_arch = "wasm32")] +use dfn_core::println; use ic_base_types::PrincipalId; use ic_nns_common::pb::v1::NeuronId; use icp_ledger::Subaccount; @@ -131,6 +137,59 @@ impl Neuron { pub fn set_dissolve_state_and_age(&mut self, dissolve_state_and_age: DissolveStateAndAge) { self.dissolve_state_and_age = dissolve_state_and_age; } + + /// Normalizes the dissolve state and age of the neuron. If any changes are made, stores an + /// audit event. + // TODO(NNS1-3068): clean up after the migration is performed. + pub fn normalize_dissolve_state_and_age(&mut self, now_seconds: u64) { + let previous_dissolve_state_and_age = self.dissolve_state_and_age(); + + let (normalized, legacy_case) = + match previous_dissolve_state_and_age.normalize(self.created_timestamp_seconds) { + Some((normalized, legacy_case)) => (normalized, legacy_case), + None => return, + }; + + println!( + "{}Neuron {} dissolve state and age got normaized from {:?} to {:?}", + LOG_PREFIX, + self.id().id, + previous_dissolve_state_and_age, + normalized + ); + + // Collect attributes for audit event logging. + let StoredDissolveStateAndAge { + dissolve_state: previous_dissolve_state, + aging_since_timestamp_seconds: previous_aging_since_timestamp_seconds, + } = StoredDissolveStateAndAge::from(self.dissolve_state_and_age()); + let previous_when_dissolved_timestamp_seconds = if let Some( + NeuronDissolveState::WhenDissolvedTimestampSeconds(when_dissolved_timestamp_seconds), + ) = previous_dissolve_state + { + Some(when_dissolved_timestamp_seconds) + } else { + None + }; + let previous_aging_since_timestamp_seconds = Some(previous_aging_since_timestamp_seconds); + let neuron_id = Some(self.id().id); + + // Apply the normalized dissolve state and age. + self.set_dissolve_state_and_age(normalized); + + // Log an audit event. + add_audit_event(AuditEvent { + timestamp_seconds: now_seconds, + payload: Some(Payload::NormalizeDissolveStateAndAge( + NormalizeDissolveStateAndAge { + neuron_id, + neuron_legacy_case: legacy_case as i32, + previous_when_dissolved_timestamp_seconds, + previous_aging_since_timestamp_seconds, + }, + )), + }); + } } impl From for NeuronProto { @@ -706,7 +765,7 @@ impl NeuronBuilder { /// An intermediate struct to represent a neuron's dissolve state and age on the storage layer. #[derive(Clone, Debug, PartialEq)] -pub(super) struct StoredDissolveStateAndAge { +pub(crate) struct StoredDissolveStateAndAge { pub dissolve_state: Option, pub aging_since_timestamp_seconds: u64, } @@ -868,13 +927,13 @@ mod tests { ), ]; - for (dissolve_state_and_age, stored_dissolved_state_and_age) in test_cases { + for (dissolve_state_and_age, stored_dissolve_state_and_age) in test_cases { assert_eq!( StoredDissolveStateAndAge::from(dissolve_state_and_age), - stored_dissolved_state_and_age.clone() + stored_dissolve_state_and_age.clone() ); assert_eq!( - DissolveStateAndAge::from(stored_dissolved_state_and_age), + DissolveStateAndAge::from(stored_dissolve_state_and_age), dissolve_state_and_age ); } diff --git a/rs/nns/governance/src/neuron_store.rs b/rs/nns/governance/src/neuron_store.rs index 28310f9bb57..4b5287dd367 100644 --- a/rs/nns/governance/src/neuron_store.rs +++ b/rs/nns/governance/src/neuron_store.rs @@ -332,14 +332,16 @@ impl NeuronStore { let clock = Box::new(IcClock::new()); let (neurons, topic_followee_index) = state; - Self { + let mut neuron_store = Self { heap_neurons: neurons .into_iter() .map(|(id, proto)| (id, Neuron::try_from(proto).unwrap())) .collect(), topic_followee_index: proto_to_heap_topic_followee_index(topic_followee_index), clock, - } + }; + neuron_store.normalize_neurons_dissolve_state_and_age(neuron_store.now()); + neuron_store } /// Takes the neuron store state which should be persisted through upgrades. @@ -859,6 +861,27 @@ impl NeuronStore { (active_neurons_in_stable_store, neuron_id_for_next_batch) } + /// Normalizes the dissolve state and age for all the neurons (heap and stable storage). + // TODO(NNS1-3068): clean up after the migration is performed. + fn normalize_neurons_dissolve_state_and_age(&mut self, now_seconds: u64) { + let mut neuron_ids_with_legacy_dissolve_state_and_age = + with_stable_neuron_store(|stable_neuron_store| { + stable_neuron_store.neuron_ids_with_legacy_dissolve_state_and_age() + }); + for neuron in self.heap_neurons.values() { + if neuron.dissolve_state_and_age().is_legacy() { + neuron_ids_with_legacy_dissolve_state_and_age.push(neuron.id()); + } + } + + for neuron_id in neuron_ids_with_legacy_dissolve_state_and_age { + self.with_neuron_mut(&neuron_id, |neuron| { + neuron.normalize_dissolve_state_and_age(now_seconds); + }) + .unwrap(); + } + } + // Census pub fn stable_neuron_store_len(&self) -> usize { diff --git a/rs/nns/governance/src/neuron_store/neuron_store_tests.rs b/rs/nns/governance/src/neuron_store/neuron_store_tests.rs index fbfbfa703d4..82ef8acabd9 100644 --- a/rs/nns/governance/src/neuron_store/neuron_store_tests.rs +++ b/rs/nns/governance/src/neuron_store/neuron_store_tests.rs @@ -1,13 +1,17 @@ use super::*; use crate::{ neuron::{DissolveStateAndAge, NeuronBuilder}, - pb::v1::neuron::Followees, - storage::with_stable_neuron_indexes, + pb::v1::{audit_event::Payload, neuron::Followees}, + storage::{with_audit_events_log, with_stable_neuron_indexes}, }; +use assert_matches::assert_matches; use ic_nervous_system_common::ONE_DAY_SECONDS; use ic_nns_constants::GOVERNANCE_CANISTER_ID; -use maplit::{btreemap, hashmap, hashset}; +use maplit::{btreemap, btreeset, hashmap, hashset}; use num_traits::bounds::LowerBounded; +use std::collections::BTreeSet; + +static CREATED_TIMESTAMP_SECONDS: u64 = 123_456_789; fn simple_neuron_builder(id: u64) -> NeuronBuilder { // Make sure different neurons have different accounts. @@ -24,7 +28,7 @@ fn simple_neuron_builder(id: u64) -> NeuronBuilder { dissolve_delay_seconds: 1, aging_since_timestamp_seconds: 0, }, - 123_456_789, + CREATED_TIMESTAMP_SECONDS, ) } @@ -633,3 +637,111 @@ fn test_get_neuron_ids_readable_by_caller() { hashset! {} ); } + +#[test] +fn test_normalize_neurons_dissolve_state_and_age() { + let mut neuron_store = NeuronStore::new(BTreeMap::new()); + let now = neuron_store.now(); + // We consider a neuron dissolved sufficiently long ago if it dissolved more than 14 days ago, + // and such neuron might be considered inactive. + let dissolved_at_sufficiently_long_ago = now - 15 * ONE_DAY_SECONDS; + + let not_dissolving = simple_neuron_builder(1) + .with_dissolve_state_and_age(DissolveStateAndAge::NotDissolving { + dissolve_delay_seconds: 1000, + aging_since_timestamp_seconds: now - 100, + }) + .build(); + let dissolving = simple_neuron_builder(2) + .with_dissolve_state_and_age(DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: now + 100, + }) + .build(); + let dissolved = simple_neuron_builder(3) + .with_dissolve_state_and_age(DissolveStateAndAge::DissolvingOrDissolved { + when_dissolved_timestamp_seconds: now - 100, + }) + .build(); + + let legacy_dissolving_with_age = simple_neuron_builder(4) + .with_dissolve_state_and_age(DissolveStateAndAge::LegacyDissolvingOrDissolved { + when_dissolved_timestamp_seconds: now + 2000, + aging_since_timestamp_seconds: now - 100, + }) + .build(); + let legacy_long_dissolved_with_age_funded = simple_neuron_builder(5) + .with_dissolve_state_and_age(DissolveStateAndAge::LegacyDissolvingOrDissolved { + when_dissolved_timestamp_seconds: dissolved_at_sufficiently_long_ago, + aging_since_timestamp_seconds: now - 100, + }) + .with_cached_neuron_stake_e8s(1) + .build(); + let legacy_long_dissolved_with_age_unfunded = simple_neuron_builder(6) + .with_dissolve_state_and_age(DissolveStateAndAge::LegacyDissolvingOrDissolved { + when_dissolved_timestamp_seconds: dissolved_at_sufficiently_long_ago, + aging_since_timestamp_seconds: now - 100, + }) + .with_cached_neuron_stake_e8s(0) + .build(); + let legacy_dissolved = simple_neuron_builder(7) + .with_dissolve_state_and_age(DissolveStateAndAge::LegacyDissolved { + aging_since_timestamp_seconds: now - 100, + }) + .build(); + let legacy_none_dissolve_state = simple_neuron_builder(8) + .with_dissolve_state_and_age(DissolveStateAndAge::LegacyNoneDissolveState { + aging_since_timestamp_seconds: now - 100, + }) + .build(); + + let neurons = vec![ + not_dissolving.clone(), + dissolving.clone(), + dissolved.clone(), + legacy_dissolving_with_age.clone(), + legacy_long_dissolved_with_age_funded.clone(), + legacy_long_dissolved_with_age_unfunded.clone(), + legacy_dissolved.clone(), + legacy_none_dissolve_state.clone(), + ]; + for neuron in neurons.iter() { + neuron_store.add_neuron(neuron.clone()).unwrap(); + } + + // Call the code under test. + neuron_store.normalize_neurons_dissolve_state_and_age(now); + + // Verify that the neurons have been normalized. + for neuron in neurons.iter() { + let dissove_dissolve_state_and_age = neuron_store + .with_neuron(&neuron.id(), |neuron| neuron.dissolve_state_and_age()) + .unwrap(); + assert_matches!( + dissove_dissolve_state_and_age, + DissolveStateAndAge::NotDissolving { .. } + | DissolveStateAndAge::DissolvingOrDissolved { .. } + ); + } + + let neurons_logged = with_audit_events_log(|log| { + log.iter() + .map(|audit_event| { + let payload = audit_event.payload.unwrap(); + match payload { + Payload::NormalizeDissolveStateAndAge(payload) => payload.neuron_id.unwrap(), + _ => panic!("Unexpected audit event"), + } + }) + .collect::>() + }); + assert_eq!( + neurons_logged, + btreeset! { + legacy_dissolving_with_age.id().id, + legacy_long_dissolved_with_age_funded.id().id, + legacy_long_dissolved_with_age_unfunded.id().id, + legacy_dissolved.id().id, + legacy_none_dissolve_state.id().id, + } + ); +} diff --git a/rs/nns/governance/src/storage/neurons.rs b/rs/nns/governance/src/storage/neurons.rs index 0da5b589b89..f2cf5fe07bc 100644 --- a/rs/nns/governance/src/storage/neurons.rs +++ b/rs/nns/governance/src/storage/neurons.rs @@ -1,8 +1,9 @@ use crate::{ - neuron::{DecomposedNeuron, Neuron}, + neuron::{DecomposedNeuron, DissolveStateAndAge, Neuron, StoredDissolveStateAndAge}, neuron_store::NeuronStoreError, pb::v1::{ - neuron::Followees, AbridgedNeuron, BallotInfo, KnownNeuronData, NeuronStakeTransfer, Topic, + neuron::DissolveState as NeuronDissolveState, neuron::Followees, AbridgedNeuron, + BallotInfo, KnownNeuronData, NeuronStakeTransfer, Topic, }, storage::validate_stable_btree_map, }; @@ -339,6 +340,21 @@ where validate_stable_btree_map(&self.transfer_map); } + /// Returns all neuron ids of the neurons with legacy dissolve state and age. + // TODO(NNS1-3068): clean up after the migration is performed. + pub fn neuron_ids_with_legacy_dissolve_state_and_age(&self) -> Vec { + self.main + .iter() + .filter_map(|(id, abridged_neuron)| { + if abridged_neuron.has_legacy_dissolve_state_and_age() { + Some(id) + } else { + None + } + }) + .collect() + } + /// Internal function to take what's in the main map and fill in the remaining data from /// the other stable storage maps. fn reconstitute_neuron(&self, neuron_id: NeuronId, main_neuron_part: AbridgedNeuron) -> Neuron { @@ -480,6 +496,21 @@ pub(crate) fn new_heap_based() -> StableNeuronStore { .build() } +impl AbridgedNeuron { + /// Returns true if the neuron has legacy dissolve state and age. We use AbridgedNeuron and + /// reconstruct the DissolveStateAndAge instead of converting it to Neuron, so that we can avoid + /// reading all the repeated fields from the stable storage. + // TODO(NNS1-3068): clean up after the migration is performed. + fn has_legacy_dissolve_state_and_age(&self) -> bool { + let stored_dissolve_state_and_age = StoredDissolveStateAndAge { + dissolve_state: self.dissolve_state.clone().map(NeuronDissolveState::from), + aging_since_timestamp_seconds: self.aging_since_timestamp_seconds, + }; + let dissolve_state_and_age = DissolveStateAndAge::from(stored_dissolve_state_and_age); + dissolve_state_and_age.is_legacy() + } +} + // impl Storable for $ProtoMessage // ======================================