Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nns): Move spawn_neurons to a one-minute timer #2934

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions rs/nns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
jasonz-dfinity marked this conversation as resolved.
Show resolved Hide resolved
}

// Seeding interval seeks to find a balance between the need for rng secrecy, and
Expand Down Expand Up @@ -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<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 @@ -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.
Expand Down Expand Up @@ -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;
}
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
Loading