Skip to content

Commit

Permalink
Move spawn_neurons to a one-minute timer
Browse files Browse the repository at this point in the history
  • Loading branch information
jasonz-dfinity committed Dec 3, 2024
1 parent bc3558e commit 04e4760
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
11 changes: 11 additions & 0 deletions rs/nns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ fn set_governance(gov: Governance) {
fn schedule_timers() {
schedule_seeding(Duration::from_nanos(0));
schedule_adjust_neurons_storage(Duration::from_nanos(0), NeuronIdProto { id: 0 });
schedule_spawn_neurons();
}

// Seeding interval seeks to find a balance between the need for rng secrecy, and
Expand Down Expand Up @@ -216,6 +217,16 @@ fn schedule_adjust_neurons_storage(delay: Duration, start_neuron_id: NeuronIdPro
});
}

const SPAWN_NEURONS_INTERVAL: Duration = Duration::from_secs(60);

fn schedule_spawn_neurons() {
ic_cdk_timers::set_timer_interval(SPAWN_NEURONS_INTERVAL, || {
spawn(async {
governance_mut().maybe_spawn_neurons().await;
});
});
}

struct CanisterEnv {
rng: Option<ChaCha20Rng>,
time_warp: GovTimeWarp,
Expand Down
5 changes: 1 addition & 4 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6548,9 +6548,6 @@ impl Governance {
// Try to update maturity modulation (once per day).
} else if self.should_update_maturity_modulation() {
self.update_maturity_modulation().await;
// Try to spawn neurons (potentially multiple times per day).
} else if self.can_spawn_neurons() {
self.spawn_neurons().await;
} else {
// This is the lowest-priority async task. All other tasks should have their own
// `else if`, like the ones above.
Expand Down Expand Up @@ -6692,7 +6689,7 @@ impl Governance {
/// This means that programming in this method needs to be extra-defensive on the handling of results so that
/// we're sure not to trap after we've acquired the global lock and made an async call, as otherwise the global
/// lock will be permanently held and no spawning will occur until a upgrade to fix it is made.
async fn spawn_neurons(&mut self) {
pub async fn maybe_spawn_neurons(&mut self) {
if !self.can_spawn_neurons() {
return;
}
Expand Down
17 changes: 10 additions & 7 deletions rs/nns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5737,7 +5737,7 @@ fn test_seed_neuron_split() {

// Spawn neurons has the least priority in the periodic tasks, so we need to run
// them often enough to make sure it happens.
fn run_periodic_tasks_on_governance_often_enough_to_spawn(gov: &mut Governance) {
fn run_periodic_tasks_often_enough_to_update_maturity_modulation(gov: &mut Governance) {
for _i in 0..5 {
gov.run_periodic_tasks().now_or_never();
}
Expand Down Expand Up @@ -5766,6 +5766,7 @@ fn test_neuron_spawn() {
from,
nonce,
);
run_periodic_tasks_often_enough_to_update_maturity_modulation(&mut gov);

let now = driver.now();
assert_eq!(
Expand Down Expand Up @@ -5898,13 +5899,13 @@ fn test_neuron_spawn() {
let creation_timestamp = driver.now();

// Running periodic tasks shouldn't cause the ICP to be minted.
run_periodic_tasks_on_governance_often_enough_to_spawn(&mut gov);
gov.maybe_spawn_neurons().now_or_never().unwrap();
driver.assert_num_neuron_accounts_exist(1);

// Advance the time by one week, should cause the neuron's ICP
// to be minted.
driver.advance_time_by(7 * 86400);
run_periodic_tasks_on_governance_often_enough_to_spawn(&mut gov);
gov.maybe_spawn_neurons().now_or_never().unwrap();
driver.assert_num_neuron_accounts_exist(2);

let child_neuron = gov
Expand Down Expand Up @@ -5953,6 +5954,7 @@ fn test_neuron_spawn_with_subaccount() {
from,
nonce,
);
run_periodic_tasks_often_enough_to_update_maturity_modulation(&mut gov);

let now = driver.now();
assert_eq!(
Expand Down Expand Up @@ -6035,7 +6037,7 @@ fn test_neuron_spawn_with_subaccount() {
driver.assert_num_neuron_accounts_exist(1);

// Running periodic tasks shouldn't cause the ICP to be minted.
run_periodic_tasks_on_governance_often_enough_to_spawn(&mut gov);
gov.maybe_spawn_neurons().now_or_never().unwrap();
driver.assert_num_neuron_accounts_exist(1);

let parent_neuron = gov
Expand All @@ -6047,7 +6049,7 @@ fn test_neuron_spawn_with_subaccount() {
// Advance the time by one week, should cause the neuron's ICP
// to be minted.
driver.advance_time_by(7 * 86400);
run_periodic_tasks_on_governance_often_enough_to_spawn(&mut gov);
gov.maybe_spawn_neurons().now_or_never().unwrap();
driver.assert_num_neuron_accounts_exist(2);

let child_neuron = gov
Expand Down Expand Up @@ -6132,6 +6134,7 @@ fn assert_neuron_spawn_partial(
from,
nonce,
);
run_periodic_tasks_often_enough_to_update_maturity_modulation(&mut gov);

let neuron = gov
.with_neuron(&id, |neuron| neuron.clone())
Expand Down Expand Up @@ -6193,7 +6196,7 @@ fn assert_neuron_spawn_partial(
.expect("The parent neuron is missing");

// Running periodic tasks shouldn't cause the ICP to be minted.
run_periodic_tasks_on_governance_often_enough_to_spawn(&mut gov);
gov.maybe_spawn_neurons().now_or_never().unwrap();
driver.assert_num_neuron_accounts_exist(1);

// Some maturity should be remaining on the parent neuron.
Expand All @@ -6205,7 +6208,7 @@ fn assert_neuron_spawn_partial(
// Advance the time by one week, should cause the neuron's ICP
// to be minted.
driver.advance_time_by(7 * 86400);
run_periodic_tasks_on_governance_often_enough_to_spawn(&mut gov);
gov.maybe_spawn_neurons().now_or_never().unwrap();
driver.assert_num_neuron_accounts_exist(2);

let child_neuron = gov
Expand Down

0 comments on commit 04e4760

Please sign in to comment.