feat(nns): NNS1-2931 Define an enum DissolveStateAndAge to represent …

…valid and invalid combinations of dissolve state and age

rs/nns/governance/src/neuron.rs → rs/nns/governance/src/neuron/mod.rs

@@ -4,6 +4,7 @@ use crate::{
         DEPRECATED_TOPICS, LOG_PREFIX, MAX_DISSOLVE_DELAY_SECONDS, MAX_NEURON_AGE_FOR_AGE_BONUS,
         MAX_NEURON_RECENT_BALLOTS, MAX_NUM_HOT_KEYS_PER_NEURON,
     },
+    neuron::types::{DissolveStateAndAge, StoredDissolvedStateAndAge},
     pb::v1::{
         governance_error::ErrorType, manage_neuron, neuron::DissolveState, Ballot, BallotInfo,
         GovernanceError, Neuron, NeuronInfo, NeuronState, NeuronType, Topic, Vote,
@@ -20,6 +21,8 @@ use std::{
     ops::RangeBounds,
 };
 
+pub mod types;
+
 impl Neuron {
     // --- Utility methods on neurons: mostly not for public consumption.
 
@@ -836,6 +839,21 @@ impl Neuron {
         }
         self
     }
+
+    /// Returns an enum representing the dissolve state and age of a neuron.
+    pub fn dissolve_state_and_age(&self) -> DissolveStateAndAge {
+        DissolveStateAndAge::from(StoredDissolvedStateAndAge {
+            dissolve_state: self.dissolve_state.clone(),
+            aging_since_timestamp_seconds: self.aging_since_timestamp_seconds,
+        })
+    }
+
+    /// Sets a neuron's dissolve state and age.
+    pub fn set_dissolve_state_and_age(&mut self, state_and_age: DissolveStateAndAge) {
+        let stored = StoredDissolvedStateAndAge::from(state_and_age);
+        self.dissolve_state = stored.dissolve_state;
+        self.aging_since_timestamp_seconds = stored.aging_since_timestamp_seconds;
+    }
 }
 
 /// Convert a RangeBounds<NeuronId> to RangeBounds<u64> which is useful for methods
