From 4765eb3176875fae90fa624c1bd2c490d7f3daa5 Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Tue, 4 Jun 2024 11:06:13 +0200 Subject: [PATCH] feat: add `log_visibility` to canister settings (#564) --- CHANGELOG.md | 2 + .../src/interfaces/management_canister.rs | 14 ++++ .../management_canister/builders.rs | 76 ++++++++++++++++++- ic-utils/src/interfaces/wallet.rs | 8 +- ref-tests/tests/ic-ref.rs | 66 +++++++++++++++- ref-tests/tests/integration.rs | 1 + 6 files changed, 163 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ba452d2..c7f032aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Changed the SyncCall and AsyncCall traits to use an associated type for their output instead of a generic parameter. * Call builders now generally implement `IntoFuture`, allowing `.call_and_wait().await` to be shortened to `.await`. +* Added `log_visibility` to canister creation and canister setting update options. + ## [0.35.0] - 2024-05-10 * Added a limit to the concurrent requests an agent will make at once. This should make server-side ratelimiting much rarer to encounter, even when sending a high volume of requests (for example, a large `ic_utils::ManagementCanister::install` call). diff --git a/ic-utils/src/interfaces/management_canister.rs b/ic-utils/src/interfaces/management_canister.rs index de4bfc66..0671fb8d 100644 --- a/ic-utils/src/interfaces/management_canister.rs +++ b/ic-utils/src/interfaces/management_canister.rs @@ -144,6 +144,18 @@ pub struct QueryStats { pub response_payload_bytes_total: Nat, } +/// Log visibility for a canister. +#[derive(Default, Clone, Copy, CandidType, Deserialize, Debug, PartialEq, Eq)] +pub enum LogVisibility { + #[default] + #[serde(rename = "controllers")] + /// Canister logs are visible to controllers only. + Controllers, + #[serde(rename = "public")] + /// Canister logs are visible to everyone. + Public, +} + /// The concrete settings of a canister. #[derive(Clone, Debug, Deserialize, CandidType)] pub struct DefiniteCanisterSettings { @@ -159,6 +171,8 @@ pub struct DefiniteCanisterSettings { pub reserved_cycles_limit: Option, /// A soft limit on the Wasm memory usage of the canister in bytes (up to 256TiB). pub wasm_memory_limit: Option, + /// The canister log visibility. Defines which principals are allowed to fetch logs. + pub log_visibility: LogVisibility, } impl std::fmt::Display for StatusCallResult { diff --git a/ic-utils/src/interfaces/management_canister/builders.rs b/ic-utils/src/interfaces/management_canister/builders.rs index e86f8727..1f5fa376 100644 --- a/ic-utils/src/interfaces/management_canister/builders.rs +++ b/ic-utils/src/interfaces/management_canister/builders.rs @@ -4,7 +4,7 @@ pub use super::attributes::{ ComputeAllocation, FreezingThreshold, MemoryAllocation, ReservedCyclesLimit, WasmMemoryLimit, }; -use super::{ChunkHash, ManagementCanister}; +use super::{ChunkHash, LogVisibility, ManagementCanister}; use crate::call::CallFuture; use crate::{ call::AsyncCall, canister::Argument, interfaces::management_canister::MgmtMethod, Canister, @@ -74,6 +74,11 @@ pub struct CanisterSettings { /// /// Must be a number between 0 and 2^48^ (i.e 256TB), inclusively. pub wasm_memory_limit: Option, + + /// The canister log visibility of the canister. + /// + /// If unspecified and a canister is being created with these settings, defaults to `Controllers`, i.e. private by default. + pub log_visibility: Option, } /// A builder for a `create_canister` call. @@ -87,6 +92,7 @@ pub struct CreateCanisterBuilder<'agent, 'canister: 'agent> { freezing_threshold: Option>, reserved_cycles_limit: Option>, wasm_memory_limit: Option>, + log_visibility: Option>, is_provisional_create: bool, amount: Option, specified_id: Option, @@ -104,6 +110,7 @@ impl<'agent, 'canister: 'agent> CreateCanisterBuilder<'agent, 'canister> { freezing_threshold: None, reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, is_provisional_create: false, amount: None, specified_id: None, @@ -318,6 +325,32 @@ impl<'agent, 'canister: 'agent> CreateCanisterBuilder<'agent, 'canister> { } } + /// Pass in a log visibility setting for the canister. + pub fn with_log_visibility(self, log_visibility: C) -> Self + where + E: std::fmt::Display, + C: TryInto, + { + self.with_optional_log_visibility(Some(log_visibility)) + } + + /// Pass in a log visibility optional setting for the canister. If this is [None], + /// it will revert the log visibility to default. + pub fn with_optional_log_visibility(self, log_visibility: Option) -> Self + where + E: std::fmt::Display, + C: TryInto, + { + Self { + log_visibility: log_visibility.map(|visibility| { + visibility + .try_into() + .map_err(|e| AgentError::MessageError(format!("{}", e))) + }), + ..self + } + } + /// Create an [AsyncCall] implementation that, when called, will create a /// canister. pub fn build(self) -> Result, AgentError> { @@ -351,6 +384,11 @@ impl<'agent, 'canister: 'agent> CreateCanisterBuilder<'agent, 'canister> { Some(Ok(x)) => Some(Nat::from(u64::from(x))), None => None, }; + let log_visibility = match self.log_visibility { + Some(Err(x)) => return Err(AgentError::MessageError(format!("{}", x))), + Some(Ok(x)) => Some(x), + None => None, + }; #[derive(Deserialize, CandidType)] struct Out { @@ -373,6 +411,7 @@ impl<'agent, 'canister: 'agent> CreateCanisterBuilder<'agent, 'canister> { freezing_threshold, reserved_cycles_limit, wasm_memory_limit, + log_visibility, }, specified_id: self.specified_id, }; @@ -390,6 +429,7 @@ impl<'agent, 'canister: 'agent> CreateCanisterBuilder<'agent, 'canister> { freezing_threshold, reserved_cycles_limit, wasm_memory_limit, + log_visibility, }) .with_effective_canister_id(self.effective_canister_id) }; @@ -899,6 +939,7 @@ pub struct UpdateCanisterBuilder<'agent, 'canister: 'agent> { freezing_threshold: Option>, reserved_cycles_limit: Option>, wasm_memory_limit: Option>, + log_visibility: Option>, } impl<'agent, 'canister: 'agent> UpdateCanisterBuilder<'agent, 'canister> { @@ -913,6 +954,7 @@ impl<'agent, 'canister: 'agent> UpdateCanisterBuilder<'agent, 'canister> { freezing_threshold: None, reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, } } @@ -1081,6 +1123,32 @@ impl<'agent, 'canister: 'agent> UpdateCanisterBuilder<'agent, 'canister> { } } + /// Pass in a log visibility setting for the canister. + pub fn with_log_visibility(self, log_visibility: C) -> Self + where + E: std::fmt::Display, + C: TryInto, + { + self.with_optional_log_visibility(Some(log_visibility)) + } + + /// Pass in a log visibility optional setting for the canister. If this is [None], + /// leaves the log visibility unchanged. + pub fn with_optional_log_visibility(self, log_visibility: Option) -> Self + where + E: std::fmt::Display, + C: TryInto, + { + Self { + log_visibility: log_visibility.map(|limit| { + limit + .try_into() + .map_err(|e| AgentError::MessageError(format!("{}", e))) + }), + ..self + } + } + /// Create an [AsyncCall] implementation that, when called, will update a /// canisters settings. pub fn build(self) -> Result, AgentError> { @@ -1120,6 +1188,11 @@ impl<'agent, 'canister: 'agent> UpdateCanisterBuilder<'agent, 'canister> { Some(Ok(x)) => Some(Nat::from(u64::from(x))), None => None, }; + let log_visibility = match self.log_visibility { + Some(Err(x)) => return Err(AgentError::MessageError(format!("{}", x))), + Some(Ok(x)) => Some(x), + None => None, + }; Ok(self .canister @@ -1133,6 +1206,7 @@ impl<'agent, 'canister: 'agent> UpdateCanisterBuilder<'agent, 'canister> { freezing_threshold, reserved_cycles_limit, wasm_memory_limit, + log_visibility, }, }) .with_effective_canister_id(self.canister_id) diff --git a/ic-utils/src/interfaces/wallet.rs b/ic-utils/src/interfaces/wallet.rs index 514b2637..5591fa4b 100644 --- a/ic-utils/src/interfaces/wallet.rs +++ b/ic-utils/src/interfaces/wallet.rs @@ -673,6 +673,7 @@ impl<'agent> WalletCanister<'agent> { freezing_threshold: freezing_threshold.map(u64::from).map(Nat::from), reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, }; self.update("wallet_create_canister") @@ -703,6 +704,7 @@ impl<'agent> WalletCanister<'agent> { freezing_threshold: freezing_threshold.map(u64::from).map(Nat::from), reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, }; self.update("wallet_create_canister128") @@ -717,9 +719,9 @@ impl<'agent> WalletCanister<'agent> { /// as the wallet does not support the setting. If you need to create a canister /// with a `reserved_cycles_limit` set, use the management canister. /// - /// This method does not have a `wasm_memory_limit` parameter, + /// This method does not have a `wasm_memory_limit` or `log_visibility` parameter, /// as the wallet does not support the setting. If you need to create a canister - /// with a `wasm_memory_limit` set, use the management canister. + /// with a `wasm_memory_limit` or `log_visibility` set, use the management canister. pub async fn wallet_create_canister( &self, cycles: u128, @@ -831,6 +833,7 @@ impl<'agent> WalletCanister<'agent> { freezing_threshold: freezing_threshold.map(u64::from).map(Nat::from), reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, }; self.update("wallet_create_wallet") @@ -861,6 +864,7 @@ impl<'agent> WalletCanister<'agent> { freezing_threshold: freezing_threshold.map(u64::from).map(Nat::from), reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, }; self.update("wallet_create_wallet128") diff --git a/ref-tests/tests/ic-ref.rs b/ref-tests/tests/ic-ref.rs index 0a4e7cf1..2bf4271e 100644 --- a/ref-tests/tests/ic-ref.rs +++ b/ref-tests/tests/ic-ref.rs @@ -724,6 +724,7 @@ mod management_canister { freezing_threshold: None, reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, }, }; @@ -1006,7 +1007,10 @@ mod extras { }; use ic_utils::{ call::AsyncCall, - interfaces::{management_canister::builders::ComputeAllocation, ManagementCanister}, + interfaces::{ + management_canister::{builders::ComputeAllocation, LogVisibility}, + ManagementCanister, + }, }; use ref_tests::get_effective_canister_id; use ref_tests::with_agent; @@ -1312,4 +1316,64 @@ mod extras { Ok(()) }) } + + #[ignore] + #[test] + fn create_with_log_visibility() { + with_agent(|agent| async move { + let ic00 = ManagementCanister::create(&agent); + + let (canister_id,) = ic00 + .create_canister() + .as_provisional_create_with_amount(None) + .with_effective_canister_id(get_effective_canister_id()) + .with_log_visibility(LogVisibility::Public) + .call_and_wait() + .await + .unwrap(); + + let result = ic00.canister_status(&canister_id).call_and_wait().await?; + assert_eq!(result.0.settings.log_visibility, LogVisibility::Public); + + Ok(()) + }) + } + + #[ignore] + #[test] + fn update_log_visibility() { + with_agent(|agent| async move { + let ic00 = ManagementCanister::create(&agent); + + let (canister_id,) = ic00 + .create_canister() + .as_provisional_create_with_amount(Some(20_000_000_000_000_u128)) + .with_effective_canister_id(get_effective_canister_id()) + .with_log_visibility(LogVisibility::Controllers) + .call_and_wait() + .await?; + + let result = ic00.canister_status(&canister_id).call_and_wait().await?; + assert_eq!(result.0.settings.log_visibility, LogVisibility::Controllers); + + ic00.update_settings(&canister_id) + .with_log_visibility(LogVisibility::Public) + .call_and_wait() + .await?; + + let result = ic00.canister_status(&canister_id).call_and_wait().await?; + assert_eq!(result.0.settings.log_visibility, LogVisibility::Public); + + let no_change: Option = None; + ic00.update_settings(&canister_id) + .with_optional_log_visibility(no_change) + .call_and_wait() + .await?; + + let result = ic00.canister_status(&canister_id).call_and_wait().await?; + assert_eq!(result.0.settings.log_visibility, LogVisibility::Public); + + Ok(()) + }) + } } diff --git a/ref-tests/tests/integration.rs b/ref-tests/tests/integration.rs index eb825755..8ad2b4ec 100644 --- a/ref-tests/tests/integration.rs +++ b/ref-tests/tests/integration.rs @@ -425,6 +425,7 @@ fn wallet_create_wallet() { freezing_threshold: None, reserved_cycles_limit: None, wasm_memory_limit: None, + log_visibility: None, }, }; let args = Argument::from_candid((create_args,));