diff --git a/src/internet_identity/src/activity_stats.rs b/src/internet_identity/src/activity_stats.rs index 09d17868d6..76fc2ace40 100644 --- a/src/internet_identity/src/activity_stats.rs +++ b/src/internet_identity/src/activity_stats.rs @@ -3,7 +3,6 @@ use crate::activity_stats::activity_counter::ActivityCounter; use crate::state; use crate::storage::anchor::{Anchor, Device}; use candid::{CandidType, Deserialize}; -use ic_cdk::api::time; use internet_identity_interface::internet_identity::types::Timestamp; pub mod activity_counter; @@ -86,10 +85,9 @@ pub struct OngoingActivityStats { pub fn update_activity_stats(anchor: &Anchor, current_device: &Device) { state::persistent_state_mut(|persistent_state| { // Active anchor stats across all domains - let active_anchor_stats = persistent_state + persistent_state .active_anchor_stats - .get_or_insert_with(|| ActivityStats::new(time())); - active_anchor_stats.update_counters(&anchor.last_activity()); + .update_counters(&anchor.last_activity()); // Active anchor stats, II domains only if let Some(domain) = current_device.ii_domain() { @@ -97,17 +95,14 @@ pub fn update_activity_stats(anchor: &Anchor, current_device: &Device) { anchor, current_domain: domain, }; - let domain_active_anchor_stats = persistent_state + persistent_state .domain_active_anchor_stats - .get_or_insert_with(|| ActivityStats::new(time())); - domain_active_anchor_stats.update_counters(&context); + .update_counters(&context); // Active authn methods stats, II domains only - let authn_method_stats = persistent_state + persistent_state .active_authn_method_stats - .get_or_insert_with(|| ActivityStats::new(time())); - - authn_method_stats.update_counters(¤t_device); + .update_counters(¤t_device); } }) } diff --git a/src/internet_identity/src/anchor_management/registration.rs b/src/internet_identity/src/anchor_management/registration.rs index 8fc70e7c51..e49c6f85db 100644 --- a/src/internet_identity/src/anchor_management/registration.rs +++ b/src/internet_identity/src/anchor_management/registration.rs @@ -1,5 +1,5 @@ use crate::anchor_management::{activity_bookkeeping, post_operation_bookkeeping}; -use crate::state::{ChallengeInfo, DEFAULT_MAX_INFLIGHT_CAPTCHAS}; +use crate::state::ChallengeInfo; use crate::storage::anchor::Device; use crate::{random_salt, secs_to_nanos, state}; use candid::Principal; @@ -26,11 +26,7 @@ pub async fn create_challenge() -> Challenge { // Error out if there are too many inflight challenges if inflight_challenges.len() - >= state::persistent_state_mut(|state| { - *state - .max_inflight_captchas - .get_or_insert(DEFAULT_MAX_INFLIGHT_CAPTCHAS) - }) as usize + >= state::persistent_state(|s| s.max_inflight_captchas) as usize { trap("too many inflight captchas"); } diff --git a/src/internet_identity/src/anchor_management/registration/rate_limit.rs b/src/internet_identity/src/anchor_management/registration/rate_limit.rs index 85dfa5ff52..d71437d0d8 100644 --- a/src/internet_identity/src/anchor_management/registration/rate_limit.rs +++ b/src/internet_identity/src/anchor_management/registration/rate_limit.rs @@ -16,10 +16,7 @@ use std::cmp::min; /// There is a maximum of `max_tokens` tokens, when reached the tokens not increase any further. /// This is the maximum number of calls that can be handled in a burst. pub fn process_rate_limit() { - let Some(config) = state::persistent_state(|ps| ps.registration_rate_limit.clone()) else { - // rate limit disabled -> nothing to do - return; - }; + let config = state::persistent_state(|ps| ps.registration_rate_limit.clone()); state::registration_rate_limit_mut(|state_opt| { let state = if let Some(state) = state_opt { diff --git a/src/internet_identity/src/delegation.rs b/src/internet_identity/src/delegation.rs index b1bb3156fd..2356156ade 100644 --- a/src/internet_identity/src/delegation.rs +++ b/src/internet_identity/src/delegation.rs @@ -87,9 +87,7 @@ fn update_latest_delegation_origins(frontend: FrontendHostname) { let now_ns = time(); persistent_state_mut(|persistent_state| { - let latest_delegation_origins = persistent_state - .latest_delegation_origins - .get_or_insert(HashMap::new()); + let latest_delegation_origins = &mut persistent_state.latest_delegation_origins; if let Some(timestamp_ns) = latest_delegation_origins.get_mut(&frontend) { *timestamp_ns = now_ns; @@ -102,7 +100,7 @@ fn update_latest_delegation_origins(frontend: FrontendHostname) { // if we still have too many entries, drop the oldest if latest_delegation_origins.len() as u64 - > persistent_state.max_num_latest_delegation_origins.unwrap() + > persistent_state.max_num_latest_delegation_origins { // if this case is hit often (i.e. we routinely have more than 1000 entries), we should // consider using a more efficient data structure diff --git a/src/internet_identity/src/http/metrics.rs b/src/internet_identity/src/http/metrics.rs index d397eca7fc..02f09b7374 100644 --- a/src/internet_identity/src/http/metrics.rs +++ b/src/internet_identity/src/http/metrics.rs @@ -138,106 +138,103 @@ fn persistent_state_metrics( w: &mut MetricsEncoder>, persistent_state: &PersistentState, ) -> Result<(), std::io::Error> { - if let Some(ref register_rate_limit_config) = persistent_state.registration_rate_limit { + let register_rate_limit_config = &persistent_state.registration_rate_limit; + w.encode_gauge( + "internet_identity_register_rate_limit_max_tokens", + register_rate_limit_config.max_tokens as f64, + "The maximum number of `register` calls that are allowed in any time window.", + )?; + w.encode_gauge( + "internet_identity_register_rate_limit_time_per_tokens_seconds", + Duration::from_nanos(register_rate_limit_config.time_per_token_ns).as_secs() as f64, + "Min number of seconds between two register calls to not exceed the rate limit (sustained).", + )?; + + let stats = &persistent_state.active_anchor_stats; + if let Some(ref daily_active_anchor_stats) = stats.completed.daily_events { w.encode_gauge( - "internet_identity_register_rate_limit_max_tokens", - register_rate_limit_config.max_tokens as f64, - "The maximum number of `register` calls that are allowed in any time window.", + "internet_identity_daily_active_anchors", + daily_active_anchor_stats.counter as f64, + "The number of unique active anchors in the last completed 24h collection window.", )?; w.encode_gauge( - "internet_identity_register_rate_limit_time_per_tokens_seconds", - Duration::from_nanos(register_rate_limit_config.time_per_token_ns).as_secs() as f64, - "Min number of seconds between two register calls to not exceed the rate limit (sustained).", + "internet_identity_daily_active_anchors_start_timestamp_seconds", + Duration::from_nanos(daily_active_anchor_stats.start_timestamp).as_secs() as f64, + "Timestamp of the last completed 24h collection window for unique active anchors.", )?; } - if let Some(ref stats) = persistent_state.active_anchor_stats { - if let Some(ref daily_active_anchor_stats) = stats.completed.daily_events { - w.encode_gauge( - "internet_identity_daily_active_anchors", - daily_active_anchor_stats.counter as f64, - "The number of unique active anchors in the last completed 24h collection window.", - )?; - w.encode_gauge( - "internet_identity_daily_active_anchors_start_timestamp_seconds", - Duration::from_nanos(daily_active_anchor_stats.start_timestamp).as_secs() as f64, - "Timestamp of the last completed 24h collection window for unique active anchors.", - )?; - } - if let Some(ref monthly_active_anchor_stats) = stats.completed.monthly_events { - w.encode_gauge( - "internet_identity_monthly_active_anchors", - monthly_active_anchor_stats.counter as f64, - "The number of unique active anchors in the last completed 30-day collection window.", - )?; - w.encode_gauge( - "internet_identity_monthly_active_anchors_start_timestamp_seconds", - Duration::from_nanos(monthly_active_anchor_stats.start_timestamp).as_secs() as f64, - "Timestamp of the last completed 30-day collection window for unique active anchors.", - )?; - } - }; - if let Some(ref stats) = persistent_state.domain_active_anchor_stats { - const BOTH_DOMAINS: &str = "both_ii_domains"; - - let labels = ActivityMetricsLabels { - daily_stats_label: "internet_identity_daily_active_anchors_by_domain", - daily_stats_doc: "The number of unique active anchors in the last completed 24h collection window aggregated by II domains used.", - monthly_stats_label: "internet_identity_monthly_active_anchors_by_domain", - monthly_stats_doc: "The number of unique active anchors in the last completed 30-day collection window aggregated by II domains used.", - }; - labelled_activity_metrics(w, stats, labels, |counter, encoder| { - encoder - .value( - &[("domain", IC0_APP_DOMAIN)], - counter.ic0_app_counter as f64, - )? - .value( - &[("domain", INTERNETCOMPUTER_ORG_DOMAIN)], - counter.internetcomputer_org_counter as f64, - )? - .value( - &[("domain", BOTH_DOMAINS)], - counter.both_ii_domains_counter as f64, - )?; - Ok(()) - })?; - }; - if let Some(ref stats) = persistent_state.active_authn_method_stats { - let labels = ActivityMetricsLabels { - daily_stats_label: "internet_identity_daily_active_authn_methods", - daily_stats_doc: "The number of unique authentication methods used in the last completed 24h collection window on II domains.", - monthly_stats_label: "internet_identity_monthly_active_authn_methods", - monthly_stats_doc: "The number of unique authentication methods used in the last completed 30-day collection window on II domains.", - }; - labelled_activity_metrics(w, stats, labels, |counter, encoder| { - encoder - .value( - &[("type", "webauthn_auth")], - counter.webauthn_auth_counter as f64, - )? - .value( - &[("type", "webauthn_recovery")], - counter.webauthn_recovery_counter as f64, - )? - .value( - &[("type", "recovery_phrase")], - counter.recovery_phrase_counter as f64, - )? - .value( - &[("type", "browser_storage_key")], - counter.browser_storage_key_counter as f64, - )? - .value(&[("type", "other")], counter.other_counter as f64)?; - Ok(()) - })?; - }; - if let Some(delegation_origins_limit) = persistent_state.max_num_latest_delegation_origins { + if let Some(ref monthly_active_anchor_stats) = stats.completed.monthly_events { + w.encode_gauge( + "internet_identity_monthly_active_anchors", + monthly_active_anchor_stats.counter as f64, + "The number of unique active anchors in the last completed 30-day collection window.", + )?; w.encode_gauge( - "internet_identity_max_num_latest_delegation_origins", - delegation_origins_limit as f64, - "The maximum number of latest delegation origins that were used with II bound devices.", + "internet_identity_monthly_active_anchors_start_timestamp_seconds", + Duration::from_nanos(monthly_active_anchor_stats.start_timestamp).as_secs() as f64, + "Timestamp of the last completed 30-day collection window for unique active anchors.", )?; } + let stats = &persistent_state.domain_active_anchor_stats; + const BOTH_DOMAINS: &str = "both_ii_domains"; + + let labels = ActivityMetricsLabels { + daily_stats_label: "internet_identity_daily_active_anchors_by_domain", + daily_stats_doc: "The number of unique active anchors in the last completed 24h collection window aggregated by II domains used.", + monthly_stats_label: "internet_identity_monthly_active_anchors_by_domain", + monthly_stats_doc: "The number of unique active anchors in the last completed 30-day collection window aggregated by II domains used.", + }; + labelled_activity_metrics(w, stats, labels, |counter, encoder| { + encoder + .value( + &[("domain", IC0_APP_DOMAIN)], + counter.ic0_app_counter as f64, + )? + .value( + &[("domain", INTERNETCOMPUTER_ORG_DOMAIN)], + counter.internetcomputer_org_counter as f64, + )? + .value( + &[("domain", BOTH_DOMAINS)], + counter.both_ii_domains_counter as f64, + )?; + Ok(()) + })?; + + let stats = &persistent_state.active_authn_method_stats; + let labels = ActivityMetricsLabels { + daily_stats_label: "internet_identity_daily_active_authn_methods", + daily_stats_doc: "The number of unique authentication methods used in the last completed 24h collection window on II domains.", + monthly_stats_label: "internet_identity_monthly_active_authn_methods", + monthly_stats_doc: "The number of unique authentication methods used in the last completed 30-day collection window on II domains.", + }; + labelled_activity_metrics(w, stats, labels, |counter, encoder| { + encoder + .value( + &[("type", "webauthn_auth")], + counter.webauthn_auth_counter as f64, + )? + .value( + &[("type", "webauthn_recovery")], + counter.webauthn_recovery_counter as f64, + )? + .value( + &[("type", "recovery_phrase")], + counter.recovery_phrase_counter as f64, + )? + .value( + &[("type", "browser_storage_key")], + counter.browser_storage_key_counter as f64, + )? + .value(&[("type", "other")], counter.other_counter as f64)?; + Ok(()) + })?; + + w.encode_gauge( + "internet_identity_max_num_latest_delegation_origins", + persistent_state.max_num_latest_delegation_origins as f64, + "The maximum number of latest delegation origins that were used with II bound devices.", + )?; Ok(()) } diff --git a/src/internet_identity/src/main.rs b/src/internet_identity/src/main.rs index ccfcf6b5b4..544b83d865 100644 --- a/src/internet_identity/src/main.rs +++ b/src/internet_identity/src/main.rs @@ -340,15 +340,10 @@ fn stats() -> InternetIdentityStats { state::persistent_state(|persistent_state| { let origins = persistent_state .latest_delegation_origins - .as_ref() - .map(|latest_delegation_origins| { - latest_delegation_origins.keys().cloned().collect() - }) - .unwrap_or_default(); - ( - origins, - persistent_state.max_num_latest_delegation_origins.unwrap(), - ) + .keys() + .cloned() + .collect(); + (origins, persistent_state.max_num_latest_delegation_origins) }); state::storage_borrow(|storage| InternetIdentityStats { @@ -402,15 +397,12 @@ fn init(maybe_arg: Option) { fn post_upgrade(maybe_arg: Option) { init_assets(); state::init_from_stable_memory(); - // immediately migrate the persistent state to the new layout - state::storage_borrow_mut(|storage| storage.migrate_persistent_state()); + // load the persistent state after initializing storage, otherwise the memory address to load it from cannot be calculated + state::load_persistent_state(); // We drop all the signatures on upgrade, users will // re-request them if needed. update_root_hash(); - // load the persistent state after initializing storage, otherwise the memory address to load it from cannot be calculated - state::load_persistent_state(); - apply_install_arg(maybe_arg); } @@ -431,17 +423,17 @@ fn apply_install_arg(maybe_arg: Option) { } if let Some(rate_limit) = arg.register_rate_limit { state::persistent_state_mut(|persistent_state| { - persistent_state.registration_rate_limit = Some(rate_limit); + persistent_state.registration_rate_limit = rate_limit; }) } if let Some(limit) = arg.max_num_latest_delegation_origins { state::persistent_state_mut(|persistent_state| { - persistent_state.max_num_latest_delegation_origins = Some(limit); + persistent_state.max_num_latest_delegation_origins = limit; }) } if let Some(limit) = arg.max_inflight_captchas { state::persistent_state_mut(|persistent_state| { - persistent_state.max_inflight_captchas = Some(limit); + persistent_state.max_inflight_captchas = limit; }) } } diff --git a/src/internet_identity/src/state.rs b/src/internet_identity/src/state.rs index 9ffe69a0f9..1ad655497e 100644 --- a/src/internet_identity/src/state.rs +++ b/src/internet_identity/src/state.rs @@ -10,7 +10,6 @@ use crate::{random_salt, Storage}; use asset_util::CertifiedAssets; use candid::{CandidType, Deserialize}; use canister_sig_util::signature_map::SignatureMap; -use ic_cdk::api::time; use ic_cdk::trap; use ic_stable_structures::DefaultMemoryImpl; use internet_identity_interface::internet_identity::types::*; @@ -90,37 +89,50 @@ pub struct PersistentState { // Amount of cycles that need to be attached when II creates a canister pub canister_creation_cycles_cost: u64, // Configuration for the rate limit on `register`, if any. - pub registration_rate_limit: Option, + pub registration_rate_limit: RateLimitConfig, // Daily and monthly active anchor statistics - pub active_anchor_stats: Option>, + pub active_anchor_stats: ActivityStats, // Daily and monthly active anchor statistics (filtered by domain) - pub domain_active_anchor_stats: Option>, + pub domain_active_anchor_stats: ActivityStats, // Daily and monthly active authentication methods on the II domains. - pub active_authn_method_stats: Option>, + pub active_authn_method_stats: ActivityStats, // Hashmap of last used delegation origins - pub latest_delegation_origins: Option>, + pub latest_delegation_origins: HashMap, // Maximum number of latest delegation origins to store - pub max_num_latest_delegation_origins: Option, + pub max_num_latest_delegation_origins: u64, // Maximum number of inflight captchas - pub max_inflight_captchas: Option, + pub max_inflight_captchas: u64, } impl Default for PersistentState { fn default() -> Self { + let time = time(); Self { archive_state: ArchiveState::default(), canister_creation_cycles_cost: 0, - registration_rate_limit: None, - active_anchor_stats: None, - domain_active_anchor_stats: None, - active_authn_method_stats: None, - latest_delegation_origins: None, - max_num_latest_delegation_origins: Some(DEFAULT_MAX_DELEGATION_ORIGINS), - max_inflight_captchas: Some(DEFAULT_MAX_INFLIGHT_CAPTCHAS), + registration_rate_limit: DEFAULT_RATE_LIMIT_CONFIG, + active_anchor_stats: ActivityStats::new(time), + domain_active_anchor_stats: ActivityStats::new(time), + active_authn_method_stats: ActivityStats::new(time), + latest_delegation_origins: HashMap::new(), + max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, + max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, } } } +#[cfg(not(test))] +fn time() -> Timestamp { + ic_cdk::api::time() +} + +/// This is required because [ic_cdk::api::time()] traps when executed in a non-canister environment. +#[cfg(test)] +fn time() -> Timestamp { + // Return a fixed time for testing + 1709647706487990000 // Tue Mar 05 2024 14:08:26 GMT+0000 +} + #[derive(Clone, Debug, CandidType, Deserialize)] pub struct RateLimitState { // Number of tokens available for calls, where each call will deduct one token. If tokens reaches @@ -241,19 +253,7 @@ pub fn save_persistent_state() { pub fn load_persistent_state() { STATE.with(|s| { - storage_borrow(|storage| match storage.read_persistent_state() { - Ok(loaded_state) => *s.persistent_state.borrow_mut() = loaded_state, - Err(err) => trap(&format!("failed to recover persistent state! Err: {err:?}")), - }) - }); - - // Initialize a sensible default for max_latest_delegation_origins - // if it is not set in the persistent state. - // This will allow us to later drop the opt and make the field u64. - persistent_state_mut(|persistent_state| { - persistent_state - .max_num_latest_delegation_origins - .get_or_insert(DEFAULT_MAX_DELEGATION_ORIGINS); + storage_borrow(|storage| *s.persistent_state.borrow_mut() = storage.read_persistent_state()) }); } diff --git a/src/internet_identity/src/storage.rs b/src/internet_identity/src/storage.rs index 7f673b66d2..04ed5f999a 100644 --- a/src/internet_identity/src/storage.rs +++ b/src/internet_identity/src/storage.rs @@ -32,10 +32,11 @@ //! ------------------------------------------- <- Start of wasm memory page 1 //! ``` //! -//! The second page and onwards is managed by the [MemoryManager] and is currently split into two -//! managed memories: +//! The second page and onwards is managed by the [MemoryManager] and is currently split into the +//! following managed memories: //! * Anchor memory: used to store the candid encoded anchors //! * Archive buffer memory: used to store the archive entries yet to be pulled by the archive canister +//! * Persistent state memory: used to store the [PersistentState] //! //! ### Anchor memory //! @@ -70,10 +71,8 @@ //! In order to keep state across upgrades that is not related to specific anchors (such as archive //! information) Internet Identity will serialize the [PersistentState] on upgrade and restore it //! again after the upgrade. -//! -//! The storage layout v8 and earlier use the first unused memory -//! location (after the anchor record of the highest allocated anchor number) to store it. -//! The storage layout v9 uses a separate virtual memory. +//! The [PersistentState] is stored in a [StableCell] in the virtual memory with id 2 managed using +//! the [MemoryManager]. //! //! ## Archive buffer memory //! @@ -92,7 +91,7 @@ use std::ops::RangeInclusive; use ic_cdk::api::trap; use ic_stable_structures::memory_manager::{MemoryId, MemoryManager, VirtualMemory}; -use ic_stable_structures::reader::{BufferedReader, Reader}; +use ic_stable_structures::reader::Reader; use ic_stable_structures::storable::Bound; use ic_stable_structures::writer::Writer; use ic_stable_structures::{Memory, RestrictedMemory, StableBTreeMap, StableCell, Storable}; @@ -115,18 +114,15 @@ mod storable_persistent_state; mod tests; /// * version 0: invalid -/// * version 1-7: no longer supported -/// * version 8: 4KB anchors, candid anchor record layout, persistent state with archive pull config, -/// with memory manager (from 2nd page on), archive entries buffer in stable memory1 -/// * version 9: same as 8, but with persistent state in separate virtual memory -const SUPPORTED_LAYOUT_VERSIONS: RangeInclusive = 8..=9; +/// * version 1-8: no longer supported +/// * version 9: 4KB anchors, candid anchor record layout, persistent state in virtual memory, +/// with memory manager (from 2nd page on), archive entries buffer in stable memory +const SUPPORTED_LAYOUT_VERSIONS: RangeInclusive = 9..=9; const DEFAULT_ENTRY_SIZE: u16 = 4096; const EMPTY_SALT: [u8; 32] = [0; 32]; const GB: u64 = 1 << 30; -const PERSISTENT_STATE_MAGIC: [u8; 4] = *b"IIPS"; // II Persistent State - /// MemoryManager parameters. const ANCHOR_MEMORY_INDEX: u8 = 0u8; const ARCHIVE_BUFFER_MEMORY_INDEX: u8 = 1u8; @@ -467,85 +463,17 @@ impl Storage { record_number as u64 * self.header.entry_size as u64 } - /// Returns the address of the first byte not yet allocated to a anchor. - /// This address exists even if the max anchor number has been reached, because there is a memory - /// reserve at the end of stable memory. - fn unused_memory_start(&self) -> u64 { - self.record_address(self.header.num_anchors) - } - pub fn write_persistent_state(&mut self, state: &PersistentState) { - match self.version() { - 8 => trap("persistent state must not be written before migration to layout version 9"), - 9 => { - self.persistent_state - .set(StorablePersistentState::from(state.clone())) - .expect("failed to write persistent state"); - } - version => trap(&format!("unsupported version: {}", version)), - }; + // The virtual memory is not limited in size, so for the expected size of the persistent + // this operation is infallible. The size of the persistent state is monitored and an alert + // is raised if the size exceeds the expected size. + self.persistent_state + .set(StorablePersistentState::from(state.clone())) + .expect("failed to write persistent state"); } - pub fn migrate_persistent_state(&mut self) { - let version = self.version(); - if version == 9 { - // nothing to migrate - return; - } - assert_eq!(version, 8); - let persistent_state = self - .read_persistent_state_v8() - .expect("failed to recover persistent state!"); - self.header.version = 9; - self.flush(); - self.write_persistent_state(&persistent_state); - } - - pub fn read_persistent_state(&self) -> Result { - match self.version() { - 8 => self.read_persistent_state_v8(), - 9 => Ok(PersistentState::from(self.persistent_state.get().clone())), - version => trap(&format!("unsupported version: {}", version)), - } - } - - /// Reads the persistent state from stable memory just outside the space allocated to the highest anchor number. - /// This is only used to restore state in `post_upgrade`. - fn read_persistent_state_v8(&self) -> Result { - let address = self.unused_memory_start(); - if address > self.anchor_memory.size() * WASM_PAGE_SIZE_IN_BYTES as u64 { - // the address where the persistent state would be is not allocated yet - return Err(PersistentStateError::NotFound); - } - - let mut reader = BufferedReader::new( - self.header.entry_size as usize, - Reader::new(&self.anchor_memory, address), - ); - let mut magic_buf: [u8; 4] = [0; 4]; - reader - .read_exact(&mut magic_buf) - // if we hit out of bounds here, this means that the persistent state has not been - // written at the expected location and thus cannot be found - .map_err(|_| PersistentStateError::NotFound)?; - - if magic_buf != PERSISTENT_STATE_MAGIC { - // magic does not match --> this is not the persistent state - return Err(PersistentStateError::NotFound); - } - - let mut size_buf: [u8; 8] = [0; 8]; - reader - .read_exact(&mut size_buf) - .map_err(PersistentStateError::ReadError)?; - - let size = u64::from_le_bytes(size_buf); - let mut data_buf = vec![0; size as usize]; - reader - .read_exact(data_buf.as_mut_slice()) - .map_err(PersistentStateError::ReadError)?; - - candid::decode_one(&data_buf).map_err(PersistentStateError::CandidError) + pub fn read_persistent_state(&self) -> PersistentState { + PersistentState::from(self.persistent_state.get().clone()) } pub fn version(&self) -> u8 { @@ -579,13 +507,6 @@ fn single_bucket_memory( ) } -#[derive(Debug)] -pub enum PersistentStateError { - CandidError(candid::error::Error), - NotFound, - ReadError(std::io::Error), -} - #[derive(Debug)] pub enum StorageError { AnchorNumberOutOfRange { diff --git a/src/internet_identity/src/storage/storable_persistent_state.rs b/src/internet_identity/src/storage/storable_persistent_state.rs index 5b9fcd2777..12c2fd4d16 100644 --- a/src/internet_identity/src/storage/storable_persistent_state.rs +++ b/src/internet_identity/src/storage/storable_persistent_state.rs @@ -3,10 +3,7 @@ use crate::activity_stats::activity_counter::authn_method_counter::AuthnMethodCo use crate::activity_stats::activity_counter::domain_active_anchor_counter::DomainActiveAnchorCounter; use crate::activity_stats::ActivityStats; use crate::archive::ArchiveState; -use crate::state::{ - PersistentState, DEFAULT_MAX_DELEGATION_ORIGINS, DEFAULT_MAX_INFLIGHT_CAPTCHAS, - DEFAULT_RATE_LIMIT_CONFIG, -}; +use crate::state::PersistentState; use candid::{CandidType, Deserialize}; use ic_stable_structures::storable::Bound; use ic_stable_structures::Storable; @@ -15,7 +12,6 @@ use internet_identity_interface::internet_identity::types::{ }; use std::borrow::Cow; use std::collections::HashMap; -use std::time::Duration; #[derive(Clone, CandidType, Deserialize, Eq, PartialEq, Debug)] pub struct StorablePersistentState { @@ -44,81 +40,38 @@ impl Storable for StorablePersistentState { impl Default for StorablePersistentState { fn default() -> Self { - let time = time(); - Self { - archive_state: ArchiveState::default(), - canister_creation_cycles_cost: 0, - registration_rate_limit: RateLimitConfig { - time_per_token_ns: Duration::from_secs(10).as_nanos() as u64, - max_tokens: 20_000, - }, - active_anchor_stats: ActivityStats::new(time), - domain_active_anchor_stats: ActivityStats::new(time), - active_authn_method_stats: ActivityStats::new(time), - latest_delegation_origins: HashMap::new(), - max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, - max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, - } + Self::from(PersistentState::default()) } } impl From for StorablePersistentState { - fn from(persistent_state: PersistentState) -> Self { - let time = time(); + fn from(s: PersistentState) -> Self { Self { - archive_state: persistent_state.archive_state, - canister_creation_cycles_cost: persistent_state.canister_creation_cycles_cost, - registration_rate_limit: persistent_state - .registration_rate_limit - .unwrap_or(DEFAULT_RATE_LIMIT_CONFIG), - active_anchor_stats: persistent_state - .active_anchor_stats - .unwrap_or(ActivityStats::new(time)), - domain_active_anchor_stats: persistent_state - .domain_active_anchor_stats - .unwrap_or(ActivityStats::new(time)), - active_authn_method_stats: persistent_state - .active_authn_method_stats - .unwrap_or(ActivityStats::new(time)), - latest_delegation_origins: persistent_state - .latest_delegation_origins - .unwrap_or_default(), - max_num_latest_delegation_origins: persistent_state - .max_num_latest_delegation_origins - .unwrap_or(DEFAULT_MAX_DELEGATION_ORIGINS), - max_inflight_captchas: persistent_state - .max_inflight_captchas - .unwrap_or(DEFAULT_MAX_INFLIGHT_CAPTCHAS), + archive_state: s.archive_state, + canister_creation_cycles_cost: s.canister_creation_cycles_cost, + registration_rate_limit: s.registration_rate_limit, + active_anchor_stats: s.active_anchor_stats, + domain_active_anchor_stats: s.domain_active_anchor_stats, + active_authn_method_stats: s.active_authn_method_stats, + latest_delegation_origins: s.latest_delegation_origins, + max_num_latest_delegation_origins: s.max_num_latest_delegation_origins, + max_inflight_captchas: s.max_inflight_captchas, } } } -#[cfg(not(test))] -fn time() -> Timestamp { - ic_cdk::api::time() -} - -/// This is required because [ic_cdk::api::time()] traps when executed in a non-canister environment. -#[cfg(test)] -fn time() -> Timestamp { - // Return a fixed time for testing - 1709647706487990000 // Tue Mar 05 2024 14:08:26 GMT+0000 -} - impl From for PersistentState { - fn from(storable_persistent_state: StorablePersistentState) -> Self { + fn from(s: StorablePersistentState) -> Self { Self { - archive_state: storable_persistent_state.archive_state, - canister_creation_cycles_cost: storable_persistent_state.canister_creation_cycles_cost, - registration_rate_limit: Some(storable_persistent_state.registration_rate_limit), - active_anchor_stats: Some(storable_persistent_state.active_anchor_stats), - domain_active_anchor_stats: Some(storable_persistent_state.domain_active_anchor_stats), - active_authn_method_stats: Some(storable_persistent_state.active_authn_method_stats), - latest_delegation_origins: Some(storable_persistent_state.latest_delegation_origins), - max_num_latest_delegation_origins: Some( - storable_persistent_state.max_num_latest_delegation_origins, - ), - max_inflight_captchas: Some(storable_persistent_state.max_inflight_captchas), + archive_state: s.archive_state, + canister_creation_cycles_cost: s.canister_creation_cycles_cost, + registration_rate_limit: s.registration_rate_limit, + active_anchor_stats: s.active_anchor_stats, + domain_active_anchor_stats: s.domain_active_anchor_stats, + active_authn_method_stats: s.active_authn_method_stats, + latest_delegation_origins: s.latest_delegation_origins, + max_num_latest_delegation_origins: s.max_num_latest_delegation_origins, + max_inflight_captchas: s.max_inflight_captchas, } } } @@ -126,6 +79,8 @@ impl From for PersistentState { #[cfg(test)] mod tests { use super::*; + use crate::state::{DEFAULT_MAX_DELEGATION_ORIGINS, DEFAULT_MAX_INFLIGHT_CAPTCHAS}; + use std::time::Duration; #[test] fn should_convert_storable_persistent_state_to_persistent_state() { @@ -139,6 +94,7 @@ mod tests { #[test] fn should_have_expected_default_values() { + let test_time = 1709647706487990000u64; let expected_defaults = StorablePersistentState { archive_state: ArchiveState::default(), canister_creation_cycles_cost: 0, @@ -146,34 +102,30 @@ mod tests { time_per_token_ns: Duration::from_secs(10).as_nanos() as u64, max_tokens: 20_000, }, - active_anchor_stats: ActivityStats::new(time()), - domain_active_anchor_stats: ActivityStats::new(time()), - active_authn_method_stats: ActivityStats::new(time()), + active_anchor_stats: ActivityStats::new(test_time), + domain_active_anchor_stats: ActivityStats::new(test_time), + active_authn_method_stats: ActivityStats::new(test_time), latest_delegation_origins: HashMap::new(), max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, }; - let storable_persistent_state = StorablePersistentState::default(); - assert_eq!(storable_persistent_state, expected_defaults); + assert_eq!(StorablePersistentState::default(), expected_defaults); let expected_defaults = PersistentState { archive_state: ArchiveState::default(), canister_creation_cycles_cost: 0, - registration_rate_limit: Some(RateLimitConfig { + registration_rate_limit: RateLimitConfig { time_per_token_ns: Duration::from_secs(10).as_nanos() as u64, max_tokens: 20_000, - }), - active_anchor_stats: Some(ActivityStats::new(time())), - domain_active_anchor_stats: Some(ActivityStats::new(time())), - active_authn_method_stats: Some(ActivityStats::new(time())), - latest_delegation_origins: Some(HashMap::new()), - max_num_latest_delegation_origins: Some(DEFAULT_MAX_DELEGATION_ORIGINS), - max_inflight_captchas: Some(DEFAULT_MAX_INFLIGHT_CAPTCHAS), + }, + active_anchor_stats: ActivityStats::new(test_time), + domain_active_anchor_stats: ActivityStats::new(test_time), + active_authn_method_stats: ActivityStats::new(test_time), + latest_delegation_origins: HashMap::new(), + max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, + max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, }; - assert_eq!( - PersistentState::from(storable_persistent_state), - expected_defaults - ); + assert_eq!(PersistentState::default(), expected_defaults); } } diff --git a/src/internet_identity/src/storage/tests.rs b/src/internet_identity/src/storage/tests.rs index 8ea3e4af13..6644e17920 100644 --- a/src/internet_identity/src/storage/tests.rs +++ b/src/internet_identity/src/storage/tests.rs @@ -3,8 +3,7 @@ use crate::activity_stats::{ActivityStats, CompletedActivityStats, OngoingActivi use crate::archive::{ArchiveData, ArchiveState}; use crate::state::PersistentState; use crate::storage::anchor::{Anchor, Device}; -use crate::storage::storable_persistent_state::StorablePersistentState; -use crate::storage::{Header, PersistentStateError, StorageError, MAX_ENTRIES}; +use crate::storage::{Header, StorageError, MAX_ENTRIES}; use crate::Storage; use candid::Principal; use ic_stable_structures::{Memory, VectorMemory}; @@ -42,19 +41,6 @@ fn should_serialize_header_v9() { assert_eq!(buf, hex::decode("49494309000000000100000000000000020000000000000000100505050505050505050505050505050505050505050505050505050505050505").unwrap()); } -#[test] -fn should_recover_header_from_memory_v8() { - let memory = VectorMemory::default(); - memory.grow(1); - memory.write(0, &hex::decode("494943080500000040e2010000000000f1fb090000000000000843434343434343434343434343434343434343434343434343434343434343430002000000000000000000000000000000000000000000000000").unwrap()); - - let storage = Storage::from_memory(memory); - assert_eq!(storage.assigned_anchor_number_range(), (123456, 654321)); - assert_eq!(storage.salt().unwrap(), &[67u8; 32]); - assert_eq!(storage.anchor_count(), 5); - assert_eq!(storage.version(), 8); -} - #[test] fn should_recover_header_from_memory_v9() { let memory = VectorMemory::default(); @@ -114,28 +100,16 @@ fn should_save_and_restore_persistent_state() { let persistent_state = sample_persistent_state(); storage.write_persistent_state(&persistent_state); - assert_eq!(storage.read_persistent_state().unwrap(), persistent_state); + assert_eq!(storage.read_persistent_state(), persistent_state); } #[test] -fn should_not_find_persistent_state_if_it_does_not_exist_v8() { - let memory = VectorMemory::default(); - memory.grow(1); - memory.write(0, &hex::decode("494943080500000040e2010000000000f1fb090000000000000843434343434343434343434343434343434343434343434343434343434343430002000000000000000000000000000000000000000000000000").unwrap()); - let mut storage = Storage::from_memory(memory.clone()); - storage.flush(); - - let result = storage.read_persistent_state(); - assert!(matches!(result, Err(PersistentStateError::NotFound))) -} - -#[test] -fn should_always_find_persistent_state_v9() { +fn should_read_default_persistent_state_from_new_storage() { let memory = VectorMemory::default(); let mut storage = Storage::new((10_000, 3_784_873), memory); storage.flush(); - assert!(storage.read_persistent_state().is_ok()); + assert_eq!(storage.read_persistent_state(), PersistentState::default()); } #[test] @@ -146,12 +120,12 @@ fn should_not_overwrite_persistent_state_with_next_anchor_v9() { storage.allocate_anchor().unwrap(); storage.write_persistent_state(&sample_persistent_state()); - assert!(storage.read_persistent_state().is_ok()); + assert_eq!(storage.read_persistent_state(), sample_persistent_state()); let anchor = storage.allocate_anchor().unwrap(); storage.write(anchor).unwrap(); - assert!(storage.read_persistent_state().is_ok()); + assert_eq!(storage.read_persistent_state(), sample_persistent_state()); } fn sample_device() -> Device { @@ -183,7 +157,7 @@ fn sample_persistent_state() -> PersistentState { }, }, canister_creation_cycles_cost: 12_346_000_000, - active_anchor_stats: Some(ActivityStats { + active_anchor_stats: ActivityStats { completed: CompletedActivityStats { daily_events: Some(ActiveAnchorCounter { start_timestamp: 965485, @@ -201,7 +175,7 @@ fn sample_persistent_state() -> PersistentState { counter: 66, }], }, - }), - ..PersistentState::from(StorablePersistentState::default()) + }, + ..PersistentState::default() } } diff --git a/src/internet_identity/stable_memory/README.md b/src/internet_identity/stable_memory/README.md index 388cdfd6c3..f71768d12c 100644 --- a/src/internet_identity/stable_memory/README.md +++ b/src/internet_identity/stable_memory/README.md @@ -9,8 +9,6 @@ These tests serve two purposes: The following stable memory backups are currently used: * `buffered_archive_entries_v9.bin.gz`: a backup with buffered archive entries. -* `clean_init_v8.bin.gz`: a clean initial state with storage layout v8. Used to test that II can be upgraded from v8 - storage layout. * `genesis-layout-migrated-to-v9.bin.gz`: a backup initially created with the first version of the stable memory layout and then incrementally migrated to the v8 layout. It contains a few well-known identities / devices, see `known_devices` in `tests/integration/stable_memory.rs`. * `genesis-memory-layout.bin`: a backup of the initial memory layout. Not migrated. Mainly used to test behavior with respect to outdated / unsupported memory layouts. * `multiple-recovery-phrases-v9.bin.gz`: a backup with an identity that has multiple recovery phrases. The input validation does no longer allow to create such an identity (only one recovery phrase is allowed). However, legacy users that are in that state need a way to make their identity consistent again. This backup is used to test exactly that. diff --git a/src/internet_identity/stable_memory/clean_init_v8.bin.gz b/src/internet_identity/stable_memory/clean_init_v8.bin.gz deleted file mode 100644 index c0ea2fd1c5..0000000000 Binary files a/src/internet_identity/stable_memory/clean_init_v8.bin.gz and /dev/null differ diff --git a/src/internet_identity/tests/integration/archive_integration.rs b/src/internet_identity/tests/integration/archive_integration.rs index b56c4c8019..b068f55541 100644 --- a/src/internet_identity/tests/integration/archive_integration.rs +++ b/src/internet_identity/tests/integration/archive_integration.rs @@ -17,22 +17,6 @@ use std::collections::HashMap; use std::time::Duration; use std::time::SystemTime; -fn setup_ii_v8(env: &StateMachine, arg: Option) -> CanisterId { - let ii_canister = install_ii_canister(env, EMPTY_WASM.clone()); - restore_compressed_stable_memory(env, ii_canister, "stable_memory/clean_init_v8.bin.gz"); - - // upgrade now auto-migrates to storage v9 - upgrade_ii_canister_with_arg(env, ii_canister, II_WASM.clone(), arg) - .expect("II upgrade failed"); - assert_eq!( - ii_api::stats(env, ii_canister) - .unwrap() - .storage_layout_version, - 9 - ); - ii_canister -} - fn setup_ii_v9(env: &StateMachine, arg: Option) -> CanisterId { let ii_canister = install_ii_canister_with_arg(env, II_WASM.clone(), arg); assert_eq!( @@ -50,7 +34,7 @@ fn ii_canisters_under_test( ) -> Vec { // the default arg for this test suite configures the archive let arg = arg.or(arg_with_wasm_hash(ARCHIVE_WASM.clone())); - vec![setup_ii_v8(env, arg.clone()), setup_ii_v9(env, arg)] + vec![setup_ii_v9(env, arg)] } /// Tests related to archive deployment (using II).