@@ -866,8 +884,8 @@ impl NeuronInfo {
 
 #[cfg(test)]
 mod tests {
-    use crate::pb::v1::{neuron::DissolveState, Neuron, NeuronState};
-    use ic_nervous_system_common::{E8, SECONDS_PER_DAY};
+    use super::*;
+    use ic_nervous_system_common::E8;
 
     const NOW: u64 = 123_456_789;
 
@@ -1167,4 +1185,35 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_neuron_state_and_age() {
+        let mut neuron = Neuron {
+            aging_since_timestamp_seconds: 123456789,
+            dissolve_state: Some(DissolveState::DissolveDelaySeconds(23456)),
+            ..Default::default()
+        };
+        assert_eq!(
+            neuron.dissolve_state_and_age(),
+            DissolveStateAndAge::NotDissolving {
+                dissolve_delay_seconds: 23456,
+                aging_since_timestamp_seconds: 123456789
+            }
+        );
+
+        neuron.set_dissolve_state_and_age(DissolveStateAndAge::DissolvingOrDissolved {
+            when_dissolved_timestamp_seconds: 234567890,
+        });
+        assert_eq!(
+            neuron.dissolve_state_and_age(),
+            DissolveStateAndAge::DissolvingOrDissolved {
+                when_dissolved_timestamp_seconds: 234567890,
+            }
+        );
+        assert_eq!(
+            neuron.dissolve_state,
+            Some(DissolveState::WhenDissolvedTimestampSeconds(234567890))
+        );
+        assert_eq!(neuron.aging_since_timestamp_seconds, u64::MAX);
+    }
 }

rs/nns/governance/src/neuron/types.rs

@@ -0,0 +1,210 @@
+use crate::pb::v1::neuron::DissolveState;
+
+/// 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
+/// legacy states remain the same unless some operations performed on the neuron makes the state/age
+/// changes. After we make sure all neuron mutations or creations must mutate states to valid ones
+/// and the invalid states have been migrated to valid ones on the mainnet, we can panic in
+/// conversion when invalid states are encountered. 2 of the legacy states
+/// (LegacyDissolvingOrDissolved and LegacyDissolved) are the cases we already know to be existing
+/// on the mainnet.
+#[derive(Clone, Debug, PartialEq)]
+pub enum DissolveStateAndAge {
+    /// A non-dissolving neuron has a dissolve delay and an aging since timestamp.
+    NotDissolving {
+        dissolve_delay_seconds: u64,
+        aging_since_timestamp_seconds: u64,
+    },
+    /// A dissolving or dissolved neuron has a dissolved timestamp and no aging since timestamp.
+    DissolvingOrDissolved {
+        when_dissolved_timestamp_seconds: u64,
+    },
+    /// We used to allow neurons to have age when they were dissolving or dissolved. This should be
+    /// mapped to DissolvingOrDissolved { when_dissolved_timestamp_seconds } and its aging singe
+    /// timestamp removed.
+    LegacyDissolvingOrDissolved {
+        when_dissolved_timestamp_seconds: u64,
+        aging_since_timestamp_seconds: u64,
+    },
+    /// When claiming a neuron, the dissolve delay is set to 0 while the neuron is considered
+    /// dissolved. Its aging_since_timestamp_seconds is set to the neuron was claimed. This state
+    /// should be mapped to DissolvingOrDissolved { when_dissolved_timestamp_seconds:
+    /// aging_since_timestamp_seconds }.
+    LegacyDissolved { aging_since_timestamp_seconds: u64 },
+
+    /// The dissolve state is None, which should have never existed, but we keep the current
+    /// behavior of considering it as a dissolved neuron.
+    LegacyNoneDissolveState { aging_since_timestamp_seconds: u64 },
+}
+
+/// An intermediate struct to represent a neuron's dissolve state and age on the storage layer.
+#[derive(Clone, Debug, PartialEq)]
+pub(super) struct StoredDissolvedStateAndAge {
+    pub dissolve_state: Option<DissolveState>,
+    pub aging_since_timestamp_seconds: u64,
+}
+
+impl From<DissolveStateAndAge> for StoredDissolvedStateAndAge {
+    fn from(dissolve_state_and_age: DissolveStateAndAge) -> Self {
+        match dissolve_state_and_age {
+            DissolveStateAndAge::NotDissolving {
+                dissolve_delay_seconds,
+                aging_since_timestamp_seconds,
+            } => StoredDissolvedStateAndAge {
+                dissolve_state: Some(DissolveState::DissolveDelaySeconds(dissolve_delay_seconds)),
+                aging_since_timestamp_seconds,
+            },
+            DissolveStateAndAge::DissolvingOrDissolved {
+                when_dissolved_timestamp_seconds,
+            } => StoredDissolvedStateAndAge {
+                dissolve_state: Some(DissolveState::WhenDissolvedTimestampSeconds(
+                    when_dissolved_timestamp_seconds,
+                )),
+                aging_since_timestamp_seconds: u64::MAX,
+            },
+            DissolveStateAndAge::LegacyDissolvingOrDissolved {
+                when_dissolved_timestamp_seconds,
+                aging_since_timestamp_seconds,
+            } => StoredDissolvedStateAndAge {
+                dissolve_state: Some(DissolveState::WhenDissolvedTimestampSeconds(
+                    when_dissolved_timestamp_seconds,
+                )),
+                aging_since_timestamp_seconds,
+            },
+            DissolveStateAndAge::LegacyDissolved {
+                aging_since_timestamp_seconds,
+            } => StoredDissolvedStateAndAge {
+                dissolve_state: Some(DissolveState::DissolveDelaySeconds(0)),
+                aging_since_timestamp_seconds,
+            },
+            DissolveStateAndAge::LegacyNoneDissolveState {
+                aging_since_timestamp_seconds,
+            } => StoredDissolvedStateAndAge {
+                dissolve_state: None,
+                aging_since_timestamp_seconds,
+            },
+        }
+    }
+}
+
+impl From<StoredDissolvedStateAndAge> for DissolveStateAndAge {
+    fn from(stored: StoredDissolvedStateAndAge) -> Self {
+        match (stored.dissolve_state, stored.aging_since_timestamp_seconds) {
+            (None, aging_since_timestamp_seconds) => DissolveStateAndAge::LegacyNoneDissolveState {
+                aging_since_timestamp_seconds,
+            },
+            (Some(DissolveState::DissolveDelaySeconds(0)), aging_since_timestamp_seconds) => {
+                DissolveStateAndAge::LegacyDissolved {
+                    aging_since_timestamp_seconds,
+                }
+            }
+            (
+                Some(DissolveState::DissolveDelaySeconds(dissolve_delay_seconds)),
+                // TODO(NNS1-2951): have a stricter guarantee about the aging_since_timestamp_seconds.
+                aging_since_timestamp_seconds,
+            ) => DissolveStateAndAge::NotDissolving {
+                dissolve_delay_seconds,
+                aging_since_timestamp_seconds,
+            },
+            (
+                Some(DissolveState::WhenDissolvedTimestampSeconds(
+                    when_dissolved_timestamp_seconds,
+                )),
+                u64::MAX,
+            ) => DissolveStateAndAge::DissolvingOrDissolved {
+                when_dissolved_timestamp_seconds,
+            },
+            (
+                Some(DissolveState::WhenDissolvedTimestampSeconds(
+                    when_dissolved_timestamp_seconds,
+                )),
+                aging_since_timestamp_seconds,
+            ) => DissolveStateAndAge::LegacyDissolvingOrDissolved {
+                when_dissolved_timestamp_seconds,
+                aging_since_timestamp_seconds,
+            },
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_dissolve_state_and_age_conversion() {
+        let test_cases = vec![
+            (
+                DissolveStateAndAge::NotDissolving {
+                    dissolve_delay_seconds: 100,
+                    aging_since_timestamp_seconds: 200,
+                },
+                StoredDissolvedStateAndAge {
+                    dissolve_state: Some(DissolveState::DissolveDelaySeconds(100)),
+                    aging_since_timestamp_seconds: 200,
+                },
+            ),
+            // TODO(NNS1-2951): have a more strict guarantee about the
+            // aging_since_timestamp_seconds. This case is theoretically possible, while we should
+            // never have such a neuron. The aging_since_timestamp_seconds should be in the past.
+            (
+                DissolveStateAndAge::NotDissolving {
+                    dissolve_delay_seconds: 100,
+                    aging_since_timestamp_seconds: u64::MAX,
+                },
+                StoredDissolvedStateAndAge {
+                    dissolve_state: Some(DissolveState::DissolveDelaySeconds(100)),
+                    aging_since_timestamp_seconds: u64::MAX,
+                },
+            ),
+            (
+                DissolveStateAndAge::DissolvingOrDissolved {
+                    when_dissolved_timestamp_seconds: 300,
+                },
+                StoredDissolvedStateAndAge {
+                    dissolve_state: Some(DissolveState::WhenDissolvedTimestampSeconds(300)),
+                    aging_since_timestamp_seconds: u64::MAX,
+                },
+            ),
+            (
+                DissolveStateAndAge::LegacyDissolvingOrDissolved {
+                    when_dissolved_timestamp_seconds: 400,
+                    aging_since_timestamp_seconds: 500,
+                },
+                StoredDissolvedStateAndAge {
+                    dissolve_state: Some(DissolveState::WhenDissolvedTimestampSeconds(400)),
+                    aging_since_timestamp_seconds: 500,
+                },
+            ),
+            (
+                DissolveStateAndAge::LegacyDissolved {
+                    aging_since_timestamp_seconds: 600,
+                },
+                StoredDissolvedStateAndAge {
+                    dissolve_state: Some(DissolveState::DissolveDelaySeconds(0)),
+                    aging_since_timestamp_seconds: 600,
+                },
+            ),
+            (
+                DissolveStateAndAge::LegacyNoneDissolveState {
+                    aging_since_timestamp_seconds: 700,
+                },
+                StoredDissolvedStateAndAge {
+                    dissolve_state: None,
+                    aging_since_timestamp_seconds: 700,
+                },
+            ),
+        ];
+
+        for (dissolve_state_and_age, stored_dissolved_state_and_age) in test_cases {
+            assert_eq!(
+                StoredDissolvedStateAndAge::from(dissolve_state_and_age.clone()),
+                stored_dissolved_state_and_age.clone()
+            );
+            assert_eq!(
+                DissolveStateAndAge::from(stored_dissolved_state_and_age),
+                dissolve_state_and_age
+            );
+        }
+    }
+}