diff --git a/Cargo.lock b/Cargo.lock index ef922b0ddac..04d2541ea11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10027,6 +10027,7 @@ dependencies = [ "ic-registry-subnet-type", "ic-registry-transport", "ic-sns-governance", + "ic-sns-governance-api", "ic-sns-init", "ic-sns-root", "ic-sns-swap", @@ -11872,7 +11873,7 @@ dependencies = [ "ic-nns-common", "ic-nns-constants", "ic-nns-governance-api", - "ic-sns-governance", + "ic-sns-governance-api", "ic-sns-init", "ic-sns-root", "ic-sns-wasm", diff --git a/rs/nervous_system/integration_tests/BUILD.bazel b/rs/nervous_system/integration_tests/BUILD.bazel index 5ed2885da3c..be5af458a6c 100644 --- a/rs/nervous_system/integration_tests/BUILD.bazel +++ b/rs/nervous_system/integration_tests/BUILD.bazel @@ -20,6 +20,7 @@ BASE_DEPENDENCIES = [ "//rs/nns/governance/api", "//rs/nns/sns-wasm", "//rs/sns/governance", + "//rs/sns/governance/api", "//rs/sns/init", "//rs/sns/root", "//rs/sns/swap", diff --git a/rs/nervous_system/integration_tests/Cargo.toml b/rs/nervous_system/integration_tests/Cargo.toml index 15a1334d584..75529f7c0ba 100644 --- a/rs/nervous_system/integration_tests/Cargo.toml +++ b/rs/nervous_system/integration_tests/Cargo.toml @@ -24,6 +24,7 @@ ic-nns-common = { path = "../../nns/common" } ic-nns-governance = { path = "../../nns/governance" } ic-nns-governance-api = { path = "../../nns/governance/api" } ic-sns-governance = { path = "../../sns/governance" } +ic-sns-governance-api = { path = "../../sns/governance/api" } ic-sns-root = { path = "../../sns/root" } ic-sns-swap = { path = "../../sns/swap" } icp-ledger = { path = "../../ledger_suite/icp" } diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index 96c08908322..88df9497c69 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -38,7 +38,7 @@ use ic_nns_test_utils::{ }, }; use ic_registry_transport::pb::v1::RegistryAtomicMutateRequest; -use ic_sns_governance::pb::v1::{ +use ic_sns_governance_api::pb::v1::{ self as sns_pb, governance::Version, AdvanceTargetVersionRequest, AdvanceTargetVersionResponse, }; use ic_sns_init::SnsCanisterInitPayloads; @@ -73,6 +73,9 @@ use std::{collections::BTreeMap, fmt::Write, ops::Range, time::Duration}; pub const STARTING_CYCLES_PER_CANISTER: u128 = 2_000_000_000_000_000; +/// How frequently the canister should attempt to refresh the cached_upgrade_steps +pub(crate) const UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS: u64 = 60 * 60; // 1 hour + pub fn fmt_bytes(bytes: &[u8]) -> String { bytes.iter().fold(String::new(), |mut output, x| { let _ = write!(output, "{:02x}", x); @@ -564,7 +567,7 @@ pub async fn install_sns_directly_with_snsw_versions( index_wasm_hash: index_sns_wasm.sha256_hash().to_vec(), }; - governance.deployed_version = Some(deployed_version); + governance.deployed_version = Some(deployed_version.into()); install_canister( root_canister_id, @@ -1121,6 +1124,7 @@ pub mod nns { use ic_nns_test_utils::sns_wasm::create_modified_sns_wasm; use ic_sns_wasm::pb::v1::{ GetWasmRequest, GetWasmResponse, ListUpgradeStepsRequest, ListUpgradeStepsResponse, + SnsVersion, }; pub async fn get_deployed_sns_by_proposal_id( @@ -1182,10 +1186,25 @@ pub mod nns { .cloned() .expect("No upgrade steps found") .version - .expect("No version found") - .into(); - - latest_version + .expect("No version found"); + + let SnsVersion { + root_wasm_hash, + governance_wasm_hash, + ledger_wasm_hash, + swap_wasm_hash, + archive_wasm_hash, + index_wasm_hash, + } = latest_version; + + Version { + root_wasm_hash, + governance_wasm_hash, + ledger_wasm_hash, + swap_wasm_hash, + archive_wasm_hash, + index_wasm_hash, + } } /// Modify the WASM for a given canister type and add it to SNS-W. @@ -1367,8 +1386,11 @@ pub mod sns { use assert_matches::assert_matches; use ic_crypto_sha2::Sha256; use ic_nervous_system_agent::sns::governance::{GovernanceCanister, SubmitProposalError}; - use ic_sns_governance::{ - governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS, pb::v1::get_neuron_response, + use ic_sns_governance_api::pb::v1::{ + get_neuron_response, + neuron::DissolveState, + upgrade_journal_entry::{self, Event}, + Neuron, }; use pocket_ic::ErrorCode; use sns_pb::UpgradeSnsControlledCanister; @@ -1377,6 +1399,30 @@ pub mod sns { pub const EXPECTED_UPGRADE_STEPS_REFRESH_MAX_SECONDS: u64 = UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS + 10; + pub fn redact_human_readable(event: Event) -> Event { + match event { + Event::UpgradeOutcome(upgrade_outcome) => { + Event::UpgradeOutcome(upgrade_journal_entry::UpgradeOutcome { + human_readable: None, + ..upgrade_outcome + }) + } + Event::UpgradeStepsReset(upgrade_steps_reset) => { + Event::UpgradeStepsReset(upgrade_journal_entry::UpgradeStepsReset { + human_readable: None, + ..upgrade_steps_reset + }) + } + Event::TargetVersionReset(target_version_reset) => { + Event::TargetVersionReset(upgrade_journal_entry::TargetVersionReset { + human_readable: None, + ..target_version_reset + }) + } + event => event, + } + } + /// Manage an SNS neuron, e.g., to make an SNS Governance proposal. async fn manage_neuron( pocket_ic: &PocketIc, @@ -1386,14 +1432,14 @@ pub mod sns { neuron_id: sns_pb::NeuronId, command: sns_pb::manage_neuron::Command, ) -> sns_pb::ManageNeuronResponse { - let sub_account = neuron_id.subaccount().unwrap(); + let subaccount = neuron_id.id.to_vec(); let result = pocket_ic .update_call( canister_id.into(), sender.into(), "manage_neuron", Encode!(&sns_pb::ManageNeuron { - subaccount: sub_account.to_vec(), + subaccount, command: Some(command), }) .unwrap(), @@ -1529,6 +1575,16 @@ pub mod sns { Decode!(&result, sns_pb::ListNeuronsResponse).unwrap() } + fn dissolve_delay_seconds(neuron: &Neuron, now_seconds: u64) -> u64 { + match neuron.dissolve_state { + Some(DissolveState::DissolveDelaySeconds(d)) => d, + Some(DissolveState::WhenDissolvedTimestampSeconds(ts)) => { + ts.saturating_sub(now_seconds) + } + None => 0, + } + } + /// Searches for the ID and controller principal of an SNS neuron that can submit proposals, /// i.e., a neuron whose `dissolve_delay_seconds` is greater that or equal 6 months. pub async fn find_neuron_with_majority_voting_power( @@ -1539,7 +1595,7 @@ pub mod sns { sns_neurons .iter() .find(|neuron| { - neuron.dissolve_delay_seconds(neuron.created_timestamp_seconds) + dissolve_delay_seconds(neuron, neuron.created_timestamp_seconds) >= 6 * 30 * ONE_DAY_SECONDS }) .map(|sns_neuron| { @@ -1592,7 +1648,7 @@ pub mod sns { }, ) .await - .map_err(|err| err.to_string()) + .map_err(|err| format!("{err:?}")) } // Upgrade; one canister at a time. @@ -1781,11 +1837,8 @@ pub mod sns { }; assert!(actual.timestamp_seconds.is_some()); assert_eq!( - &actual - .event - .clone() - .map(|event| event.redact_human_readable()), - &Some(expected.clone().redact_human_readable()), + &actual.event.clone().map(redact_human_readable), + &Some(redact_human_readable(expected.clone())), "Upgrade journal entry at index {} does not match", index ); diff --git a/rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs b/rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs index 34e7bfaaeaf..26d8d3d1b92 100644 --- a/rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs +++ b/rs/nervous_system/integration_tests/tests/advance_sns_target_version.rs @@ -11,9 +11,13 @@ use ic_nervous_system_integration_tests::{ }, }; use ic_nns_test_utils::sns_wasm::create_modified_sns_wasm; -use ic_sns_governance::pb::v1::upgrade_journal_entry::UpgradeStepsReset; -use ic_sns_governance::{ - governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS, +use ic_sns_governance::governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS; +use ic_sns_governance_api::pb::v1::{ + governance::Versions, + upgrade_journal_entry::{upgrade_outcome, upgrade_started, UpgradeStepsReset}, + Empty, +}; +use ic_sns_governance_api::{ pb::v1 as sns_pb, pb::v1::upgrade_journal_entry::{Event, TargetVersionSet, UpgradeStepsRefreshed}, }; @@ -71,10 +75,14 @@ async fn test_get_upgrade_journal() { // Step 1: Check that the upgrade journal contains the initial version right after SNS creation. let mut expected_upgrade_journal_entries = vec![]; { - expected_upgrade_journal_entries.push(Event::UpgradeStepsReset(UpgradeStepsReset::new( - "this message will be redacted to keep the test spec more abstract".to_string(), - vec![initial_sns_version.clone()], - ))); + expected_upgrade_journal_entries.push(Event::UpgradeStepsReset(UpgradeStepsReset { + human_readable: Some( + "this message will be redacted to keep the test spec more abstract".to_string(), + ), + upgrade_steps: Some(Versions { + versions: vec![initial_sns_version.clone()], + }), + })); sns::governance::assert_upgrade_journal( &pocket_ic, @@ -158,11 +166,15 @@ async fn test_get_upgrade_journal() { { expected_upgrade_journal_entries.push(Event::UpgradeStepsRefreshed( - UpgradeStepsRefreshed::new(vec![ - initial_sns_version.clone(), - new_sns_version_1.clone(), - new_sns_version_2.clone(), - ]), + UpgradeStepsRefreshed { + upgrade_steps: Some(Versions { + versions: vec![ + initial_sns_version.clone(), + new_sns_version_1.clone(), + new_sns_version_2.clone(), + ], + }), + }, )); sns::governance::assert_upgrade_journal( @@ -178,11 +190,11 @@ async fn test_get_upgrade_journal() { .await .unwrap(); - expected_upgrade_journal_entries.push(Event::TargetVersionSet(TargetVersionSet::new( - None, - new_sns_version_2.clone(), - false, - ))); + expected_upgrade_journal_entries.push(Event::TargetVersionSet(TargetVersionSet { + old_target_version: None, + new_target_version: Some(new_sns_version_2.clone()), + is_advanced_automatically: Some(false), + })); sns::governance::assert_upgrade_journal( &pocket_ic, @@ -225,35 +237,45 @@ async fn test_get_upgrade_journal() { { expected_upgrade_journal_entries.push( sns_pb::upgrade_journal_entry::Event::UpgradeStarted( - sns_pb::upgrade_journal_entry::UpgradeStarted::from_behind_target( - initial_sns_version.clone(), - new_sns_version_1.clone(), - ), + sns_pb::upgrade_journal_entry::UpgradeStarted { + current_version: Some(initial_sns_version.clone()), + expected_version: Some(new_sns_version_1.clone()), + reason: Some(upgrade_started::Reason::BehindTargetVersion(Empty {})), + }, ), ); expected_upgrade_journal_entries.push( sns_pb::upgrade_journal_entry::Event::UpgradeOutcome( - sns_pb::upgrade_journal_entry::UpgradeOutcome::success( - "this message will be redacted to keep the test spec more abstract".to_string(), - ), + sns_pb::upgrade_journal_entry::UpgradeOutcome { + human_readable: Some( + "this message will be redacted to keep the test spec more abstract" + .to_string(), + ), + status: Some(upgrade_outcome::Status::Success(Empty {})), + }, ), ); expected_upgrade_journal_entries.push( sns_pb::upgrade_journal_entry::Event::UpgradeStarted( - sns_pb::upgrade_journal_entry::UpgradeStarted::from_behind_target( - new_sns_version_1.clone(), - new_sns_version_2.clone(), - ), + sns_pb::upgrade_journal_entry::UpgradeStarted { + current_version: Some(new_sns_version_1.clone()), + expected_version: Some(new_sns_version_2.clone()), + reason: todo!(), + }, ), ); expected_upgrade_journal_entries.push( sns_pb::upgrade_journal_entry::Event::UpgradeOutcome( - sns_pb::upgrade_journal_entry::UpgradeOutcome::success( - "this message will be redacted to keep the test spec more abstract".to_string(), - ), + sns_pb::upgrade_journal_entry::UpgradeOutcome { + human_readable: Some( + "this message will be redacted to keep the test spec more abstract" + .to_string(), + ), + status: Some(upgrade_outcome::Status::Success(Empty {})), + }, ), ); diff --git a/rs/nervous_system/integration_tests/tests/sns_lifecycle.rs b/rs/nervous_system/integration_tests/tests/sns_lifecycle.rs index 14c65dee43d..7bd550a6032 100644 --- a/rs/nervous_system/integration_tests/tests/sns_lifecycle.rs +++ b/rs/nervous_system/integration_tests/tests/sns_lifecycle.rs @@ -24,10 +24,8 @@ use ic_nns_governance_api::pb::v1::{ get_neurons_fund_audit_info_response, neurons_fund_snapshot::NeuronsFundNeuronPortion, CreateServiceNervousSystem, Neuron, }; -use ic_sns_governance::{ - governance::TREASURY_SUBACCOUNT_NONCE, - pb::v1::{self as sns_pb, NeuronPermissionType}, -}; +use ic_sns_governance::governance::TREASURY_SUBACCOUNT_NONCE; +use ic_sns_governance_api::pb::v1::{self as sns_pb, NeuronPermissionType}; use ic_sns_init::distributions::MAX_DEVELOPER_DISTRIBUTION_COUNT; use ic_sns_root::CanisterSummary; use ic_sns_swap::{ diff --git a/rs/nervous_system/integration_tests/tests/sns_upgrade_test_utils.rs b/rs/nervous_system/integration_tests/tests/sns_upgrade_test_utils.rs index edcc04f4b6d..9afe71bd095 100644 --- a/rs/nervous_system/integration_tests/tests/sns_upgrade_test_utils.rs +++ b/rs/nervous_system/integration_tests/tests/sns_upgrade_test_utils.rs @@ -12,7 +12,7 @@ use ic_nervous_system_integration_tests::{ SectionTimer, }; use ic_sns_governance::governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS; -use ic_sns_governance::pb::v1::upgrade_journal_entry; +use ic_sns_governance_api::pb::v1::upgrade_journal_entry; use ic_sns_swap::pb::v1::Lifecycle; use ic_sns_wasm::pb::v1::SnsCanisterType; diff --git a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs index 0fa5a0cb732..751238dc165 100644 --- a/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs +++ b/rs/nervous_system/integration_tests/tests/upgrade_sns_controlled_canister_with_large_wasm.rs @@ -12,7 +12,7 @@ use ic_nervous_system_integration_tests::{ }; use ic_nns_constants::ROOT_CANISTER_ID; use ic_nns_test_utils::common::modify_wasm_bytes; -use ic_sns_governance::pb::v1::{ChunkedCanisterWasm, UpgradeSnsControlledCanister}; +use ic_sns_governance_api::pb::v1::{ChunkedCanisterWasm, UpgradeSnsControlledCanister}; use ic_sns_swap::pb::v1::Lifecycle; use pocket_ic::nonblocking::PocketIc; use pocket_ic::PocketIcBuilder; diff --git a/rs/sns/cli/Cargo.toml b/rs/sns/cli/Cargo.toml index 362c8eb2dc3..a543ac57799 100644 --- a/rs/sns/cli/Cargo.toml +++ b/rs/sns/cli/Cargo.toml @@ -34,7 +34,7 @@ cycles-minting-canister = { path = "../../nns/cmc" } ic-nns-common = { path = "../../nns/common" } ic-nns-constants = { path = "../../nns/constants" } ic-nns-governance-api = { path = "../../nns/governance/api" } -ic-sns-governance-api = { path = "../governance" } +ic-sns-governance-api = { path = "../governance/api" } ic-sns-init = { path = "../init" } ic-sns-root = { path = "../root" } ic-sns-wasm = { path = "../../nns/sns-wasm" } diff --git a/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs b/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs index c33cc19d9c1..243ef08e4b9 100644 --- a/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs +++ b/rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs @@ -11,7 +11,16 @@ pub struct NeuronPermission { /// The id of a specific neuron, which equals the neuron's subaccount on the ledger canister /// (the account that holds the neuron's staked tokens). #[derive( - Default, candid::CandidType, candid::Deserialize, Debug, Eq, std::hash::Hash, Clone, PartialEq, + Default, + candid::CandidType, + candid::Deserialize, + Debug, + Eq, + std::hash::Hash, + Clone, + PartialEq, + PartialOrd, + Ord, )] pub struct NeuronId { #[serde(with = "serde_bytes")] @@ -587,6 +596,7 @@ pub mod governance_error { Hash, PartialOrd, Ord, + ::prost::Enumeration, )] #[repr(i32)] pub enum ErrorType { @@ -2380,6 +2390,7 @@ pub struct Account { Hash, PartialOrd, Ord, + ::prost::Enumeration, )] #[repr(i32)] pub enum NeuronPermissionType { diff --git a/rs/sns/governance/src/upgrade_journal.rs b/rs/sns/governance/src/upgrade_journal.rs index 8735d3f167e..865152655ed 100644 --- a/rs/sns/governance/src/upgrade_journal.rs +++ b/rs/sns/governance/src/upgrade_journal.rs @@ -216,34 +216,6 @@ impl From for upgrade_journal_entry:: } } -impl upgrade_journal_entry::Event { - /// Useful for specifying expected states of the SNS upgrade journal in a way that isn't - /// overly fragile. - pub fn redact_human_readable(self) -> Self { - match self { - Self::UpgradeOutcome(upgrade_outcome) => { - Self::UpgradeOutcome(upgrade_journal_entry::UpgradeOutcome { - human_readable: None, - ..upgrade_outcome - }) - } - Self::UpgradeStepsReset(upgrade_steps_reset) => { - Self::UpgradeStepsReset(upgrade_journal_entry::UpgradeStepsReset { - human_readable: None, - ..upgrade_steps_reset - }) - } - Self::TargetVersionReset(target_version_reset) => { - Self::TargetVersionReset(upgrade_journal_entry::TargetVersionReset { - human_readable: None, - ..target_version_reset - }) - } - event => event, - } - } -} - pub fn serve_journal(journal: &UpgradeJournal) -> ic_canisters_http_types::HttpResponse { use ic_canisters_http_types::HttpResponseBuilder; let journal = ic_sns_governance_api::pb::v1::UpgradeJournal::from(journal.clone());