From 2bc9b2fd53a15062fdd9d3f4c819b417854f538f Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Tue, 3 Mar 2026 17:26:03 +0100 Subject: [PATCH 01/25] DEFI-2677: General VisibilitySettings --- rs/execution_environment/src/canister_logs.rs | 18 ++----- .../src/canister_settings.rs | 39 +++++++++++++- .../src/canister_settings/tests.rs | 54 +++++++++++++++++++ 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/rs/execution_environment/src/canister_logs.rs b/rs/execution_environment/src/canister_logs.rs index 4bd74b18f7bc..fc2aae6f557e 100644 --- a/rs/execution_environment/src/canister_logs.rs +++ b/rs/execution_environment/src/canister_logs.rs @@ -1,3 +1,4 @@ +use crate::canister_settings::VisibilitySettings; use ic_config::flag_status::FlagStatus; use ic_error_types::{ErrorCode, UserError}; use ic_management_canister_types_private::{ @@ -42,22 +43,13 @@ pub(crate) fn check_log_visibility_permission( log_visibility: &LogVisibilityV2, controllers: &std::collections::BTreeSet, ) -> Result<(), UserError> { - let has_access = match log_visibility { - LogVisibilityV2::Public => true, - LogVisibilityV2::Controllers => controllers.contains(caller), - LogVisibilityV2::AllowedViewers(principals) => { - principals.get().contains(caller) || controllers.contains(caller) - } - }; - - if has_access { - Ok(()) - } else { - Err(UserError::new( + if !VisibilitySettings::from(log_visibility).has_access(caller, controllers) { + return Err(UserError::new( ErrorCode::CanisterRejectedMessage, format!("Caller {caller} is not allowed to access canister logs"), - )) + )); } + Ok(()) } fn filter_records( diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs index 877a94916f84..457ec22e2d8f 100644 --- a/rs/execution_environment/src/canister_settings.rs +++ b/rs/execution_environment/src/canister_settings.rs @@ -1,11 +1,13 @@ use ic_base_types::{EnvironmentVariables, NumBytes, NumSeconds}; use ic_error_types::{ErrorCode, UserError}; -use ic_management_canister_types_private::{CanisterSettingsArgs, LogVisibilityV2}; +use ic_management_canister_types_private::{ + BoundedAllowedViewers, CanisterSettingsArgs, LogVisibilityV2, +}; use ic_types::{ ComputeAllocation, Cycles, InvalidComputeAllocationError, MemoryAllocation, PrincipalId, }; use num_traits::cast::ToPrimitive; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::convert::TryFrom; #[cfg(test)] @@ -483,3 +485,36 @@ impl ValidatedCanisterSettings { self.environment_variables.as_ref() } } + +#[derive(Clone, Eq, PartialEq, Debug)] +pub(crate) enum VisibilitySettings<'a> { + Public, + Controllers, + AllowedViewers(&'a BoundedAllowedViewers), +} + +impl<'a> From<&'a LogVisibilityV2> for VisibilitySettings<'a> { + fn from(v: &'a LogVisibilityV2) -> Self { + match v { + LogVisibilityV2::Public => Self::Public, + LogVisibilityV2::Controllers => Self::Controllers, + LogVisibilityV2::AllowedViewers(principals) => Self::AllowedViewers(principals), + } + } +} + +impl VisibilitySettings<'_> { + pub(crate) fn has_access( + &self, + caller: &PrincipalId, + controllers: &BTreeSet, + ) -> bool { + match self { + Self::Public => true, + Self::Controllers => controllers.contains(caller), + Self::AllowedViewers(allowed) => { + allowed.get().contains(caller) || controllers.contains(caller) + } + } + } +} diff --git a/rs/execution_environment/src/canister_settings/tests.rs b/rs/execution_environment/src/canister_settings/tests.rs index 3dde4fbf7590..ecbd37ec315d 100644 --- a/rs/execution_environment/src/canister_settings/tests.rs +++ b/rs/execution_environment/src/canister_settings/tests.rs @@ -86,3 +86,57 @@ fn test_environment_variables_hash_output() { let actual = EnvironmentVariables::new(env_vars).hash(); assert_eq!(actual, expected); } + +mod visibility_settings { + use crate::canister_settings::VisibilitySettings; + use ic_management_canister_types_private::BoundedAllowedViewers; + use ic_types::PrincipalId; + use proptest::prelude::*; + use std::collections::BTreeSet; + + + proptest! { + #[test] + fn public_always_grants_access( + caller in arb_principal_id(), + controllers in arb_controllers(), + ) { + prop_assert!(VisibilitySettings::Public.has_access(&caller, &controllers)); + } + + #[test] + fn controllers_grants_access_only_to_controllers( + caller in arb_principal_id(), + controllers in arb_controllers(), + ) { + prop_assert_eq!( + VisibilitySettings::Controllers.has_access(&caller, &controllers), + controllers.contains(&caller), + ); + } + + #[test] + fn allowed_viewers_grants_access_to_viewers_and_controllers( + caller in arb_principal_id(), + controllers in arb_controllers(), + viewers in proptest::collection::vec(arb_principal_id(), 0..=10), + ) { + let allowed = BoundedAllowedViewers::new(viewers.clone()); + let settings = VisibilitySettings::AllowedViewers(&allowed); + prop_assert_eq!( + settings.has_access(&caller, &controllers), + viewers.contains(&caller) || controllers.contains(&caller), + ); + } + } + + fn arb_principal_id() -> BoxedStrategy { + (0..u64::MAX) + .prop_map(PrincipalId::new_user_test_id) + .boxed() + } + + fn arb_controllers() -> BoxedStrategy> { + proptest::collection::btree_set(arb_principal_id(), 0..=10).boxed() + } +} From 5c7072f6cf66c1c157e14d04388654dc01d1b0f2 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Tue, 3 Mar 2026 21:01:03 +0100 Subject: [PATCH 02/25] DEFI-2677: Snapshot visibility settings --- .../src/canister_manager.rs | 6 + .../src/canister_settings.rs | 37 +++++- .../src/canister_settings/tests.rs | 1 - .../clients/src/update_settings.rs | 10 ++ rs/nns/governance/api/src/types.rs | 45 +++++++ .../ic_nns_governance/pb/v1/governance.proto | 10 ++ .../src/gen/ic_nns_governance.pb.v1.rs | 48 ++++++++ rs/nns/governance/src/pb/conversions/mod.rs | 2 + .../src/proposals/update_canister_settings.rs | 28 ++++- .../v1/canister_state_bits.proto | 16 ++- .../gen/state/state.canister_state_bits.v1.rs | 27 ++++- rs/replicated_state/src/canister_state.rs | 5 + .../src/canister_state/system_state.rs | 9 +- .../api/src/ic_sns_governance.pb.v1.rs | 1 + rs/sns/governance/canister/governance.did | 1 + .../ic_sns_governance/pb/v1/governance.proto | 11 ++ .../src/gen/ic_sns_governance.pb.v1.rs | 46 +++++++ rs/sns/governance/src/pb/conversions.rs | 2 + rs/sns/governance/src/proposal.rs | 13 +- rs/sns/governance/src/sns_root_types.rs | 46 +++++++ rs/sns/governance/src/types.rs | 2 + rs/sns/root/canister/root.did | 1 + .../root/proto/ic_sns_root/pb/v1/root.proto | 11 ++ rs/sns/root/src/gen/ic_sns_root.pb.v1.rs | 46 +++++++ rs/sns/root/src/lib.rs | 3 +- rs/sns/root/src/pb/mod.rs | 16 +++ rs/state_layout/src/state_layout.rs | 3 +- rs/state_layout/src/state_layout/proto.rs | 9 ++ rs/state_manager/src/checkpoint.rs | 1 + rs/state_manager/src/tip.rs | 1 + rs/types/management_canister_types/src/lib.rs | 112 ++++++++++++++++++ .../management_canister_types/tests/ic.did | 8 ++ 32 files changed, 566 insertions(+), 11 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 839bc73fa9bb..ec9e32295564 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -516,6 +516,7 @@ impl CanisterManager { settings.reserved_cycles_limit(), reservation_cycles, settings.log_visibility().cloned(), + settings.snapshot_visibility().cloned(), log_memory_limit, settings.wasm_memory_limit(), settings.environment_variables().cloned(), @@ -590,6 +591,9 @@ impl CanisterManager { if let Some(log_visibility) = settings.log_visibility() { canister.system_state.log_visibility = log_visibility.clone(); } + if let Some(snapshot_visibility) = settings.snapshot_visibility() { + canister.system_state.snapshot_visibility = snapshot_visibility.clone(); + } if let Some(log_memory_limit) = settings.log_memory_limit() { canister .system_state @@ -1167,6 +1171,7 @@ impl CanisterManager { let freeze_threshold = canister.system_state.freeze_threshold; let reserved_cycles_limit = canister.system_state.reserved_balance_limit(); let log_visibility = canister.system_state.log_visibility.clone(); + let snapshot_visibility = canister.system_state.snapshot_visibility.clone(); let log_memory_limit = canister.log_memory_limit().get(); let wasm_memory_limit = canister.system_state.wasm_memory_limit; let wasm_memory_threshold = canister.system_state.wasm_memory_threshold; @@ -1197,6 +1202,7 @@ impl CanisterManager { freeze_threshold.get(), reserved_cycles_limit.map(|x| x.get()), log_visibility, + snapshot_visibility, log_memory_limit, self.cycles_account_manager .idle_cycles_burned_rate( diff --git a/rs/execution_environment/src/canister_settings.rs b/rs/execution_environment/src/canister_settings.rs index 457ec22e2d8f..3bf6b3ecaeb3 100644 --- a/rs/execution_environment/src/canister_settings.rs +++ b/rs/execution_environment/src/canister_settings.rs @@ -1,7 +1,7 @@ use ic_base_types::{EnvironmentVariables, NumBytes, NumSeconds}; use ic_error_types::{ErrorCode, UserError}; use ic_management_canister_types_private::{ - BoundedAllowedViewers, CanisterSettingsArgs, LogVisibilityV2, + BoundedAllowedViewers, CanisterSettingsArgs, LogVisibilityV2, SnapshotVisibility, }; use ic_types::{ ComputeAllocation, Cycles, InvalidComputeAllocationError, MemoryAllocation, PrincipalId, @@ -27,6 +27,7 @@ pub(crate) struct CanisterSettings { pub(crate) freezing_threshold: Option, pub(crate) reserved_cycles_limit: Option, pub(crate) log_visibility: Option, + pub(crate) snapshot_visibility: Option, pub(crate) log_memory_limit: Option, pub(crate) wasm_memory_limit: Option, pub(crate) environment_variables: Option, @@ -41,6 +42,7 @@ impl CanisterSettings { freezing_threshold: Option, reserved_cycles_limit: Option, log_visibility: Option, + snapshot_visibility: Option, log_memory_limit: Option, wasm_memory_limit: Option, environment_variables: Option, @@ -53,6 +55,7 @@ impl CanisterSettings { freezing_threshold, reserved_cycles_limit, log_visibility, + snapshot_visibility, log_memory_limit, wasm_memory_limit, environment_variables, @@ -87,6 +90,10 @@ impl CanisterSettings { self.log_visibility.as_ref() } + pub fn snapshot_visibility(&self) -> Option<&SnapshotVisibility> { + self.snapshot_visibility.as_ref() + } + pub fn log_memory_limit(&self) -> Option { self.log_memory_limit } @@ -193,6 +200,7 @@ impl TryFrom for CanisterSettings { freezing_threshold, reserved_cycles_limit, input.log_visibility, + input.snapshot_visibility, log_memory_limit, wasm_memory_limit, environment_variables, @@ -219,6 +227,7 @@ pub(crate) struct CanisterSettingsBuilder { freezing_threshold: Option, reserved_cycles_limit: Option, log_visibility: Option, + snapshot_visibility: Option, log_memory_limit: Option, wasm_memory_limit: Option, environment_variables: Option, @@ -235,6 +244,7 @@ impl CanisterSettingsBuilder { freezing_threshold: None, reserved_cycles_limit: None, log_visibility: None, + snapshot_visibility: None, log_memory_limit: None, wasm_memory_limit: None, environment_variables: None, @@ -250,6 +260,7 @@ impl CanisterSettingsBuilder { freezing_threshold: self.freezing_threshold, reserved_cycles_limit: self.reserved_cycles_limit, log_visibility: self.log_visibility, + snapshot_visibility: self.snapshot_visibility, log_memory_limit: self.log_memory_limit, wasm_memory_limit: self.wasm_memory_limit, environment_variables: self.environment_variables, @@ -305,6 +316,13 @@ impl CanisterSettingsBuilder { } } + pub fn with_snapshot_visibility(self, snapshot_visibility: SnapshotVisibility) -> Self { + Self { + snapshot_visibility: Some(snapshot_visibility), + ..self + } + } + pub fn with_log_memory_limit(self, log_memory_limit: NumBytes) -> Self { Self { log_memory_limit: Some(log_memory_limit), @@ -407,6 +425,7 @@ pub(crate) struct ValidatedCanisterSettings { reserved_cycles_limit: Option, reservation_cycles: Cycles, log_visibility: Option, + snapshot_visibility: Option, log_memory_limit: Option, wasm_memory_limit: Option, environment_variables: Option, @@ -422,6 +441,7 @@ impl ValidatedCanisterSettings { reserved_cycles_limit: Option, reservation_cycles: Cycles, log_visibility: Option, + snapshot_visibility: Option, log_memory_limit: Option, wasm_memory_limit: Option, environment_variables: Option, @@ -435,6 +455,7 @@ impl ValidatedCanisterSettings { reserved_cycles_limit, reservation_cycles, log_visibility, + snapshot_visibility, log_memory_limit, wasm_memory_limit, environment_variables, @@ -473,6 +494,10 @@ impl ValidatedCanisterSettings { self.log_visibility.as_ref() } + pub fn snapshot_visibility(&self) -> Option<&SnapshotVisibility> { + self.snapshot_visibility.as_ref() + } + pub fn log_memory_limit(&self) -> Option { self.log_memory_limit } @@ -503,6 +528,16 @@ impl<'a> From<&'a LogVisibilityV2> for VisibilitySettings<'a> { } } +impl<'a> From<&'a SnapshotVisibility> for VisibilitySettings<'a> { + fn from(v: &'a SnapshotVisibility) -> Self { + match v { + SnapshotVisibility::Public => Self::Public, + SnapshotVisibility::Controllers => Self::Controllers, + SnapshotVisibility::AllowedViewers(principals) => Self::AllowedViewers(principals), + } + } +} + impl VisibilitySettings<'_> { pub(crate) fn has_access( &self, diff --git a/rs/execution_environment/src/canister_settings/tests.rs b/rs/execution_environment/src/canister_settings/tests.rs index ecbd37ec315d..69f8da923e1f 100644 --- a/rs/execution_environment/src/canister_settings/tests.rs +++ b/rs/execution_environment/src/canister_settings/tests.rs @@ -94,7 +94,6 @@ mod visibility_settings { use proptest::prelude::*; use std::collections::BTreeSet; - proptest! { #[test] fn public_always_grants_access( diff --git a/rs/nervous_system/clients/src/update_settings.rs b/rs/nervous_system/clients/src/update_settings.rs index ebf90f793d83..7990095d03a9 100644 --- a/rs/nervous_system/clients/src/update_settings.rs +++ b/rs/nervous_system/clients/src/update_settings.rs @@ -22,6 +22,15 @@ pub enum LogVisibility { Public, } +#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default, CandidType, Deserialize)] +pub enum SnapshotVisibility { + #[default] + #[serde(rename = "controllers")] + Controllers, + #[serde(rename = "public")] + Public, +} + /// The CanisterSettings struct as defined in the ic-interface-spec /// https://internetcomputer.org/docs/current/references/ic-interface-spec/#ic-candid #[derive(Clone, Eq, PartialEq, Hash, Debug, Default, CandidType, Deserialize)] @@ -32,6 +41,7 @@ pub struct CanisterSettings { pub freezing_threshold: Option, pub reserved_cycles_limit: Option, pub log_visibility: Option, + pub snapshot_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, } diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index e6434821f570..e5a4ce48949b 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -2649,6 +2649,7 @@ pub mod update_canister_settings { pub log_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, + pub snapshot_visibility: Option, } /// Log visibility of a canister. #[derive( @@ -2694,6 +2695,50 @@ pub mod update_canister_settings { } } } + /// Snapshot visibility of a canister. + #[derive( + candid::CandidType, + candid::Deserialize, + serde::Serialize, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + )] + #[repr(i32)] + pub enum SnapshotVisibility { + Unspecified = 0, + /// Snapshots are visible to the controllers of the dapp canister. + Controllers = 1, + /// Snapshots are visible to the public. + Public = 2, + } + impl SnapshotVisibility { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + SnapshotVisibility::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", + SnapshotVisibility::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", + SnapshotVisibility::Public => "SNAPSHOT_VISIBILITY_PUBLIC", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> Option { + match value { + "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), + "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), + "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), + _ => None, + } + } + } } #[derive( candid::CandidType, candid::Deserialize, serde::Serialize, Clone, PartialEq, Debug, Default, diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 99188204cd9d..2005f112d9e5 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -1916,6 +1916,15 @@ message UpdateCanisterSettings { LOG_VISIBILITY_PUBLIC = 2; } + // Snapshot visibility of a canister. + enum SnapshotVisibility { + SNAPSHOT_VISIBILITY_UNSPECIFIED = 0; + // Snapshots are visible to the controllers of the dapp canister. + SNAPSHOT_VISIBILITY_CONTROLLERS = 1; + // Snapshots are visible to the public. + SNAPSHOT_VISIBILITY_PUBLIC = 2; + } + // The controllers of the canister. We use a message to wrap the repeated field because prost does // not generate `Option>` for repeated fields. message Controllers { @@ -1933,6 +1942,7 @@ message UpdateCanisterSettings { optional LogVisibility log_visibility = 5; optional uint64 wasm_memory_limit = 6; optional uint64 wasm_memory_threshold = 7; + optional SnapshotVisibility snapshot_visibility = 8; } // The settings to update. Required. diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index 8a284eed81d5..279ff30e4307 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -2784,6 +2784,8 @@ pub mod update_canister_settings { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "7")] pub wasm_memory_threshold: ::core::option::Option, + #[prost(enumeration = "SnapshotVisibility", optional, tag = "8")] + pub snapshot_visibility: ::core::option::Option, } /// Log visibility of a canister. #[derive( @@ -2831,6 +2833,52 @@ pub mod update_canister_settings { } } } + /// Snapshot visibility of a canister. + #[derive( + candid::CandidType, + candid::Deserialize, + serde::Serialize, + comparable::Comparable, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + ::prost::Enumeration, + )] + #[repr(i32)] + pub enum SnapshotVisibility { + Unspecified = 0, + /// Snapshots are visible to the controllers of the dapp canister. + Controllers = 1, + /// Snapshots are visible to the public. + Public = 2, + } + impl SnapshotVisibility { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", + Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", + Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), + "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), + "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), + _ => None, + } + } + } } #[derive( candid::CandidType, diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index 382311f19f38..96c5a38472a2 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -2826,6 +2826,7 @@ impl From log_visibility: item.log_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, + snapshot_visibility: item.snapshot_visibility, } } } @@ -2842,6 +2843,7 @@ impl From log_visibility: item.log_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, + snapshot_visibility: item.snapshot_visibility, } } } diff --git a/rs/nns/governance/src/proposals/update_canister_settings.rs b/rs/nns/governance/src/proposals/update_canister_settings.rs index 2c5fafcd9460..75aaab5470b2 100644 --- a/rs/nns/governance/src/proposals/update_canister_settings.rs +++ b/rs/nns/governance/src/proposals/update_canister_settings.rs @@ -1,7 +1,7 @@ use crate::{ pb::v1::{ GovernanceError, SelfDescribingValue, Topic, UpdateCanisterSettings, - update_canister_settings::{CanisterSettings, LogVisibility}, + update_canister_settings::{CanisterSettings, LogVisibility, SnapshotVisibility}, }, proposals::{ call_canister::CallCanister, @@ -17,6 +17,7 @@ use candid::{Encode, Nat}; use ic_base_types::CanisterId; use ic_nervous_system_clients::update_settings::{ CanisterSettings as RootCanisterSettings, LogVisibility as RootLogVisibility, + SnapshotVisibility as RootSnapshotVisibility, }; use ic_nns_constants::{LIFELINE_CANISTER_ID, ROOT_CANISTER_ID}; use ic_nns_handler_root_interface::UpdateCanisterSettingsRequest; @@ -56,6 +57,19 @@ impl UpdateCanisterSettings { } } + fn valid_snapshot_visibility( + snapshot_visibility_i32: i32, + ) -> Result { + let snapshot_visibility = SnapshotVisibility::try_from(snapshot_visibility_i32); + match snapshot_visibility { + Ok(SnapshotVisibility::Controllers) => Ok(RootSnapshotVisibility::Controllers), + Ok(SnapshotVisibility::Public) => Ok(RootSnapshotVisibility::Public), + Ok(SnapshotVisibility::Unspecified) | Err(_) => Err(invalid_proposal_error(&format!( + "Invalid snapshot visibility {snapshot_visibility_i32}" + ))), + } + } + fn valid_canister_settings(&self) -> Result { let settings = self .settings @@ -67,6 +81,7 @@ impl UpdateCanisterSettings { && settings.memory_allocation.is_none() && settings.freezing_threshold.is_none() && settings.log_visibility.is_none() + && settings.snapshot_visibility.is_none() && settings.wasm_memory_limit.is_none() && settings.wasm_memory_threshold.is_none() { @@ -88,6 +103,12 @@ impl UpdateCanisterSettings { Some(log_visibility) => Some(Self::valid_log_visibility(log_visibility)?), None => None, }; + let snapshot_visibility = match settings.snapshot_visibility { + Some(snapshot_visibility) => { + Some(Self::valid_snapshot_visibility(snapshot_visibility)?) + } + None => None, + }; // Reserved cycles limit is not supported yet. let reserved_cycles_limit = None; Ok(RootCanisterSettings { @@ -97,6 +118,7 @@ impl UpdateCanisterSettings { freezing_threshold, wasm_memory_limit, log_visibility, + snapshot_visibility, reserved_cycles_limit, wasm_memory_threshold, }) @@ -160,6 +182,7 @@ impl From for SelfDescribingValue { log_visibility, wasm_memory_limit, wasm_memory_threshold, + snapshot_visibility, } = settings; // Flatten the nested `controllers` field. More specifically, get rid of the intermediate @@ -169,6 +192,8 @@ impl From for SelfDescribingValue { let controllers = controllers.map(|controllers| controllers.controllers); let log_visibility = log_visibility.map(SelfDescribingProstEnum::::new); + let snapshot_visibility = + snapshot_visibility.map(SelfDescribingProstEnum::::new); ValueBuilder::new() .add_field("controllers", controllers) @@ -178,6 +203,7 @@ impl From for SelfDescribingValue { .add_field("wasm_memory_limit", wasm_memory_limit) .add_field("wasm_memory_threshold", wasm_memory_threshold) .add_field("log_visibility", log_visibility) + .add_field("snapshot_visibility", snapshot_visibility) .build() } } diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index e0803fcf57f0..680461b4bce0 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -377,6 +377,18 @@ message LogVisibilityV2 { } } +message SnapshotVisibilityAllowedViewers { + repeated types.v1.PrincipalId principals = 1; +} + +message SnapshotVisibility { + oneof snapshot_visibility { + int32 controllers = 1; + int32 public = 2; + SnapshotVisibilityAllowedViewers allowed_viewers = 3; + } +} + message CanisterLogRecord { uint64 idx = 1; uint64 timestamp_nanos = 2; @@ -410,7 +422,7 @@ message TaskQueue { repeated ExecutionTask queue = 3; } -// Next ID: 64 +// Next ID: 65 message CanisterStateBits { reserved 1, 3, 6, 9, 10, 14, 16, 17, 24, 30, 35, 42, 47, 53; @@ -491,4 +503,6 @@ message CanisterStateBits { TaskQueue tasks = 54; // A map of environment variable names to their values map environment_variables = 55; + // Snapshot visibility for the canister. + SnapshotVisibility snapshot_visibility = 64; } diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index 8c6dfeda8d00..486e163730cb 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -591,6 +591,28 @@ pub mod log_visibility_v2 { } } #[derive(Clone, PartialEq, ::prost::Message)] +pub struct SnapshotVisibilityAllowedViewers { + #[prost(message, repeated, tag = "1")] + pub principals: ::prost::alloc::vec::Vec, +} +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct SnapshotVisibility { + #[prost(oneof = "snapshot_visibility::SnapshotVisibility", tags = "1, 2, 3")] + pub snapshot_visibility: ::core::option::Option, +} +/// Nested message and enum types in `SnapshotVisibility`. +pub mod snapshot_visibility { + #[derive(Clone, PartialEq, ::prost::Oneof)] + pub enum SnapshotVisibility { + #[prost(int32, tag = "1")] + Controllers(i32), + #[prost(int32, tag = "2")] + Public(i32), + #[prost(message, tag = "3")] + AllowedViewers(super::SnapshotVisibilityAllowedViewers), + } +} +#[derive(Clone, PartialEq, ::prost::Message)] pub struct CanisterLogRecord { #[prost(uint64, tag = "1")] pub idx: u64, @@ -617,7 +639,7 @@ pub struct TaskQueue { #[prost(message, repeated, tag = "3")] pub queue: ::prost::alloc::vec::Vec, } -/// Next ID: 64 +/// Next ID: 65 #[derive(Clone, PartialEq, ::prost::Message)] pub struct CanisterStateBits { #[prost(uint64, tag = "2")] @@ -740,6 +762,9 @@ pub struct CanisterStateBits { ::prost::alloc::string::String, ::prost::alloc::string::String, >, + /// Snapshot visibility for the canister. + #[prost(message, optional, tag = "64")] + pub snapshot_visibility: ::core::option::Option, #[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")] pub canister_status: ::core::option::Option, } diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 3fcebbffea64..6fb2d53bdba9 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -17,6 +17,7 @@ use ic_interfaces::execution_environment::{ }; use ic_management_canister_types_private::{ CanisterChangeDetails, CanisterChangeOrigin, CanisterStatusType, LogVisibilityV2, + SnapshotVisibility, }; use ic_registry_subnet_type::SubnetType; use ic_types::messages::{CanisterMessage, Ingress, Request, RequestOrResponse, Response}; @@ -100,6 +101,10 @@ impl CanisterState { &self.system_state.log_visibility } + pub fn snapshot_visibility(&self) -> &SnapshotVisibility { + &self.system_state.snapshot_visibility + } + /// Returns the difference in time since the canister was last charged for resource allocations. pub fn duration_since_last_allocation_charge(&self, current_time: Time) -> Duration { debug_assert!( diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 38ff59a5d33a..0d1c5915755f 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -28,7 +28,7 @@ use ic_interfaces::execution_environment::HypervisorError; use ic_logger::{ReplicaLogger, error}; use ic_management_canister_types_private::{ CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterStatusType, - LogVisibilityV2, + LogVisibilityV2, SnapshotVisibility, }; use ic_registry_subnet_type::SubnetType; use ic_types::batch::TotalQueryStats; @@ -482,6 +482,9 @@ pub struct SystemState { /// Log visibility of the canister. pub log_visibility: LogVisibilityV2, + /// Snapshot visibility of the canister. + pub snapshot_visibility: SnapshotVisibility, + /// Log records of the canister. #[validate_eq(CompareWithValidateEq)] pub canister_log: CanisterLog, @@ -675,6 +678,7 @@ impl SystemState { canister_history: CanisterHistory::default(), wasm_chunk_store, log_visibility: Default::default(), + snapshot_visibility: Default::default(), // TODO(EXC-2118): CanisterLog does not store log records efficiently, // therefore it should not scale to memory limit from above. // Remove this field after migration is done. @@ -711,6 +715,7 @@ impl SystemState { wasm_chunk_store_data: PageMap, wasm_chunk_store_metadata: WasmChunkStoreMetadata, log_visibility: LogVisibilityV2, + snapshot_visibility: SnapshotVisibility, canister_log: CanisterLog, log_memory_store_data: Option, wasm_memory_limit: Option, @@ -745,6 +750,7 @@ impl SystemState { wasm_chunk_store_metadata, ), log_visibility, + snapshot_visibility, canister_log, log_memory_store: LogMemoryStore::from_checkpoint( LOG_MEMORY_STORE_FEATURE, @@ -2442,6 +2448,7 @@ pub mod testing { canister_history: Default::default(), wasm_chunk_store: WasmChunkStore::new_for_testing(), log_visibility: Default::default(), + snapshot_visibility: Default::default(), // TODO(EXC-2118): CanisterLog does not store log records efficiently, // therefore it should not scale to memory limit from above. // Remove this field after migration is done. 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 7045e11ab8ff..21b9f0a8a803 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 @@ -591,6 +591,7 @@ pub struct ManageDappCanisterSettings { pub log_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, + pub snapshot_visibility: Option, } /// Unlike `Governance.Version`, this message has optional fields and is the recommended one /// to use in APIs that can evolve. For example, the SNS Governance could eventually support diff --git a/rs/sns/governance/canister/governance.did b/rs/sns/governance/canister/governance.did index 71e5e5c3d2cf..2fa7a484d40c 100644 --- a/rs/sns/governance/canister/governance.did +++ b/rs/sns/governance/canister/governance.did @@ -458,6 +458,7 @@ type ManageDappCanisterSettings = record { canister_ids : vec principal; reserved_cycles_limit : opt nat64; log_visibility : opt int32; + snapshot_visibility : opt int32; wasm_memory_limit : opt nat64; memory_allocation : opt nat64; compute_allocation : opt nat64; diff --git a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto index 0dd3f12e724d..f7d5200e14ef 100644 --- a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto +++ b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto @@ -547,6 +547,16 @@ enum LogVisibility { LOG_VISIBILITY_PUBLIC = 2; } +enum SnapshotVisibility { + SNAPSHOT_VISIBILITY_UNSPECIFIED = 0; + + // Snapshots are visible to the controllers of the dapp canister. + SNAPSHOT_VISIBILITY_CONTROLLERS = 1; + + // Snapshots are visible to the public. + SNAPSHOT_VISIBILITY_PUBLIC = 2; +} + // A proposal to manage the settings of one or more dapp canisters. message ManageDappCanisterSettings { // The canister IDs of the dapp canisters to be modified. @@ -562,6 +572,7 @@ message ManageDappCanisterSettings { optional uint64 wasm_memory_limit = 7; optional uint64 wasm_memory_threshold = 8; + optional SnapshotVisibility snapshot_visibility = 9; } // Unlike `Governance.Version`, this message has optional fields and is the recommended one diff --git a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs index 5d9e821d2a31..3765d38db9a3 100644 --- a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs +++ b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs @@ -893,6 +893,8 @@ pub struct ManageDappCanisterSettings { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "8")] pub wasm_memory_threshold: ::core::option::Option, + #[prost(enumeration = "SnapshotVisibility", optional, tag = "9")] + pub snapshot_visibility: ::core::option::Option, } /// Unlike `Governance.Version`, this message has optional fields and is the recommended one /// to be used in APIs that can evolve. For example, the SNS Governance could eventually support @@ -4501,6 +4503,50 @@ impl LogVisibility { ::prost::Enumeration, )] #[repr(i32)] +pub enum SnapshotVisibility { + Unspecified = 0, + /// Snapshots are visible to the controllers of the dapp canister. + Controllers = 1, + /// Snapshots are visible to the public. + Public = 2, +} +impl SnapshotVisibility { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", + Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", + Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), + "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), + "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), + _ => None, + } + } +} +#[derive( + candid::CandidType, + candid::Deserialize, + comparable::Comparable, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + ::prost::Enumeration, +)] +#[repr(i32)] pub enum ProposalDecisionStatus { Unspecified = 0, /// The proposal is open for voting and a decision (adopt/reject) has yet to be made. diff --git a/rs/sns/governance/src/pb/conversions.rs b/rs/sns/governance/src/pb/conversions.rs index 85b70535c3fd..837d0da75d67 100644 --- a/rs/sns/governance/src/pb/conversions.rs +++ b/rs/sns/governance/src/pb/conversions.rs @@ -804,6 +804,7 @@ impl From for pb_api::ManageDappCanisterSettings freezing_threshold: item.freezing_threshold, reserved_cycles_limit: item.reserved_cycles_limit, log_visibility: item.log_visibility, + snapshot_visibility: item.snapshot_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, } @@ -818,6 +819,7 @@ impl From for pb::ManageDappCanisterSettings freezing_threshold: item.freezing_threshold, reserved_cycles_limit: item.reserved_cycles_limit, log_visibility: item.log_visibility, + snapshot_visibility: item.snapshot_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, } diff --git a/rs/sns/governance/src/proposal.rs b/rs/sns/governance/src/proposal.rs index 2a33b926126c..93ef14066915 100644 --- a/rs/sns/governance/src/proposal.rs +++ b/rs/sns/governance/src/proposal.rs @@ -17,9 +17,9 @@ use crate::{ ManageDappCanisterSettings, ManageLedgerParameters, ManageSnsMetadata, MintSnsTokens, Motion, NervousSystemFunction, NervousSystemParameters, Proposal, ProposalData, ProposalDecisionStatus, ProposalId, ProposalRewardStatus, RegisterDappCanisters, - RegisterExtension, SetTopicsForCustomProposals, SnsVersion, Tally, Topic, Topic as TopicPb, - TransferSnsTreasuryFunds, UpgradeExtension, UpgradeSnsControlledCanister, - UpgradeSnsToNextVersion, Valuation as ValuationPb, Vote, + RegisterExtension, SetTopicsForCustomProposals, SnapshotVisibility, SnsVersion, Tally, + Topic, Topic as TopicPb, TransferSnsTreasuryFunds, UpgradeExtension, + UpgradeSnsControlledCanister, UpgradeSnsToNextVersion, Valuation as ValuationPb, Vote, governance::{SnsMetadata, Version}, governance_error::ErrorType, nervous_system_function::{FunctionType, GenericNervousSystemFunction}, @@ -1851,6 +1851,13 @@ fn validate_and_render_manage_dapp_canister_settings( ); no_change = false; } + if let Some(snapshot_visibility) = &manage_dapp_canister_settings.snapshot_visibility { + render += &format!( + "# Set snapshot visibility to: {:?} \n", + SnapshotVisibility::try_from(*snapshot_visibility).unwrap_or_default() + ); + no_change = false; + } if let Some(wasm_memory_limit) = &manage_dapp_canister_settings.wasm_memory_limit { render += &format!("# Set Wasm memory limit to: {wasm_memory_limit}\n"); no_change = false; diff --git a/rs/sns/governance/src/sns_root_types.rs b/rs/sns/governance/src/sns_root_types.rs index 2ce6e1540d0b..009fbd012a20 100644 --- a/rs/sns/governance/src/sns_root_types.rs +++ b/rs/sns/governance/src/sns_root_types.rs @@ -358,6 +358,8 @@ pub struct ManageDappCanisterSettingsRequest { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "8")] pub wasm_memory_threshold: ::core::option::Option, + #[prost(enumeration = "SnapshotVisibility", optional, tag = "9")] + pub snapshot_visibility: ::core::option::Option, } #[derive( candid::CandidType, @@ -416,3 +418,47 @@ impl LogVisibility { } } } +#[derive( + candid::CandidType, + candid::Deserialize, + comparable::Comparable, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + ::prost::Enumeration, +)] +#[repr(i32)] +pub enum SnapshotVisibility { + Unspecified = 0, + /// Snapshots are visible to the controllers of the dapp canister. + Controllers = 1, + /// Snapshots are visible to the public. + Public = 2, +} +impl SnapshotVisibility { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", + Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", + Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), + "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), + "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), + _ => None, + } + } +} diff --git a/rs/sns/governance/src/types.rs b/rs/sns/governance/src/types.rs index e98eca076e41..f72f4cd9f6e7 100644 --- a/rs/sns/governance/src/types.rs +++ b/rs/sns/governance/src/types.rs @@ -2602,6 +2602,7 @@ impl From for ManageDappCanisterSettingsRequest { freezing_threshold, reserved_cycles_limit, log_visibility, + snapshot_visibility, wasm_memory_limit, wasm_memory_threshold, } = manage_dapp_canister_settings; @@ -2613,6 +2614,7 @@ impl From for ManageDappCanisterSettingsRequest { freezing_threshold, reserved_cycles_limit, log_visibility, + snapshot_visibility, wasm_memory_limit, wasm_memory_threshold, } diff --git a/rs/sns/root/canister/root.did b/rs/sns/root/canister/root.did index 2acd56a5e407..87630b5320c5 100644 --- a/rs/sns/root/canister/root.did +++ b/rs/sns/root/canister/root.did @@ -141,6 +141,7 @@ type ManageDappCanisterSettingsRequest = record { canister_ids : vec principal; reserved_cycles_limit : opt nat64; log_visibility : opt int32; + snapshot_visibility : opt int32; wasm_memory_limit : opt nat64; memory_allocation : opt nat64; compute_allocation : opt nat64; diff --git a/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto b/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto index 00d31fc5e03b..479b6aa6bb0f 100644 --- a/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto +++ b/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto @@ -154,6 +154,16 @@ enum LogVisibility { LOG_VISIBILITY_PUBLIC = 2; } +enum SnapshotVisibility { + SNAPSHOT_VISIBILITY_UNSPECIFIED = 0; + + // Snapshots are visible to the controllers of the dapp canister. + SNAPSHOT_VISIBILITY_CONTROLLERS = 1; + + // Snapshots are visible to the public. + SNAPSHOT_VISIBILITY_PUBLIC = 2; +} + message ManageDappCanisterSettingsRequest { repeated ic_base_types.pb.v1.PrincipalId canister_ids = 1; optional uint64 compute_allocation = 2; @@ -163,6 +173,7 @@ message ManageDappCanisterSettingsRequest { optional LogVisibility log_visibility = 6; optional uint64 wasm_memory_limit = 7; optional uint64 wasm_memory_threshold = 8; + optional SnapshotVisibility snapshot_visibility = 9; } message ManageDappCanisterSettingsResponse { diff --git a/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs b/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs index 2ce6e1540d0b..009fbd012a20 100644 --- a/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs +++ b/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs @@ -358,6 +358,8 @@ pub struct ManageDappCanisterSettingsRequest { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "8")] pub wasm_memory_threshold: ::core::option::Option, + #[prost(enumeration = "SnapshotVisibility", optional, tag = "9")] + pub snapshot_visibility: ::core::option::Option, } #[derive( candid::CandidType, @@ -416,3 +418,47 @@ impl LogVisibility { } } } +#[derive( + candid::CandidType, + candid::Deserialize, + comparable::Comparable, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + ::prost::Enumeration, +)] +#[repr(i32)] +pub enum SnapshotVisibility { + Unspecified = 0, + /// Snapshots are visible to the controllers of the dapp canister. + Controllers = 1, + /// Snapshots are visible to the public. + Public = 2, +} +impl SnapshotVisibility { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", + Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", + Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), + "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), + "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), + _ => None, + } + } +} diff --git a/rs/sns/root/src/lib.rs b/rs/sns/root/src/lib.rs index 9ca326c1e92b..7374f87f79a2 100644 --- a/rs/sns/root/src/lib.rs +++ b/rs/sns/root/src/lib.rs @@ -18,7 +18,7 @@ use ic_nervous_system_clients::{ canister_id_record::CanisterIdRecord, canister_status::CanisterStatusResultV2, management_canister_client::ManagementCanisterClient, - update_settings::{CanisterSettings, LogVisibility, UpdateSettings}, + update_settings::{CanisterSettings, LogVisibility, SnapshotVisibility, UpdateSettings}, }; use ic_nervous_system_runtime::{CdkRuntime, Runtime}; use ic_sns_swap::pb::v1::GetCanisterStatusRequest; @@ -194,6 +194,7 @@ impl ValidatedManageDappCanisterSettingsRequest { freezing_threshold: request.freezing_threshold.map(Nat::from), reserved_cycles_limit: request.reserved_cycles_limit.map(Nat::from), log_visibility: LogVisibility::try_from(request.log_visibility()).ok(), + snapshot_visibility: SnapshotVisibility::try_from(request.snapshot_visibility()).ok(), wasm_memory_limit: request.wasm_memory_limit.map(Nat::from), wasm_memory_threshold: request.wasm_memory_threshold.map(Nat::from), }; diff --git a/rs/sns/root/src/pb/mod.rs b/rs/sns/root/src/pb/mod.rs index 02dccc001a23..34abea189bdc 100644 --- a/rs/sns/root/src/pb/mod.rs +++ b/rs/sns/root/src/pb/mod.rs @@ -13,3 +13,19 @@ impl TryFrom for ic_nervous_system_clients::update_settings:: } } } + +impl TryFrom + for ic_nervous_system_clients::update_settings::SnapshotVisibility +{ + type Error = String; + + fn try_from(snapshot_visibility: v1::SnapshotVisibility) -> Result { + match snapshot_visibility { + v1::SnapshotVisibility::Unspecified => { + Err("Unspecified snapshot visibility".to_string()) + } + v1::SnapshotVisibility::Controllers => Ok(Self::Controllers), + v1::SnapshotVisibility::Public => Ok(Self::Public), + } + } +} diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index 5110b96d77c0..5a936cba393f 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -1,7 +1,7 @@ use ic_base_types::{NumBytes, NumSeconds}; use ic_logger::{ReplicaLogger, error, info, warn}; use ic_management_canister_types_private::{ - Global, LogVisibilityV2, OnLowWasmMemoryHookStatus, SnapshotSource, + Global, LogVisibilityV2, OnLowWasmMemoryHookStatus, SnapshotSource, SnapshotVisibility, }; use ic_metrics::{MetricsRegistry, buckets::decimal_buckets}; use ic_protobuf::state::{ @@ -206,6 +206,7 @@ pub struct CanisterStateBits { pub wasm_chunk_store_metadata: WasmChunkStoreMetadata, pub total_query_stats: TotalQueryStats, pub log_visibility: LogVisibilityV2, + pub snapshot_visibility: SnapshotVisibility, pub log_memory_limit: NumBytes, pub canister_log: CanisterLog, pub wasm_memory_limit: Option, diff --git a/rs/state_layout/src/state_layout/proto.rs b/rs/state_layout/src/state_layout/proto.rs index 3dbf3e309c83..d1cc51547865 100644 --- a/rs/state_layout/src/state_layout/proto.rs +++ b/rs/state_layout/src/state_layout/proto.rs @@ -59,6 +59,10 @@ impl From for pb_canister_state_bits::CanisterStateBits { total_query_stats: Some((&item.total_query_stats).into()), log_visibility_v2: pb_canister_state_bits::LogVisibilityV2::from(&item.log_visibility) .into(), + snapshot_visibility: pb_canister_state_bits::SnapshotVisibility::from( + &item.snapshot_visibility, + ) + .into(), log_memory_limit: item.log_memory_limit.get(), canister_log_records: item .canister_log @@ -197,6 +201,11 @@ impl TryFrom for CanisterStateBits { "CanisterStateBits::log_visibility_v2", ) .unwrap_or_default(), + snapshot_visibility: try_from_option_field( + value.snapshot_visibility, + "CanisterStateBits::snapshot_visibility", + ) + .unwrap_or_default(), log_memory_limit: NumBytes::from(value.log_memory_limit), canister_log: CanisterLog::new_aggregate( value.next_canister_log_record_idx, diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index b4c9270299e2..22163a376074 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -946,6 +946,7 @@ pub fn load_canister_state( wasm_chunk_store_data, canister_state_bits.wasm_chunk_store_metadata, canister_state_bits.log_visibility, + canister_state_bits.snapshot_visibility, canister_state_bits.canister_log, log_memory_store_data, canister_state_bits.wasm_memory_limit, diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index 5ce35c7a4a34..365352da9ba8 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -1385,6 +1385,7 @@ fn serialize_canister_protos_to_checkpoint_readwrite( .clone(), total_query_stats: canister_state.system_state.total_query_stats.clone(), log_visibility: canister_state.system_state.log_visibility.clone(), + snapshot_visibility: canister_state.system_state.snapshot_visibility.clone(), log_memory_limit: canister_state.log_memory_limit(), canister_log: canister_state.system_state.canister_log.clone(), wasm_memory_limit: canister_state.system_state.wasm_memory_limit, diff --git a/rs/types/management_canister_types/src/lib.rs b/rs/types/management_canister_types/src/lib.rs index adf7c622d2ef..2ec582d76bc9 100644 --- a/rs/types/management_canister_types/src/lib.rs +++ b/rs/types/management_canister_types/src/lib.rs @@ -1278,6 +1278,97 @@ impl TryFrom for LogVisibilityV2 { } } +/// Snapshot visibility for a canister. +/// ```text +/// variant { +/// controllers; +/// public; +/// allowed_viewers : vec principal; +/// } +/// ``` +#[derive(Clone, Eq, PartialEq, Debug, Default, CandidType, Deserialize, EnumIter)] +pub enum SnapshotVisibility { + #[default] + #[serde(rename = "controllers")] + Controllers, + #[serde(rename = "public")] + Public, + #[serde(rename = "allowed_viewers")] + AllowedViewers(BoundedAllowedViewers), +} + +impl Payload<'_> for SnapshotVisibility {} + +impl From<&SnapshotVisibility> for pb_canister_state_bits::SnapshotVisibility { + fn from(item: &SnapshotVisibility) -> Self { + match item { + SnapshotVisibility::Controllers => pb_canister_state_bits::SnapshotVisibility { + snapshot_visibility: Some( + pb_canister_state_bits::snapshot_visibility::SnapshotVisibility::Controllers(1), + ), + }, + SnapshotVisibility::Public => pb_canister_state_bits::SnapshotVisibility { + snapshot_visibility: Some( + pb_canister_state_bits::snapshot_visibility::SnapshotVisibility::Public(2), + ), + }, + SnapshotVisibility::AllowedViewers(principals) => { + pb_canister_state_bits::SnapshotVisibility { + snapshot_visibility: Some( + pb_canister_state_bits::snapshot_visibility::SnapshotVisibility::AllowedViewers( + pb_canister_state_bits::SnapshotVisibilityAllowedViewers { + principals: principals + .get() + .iter() + .map(|c| (*c).into()) + .collect::>() + .clone(), + }, + ), + ), + } + } + } + } +} + +impl TryFrom for SnapshotVisibility { + type Error = ProxyDecodeError; + + fn try_from(item: pb_canister_state_bits::SnapshotVisibility) -> Result { + let Some(snapshot_visibility) = item.snapshot_visibility else { + return Err(ProxyDecodeError::MissingField( + "SnapshotVisibility::snapshot_visibility", + )); + }; + match snapshot_visibility { + pb_canister_state_bits::snapshot_visibility::SnapshotVisibility::Controllers(_) => { + Ok(Self::Controllers) + } + pb_canister_state_bits::snapshot_visibility::SnapshotVisibility::Public(_) => { + Ok(Self::Public) + } + pb_canister_state_bits::snapshot_visibility::SnapshotVisibility::AllowedViewers( + data, + ) => { + let principals = data + .principals + .iter() + .map(|p| { + PrincipalId::try_from(p.raw.clone()).map_err(|e| { + ProxyDecodeError::ValueOutOfRange { + typ: "PrincipalId", + err: e.to_string(), + } + }) + }) + .collect::, _>>()?; + Ok(Self::AllowedViewers(BoundedAllowedViewers::new(principals))) + } + } + } +} + /// Struct used for encoding/decoding /// ```text /// record { @@ -1303,6 +1394,7 @@ pub struct DefiniteCanisterSettingsArgs { freezing_threshold: candid::Nat, reserved_cycles_limit: candid::Nat, log_visibility: LogVisibilityV2, + snapshot_visibility: SnapshotVisibility, log_memory_limit: candid::Nat, wasm_memory_limit: candid::Nat, wasm_memory_threshold: candid::Nat, @@ -1318,6 +1410,7 @@ impl DefiniteCanisterSettingsArgs { freezing_threshold: u64, reserved_cycles_limit: Option, log_visibility: LogVisibilityV2, + snapshot_visibility: SnapshotVisibility, log_memory_limit: u64, wasm_memory_limit: Option, wasm_memory_threshold: u64, @@ -1341,6 +1434,7 @@ impl DefiniteCanisterSettingsArgs { freezing_threshold: candid::Nat::from(freezing_threshold), reserved_cycles_limit, log_visibility, + snapshot_visibility, log_memory_limit: candid::Nat::from(log_memory_limit), wasm_memory_limit, wasm_memory_threshold: candid::Nat::from(wasm_memory_threshold), @@ -1360,6 +1454,10 @@ impl DefiniteCanisterSettingsArgs { &self.log_visibility } + pub fn snapshot_visibility(&self) -> &SnapshotVisibility { + &self.snapshot_visibility + } + pub fn log_memory_limit(&self) -> candid::Nat { self.log_memory_limit.clone() } @@ -1489,6 +1587,7 @@ impl CanisterStatusResultV2 { freezing_threshold: u64, reserved_cycles_limit: Option, log_visibility: LogVisibilityV2, + snapshot_visibility: SnapshotVisibility, log_memory_limit: u64, idle_cycles_burned_per_day: u128, reserved_cycles: u128, @@ -1530,6 +1629,7 @@ impl CanisterStatusResultV2 { freezing_threshold, reserved_cycles_limit, log_visibility, + snapshot_visibility, log_memory_limit, wasm_memory_limit, wasm_memory_threshold, @@ -2272,6 +2372,7 @@ pub struct CanisterSettingsArgs { pub freezing_threshold: Option, pub reserved_cycles_limit: Option, pub log_visibility: Option, + pub snapshot_visibility: Option, pub log_memory_limit: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, @@ -2291,6 +2392,7 @@ impl CanisterSettingsArgs { freezing_threshold: None, reserved_cycles_limit: None, log_visibility: None, + snapshot_visibility: None, log_memory_limit: None, wasm_memory_limit: None, wasm_memory_threshold: None, @@ -2307,6 +2409,7 @@ pub struct CanisterSettingsArgsBuilder { freezing_threshold: Option, reserved_cycles_limit: Option, log_visibility: Option, + snapshot_visibility: Option, log_memory_limit: Option, wasm_memory_limit: Option, wasm_memory_threshold: Option, @@ -2327,6 +2430,7 @@ impl CanisterSettingsArgsBuilder { freezing_threshold: self.freezing_threshold, reserved_cycles_limit: self.reserved_cycles_limit, log_visibility: self.log_visibility, + snapshot_visibility: self.snapshot_visibility, log_memory_limit: self.log_memory_limit, wasm_memory_limit: self.wasm_memory_limit, wasm_memory_threshold: self.wasm_memory_threshold, @@ -2412,6 +2516,14 @@ impl CanisterSettingsArgsBuilder { } } + /// Sets the snapshot visibility. + pub fn with_snapshot_visibility(self, snapshot_visibility: SnapshotVisibility) -> Self { + Self { + snapshot_visibility: Some(snapshot_visibility), + ..self + } + } + /// Sets the log capacity in bytes. pub fn with_log_memory_limit(self, log_memory_limit: u64) -> Self { Self { diff --git a/rs/types/management_canister_types/tests/ic.did b/rs/types/management_canister_types/tests/ic.did index 343cc0897f2c..dd45e995321f 100644 --- a/rs/types/management_canister_types/tests/ic.did +++ b/rs/types/management_canister_types/tests/ic.did @@ -8,6 +8,12 @@ type log_visibility = variant { allowed_viewers : vec principal; }; +type snapshot_visibility = variant { + controllers; + public; + allowed_viewers : vec principal; +}; + type environment_variable = record { name: text; value: text; @@ -20,6 +26,7 @@ type canister_settings = record { freezing_threshold : opt nat; reserved_cycles_limit : opt nat; log_visibility : opt log_visibility; + snapshot_visibility : opt snapshot_visibility; log_memory_limit : opt nat; wasm_memory_limit : opt nat; wasm_memory_threshold : opt nat; @@ -34,6 +41,7 @@ type definite_canister_settings = record { freezing_threshold : nat; reserved_cycles_limit : nat; log_visibility : log_visibility; + snapshot_visibility : snapshot_visibility; log_memory_limit : nat; wasm_memory_limit : nat; wasm_memory_threshold: nat; From 938314a6d2632aff905e113fd8e42c2515b9d43f Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 08:34:18 +0100 Subject: [PATCH 03/25] DEFI-2677: fix Pocket IC --- rs/pocket_ic_server/src/pocket_ic.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index 617dfc30f8b3..c663f3273286 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -69,7 +69,7 @@ use ic_management_canister_types_private::{ CanisterSnapshotDataKind, CanisterSnapshotDataOffset, EcdsaCurve, EcdsaKeyId, LogVisibilityV2, MasterPublicKeyId, Method as Ic00Method, ProvisionalCreateCanisterWithCyclesArgs, ReadCanisterSnapshotDataArgs, ReadCanisterSnapshotMetadataArgs, - ReadCanisterSnapshotMetadataResponse, SchnorrAlgorithm, SchnorrKeyId, + ReadCanisterSnapshotMetadataResponse, SchnorrAlgorithm, SchnorrKeyId, SnapshotVisibility, UploadCanisterSnapshotDataArgs, UploadCanisterSnapshotMetadataArgs, VetKdCurve, VetKdKeyId, }; use ic_metrics::MetricsRegistry; @@ -1146,6 +1146,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(REGISTRY_CANISTER_ID.get()), @@ -1229,6 +1230,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(CYCLES_MINTING_CANISTER_ID.get()), @@ -1399,6 +1401,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(LEDGER_CANISTER_ID.get()), @@ -1480,6 +1483,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(LEDGER_INDEX_CANISTER_ID.get()), @@ -1559,6 +1563,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = ii_subnet.state_machine.create_canister_with_cycles( Some(CYCLES_LEDGER_CANISTER_ID.get()), @@ -1623,6 +1628,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = ii_subnet.state_machine.create_canister_with_cycles( Some(CYCLES_LEDGER_INDEX_CANISTER_ID.get()), @@ -1690,6 +1696,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(4_294_967_296_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(GOVERNANCE_CANISTER_ID.get()), @@ -1767,6 +1774,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(ROOT_CANISTER_ID.get()), @@ -1834,6 +1842,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(SNS_WASM_CANISTER_ID.get()), @@ -1932,6 +1941,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = sns_subnet.state_machine.create_canister_with_cycles( Some(SNS_AGGREGATOR_CANISTER_ID.get()), @@ -2005,6 +2015,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = ii_subnet.state_machine.create_canister_with_cycles( Some(IDENTITY_CANISTER_ID.get()), @@ -2182,6 +2193,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(NNS_UI_CANISTER_ID.get()), @@ -2279,6 +2291,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(2_000_000_000_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = btc_subnet.state_machine.create_canister_with_cycles( Some(BITCOIN_TESTNET_CANISTER_ID.get()), @@ -2353,6 +2366,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = btc_subnet.state_machine.create_canister_with_cycles( Some(DOGECOIN_CANISTER_ID.get()), @@ -2419,6 +2433,7 @@ impl PocketIcSubnets { wasm_memory_limit: Some(3_221_225_472_u64.into()), wasm_memory_threshold: Some(0_u64.into()), environment_variables: None, + snapshot_visibility: Some(SnapshotVisibility::Controllers), }; let canister_id = nns_subnet.state_machine.create_canister_with_cycles( Some(MIGRATION_CANISTER_ID.get()), From 7c4f84e258deffa422a85325fe17436e23972765 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 09:33:28 +0100 Subject: [PATCH 04/25] DEFI-2677: fix registry admin --- rs/registry/admin/bin/main.rs | 16 ++++++++++++++-- rs/registry/admin/bin/types.rs | 8 ++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/rs/registry/admin/bin/main.rs b/rs/registry/admin/bin/main.rs index b5ed390e36a3..77ea30da05ed 100644 --- a/rs/registry/admin/bin/main.rs +++ b/rs/registry/admin/bin/main.rs @@ -64,6 +64,7 @@ use ic_nns_governance_api::{ subnet_rental::{RentalConditionId, SubnetRentalRequest}, update_canister_settings::{ CanisterSettings, Controllers, LogVisibility as GovernanceLogVisibility, + SnapshotVisibility as GovernanceSnapshotVisibility, }, }; use ic_nns_handler_root::root_proposals::{GovernanceUpgradeRootProposal, RootProposalBallot}; @@ -167,8 +168,8 @@ use std::{ }; use types::{ LogVisibility, NodeDetails, ProposalAction, ProposalMetadata, ProposalPayload, - ProvisionalWhitelistRecord, Registry, RegistryRecord, RegistryValue, SubnetDescriptor, - SubnetRecord, + ProvisionalWhitelistRecord, Registry, RegistryRecord, RegistryValue, SnapshotVisibility, + SubnetDescriptor, SubnetRecord, }; use update_subnet::ProposeToUpdateSubnetCmd; use url::Url; @@ -1548,6 +1549,9 @@ struct ProposeToUpdateCanisterSettingsCmd { #[clap(long)] /// If set, it will update the canister's wasm memory threshold to this value. wasm_memory_threshold: Option, + #[clap(long)] + /// If set, it will update the canister's snapshot visibility to this value. + snapshot_visibility: Option, } impl ProposalTitle for ProposeToUpdateCanisterSettingsCmd { @@ -1583,6 +1587,13 @@ impl ProposalAction for ProposeToUpdateCanisterSettingsCmd { Some(LogVisibility::Public) => Some(GovernanceLogVisibility::Public as i32), None => None, }; + let snapshot_visibility = match self.snapshot_visibility { + Some(SnapshotVisibility::Controllers) => { + Some(GovernanceSnapshotVisibility::Controllers as i32) + } + Some(SnapshotVisibility::Public) => Some(GovernanceSnapshotVisibility::Public as i32), + None => None, + }; let update_settings = UpdateCanisterSettings { canister_id, @@ -1594,6 +1605,7 @@ impl ProposalAction for ProposeToUpdateCanisterSettingsCmd { wasm_memory_limit, log_visibility, wasm_memory_threshold, + snapshot_visibility, }), }; diff --git a/rs/registry/admin/bin/types.rs b/rs/registry/admin/bin/types.rs index be4cea983e59..c964afbab6fc 100644 --- a/rs/registry/admin/bin/types.rs +++ b/rs/registry/admin/bin/types.rs @@ -250,6 +250,14 @@ pub enum LogVisibility { Public, } +#[derive(Copy, Clone, Eq, PartialEq, Debug, Deserialize, EnumString, Serialize)] +pub enum SnapshotVisibility { + #[strum(serialize = "controllers")] + Controllers, + #[strum(serialize = "public")] + Public, +} + /// Trait to extract the payload for each proposal type. /// This trait is async as building some payloads requires async calls. #[async_trait] From 507e627107ab79705926b407137ee564c5b59e0e Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 10:23:46 +0100 Subject: [PATCH 05/25] DEFI-2677: fix state_layout --- rs/state_layout/src/state_layout/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/state_layout/src/state_layout/tests.rs b/rs/state_layout/src/state_layout/tests.rs index 5e7dc2c52d38..90082978fc4d 100644 --- a/rs/state_layout/src/state_layout/tests.rs +++ b/rs/state_layout/src/state_layout/tests.rs @@ -59,6 +59,7 @@ fn default_canister_state_bits() -> CanisterStateBits { wasm_chunk_store_metadata: WasmChunkStoreMetadata::default(), total_query_stats: TotalQueryStats::default(), log_visibility: Default::default(), + snapshot_visibility: Default::default(), log_memory_limit: NumBytes::from(0), canister_log: CanisterLog::default_aggregate(), wasm_memory_limit: None, From 469bdd89cfe093c61646cfba8bd467e00c6d7531 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 10:58:14 +0100 Subject: [PATCH 06/25] DEFI-2677: fix governance --- .../governance/src/proposals/update_canister_settings.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rs/nns/governance/src/proposals/update_canister_settings.rs b/rs/nns/governance/src/proposals/update_canister_settings.rs index 75aaab5470b2..aea9cb4d4cc1 100644 --- a/rs/nns/governance/src/proposals/update_canister_settings.rs +++ b/rs/nns/governance/src/proposals/update_canister_settings.rs @@ -313,6 +313,7 @@ mod tests { compute_allocation: Some(10), freezing_threshold: Some(100), log_visibility: Some(LogVisibility::Public as i32), + snapshot_visibility: Some(SnapshotVisibility::Public as i32), }), }; @@ -343,6 +344,7 @@ mod tests { compute_allocation: Some(Nat::from(10u64)), freezing_threshold: Some(Nat::from(100u64)), log_visibility: Some(RootLogVisibility::Public), + snapshot_visibility: Some(RootSnapshotVisibility::Public), reserved_cycles_limit: None, wasm_memory_threshold: Some(Nat::from(1u64 << 30)), } @@ -365,6 +367,7 @@ mod tests { compute_allocation: Some(10), freezing_threshold: Some(100), log_visibility: Some(LogVisibility::Public as i32), + snapshot_visibility: Some(SnapshotVisibility::Public as i32), }), }; @@ -393,6 +396,7 @@ mod tests { compute_allocation: Some(Nat::from(10u64)), freezing_threshold: Some(Nat::from(100u64)), log_visibility: Some(RootLogVisibility::Public), + snapshot_visibility: Some(RootSnapshotVisibility::Public), reserved_cycles_limit: None, wasm_memory_threshold: Some(Nat::from(1u64 << 30)), } @@ -438,6 +442,7 @@ mod tests { compute_allocation: Some(10), freezing_threshold: Some(100), log_visibility: Some(LogVisibility::Public as i32), + snapshot_visibility: Some(SnapshotVisibility::Public as i32), }), }; @@ -459,6 +464,7 @@ mod tests { "compute_allocation".to_string() => SelfDescribingValue::from(10_u64), "freezing_threshold".to_string() => SelfDescribingValue::from(100_u64), "log_visibility".to_string() => SelfDescribingValue::from("Public"), + "snapshot_visibility".to_string() => SelfDescribingValue::from("Public"), }), }) ); @@ -489,6 +495,7 @@ mod tests { "wasm_memory_limit".to_string() => SelfDescribingValue::Null, "wasm_memory_threshold".to_string() => SelfDescribingValue::Null, "log_visibility".to_string() => SelfDescribingValue::Null, + "snapshot_visibility".to_string() => SelfDescribingValue::Null, }), }) ); From 33efe612d48efc54e18f978ed1384d3d8ab93022 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 11:42:09 +0100 Subject: [PATCH 07/25] DEFI-2677: fix SNS --- rs/sns/governance/src/proposal.rs | 3 +++ rs/sns/integration_tests/src/manage_dapp_canister_settings.rs | 3 ++- rs/sns/root/src/lib.rs | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/rs/sns/governance/src/proposal.rs b/rs/sns/governance/src/proposal.rs index 93ef14066915..297a32ec15b0 100644 --- a/rs/sns/governance/src/proposal.rs +++ b/rs/sns/governance/src/proposal.rs @@ -5143,6 +5143,7 @@ Version { freezing_threshold: Some(1_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(LogVisibility::Public as i32), + snapshot_visibility: Some(SnapshotVisibility::Public as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }) @@ -5274,6 +5275,7 @@ Payload rendering here"# freezing_threshold: Some(1_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(LogVisibility::Public as i32), + snapshot_visibility: Some(SnapshotVisibility::Public as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }) @@ -5289,6 +5291,7 @@ Payload rendering here"# # Set freezing threshold to: 1000 seconds\n\ # Set reserved cycles limit to: 1000000000000 \n\ # Set log visibility to: Public \n\ + # Set snapshot visibility to: Public \n\ # Set Wasm memory limit to: 1000000000\n" ); } diff --git a/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs b/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs index 26b6d23dab05..5b57bcb27a79 100644 --- a/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs +++ b/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs @@ -11,7 +11,7 @@ use ic_nns_test_utils::state_test_helpers::{ }; use ic_sns_governance::pb::v1::{ LogVisibility, ManageDappCanisterSettings, NervousSystemParameters, NeuronPermissionList, - NeuronPermissionType, Proposal, proposal::Action, + NeuronPermissionType, Proposal, SnapshotVisibility, proposal::Action, }; use ic_sns_test_utils::{ itest_helpers::SnsTestsInitPayloadBuilder, @@ -121,6 +121,7 @@ fn test_manage_dapp_canister_settings_successful() { freezing_threshold: Some(0), reserved_cycles_limit: Some(0), log_visibility: Some(LogVisibility::Controllers as i32), + snapshot_visibility: Some(SnapshotVisibility::Controllers as i32), wasm_memory_limit: Some(2_000_000_000), wasm_memory_threshold: Some(0), }, diff --git a/rs/sns/root/src/lib.rs b/rs/sns/root/src/lib.rs index 7374f87f79a2..7769ded203bc 100644 --- a/rs/sns/root/src/lib.rs +++ b/rs/sns/root/src/lib.rs @@ -2381,6 +2381,7 @@ mod tests { freezing_threshold: Some(100_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(crate::pb::v1::LogVisibility::Controllers as i32), + snapshot_visibility: Some(crate::pb::v1::SnapshotVisibility::Controllers as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }; @@ -2407,6 +2408,7 @@ mod tests { freezing_threshold: Some(Nat::from(100_000u64)), reserved_cycles_limit: Some(Nat::from(1_000_000_000_000u64)), log_visibility: Some(LogVisibility::Controllers), + snapshot_visibility: Some(SnapshotVisibility::Controllers), wasm_memory_limit: Some(Nat::from(1_000_000_000u64)), wasm_memory_threshold: Some(Nat::from(1_000_000u64)), }, @@ -2427,6 +2429,7 @@ mod tests { freezing_threshold: Some(100_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(crate::pb::v1::LogVisibility::Controllers as i32), + snapshot_visibility: Some(crate::pb::v1::SnapshotVisibility::Controllers as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }; From 5409fde65173300b136826aa5a84fd9dad5ca286 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 19:42:07 +0100 Subject: [PATCH 08/25] DEFI-2677: fix NNS --- rs/nns/integration_tests/src/update_canister_settings.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/nns/integration_tests/src/update_canister_settings.rs b/rs/nns/integration_tests/src/update_canister_settings.rs index 8535bba04fab..6e1da43e0de0 100644 --- a/rs/nns/integration_tests/src/update_canister_settings.rs +++ b/rs/nns/integration_tests/src/update_canister_settings.rs @@ -7,6 +7,7 @@ use ic_nns_governance_api::{ manage_neuron_response::Command, update_canister_settings::{ CanisterSettings, Controllers, LogVisibility as GovernanceLogVisibility, + SnapshotVisibility as GovernanceSnapshotVisibility, }, }; use ic_nns_test_utils::{ @@ -93,6 +94,7 @@ fn test_update_canister_settings_proposal( freezing_threshold: Some(target_freezing_threshold), wasm_memory_limit: Some(target_wasm_memory_limit), log_visibility: Some(GovernanceLogVisibility::Public as i32), + snapshot_visibility: Some(GovernanceSnapshotVisibility::Public as i32), wasm_memory_threshold: Some(target_wasm_memory_threshold), }), }, From 198f0a84481448a8ca47066545931ed107de5c9e Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Wed, 4 Mar 2026 19:42:16 +0100 Subject: [PATCH 09/25] DEFI-2677: fix replica_tests --- rs/replica_tests/tests/canister_lifecycle.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rs/replica_tests/tests/canister_lifecycle.rs b/rs/replica_tests/tests/canister_lifecycle.rs index 448d0b9a6b7e..f01013d4bdae 100644 --- a/rs/replica_tests/tests/canister_lifecycle.rs +++ b/rs/replica_tests/tests/canister_lifecycle.rs @@ -9,7 +9,7 @@ use ic_error_types::{ErrorCode, RejectCode}; use ic_management_canister_types_private::{ self as ic00, CanisterChange, CanisterIdRecord, CanisterInstallMode, CanisterSettingsArgsBuilder, CanisterStatusResultV2, CanisterStatusType, EmptyBlob, IC_00, - InstallCodeArgs, LogVisibilityV2, Method, Payload, UpdateSettingsArgs, + InstallCodeArgs, LogVisibilityV2, Method, Payload, SnapshotVisibility, UpdateSettingsArgs, }; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_replica_tests as utils; @@ -726,6 +726,7 @@ fn can_get_canister_information() { 2592000, Some(5_000_000_000_000u128), LogVisibilityV2::default(), + SnapshotVisibility::default(), TEST_DEFAULT_LOG_MEMORY_LIMIT, 0u128, 0u128, @@ -796,6 +797,7 @@ fn can_get_canister_information() { 259200, None, LogVisibilityV2::default(), + SnapshotVisibility::default(), TEST_DEFAULT_LOG_MEMORY_LIMIT, 0u128, 0u128, From 1cc4d3e6521447a493acb06fed616116160d03bb Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Fri, 6 Mar 2026 07:41:52 +0100 Subject: [PATCH 10/25] DEFI-2677: fix tests --- rs/cycles_account_manager/src/lib.rs | 2 +- rs/nns/cmc/cmc.did | 1 + rs/nns/governance/canister/governance.did | 1 + rs/nns/handlers/root/impl/canister/root.did | 6 ++++++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rs/cycles_account_manager/src/lib.rs b/rs/cycles_account_manager/src/lib.rs index 199993ecfed4..88e8bda0ce18 100644 --- a/rs/cycles_account_manager/src/lib.rs +++ b/rs/cycles_account_manager/src/lib.rs @@ -47,7 +47,7 @@ const DAY: Duration = Duration::from_secs(SECONDS_PER_DAY as u64); /// Maximum payload size of a management call to update_settings /// overriding the canister's freezing threshold. -const MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: usize = 324; +const MAX_DELAYED_INGRESS_COST_PAYLOAD_SIZE: usize = 338; /// Errors returned by the [`CyclesAccountManager`]. #[derive(Clone, Eq, PartialEq, Debug)] diff --git a/rs/nns/cmc/cmc.did b/rs/nns/cmc/cmc.did index 9a3fde853393..e67d057cc589 100644 --- a/rs/nns/cmc/cmc.did +++ b/rs/nns/cmc/cmc.did @@ -16,6 +16,7 @@ type CanisterSettings = record { freezing_threshold : opt nat; reserved_cycles_limit : opt nat; log_visibility : opt log_visibility; + snapshot_visibility : opt log_visibility; wasm_memory_limit : opt nat; wasm_memory_threshold : opt nat; environment_variables : opt vec environment_variable; diff --git a/rs/nns/governance/canister/governance.did b/rs/nns/governance/canister/governance.did index d840fb9f2bba..09294ced6871 100644 --- a/rs/nns/governance/canister/governance.did +++ b/rs/nns/governance/canister/governance.did @@ -66,6 +66,7 @@ type CanisterSettings = record { freezing_threshold : opt nat64; controllers : opt Controllers; log_visibility : opt int32; + snapshot_visibility : opt int32; wasm_memory_limit : opt nat64; memory_allocation : opt nat64; compute_allocation : opt nat64; diff --git a/rs/nns/handlers/root/impl/canister/root.did b/rs/nns/handlers/root/impl/canister/root.did index bc64fac0d44d..bed3bf518e4c 100644 --- a/rs/nns/handlers/root/impl/canister/root.did +++ b/rs/nns/handlers/root/impl/canister/root.did @@ -27,6 +27,7 @@ type CanisterSettings = record { controllers : opt vec principal; reserved_cycles_limit : opt nat; log_visibility : opt LogVisibility; + snapshot_visibility : opt SnapshotVisibility; wasm_memory_limit : opt nat; memory_allocation : opt nat; compute_allocation : opt nat; @@ -112,6 +113,11 @@ type LogVisibility = variant { public; }; +type SnapshotVisibility = variant { + controllers; + public; +}; + type CanisterStatusLogVisibility = variant { controllers; public; From 3dc83f3456594a29445e3e2453ef5980c943b6ee Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Fri, 6 Mar 2026 09:50:07 +0100 Subject: [PATCH 11/25] DEFI-2677: fix `governance-canister.wasm.gz_size_check`. Current size without PR is already 2_097_610. --- rs/nns/nns.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/nns/nns.bzl b/rs/nns/nns.bzl index d996621de1ab..82859f2ffe1e 100644 --- a/rs/nns/nns.bzl +++ b/rs/nns/nns.bzl @@ -16,7 +16,7 @@ but other things could be added here later. CANISTER_NAME_TO_MAX_COMPRESSED_WASM_SIZE_E5_BYTES = { "cycles-minting-canister.wasm.gz": 6, "genesis-token-canister.wasm.gz": 3, - "governance-canister.wasm.gz": 21, + "governance-canister.wasm.gz": 22, "governance-canister_test.wasm.gz": 23, "registry-canister.wasm.gz": 16, "root-canister.wasm.gz": 5, From c3fd07d21cc22e691c5bff152752a1200a192a69 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Fri, 6 Mar 2026 11:50:04 +0100 Subject: [PATCH 12/25] DEFI-2667: permission for list_canister_snapshot --- rs/execution_environment/src/canister_manager.rs | 12 +++++------- rs/execution_environment/src/execution/common.rs | 16 ++++++++++++++++ .../src/execution_environment.rs | 3 +-- .../tests/canister_snapshots.rs | 8 ++------ 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index ec9e32295564..ad8080192e1c 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -1,7 +1,5 @@ use crate::as_round_instructions; -use crate::execution::common::{ - validate_controller, validate_controller_or_subnet_admin, validate_subnet_admin, -}; +use crate::execution::common::{validate_controller, validate_controller_or_subnet_admin, validate_snapshot_visibility, validate_subnet_admin}; use crate::execution::install_code::OriginalContext; use crate::execution::{install::execute_install, upgrade::execute_upgrade}; use crate::execution_environment::{ @@ -2568,15 +2566,15 @@ impl CanisterManager { /// Returns the canister snapshots list, or /// an error if it failed to retrieve the information. /// - /// Retrieving the canister snapshots list can only be initiated by the controllers. + /// Retrieving the canister snapshots list can only be initiated a principal + /// allowed by the canister snapshot visibility settings. pub(crate) fn list_canister_snapshot( &self, sender: PrincipalId, canister: &CanisterState, state: &ReplicatedState, - ) -> Result, CanisterManagerError> { - // Check sender is a controller. - validate_controller(canister, &sender)?; + ) -> Result, UserError> { + validate_snapshot_visibility(canister, &sender, "list canister snapshots")?; let mut responses = vec![]; for (snapshot_id, snapshot) in state diff --git a/rs/execution_environment/src/execution/common.rs b/rs/execution_environment/src/execution/common.rs index d53c9686d19f..25a2c5755ca1 100644 --- a/rs/execution_environment/src/execution/common.rs +++ b/rs/execution_environment/src/execution/common.rs @@ -355,6 +355,22 @@ pub(crate) fn validate_controller( Ok(()) } +pub(crate) fn validate_snapshot_visibility( + canister: &CanisterState, + caller: &PrincipalId, + err_msg: &str, +) -> Result<(), UserError> { + if !crate::canister_settings::VisibilitySettings::from(canister.snapshot_visibility()) + .has_access(caller, canister.controllers()) + { + return Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to {err_msg}"), + )); + } + Ok(()) +} + pub(crate) fn validate_subnet_admin( subnet_admins: &BTreeSet, sender: &PrincipalId, diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index a1ee86bb7341..a3480969b986 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -2724,8 +2724,7 @@ impl ExecutionEnvironment { let result = self .canister_manager - .list_canister_snapshot(sender, canister, state) - .map_err(UserError::from)?; + .list_canister_snapshot(sender, canister, state)?; Ok(Encode!(&result).unwrap()) } diff --git a/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs index 5fd3d2427d9d..72d29ffcf4b0 100644 --- a/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs @@ -1098,13 +1098,9 @@ fn list_canister_snapshot_fails_invalid_controller() { if let RequestOrResponse::Response(res) = response { assert_eq!(res.originator, *receiver); res.response_payload.assert_contains_reject( - RejectCode::CanisterError, + RejectCode::CanisterReject, &format!( - "Only the controllers of the canister {} can control it.\n\ - Canister's controllers: {}\n\ - Sender's ID: {}", - canister_id, - test.user_id().get(), + "Caller {} is not allowed to list canister snapshots", caller_canister.get(), ), ); From 2ca1c6e67f1a103c16120bbb58beffe4c7459616 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Fri, 6 Mar 2026 13:57:14 +0100 Subject: [PATCH 13/25] DEFI-2677: StateMachine list canister snapshots --- rs/state_machine_tests/src/lib.rs | 38 +++++++++++++++++-- rs/types/management_canister_types/src/lib.rs | 4 ++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 9d8ecc656ac4..1a8a7bf2c440 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -60,10 +60,11 @@ use ic_logger::replica_logger::test_logger; use ic_logger::{ReplicaLogger, error}; use ic_management_canister_types_private::{ self as ic00, CanisterIdRecord, CanisterSnapshotDataKind, CanisterSnapshotDataOffset, - InstallCodeArgs, MasterPublicKeyId, Method, Payload, ReadCanisterSnapshotDataArgs, - ReadCanisterSnapshotDataResponse, ReadCanisterSnapshotMetadataArgs, - ReadCanisterSnapshotMetadataResponse, UploadCanisterSnapshotDataArgs, - UploadCanisterSnapshotMetadataArgs, UploadCanisterSnapshotMetadataResponse, + InstallCodeArgs, ListCanisterSnapshotArgs, ListCanisterSnapshotResponse, MasterPublicKeyId, + Method, Payload, ReadCanisterSnapshotDataArgs, ReadCanisterSnapshotDataResponse, + ReadCanisterSnapshotMetadataArgs, ReadCanisterSnapshotMetadataResponse, + UploadCanisterSnapshotDataArgs, UploadCanisterSnapshotMetadataArgs, + UploadCanisterSnapshotMetadataResponse, }; use ic_management_canister_types_private::{ CanisterHttpResponsePayload, CanisterInstallMode, CanisterSettingsArgs, @@ -3948,6 +3949,35 @@ impl StateMachine { })? } + /// Create a canister snapshot. + pub fn list_canister_snapshot( + &self, + args: ListCanisterSnapshotArgs, + ) -> Result { + let sender = self.get_controller(&args.get_canister_id()); + self.list_canister_snapshot_as(args, sender) + } + + /// Create a canister snapshot. + pub fn list_canister_snapshot_as( + &self, + args: ListCanisterSnapshotArgs, + sender: PrincipalId, + ) -> Result { + self.execute_ingress_as( + sender, + ic00::IC_00, + Method::ListCanisterSnapshots, + args.encode(), + ) + .map(|res| match res { + WasmResult::Reply(data) => ListCanisterSnapshotResponse::decode(&data), + WasmResult::Reject(reason) => { + panic!("take_canister_snapshot call rejected: {reason}") + } + })? + } + /// Helper to download the whole snapshot chunk store. pub fn get_snapshot_chunk_store( &self, diff --git a/rs/types/management_canister_types/src/lib.rs b/rs/types/management_canister_types/src/lib.rs index 2ec582d76bc9..2d2c8d1c94db 100644 --- a/rs/types/management_canister_types/src/lib.rs +++ b/rs/types/management_canister_types/src/lib.rs @@ -4258,6 +4258,10 @@ impl ListCanisterSnapshotArgs { impl Payload<'_> for ListCanisterSnapshotArgs {} +pub type ListCanisterSnapshotResponse = Vec; + +impl Payload<'_> for ListCanisterSnapshotResponse {} + /// An enum representing the possible values of a global variable. #[derive(Copy, Clone, Debug, Deserialize, Serialize, EnumIter, CandidType)] pub enum Global { From c65cb7314488c775e55a79671a26f5b8c29548c0 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Fri, 6 Mar 2026 13:57:30 +0100 Subject: [PATCH 14/25] DEFI-2677: failing test --- .../tests/canister_snapshots.rs | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/rs/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/tests/canister_snapshots.rs index dabc1ca67a5c..e9321bcec8cc 100644 --- a/rs/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/tests/canister_snapshots.rs @@ -1067,3 +1067,135 @@ fn canister_snapshots_and_memory_allocation() { env.take_canister_snapshot(args).unwrap(); } } + +mod snapshot_visibility { + use ic_base_types::{CanisterId, PrincipalId}; + use ic_error_types::{ErrorCode, UserError}; + use ic_management_canister_types_private::{ + BoundedAllowedViewers, CanisterSettingsArgsBuilder, CanisterSnapshotResponse, + ListCanisterSnapshotArgs, SnapshotVisibility, TakeCanisterSnapshotArgs, + }; + use ic_state_machine_tests::{StateMachine, StateMachineBuilder}; + use ic_universal_canister::UNIVERSAL_CANISTER_WASM; + + #[test] + fn test_visibility_of_list_canister_snapshots() { + let env = StateMachineBuilder::new().build(); + let controller = PrincipalId::new_user_test_id(1); + let not_a_controller = PrincipalId::new_user_test_id(2); + let allowed_viewer = PrincipalId::new_user_test_id(3); + let not_allowed_viewer = PrincipalId::new_user_test_id(4); + let allowed_viewers = BoundedAllowedViewers::new(vec![allowed_viewer]); + fn not_allowed_error(caller: &PrincipalId) -> Result<(), UserError> { + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to access canister logs"), + )) + } + + let test_cases = vec![ + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: controller, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: not_a_controller, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: controller, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: not_a_controller, + expected_result: not_allowed_error(¬_a_controller), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), + sender: allowed_viewer, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), + sender: not_allowed_viewer, + expected_result: not_allowed_error(¬_allowed_viewer), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers), + sender: controller, + expected_result: Ok(()), + }, + ]; + for test_case in &test_cases { + let canister_id = env + .install_canister( + UNIVERSAL_CANISTER_WASM.to_vec(), + vec![], + Some( + CanisterSettingsArgsBuilder::new() + .with_snapshot_visibility(test_case.snapshot_visibility.clone()) + .with_controllers(vec![controller]) + .build(), + ), + ) + .unwrap(); + + test_case.check_list_canister_snapshot(canister_id, &env, &[]); + + let snapshot = env + .take_canister_snapshot(TakeCanisterSnapshotArgs { + canister_id: canister_id.get(), + replace_snapshot: None, + uninstall_code: None, + sender_canister_version: None, + }) + .unwrap(); + + test_case.check_list_canister_snapshot(canister_id, &env, &[snapshot]); + } + } + + #[derive(Debug)] + struct TestCase { + snapshot_visibility: SnapshotVisibility, + sender: PrincipalId, + expected_result: Result<(), UserError>, + } + + impl TestCase { + fn check_list_canister_snapshot( + &self, + canister_id: CanisterId, + env: &StateMachine, + snapshots: &[CanisterSnapshotResponse], + ) { + match &self.expected_result { + Err(err) => { + assert_eq!( + env.list_canister_snapshot_as( + ListCanisterSnapshotArgs::new(canister_id), + self.sender + ) + .as_ref(), + Err(err), + "{self:?}" + ); + } + Ok(()) => { + assert_eq!( + env.list_canister_snapshot_as( + ListCanisterSnapshotArgs::new(canister_id), + self.sender + ), + Ok(snapshots.to_vec()), + "{self:?}" + ); + } + } + } + } +} From e6cd2ecd72b56763a2d285f1d57610f8df6ecabd Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Fri, 6 Mar 2026 14:34:35 +0100 Subject: [PATCH 15/25] DEFI-2677: fix allow ingress messages to list canister snapshots --- .../src/canister_manager.rs | 23 +++++++++++++++++-- .../tests/canister_snapshots.rs | 2 +- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index ad8080192e1c..1ac408ed6eda 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -1,5 +1,8 @@ use crate::as_round_instructions; -use crate::execution::common::{validate_controller, validate_controller_or_subnet_admin, validate_snapshot_visibility, validate_subnet_admin}; +use crate::execution::common::{ + validate_controller, validate_controller_or_subnet_admin, validate_snapshot_visibility, + validate_subnet_admin, +}; use crate::execution::install_code::OriginalContext; use crate::execution::{install::execute_install, upgrade::execute_upgrade}; use crate::execution_environment::{ @@ -226,7 +229,6 @@ impl CanisterManager { | Ok(Ic00Method::ClearChunkStore) | Ok(Ic00Method::TakeCanisterSnapshot) | Ok(Ic00Method::LoadCanisterSnapshot) - | Ok(Ic00Method::ListCanisterSnapshots) | Ok(Ic00Method::DeleteCanisterSnapshot) | Ok(Ic00Method::ReadCanisterSnapshotMetadata) | Ok(Ic00Method::ReadCanisterSnapshotData) @@ -255,6 +257,23 @@ impl CanisterManager { } }, + Ok(Ic00Method::ListCanisterSnapshots) => { + match effective_canister_id { + Some(canister_id) => { + let canister_state = state.canister_state(&canister_id).ok_or_else(|| UserError::new( + ErrorCode::CanisterNotFound, + format!("Canister {canister_id} not found"), + ))?; + validate_snapshot_visibility(canister_state, &sender.get(), "list canister snapshots")?; + Ok(()) + }, + None => Err(UserError::new( + ErrorCode::InvalidManagementPayload, + format!("Failed to decode payload for ic00 method: {method_name}"), + )), + } + }, + Ok(Ic00Method::FetchCanisterLogs) => Err(UserError::new( ErrorCode::CanisterRejectedMessage, format!( diff --git a/rs/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/tests/canister_snapshots.rs index e9321bcec8cc..9f3fa9a7fcc0 100644 --- a/rs/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/tests/canister_snapshots.rs @@ -1089,7 +1089,7 @@ mod snapshot_visibility { fn not_allowed_error(caller: &PrincipalId) -> Result<(), UserError> { Err(UserError::new( ErrorCode::CanisterRejectedMessage, - format!("Caller {caller} is not allowed to access canister logs"), + format!("Caller {caller} is not allowed to list canister snapshots"), )) } From 79a640d813b40bcf178dd1de6d48a47a1f6955f4 Mon Sep 17 00:00:00 2001 From: gregorydemay <112856886+gregorydemay@users.noreply.github.com> Date: Mon, 9 Mar 2026 15:23:55 +0100 Subject: [PATCH 16/25] Typos and renaming Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com> --- rs/execution_environment/src/canister_manager.rs | 2 +- rs/state_machine_tests/src/lib.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 1ac408ed6eda..2783a260970c 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -2585,7 +2585,7 @@ impl CanisterManager { /// Returns the canister snapshots list, or /// an error if it failed to retrieve the information. /// - /// Retrieving the canister snapshots list can only be initiated a principal + /// Retrieving the canister snapshots list can only be initiated by a principal /// allowed by the canister snapshot visibility settings. pub(crate) fn list_canister_snapshot( &self, diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 1a8a7bf2c440..e9512efa4f75 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -3949,8 +3949,8 @@ impl StateMachine { })? } - /// Create a canister snapshot. - pub fn list_canister_snapshot( + /// List canister snapshots. + pub fn list_canister_snapshots( &self, args: ListCanisterSnapshotArgs, ) -> Result { @@ -3958,8 +3958,8 @@ impl StateMachine { self.list_canister_snapshot_as(args, sender) } - /// Create a canister snapshot. - pub fn list_canister_snapshot_as( + /// List canister snapshots. + pub fn list_canister_snapshots_as( &self, args: ListCanisterSnapshotArgs, sender: PrincipalId, @@ -3973,7 +3973,7 @@ impl StateMachine { .map(|res| match res { WasmResult::Reply(data) => ListCanisterSnapshotResponse::decode(&data), WasmResult::Reject(reason) => { - panic!("take_canister_snapshot call rejected: {reason}") + panic!("list_canister_snapshots call rejected: {reason}") } })? } From 5f16df93db4e07eaee0d3835196614886930c689 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Mon, 9 Mar 2026 15:38:19 +0100 Subject: [PATCH 17/25] DEFI-2667: fix renaming --- rs/execution_environment/tests/canister_snapshots.rs | 4 ++-- rs/state_machine_tests/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/tests/canister_snapshots.rs index 9f3fa9a7fcc0..334951a8f65b 100644 --- a/rs/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/tests/canister_snapshots.rs @@ -1176,7 +1176,7 @@ mod snapshot_visibility { match &self.expected_result { Err(err) => { assert_eq!( - env.list_canister_snapshot_as( + env.list_canister_snapshots_as( ListCanisterSnapshotArgs::new(canister_id), self.sender ) @@ -1187,7 +1187,7 @@ mod snapshot_visibility { } Ok(()) => { assert_eq!( - env.list_canister_snapshot_as( + env.list_canister_snapshots_as( ListCanisterSnapshotArgs::new(canister_id), self.sender ), diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index e9512efa4f75..3a3610a3966e 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -3955,7 +3955,7 @@ impl StateMachine { args: ListCanisterSnapshotArgs, ) -> Result { let sender = self.get_controller(&args.get_canister_id()); - self.list_canister_snapshot_as(args, sender) + self.list_canister_snapshots_as(args, sender) } /// List canister snapshots. From fced026c6ee282d12672eb859feb6cbd518ef8ce Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Mon, 9 Mar 2026 15:41:47 +0100 Subject: [PATCH 18/25] DEFI-2667: add test cases --- .../tests/canister_snapshots.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/rs/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/tests/canister_snapshots.rs index 334951a8f65b..ef3cd32493e1 100644 --- a/rs/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/tests/canister_snapshots.rs @@ -1104,6 +1104,16 @@ mod snapshot_visibility { sender: not_a_controller, expected_result: Ok(()), }, + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: allowed_viewer, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: not_allowed_viewer, + expected_result: Ok(()), + }, TestCase { snapshot_visibility: SnapshotVisibility::Controllers, sender: controller, @@ -1114,11 +1124,26 @@ mod snapshot_visibility { sender: not_a_controller, expected_result: not_allowed_error(¬_a_controller), }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: allowed_viewer, + expected_result: not_allowed_error(&allowed_viewer), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: not_allowed_viewer, + expected_result: not_allowed_error(¬_allowed_viewer), + }, TestCase { snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), sender: allowed_viewer, expected_result: Ok(()), }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), + sender: not_a_controller, + expected_result: not_allowed_error(¬_a_controller), + }, TestCase { snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), sender: not_allowed_viewer, From a0af6bda6c839b7f336e8ddd9edeea497715c7a3 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 12 Mar 2026 07:23:41 +0100 Subject: [PATCH 19/25] DEFI-2667: undo governance changes --- .../clients/src/update_settings.rs | 10 ---- rs/nns/governance/api/src/types.rs | 45 ----------------- rs/nns/governance/canister/governance.did | 1 - .../ic_nns_governance/pb/v1/governance.proto | 10 ---- .../src/gen/ic_nns_governance.pb.v1.rs | 48 ------------------- rs/nns/governance/src/pb/conversions/mod.rs | 2 - .../src/proposals/update_canister_settings.rs | 35 +------------- rs/nns/handlers/root/impl/canister/root.did | 6 --- .../src/update_canister_settings.rs | 2 - rs/nns/nns.bzl | 2 +- rs/registry/admin/bin/main.rs | 16 +------ rs/registry/admin/bin/types.rs | 8 ---- .../api/src/ic_sns_governance.pb.v1.rs | 1 - rs/sns/governance/canister/governance.did | 1 - .../ic_sns_governance/pb/v1/governance.proto | 11 ----- .../src/gen/ic_sns_governance.pb.v1.rs | 46 ------------------ rs/sns/governance/src/pb/conversions.rs | 2 - rs/sns/governance/src/proposal.rs | 16 ++----- rs/sns/governance/src/sns_root_types.rs | 46 ------------------ rs/sns/governance/src/types.rs | 2 - .../src/manage_dapp_canister_settings.rs | 3 +- rs/sns/root/canister/root.did | 1 - .../root/proto/ic_sns_root/pb/v1/root.proto | 11 ----- rs/sns/root/src/gen/ic_sns_root.pb.v1.rs | 46 ------------------ rs/sns/root/src/lib.rs | 6 +-- rs/sns/root/src/pb/mod.rs | 16 ------- 26 files changed, 9 insertions(+), 384 deletions(-) diff --git a/rs/nervous_system/clients/src/update_settings.rs b/rs/nervous_system/clients/src/update_settings.rs index 7990095d03a9..ebf90f793d83 100644 --- a/rs/nervous_system/clients/src/update_settings.rs +++ b/rs/nervous_system/clients/src/update_settings.rs @@ -22,15 +22,6 @@ pub enum LogVisibility { Public, } -#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default, CandidType, Deserialize)] -pub enum SnapshotVisibility { - #[default] - #[serde(rename = "controllers")] - Controllers, - #[serde(rename = "public")] - Public, -} - /// The CanisterSettings struct as defined in the ic-interface-spec /// https://internetcomputer.org/docs/current/references/ic-interface-spec/#ic-candid #[derive(Clone, Eq, PartialEq, Hash, Debug, Default, CandidType, Deserialize)] @@ -41,7 +32,6 @@ pub struct CanisterSettings { pub freezing_threshold: Option, pub reserved_cycles_limit: Option, pub log_visibility: Option, - pub snapshot_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, } diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index a393b696979f..5e6031a3b2d5 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -2649,7 +2649,6 @@ pub mod update_canister_settings { pub log_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, - pub snapshot_visibility: Option, } /// Log visibility of a canister. #[derive( @@ -2695,50 +2694,6 @@ pub mod update_canister_settings { } } } - /// Snapshot visibility of a canister. - #[derive( - candid::CandidType, - candid::Deserialize, - serde::Serialize, - Clone, - Copy, - Debug, - PartialEq, - Eq, - Hash, - PartialOrd, - Ord, - )] - #[repr(i32)] - pub enum SnapshotVisibility { - Unspecified = 0, - /// Snapshots are visible to the controllers of the dapp canister. - Controllers = 1, - /// Snapshots are visible to the public. - Public = 2, - } - impl SnapshotVisibility { - /// String value of the enum field names used in the ProtoBuf definition. - /// - /// The values are not transformed in any way and thus are considered stable - /// (if the ProtoBuf definition does not change) and safe for programmatic use. - pub fn as_str_name(&self) -> &'static str { - match self { - SnapshotVisibility::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", - SnapshotVisibility::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", - SnapshotVisibility::Public => "SNAPSHOT_VISIBILITY_PUBLIC", - } - } - /// Creates an enum from field names used in the ProtoBuf definition. - pub fn from_str_name(value: &str) -> Option { - match value { - "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), - "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), - "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), - _ => None, - } - } - } } #[derive( candid::CandidType, candid::Deserialize, serde::Serialize, Clone, PartialEq, Debug, Default, diff --git a/rs/nns/governance/canister/governance.did b/rs/nns/governance/canister/governance.did index 85b53d434d57..928179d3d6e1 100644 --- a/rs/nns/governance/canister/governance.did +++ b/rs/nns/governance/canister/governance.did @@ -66,7 +66,6 @@ type CanisterSettings = record { freezing_threshold : opt nat64; controllers : opt Controllers; log_visibility : opt int32; - snapshot_visibility : opt int32; wasm_memory_limit : opt nat64; memory_allocation : opt nat64; compute_allocation : opt nat64; diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 08bb20e6db1e..780110006a71 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -1916,15 +1916,6 @@ message UpdateCanisterSettings { LOG_VISIBILITY_PUBLIC = 2; } - // Snapshot visibility of a canister. - enum SnapshotVisibility { - SNAPSHOT_VISIBILITY_UNSPECIFIED = 0; - // Snapshots are visible to the controllers of the dapp canister. - SNAPSHOT_VISIBILITY_CONTROLLERS = 1; - // Snapshots are visible to the public. - SNAPSHOT_VISIBILITY_PUBLIC = 2; - } - // The controllers of the canister. We use a message to wrap the repeated field because prost does // not generate `Option>` for repeated fields. message Controllers { @@ -1942,7 +1933,6 @@ message UpdateCanisterSettings { optional LogVisibility log_visibility = 5; optional uint64 wasm_memory_limit = 6; optional uint64 wasm_memory_threshold = 7; - optional SnapshotVisibility snapshot_visibility = 8; } // The settings to update. Required. diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index 762be910c013..ef17454fcc3c 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -2802,8 +2802,6 @@ pub mod update_canister_settings { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "7")] pub wasm_memory_threshold: ::core::option::Option, - #[prost(enumeration = "SnapshotVisibility", optional, tag = "8")] - pub snapshot_visibility: ::core::option::Option, } /// Log visibility of a canister. #[derive( @@ -2851,52 +2849,6 @@ pub mod update_canister_settings { } } } - /// Snapshot visibility of a canister. - #[derive( - candid::CandidType, - candid::Deserialize, - serde::Serialize, - comparable::Comparable, - Clone, - Copy, - Debug, - PartialEq, - Eq, - Hash, - PartialOrd, - Ord, - ::prost::Enumeration, - )] - #[repr(i32)] - pub enum SnapshotVisibility { - Unspecified = 0, - /// Snapshots are visible to the controllers of the dapp canister. - Controllers = 1, - /// Snapshots are visible to the public. - Public = 2, - } - impl SnapshotVisibility { - /// String value of the enum field names used in the ProtoBuf definition. - /// - /// The values are not transformed in any way and thus are considered stable - /// (if the ProtoBuf definition does not change) and safe for programmatic use. - pub fn as_str_name(&self) -> &'static str { - match self { - Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", - Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", - Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", - } - } - /// Creates an enum from field names used in the ProtoBuf definition. - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), - "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), - "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), - _ => None, - } - } - } } #[derive( candid::CandidType, diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index ca9f233ca93c..346a4021c9c7 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -2761,7 +2761,6 @@ impl From log_visibility: item.log_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, - snapshot_visibility: item.snapshot_visibility, } } } @@ -2778,7 +2777,6 @@ impl From log_visibility: item.log_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, - snapshot_visibility: item.snapshot_visibility, } } } diff --git a/rs/nns/governance/src/proposals/update_canister_settings.rs b/rs/nns/governance/src/proposals/update_canister_settings.rs index e942b95e92ba..309aba0dd255 100644 --- a/rs/nns/governance/src/proposals/update_canister_settings.rs +++ b/rs/nns/governance/src/proposals/update_canister_settings.rs @@ -1,7 +1,7 @@ use crate::{ pb::v1::{ GovernanceError, SelfDescribingValue, Topic, UpdateCanisterSettings, - update_canister_settings::{CanisterSettings, LogVisibility, SnapshotVisibility}, + update_canister_settings::{CanisterSettings, LogVisibility}, }, proposals::{ call_canister::CallCanister, @@ -15,7 +15,6 @@ use candid::{Encode, Nat}; use ic_base_types::CanisterId; use ic_nervous_system_clients::update_settings::{ CanisterSettings as RootCanisterSettings, LogVisibility as RootLogVisibility, - SnapshotVisibility as RootSnapshotVisibility, }; use ic_nns_constants::{LIFELINE_CANISTER_ID, ROOT_CANISTER_ID}; use ic_nns_handler_root_interface::UpdateCanisterSettingsRequest; @@ -55,19 +54,6 @@ impl UpdateCanisterSettings { } } - fn valid_snapshot_visibility( - snapshot_visibility_i32: i32, - ) -> Result { - let snapshot_visibility = SnapshotVisibility::try_from(snapshot_visibility_i32); - match snapshot_visibility { - Ok(SnapshotVisibility::Controllers) => Ok(RootSnapshotVisibility::Controllers), - Ok(SnapshotVisibility::Public) => Ok(RootSnapshotVisibility::Public), - Ok(SnapshotVisibility::Unspecified) | Err(_) => Err(invalid_proposal_error(&format!( - "Invalid snapshot visibility {snapshot_visibility_i32}" - ))), - } - } - fn valid_canister_settings(&self) -> Result { let settings = self .settings @@ -79,7 +65,6 @@ impl UpdateCanisterSettings { && settings.memory_allocation.is_none() && settings.freezing_threshold.is_none() && settings.log_visibility.is_none() - && settings.snapshot_visibility.is_none() && settings.wasm_memory_limit.is_none() && settings.wasm_memory_threshold.is_none() { @@ -101,12 +86,6 @@ impl UpdateCanisterSettings { Some(log_visibility) => Some(Self::valid_log_visibility(log_visibility)?), None => None, }; - let snapshot_visibility = match settings.snapshot_visibility { - Some(snapshot_visibility) => { - Some(Self::valid_snapshot_visibility(snapshot_visibility)?) - } - None => None, - }; // Reserved cycles limit is not supported yet. let reserved_cycles_limit = None; Ok(RootCanisterSettings { @@ -116,7 +95,6 @@ impl UpdateCanisterSettings { freezing_threshold, wasm_memory_limit, log_visibility, - snapshot_visibility, reserved_cycles_limit, wasm_memory_threshold, }) @@ -182,7 +160,6 @@ impl From for SelfDescribingValue { log_visibility, wasm_memory_limit, wasm_memory_threshold, - snapshot_visibility, } = settings; // Flatten the nested `controllers` field. More specifically, get rid of the intermediate @@ -192,8 +169,6 @@ impl From for SelfDescribingValue { let controllers = controllers.map(|controllers| controllers.controllers); let log_visibility = log_visibility.map(SelfDescribingProstEnum::::new); - let snapshot_visibility = - snapshot_visibility.map(SelfDescribingProstEnum::::new); ValueBuilder::new() .add_field("controllers", controllers) @@ -203,7 +178,6 @@ impl From for SelfDescribingValue { .add_field("wasm_memory_limit", wasm_memory_limit) .add_field("wasm_memory_threshold", wasm_memory_threshold) .add_field("log_visibility", log_visibility) - .add_field("snapshot_visibility", snapshot_visibility) .build() } } @@ -311,7 +285,6 @@ mod tests { compute_allocation: Some(10), freezing_threshold: Some(100), log_visibility: Some(LogVisibility::Public as i32), - snapshot_visibility: Some(SnapshotVisibility::Public as i32), }), }; @@ -342,7 +315,6 @@ mod tests { compute_allocation: Some(Nat::from(10u64)), freezing_threshold: Some(Nat::from(100u64)), log_visibility: Some(RootLogVisibility::Public), - snapshot_visibility: Some(RootSnapshotVisibility::Public), reserved_cycles_limit: None, wasm_memory_threshold: Some(Nat::from(1u64 << 30)), } @@ -365,7 +337,6 @@ mod tests { compute_allocation: Some(10), freezing_threshold: Some(100), log_visibility: Some(LogVisibility::Public as i32), - snapshot_visibility: Some(SnapshotVisibility::Public as i32), }), }; @@ -394,7 +365,6 @@ mod tests { compute_allocation: Some(Nat::from(10u64)), freezing_threshold: Some(Nat::from(100u64)), log_visibility: Some(RootLogVisibility::Public), - snapshot_visibility: Some(RootSnapshotVisibility::Public), reserved_cycles_limit: None, wasm_memory_threshold: Some(Nat::from(1u64 << 30)), } @@ -440,7 +410,6 @@ mod tests { compute_allocation: Some(10), freezing_threshold: Some(100), log_visibility: Some(LogVisibility::Public as i32), - snapshot_visibility: Some(SnapshotVisibility::Public as i32), }), }; @@ -462,7 +431,6 @@ mod tests { "compute_allocation".to_string() => SelfDescribingValue::from(10_u64), "freezing_threshold".to_string() => SelfDescribingValue::from(100_u64), "log_visibility".to_string() => SelfDescribingValue::from("Public"), - "snapshot_visibility".to_string() => SelfDescribingValue::from("Public"), }), }) ); @@ -493,7 +461,6 @@ mod tests { "wasm_memory_limit".to_string() => SelfDescribingValue::Null, "wasm_memory_threshold".to_string() => SelfDescribingValue::Null, "log_visibility".to_string() => SelfDescribingValue::Null, - "snapshot_visibility".to_string() => SelfDescribingValue::Null, }), }) ); diff --git a/rs/nns/handlers/root/impl/canister/root.did b/rs/nns/handlers/root/impl/canister/root.did index bed3bf518e4c..bc64fac0d44d 100644 --- a/rs/nns/handlers/root/impl/canister/root.did +++ b/rs/nns/handlers/root/impl/canister/root.did @@ -27,7 +27,6 @@ type CanisterSettings = record { controllers : opt vec principal; reserved_cycles_limit : opt nat; log_visibility : opt LogVisibility; - snapshot_visibility : opt SnapshotVisibility; wasm_memory_limit : opt nat; memory_allocation : opt nat; compute_allocation : opt nat; @@ -113,11 +112,6 @@ type LogVisibility = variant { public; }; -type SnapshotVisibility = variant { - controllers; - public; -}; - type CanisterStatusLogVisibility = variant { controllers; public; diff --git a/rs/nns/integration_tests/src/update_canister_settings.rs b/rs/nns/integration_tests/src/update_canister_settings.rs index 6e1da43e0de0..8535bba04fab 100644 --- a/rs/nns/integration_tests/src/update_canister_settings.rs +++ b/rs/nns/integration_tests/src/update_canister_settings.rs @@ -7,7 +7,6 @@ use ic_nns_governance_api::{ manage_neuron_response::Command, update_canister_settings::{ CanisterSettings, Controllers, LogVisibility as GovernanceLogVisibility, - SnapshotVisibility as GovernanceSnapshotVisibility, }, }; use ic_nns_test_utils::{ @@ -94,7 +93,6 @@ fn test_update_canister_settings_proposal( freezing_threshold: Some(target_freezing_threshold), wasm_memory_limit: Some(target_wasm_memory_limit), log_visibility: Some(GovernanceLogVisibility::Public as i32), - snapshot_visibility: Some(GovernanceSnapshotVisibility::Public as i32), wasm_memory_threshold: Some(target_wasm_memory_threshold), }), }, diff --git a/rs/nns/nns.bzl b/rs/nns/nns.bzl index f366efeaa215..4d19b381e178 100644 --- a/rs/nns/nns.bzl +++ b/rs/nns/nns.bzl @@ -21,7 +21,7 @@ but other things could be added here later. CANISTER_NAME_TO_MAX_COMPRESSED_WASM_SIZE_E5_BYTES = { "cycles-minting-canister.wasm.gz": 6, "genesis-token-canister.wasm.gz": 3, - "governance-canister.wasm.gz": 22, + "governance-canister.wasm.gz": 21, "governance-canister_test.wasm.gz": 23, "registry-canister.wasm.gz": 16, "root-canister.wasm.gz": 5, diff --git a/rs/registry/admin/bin/main.rs b/rs/registry/admin/bin/main.rs index a5d8ef4011cd..9d9055b3e225 100644 --- a/rs/registry/admin/bin/main.rs +++ b/rs/registry/admin/bin/main.rs @@ -64,7 +64,6 @@ use ic_nns_governance_api::{ subnet_rental::{RentalConditionId, SubnetRentalRequest}, update_canister_settings::{ CanisterSettings, Controllers, LogVisibility as GovernanceLogVisibility, - SnapshotVisibility as GovernanceSnapshotVisibility, }, }; use ic_nns_handler_root::root_proposals::{GovernanceUpgradeRootProposal, RootProposalBallot}; @@ -168,8 +167,8 @@ use std::{ }; use types::{ LogVisibility, NodeDetails, ProposalAction, ProposalMetadata, ProposalPayload, - ProvisionalWhitelistRecord, Registry, RegistryRecord, RegistryValue, SnapshotVisibility, - SubnetDescriptor, SubnetRecord, + ProvisionalWhitelistRecord, Registry, RegistryRecord, RegistryValue, SubnetDescriptor, + SubnetRecord, }; use update_subnet::ProposeToUpdateSubnetCmd; use url::Url; @@ -1549,9 +1548,6 @@ struct ProposeToUpdateCanisterSettingsCmd { #[clap(long)] /// If set, it will update the canister's wasm memory threshold to this value. wasm_memory_threshold: Option, - #[clap(long)] - /// If set, it will update the canister's snapshot visibility to this value. - snapshot_visibility: Option, } impl ProposalTitle for ProposeToUpdateCanisterSettingsCmd { @@ -1587,13 +1583,6 @@ impl ProposalAction for ProposeToUpdateCanisterSettingsCmd { Some(LogVisibility::Public) => Some(GovernanceLogVisibility::Public as i32), None => None, }; - let snapshot_visibility = match self.snapshot_visibility { - Some(SnapshotVisibility::Controllers) => { - Some(GovernanceSnapshotVisibility::Controllers as i32) - } - Some(SnapshotVisibility::Public) => Some(GovernanceSnapshotVisibility::Public as i32), - None => None, - }; let update_settings = UpdateCanisterSettings { canister_id, @@ -1605,7 +1594,6 @@ impl ProposalAction for ProposeToUpdateCanisterSettingsCmd { wasm_memory_limit, log_visibility, wasm_memory_threshold, - snapshot_visibility, }), }; diff --git a/rs/registry/admin/bin/types.rs b/rs/registry/admin/bin/types.rs index c964afbab6fc..be4cea983e59 100644 --- a/rs/registry/admin/bin/types.rs +++ b/rs/registry/admin/bin/types.rs @@ -250,14 +250,6 @@ pub enum LogVisibility { Public, } -#[derive(Copy, Clone, Eq, PartialEq, Debug, Deserialize, EnumString, Serialize)] -pub enum SnapshotVisibility { - #[strum(serialize = "controllers")] - Controllers, - #[strum(serialize = "public")] - Public, -} - /// Trait to extract the payload for each proposal type. /// This trait is async as building some payloads requires async calls. #[async_trait] 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 21b9f0a8a803..7045e11ab8ff 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 @@ -591,7 +591,6 @@ pub struct ManageDappCanisterSettings { pub log_visibility: Option, pub wasm_memory_limit: Option, pub wasm_memory_threshold: Option, - pub snapshot_visibility: Option, } /// Unlike `Governance.Version`, this message has optional fields and is the recommended one /// to use in APIs that can evolve. For example, the SNS Governance could eventually support diff --git a/rs/sns/governance/canister/governance.did b/rs/sns/governance/canister/governance.did index 2fa7a484d40c..71e5e5c3d2cf 100644 --- a/rs/sns/governance/canister/governance.did +++ b/rs/sns/governance/canister/governance.did @@ -458,7 +458,6 @@ type ManageDappCanisterSettings = record { canister_ids : vec principal; reserved_cycles_limit : opt nat64; log_visibility : opt int32; - snapshot_visibility : opt int32; wasm_memory_limit : opt nat64; memory_allocation : opt nat64; compute_allocation : opt nat64; diff --git a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto index f7d5200e14ef..0dd3f12e724d 100644 --- a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto +++ b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto @@ -547,16 +547,6 @@ enum LogVisibility { LOG_VISIBILITY_PUBLIC = 2; } -enum SnapshotVisibility { - SNAPSHOT_VISIBILITY_UNSPECIFIED = 0; - - // Snapshots are visible to the controllers of the dapp canister. - SNAPSHOT_VISIBILITY_CONTROLLERS = 1; - - // Snapshots are visible to the public. - SNAPSHOT_VISIBILITY_PUBLIC = 2; -} - // A proposal to manage the settings of one or more dapp canisters. message ManageDappCanisterSettings { // The canister IDs of the dapp canisters to be modified. @@ -572,7 +562,6 @@ message ManageDappCanisterSettings { optional uint64 wasm_memory_limit = 7; optional uint64 wasm_memory_threshold = 8; - optional SnapshotVisibility snapshot_visibility = 9; } // Unlike `Governance.Version`, this message has optional fields and is the recommended one diff --git a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs index 3765d38db9a3..5d9e821d2a31 100644 --- a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs +++ b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs @@ -893,8 +893,6 @@ pub struct ManageDappCanisterSettings { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "8")] pub wasm_memory_threshold: ::core::option::Option, - #[prost(enumeration = "SnapshotVisibility", optional, tag = "9")] - pub snapshot_visibility: ::core::option::Option, } /// Unlike `Governance.Version`, this message has optional fields and is the recommended one /// to be used in APIs that can evolve. For example, the SNS Governance could eventually support @@ -4503,50 +4501,6 @@ impl LogVisibility { ::prost::Enumeration, )] #[repr(i32)] -pub enum SnapshotVisibility { - Unspecified = 0, - /// Snapshots are visible to the controllers of the dapp canister. - Controllers = 1, - /// Snapshots are visible to the public. - Public = 2, -} -impl SnapshotVisibility { - /// String value of the enum field names used in the ProtoBuf definition. - /// - /// The values are not transformed in any way and thus are considered stable - /// (if the ProtoBuf definition does not change) and safe for programmatic use. - pub fn as_str_name(&self) -> &'static str { - match self { - Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", - Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", - Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", - } - } - /// Creates an enum from field names used in the ProtoBuf definition. - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), - "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), - "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), - _ => None, - } - } -} -#[derive( - candid::CandidType, - candid::Deserialize, - comparable::Comparable, - Clone, - Copy, - Debug, - PartialEq, - Eq, - Hash, - PartialOrd, - Ord, - ::prost::Enumeration, -)] -#[repr(i32)] pub enum ProposalDecisionStatus { Unspecified = 0, /// The proposal is open for voting and a decision (adopt/reject) has yet to be made. diff --git a/rs/sns/governance/src/pb/conversions.rs b/rs/sns/governance/src/pb/conversions.rs index 837d0da75d67..85b70535c3fd 100644 --- a/rs/sns/governance/src/pb/conversions.rs +++ b/rs/sns/governance/src/pb/conversions.rs @@ -804,7 +804,6 @@ impl From for pb_api::ManageDappCanisterSettings freezing_threshold: item.freezing_threshold, reserved_cycles_limit: item.reserved_cycles_limit, log_visibility: item.log_visibility, - snapshot_visibility: item.snapshot_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, } @@ -819,7 +818,6 @@ impl From for pb::ManageDappCanisterSettings freezing_threshold: item.freezing_threshold, reserved_cycles_limit: item.reserved_cycles_limit, log_visibility: item.log_visibility, - snapshot_visibility: item.snapshot_visibility, wasm_memory_limit: item.wasm_memory_limit, wasm_memory_threshold: item.wasm_memory_threshold, } diff --git a/rs/sns/governance/src/proposal.rs b/rs/sns/governance/src/proposal.rs index 297a32ec15b0..2a33b926126c 100644 --- a/rs/sns/governance/src/proposal.rs +++ b/rs/sns/governance/src/proposal.rs @@ -17,9 +17,9 @@ use crate::{ ManageDappCanisterSettings, ManageLedgerParameters, ManageSnsMetadata, MintSnsTokens, Motion, NervousSystemFunction, NervousSystemParameters, Proposal, ProposalData, ProposalDecisionStatus, ProposalId, ProposalRewardStatus, RegisterDappCanisters, - RegisterExtension, SetTopicsForCustomProposals, SnapshotVisibility, SnsVersion, Tally, - Topic, Topic as TopicPb, TransferSnsTreasuryFunds, UpgradeExtension, - UpgradeSnsControlledCanister, UpgradeSnsToNextVersion, Valuation as ValuationPb, Vote, + RegisterExtension, SetTopicsForCustomProposals, SnsVersion, Tally, Topic, Topic as TopicPb, + TransferSnsTreasuryFunds, UpgradeExtension, UpgradeSnsControlledCanister, + UpgradeSnsToNextVersion, Valuation as ValuationPb, Vote, governance::{SnsMetadata, Version}, governance_error::ErrorType, nervous_system_function::{FunctionType, GenericNervousSystemFunction}, @@ -1851,13 +1851,6 @@ fn validate_and_render_manage_dapp_canister_settings( ); no_change = false; } - if let Some(snapshot_visibility) = &manage_dapp_canister_settings.snapshot_visibility { - render += &format!( - "# Set snapshot visibility to: {:?} \n", - SnapshotVisibility::try_from(*snapshot_visibility).unwrap_or_default() - ); - no_change = false; - } if let Some(wasm_memory_limit) = &manage_dapp_canister_settings.wasm_memory_limit { render += &format!("# Set Wasm memory limit to: {wasm_memory_limit}\n"); no_change = false; @@ -5143,7 +5136,6 @@ Version { freezing_threshold: Some(1_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(LogVisibility::Public as i32), - snapshot_visibility: Some(SnapshotVisibility::Public as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }) @@ -5275,7 +5267,6 @@ Payload rendering here"# freezing_threshold: Some(1_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(LogVisibility::Public as i32), - snapshot_visibility: Some(SnapshotVisibility::Public as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }) @@ -5291,7 +5282,6 @@ Payload rendering here"# # Set freezing threshold to: 1000 seconds\n\ # Set reserved cycles limit to: 1000000000000 \n\ # Set log visibility to: Public \n\ - # Set snapshot visibility to: Public \n\ # Set Wasm memory limit to: 1000000000\n" ); } diff --git a/rs/sns/governance/src/sns_root_types.rs b/rs/sns/governance/src/sns_root_types.rs index 009fbd012a20..2ce6e1540d0b 100644 --- a/rs/sns/governance/src/sns_root_types.rs +++ b/rs/sns/governance/src/sns_root_types.rs @@ -358,8 +358,6 @@ pub struct ManageDappCanisterSettingsRequest { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "8")] pub wasm_memory_threshold: ::core::option::Option, - #[prost(enumeration = "SnapshotVisibility", optional, tag = "9")] - pub snapshot_visibility: ::core::option::Option, } #[derive( candid::CandidType, @@ -418,47 +416,3 @@ impl LogVisibility { } } } -#[derive( - candid::CandidType, - candid::Deserialize, - comparable::Comparable, - Clone, - Copy, - Debug, - PartialEq, - Eq, - Hash, - PartialOrd, - Ord, - ::prost::Enumeration, -)] -#[repr(i32)] -pub enum SnapshotVisibility { - Unspecified = 0, - /// Snapshots are visible to the controllers of the dapp canister. - Controllers = 1, - /// Snapshots are visible to the public. - Public = 2, -} -impl SnapshotVisibility { - /// String value of the enum field names used in the ProtoBuf definition. - /// - /// The values are not transformed in any way and thus are considered stable - /// (if the ProtoBuf definition does not change) and safe for programmatic use. - pub fn as_str_name(&self) -> &'static str { - match self { - Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", - Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", - Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", - } - } - /// Creates an enum from field names used in the ProtoBuf definition. - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), - "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), - "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), - _ => None, - } - } -} diff --git a/rs/sns/governance/src/types.rs b/rs/sns/governance/src/types.rs index f72f4cd9f6e7..e98eca076e41 100644 --- a/rs/sns/governance/src/types.rs +++ b/rs/sns/governance/src/types.rs @@ -2602,7 +2602,6 @@ impl From for ManageDappCanisterSettingsRequest { freezing_threshold, reserved_cycles_limit, log_visibility, - snapshot_visibility, wasm_memory_limit, wasm_memory_threshold, } = manage_dapp_canister_settings; @@ -2614,7 +2613,6 @@ impl From for ManageDappCanisterSettingsRequest { freezing_threshold, reserved_cycles_limit, log_visibility, - snapshot_visibility, wasm_memory_limit, wasm_memory_threshold, } diff --git a/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs b/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs index 5b57bcb27a79..26b6d23dab05 100644 --- a/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs +++ b/rs/sns/integration_tests/src/manage_dapp_canister_settings.rs @@ -11,7 +11,7 @@ use ic_nns_test_utils::state_test_helpers::{ }; use ic_sns_governance::pb::v1::{ LogVisibility, ManageDappCanisterSettings, NervousSystemParameters, NeuronPermissionList, - NeuronPermissionType, Proposal, SnapshotVisibility, proposal::Action, + NeuronPermissionType, Proposal, proposal::Action, }; use ic_sns_test_utils::{ itest_helpers::SnsTestsInitPayloadBuilder, @@ -121,7 +121,6 @@ fn test_manage_dapp_canister_settings_successful() { freezing_threshold: Some(0), reserved_cycles_limit: Some(0), log_visibility: Some(LogVisibility::Controllers as i32), - snapshot_visibility: Some(SnapshotVisibility::Controllers as i32), wasm_memory_limit: Some(2_000_000_000), wasm_memory_threshold: Some(0), }, diff --git a/rs/sns/root/canister/root.did b/rs/sns/root/canister/root.did index 87630b5320c5..2acd56a5e407 100644 --- a/rs/sns/root/canister/root.did +++ b/rs/sns/root/canister/root.did @@ -141,7 +141,6 @@ type ManageDappCanisterSettingsRequest = record { canister_ids : vec principal; reserved_cycles_limit : opt nat64; log_visibility : opt int32; - snapshot_visibility : opt int32; wasm_memory_limit : opt nat64; memory_allocation : opt nat64; compute_allocation : opt nat64; diff --git a/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto b/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto index 479b6aa6bb0f..00d31fc5e03b 100644 --- a/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto +++ b/rs/sns/root/proto/ic_sns_root/pb/v1/root.proto @@ -154,16 +154,6 @@ enum LogVisibility { LOG_VISIBILITY_PUBLIC = 2; } -enum SnapshotVisibility { - SNAPSHOT_VISIBILITY_UNSPECIFIED = 0; - - // Snapshots are visible to the controllers of the dapp canister. - SNAPSHOT_VISIBILITY_CONTROLLERS = 1; - - // Snapshots are visible to the public. - SNAPSHOT_VISIBILITY_PUBLIC = 2; -} - message ManageDappCanisterSettingsRequest { repeated ic_base_types.pb.v1.PrincipalId canister_ids = 1; optional uint64 compute_allocation = 2; @@ -173,7 +163,6 @@ message ManageDappCanisterSettingsRequest { optional LogVisibility log_visibility = 6; optional uint64 wasm_memory_limit = 7; optional uint64 wasm_memory_threshold = 8; - optional SnapshotVisibility snapshot_visibility = 9; } message ManageDappCanisterSettingsResponse { diff --git a/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs b/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs index 009fbd012a20..2ce6e1540d0b 100644 --- a/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs +++ b/rs/sns/root/src/gen/ic_sns_root.pb.v1.rs @@ -358,8 +358,6 @@ pub struct ManageDappCanisterSettingsRequest { pub wasm_memory_limit: ::core::option::Option, #[prost(uint64, optional, tag = "8")] pub wasm_memory_threshold: ::core::option::Option, - #[prost(enumeration = "SnapshotVisibility", optional, tag = "9")] - pub snapshot_visibility: ::core::option::Option, } #[derive( candid::CandidType, @@ -418,47 +416,3 @@ impl LogVisibility { } } } -#[derive( - candid::CandidType, - candid::Deserialize, - comparable::Comparable, - Clone, - Copy, - Debug, - PartialEq, - Eq, - Hash, - PartialOrd, - Ord, - ::prost::Enumeration, -)] -#[repr(i32)] -pub enum SnapshotVisibility { - Unspecified = 0, - /// Snapshots are visible to the controllers of the dapp canister. - Controllers = 1, - /// Snapshots are visible to the public. - Public = 2, -} -impl SnapshotVisibility { - /// String value of the enum field names used in the ProtoBuf definition. - /// - /// The values are not transformed in any way and thus are considered stable - /// (if the ProtoBuf definition does not change) and safe for programmatic use. - pub fn as_str_name(&self) -> &'static str { - match self { - Self::Unspecified => "SNAPSHOT_VISIBILITY_UNSPECIFIED", - Self::Controllers => "SNAPSHOT_VISIBILITY_CONTROLLERS", - Self::Public => "SNAPSHOT_VISIBILITY_PUBLIC", - } - } - /// Creates an enum from field names used in the ProtoBuf definition. - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "SNAPSHOT_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified), - "SNAPSHOT_VISIBILITY_CONTROLLERS" => Some(Self::Controllers), - "SNAPSHOT_VISIBILITY_PUBLIC" => Some(Self::Public), - _ => None, - } - } -} diff --git a/rs/sns/root/src/lib.rs b/rs/sns/root/src/lib.rs index 7769ded203bc..9ca326c1e92b 100644 --- a/rs/sns/root/src/lib.rs +++ b/rs/sns/root/src/lib.rs @@ -18,7 +18,7 @@ use ic_nervous_system_clients::{ canister_id_record::CanisterIdRecord, canister_status::CanisterStatusResultV2, management_canister_client::ManagementCanisterClient, - update_settings::{CanisterSettings, LogVisibility, SnapshotVisibility, UpdateSettings}, + update_settings::{CanisterSettings, LogVisibility, UpdateSettings}, }; use ic_nervous_system_runtime::{CdkRuntime, Runtime}; use ic_sns_swap::pb::v1::GetCanisterStatusRequest; @@ -194,7 +194,6 @@ impl ValidatedManageDappCanisterSettingsRequest { freezing_threshold: request.freezing_threshold.map(Nat::from), reserved_cycles_limit: request.reserved_cycles_limit.map(Nat::from), log_visibility: LogVisibility::try_from(request.log_visibility()).ok(), - snapshot_visibility: SnapshotVisibility::try_from(request.snapshot_visibility()).ok(), wasm_memory_limit: request.wasm_memory_limit.map(Nat::from), wasm_memory_threshold: request.wasm_memory_threshold.map(Nat::from), }; @@ -2381,7 +2380,6 @@ mod tests { freezing_threshold: Some(100_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(crate::pb::v1::LogVisibility::Controllers as i32), - snapshot_visibility: Some(crate::pb::v1::SnapshotVisibility::Controllers as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }; @@ -2408,7 +2406,6 @@ mod tests { freezing_threshold: Some(Nat::from(100_000u64)), reserved_cycles_limit: Some(Nat::from(1_000_000_000_000u64)), log_visibility: Some(LogVisibility::Controllers), - snapshot_visibility: Some(SnapshotVisibility::Controllers), wasm_memory_limit: Some(Nat::from(1_000_000_000u64)), wasm_memory_threshold: Some(Nat::from(1_000_000u64)), }, @@ -2429,7 +2426,6 @@ mod tests { freezing_threshold: Some(100_000), reserved_cycles_limit: Some(1_000_000_000_000), log_visibility: Some(crate::pb::v1::LogVisibility::Controllers as i32), - snapshot_visibility: Some(crate::pb::v1::SnapshotVisibility::Controllers as i32), wasm_memory_limit: Some(1_000_000_000), wasm_memory_threshold: Some(1_000_000), }; diff --git a/rs/sns/root/src/pb/mod.rs b/rs/sns/root/src/pb/mod.rs index 34abea189bdc..02dccc001a23 100644 --- a/rs/sns/root/src/pb/mod.rs +++ b/rs/sns/root/src/pb/mod.rs @@ -13,19 +13,3 @@ impl TryFrom for ic_nervous_system_clients::update_settings:: } } } - -impl TryFrom - for ic_nervous_system_clients::update_settings::SnapshotVisibility -{ - type Error = String; - - fn try_from(snapshot_visibility: v1::SnapshotVisibility) -> Result { - match snapshot_visibility { - v1::SnapshotVisibility::Unspecified => { - Err("Unspecified snapshot visibility".to_string()) - } - v1::SnapshotVisibility::Controllers => Ok(Self::Controllers), - v1::SnapshotVisibility::Public => Ok(Self::Public), - } - } -} From 86a627202a37e319ae96ba1acca940748058af13 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 12 Mar 2026 14:39:47 +0100 Subject: [PATCH 20/25] DEFI-2667: Implement rest of logic for `read_canister_snapshot_metadata` and `read_canister_snapshot_data` --- .../src/canister_manager.rs | 43 +-- .../src/execution/common.rs | 4 +- .../src/execution_environment.rs | 4 +- .../tests/canister_snapshots.rs | 248 ++++++++++++++---- rs/state_machine_tests/src/lib.rs | 16 ++ 5 files changed, 240 insertions(+), 75 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 8f36c293b50d..1a92f4aecea0 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -232,8 +232,6 @@ impl CanisterManager { | Ok(Ic00Method::TakeCanisterSnapshot) | Ok(Ic00Method::LoadCanisterSnapshot) | Ok(Ic00Method::DeleteCanisterSnapshot) - | Ok(Ic00Method::ReadCanisterSnapshotMetadata) - | Ok(Ic00Method::ReadCanisterSnapshotData) | Ok(Ic00Method::UploadCanisterSnapshotMetadata) | Ok(Ic00Method::UploadCanisterSnapshotData) => { match effective_canister_id { @@ -259,14 +257,16 @@ impl CanisterManager { } }, - Ok(Ic00Method::ListCanisterSnapshots) => { + Ok(Ic00Method::ListCanisterSnapshots) + | Ok(Ic00Method::ReadCanisterSnapshotMetadata) + | Ok(Ic00Method::ReadCanisterSnapshotData) => { match effective_canister_id { Some(canister_id) => { let canister_state = state.canister_state(&canister_id).ok_or_else(|| UserError::new( ErrorCode::CanisterNotFound, format!("Canister {canister_id} not found"), ))?; - validate_snapshot_visibility(canister_state, &sender.get(), "list canister snapshots")?; + validate_snapshot_visibility(canister_state, &sender.get(), method_name)?; Ok(()) }, None => Err(UserError::new( @@ -2634,10 +2634,11 @@ impl CanisterManager { snapshot_id: SnapshotId, canister: &CanisterState, state: &ReplicatedState, - ) -> Result { - // Check sender is a controller. - validate_controller(canister, &sender)?; - let snapshot = self.get_snapshot(canister.canister_id(), snapshot_id, state)?; + ) -> Result { + validate_snapshot_visibility(canister, &sender, "read canister snapshot metadata")?; + let snapshot = self + .get_snapshot(canister.canister_id(), snapshot_id, state) + .map_err(UserError::from)?; // A snapshot also contains the instruction counter as the last global // (because it is *appended* during WASM instrumentation). // We pop that last global (which is merely an implementation detail) @@ -2682,13 +2683,14 @@ impl CanisterManager { state: &ReplicatedState, subnet_size: usize, round_limits: &mut RoundLimits, - ) -> Result<(ReadCanisterSnapshotDataResponse, NumInstructions), CanisterManagerError> { - // Check sender is a controller. - validate_controller(canister, &sender)?; - let snapshot = self.get_snapshot(canister.canister_id(), snapshot_id, state)?; + ) -> Result<(ReadCanisterSnapshotDataResponse, NumInstructions), UserError> { + validate_snapshot_visibility(canister, &sender, "read_canister_snapshot_data")?; + let snapshot = self + .get_snapshot(canister.canister_id(), snapshot_id, state) + .map_err(UserError::from)?; // Charge upfront for the baseline plus the maximum possible size of the returned slice or fail. - let num_response_bytes = get_response_size(&kind)?; + let num_response_bytes = get_response_size(&kind).map_err(UserError::from)?; let num_instructions = self .config .canister_snapshot_data_baseline_instructions @@ -2703,10 +2705,12 @@ impl CanisterManager { state.get_own_cost_schedule(), ) { - return Err(CanisterManagerError::CanisterSnapshotNotEnoughCycles(err)); + return Err(UserError::from( + CanisterManagerError::CanisterSnapshotNotEnoughCycles(err), + )); }; round_limits.instructions -= as_round_instructions(num_instructions); - let res = match kind { + let res: Result, CanisterManagerError> = match kind { CanisterSnapshotDataKind::StableMemory { offset, size } => { let stable_memory = snapshot.execution_snapshot().stable_memory.clone(); match CanisterSnapshot::get_memory_chunk(stable_memory, offset, size) { @@ -2729,20 +2733,21 @@ impl CanisterManager { } CanisterSnapshotDataKind::WasmChunk { hash } => { let Ok(hash) = ::try_from(hash.clone()) else { - return Err(CanisterManagerError::WasmChunkStoreError { + return Err(UserError::from(CanisterManagerError::WasmChunkStoreError { message: format!("Bytes {hash:02x?} are not a valid WasmChunkHash."), - }); + })); }; let Some(chunk) = snapshot.chunk_store().get_chunk_complete(&hash) else { - return Err(CanisterManagerError::WasmChunkStoreError { + return Err(UserError::from(CanisterManagerError::WasmChunkStoreError { message: format!("WasmChunkHash {hash:02x?} not found."), - }); + })); }; Ok(chunk) } }; res.map(|x| (ReadCanisterSnapshotDataResponse::new(x), num_instructions)) + .map_err(UserError::from) } /// Creates a new snapshot based on the provided metadata and returns the new snapshot ID. diff --git a/rs/execution_environment/src/execution/common.rs b/rs/execution_environment/src/execution/common.rs index 7591ef6c5e36..1e5ce42ec982 100644 --- a/rs/execution_environment/src/execution/common.rs +++ b/rs/execution_environment/src/execution/common.rs @@ -358,14 +358,14 @@ pub(crate) fn validate_controller( pub(crate) fn validate_snapshot_visibility( canister: &CanisterState, caller: &PrincipalId, - err_msg: &str, + method_name: &str, ) -> Result<(), UserError> { if !crate::canister_settings::VisibilitySettings::from(canister.snapshot_visibility()) .has_access(caller, canister.controllers()) { return Err(UserError::new( ErrorCode::CanisterRejectedMessage, - format!("Caller {caller} is not allowed to {err_msg}"), + format!("Caller {caller} is not allowed to call {method_name}"), )); } Ok(()) diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index a8323b237668..c82502cae225 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -2801,7 +2801,7 @@ impl ExecutionEnvironment { round_limits, ) { Ok((result, num_instructions)) => (Ok(Encode!(&result).unwrap()), num_instructions), - Err(err) => (Err(UserError::from(err)), NumInstructions::new(0)), + Err(err) => (Err(err), NumInstructions::new(0)), }; // Put canister back. @@ -2872,7 +2872,7 @@ impl ExecutionEnvironment { .read_snapshot_metadata(sender, snapshot_id, canister, state) { Ok(response) => (Ok(Encode!(&response).unwrap()), NumInstructions::new(0)), - Err(e) => (Err(UserError::from(e)), NumInstructions::new(0)), + Err(e) => (Err(e), NumInstructions::new(0)), } } diff --git a/rs/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/tests/canister_snapshots.rs index db499efe7ec0..e44d23e346a9 100644 --- a/rs/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/tests/canister_snapshots.rs @@ -1072,8 +1072,9 @@ mod snapshot_visibility { use ic_base_types::{CanisterId, PrincipalId}; use ic_error_types::{ErrorCode, UserError}; use ic_management_canister_types_private::{ - BoundedAllowedViewers, CanisterSettingsArgsBuilder, CanisterSnapshotResponse, - ListCanisterSnapshotArgs, SnapshotVisibility, TakeCanisterSnapshotArgs, + BoundedAllowedViewers, CanisterSettingsArgsBuilder, CanisterSnapshotDataKind, + CanisterSnapshotResponse, ListCanisterSnapshotArgs, ReadCanisterSnapshotDataArgs, + ReadCanisterSnapshotMetadataArgs, SnapshotVisibility, TakeCanisterSnapshotArgs, }; use ic_state_machine_tests::{StateMachine, StateMachineBuilder}; use ic_universal_canister::UNIVERSAL_CANISTER_WASM; @@ -1082,18 +1083,138 @@ mod snapshot_visibility { fn test_visibility_of_list_canister_snapshots() { let env = StateMachineBuilder::new().build(); let controller = PrincipalId::new_user_test_id(1); - let not_a_controller = PrincipalId::new_user_test_id(2); - let allowed_viewer = PrincipalId::new_user_test_id(3); - let not_allowed_viewer = PrincipalId::new_user_test_id(4); - let allowed_viewers = BoundedAllowedViewers::new(vec![allowed_viewer]); - fn not_allowed_error(caller: &PrincipalId) -> Result<(), UserError> { - Err(UserError::new( - ErrorCode::CanisterRejectedMessage, - format!("Caller {caller} is not allowed to list canister snapshots"), - )) + + let test_cases = visibility_test_cases( + controller, + PrincipalId::new_user_test_id(2), + PrincipalId::new_user_test_id(3), + PrincipalId::new_user_test_id(4), + |caller| { + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to call list_canister_snapshots"), + )) + }, + ); + + for test_case in &test_cases { + let canister_id = env + .install_canister( + UNIVERSAL_CANISTER_WASM.to_vec(), + vec![], + Some( + CanisterSettingsArgsBuilder::new() + .with_snapshot_visibility(test_case.snapshot_visibility.clone()) + .with_controllers(vec![controller]) + .build(), + ), + ) + .unwrap(); + + test_case.check_list_canister_snapshot(canister_id, &env, &[]); + + let snapshot = env + .take_canister_snapshot(TakeCanisterSnapshotArgs { + canister_id: canister_id.get(), + replace_snapshot: None, + uninstall_code: None, + sender_canister_version: None, + }) + .unwrap(); + + test_case.check_list_canister_snapshot(canister_id, &env, &[snapshot]); + } + } + + #[derive(Debug)] + struct TestCase { + snapshot_visibility: SnapshotVisibility, + sender: PrincipalId, + expected_result: Result<(), UserError>, + } + + impl TestCase { + fn check_list_canister_snapshot( + &self, + canister_id: CanisterId, + env: &StateMachine, + snapshots: &[CanisterSnapshotResponse], + ) { + match &self.expected_result { + Err(err) => { + assert_eq!( + env.list_canister_snapshots_as( + ListCanisterSnapshotArgs::new(canister_id), + self.sender + ) + .as_ref(), + Err(err), + "{self:?}" + ); + } + Ok(()) => { + assert_eq!( + env.list_canister_snapshots_as( + ListCanisterSnapshotArgs::new(canister_id), + self.sender + ), + Ok(snapshots.to_vec()), + "{self:?}" + ); + } + } + } + + fn check_read_canister_snapshot_metadata( + &self, + canister_id: CanisterId, + snapshot_id: ic_base_types::SnapshotId, + env: &StateMachine, + ) { + let args = ReadCanisterSnapshotMetadataArgs::new(canister_id, snapshot_id); + let result = env.read_canister_snapshot_metadata_as(&args, self.sender); + match &self.expected_result { + Err(err) => { + assert_eq!(result.as_ref().err(), Some(err), "{self:?}"); + } + Ok(()) => { + assert!(result.is_ok(), "{self:?}: {}", result.unwrap_err()); + } + } + } + + fn check_read_canister_snapshot_data( + &self, + canister_id: CanisterId, + snapshot_id: ic_base_types::SnapshotId, + env: &StateMachine, + ) { + let args = ReadCanisterSnapshotDataArgs::new( + canister_id, + snapshot_id, + CanisterSnapshotDataKind::StableMemory { offset: 0, size: 0 }, + ); + let result = env.read_canister_snapshot_data_as(&args, self.sender); + match &self.expected_result { + Err(err) => { + assert_eq!(result.as_ref().err(), Some(err), "{self:?}"); + } + Ok(()) => { + assert!(result.is_ok(), "{self:?}: {}", result.unwrap_err()); + } + } } + } - let test_cases = vec![ + fn visibility_test_cases( + controller: PrincipalId, + not_a_controller: PrincipalId, + allowed_viewer: PrincipalId, + not_allowed_viewer: PrincipalId, + not_allowed_error: impl Fn(&PrincipalId) -> Result<(), UserError>, + ) -> Vec { + let allowed_viewers = BoundedAllowedViewers::new(vec![allowed_viewer]); + vec![ TestCase { snapshot_visibility: SnapshotVisibility::Public, sender: controller, @@ -1154,7 +1275,27 @@ mod snapshot_visibility { sender: controller, expected_result: Ok(()), }, - ]; + ] + } + + #[test] + fn test_visibility_of_read_canister_snapshot_metadata() { + let env = StateMachineBuilder::new().build(); + let controller = PrincipalId::new_user_test_id(1); + + let test_cases = visibility_test_cases( + controller, + PrincipalId::new_user_test_id(2), + PrincipalId::new_user_test_id(3), + PrincipalId::new_user_test_id(4), + |caller| { + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to call read_canister_snapshot_metadata"), + )) + }, + ); + for test_case in &test_cases { let canister_id = env .install_canister( @@ -1169,8 +1310,6 @@ mod snapshot_visibility { ) .unwrap(); - test_case.check_list_canister_snapshot(canister_id, &env, &[]); - let snapshot = env .take_canister_snapshot(TakeCanisterSnapshotArgs { canister_id: canister_id.get(), @@ -1180,47 +1319,52 @@ mod snapshot_visibility { }) .unwrap(); - test_case.check_list_canister_snapshot(canister_id, &env, &[snapshot]); + test_case.check_read_canister_snapshot_metadata(canister_id, snapshot.id, &env); } } - #[derive(Debug)] - struct TestCase { - snapshot_visibility: SnapshotVisibility, - sender: PrincipalId, - expected_result: Result<(), UserError>, - } + #[test] + fn test_visibility_of_read_canister_snapshot_data() { + let env = StateMachineBuilder::new().build(); + let controller = PrincipalId::new_user_test_id(1); - impl TestCase { - fn check_list_canister_snapshot( - &self, - canister_id: CanisterId, - env: &StateMachine, - snapshots: &[CanisterSnapshotResponse], - ) { - match &self.expected_result { - Err(err) => { - assert_eq!( - env.list_canister_snapshots_as( - ListCanisterSnapshotArgs::new(canister_id), - self.sender - ) - .as_ref(), - Err(err), - "{self:?}" - ); - } - Ok(()) => { - assert_eq!( - env.list_canister_snapshots_as( - ListCanisterSnapshotArgs::new(canister_id), - self.sender - ), - Ok(snapshots.to_vec()), - "{self:?}" - ); - } - } + let test_cases = visibility_test_cases( + controller, + PrincipalId::new_user_test_id(2), + PrincipalId::new_user_test_id(3), + PrincipalId::new_user_test_id(4), + |caller| { + Err(UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to call read_canister_snapshot_data"), + )) + }, + ); + + for test_case in &test_cases { + let canister_id = env + .install_canister( + UNIVERSAL_CANISTER_WASM.to_vec(), + vec![], + Some( + CanisterSettingsArgsBuilder::new() + .with_snapshot_visibility(test_case.snapshot_visibility.clone()) + .with_controllers(vec![controller]) + .build(), + ), + ) + .unwrap(); + + let snapshot = env + .take_canister_snapshot(TakeCanisterSnapshotArgs { + canister_id: canister_id.get(), + replace_snapshot: None, + uninstall_code: None, + sender_canister_version: None, + }) + .unwrap(); + + test_case.check_read_canister_snapshot_data(canister_id, snapshot.id, &env); } } } diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 38826ad00d47..1da8cb6b67a2 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -3916,6 +3916,14 @@ impl StateMachine { args: &ReadCanisterSnapshotMetadataArgs, ) -> Result { let sender = self.get_controller(&args.get_canister_id()); + self.read_canister_snapshot_metadata_as(args, sender) + } + + pub fn read_canister_snapshot_metadata_as( + &self, + args: &ReadCanisterSnapshotMetadataArgs, + sender: PrincipalId, + ) -> Result { self.execute_ingress_as( sender, ic00::IC_00, @@ -3935,6 +3943,14 @@ impl StateMachine { args: &ReadCanisterSnapshotDataArgs, ) -> Result { let sender = self.get_controller(&args.get_canister_id()); + self.read_canister_snapshot_data_as(args, sender) + } + + pub fn read_canister_snapshot_data_as( + &self, + args: &ReadCanisterSnapshotDataArgs, + sender: PrincipalId, + ) -> Result { self.execute_ingress_as( sender, ic00::IC_00, From 8ee13449909318b94b725ee17190f799060535f9 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 12 Mar 2026 15:37:28 +0100 Subject: [PATCH 21/25] DEFI-2667: refactor test --- .../tests/canister_snapshots.rs | 294 ++++++------------ 1 file changed, 98 insertions(+), 196 deletions(-) diff --git a/rs/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/tests/canister_snapshots.rs index e44d23e346a9..f0f48beaaf31 100644 --- a/rs/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/tests/canister_snapshots.rs @@ -1080,22 +1080,75 @@ mod snapshot_visibility { use ic_universal_canister::UNIVERSAL_CANISTER_WASM; #[test] - fn test_visibility_of_list_canister_snapshots() { + fn test_visibility_of_canister_snapshots() { let env = StateMachineBuilder::new().build(); let controller = PrincipalId::new_user_test_id(1); - - let test_cases = visibility_test_cases( - controller, - PrincipalId::new_user_test_id(2), - PrincipalId::new_user_test_id(3), - PrincipalId::new_user_test_id(4), - |caller| { - Err(UserError::new( - ErrorCode::CanisterRejectedMessage, - format!("Caller {caller} is not allowed to call list_canister_snapshots"), - )) + let not_a_controller = PrincipalId::new_user_test_id(2); + let allowed_viewer = PrincipalId::new_user_test_id(3); + let not_allowed_viewer = PrincipalId::new_user_test_id(4); + let allowed_viewers = BoundedAllowedViewers::new(vec![allowed_viewer]); + let test_cases = vec![ + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: controller, + expected_result: Ok(()), }, - ); + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: not_a_controller, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: allowed_viewer, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Public, + sender: not_allowed_viewer, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: controller, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: not_a_controller, + expected_result: Err(not_a_controller), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: allowed_viewer, + expected_result: Err(allowed_viewer), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::Controllers, + sender: not_allowed_viewer, + expected_result: Err(not_allowed_viewer), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), + sender: allowed_viewer, + expected_result: Ok(()), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), + sender: not_a_controller, + expected_result: Err(not_a_controller), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), + sender: not_allowed_viewer, + expected_result: Err(not_allowed_viewer), + }, + TestCase { + snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers), + sender: controller, + expected_result: Ok(()), + }, + ]; for test_case in &test_cases { let canister_id = env @@ -1111,7 +1164,7 @@ mod snapshot_visibility { ) .unwrap(); - test_case.check_list_canister_snapshot(canister_id, &env, &[]); + test_case.check_list_canister_snapshot(canister_id, &[], &env); let snapshot = env .take_canister_snapshot(TakeCanisterSnapshotArgs { @@ -1121,8 +1174,11 @@ mod snapshot_visibility { sender_canister_version: None, }) .unwrap(); + let snapshot_id = snapshot.snapshot_id(); - test_case.check_list_canister_snapshot(canister_id, &env, &[snapshot]); + test_case.check_list_canister_snapshot(canister_id, &[snapshot], &env); + test_case.check_read_canister_snapshot_metadata(canister_id, snapshot_id, &env); + test_case.check_read_canister_snapshot_data(canister_id, snapshot_id, &env); } } @@ -1130,37 +1186,30 @@ mod snapshot_visibility { struct TestCase { snapshot_visibility: SnapshotVisibility, sender: PrincipalId, - expected_result: Result<(), UserError>, + expected_result: Result<(), PrincipalId>, } impl TestCase { fn check_list_canister_snapshot( &self, canister_id: CanisterId, - env: &StateMachine, snapshots: &[CanisterSnapshotResponse], + env: &StateMachine, ) { + let result = env.list_canister_snapshots_as( + ListCanisterSnapshotArgs::new(canister_id), + self.sender, + ); match &self.expected_result { - Err(err) => { + Err(caller) => { assert_eq!( - env.list_canister_snapshots_as( - ListCanisterSnapshotArgs::new(canister_id), - self.sender - ) - .as_ref(), - Err(err), + result.as_ref().err(), + Some(&expected_error(caller, "list_canister_snapshots")), "{self:?}" ); } Ok(()) => { - assert_eq!( - env.list_canister_snapshots_as( - ListCanisterSnapshotArgs::new(canister_id), - self.sender - ), - Ok(snapshots.to_vec()), - "{self:?}" - ); + assert_eq!(result, Ok(snapshots.to_vec()), "{self:?}"); } } } @@ -1174,8 +1223,12 @@ mod snapshot_visibility { let args = ReadCanisterSnapshotMetadataArgs::new(canister_id, snapshot_id); let result = env.read_canister_snapshot_metadata_as(&args, self.sender); match &self.expected_result { - Err(err) => { - assert_eq!(result.as_ref().err(), Some(err), "{self:?}"); + Err(caller) => { + assert_eq!( + result.as_ref().err(), + Some(&expected_error(caller, "read_canister_snapshot_metadata")), + "{self:?}" + ); } Ok(()) => { assert!(result.is_ok(), "{self:?}: {}", result.unwrap_err()); @@ -1196,8 +1249,12 @@ mod snapshot_visibility { ); let result = env.read_canister_snapshot_data_as(&args, self.sender); match &self.expected_result { - Err(err) => { - assert_eq!(result.as_ref().err(), Some(err), "{self:?}"); + Err(caller) => { + assert_eq!( + result.as_ref().err(), + Some(&expected_error(caller, "read_canister_snapshot_data")), + "{self:?}" + ); } Ok(()) => { assert!(result.is_ok(), "{self:?}: {}", result.unwrap_err()); @@ -1206,165 +1263,10 @@ mod snapshot_visibility { } } - fn visibility_test_cases( - controller: PrincipalId, - not_a_controller: PrincipalId, - allowed_viewer: PrincipalId, - not_allowed_viewer: PrincipalId, - not_allowed_error: impl Fn(&PrincipalId) -> Result<(), UserError>, - ) -> Vec { - let allowed_viewers = BoundedAllowedViewers::new(vec![allowed_viewer]); - vec![ - TestCase { - snapshot_visibility: SnapshotVisibility::Public, - sender: controller, - expected_result: Ok(()), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Public, - sender: not_a_controller, - expected_result: Ok(()), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Public, - sender: allowed_viewer, - expected_result: Ok(()), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Public, - sender: not_allowed_viewer, - expected_result: Ok(()), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Controllers, - sender: controller, - expected_result: Ok(()), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Controllers, - sender: not_a_controller, - expected_result: not_allowed_error(¬_a_controller), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Controllers, - sender: allowed_viewer, - expected_result: not_allowed_error(&allowed_viewer), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::Controllers, - sender: not_allowed_viewer, - expected_result: not_allowed_error(¬_allowed_viewer), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), - sender: allowed_viewer, - expected_result: Ok(()), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), - sender: not_a_controller, - expected_result: not_allowed_error(¬_a_controller), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers.clone()), - sender: not_allowed_viewer, - expected_result: not_allowed_error(¬_allowed_viewer), - }, - TestCase { - snapshot_visibility: SnapshotVisibility::AllowedViewers(allowed_viewers), - sender: controller, - expected_result: Ok(()), - }, - ] - } - - #[test] - fn test_visibility_of_read_canister_snapshot_metadata() { - let env = StateMachineBuilder::new().build(); - let controller = PrincipalId::new_user_test_id(1); - - let test_cases = visibility_test_cases( - controller, - PrincipalId::new_user_test_id(2), - PrincipalId::new_user_test_id(3), - PrincipalId::new_user_test_id(4), - |caller| { - Err(UserError::new( - ErrorCode::CanisterRejectedMessage, - format!("Caller {caller} is not allowed to call read_canister_snapshot_metadata"), - )) - }, - ); - - for test_case in &test_cases { - let canister_id = env - .install_canister( - UNIVERSAL_CANISTER_WASM.to_vec(), - vec![], - Some( - CanisterSettingsArgsBuilder::new() - .with_snapshot_visibility(test_case.snapshot_visibility.clone()) - .with_controllers(vec![controller]) - .build(), - ), - ) - .unwrap(); - - let snapshot = env - .take_canister_snapshot(TakeCanisterSnapshotArgs { - canister_id: canister_id.get(), - replace_snapshot: None, - uninstall_code: None, - sender_canister_version: None, - }) - .unwrap(); - - test_case.check_read_canister_snapshot_metadata(canister_id, snapshot.id, &env); - } - } - - #[test] - fn test_visibility_of_read_canister_snapshot_data() { - let env = StateMachineBuilder::new().build(); - let controller = PrincipalId::new_user_test_id(1); - - let test_cases = visibility_test_cases( - controller, - PrincipalId::new_user_test_id(2), - PrincipalId::new_user_test_id(3), - PrincipalId::new_user_test_id(4), - |caller| { - Err(UserError::new( - ErrorCode::CanisterRejectedMessage, - format!("Caller {caller} is not allowed to call read_canister_snapshot_data"), - )) - }, - ); - - for test_case in &test_cases { - let canister_id = env - .install_canister( - UNIVERSAL_CANISTER_WASM.to_vec(), - vec![], - Some( - CanisterSettingsArgsBuilder::new() - .with_snapshot_visibility(test_case.snapshot_visibility.clone()) - .with_controllers(vec![controller]) - .build(), - ), - ) - .unwrap(); - - let snapshot = env - .take_canister_snapshot(TakeCanisterSnapshotArgs { - canister_id: canister_id.get(), - replace_snapshot: None, - uninstall_code: None, - sender_canister_version: None, - }) - .unwrap(); - - test_case.check_read_canister_snapshot_data(canister_id, snapshot.id, &env); - } + fn expected_error(caller: &PrincipalId, method: &str) -> UserError { + UserError::new( + ErrorCode::CanisterRejectedMessage, + format!("Caller {caller} is not allowed to call {method}"), + ) } } From 6d594d55485321ae41249a78fdbaae2e0de2a6cd Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 12 Mar 2026 17:00:49 +0100 Subject: [PATCH 22/25] DEFI-2667: fix tests --- rs/execution_environment/src/canister_manager.rs | 2 +- .../src/execution_environment/tests/canister_snapshots.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 1a92f4aecea0..8ba712028a01 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -2560,7 +2560,7 @@ impl CanisterManager { canister: &CanisterState, state: &ReplicatedState, ) -> Result, UserError> { - validate_snapshot_visibility(canister, &sender, "list canister snapshots")?; + validate_snapshot_visibility(canister, &sender, "list_canister_snapshots")?; let mut responses = vec![]; for (snapshot_id, snapshot) in state diff --git a/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs b/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs index 0c2dbac96cc5..dd5cb4957175 100644 --- a/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs +++ b/rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs @@ -1097,7 +1097,7 @@ fn list_canister_snapshot_fails_invalid_controller() { res.response_payload.assert_contains_reject( RejectCode::CanisterReject, &format!( - "Caller {} is not allowed to list canister snapshots", + "Caller {} is not allowed to call list_canister_snapshots", caller_canister.get(), ), ); @@ -2005,7 +2005,7 @@ fn read_canister_snapshot_metadata_fails_invalid_controller() { let error = test .subnet_message("read_canister_snapshot_metadata", args.encode()) .unwrap_err(); - assert_eq!(error.code(), ErrorCode::CanisterInvalidController); + assert_eq!(error.code(), ErrorCode::CanisterRejectedMessage); } fn read_canister_snapshot_data( @@ -2375,7 +2375,7 @@ fn read_canister_snapshot_data_fails_invalid_controller() { let error = test .subnet_message("read_canister_snapshot_data", args.encode()) .unwrap_err(); - assert_eq!(error.code(), ErrorCode::CanisterInvalidController); + assert_eq!(error.code(), ErrorCode::CanisterRejectedMessage); } #[test] From af4043f179b84a0b77426c70d03e01debd58c84b Mon Sep 17 00:00:00 2001 From: gregorydemay <112856886+gregorydemay@users.noreply.github.com> Date: Fri, 13 Mar 2026 16:11:06 +0100 Subject: [PATCH 23/25] Update rs/execution_environment/src/canister_manager.rs Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com> --- rs/execution_environment/src/canister_manager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 9289475904db..527ff72ea823 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -2634,7 +2634,7 @@ impl CanisterManager { snapshot_id: SnapshotId, canister: &CanisterState, ) -> Result { - validate_snapshot_visibility(canister, &sender, "read canister snapshot metadata")?; + validate_snapshot_visibility(canister, &sender, "read_canister_snapshot_metadata")?; let snapshot = self .get_snapshot(canister, snapshot_id) .map_err(UserError::from)?; From 4451041bb62472d5b886134a2228e904bdd96833 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Mon, 16 Mar 2026 11:03:04 +0100 Subject: [PATCH 24/25] lint --- .../src/execution_environment.rs | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index 978bab5aa2a6..c5c98120f971 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -2622,12 +2622,9 @@ impl ExecutionEnvironment { ) -> Result, UserError> { let canister = get_canister(args.get_canister_id(), state)?; - let result = self - .canister_manager + self.canister_manager .list_canister_snapshot(sender, canister) - .map_err(UserError::from)?; - - Ok(Encode!(&result).unwrap()) + .map(|result| Encode!(&result).unwrap()) } /// Deletes the specified canister snapshot if it exists. @@ -2691,18 +2688,18 @@ impl ExecutionEnvironment { Some(canister) => canister, }; - let result = match self.canister_manager.read_snapshot_data( - sender, - Arc::make_mut(&mut canister), - args.get_snapshot_id(), - args.kind, - state, - subnet_size, - round_limits, - ) { - Ok(result) => Ok(Encode!(&result).unwrap()), - Err(err) => Err(UserError::from(err)), - }; + let result = self + .canister_manager + .read_snapshot_data( + sender, + Arc::make_mut(&mut canister), + args.get_snapshot_id(), + args.kind, + state, + subnet_size, + round_limits, + ) + .map(|response| Encode!(&response).unwrap()); // Put canister back. state.put_canister_state(canister); @@ -2764,13 +2761,9 @@ impl ExecutionEnvironment { ) -> Result, UserError> { let canister = get_canister(args.get_canister_id(), state)?; let snapshot_id = args.get_snapshot_id(); - match self - .canister_manager + self.canister_manager .read_snapshot_metadata(sender, snapshot_id, canister) - { - Ok(response) => Ok(Encode!(&response).unwrap()), - Err(e) => Err(UserError::from(e)), - } + .map(|response| Encode!(&response).unwrap()) } fn create_snapshot_from_metadata( From 692f824562242ea5300f523ae2087d16f9a3d081 Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Mon, 16 Mar 2026 10:10:46 +0000 Subject: [PATCH 25/25] Automatically fixing code for linting and formatting issues --- rs/execution_environment/src/canister_manager.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index e44a606907d4..e23fb161827e 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -2588,7 +2588,11 @@ impl CanisterManager { snapshot_id: SnapshotId, canister: &CanisterState, ) -> Result { - validate_snapshot_visibility(canister, &sender, "read read_canister_snapshot_metadata snapshot metadata")?; + validate_snapshot_visibility( + canister, + &sender, + "read read_canister_snapshot_metadata snapshot metadata", + )?; let snapshot = self .get_snapshot(canister, snapshot_id) .map_err(UserError::from)?;