feat: add log_visibility_v2

rs/execution_environment/src/canister_manager.rs

@@ -619,7 +619,7 @@ impl CanisterManager {
             canister.system_state.freeze_threshold = freezing_threshold;
         }
         if let Some(log_visibility) = settings.log_visibility() {
-            canister.system_state.log_visibility = log_visibility;
+            canister.system_state.log_visibility = log_visibility.clone();
         }
         if let Some(wasm_memory_limit) = settings.wasm_memory_limit() {
             canister.system_state.wasm_memory_limit = Some(wasm_memory_limit);
@@ -1157,7 +1157,7 @@ impl CanisterManager {
         let memory_allocation = canister.memory_allocation();
         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;
+        let log_visibility = canister.system_state.log_visibility.clone();
         let wasm_memory_limit = canister.system_state.wasm_memory_limit;
 
         Ok(CanisterStatusResultV2::new(

rs/execution_environment/src/canister_settings.rs

@@ -76,8 +76,8 @@ impl CanisterSettings {
         self.reserved_cycles_limit
     }
 
-    pub fn log_visibility(&self) -> Option<LogVisibility> {
-        self.log_visibility
+    pub fn log_visibility(&self) -> Option<&LogVisibility> {
+        self.log_visibility.as_ref()
     }
 
     pub fn wasm_memory_limit(&self) -> Option<NumBytes> {
@@ -381,8 +381,8 @@ impl ValidatedCanisterSettings {
         self.reservation_cycles
     }
 
-    pub fn log_visibility(&self) -> Option<LogVisibility> {
-        self.log_visibility
+    pub fn log_visibility(&self) -> Option<&LogVisibility> {
+        self.log_visibility.as_ref()
     }
 
     pub fn wasm_memory_limit(&self) -> Option<NumBytes> {
@@ -592,7 +592,7 @@ pub(crate) fn validate_canister_settings(
         freezing_threshold: settings.freezing_threshold(),
         reserved_cycles_limit: settings.reserved_cycles_limit(),
         reservation_cycles,
-        log_visibility: settings.log_visibility(),
+        log_visibility: settings.log_visibility().cloned(),
         wasm_memory_limit: settings.wasm_memory_limit(),
     })
 }

rs/execution_environment/src/execution_environment/tests.rs

@@ -3517,7 +3517,7 @@ fn test_canister_settings_log_visibility_default_controllers() {
     // Assert.
     assert_eq!(
         canister_status.settings().log_visibility(),
-        LogVisibility::Controllers
+        &LogVisibility::Controllers
     );
 }
 
@@ -3539,7 +3539,7 @@ fn test_canister_settings_log_visibility_create_with_settings() {
     // Assert.
     assert_eq!(
         canister_status.settings().log_visibility(),
-        LogVisibility::Public
+        &LogVisibility::Public
     );
 }
 
@@ -3556,7 +3556,7 @@ fn test_canister_settings_log_visibility_set_to_public() {
     // Assert.
     assert_eq!(
         canister_status.settings().log_visibility(),
-        LogVisibility::Public
+        &LogVisibility::Public
     );
 }
 

rs/execution_environment/src/query_handler.rs

@@ -297,8 +297,9 @@ fn fetch_canister_logs(
 
     match canister.log_visibility() {
         LogVisibility::Public => Ok(()),
+        LogVisibility::AllowedViewers(_) if canister.controllers().contains(&sender) => Ok(()),
         LogVisibility::Controllers if canister.controllers().contains(&sender) => Ok(()),
-        LogVisibility::Controllers => Err(UserError::new(
+        LogVisibility::AllowedViewers(_) | LogVisibility::Controllers => Err(UserError::new(
             ErrorCode::CanisterRejectedMessage,
             format!(
                 "Caller {} is not allowed to query ic00 method {}",

rs/nns/cmc/cmc.did

@@ -3,6 +3,7 @@ type BlockIndex = nat64;
 type log_visibility = variant {
   controllers;
   public;
+  allowed_viewers : vec principal;
 };
 type CanisterSettings = record {
   controllers : opt vec principal;

rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto

@@ -312,10 +312,27 @@ message WasmChunkStoreMetadata {
   uint64 size = 2;
 }
 
+// TODO(EXC-1670): Migrate to LogVisibilityV2.
+// The current enum only supports i32 values, which limits the
+// storage of allowed_viewers principals. LogVisibilityV2 will
+// support both enum values and a list of principals.
 enum LogVisibility {
   LOG_VISIBILITY_UNSPECIFIED = 0;
   LOG_VISIBILITY_CONTROLLERS = 1;
   LOG_VISIBILITY_PUBLIC = 2;
+  LOG_VISIBILITY_EMPTY_ALLOWED_VIEWERS = 3;
+}
+
+enum LogVisibilityEnum {
+  LOG_VISIBILITY_ENUM_UNSPECIFIED = 0;
+  LOG_VISIBILITY_ENUM_CONTROLLERS = 1;
+  LOG_VISIBILITY_ENUM_ALLOWED_VIEWERS = 2;
+  LOG_VISIBILITY_ENUM_PUBLIC = 3;
+}
+
+message LogVisibilityV2 {
+  LogVisibilityEnum log_visibility_enum = 1;
+  repeated types.v1.PrincipalId allowed_viewers = 2;
 }
 
 message CanisterLogRecord {
@@ -403,7 +420,7 @@ message CanisterStateBits {
   // Statistics on query execution for entire lifetime of canister.
   TotalQueryStats total_query_stats = 41;
   // Log visibility for the canister.
-  LogVisibility log_visibility = 42;
+  LogVisibility log_visibility = 42; // TODO(EXC-1670): remove this field.
   // Log records of the canister.
   repeated CanisterLogRecord canister_log_records = 43;
   // The index of the next log record to be created.
@@ -419,4 +436,6 @@ message CanisterStateBits {
   int64 priority_credit = 48;
   LongExecutionMode long_execution_mode = 49;
   optional uint64 wasm_memory_threshold = 50;
+  // Log visibility for the canister.
+  LogVisibilityV2 log_visibility_v2 = 51;
 }

rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs

@@ -555,6 +555,14 @@ pub struct WasmChunkStoreMetadata {
 }
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
+pub struct LogVisibilityV2 {
+    #[prost(enumeration = "LogVisibilityEnum", tag = "1")]
+    pub log_visibility_enum: i32,
+    #[prost(message, repeated, tag = "2")]
+    pub allowed_viewers: ::prost::alloc::vec::Vec<super::super::super::types::v1::PrincipalId>,
+}
+#[allow(clippy::derive_partial_eq_without_eq)]
+#[derive(Clone, PartialEq, ::prost::Message)]
 pub struct CanisterLogRecord {
     #[prost(uint64, tag = "1")]
     pub idx: u64,
@@ -647,6 +655,8 @@ pub struct CanisterStateBits {
     #[prost(message, optional, tag = "41")]
     pub total_query_stats: ::core::option::Option<TotalQueryStats>,
     /// Log visibility for the canister.
+    ///
+    /// TODO(EXC-1670): remove this field.
     #[prost(enumeration = "LogVisibility", tag = "42")]
     pub log_visibility: i32,
     /// Log records of the canister.
@@ -670,6 +680,9 @@ pub struct CanisterStateBits {
     pub long_execution_mode: i32,
     #[prost(uint64, optional, tag = "50")]
     pub wasm_memory_threshold: ::core::option::Option<u64>,
+    /// Log visibility for the canister.
+    #[prost(message, optional, tag = "51")]
+    pub log_visibility_v2: ::core::option::Option<LogVisibilityV2>,
     #[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")]
     pub canister_status: ::core::option::Option<canister_state_bits::CanisterStatus>,
 }
@@ -813,12 +826,17 @@ impl CyclesUseCase {
         }
     }
 }
+/// TODO(EXC-1670): Migrate to LogVisibilityV2.
+/// The current enum only supports i32 values, which limits the
+/// storage of allowed_viewers principals. LogVisibilityV2 will
+/// support both enum values and a list of principals.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
 #[repr(i32)]
 pub enum LogVisibility {
     Unspecified = 0,
     Controllers = 1,
     Public = 2,
+    EmptyAllowedViewers = 3,
 }
 impl LogVisibility {
     /// String value of the enum field names used in the ProtoBuf definition.
@@ -830,6 +848,7 @@ impl LogVisibility {
             LogVisibility::Unspecified => "LOG_VISIBILITY_UNSPECIFIED",
             LogVisibility::Controllers => "LOG_VISIBILITY_CONTROLLERS",
             LogVisibility::Public => "LOG_VISIBILITY_PUBLIC",
+            LogVisibility::EmptyAllowedViewers => "LOG_VISIBILITY_EMPTY_ALLOWED_VIEWERS",
         }
     }
     /// Creates an enum from field names used in the ProtoBuf definition.
@@ -838,6 +857,39 @@ impl LogVisibility {
             "LOG_VISIBILITY_UNSPECIFIED" => Some(Self::Unspecified),
             "LOG_VISIBILITY_CONTROLLERS" => Some(Self::Controllers),
             "LOG_VISIBILITY_PUBLIC" => Some(Self::Public),
+            "LOG_VISIBILITY_EMPTY_ALLOWED_VIEWERS" => Some(Self::EmptyAllowedViewers),
+            _ => None,
+        }
+    }
+}
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
+#[repr(i32)]
+pub enum LogVisibilityEnum {
+    Unspecified = 0,
+    Controllers = 1,
+    AllowedViewers = 2,
+    Public = 3,
+}
+impl LogVisibilityEnum {
+    /// 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 {
+            LogVisibilityEnum::Unspecified => "LOG_VISIBILITY_ENUM_UNSPECIFIED",
+            LogVisibilityEnum::Controllers => "LOG_VISIBILITY_ENUM_CONTROLLERS",
+            LogVisibilityEnum::AllowedViewers => "LOG_VISIBILITY_ENUM_ALLOWED_VIEWERS",
+            LogVisibilityEnum::Public => "LOG_VISIBILITY_ENUM_PUBLIC",
+        }
+    }
+    /// Creates an enum from field names used in the ProtoBuf definition.
+    pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
+        match value {
+            "LOG_VISIBILITY_ENUM_UNSPECIFIED" => Some(Self::Unspecified),
+            "LOG_VISIBILITY_ENUM_CONTROLLERS" => Some(Self::Controllers),
+            "LOG_VISIBILITY_ENUM_ALLOWED_VIEWERS" => Some(Self::AllowedViewers),
+            "LOG_VISIBILITY_ENUM_PUBLIC" => Some(Self::Public),
             _ => None,
         }
     }

rs/replicated_state/src/canister_state.rs

@@ -153,8 +153,8 @@ impl CanisterState {
         &self.system_state.controllers
     }
 
-    pub fn log_visibility(&self) -> LogVisibility {
-        self.system_state.log_visibility
+    pub fn log_visibility(&self) -> &LogVisibility {
+        &self.system_state.log_visibility
     }
 
     /// Returns the difference in time since the canister was last charged for resource allocations.

rs/replicated_state/src/canister_state/tests.rs

@@ -619,6 +619,13 @@ fn canister_state_log_visibility_round_trip() {
 
         assert_eq!(initial, round_trip);
     }
+
+    for initial in LogVisibility::iter() {
+        let encoded = pb::LogVisibilityV2::from(&initial);
+        let round_trip = LogVisibility::from(encoded);
+
+        assert_eq!(initial, round_trip);
+    }
 }
 
 #[test]
@@ -657,12 +664,7 @@ fn long_execution_mode_decoding() {
 fn compatibility_for_log_visibility() {
     // If this fails, you are making a potentially incompatible change to `LogVisibility`.
     // See note [Handling changes to Enums in Replicated State] for how to proceed.
-    assert_eq!(
-        LogVisibility::iter()
-            .map(|x| x as i32)
-            .collect::<Vec<i32>>(),
-        [1, 2]
-    );
+    assert_eq!(LogVisibility::iter().count(), 3);
 }
 
 #[test]

rs/state_layout/src/state_layout.rs

@@ -1946,6 +1946,8 @@ impl From<CanisterStateBits> for pb_canister_state_bits::CanisterStateBits {
             next_canister_log_record_idx: item.canister_log.next_idx(),
             wasm_memory_limit: item.wasm_memory_limit.map(|v| v.get()),
             next_snapshot_id: item.next_snapshot_id,
+            log_visibility_v2: pb_canister_state_bits::LogVisibilityV2::from(&item.log_visibility)
+                .into(),
         }
     }
 }
@@ -2016,6 +2018,25 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> for CanisterStateBits {
             );
         }
 
+        // TODO(EXC-1670): remove after migration to `pb_canister_state_bits::LogVisibilityV2`.
+        // First try to decode `log_visibility_v2` and if it fails, fallback to `log_visibility`.
+        // This should populate `allowed_viewers` correctly with the list of principals.
+        let log_visibility: LogVisibility = match try_from_option_field(
+            value.log_visibility_v2,
+            "CanisterStateBits::log_visibility_v2",
+        ) {
+            Ok(log_visibility_v2) => log_visibility_v2,
+            Err(_) => pb_canister_state_bits::LogVisibility::try_from(value.log_visibility)
+                .map_err(|_| ProxyDecodeError::ValueOutOfRange {
+                    typ: "LogVisibility",
+                    err: format!(
+                        "Unexpected value of log visibility: {}",
+                        value.log_visibility
+                    ),
+                })?
+                .into(),
+        };
+
         Ok(Self {
             controllers,
             last_full_execution_round: value.last_full_execution_round.into(),
@@ -2082,17 +2103,7 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> for CanisterStateBits {
                 "CanisterStateBits::total_query_stats",
             )
             .unwrap_or_default(),
-            log_visibility: LogVisibility::from(
-                pb_canister_state_bits::LogVisibility::try_from(value.log_visibility).map_err(
-                    |_| ProxyDecodeError::ValueOutOfRange {
-                        typ: "LogVisibility",
-                        err: format!(
-                            "Unexpected value of log visibility: {}",
-                            value.log_visibility
-                        ),
-                    },
-                )?,
-            ),
+            log_visibility,
             canister_log: CanisterLog::new(
                 value.next_canister_log_record_idx,
                 value

rs/state_manager/src/tip.rs

@@ -897,7 +897,7 @@ fn serialize_canister_to_tip(
                 .metadata()
                 .clone(),
             total_query_stats: canister_state.scheduler_state.total_query_stats.clone(),
-            log_visibility: canister_state.system_state.log_visibility,
+            log_visibility: canister_state.system_state.log_visibility.clone(),
             canister_log: canister_state.system_state.canister_log.clone(),
             wasm_memory_limit: canister_state.system_state.wasm_memory_limit,
             next_snapshot_id: canister_state.system_state.next_snapshot_id,

rs/types/management_canister_types/src/bounded_vec.rs

@@ -10,7 +10,7 @@ pub const UNBOUNDED: usize = usize::MAX;
 /// - number of elements
 /// - total data size in bytes
 /// - single element data size in bytes
-#[derive(CandidType, Debug, Clone, PartialEq, Eq)]
+#[derive(CandidType, Debug, Clone, PartialEq, Eq, Default)]
 pub struct BoundedVec<
     const MAX_ALLOWED_LEN: usize,
     const MAX_ALLOWED_TOTAL_DATA_SIZE: usize,