Skip to content

Commit

Permalink
fix(nns): Fix for a particular locked neuron (#3311)
Browse files Browse the repository at this point in the history
A neuron was locked, which logged `Inflight commands: {
17912780790050115461: NeuronInFlightCommand { timestamp: 1728911670,
command: Some( Spawn( NeuronId { id: 17912780790050115461, }, ), ), },
}`

We identified the account as:
https://dashboard.internetcomputer.org/account/9765cc69bd6580023bee8fa8280fb630977b9f850da49d73cddcb99512272abb
And the neuron can be seen here:
https://dashboard.internetcomputer.org/neuron/17912780790050115461

As can be seen, no ICP was minted, but the neuron's cached stake was set
and maturity was cleared. The fix is to mint the ICP and unlock the
neuron.

Fix to prevent this from happening in the future is here:
#3226
  • Loading branch information
max-dfinity authored and daniel-wong-dfinity-org committed Jan 7, 2025
1 parent 75db1b8 commit d722dd8
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 1 deletion.
15 changes: 15 additions & 0 deletions rs/nns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ fn schedule_timers() {

// TODO(NNS1-3446): Delete. (This only needs to be run once, but can safely be run multiple times).
schedule_backfill_voting_power_refreshed_timestamps(Duration::from_secs(0));

// Schedule the fix for the locked neuron
schedule_locked_spawning_neuron_fix();
}

// Seeding interval seeks to find a balance between the need for rng secrecy, and
Expand Down Expand Up @@ -325,6 +328,18 @@ fn schedule_vote_processing() {
});
}

// TODO(NNS1-3526): Remove this method once it is released.
fn schedule_locked_spawning_neuron_fix() {
ic_cdk_timers::set_timer(Duration::from_secs(0), || {
spawn(async {
governance_mut()
.fix_locked_spawn_neuron()
.await
.expect("Failed to fix locked neuron");
});
});
}

struct CanisterEnv {
rng: Option<ChaCha20Rng>,
time_warp: GovTimeWarp,
Expand Down
61 changes: 61 additions & 0 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7060,6 +7060,67 @@ impl Governance {
self.heap_data.spawning_neurons = Some(false);
}

// TODO(NNS1-3526): Remove this method once it is released.
pub async fn fix_locked_spawn_neuron(&mut self) -> Result<(), GovernanceError> {
// ID of neuron that was locked when trying to spawn it due to ledger upgrade.
// Neuron's state was updated, but the ledger transaction did not finish.
const TARGETED_LOCK_TIMESTAMP: u64 = 1728911670;

let id = 17912780790050115461;
let neuron_id = NeuronId { id };

let now_seconds = self.env.now();

match self.heap_data.in_flight_commands.get(&id) {
None => {
return Ok(());
}
Some(existing_lock) => {
let NeuronInFlightCommand {
timestamp,
command: _,
} = existing_lock;

// We check the exact timestamp so that new locks couldn't trigger this condition
// which would allow that neuron to repeatedly mint under the right conditions.
if *timestamp != TARGETED_LOCK_TIMESTAMP {
return Ok(());
}
}
};

let (neuron_stake, subaccount) = self.with_neuron(&neuron_id, |neuron| {
let neuron_stake = neuron.cached_neuron_stake_e8s;
let subaccount = neuron.subaccount();
(neuron_stake, subaccount)
})?;

// Mint the ICP
match self
.ledger
.transfer_funds(
neuron_stake,
0, // Minting transfer don't pay a fee.
None,
neuron_subaccount(subaccount),
now_seconds,
)
.await
{
Ok(_) => {
self.heap_data.in_flight_commands.remove(&id);
Ok(())
}
Err(error) => Err(GovernanceError::new_with_message(
ErrorType::Unavailable,
format!(
"Error fixing locked neuron: {:?}. Ledger update failed with err: {:?}.",
neuron_id, error
),
)),
}
}

/// Return `true` if rewards should be distributed, `false` otherwise
fn should_distribute_rewards(&self) -> bool {
let latest_distribution_nominal_end_timestamp_seconds =
Expand Down
72 changes: 71 additions & 1 deletion rs/nns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ use std::{

#[cfg(feature = "tla")]
use ic_nns_governance::governance::tla::{check_traces as tla_check_traces, TLA_TRACES_LKEY};
use ic_nns_governance::storage::reset_stable_memory;
use ic_nns_governance::{
pb::v1::governance::{neuron_in_flight_command, NeuronInFlightCommand},
storage::reset_stable_memory,
};
#[cfg(feature = "tla")]
use tla_instrumentation_proc_macros::with_tla_trace_check;

Expand Down Expand Up @@ -14912,3 +14915,70 @@ impl CMC for StubCMC {
unimplemented!()
}
}

// TODO(NNS1-3526): Remove after deployed and confirmed fix
#[test]
fn test_locked_neuron_is_unlocked_and_icp_is_minted() {
let epoch = DEFAULT_TEST_START_TIMESTAMP_SECONDS + (20 * ONE_YEAR_SECONDS);
let neuron_id = 17912780790050115461;
let mut nns = NNSBuilder::new()
.set_start_time(epoch)
.add_neuron(
NeuronBuilder::new(neuron_id, 1_200_000, PrincipalId::new_user_test_id(42))
.set_dissolve_state(Some(DissolveState::WhenDissolvedTimestampSeconds(epoch))),
)
.create();

nns.governance.heap_data.in_flight_commands.insert(
neuron_id,
NeuronInFlightCommand {
timestamp: 1728911670,
command: Some(neuron_in_flight_command::Command::Spawn(NeuronId {
id: neuron_id,
})),
},
);

// B/c of how this test is setup, the neuron will have stake.
// We just want to make sure it can only incrase once.
assert_eq!(
nns.get_account_balance(nns.get_neuron_account_id(neuron_id)),
1_200_000
);

nns.governance
.fix_locked_spawn_neuron()
.now_or_never()
.unwrap()
.expect("Failed to fix locked spawn neuron");

assert_eq!(nns.governance.heap_data.in_flight_commands.len(), 0);
assert_eq!(
nns.get_account_balance(nns.get_neuron_account_id(neuron_id)),
1_200_000 * 2
);

// Nothing happens if you call it again with a differnt lock time.
nns.governance.heap_data.in_flight_commands.insert(
neuron_id,
NeuronInFlightCommand {
timestamp: 1728911671,
command: Some(neuron_in_flight_command::Command::Spawn(NeuronId {
id: neuron_id,
})),
},
);
nns.governance
.fix_locked_spawn_neuron()
.now_or_never()
.unwrap()
.expect("Failed to fix locked spawn neuron");

// Lock is not cleared b/c we didn't target that lock
assert_eq!(nns.governance.heap_data.in_flight_commands.len(), 1);
// Balance doesn't change this time.
assert_eq!(
nns.get_account_balance(nns.get_neuron_account_id(neuron_id)),
1_200_000 * 2
);
}

0 comments on commit d722dd8

Please sign in to comment.