From d19a1b45ee58c36e7e5555212c927e6d336ca7a7 Mon Sep 17 00:00:00 2001 From: max-dfinity <100170574+max-dfinity@users.noreply.github.com> Date: Thu, 2 Jan 2025 11:15:18 -0800 Subject: [PATCH] fix(nns): Revert spawn state when ledger unavailable and drop lock (#3226) This removes an edge case where a neuron can become locked when the ledger is unavailable during a neuron operation (such as during an upgrade). If the neuron happens to spawn during a ledger upgrade, the neuron will become locked, and therefore stuck. --- rs/nns/governance/src/governance.rs | 38 +++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index 23ed8435e91..28b809f7cd5 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -6947,16 +6947,17 @@ impl Governance { LOG_PREFIX, neuron ); - let staked_neuron_clone = self + let (staked_neuron_clone, original_spawn_at_timestamp_seconds) = self .with_neuron_mut(&neuron_id, |neuron| { // Reset the neuron's maturity and set that it's spawning before we actually mint // the stake. This is conservative to prevent a neuron having _both_ the stake and // the maturity at any point in time. + let original_spawn_ts = neuron.spawn_at_timestamp_seconds; neuron.maturity_e8s_equivalent = 0; neuron.spawn_at_timestamp_seconds = None; neuron.cached_neuron_stake_e8s = neuron_stake; - neuron.clone() + (neuron.clone(), original_spawn_ts) }) .unwrap(); @@ -6986,18 +6987,35 @@ impl Governance { ); } Err(error) => { - // Retain the neuron lock, the neuron won't be able to undergo stake changing - // operations until this is fixed. - // This is different from what we do in most places because we usually rely - // on trapping to retain the lock, but we can't do that here since we're not - // working on a single neuron. - lock.retain(); println!( - "{}Error spawning neuron: {:?}. Ledger update failed with err: {:?}.", + "{}Error spawning neuron: {:?}. Ledger update failed with err: {:?}. \ + Reverting state, so another attempt can be made.", LOG_PREFIX, neuron_id, error, - ); + ); + match self.with_neuron_mut(&neuron_id, |neuron| { + neuron.maturity_e8s_equivalent = neuron_stake; + neuron.cached_neuron_stake_e8s = 0; + neuron.spawn_at_timestamp_seconds = + original_spawn_at_timestamp_seconds; + }) { + Ok(_) => (), + Err(e) => { + println!( + "{} Error reverting state for neuron: {:?}. Retaining lock: {}", + LOG_PREFIX, + neuron_id, + e + ); + // Retain the neuron lock, the neuron won't be able to undergo stake changing + // operations until this is fixed. + // This is different from what we do in most places because we usually rely + // on trapping to retain the lock, but we can't do that here since we're not + // working on a single neuron. + lock.retain(); + } + }; } }; }