From 71e9ecbfbe90a263e7dabd685975c29a44db6e5d Mon Sep 17 00:00:00 2001 From: Jason Zhu Date: Tue, 3 Dec 2024 00:24:03 +0000 Subject: [PATCH] Move spawn_neurons to a one-minute timer --- rs/nns/governance/canister/canister.rs | 11 +++++++++++ rs/nns/governance/src/governance.rs | 5 +---- rs/nns/governance/tests/governance.rs | 17 ++++++++++------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/rs/nns/governance/canister/canister.rs b/rs/nns/governance/canister/canister.rs index 4e4d88a1e98..9cf20585bcb 100644 --- a/rs/nns/governance/canister/canister.rs +++ b/rs/nns/governance/canister/canister.rs @@ -165,6 +165,7 @@ fn schedule_timers() { schedule_seeding(Duration::from_nanos(0)); schedule_adjust_neurons_storage(Duration::from_nanos(0), NeuronIdProto { id: 0 }); schedule_prune_following(Duration::from_secs(0)); + schedule_spawn_neurons(); } // Seeding interval seeks to find a balance between the need for rng secrecy, and @@ -268,6 +269,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, time_warp: GovTimeWarp, diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index fb8fed7e84d..809f901bf08 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -6558,9 +6558,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. @@ -6702,7 +6699,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; } diff --git a/rs/nns/governance/tests/governance.rs b/rs/nns/governance/tests/governance.rs index 86023027262..d10fe64cc6f 100644 --- a/rs/nns/governance/tests/governance.rs +++ b/rs/nns/governance/tests/governance.rs @@ -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(); } @@ -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!( @@ -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 @@ -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!( @@ -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 @@ -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 @@ -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()) @@ -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. @@ -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