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(sns): Add SnsRoot.reset_timers #2216

Merged
merged 16 commits into from
Oct 25, 2024
Merged
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ message Decimal {
message ResetTimersRequest {}
message ResetTimersResponse {}

// TODO[NNS1-3420] This type can be refined into different internal API types, depending on
// TODO[NNS1-3420] the needs of a particular canister. The fields of this type represent
// TODO[NNS1-3420] over-approximation of the fields that might be relevant for observing and
// TODO[NNS1-3420] managing timers in nervous system-related canisters.
message Timers {
// Indicates whether this canister (still) requires (timer-based) periodic tasks.
//
Expand Down
4 changes: 4 additions & 0 deletions rs/nervous_system/proto/src/gen/ic_nervous_system.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ pub struct ResetTimersRequest {}
::prost::Message,
)]
pub struct ResetTimersResponse {}
/// TODO\[NNS1-3420\] This type can be refined into different internal API types, depending on
/// TODO\[NNS1-3420\] the needs of a particular canister. The fields of this type represent
/// TODO\[NNS1-3420\] over-approximation of the fields that might be relevant for observing and
/// TODO\[NNS1-3420\] managing timers in nervous system-related canisters.
#[derive(
Eq,
candid::CandidType,
Expand Down
1 change: 1 addition & 0 deletions rs/sns/init/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ impl SnsInitPayload {
archive_canister_ids: vec![],
index_canister_id: Some(sns_canister_ids.index),
testflight,
timers: None,
}
}

Expand Down
1 change: 1 addition & 0 deletions rs/sns/integration_tests/src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ fn test_get_status() {
archive_canister_ids: vec![],
index_canister_id: Some(PrincipalId::new_user_test_id(45)),
testflight: false,
timers: None,
},
)
.await;
Expand Down
115 changes: 90 additions & 25 deletions rs/sns/integration_tests/src/timers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ use candid::{Decode, Encode, Principal};
use ic_nervous_system_proto::pb::v1::{
GetTimersRequest, GetTimersResponse, ResetTimersRequest, ResetTimersResponse, Timers,
};
use ic_nns_test_utils::sns_wasm::build_governance_sns_wasm;
use ic_nns_test_utils::sns_wasm::{build_governance_sns_wasm, build_root_sns_wasm};
use ic_sns_governance::{init::GovernanceCanisterInitPayloadBuilder, pb::v1::Governance};
use ic_sns_root::pb::v1::SnsRootCanister;
use ic_sns_swap::pb::v1::{
GetStateRequest, GetStateResponse, Init, Lifecycle, NeuronBasketConstructionParameters,
};
Expand All @@ -14,11 +15,13 @@ use ic_types::{CanisterId, PrincipalId};
use pretty_assertions::assert_eq;
use std::time::{Duration, SystemTime};

const TWO_WEEKS_SECONDS: u64 = 14 * 24 * 60 * 60;
const ONE_DAY_SECONDS: u64 = 24 * 60 * 60;
const ONE_WEEK_SECONDS: u64 = 7 * ONE_DAY_SECONDS;

