Merge branch 'jason/misc-no-replace-neuron-subaccount' into 'master'

test(nns): NNS1-3147 Do not allow replacing neuron subaccount in testing

Replacing neuron subaccount is fundamentally incorrect in NNS Governance: (1) it breaks the internal neuron index (2) it can cause losing of funds if such neuron is refreshed after getting a different subaccount. Note that there is no problem with production code, only the governance test canister where the update_neuron method exists. 

See merge request dfinity-lab/public/ic!19931

rs/nns/governance/src/governance.rs

@@ -1928,15 +1928,21 @@ impl Governance {
     ///
     /// Preconditions:
     /// - the given `neuron` already exists in `self.neuron_store.neurons`
-    /// - the controller principal is self-authenticating
     #[cfg(feature = "test")]
     pub fn update_neuron(&mut self, neuron: NeuronProto) -> Result<(), GovernanceError> {
-        let neuron_id = neuron.id.expect("Neuron must have a NeuronId");
-        // Must clobber an existing neuron.
-        self.with_neuron_mut(&neuron_id, |old_neuron| {
-            // Converting from API type to internal type.
-            *old_neuron = Neuron::try_from(neuron).unwrap();
-        })
+        // Converting from API type to internal type.
+        let new_neuron = Neuron::try_from(neuron).expect("Neuron must be valid");
+
+        self.with_neuron_mut(&new_neuron.id(), |old_neuron| {
+            if new_neuron.subaccount() != old_neuron.subaccount() {
+                return Err(GovernanceError::new_with_message(
+                    ErrorType::PreconditionFailed,
+                    "Cannot change the subaccount of a neuron".to_string(),
+                ));
+            }
+            *old_neuron = new_neuron;
+            Ok(())
+        })?
     }
 
     /// Add a neuron to the list of neurons.