feat: add log_visibility to canister settings manifest

README.md

@@ -49,6 +49,7 @@ Contributions are welcome! See [CONTRIBUTING.md](.github/CONTRIBUTING.md) for de
 ### Prerequisites
 
 - Rust 1.88.0+ ([rustup.rs](https://rustup.rs/))
+- `wasm-tools` — Install via `cargo install wasm-tools` (required for test suite)
 - Platform dependencies:
 
 | Platform      | Install                                                                                                  |

crates/icp-canister-interfaces/src/cycles_ledger.rs

@@ -9,11 +9,22 @@ pub const CYCLES_LEDGER_CID: &str = "um5iw-rqaaa-aaaaq-qaaba-cai";
 pub const CYCLES_LEDGER_PRINCIPAL: Principal =
     Principal::from_slice(&[0, 0, 0, 0, 2, 16, 0, 2, 1, 1]);
 
+/// Log visibility setting for a canister.
+/// Matches the cycles ledger's LogVisibility variant type.
+#[derive(Clone, Debug, CandidType, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum LogVisibility {
+    Controllers,
+    Public,
+    AllowedViewers(Vec<Principal>),
+}
+
 #[derive(Clone, Debug, CandidType, Deserialize)]
 pub struct CanisterSettingsArg {
     pub freezing_threshold: Option<Nat>,
     pub controllers: Option<Vec<Principal>>,
     pub reserved_cycles_limit: Option<Nat>,
+    pub log_visibility: Option<LogVisibility>,
     pub memory_allocation: Option<Nat>,
     pub compute_allocation: Option<Nat>,
 }

crates/icp-cli/src/commands/canister/create.rs

@@ -78,6 +78,7 @@ impl CreateArgs {
                 .reserved_cycles_limit
                 .or(default.settings.reserved_cycles_limit)
                 .map(Nat::from),
+            log_visibility: default.settings.log_visibility.clone().map(Into::into),
             memory_allocation: self
                 .settings
                 .memory_allocation

crates/icp-cli/src/commands/canister/settings/update.rs

@@ -260,6 +260,11 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), anyhow:
         update = update.with_wasm_memory_threshold(wasm_memory_threshold.as_u64());
     }
     if let Some(log_visibility) = log_visibility {
+        if configured_settings.log_visibility.is_some() {
+            ctx.term.write_line(
+                "Warning: Log visibility is already set in icp.yaml; this new value will be overridden on next settings sync"
+            )?
+        }
         update = update.with_log_visibility(log_visibility);
     }
     if let Some(environment_variables) = environment_variables {

crates/icp-cli/src/operations/settings.rs

@@ -1,9 +1,12 @@
-use std::{collections::HashMap, sync::Arc};
+use std::{
+    collections::{HashMap, HashSet},
+    sync::Arc,
+};
 
 use candid::Principal;
 use futures::{StreamExt, stream::FuturesOrdered};
 use ic_agent::{Agent, AgentError};
-use ic_management_canister_types::EnvironmentVariable;
+use ic_management_canister_types::{EnvironmentVariable, LogVisibility as IcLogVisibility};
 use ic_utils::interfaces::ManagementCanister;
 use icp::{Canister, canister::Settings, context::TermWriter};
 use itertools::Itertools;
@@ -41,6 +44,29 @@ struct SettingsFailure {
     error: SyncSettingsOperationError,
 }
 
+/// Compare two LogVisibility values in an order-insensitive manner.
+/// For AllowedViewers, the principal lists are compared as sets.
+fn log_visibility_eq(a: &IcLogVisibility, b: &IcLogVisibility) -> bool {
+    match (a, b) {
+        (IcLogVisibility::Controllers, IcLogVisibility::Controllers) => true,
+        (IcLogVisibility::Public, IcLogVisibility::Public) => true,
+        (IcLogVisibility::AllowedViewers(va), IcLogVisibility::AllowedViewers(vb)) => {
+            let set_a: HashSet<_> = va.iter().collect();
+            let set_b: HashSet<_> = vb.iter().collect();
+            set_a == set_b
+        }
+        _ => false,
+    }
+}
+
+/// Compare two environment variable lists in an order-insensitive manner.
+/// Uses HashMap comparison (by name -> value).
+fn environment_variables_eq(a: &[EnvironmentVariable], b: &[EnvironmentVariable]) -> bool {
+    let map_a: HashMap<_, _> = a.iter().map(|ev| (&ev.name, &ev.value)).collect();
+    let map_b: HashMap<_, _> = b.iter().map(|ev| (&ev.name, &ev.value)).collect();
+    map_a == map_b
+}
+
 pub(crate) async fn sync_settings(
     mgmt: &ManagementCanister<'_>,
     cid: &Principal,
@@ -51,6 +77,7 @@ pub(crate) async fn sync_settings(
         .await
         .context(FetchCurrentSettingsSnafu { canister: *cid })?;
     let &Settings {
+        ref log_visibility,
         compute_allocation,
         memory_allocation,
         freezing_threshold,
@@ -61,6 +88,10 @@ pub(crate) async fn sync_settings(
     } = &canister.settings;
     let current_settings = status.settings;
 
+    // Convert our log_visibility to IC type for comparison and update
+    let log_visibility_setting: Option<IcLogVisibility> =
+        log_visibility.clone().map(IcLogVisibility::from);
+
     let environment_variable_setting =
         if let Some(configured_environment_variables) = &environment_variables {
             let mut merged_environment_variables: HashMap<_, _> = current_settings
@@ -79,20 +110,24 @@ pub(crate) async fn sync_settings(
         } else {
             None
         };
-    if compute_allocation.is_none_or(|s| s == current_settings.compute_allocation)
+    if log_visibility_setting
+        .as_ref()
+        .is_none_or(|s| log_visibility_eq(s, &current_settings.log_visibility))
+        && compute_allocation.is_none_or(|s| s == current_settings.compute_allocation)
         && memory_allocation.is_none_or(|s| s == current_settings.memory_allocation)
         && freezing_threshold.is_none_or(|s| s == current_settings.freezing_threshold)
         && reserved_cycles_limit.is_none_or(|s| s == current_settings.reserved_cycles_limit)
         && wasm_memory_limit.is_none_or(|s| s == current_settings.wasm_memory_limit)
         && wasm_memory_threshold.is_none_or(|s| s == current_settings.wasm_memory_threshold)
         && environment_variable_setting
             .as_ref()
-            .is_none_or(|s| *s == current_settings.environment_variables)
+            .is_none_or(|s| environment_variables_eq(s, &current_settings.environment_variables))
     {
         // No changes needed
         return Ok(());
     }
     mgmt.update_settings(cid)
+        .with_optional_log_visibility(log_visibility_setting)
         .with_optional_compute_allocation(compute_allocation)
         .with_optional_memory_allocation(memory_allocation)
         .with_optional_freezing_threshold(freezing_threshold)
@@ -185,3 +220,205 @@ pub(crate) async fn sync_settings_many(
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn log_visibility_eq_controllers() {
+        assert!(log_visibility_eq(
+            &IcLogVisibility::Controllers,
+            &IcLogVisibility::Controllers
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_public() {
+        assert!(log_visibility_eq(
+            &IcLogVisibility::Public,
+            &IcLogVisibility::Public
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_different_variants() {
+        assert!(!log_visibility_eq(
+            &IcLogVisibility::Controllers,
+            &IcLogVisibility::Public
+        ));
+        assert!(!log_visibility_eq(
+            &IcLogVisibility::Public,
+            &IcLogVisibility::Controllers
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_allowed_viewers_same_order() {
+        let p1 = Principal::from_text("aaaaa-aa").unwrap();
+        let p2 = Principal::from_text("2vxsx-fae").unwrap();
+
+        assert!(log_visibility_eq(
+            &IcLogVisibility::AllowedViewers(vec![p1, p2]),
+            &IcLogVisibility::AllowedViewers(vec![p1, p2])
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_allowed_viewers_different_order() {
+        let p1 = Principal::from_text("aaaaa-aa").unwrap();
+        let p2 = Principal::from_text("2vxsx-fae").unwrap();
+
+        // Order should not matter
+        assert!(log_visibility_eq(
+            &IcLogVisibility::AllowedViewers(vec![p1, p2]),
+            &IcLogVisibility::AllowedViewers(vec![p2, p1])
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_allowed_viewers_different_principals() {
+        let p1 = Principal::from_text("aaaaa-aa").unwrap();
+        let p2 = Principal::from_text("2vxsx-fae").unwrap();
+        let p3 = Principal::from_text("ryjl3-tyaaa-aaaaa-aaaba-cai").unwrap();
+
+        assert!(!log_visibility_eq(
+            &IcLogVisibility::AllowedViewers(vec![p1, p2]),
+            &IcLogVisibility::AllowedViewers(vec![p1, p3])
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_allowed_viewers_different_length() {
+        let p1 = Principal::from_text("aaaaa-aa").unwrap();
+        let p2 = Principal::from_text("2vxsx-fae").unwrap();
+
+        assert!(!log_visibility_eq(
+            &IcLogVisibility::AllowedViewers(vec![p1]),
+            &IcLogVisibility::AllowedViewers(vec![p1, p2])
+        ));
+    }
+
+    #[test]
+    fn log_visibility_eq_allowed_viewers_vs_other() {
+        let p1 = Principal::from_text("aaaaa-aa").unwrap();
+
+        assert!(!log_visibility_eq(
+            &IcLogVisibility::AllowedViewers(vec![p1]),
+            &IcLogVisibility::Controllers
+        ));
+        assert!(!log_visibility_eq(
+            &IcLogVisibility::AllowedViewers(vec![p1]),
+            &IcLogVisibility::Public
+        ));
+    }
+
+    #[test]
+    fn environment_variables_eq_same_order() {
+        let vars1 = vec![
+            EnvironmentVariable {
+                name: "A".to_string(),
+                value: "1".to_string(),
+            },
+            EnvironmentVariable {
+                name: "B".to_string(),
+                value: "2".to_string(),
+            },
+        ];
+        let vars2 = vec![
+            EnvironmentVariable {
+                name: "A".to_string(),
+                value: "1".to_string(),
+            },
+            EnvironmentVariable {
+                name: "B".to_string(),
+                value: "2".to_string(),
+            },
+        ];
+
+        assert!(environment_variables_eq(&vars1, &vars2));
+    }
+
+    #[test]
+    fn environment_variables_eq_different_order() {
+        let vars1 = vec![
+            EnvironmentVariable {
+                name: "A".to_string(),
+                value: "1".to_string(),
+            },
+            EnvironmentVariable {
+                name: "B".to_string(),
+                value: "2".to_string(),
+            },
+        ];
+        let vars2 = vec![
+            EnvironmentVariable {
+                name: "B".to_string(),
+                value: "2".to_string(),
+            },
+            EnvironmentVariable {
+                name: "A".to_string(),
+                value: "1".to_string(),
+            },
+        ];
+
+        // Order should not matter
+        assert!(environment_variables_eq(&vars1, &vars2));
+    }
+
+    #[test]
+    fn environment_variables_eq_different_values() {
+        let vars1 = vec![EnvironmentVariable {
+            name: "A".to_string(),
+            value: "1".to_string(),
+        }];
+        let vars2 = vec![EnvironmentVariable {
+            name: "A".to_string(),
+            value: "2".to_string(),
+        }];
+
+        assert!(!environment_variables_eq(&vars1, &vars2));
+    }
+
+    #[test]
+    fn environment_variables_eq_different_keys() {
+        let vars1 = vec![EnvironmentVariable {
+            name: "A".to_string(),
+            value: "1".to_string(),
+        }];
+        let vars2 = vec![EnvironmentVariable {
+            name: "B".to_string(),
+            value: "1".to_string(),
+        }];
+
+        assert!(!environment_variables_eq(&vars1, &vars2));
+    }
+
+    #[test]
+    fn environment_variables_eq_different_length() {
+        let vars1 = vec![EnvironmentVariable {
+            name: "A".to_string(),
+            value: "1".to_string(),
+        }];
+        let vars2 = vec![
+            EnvironmentVariable {
+                name: "A".to_string(),
+                value: "1".to_string(),
+            },
+            EnvironmentVariable {
+                name: "B".to_string(),
+                value: "2".to_string(),
+            },
+        ];
+
+        assert!(!environment_variables_eq(&vars1, &vars2));
+    }
+
+    #[test]
+    fn environment_variables_eq_empty() {
+        let vars1: Vec<EnvironmentVariable> = vec![];
+        let vars2: Vec<EnvironmentVariable> = vec![];
+
+        assert!(environment_variables_eq(&vars1, &vars2));
+    }
+}