From b5714ef0881b377517540f96f0ea35f99efb9500 Mon Sep 17 00:00:00 2001 From: sydhds Date: Wed, 31 May 2023 15:04:23 +0200 Subject: [PATCH 1/3] Use saturating_add/sub when updating mip store stats + update versioning documentation --- massa-node/src/main.rs | 1 - massa-versioning/src/lib.rs | 24 ++++--- massa-versioning/src/versioning.rs | 108 ++++++++++++++++++++++++----- 3 files changed, 103 insertions(+), 30 deletions(-) diff --git a/massa-node/src/main.rs b/massa-node/src/main.rs index 7364e5a04e4..297339e0630 100644 --- a/massa-node/src/main.rs +++ b/massa-node/src/main.rs @@ -80,7 +80,6 @@ use massa_protocol_worker::{create_protocol_controller, start_protocol_controlle use massa_storage::Storage; use massa_time::MassaTime; use massa_versioning::versioning::{MipComponent, MipInfo, MipState, MipStatsConfig, MipStore}; - use massa_wallet::Wallet; use parking_lot::RwLock; use peernet::transports::TransportType; diff --git a/massa-versioning/src/lib.rs b/massa-versioning/src/lib.rs index 08bddfdb3f1..35b3feb445e 100644 --- a/massa-versioning/src/lib.rs +++ b/massa-versioning/src/lib.rs @@ -9,13 +9,22 @@ //! MIPState -> Deployment state of a MIPInfo //! MIPStore -> A map of MIPInfo -> MipState //! -//! # Note on MipInfo versions +//! # Notes on MipInfo versions //! -//! There is 2 different versions: +//! There is 2 different 'versions': //! * version == Network version -> This is the network version to announce and thus to store in block header -//! * component_version -> This is the version for the associated component and is used in VersioningFactory +//! * component_version -> This is the version for the associated component and is used in VersioningFactory (e.g KeyPair, Address, VM) //! -//! # Note on MipState +//! # Notes on MipInfo timings and stats +//! +//! So in the execution module and after a block become final, we update the MipStore stats (in a blocking way) then continue one. +//! By updating the stats, we mean sending: (Slot timestamp, Option<(current: u32, advertising: Option)>). +//! Using the slot timestamp, ensure that the trigger (and the trigger timeout) is a consensus by all nodes +//! (This means that the trigger and the trigger timeout are not timer based). +//! In order to have all nodes in sync (because of node various delays), the state is se to active +//! after an activation delay (duration is required to be > 1 cycle). +//! +//! # Notes on MipState //! //! MipState has: //! * A state machine (stores the current state of deployment for a MipInfo) @@ -32,12 +41,7 @@ //! * + When we init MipStore (at startup), this ensures that we have a time ranges & versions coherent list of MipInfo //! * For instance, this can avoid to have 2 MipInfo with the same name //! -//! # Advancing MipState -//! -//! The Versioning middleware is designed to process all finalized blocks and update the corresponding state -//! **It is not yet implemented** -//! -//! # VersioningFactory +//! # Versioning Factory //! //! A Factory trait is there to ease the development of factory for Versioned component (e.g. address, block) //! diff --git a/massa-versioning/src/versioning.rs b/massa-versioning/src/versioning.rs index d3a848a970f..52dffb4996e 100644 --- a/massa-versioning/src/versioning.rs +++ b/massa-versioning/src/versioning.rs @@ -643,8 +643,11 @@ pub struct MipStatsConfig { /// In order for a MIP to be accepted, we compute statistics about other node 'network' version announcement #[derive(Debug, Clone, PartialEq)] pub(crate) struct MipStoreStats { + // config for max counters + block to consider when computing the vote ratio pub(crate) config: MipStatsConfig, + // used to clean up the counters (pop the oldest then subtract matching counter) pub(crate) latest_announcements: VecDeque, + // counter per network version pub(crate) network_version_counters: BTreeMap, } @@ -845,21 +848,23 @@ impl MipStoreRaw { .push_back(announced_network_version); // We update the count of the received version - *self + let entry_value = self .stats .network_version_counters .entry(announced_network_version) - .or_default() += 1; + .or_default(); + *entry_value = entry_value.saturating_add(1); if let Some(removed_version) = removed_version_ { - *self + let entry_value = self .stats .network_version_counters .entry(removed_version) - .or_insert(1) -= 1; + .or_insert(1); + *entry_value = entry_value.saturating_sub(1); } - // Cleanup for the counters + // Cleanup the counters if self.stats.network_version_counters.len() > self.stats.config.counters_max { if let Some((version, count)) = self.stats.network_version_counters.pop_first() { // TODO: return version / count for unit tests? @@ -888,7 +893,7 @@ impl MipStoreRaw { let vote_ratio = Amount::from_mantissa_scale(vote_ratio_.round() as u64, 0); - debug!("(VERSIONING LOG) vote_ration = {} (from version counter = {} and blocks considered = {})", vote_ratio, network_version_count, block_count_considered); + println!("(VERSIONING LOG) vote_ratio = {} (from version counter = {} and blocks considered = {})", vote_ratio, network_version_count, block_count_considered); let advance_msg = Advance { start_timestamp: mi.start, @@ -1225,8 +1230,8 @@ impl TryFrom<([(MipInfo, MipState); N], MipStatsConfig)> for Mip #[cfg(test)] mod test { use super::*; - use std::assert_matches::assert_matches; + use std::assert_matches::assert_matches; use std::str::FromStr; use chrono::{Days, NaiveDate, NaiveDateTime}; @@ -1248,6 +1253,7 @@ mod test { } } + // helper impl From<(&MipInfo, &Amount, &MassaTime)> for Advance { fn from((mip_info, threshold, now): (&MipInfo, &Amount, &MassaTime)) -> Self { Self { @@ -1261,7 +1267,7 @@ mod test { } fn get_a_version_info() -> (NaiveDateTime, NaiveDateTime, MipInfo) { - // A helper function to provide a default VersioningInfo + // A helper function to provide a default MipInfo // Models a Massa Improvements Proposal (MIP-0002), transitioning component address to v2 let start: NaiveDateTime = NaiveDate::from_ymd_opt(2017, 11, 01) @@ -1527,9 +1533,9 @@ mod test { #[test] fn test_is_coherent_with() { - // Test MipStateHistory::is_coherent_with + // Test MipStateHistory::is_coherent_with (coherence of MIP state against its MIP info) - // Given the following versioning info, we expect state + // Given the following MIP info, we expect state // Defined @ time <= 2 // Started @ time > 2 && <= 5 // LockedIn @ time > time(Started) && <= 5 @@ -1603,7 +1609,9 @@ mod test { } #[test] - fn test_merge_with() { + fn test_update_with() { + // Test MipStoreRaw.update_with method (e.g. update a store from another, used in bootstrap) + let vi_1 = MipInfo { name: "MIP-0002".to_string(), version: 2, @@ -1659,10 +1667,10 @@ mod test { } #[test] - fn test_merge_with_invalid() { - // Test updating a versioning store with another invalid: - // 1- overlapping time range - // 2- overlapping versioning component + fn test_update_with_invalid() { + // Test updating a MIP store with another invalid one: + // case 1: overlapping time range + // case 2: overlapping versioning component // part 0 - defines data for the test let vi_1 = MipInfo { @@ -1693,7 +1701,7 @@ mod test { counters_max: 5, }; - // part 1 + // case 1 { let mut vs_raw_1 = MipStoreRaw::try_from(( [(vi_1.clone(), vs_1.clone()), (vi_2.clone(), vs_2.clone())], @@ -1733,7 +1741,7 @@ mod test { } } - // part 2 + // case 2 { let mut vs_raw_1 = MipStoreRaw::try_from(( [(vi_1.clone(), vs_1.clone()), (vi_2.clone(), vs_2.clone())], @@ -1775,7 +1783,7 @@ mod test { #[test] fn test_update_with_unknown() { - // Test update_with with unknown MipComponent + // Test update_with with unknown MipComponent (can happen if a node software is outdated) // data let mip_stats_config = MipStatsConfig { @@ -1816,7 +1824,7 @@ mod test { let shutdown_start = Slot::new(2, 0); let shutdown_end = Slot::new(8, 0); - // helper to make the easy to read + // helper functions so the test code is easy to read let get_slot_ts = |slot| get_block_slot_timestamp(THREAD_COUNT, T0, genesis_timestamp, slot).unwrap(); let is_coherent = |store: &MipStoreRaw, shutdown_start, shutdown_end| { @@ -2150,4 +2158,66 @@ mod test { // println!("st2_raw: {:?}", st2_raw); assert_eq!(st1_raw, st2_raw); } + + fn test_mip_store_stats() { + // Test MipStoreRaw stats + + // helper functions so the test code is easy to read + let genesis_timestamp = MassaTime::from_millis(0); + let get_slot_ts = + |slot| get_block_slot_timestamp(THREAD_COUNT, T0, genesis_timestamp, slot).unwrap(); + + let mip_stats_config = MipStatsConfig { + block_count_considered: 2, + counters_max: 1, + }; + let activation_delay = MassaTime::from_millis(100); + let timeout = MassaTime::now() + .unwrap() + .saturating_add(MassaTime::from_millis(50_000)); // + 50 seconds + let mi_1 = MipInfo { + name: "MIP-0001".to_string(), + version: 1, + components: BTreeMap::from([(MipComponent::Address, 1)]), + start: MassaTime::from_millis(2), + timeout, + activation_delay, + }; + let ms_1 = advance_state_until(ComponentState::started(Amount::zero()), &mi_1); + + let mut mip_store = + MipStoreRaw::try_from(([(mi_1.clone(), ms_1)], mip_stats_config)).unwrap(); + + // + // mip_store.update_network_version_stats(get_slot_ts(Slot::new(1, 0)), Some((0, 0))); + // TODO: should not add a counter for version 0 ? + // assert_eq!(mip_store.stats.network_version_counters.len(), 0); + + // Current network version is 0, next one is 1 + mip_store.update_network_version_stats(get_slot_ts(Slot::new(1, 0)), Some((0, 1))); + assert_eq!(mip_store.stats.network_version_counters.len(), 1); + assert_eq!(mip_store.stats.network_version_counters.get(&1), Some(&1)); + + mip_store.update_network_version_stats(get_slot_ts(Slot::new(1, 0)), Some((0, 1))); + assert_eq!(mip_store.stats.network_version_counters.len(), 1); + assert_eq!(mip_store.stats.network_version_counters.get(&1), Some(&2)); + + // Check that MipInfo is now + let (mi_, ms_) = mip_store.store.last_key_value().unwrap(); + assert_eq!(*mi_, mi_1); + assert_matches!(ms_.state, ComponentState::LockedIn(..)); + + let mut at = MassaTime::now().unwrap(); + at = at.saturating_add(activation_delay); + assert_eq!( + ms_.state_at(at, mi_1.start, mi_1.timeout), + Ok(ComponentStateTypeId::Active) + ); + + // Now network version is 1, next one is 2 + mip_store.update_network_version_stats(get_slot_ts(Slot::new(1, 0)), Some((1, 2))); + // Config is set to allow only 1 counter + assert_eq!(mip_store.stats.network_version_counters.len(), 1); + assert_eq!(mip_store.stats.network_version_counters.get(&2), Some(&1)); + } } From c537e477051fa965363c19c16edddadf95d29b80 Mon Sep 17 00:00:00 2001 From: sydhds Date: Wed, 31 May 2023 15:14:34 +0200 Subject: [PATCH 2/3] Restore debug print --- massa-versioning/src/versioning.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/massa-versioning/src/versioning.rs b/massa-versioning/src/versioning.rs index 52dffb4996e..0b09927d59b 100644 --- a/massa-versioning/src/versioning.rs +++ b/massa-versioning/src/versioning.rs @@ -893,7 +893,7 @@ impl MipStoreRaw { let vote_ratio = Amount::from_mantissa_scale(vote_ratio_.round() as u64, 0); - println!("(VERSIONING LOG) vote_ratio = {} (from version counter = {} and blocks considered = {})", vote_ratio, network_version_count, block_count_considered); + debug!("(VERSIONING LOG) vote_ratio = {} (from version counter = {} and blocks considered = {})", vote_ratio, network_version_count, block_count_considered); let advance_msg = Advance { start_timestamp: mi.start, From 46a07ca518a7124dca9ea2408f9cb4193340b927 Mon Sep 17 00:00:00 2001 From: sydhds Date: Thu, 1 Jun 2023 11:32:54 +0200 Subject: [PATCH 3/3] Move MIP info declaration in its own file --- massa-node/src/main.rs | 27 +++++++-------------------- massa-versioning/src/lib.rs | 6 ++++-- massa-versioning/src/mips.rs | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+), 22 deletions(-) create mode 100644 massa-versioning/src/mips.rs diff --git a/massa-node/src/main.rs b/massa-node/src/main.rs index 297339e0630..fa603af3b4e 100644 --- a/massa-node/src/main.rs +++ b/massa-node/src/main.rs @@ -79,11 +79,14 @@ use massa_protocol_exports::{ProtocolConfig, ProtocolManager}; use massa_protocol_worker::{create_protocol_controller, start_protocol_controller}; use massa_storage::Storage; use massa_time::MassaTime; -use massa_versioning::versioning::{MipComponent, MipInfo, MipState, MipStatsConfig, MipStore}; +use massa_versioning::{ + mips::MIP_LIST, + versioning::{MipStatsConfig, MipStore}, +}; use massa_wallet::Wallet; use parking_lot::RwLock; use peernet::transports::TransportType; -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use std::path::PathBuf; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Condvar, Mutex}; @@ -234,24 +237,8 @@ async fn launch( block_count_considered: MIP_STORE_STATS_BLOCK_CONSIDERED, counters_max: MIP_STORE_STATS_COUNTERS_MAX, }; - let mip_store = MipStore::try_from(( - [( - MipInfo { - name: "MIP-0000".to_string(), - version: 0, - components: BTreeMap::from([ - (MipComponent::Address, 0), - (MipComponent::KeyPair, 0), - ]), - start: MassaTime::from_millis(0), - timeout: MassaTime::from_millis(0), - activation_delay: MassaTime::from_millis(0), - }, - MipState::new(MassaTime::from_millis(0)), - )], - mip_stats_config, - )) - .expect("mip store creation failed"); + let mip_store = + MipStore::try_from((MIP_LIST, mip_stats_config)).expect("mip store creation failed"); // Create final state, either from a snapshot, or from scratch let final_state = Arc::new(parking_lot::RwLock::new( diff --git a/massa-versioning/src/lib.rs b/massa-versioning/src/lib.rs index 35b3feb445e..0af2efe14f1 100644 --- a/massa-versioning/src/lib.rs +++ b/massa-versioning/src/lib.rs @@ -9,6 +9,7 @@ //! MIPState -> Deployment state of a MIPInfo //! MIPStore -> A map of MIPInfo -> MipState //! +//! //! # Notes on MipInfo versions //! //! There is 2 different 'versions': @@ -17,11 +18,11 @@ //! //! # Notes on MipInfo timings and stats //! -//! So in the execution module and after a block become final, we update the MipStore stats (in a blocking way) then continue one. +//! So in the execution module and after a block become final, we update the MipStore stats (in a blocking way). //! By updating the stats, we mean sending: (Slot timestamp, Option<(current: u32, advertising: Option)>). //! Using the slot timestamp, ensure that the trigger (and the trigger timeout) is a consensus by all nodes //! (This means that the trigger and the trigger timeout are not timer based). -//! In order to have all nodes in sync (because of node various delays), the state is se to active +//! In order to have all nodes in sync (because of node various delays), the state is set to active //! after an activation delay (duration is required to be > 1 cycle). //! //! # Notes on MipState @@ -53,6 +54,7 @@ pub mod address_factory; pub mod grpc_mapping; pub mod keypair_factory; +pub mod mips; pub mod versioning; pub mod versioning_factory; pub mod versioning_ser_der; diff --git a/massa-versioning/src/mips.rs b/massa-versioning/src/mips.rs new file mode 100644 index 00000000000..2230c7573fa --- /dev/null +++ b/massa-versioning/src/mips.rs @@ -0,0 +1,20 @@ +#[allow(unused_imports)] +use crate::versioning::{MipComponent, MipInfo, MipState}; + +pub const MIP_LIST: [(MipInfo, MipState); 0] = [ + // MIP placeholder +/* ( + MipInfo { + name: "MIP-0000".to_string(), + version: 0, + components: BTreeMap::from([ + (MipComponent::Address, 0), + (MipComponent::KeyPair, 0), + ]), + start: MassaTime::from_millis(0), + timeout: MassaTime::from_millis(0), + activation_delay: MassaTime::from_millis(0), + }, + MipState::new(MassaTime::from_millis(0)), +) */ +];