From 45dfc1f981ae75e1fc61f171cd02249b5f344df2 Mon Sep 17 00:00:00 2001
From: Max Summe <maximilian.summe@dfinity.org>
Date: Fri, 25 Oct 2024 13:29:48 -0700
Subject: [PATCH] one other edge case

---
 rs/nns/governance/src/governance.rs           | 17 +++++++++---
 .../src/neuron/dissolve_state_and_age.rs      | 27 ++++++++++---------
 rs/nns/governance/src/neuron_store.rs         | 23 +---------------
 3 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs
index 3f6ab32fa98..ab5aaebc3a1 100644
--- a/rs/nns/governance/src/governance.rs
+++ b/rs/nns/governance/src/governance.rs
@@ -4213,15 +4213,24 @@ impl Governance {
                     reward_to_neuron.dissolve_delay_seconds,
                     MAX_DISSOLVE_DELAY_SECONDS,
                 );
+
+                let dissolve_state_and_age = if dissolve_delay_seconds > 0 {
+                    DissolveStateAndAge::NotDissolving {
+                        dissolve_delay_seconds,
+                        aging_since_timestamp_seconds: now,
+                    }
+                } else {
+                    DissolveStateAndAge::DissolvingOrDissolved {
+                        when_dissolved_timestamp_seconds: now,
+                    }
+                };
+
                 // Transfer successful.
                 let neuron = NeuronBuilder::new(
                     nid,
                     to_subaccount,
                     *np_principal,
-                    DissolveStateAndAge::NotDissolving {
-                        dissolve_delay_seconds,
-                        aging_since_timestamp_seconds: now,
-                    },
+                    dissolve_state_and_age,
                     now,
                 )
                 .with_followees(self.heap_data.default_followees.clone())
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 ea5c52559eb..cd46233edcc 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,6 @@
-use crate::{governance::MAX_DISSOLVE_DELAY_SECONDS, pb::v1::NeuronState};
+use crate::{
+    governance::MAX_DISSOLVE_DELAY_SECONDS, neuron::StoredDissolveStateAndAge, pb::v1::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
@@ -21,17 +23,18 @@ pub enum DissolveStateAndAge {
 
 impl DissolveStateAndAge {
     pub fn validate(self) -> Result<Self, String> {
-        match self {
-            DissolveStateAndAge::NotDissolving {
-                dissolve_delay_seconds,
-                aging_since_timestamp_seconds: _,
-            } => {
-                if dissolve_delay_seconds == 0 {
-                    return Err("Dissolve delay must be greater than 0".to_string());
-                }
-            }
-            DissolveStateAndAge::DissolvingOrDissolved { .. } => {}
-        };
+        let original = self;
+        let stored_dissolve_state_and_age = StoredDissolveStateAndAge::from(self);
+
+        let validated_dissolve_state_and_age = Self::try_from(stored_dissolve_state_and_age)
+            .map_err(|e| format!("Invalid dissolve state and age: {}", e))?;
+
+        if validated_dissolve_state_and_age != original {
+            return Err( format!(
+                    "Dissolve state and age is not valid, as it does not serialize/deserialize symmetrically. In: {:?}, Out: {:?}",
+                    original, validated_dissolve_state_and_age
+                ));
+        }
 
         Ok(self)
     }
diff --git a/rs/nns/governance/src/neuron_store.rs b/rs/nns/governance/src/neuron_store.rs
index bec0aad8fb1..5eedc59ea9f 100644
--- a/rs/nns/governance/src/neuron_store.rs
+++ b/rs/nns/governance/src/neuron_store.rs
@@ -34,7 +34,6 @@ use std::{
 };
 
 pub mod metrics;
-use crate::neuron::{DissolveStateAndAge, StoredDissolveStateAndAge};
 pub(crate) use metrics::NeuronMetrics;
 
 #[derive(Eq, PartialEq, Debug)]
@@ -476,31 +475,11 @@ impl NeuronStore {
     }
 
     fn validate_neuron(&self, neuron: &Neuron) -> Result<(), NeuronStoreError> {
-        let dissolve_state_and_age = neuron.dissolve_state_and_age();
-
-        let stored_dissolve_state_and_age = StoredDissolveStateAndAge::from(dissolve_state_and_age);
-
-        let validated_dissolve_state_and_age =
-            DissolveStateAndAge::try_from(stored_dissolve_state_and_age).map_err(|e| {
-                NeuronStoreError::InvalidData {
-                    reason: format!("Invalid dissolve state and age: {}", e),
-                }
-            })?;
-
-        if validated_dissolve_state_and_age != dissolve_state_and_age {
-            return Err(NeuronStoreError::InvalidData {
-                reason: format!(
-                    "Dissolve state and age is not valid, as it reads and writes in a different shape. In: {:?}, Out: {:?}",
-                    dissolve_state_and_age, validated_dissolve_state_and_age
-                ),
-            });
-        }
-
         neuron
             .dissolve_state_and_age()
             .validate()
             .map_err(|reason| NeuronStoreError::InvalidData {
-                reason: format!("Neuron is invalid: {}", reason),
+                reason: format!("Neuron cannot be saved: {}", reason),
             })?;
 
         Ok(())