refactor: [EXC-1572] move CanisterLog type to ic_types

rs/execution_environment/src/execution/install_code.rs

@@ -11,7 +11,7 @@ use ic_interfaces::execution_environment::{
 };
 use ic_logger::{error, fatal, info, warn};
 use ic_management_canister_types::{
-    CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallModeV2, CanisterLog,
+    CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallModeV2,
 };
 use ic_replicated_state::canister_state::system_state::ReservationError;
 use ic_replicated_state::metadata_state::subnet_call_context_manager::InstallCodeCallId;
@@ -20,7 +20,7 @@ use ic_state_layout::{CanisterLayout, CheckpointLayout, ReadOnly};
 use ic_sys::PAGE_SIZE;
 use ic_system_api::ExecutionParameters;
 use ic_types::{
-    funds::Cycles, messages::CanisterCall, CanisterTimer, ComputeAllocation, Height,
+    funds::Cycles, messages::CanisterCall, CanisterLog, CanisterTimer, ComputeAllocation, Height,
     MemoryAllocation, NumInstructions, Time,
 };
 use ic_wasm_types::WasmHash;

rs/execution_environment/tests/canister_logging.rs

@@ -5,7 +5,7 @@ use ic_config::subnet_config::SubnetConfig;
 use ic_management_canister_types::{
     CanisterIdRecord, CanisterInstallMode, CanisterLogRecord, CanisterSettingsArgs,
     CanisterSettingsArgsBuilder, DataSize, FetchCanisterLogsRequest, FetchCanisterLogsResponse,
-    LogVisibility, Payload, MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE,
+    LogVisibility, Payload,
 };
 use ic_registry_subnet_type::SubnetType;
 use ic_state_machine_tests::{
@@ -14,7 +14,9 @@ use ic_state_machine_tests::{
 };
 use ic_test_utilities_execution_environment::{get_reply, wat_canister, wat_fn};
 use ic_test_utilities_metrics::fetch_histogram_stats;
-use ic_types::{ingress::WasmResult, CanisterId, Cycles, NumInstructions};
+use ic_types::{
+    ingress::WasmResult, CanisterId, Cycles, NumInstructions, MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE,
+};
 use more_asserts::{assert_le, assert_lt};
 use proptest::{prelude::ProptestConfig, prop_assume};
 use std::time::{Duration, SystemTime};

rs/interfaces/src/execution_environment.rs

@@ -4,7 +4,7 @@ mod errors;
 pub use errors::{CanisterOutOfCyclesError, HypervisorError, TrapCode};
 use ic_base_types::NumBytes;
 use ic_error_types::UserError;
-use ic_management_canister_types::{CanisterLog, EcdsaKeyId};
+use ic_management_canister_types::EcdsaKeyId;
 use ic_registry_provisional_whitelist::ProvisionalWhitelist;
 use ic_registry_subnet_type::SubnetType;
 use ic_sys::{PageBytes, PageIndex};
@@ -16,12 +16,14 @@ use ic_types::{
         AnonymousQuery, AnonymousQueryResponse, CertificateDelegation, MessageId,
         SignedIngressContent, UserQuery,
     },
-    Cycles, ExecutionRound, Height, NumInstructions, NumOsPages, Randomness, Time,
+    CanisterLog, Cycles, ExecutionRound, Height, NumInstructions, NumOsPages, Randomness, Time,
 };
 use serde::{Deserialize, Serialize};
-use std::{collections::BTreeMap, ops};
-use std::{collections::BTreeSet, convert::TryFrom};
-use std::{convert::Infallible, fmt};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    convert::{Infallible, TryFrom},
+    fmt, ops,
+};
 use tower::util::BoxCloneService;
 
 /// Instance execution statistics. The stats are cumulative and

rs/replicated_state/src/canister_state.rs

@@ -8,16 +8,16 @@ use crate::canister_state::queues::CanisterOutputQueuesIterator;
 use crate::canister_state::system_state::{CanisterStatus, ExecutionTask, SystemState};
 use crate::{InputQueueType, StateError};
 pub use execution_state::{EmbedderCache, ExecutionState, ExportedFunctions, Global};