fn swap_init(now: SystemTime) -> Init {
let now = now.duration_since(SystemTime::UNIX_EPOCH).unwrap();
let swap_due_timestamp_seconds = Some((now + Duration::from_secs(TWO_WEEKS_SECONDS)).as_secs());
let swap_due_timestamp_seconds =
Some((now + Duration::from_secs(2 * ONE_WEEK_SECONDS)).as_secs());

Init {
swap_due_timestamp_seconds,
Expand Down Expand Up @@ -52,7 +55,7 @@ fn swap_init(now: SystemTime) -> Init {
}
}

fn governance_proto() -> Governance {
fn governance_init() -> Governance {
GovernanceCanisterInitPayloadBuilder::new()
.with_root_canister_id(PrincipalId::new_anonymous())
.with_ledger_canister_id(PrincipalId::new_anonymous())
Expand All @@ -61,6 +64,19 @@ fn governance_proto() -> Governance {
.build()
}

fn root_init() -> SnsRootCanister {
SnsRootCanister {
governance_canister_id: Some(PrincipalId::new_anonymous()),
ledger_canister_id: Some(PrincipalId::new_anonymous()),
swap_canister_id: Some(PrincipalId::new_anonymous()),
index_canister_id: Some(PrincipalId::new_anonymous()),
archive_canister_ids: vec![],
dapp_canister_ids: vec![],
testflight: false,
timers: None,
}
}

fn get_timers(state_machine: &StateMachine, canister_id: CanisterId) -> Option<Timers> {
let payload = Encode!(&GetTimersRequest {}).unwrap();
let response = state_machine
Expand Down Expand Up @@ -91,7 +107,12 @@ fn try_reset_timers(state_machine: &StateMachine, canister_id: CanisterId) -> Re
/// Assumes that `canister_id` is an ID of an already installed canister that implements:
/// - `get_timers`
/// - `reset_timers`
fn run_canister_reset_timers_test(state_machine: &StateMachine, canister_id: CanisterId) {
fn run_canister_reset_timers_test(
state_machine: &StateMachine,
canister_id: CanisterId,
reset_timers_cool_down_interval_seconds: u64,
run_periodic_tasks_interval_seconds: u64,
) {
let last_spawned_timestamp_seconds = {
let timers = get_timers(state_machine, canister_id);
let last_reset_timestamp_seconds = assert_matches!(timers, Some(Timers {
Expand All @@ -100,9 +121,9 @@ fn run_canister_reset_timers_test(state_machine: &StateMachine, canister_id: Can
..
}) => last_reset_timestamp_seconds);

// Resetting the timers cannot be done sooner than `RESET_TIMERS_COOL_DOWN_INTERVAL` after
// the canister is initialized.
state_machine.advance_time(Duration::from_secs(1000));
// Resetting the timers cannot be done sooner than `reset_timers_cool_down_interval_seconds`
// after the canister is initialized.
state_machine.advance_time(Duration::from_secs(reset_timers_cool_down_interval_seconds));
state_machine.tick();

let timers = get_timers(state_machine, canister_id);
Expand All @@ -117,7 +138,7 @@ fn run_canister_reset_timers_test(state_machine: &StateMachine, canister_id: Can

assert_eq!(
last_spawned_timestamp_seconds,
last_reset_timestamp_seconds + 1000
last_reset_timestamp_seconds + reset_timers_cool_down_interval_seconds
);
last_spawned_timestamp_seconds
};
Expand Down Expand Up @@ -148,7 +169,7 @@ fn run_canister_reset_timers_test(state_machine: &StateMachine, canister_id: Can
last_spawned_before_reset_timestamp_seconds
);

state_machine.advance_time(Duration::from_secs(100));
state_machine.advance_time(Duration::from_secs(run_periodic_tasks_interval_seconds));
state_machine.tick();

let timers = get_timers(state_machine, canister_id);
Expand All @@ -163,17 +184,19 @@ fn run_canister_reset_timers_test(state_machine: &StateMachine, canister_id: Can

assert_eq!(
last_spawned_timestamp_seconds,
last_spawned_before_reset_timestamp_seconds + 100
last_spawned_before_reset_timestamp_seconds + run_periodic_tasks_interval_seconds
);
}
}

fn run_canister_reset_timers_cannot_be_spammed_test(
state_machine: &StateMachine,
canister_id: CanisterId,
reset_timers_cool_down_interval_seconds: u64,
) {
// Ensure there was more than 600 seconds since the timers were initialized.
state_machine.advance_time(Duration::from_secs(600));
// Ensure there was more than `reset_timers_cool_down_interval_seconds` seconds since the timers
// were initialized.
state_machine.advance_time(Duration::from_secs(reset_timers_cool_down_interval_seconds));
state_machine.tick();

let get_last_spawned_timestamp_seconds = || {
Expand All @@ -195,12 +218,20 @@ fn run_canister_reset_timers_cannot_be_spammed_test(
let last_spawned_timestamp_seconds_1 = get_last_spawned_timestamp_seconds();

// Attempt to reset the timers again, after a small delay.
state_machine.advance_time(Duration::from_secs(500));
let insufficient_for_resetting_timers_by_seconds = reset_timers_cool_down_interval_seconds
.checked_sub(100)
.unwrap();
state_machine.advance_time(Duration::from_secs(
insufficient_for_resetting_timers_by_seconds,
));
state_machine.tick();

{
let err_text = try_reset_timers(state_machine, canister_id).unwrap_err();
assert!(err_text.contains("Reset has already been called within the past 600 seconds"));
assert!(err_text.contains(&format!(
"Reset has already been called within the past {} seconds",
reset_timers_cool_down_interval_seconds
)));
}

let last_spawned_timestamp_seconds_2 = get_last_spawned_timestamp_seconds();
Expand All @@ -212,7 +243,7 @@ fn run_canister_reset_timers_cannot_be_spammed_test(
);

// Attempt to reset the timers again after reset cool down.
state_machine.advance_time(Duration::from_secs(101));
state_machine.advance_time(Duration::from_secs(100));
state_machine.tick();

try_reset_timers(state_machine, canister_id).unwrap_or_else(|err| {
Expand All @@ -224,10 +255,9 @@ fn run_canister_reset_timers_cannot_be_spammed_test(

let last_spawned_timestamp_seconds_3 = get_last_spawned_timestamp_seconds();

// Waited for 500 + 101 seconds after the last reset.
assert_eq!(
last_spawned_timestamp_seconds_3,
last_spawned_timestamp_seconds_2 + 601
last_spawned_timestamp_seconds_2 + reset_timers_cool_down_interval_seconds
);
}

Expand Down Expand Up @@ -280,7 +310,7 @@ fn test_swap_periodic_tasks_disabled_eventually() {
// (1) advance the Swap lifecycle
// (2) set already_tried_to_auto_finalize
// (3) unset requires_periodic_tasks
state_machine.advance_time(Duration::from_secs(TWO_WEEKS_SECONDS));
state_machine.advance_time(Duration::from_secs(2 * ONE_WEEK_SECONDS));
state_machine.tick();
state_machine.advance_time(Duration::from_secs(100));
state_machine.tick();
Expand Down Expand Up @@ -315,7 +345,7 @@ fn test_swap_reset_timers() {
.unwrap()
};

run_canister_reset_timers_test(&state_machine, canister_id)
run_canister_reset_timers_test(&state_machine, canister_id, 600, 60);
}

#[test]
Expand All @@ -325,13 +355,33 @@ fn test_governance_reset_timers() {
// Install the Governance canister.
let canister_id = {
let wasm = build_governance_sns_wasm().wasm;
let args = Encode!(&governance_proto()).unwrap();
let args = Encode!(&governance_init()).unwrap();
state_machine
.install_canister(wasm.clone(), args, None)
.unwrap()
};

run_canister_reset_timers_test(&state_machine, canister_id);
run_canister_reset_timers_test(&state_machine, canister_id, 600, 60);
}

#[test]
fn test_root_reset_timers() {
let state_machine = state_machine_builder_for_sns_tests().build();

let canister_id = {
let wasm = build_root_sns_wasm().wasm;
let args = Encode!(&root_init()).unwrap();
state_machine
.install_canister(wasm.clone(), args, None)
.unwrap()
};

run_canister_reset_timers_test(
&state_machine,
canister_id,
ONE_WEEK_SECONDS,
ONE_DAY_SECONDS,
);
}

#[test]
Expand All @@ -347,7 +397,7 @@ fn test_swap_reset_timers_cannot_be_spammed() {
.unwrap()
};

run_canister_reset_timers_cannot_be_spammed_test(&state_machine, canister_id);
run_canister_reset_timers_cannot_be_spammed_test(&state_machine, canister_id, 600);
}

#[test]
Expand All @@ -356,11 +406,26 @@ fn test_governance_reset_timers_cannot_be_spammed() {

let canister_id = {
let wasm = build_governance_sns_wasm().wasm;
let args = Encode!(&governance_proto()).unwrap();
let args = Encode!(&governance_init()).unwrap();
state_machine
.install_canister(wasm.clone(), args, None)
.unwrap()
};

run_canister_reset_timers_cannot_be_spammed_test(&state_machine, canister_id, 600);
}

#[test]
fn test_root_reset_timers_cannot_be_spammed() {
let state_machine = state_machine_builder_for_sns_tests().build();

let canister_id = {
let wasm = build_root_sns_wasm().wasm;
let args = Encode!(&root_init()).unwrap();
state_machine
.install_canister(wasm.clone(), args, None)
.unwrap()
};

run_canister_reset_timers_cannot_be_spammed_test(&state_machine, canister_id);
run_canister_reset_timers_cannot_be_spammed_test(&state_machine, canister_id, ONE_WEEK_SECONDS);
}
2 changes: 2 additions & 0 deletions rs/sns/root/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ DEPENDENCIES = [
"//packages/icrc-ledger-types:icrc_ledger_types",
"//rs/nervous_system/clients",
"//rs/nervous_system/common",
"//rs/nervous_system/proto",
"//rs/nervous_system/root",
"//rs/nervous_system/runtime",
"//rs/rust_canisters/canister_log",
Expand Down Expand Up @@ -87,6 +88,7 @@ generated_files_check(
srcs = ["tests/check_generated_files.rs"],
data = glob(["src/gen/*.rs"]) + [
":protos",
"//rs/nervous_system/proto:protos",
"//rs/types/base_types:protos",
],
manifest_dir = "rs/sns/root",
Expand Down
1 change: 1 addition & 0 deletions rs/sns/root/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ ic-metrics-encoder = "1"
ic-nervous-system-clients = { path = "../../nervous_system/clients" }
ic-nervous-system-common = { path = "../../nervous_system/common" }
ic-nervous-system-common-build-metadata = { path = "../../nervous_system/common/build_metadata" }
ic-nervous-system-proto = { path = "../../nervous_system/proto" }
ic-nervous-system-root = { path = "../../nervous_system/root" }
ic-nervous-system-runtime = { path = "../../nervous_system/runtime" }
ic-sns-swap = { path = "../swap" }
Expand Down
Loading