Merge branch 'dderler/state-invariants-note' into 'master'

doc: Add guidelines for replicated state invariants

In !16020 a new option for handling errors upon deserializing the state was introduced. This MR adds the guideline on when to use this new option and when other variants are preferrable. 

See merge request dfinity-lab/public/ic!16063

rs/replicated_state/src/lib.rs

@@ -1,3 +1,33 @@
+//! Replicated state types.
+//!
+//! Note [Replicated State Invariants]
+//! ==================================
+//!
+//! Guidelines for handling invariants that are internal to datastructures in the
+//! replicated state:
+//!
+//! - In this context, the term invariant is used to refer to something that (1) holds
+//!   all the time, and (2) whose violation would affect code correctness:
+//!   - We check these during deserialization and return an error if they don't hold.
+//!   - It is also fine to assert/debug_assert (depending on how expensive these checks
+//!     are) for them in production code.
+//!   - Proptests for these invariants are recommended, but can be skipped if there is
+//!     consensus that they are not needed.
+//! - There can also be soft invariants which are a superset of the invariants above.
+//!   - These include things that don't affect correctness of the code, but we still
+//!     aim to uphold them at all times.
+//!   - They can be self healing, i.e., a violation will be fixed upon the next
+//!     modification of after the next couple of modifications.
+//!   - We don't assert for them in production code, but may debug_assert and raise
+//!     critical errors in case of a violation upon deserialization (cf. deserialization
+//!     of `BlockmakerMetricsTimeSeries`)
+//!   - An example for a soft invariant is an upper bound on the number of elements
+//!     in a datastructure which maintains a sliding window of snapshots, where the
+//!     actual number of snapshots does not affect correctness and we just want to make
+//!     sure it doesn't grow indefinitely.
+//! - We don't attempt to restore invariants or soft invariants upon deserializing
+//!   as it could change the past.
+//!
 mod bitcoin;
 pub mod canister_state;
 pub(crate) mod hash;

rs/replicated_state/src/metadata_state.rs

@@ -2034,6 +2034,8 @@ impl BlockmakerMetricsTimeSeries {
     /// Check if any soft invariant is violated. We use soft invariants to refer
     /// to any invariants that the code maintains at all times, but the correctness
     /// of the code is not influenced if they break.
+    ///
+    /// Also see note [Replicated State Invariants].
     fn check_soft_invariants(&self) -> Result<(), String> {
         if self
             .0

rs/state_manager/src/lib.rs

@@ -92,6 +92,7 @@ const CRITICAL_ERROR_CHUNK_ID_USAGE_NEARING_LIMITS: &str =
     "state_sync_chunk_id_usage_nearing_limits";
 
 /// Critical error tracking broken soft invariants encountered upon checkpoint loading.
+/// See note [Replicated State Invariants].
 pub(crate) const CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN: &str =
     "state_manager_checkpoint_soft_invariant_broken";