-use ic_management_canister_types::{CanisterLog, CanisterStatusType, LogVisibility};
+use ic_management_canister_types::{CanisterStatusType, LogVisibility};
 use ic_registry_subnet_type::SubnetType;
 use ic_types::batch::TotalQueryStats;
 use ic_types::methods::SystemMethod;
 use ic_types::time::UNIX_EPOCH;
 use ic_types::{
     messages::{CanisterMessage, Ingress, Request, RequestOrResponse, Response},
     methods::WasmMethod,
-    AccumulatedPriority, CanisterId, ComputeAllocation, ExecutionRound, MemoryAllocation, NumBytes,
-    PrincipalId, Time,
+    AccumulatedPriority, CanisterId, CanisterLog, ComputeAllocation, ExecutionRound,
+    MemoryAllocation, NumBytes, PrincipalId, Time,
 };
 use ic_types::{LongExecutionMode, NumInstructions};
 use phantom_newtype::AmountOf;

rs/replicated_state/src/canister_state/system_state.rs

@@ -12,7 +12,7 @@ pub use call_context_manager::{CallContext, CallContextAction, CallContextManage
 use ic_base_types::NumSeconds;
 use ic_logger::{error, ReplicaLogger};
 use ic_management_canister_types::{
-    CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterLog, LogVisibility,
+    CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, LogVisibility,
 };
 use ic_protobuf::proxy::{try_from_option_field, ProxyDecodeError};
 use ic_protobuf::state::canister_state_bits::v1 as pb;
@@ -22,7 +22,9 @@ use ic_types::messages::{
     Request, RequestOrResponse, Response, StopCanisterContext,
 };
 use ic_types::nominal_cycles::NominalCycles;
-use ic_types::{CanisterId, CanisterTimer, Cycles, MemoryAllocation, NumBytes, PrincipalId, Time};
+use ic_types::{
+    CanisterId, CanisterLog, CanisterTimer, Cycles, MemoryAllocation, NumBytes, PrincipalId, Time,
+};
 use lazy_static::lazy_static;
 use maplit::btreeset;
 use prometheus::IntCounter;

rs/state_layout/src/state_layout.rs

@@ -4,7 +4,7 @@ use crate::utils::do_copy;
 use ic_base_types::{NumBytes, NumSeconds};
 use ic_config::flag_status::FlagStatus;
 use ic_logger::{error, info, warn, ReplicaLogger};
-use ic_management_canister_types::{CanisterLog, LogVisibility};
+use ic_management_canister_types::LogVisibility;
 use ic_metrics::{buckets::decimal_buckets, MetricsRegistry};
 use ic_protobuf::{
     proxy::{try_from_option_field, ProxyDecodeError},
@@ -25,8 +25,8 @@ use ic_replicated_state::{
 use ic_sys::{fs::sync_path, mmap::ScopedMmap};
 use ic_types::{
     batch::TotalQueryStats, nominal_cycles::NominalCycles, AccumulatedPriority, CanisterId,
-    ComputeAllocation, Cycles, ExecutionRound, Height, MemoryAllocation, NumInstructions,
-    PrincipalId, SnapshotId, Time,
+    CanisterLog, ComputeAllocation, Cycles, ExecutionRound, Height, MemoryAllocation,
+    NumInstructions, PrincipalId, SnapshotId, Time,
 };
 use ic_utils::thread::parallel_map;
 use ic_wasm_types::{CanisterModule, WasmHash};

rs/state_machine_tests/src/lib.rs

@@ -33,7 +33,7 @@ use ic_interfaces_registry::RegistryClient;
 use ic_interfaces_state_manager::{CertificationScope, StateHashError, StateManager, StateReader};
 use ic_logger::ReplicaLogger;
 use ic_management_canister_types::{
-    self as ic00, CanisterIdRecord, CanisterLog, InstallCodeArgs, Method, Payload,
+    self as ic00, CanisterIdRecord, InstallCodeArgs, Method, Payload,
 };
 pub use ic_management_canister_types::{
     CanisterHttpResponsePayload, CanisterInstallMode, CanisterSettingsArgs, CanisterStatusResultV2,
@@ -114,7 +114,7 @@ use ic_types::{
         SignedIngressContent, UserQuery, EXPECTED_MESSAGE_ID_LENGTH,
     },
     xnet::StreamIndex,
-    CountBytes, CryptoHashOfPartialState, Height, NodeId, Randomness, RegistryVersion,
+    CanisterLog, CountBytes, CryptoHashOfPartialState, Height, NodeId, Randomness, RegistryVersion,
 };
 pub use ic_types::{
     ingress::{IngressState, IngressStatus, WasmResult},

rs/system_api/src/lib.rs

@@ -16,7 +16,6 @@ use ic_interfaces::execution_environment::{
     TrapCode::{self, CyclesAmountTooBigFor64Bit},
 };
 use ic_logger::{error, ReplicaLogger};
-use ic_management_canister_types::CanisterLog;
 use ic_registry_subnet_type::SubnetType;
 use ic_replicated_state::{
     canister_state::WASM_PAGE_SIZE_IN_BYTES, memory_required_to_push_request, Memory, NumWasmPages,
@@ -27,7 +26,7 @@ use ic_types::{
     ingress::WasmResult,
     messages::{CallContextId, RejectContext, Request, MAX_INTER_CANISTER_PAYLOAD_IN_BYTES},
     methods::{SystemMethod, WasmClosure},
-    CanisterId, CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumBytes,
+    CanisterId, CanisterLog, CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumBytes,
     NumInstructions, NumOsPages, PrincipalId, SubnetId, Time, MAX_STABLE_MEMORY_IN_BYTES,
 };
 use ic_utils::deterministic_operations::deterministic_copy_from_slice;

rs/system_api/src/sandbox_safe_system_state.rs

@@ -10,9 +10,8 @@ use ic_error_types::{ErrorCode, RejectCode, UserError};
 use ic_interfaces::execution_environment::{HypervisorError, HypervisorResult};
 use ic_logger::{info, ReplicaLogger};
 use ic_management_canister_types::{
-    CanisterLog, CreateCanisterArgs, InstallChunkedCodeArgs, InstallCodeArgsV2,
-    Method as Ic00Method, Payload, ProvisionalCreateCanisterWithCyclesArgs, UninstallCodeArgs,
-    UpdateSettingsArgs, IC_00,
+    CreateCanisterArgs, InstallChunkedCodeArgs, InstallCodeArgsV2, Method as Ic00Method, Payload,
+    ProvisionalCreateCanisterWithCyclesArgs, UninstallCodeArgs, UpdateSettingsArgs, IC_00,
 };
 use ic_nns_constants::CYCLES_MINTING_CANISTER_ID;
 use ic_registry_subnet_type::SubnetType;
@@ -24,7 +23,7 @@ use ic_types::{
     messages::{CallContextId, CallbackId, RejectContext, Request, RequestMetadata, NO_DEADLINE},
     methods::Callback,
     time::CoarseTime,
-    CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumInstructions, Time,
+    CanisterLog, CanisterTimer, ComputeAllocation, Cycles, MemoryAllocation, NumInstructions, Time,
 };
 use ic_wasm_types::WasmEngineError;
 use serde::{Deserialize, Serialize};

rs/system_api/tests/system_api.rs

@@ -9,7 +9,7 @@ use ic_interfaces::execution_environment::{
     SubnetAvailableMemory, SystemApi, TrapCode,
 };
 use ic_logger::replica_logger::no_op_logger;
-use ic_management_canister_types::{DataSize, MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE};
+use ic_management_canister_types::DataSize;
 use ic_registry_subnet_type::SubnetType;
 use ic_replicated_state::{
     testing::CanisterQueuesTesting, CallOrigin, Memory, NetworkTopology, SystemState,
@@ -33,6 +33,7 @@ use ic_types::{
     time,
     time::UNIX_EPOCH,
     CanisterTimer, CountBytes, Cycles, NumInstructions, PrincipalId, Time,
+    MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE,
 };
 use std::{
     collections::BTreeSet,

rs/types/management_canister_types/src/lib.rs

@@ -31,14 +31,7 @@ pub use provisional::{ProvisionalCreateCanisterWithCyclesArgs, ProvisionalTopUpC
 use serde::Serialize;
 use serde_bytes::ByteBuf;
 use std::mem::size_of;
-use std::{
-    collections::{BTreeSet, VecDeque},
-    convert::TryFrom,
-    error::Error,
-    fmt,
-    slice::Iter,
-    str::FromStr,
-};
+use std::{collections::BTreeSet, convert::TryFrom, error::Error, fmt, slice::Iter, str::FromStr};
 use strum_macros::{Display, EnumIter, EnumString};
 
 /// The id of the management canister.
@@ -2556,115 +2549,6 @@ impl From<pb_canister_state_bits::CanisterLogRecord> for CanisterLogRecord {
     }
 }
 
-/// The maximum allowed size of a canister log buffer.
-pub const MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE: usize = 4 * 1024;
-
-// TODO(EXC-1572): needs refactoring to find a proper place to put this.
-/// Holds canister log records and keeps track of the next canister log record index.
-#[derive(Default, Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
-pub struct CanisterLog {
-    next_idx: u64,
-    records: VecDeque<CanisterLogRecord>,
-}
-
-impl CanisterLog {
-    /// Creates a new `CanisterLog` with the given next index and records.
-    pub fn new(next_idx: u64, records: Vec<CanisterLogRecord>) -> Self {
-        Self {
-            next_idx,
-            records: VecDeque::from(records),
-        }
-    }
-
-    /// Creates a new `CanisterLog` with the given next index and an empty records list.
-    pub fn new_with_next_index(next_idx: u64) -> Self {
-        Self {
-            next_idx,
-            records: Default::default(),
-        }
-    }
-
-    /// Returns the next canister log record index.
-    pub fn next_idx(&self) -> u64 {
-        self.next_idx
-    }
-
-    /// Returns the canister log records.
-    pub fn records(&self) -> &VecDeque<CanisterLogRecord> {
-        &self.records
-    }
-
-    /// Clears the canister log records.
-    pub fn clear(&mut self) {
-        self.records.clear();
-    }
-
-    /// Returns the maximum allowed size of a canister log buffer.
-    pub fn capacity(&self) -> usize {
-        MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE
-    }
-
-    /// Returns the used space in the canister log buffer.
-    pub fn used_space(&self) -> usize {
-        self.records.data_size()
-    }
-
-    /// Returns the remaining space in the canister log buffer.
-    pub fn remaining_space(&self) -> usize {
-        MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE.saturating_sub(self.records.data_size())
-    }
-
-    /// Removes old records to make enough free space for new data within the limit.
-    fn make_free_space_within_limit(&mut self, new_data_size: usize) {
-        let mut total_size = new_data_size + self.records.data_size();
-        while total_size > MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE {
-            if let Some(removed_record) = self.records.pop_front() {
-                total_size -= removed_record.data_size();
-            } else {
-                break; // No more records to pop, limit reached.
-            }
-        }
-    }
-
-    /// Adds a new log record.
-    pub fn add_record(&mut self, is_enabled: bool, timestamp_nanos: u64, content: &[u8]) {
-        if !is_enabled {
-            // If logging is disabled do not add new records,
-            // but still make sure the buffer is within limit.
-            self.make_free_space_within_limit(0);
-            return;
-        }
-
-        // LINT.IfChange
-        // Keep the new log record size within limit,
-        // this must be in sync with `logging_charge_bytes` in `system_api.rs`.
-        let max_content_size =
-            MAX_ALLOWED_CANISTER_LOG_BUFFER_SIZE - CanisterLogRecord::default().data_size();
-        let size = content.len().min(max_content_size);
-        let record = CanisterLogRecord {
-            idx: self.next_idx,
-            timestamp_nanos,
-            content: content[..size].to_vec(),
-        };
-        self.make_free_space_within_limit(record.data_size());
-        self.records.push_back(record);
-        // LINT.ThenChange(logging_charge_bytes_rule)
-        // Update the next canister log record index.
-        self.next_idx += 1;
-    }
-
-    /// Moves all the logs from `other` to `self`.
-    pub fn append(&mut self, other: &mut Self) {
-        // Assume records sorted cronologically (with increasing idx) and
-        // update the system state's next index with the last record's index.
-        if let Some(last) = other.records.back() {
-            self.next_idx = last.idx + 1;
-        }
-        self.make_free_space_within_limit(other.records.data_size());
-        self.records.append(&mut other.records);
-    }
-}
-
 /// `CandidType` for `FetchCanisterLogsResponse`
 /// ```text
 /// record {