diff --git a/Cargo.lock b/Cargo.lock index 8f87aff80..d2eed4200 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2429,9 +2429,9 @@ dependencies = [ [[package]] name = "ic-stable-structures" -version = "0.6.5" +version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03f3044466a69802de74e710dc0300b706a05696a0531c942ca856751a13b0db" +checksum = "fcaf89c1bc326c72498bcc0cd954f2edf718c018e7c586d2193d701d3c9af29a" dependencies = [ "ic_principal", ] diff --git a/Cargo.toml b/Cargo.toml index 5d6625d5e..12c521b1c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,7 @@ ic-cdk = "0.16.0" ic-cdk-macros = "0.16.0" ic-cdk-timers = "0.9.0" ic-ledger-types = "0.12.0" -ic-stable-structures = "0.6.4" +ic-stable-structures = "0.6.6" icrc-ledger-types = "0.1.6" ic-utils = "0.38" itertools = "0.13.0" diff --git a/core/station/impl/results.yml b/core/station/impl/results.yml index 0c4028f38..108226765 100644 --- a/core/station/impl/results.yml +++ b/core/station/impl/results.yml @@ -13,7 +13,7 @@ benches: scopes: {} find_500_external_canister_policies_from_50k_dataset: total: - instructions: 27465679 + instructions: 28629145 heap_increase: 0 stable_memory_increase: 0 scopes: {} @@ -25,32 +25,32 @@ benches: scopes: {} list_1k_requests: total: - instructions: 142176376 + instructions: 142128177 heap_increase: 14 stable_memory_increase: 0 scopes: {} list_external_canisters_with_all_statuses: total: - instructions: 213172583 + instructions: 213226507 heap_increase: 0 stable_memory_increase: 0 scopes: {} repository_find_1k_requests_from_10k_dataset_default_filters: total: - instructions: 92231168 + instructions: 92053986 heap_increase: 17 stable_memory_increase: 0 scopes: {} service_filter_5k_requests_from_100k_dataset: total: - instructions: 680919877 + instructions: 680738365 heap_increase: 106 stable_memory_increase: 16 scopes: {} service_find_all_requests_from_2k_dataset: total: - instructions: 275772307 + instructions: 275726235 heap_increase: 44 stable_memory_increase: 16 scopes: {} -version: 0.1.5 +version: 0.1.8 diff --git a/core/station/impl/src/mappers/account.rs b/core/station/impl/src/mappers/account.rs index e7005fee3..42d1ab359 100644 --- a/core/station/impl/src/mappers/account.rs +++ b/core/station/impl/src/mappers/account.rs @@ -216,3 +216,23 @@ impl From for ChangeAssets { } } } + +impl From for upgrader_api::MultiAssetAccount { + fn from(account: Account) -> Self { + Self { + id: Uuid::from_bytes(account.id).hyphenated().to_string(), + seed: account.seed, + assets: account + .assets + .iter() + .map(|account_asset| { + Uuid::from_bytes(account_asset.asset_id) + .hyphenated() + .to_string() + }) + .collect(), + name: account.name.clone(), + metadata: account.metadata.clone().into(), + } + } +} diff --git a/core/station/impl/src/mappers/asset.rs b/core/station/impl/src/mappers/asset.rs index 73a0795ef..f34f3dff0 100644 --- a/core/station/impl/src/mappers/asset.rs +++ b/core/station/impl/src/mappers/asset.rs @@ -26,3 +26,17 @@ impl From for AssetCallerPrivilegesDTO { } } } + +impl From for upgrader_api::Asset { + fn from(asset: Asset) -> Self { + upgrader_api::Asset { + id: Uuid::from_bytes(asset.id).hyphenated().to_string(), + blockchain: asset.blockchain.to_string(), + symbol: asset.symbol.clone(), + name: asset.name.clone(), + decimals: asset.decimals, + standards: asset.standards.iter().map(|s| s.to_string()).collect(), + metadata: asset.metadata.clone().into(), + } + } +} diff --git a/core/station/impl/src/models/request.rs b/core/station/impl/src/models/request.rs index 5e3932589..057b61727 100644 --- a/core/station/impl/src/models/request.rs +++ b/core/station/impl/src/models/request.rs @@ -198,6 +198,7 @@ fn validate_request_operation_foreign_keys( RequestOperation::ManageSystemInfo(_) => (), RequestOperation::Transfer(op) => { EnsureAccount::id_exists(&op.input.from_account_id)?; + EnsureAsset::id_exists(&op.input.from_asset_id)?; } RequestOperation::AddAccount(op) => { op.input.read_permission.validate()?; diff --git a/core/station/impl/src/repositories/account.rs b/core/station/impl/src/repositories/account.rs index 72a781bd5..9fa81c3fd 100644 --- a/core/station/impl/src/repositories/account.rs +++ b/core/station/impl/src/repositories/account.rs @@ -1,11 +1,14 @@ -use super::indexes::unique_index::UniqueIndexRepository; +use super::{indexes::unique_index::UniqueIndexRepository, InsertEntryObserverArgs}; use crate::{ core::{ metrics::ACCOUNT_METRICS, observer::Observer, utils::format_unique_string, with_memory_manager, Memory, ACCOUNT_MEMORY_ID, }, models::{indexes::unique_index::UniqueIndexKey, Account, AccountId, AccountKey}, - services::disaster_recovery_sync_accounts_and_assets_on_change, + services::{ + disaster_recovery_sync_accounts_and_assets_on_insert, + disaster_recovery_sync_accounts_and_assets_on_remove, + }, }; use ic_stable_structures::{memory_manager::VirtualMemory, StableBTreeMap}; use lazy_static::lazy_static; @@ -30,16 +33,21 @@ lazy_static! { #[derive(Debug)] pub struct AccountRepository { unique_index: UniqueIndexRepository, - change_observer: Observer<()>, + insert_observer: Observer>, + remove_observer: Observer<()>, } impl Default for AccountRepository { fn default() -> Self { - let mut change_observer = Observer::default(); - disaster_recovery_sync_accounts_and_assets_on_change(&mut change_observer); + let mut remove_observer = Observer::default(); + disaster_recovery_sync_accounts_and_assets_on_remove(&mut remove_observer); + + let mut insert_observer = Observer::default(); + disaster_recovery_sync_accounts_and_assets_on_insert(&mut insert_observer); Self { - change_observer, + insert_observer, + remove_observer, unique_index: UniqueIndexRepository::default(), } } @@ -94,9 +102,14 @@ impl Repository> for AccountRepositor self.save_entry_indexes(&value, prev.as_ref()); - self.change_observer.notify(&()); + let args = InsertEntryObserverArgs { + current: value, + prev, + }; - prev + self.insert_observer.notify(&args); + + args.prev }) } @@ -117,7 +130,7 @@ impl Repository> for AccountRepositor self.remove_entry_indexes(prev); } - self.change_observer.notify(&()); + self.remove_observer.notify(&()); prev }) @@ -153,13 +166,6 @@ impl AccountRepository { self.unique_index .get(&UniqueIndexKey::AccountName(format_unique_string(name))) } - - pub fn with_empty_observers() -> Self { - Self { - change_observer: Observer::default(), - ..Default::default() - } - } } #[derive(Debug, Clone)] diff --git a/core/station/impl/src/repositories/asset.rs b/core/station/impl/src/repositories/asset.rs index 4fc8f6870..e77c689e8 100644 --- a/core/station/impl/src/repositories/asset.rs +++ b/core/station/impl/src/repositories/asset.rs @@ -1,11 +1,14 @@ -use super::indexes::unique_index::UniqueIndexRepository; +use super::{indexes::unique_index::UniqueIndexRepository, InsertEntryObserverArgs}; use crate::{ core::{ cache::Cache, ic_cdk::api::print, metrics::ASSET_METRICS, observer::Observer, with_memory_manager, Memory, ASSET_MEMORY_ID, }, models::{indexes::unique_index::UniqueIndexKey, Asset, AssetId}, - services::disaster_recovery_sync_accounts_and_assets_on_change, + services::{ + disaster_recovery_sync_accounts_and_assets_on_insert, + disaster_recovery_sync_accounts_and_assets_on_remove, + }, }; use ic_stable_structures::{memory_manager::VirtualMemory, StableBTreeMap}; use lazy_static::lazy_static; @@ -33,16 +36,21 @@ lazy_static! { #[derive(Debug)] pub struct AssetRepository { unique_index: UniqueIndexRepository, - change_observer: Observer<()>, + insert_observer: Observer>, + remove_observer: Observer<()>, } impl Default for AssetRepository { fn default() -> Self { - let mut change_observer = Observer::default(); - disaster_recovery_sync_accounts_and_assets_on_change(&mut change_observer); + let mut remove_observer = Observer::default(); + disaster_recovery_sync_accounts_and_assets_on_remove(&mut remove_observer); + + let mut insert_observer = Observer::default(); + disaster_recovery_sync_accounts_and_assets_on_insert(&mut insert_observer); Self { - change_observer, + insert_observer, + remove_observer, unique_index: UniqueIndexRepository::default(), } } @@ -130,9 +138,14 @@ impl Repository> for AssetRepository { self.save_entry_indexes(&value, prev.as_ref()); - self.change_observer.notify(&()); + let args = InsertEntryObserverArgs { + current: value, + prev, + }; - prev + self.insert_observer.notify(&args); + + args.prev }) } @@ -153,7 +166,7 @@ impl Repository> for AssetRepository { self.remove_entry_indexes(prev); } - self.change_observer.notify(&()); + self.remove_observer.notify(&()); prev }) diff --git a/core/station/impl/src/repositories/mod.rs b/core/station/impl/src/repositories/mod.rs index 73bff8399..17e5d68ad 100644 --- a/core/station/impl/src/repositories/mod.rs +++ b/core/station/impl/src/repositories/mod.rs @@ -36,3 +36,8 @@ pub use asset::*; pub mod permission; pub mod indexes; + +pub struct InsertEntryObserverArgs { + pub current: T, + pub prev: Option, +} diff --git a/core/station/impl/src/services/disaster_recovery.rs b/core/station/impl/src/services/disaster_recovery.rs index 4175325d0..f03205d8b 100644 --- a/core/station/impl/src/services/disaster_recovery.rs +++ b/core/station/impl/src/services/disaster_recovery.rs @@ -8,8 +8,11 @@ use super::{SystemService, UserService, USER_SERVICE}; use crate::{ core::observer::Observer, errors::DisasterRecoveryError, - models::{User, UserStatus}, - repositories::{AccountRepository, AssetRepository, ACCOUNT_REPOSITORY, ASSET_REPOSITORY}, + models::{Account, Asset, User, UserStatus}, + repositories::{ + AccountRepository, AssetRepository, InsertEntryObserverArgs, ACCOUNT_REPOSITORY, + ASSET_REPOSITORY, + }, services::SYSTEM_SERVICE, }; use orbit_essentials::repository::Repository; @@ -41,36 +44,8 @@ impl DisasterRecoveryService { upgrader_canister_id, "set_disaster_recovery_accounts_and_assets", (upgrader_api::SetDisasterRecoveryAccountsAndAssetsInput { - accounts: accounts - .iter() - .map(|account| upgrader_api::MultiAssetAccount { - id: Uuid::from_bytes(account.id).hyphenated().to_string(), - seed: account.seed, - assets: account - .assets - .iter() - .map(|account_asset| { - Uuid::from_bytes(account_asset.asset_id) - .hyphenated() - .to_string() - }) - .collect(), - name: account.name.clone(), - metadata: account.metadata.clone().into(), - }) - .collect(), - assets: assets - .iter() - .map(|asset| upgrader_api::Asset { - id: Uuid::from_bytes(asset.id).hyphenated().to_string(), - blockchain: asset.blockchain.to_string(), - symbol: asset.symbol.clone(), - name: asset.name.clone(), - decimals: asset.decimals, - standards: asset.standards.iter().map(|s| s.to_string()).collect(), - metadata: asset.metadata.clone().into(), - }) - .collect(), + accounts: accounts.into_iter().map(Into::into).collect(), + assets: assets.into_iter().map(Into::into).collect(), },), ) .await @@ -201,13 +176,87 @@ pub fn disaster_recovery_observes_remove_user(observer: &mut Observer) { })); } -pub fn disaster_recovery_sync_accounts_and_assets_on_change(observer: &mut Observer<()>) { +#[cfg(test)] +thread_local! { + static SYNC_CALLED: std::cell::RefCell = const { std::cell::RefCell::new(0) }; +} + +pub fn disaster_recovery_sync_accounts_and_assets_on_remove(observer: &mut Observer<()>) { observer.add_listener(Box::new(|_| { if !SYSTEM_SERVICE.is_healthy() { // Skip syncing during system init return; } + #[cfg(test)] + SYNC_CALLED.with(|sync_called| { + *sync_called.borrow_mut() += 1; + }); + + crate::core::ic_cdk::spawn(async { + if let Err(error) = DISASTER_RECOVERY_SERVICE.sync_accounts_and_assets().await { + crate::core::ic_cdk::api::print(format!( + "Failed to sync accounts and assets: {}", + error, + )); + } + }); + })); +} + +/// A trait for comparing two values for equality in the context of Disaster Recovery. +/// Two values are considered equal if they are the same when serialized into the format +/// stored by the Upgrader. +pub trait SyncEq { + fn sync_eq(&self) -> bool; +} + +impl SyncEq for InsertEntryObserverArgs { + fn sync_eq(&self) -> bool { + if let Some(prev) = &self.prev { + let current_synced: upgrader_api::MultiAssetAccount = self.current.clone().into(); + let prev_synced: upgrader_api::MultiAssetAccount = prev.clone().into(); + + current_synced == prev_synced + } else { + false + } + } +} + +impl SyncEq for InsertEntryObserverArgs { + fn sync_eq(&self) -> bool { + if let Some(prev) = &self.prev { + let current_synced: upgrader_api::Asset = self.current.clone().into(); + let prev_synced: upgrader_api::Asset = prev.clone().into(); + + current_synced == prev_synced + } else { + false + } + } +} + +pub fn disaster_recovery_sync_accounts_and_assets_on_insert(observer: &mut Observer) +where + T: SyncEq, +{ + observer.add_listener(Box::new(|sync_cmp| { + if !SYSTEM_SERVICE.is_healthy() { + // Skip syncing during system init + return; + } + + if sync_cmp.sync_eq() { + // Skip syncing if the account or asset hasn't changed + return; + } + + #[cfg(test)] + SYNC_CALLED.with(|sync_called| { + *sync_called.borrow_mut() += 1; + }); + crate::core::ic_cdk::spawn(async { if let Err(error) = DISASTER_RECOVERY_SERVICE.sync_accounts_and_assets().await { crate::core::ic_cdk::api::print(format!( @@ -218,3 +267,135 @@ pub fn disaster_recovery_sync_accounts_and_assets_on_change(observer: &mut Obser }); })); } + +#[cfg(test)] +mod tests { + + use orbit_essentials::{model::ModelKey, repository::Repository}; + + use crate::{ + core::test_utils::init_canister_system, + models::{ + account_test_utils::mock_account, asset_test_utils::mock_asset, AccountAsset, + AccountBalance, + }, + repositories::{InsertEntryObserverArgs, ACCOUNT_REPOSITORY, ASSET_REPOSITORY}, + services::SyncEq, + }; + + use super::SYNC_CALLED; + + #[test] + fn test_account_eq() { + let prev_account = mock_account(); + let mut current_account = prev_account.clone(); + + assert!(!InsertEntryObserverArgs { + current: current_account.clone(), + prev: None, + } + .sync_eq()); + + assert!(InsertEntryObserverArgs { + current: current_account.clone(), + prev: Some(prev_account.clone()), + } + .sync_eq()); + + current_account.assets[0].balance = Some(AccountBalance { + balance: 1000u64.into(), + last_modification_timestamp: 1, + }); + + // Account has not changed as far as the sync is concerned + assert!(InsertEntryObserverArgs { + current: current_account.clone(), + prev: Some(prev_account.clone()), + } + .sync_eq()); + + current_account.assets.push(AccountAsset { + asset_id: [1; 16], + balance: None, + }); + + // Account has changed + assert!(!InsertEntryObserverArgs { + current: current_account.clone(), + prev: Some(prev_account.clone()), + } + .sync_eq()); + } + + #[test] + fn test_asset_eq() { + let prev_asset = mock_asset(); + let mut current_asset = prev_asset.clone(); + + assert!(!InsertEntryObserverArgs { + current: current_asset.clone(), + prev: None, + } + .sync_eq()); + + assert!(InsertEntryObserverArgs { + current: current_asset.clone(), + prev: Some(prev_asset.clone()), + } + .sync_eq()); + + current_asset + .metadata + .change(crate::models::ChangeMetadata::RemoveKeys(vec![ + "index_canister_id".to_string(), + ])); + + // Asset has changed + assert!(!InsertEntryObserverArgs { + current: current_asset.clone(), + prev: Some(prev_asset.clone()), + } + .sync_eq()); + } + + #[test] + fn test_sync_call() { + init_canister_system(); + + let mut asset = mock_asset(); + ASSET_REPOSITORY.insert(asset.key(), asset.clone()); + assert_eq!(SYNC_CALLED.with(|sync_called| *sync_called.borrow()), 1); + + let mut account = mock_account(); + ACCOUNT_REPOSITORY.insert(account.to_key(), account.clone()); + assert_eq!(SYNC_CALLED.with(|sync_called| *sync_called.borrow()), 2); + + account.assets[0].balance = Some(AccountBalance { + balance: 1000u64.into(), + last_modification_timestamp: 1, + }); + ACCOUNT_REPOSITORY.insert(account.to_key(), account.clone()); + // Account has not changed as far as the sync is concerned + assert_eq!(SYNC_CALLED.with(|sync_called| *sync_called.borrow()), 2); + + account.assets.push(AccountAsset { + asset_id: [1; 16], + balance: None, + }); + ACCOUNT_REPOSITORY.insert(account.to_key(), account.clone()); + // Account has changed + assert_eq!(SYNC_CALLED.with(|sync_called| *sync_called.borrow()), 3); + + asset + .metadata + .change(crate::models::ChangeMetadata::RemoveKeys(vec![ + "index_canister_id".to_string(), + ])); + ASSET_REPOSITORY.insert(asset.key(), asset.clone()); + // Asset has changed + assert_eq!(SYNC_CALLED.with(|sync_called| *sync_called.borrow()), 4); + + ASSET_REPOSITORY.remove(&asset.key()); + assert_eq!(SYNC_CALLED.with(|sync_called| *sync_called.borrow()), 5); + } +} diff --git a/core/upgrader/api/spec.did b/core/upgrader/api/spec.did index 9ba6dc764..e8f087033 100644 --- a/core/upgrader/api/spec.did +++ b/core/upgrader/api/spec.did @@ -296,4 +296,5 @@ service : (InitArg) -> { "get_disaster_recovery_state" : () -> (GetDisasterRecoveryStateResult) query; "request_disaster_recovery" : (RequestDisasterRecoveryInput) -> (RequestDisasterRecoveryResult); "get_logs" : (GetLogsInput) -> (GetLogsResult) query; + "deprecated_get_logs" : (GetLogsInput) -> (GetLogsResult) query; }; diff --git a/core/upgrader/api/src/lib.rs b/core/upgrader/api/src/lib.rs index 9eabc2b75..ef42fecb3 100644 --- a/core/upgrader/api/src/lib.rs +++ b/core/upgrader/api/src/lib.rs @@ -65,7 +65,7 @@ pub struct Account { pub metadata: Vec, } -#[derive(Clone, Debug, CandidType, Deserialize)] +#[derive(Clone, Debug, CandidType, Deserialize, PartialEq, Eq)] pub struct Asset { /// The asset id, which is a UUID. pub id: UuidDTO, @@ -86,7 +86,7 @@ pub struct Asset { pub metadata: Vec, } -#[derive(Clone, Debug, CandidType, Deserialize)] +#[derive(Clone, Debug, CandidType, Deserialize, PartialEq, Eq)] pub struct MultiAssetAccount { /// The account id, which is a UUID. pub id: UuidDTO, diff --git a/core/upgrader/impl/src/controllers/logs.rs b/core/upgrader/impl/src/controllers/logs.rs index 6e55bc133..962f01c2a 100644 --- a/core/upgrader/impl/src/controllers/logs.rs +++ b/core/upgrader/impl/src/controllers/logs.rs @@ -28,6 +28,13 @@ fn get_logs(input: upgrader_api::GetLogsInput) -> ApiResult ApiResult { + CONTROLLER.deprecated_get_logs(input) +} + pub struct LogsController { disaster_recover_service: Arc, logger_service: Arc, @@ -59,4 +66,31 @@ impl LogsController { Err(UpgraderApiError::Unauthorized.into()) } } + + // Supports fetching the logs from the deprecated log storage. + pub fn deprecated_get_logs( + &self, + input: upgrader_api::GetLogsInput, + ) -> ApiResult { + let caller = caller(); + + if is_controller(&caller) || self.disaster_recover_service.is_committee_member(&caller) { + let GetLogsResult { + logs, + next_offset, + total, + } = self.logger_service.deprecated_get_logs( + input.pagination.as_ref().and_then(|p| p.offset), + input.pagination.as_ref().and_then(|p| p.limit), + ); + + Ok(upgrader_api::GetLogsResponse { + logs: logs.into_iter().map(|l| l.into()).collect(), + total, + next_offset, + }) + } else { + Err(UpgraderApiError::Unauthorized.into()) + } + } } diff --git a/core/upgrader/impl/src/lib.rs b/core/upgrader/impl/src/lib.rs index b6c7379d5..4f3ea2a15 100644 --- a/core/upgrader/impl/src/lib.rs +++ b/core/upgrader/impl/src/lib.rs @@ -33,8 +33,9 @@ type LocalRef = &'static LocalKey>; const MEMORY_ID_TARGET_CANISTER_ID: u8 = 0; const MEMORY_ID_DISASTER_RECOVERY: u8 = 1; -const MEMORY_ID_LOG_INDEX: u8 = 2; -const MEMORY_ID_LOG_DATA: u8 = 3; +const DEPRECATED_MEMORY_ID_LOG_INDEX: u8 = 2; +const DEPRECATED_MEMORY_ID_LOG_DATA: u8 = 3; +const MEMORY_ID_LOGS: u8 = 4; thread_local! { static MEMORY_MANAGER: RefCell> = diff --git a/core/upgrader/impl/src/model/logging.rs b/core/upgrader/impl/src/model/logging.rs index 780277fdb..6ce78bbb5 100644 --- a/core/upgrader/impl/src/model/logging.rs +++ b/core/upgrader/impl/src/model/logging.rs @@ -1,4 +1,4 @@ -use crate::upgrader_ic_cdk::api::time; +use crate::upgrader_ic_cdk::next_time; use orbit_essentials::{storable, types::Timestamp, utils::timestamp_to_rfc3339}; use serde::Serialize; @@ -171,7 +171,7 @@ impl LogEntryType { impl LogEntry { pub fn try_from_entry_type(entry_type: LogEntryType) -> Result { Ok(LogEntry { - time: time(), + time: next_time(), entry_type: entry_type.to_type_string(), message: entry_type.to_message(), data_json: entry_type.to_json_string()?, diff --git a/core/upgrader/impl/src/services/logger.rs b/core/upgrader/impl/src/services/logger.rs index ccfe7d517..0ec46e613 100644 --- a/core/upgrader/impl/src/services/logger.rs +++ b/core/upgrader/impl/src/services/logger.rs @@ -1,25 +1,32 @@ use std::{cell::RefCell, sync::Arc}; -use ic_stable_structures::{memory_manager::MemoryId, Log}; +use ic_stable_structures::{memory_manager::MemoryId, BTreeMap, Log}; use lazy_static::lazy_static; +use orbit_essentials::types::Timestamp; use crate::{ model::{LogEntry, LogEntryType}, - Memory, MEMORY_ID_LOG_DATA, MEMORY_ID_LOG_INDEX, MEMORY_MANAGER, + Memory, DEPRECATED_MEMORY_ID_LOG_DATA, DEPRECATED_MEMORY_ID_LOG_INDEX, MEMORY_ID_LOGS, + MEMORY_MANAGER, }; pub const MAX_GET_LOGS_LIMIT: u64 = 100; pub const DEFAULT_GET_LOGS_LIMIT: u64 = 10; +pub const MAX_LOG_ENTRIES: u64 = 25000; thread_local! { - - static STORAGE: RefCell> = RefCell::new( - Log::init( - MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(MEMORY_ID_LOG_INDEX))), - MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(MEMORY_ID_LOG_DATA))), - ).expect("Failed to initialize log storage") - ); - + static DEPRECATED_STORAGE: RefCell> = RefCell::new( + Log::init( + MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(DEPRECATED_MEMORY_ID_LOG_INDEX))), + MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(DEPRECATED_MEMORY_ID_LOG_DATA))), + ).expect("Failed to initialize deprecated log storage") + ); + + static STORAGE: RefCell> = RefCell::new( + BTreeMap::init( + MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(MEMORY_ID_LOGS))), + ) + ); } lazy_static! { @@ -40,12 +47,12 @@ impl LoggerService { /// Tries to log an entry to the storage. pub fn try_log(&self, entry_type: LogEntryType) -> Result<(), String> { let entry = LogEntry::try_from_entry_type(entry_type)?; - STORAGE.with(|storage| { - storage - .borrow_mut() - .append(&entry) - .map_err(|err| format!("Failed to log entry: {:?}", err)) - })?; + STORAGE.with_borrow_mut(|storage| { + if storage.len() >= MAX_LOG_ENTRIES { + let _ = storage.pop_first(); + } + storage.insert(entry.time, entry); + }); Ok(()) } @@ -71,6 +78,47 @@ impl LoggerService { }; } + let offset = offset.unwrap_or(0); + let limit = limit + .unwrap_or(DEFAULT_GET_LOGS_LIMIT) + .min(MAX_GET_LOGS_LIMIT); + + let logs = borrowed + .iter() + .rev() + .skip(offset as usize) + .take(limit as usize) + .map(|(_, v)| v) + .collect::>(); + + let next_offset = if total > offset + limit { + Some(offset + limit) + } else { + None + }; + GetLogsResult { + logs, + total, + next_offset, + } + }) + } + + /// Returns logs from the deprecated storage starting from the end of the log. + pub fn deprecated_get_logs(&self, offset: Option, limit: Option) -> GetLogsResult { + DEPRECATED_STORAGE.with(|storage| { + let borrowed = storage.borrow(); + + let total = borrowed.len(); + + if total == 0 { + return GetLogsResult { + logs: vec![], + total, + next_offset: None, + }; + } + let offset = offset.unwrap_or(0); let limit = limit .unwrap_or(DEFAULT_GET_LOGS_LIMIT) @@ -153,4 +201,43 @@ mod tests { assert_eq!(result.next_offset, None); assert_eq!(result.logs[0].entry_type, "set_committee".to_owned()); } + + #[test] + fn test_log_trimming() { + for _ in 0..MAX_LOG_ENTRIES { + LOGGER_SERVICE.log(LogEntryType::SetCommittee(SetCommitteeLog { + committee: mock_committee(), + })); + } + + let result = LOGGER_SERVICE.get_logs(None, None); + assert_eq!(result.total, MAX_LOG_ENTRIES); + + let latest_log_time = result.logs.last().unwrap().time; + + LOGGER_SERVICE.log(LogEntryType::SetCommittee(SetCommitteeLog { + committee: mock_committee(), + })); + + let result = LOGGER_SERVICE.get_logs(None, None); + + assert_eq!(result.total, MAX_LOG_ENTRIES); + assert_ne!(result.logs.last().unwrap().time, latest_log_time); + } + + #[test] + fn test_deprecated_storage() { + let logger_service = LoggerService::default(); + logger_service.log(LogEntryType::SetCommittee(SetCommitteeLog { + committee: mock_committee(), + })); + + // new logs should be in the new storage + let result = logger_service.get_logs(None, None); + assert_eq!(result.total, 1); + + // deprecated logs should not get new logs + let result = logger_service.deprecated_get_logs(None, None); + assert_eq!(result.total, 0); + } } diff --git a/scripts/benchmark-canister.sh b/scripts/benchmark-canister.sh index 841225d99..5781fa05c 100755 --- a/scripts/benchmark-canister.sh +++ b/scripts/benchmark-canister.sh @@ -25,7 +25,7 @@ print_message "Benchmarking canister at $CANISTER_PATH" # Install canbench if not already installed if ! cargo install --list | grep -q canbench; then print_message "Installing canbench..." - cargo install canbench --version 0.1.5 + cargo install canbench --version 0.1.8 fi # Changes to the canister path @@ -42,8 +42,8 @@ canbench --less-verbose >"$CANBENCH_TMP_OUTPUT" if grep -q "(regress\|(improved by \|(new)" "$CANBENCH_TMP_OUTPUT"; then # Check if running in GitHub Actions and print the CANBENCH_TMP_OUTPUT file if so if [ "${GITHUB_ACTIONS:-}" = "true" ]; then - print_message "Review the benchmark results below:" - cat "$CANBENCH_TMP_OUTPUT" + print_message "Review the benchmark results below:" + cat "$CANBENCH_TMP_OUTPUT" fi print_message "Benchmarking completed.