migration to new block commitment data type; tests included

Signed-off-by: Uncle Stinky <uncle.stinky@ghostchain.io>

pallets/slow-clap/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ghost-slow-clap"
-version = "0.4.19"
+version = "0.4.22"
 description = "Applause protocol for the EVM bridge"
 license.workspace = true
 authors.workspace = true

pallets/slow-clap/src/lib.rs

@@ -43,6 +43,7 @@ use ghost_networks::{
 };
 use ghost_traits::exposure::ExposureListener;
 
+pub mod migrations;
 pub mod weights;
 pub use crate::weights::WeightInfo;
 mod benchmarking;
@@ -79,8 +80,7 @@ const FETCH_TIMEOUT_PERIOD: u64 = 3_000;
 const LOCK_BLOCK_EXPIRATION: u64 = 20;
 
 const ONE_HOUR_MILLIS: u64 = 3_600_000;
-
-const CHECK_BLOCK_INTERVAL: u64 = 300;
+const BLOCK_CHECK_CYCLES: u64 = 8;
 const BLOCK_COMMITMENT_DELAY: u64 = 100;
 
 pub type AuthIndex = u32;
@@ -271,7 +271,7 @@ type OffchainResult<T, A> = Result<A, OffchainErr<NetworkIdOf<T>>>;
 pub mod pallet {
     use super::*;
 
-    const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
+    const STORAGE_VERSION: StorageVersion = StorageVersion::new(3);
 
     #[pallet::pallet]
     #[pallet::storage_version(STORAGE_VERSION)]
@@ -317,6 +317,9 @@ pub mod pallet {
         #[pallet::constant]
         type MinAuthoritiesNumber: Get<u32>;
 
+        #[pallet::constant]
+        type EpochDuration: Get<u64>;
+
         type WeightInfo: WeightInfo;
     }
 
@@ -479,16 +482,20 @@ pub mod pallet {
     #[pallet::hooks]
     impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
         fn on_initialize(current_block: BlockNumberFor<T>) -> Weight {
-            // TODO: what about start of the session???
             let mut weight = T::DbWeight::get().reads(1);
-
             let networks_count = T::NetworkDataHandler::count();
             let current_block_number: u64 = current_block.unique_saturated_into();
-            //let cycle_start = (current_block_number / CHECK_BLOCK_INTERVAL) * CHECK_BLOCK_INTERVAL;
-            let cycle_offset = current_block_number % CHECK_BLOCK_INTERVAL;
 
-            if cycle_offset >= networks_count.into() {
-                return Default::default();
+            let check_block_interval = T::EpochDuration::get().saturating_div(BLOCK_CHECK_CYCLES);
+            if check_block_interval == 0 {
+                return weight;
+            }
+
+            let cycle_offset = current_block_number % check_block_interval;
+            let block_in_epoch = current_block_number % T::EpochDuration::get();
+
+            if cycle_offset >= networks_count.into() || block_in_epoch < check_block_interval {
+                return weight;
             }
 
             let converted_block: usize = current_block_number.unique_saturated_into();
@@ -513,9 +520,11 @@ pub mod pallet {
                     .unwrap_or(BitMap::new(validators.len() as u32));
 
                 let max_external_block_deviation = ONE_HOUR_MILLIS
-                    .saturating_mul(6) // TODO: make it constant or calculate
+                    .saturating_mul(6)
                     .saturating_div(network_data.avg_block_speed);
 
+                let max_internal_block_deviation = check_block_interval.unique_saturated_into();
+
                 let mut stored_blocks: Vec<(AuthIndex, ExternalBlockNumber)> = Vec::new();
                 let mut block_updates: Vec<(AuthIndex, BlockNumberFor<T>)> = Vec::new();
 
@@ -539,7 +548,7 @@ pub mod pallet {
                     &disabled_bitmap,
                     &mut block_updates,
                     validators_len,
-                    CHECK_BLOCK_INTERVAL.unique_saturated_into(), // TODO: revisit
+                    max_internal_block_deviation,
                 );
 
                 let offence_bitmap =
@@ -1318,7 +1327,6 @@ impl<T: Config> Pallet<T> {
         Authorities::<T>::set(&session_index, bounded_authorities);
 
         let mut disabled_bitmap = BitMap::new(authorities_len as AuthIndex);
-        // TODO: make me better
         for disabled_index in T::DisabledValidators::disabled_validators() {
             disabled_bitmap.set(disabled_index);
         }

pallets/slow-clap/src/migrations/mod.rs

@@ -0,0 +1,9 @@
+pub mod v3;
+
+pub type MigrateV2ToV3<T> = frame_support::migrations::VersionedMigration<
+    2,
+    3,
+    v3::CommitmentDetailsTypesChanged<T>,
+    crate::Pallet<T>,
+    <T as frame_system::Config>::DbWeight,
+>;

pallets/slow-clap/src/migrations/v3.rs

@@ -0,0 +1,71 @@
+use codec::{Decode, Encode, MaxEncodedLen};
+use frame_support::{
+    storage,
+    traits::{Get, UncheckedOnRuntimeUpgrade},
+    weights::Weight,
+};
+use ghost_networks::NetworkDataInspectHandler;
+use scale_info::TypeInfo;
+use sp_runtime::traits::{BlockNumberProvider, UniqueSaturatedInto};
+use sp_runtime::RuntimeDebug;
+use sp_std::marker::PhantomData;
+
+use crate::{AuthIndex, BTreeMap, BlockCommitments, CommitmentDetails, Config, Pallet, LOG_TARGET};
+
+#[derive(
+    RuntimeDebug,
+    Default,
+    Copy,
+    Clone,
+    Eq,
+    PartialEq,
+    Ord,
+    PartialOrd,
+    Encode,
+    Decode,
+    TypeInfo,
+    MaxEncodedLen,
+)]
+pub struct OldCommitmentDetails {
+    pub last_stored_block: u64,
+    pub last_updated: u64,
+    pub commits: u64,
+}
+
+pub struct CommitmentDetailsTypesChanged<T>(PhantomData<T>);
+impl<T: Config> UncheckedOnRuntimeUpgrade for CommitmentDetailsTypesChanged<T> {
+    fn on_runtime_upgrade() -> Weight {
+        let mut weight = T::DbWeight::get().reads(3);
+        let current_block_number = <Pallet<T> as BlockNumberProvider>::current_block_number();
+
+        for network_id in T::NetworkDataHandler::iter_indexes() {
+            let mut new_btree_map = BTreeMap::new();
+
+            let key_hash = BlockCommitments::<T>::hashed_key_for(&network_id);
+            let old_commitments = storage::unhashed::take_or_default::<
+                BTreeMap<AuthIndex, OldCommitmentDetails>,
+            >(&key_hash);
+
+            for (authority_index, details) in old_commitments.iter() {
+                new_btree_map.insert(
+                    authority_index,
+                    CommitmentDetails {
+                        last_stored_block: details.last_stored_block,
+                        last_updated: current_block_number,
+                        commits: details.commits.unique_saturated_into(),
+                    },
+                );
+            }
+            BlockCommitments::<T>::insert(&network_id, new_btree_map);
+            weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
+
+            log::info!(
+                target: LOG_TARGET,
+                "👻 Block commitments migrated from unix timestamp to internal block for network {:?}",
+                network_id,
+            );
+        }
+
+        weight
+    }
+}

pallets/slow-clap/src/mock.rs

@@ -159,6 +159,7 @@ parameter_types! {
     pub static ExistentialDeposit: u64 = 2;
     pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE;
     pub const HistoryDepth: u32 = 10;
+    pub const EpochDuration: u64 = 80;
 }
 
 #[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)]
@@ -213,6 +214,7 @@ impl Config for Runtime {
     type UnsignedPriority = ConstU64<{ 1 << 20 }>;
     type HistoryDepth = HistoryDepth;
     type MinAuthoritiesNumber = ConstU32<1>;
+    type EpochDuration = EpochDuration;
 
     type WeightInfo = ();
 }

pallets/slow-clap/src/tests.rs

@@ -3,8 +3,8 @@
 use std::collections::HashMap;
 
 use super::*;
-use crate::evm_types::Log;
 use crate::mock::*;
+use crate::{evm_types::Log, migrations::v3::OldCommitmentDetails};
 
 use frame_support::{assert_err, assert_ok, dispatch};
 use sp_core::offchain::{
@@ -1105,10 +1105,8 @@ fn should_disable_on_commitment_inactivity() {
         prepare_evm_network(None, None);
 
         assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-        System::set_block_number(420);
-
-        let last_stored_block = 1337;
-        let current_block = SlowClap::current_block_number();
+        System::set_block_number(5 * EpochDuration::get() / 2);
+        let last_stored_block = 69;
 
         for i in 0..3 {
             assert_ok!(do_block_commitment(
@@ -1119,10 +1117,8 @@ fn should_disable_on_commitment_inactivity() {
             ));
         }
 
-        let delay = ONE_HOUR_MILLIS.saturating_mul(6);
-        System::set_block_number(current_block + delay + 1);
-
-        SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
+        let current_block = System::current_block_number();
+        SlowClap::on_initialize(current_block);
         System::assert_has_event(RuntimeEvent::SlowClap(
             crate::Event::SomeAuthoritiesDelayed {
                 delayed: vec![(3, 3)],
@@ -1205,7 +1201,11 @@ fn should_disable_on_commitment_block_deviation() {
             bad_last_stored_block,
         ));
 
-        SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
+        let to_initialize = EpochDuration::get()
+            .saturating_div(BLOCK_CHECK_CYCLES)
+            .saturating_mul(2);
+
+        SlowClap::on_initialize(to_initialize);
         System::assert_has_event(RuntimeEvent::SlowClap(
             crate::Event::SomeAuthoritiesDelayed {
                 delayed: vec![(3, 3)],
@@ -1292,6 +1292,7 @@ fn should_not_slash_by_applause_if_disabled_by_commitment() {
         prepare_evm_network(None, None);
 
         assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
+        System::set_block_number(5 * EpochDuration::get() / 2);
 
         let last_stored_block = 9_500_000;
         let current_block = SlowClap::current_block_number();
@@ -1317,7 +1318,7 @@ fn should_not_slash_by_applause_if_disabled_by_commitment() {
             ));
         }
 
-        SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
+        SlowClap::on_initialize(current_block);
         System::assert_has_event(RuntimeEvent::SlowClap(
             crate::Event::AuthoritiesCommitmentEquilibrium,
         ));
@@ -1387,9 +1388,8 @@ fn should_split_commit_slash_between_active_validators() {
         prepare_evm_network(None, None);
 
         assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-
-        let last_stored_block = 9_500_000;
-        let current_block = SlowClap::current_block_number();
+        System::set_block_number(5 * EpochDuration::get() / 2);
+        let last_stored_block = 69;
 
         for i in 0..3 {
             assert_ok!(do_block_commitment(
@@ -1399,10 +1399,12 @@ fn should_split_commit_slash_between_active_validators() {
                 last_stored_block,
             ));
         }
+
+        let current_block = SlowClap::current_block_number();
         let offences = Offences::get();
         assert_eq!(offences.len(), 0);
 
-        SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
+        SlowClap::on_initialize(current_block);
         System::assert_has_event(RuntimeEvent::SlowClap(
             crate::Event::SomeAuthoritiesDelayed {
                 delayed: vec![(3, 3)],
@@ -1442,8 +1444,15 @@ fn should_check_different_networks_during_on_initialize() {
             prepare_evm_network(Some(network_id), None);
         }
 
-        let check_interval = times * CHECK_BLOCK_INTERVAL;
-        for check_block in 0..check_interval {
+        let expected_events_count: u64 = BLOCK_CHECK_CYCLES
+            .saturating_sub(1)
+            .saturating_mul(networks_count as u64)
+            .saturating_mul(times);
+
+        let expected_events_count: usize = expected_events_count.try_into().unwrap();
+
+        let epochs_passed = times * EpochDuration::get();
+        for check_block in 0..epochs_passed {
             SlowClap::on_initialize(check_block);
         }
 
@@ -1468,12 +1477,13 @@ fn should_check_different_networks_during_on_initialize() {
                 }
             });
 
-        let expected_events = networks_count * (times as u32);
-        assert_eq!(total_number_of_events, expected_events as usize);
+        assert_eq!(total_number_of_events, expected_events_count);
         assert_eq!(check_commitment_events.len() as u32, networks_count);
         for network_id in 0..networks_count {
             let network_id_count = check_commitment_events.get(&network_id).unwrap();
-            assert_eq!(*network_id_count, times);
+            let expected_events_count =
+                expected_events_count.saturating_div(networks_count as usize);
+            assert_eq!(*network_id_count, expected_events_count);
         }
     });
 }
@@ -1619,6 +1629,64 @@ fn should_get_balanced_responses_correctly() {
     });
 }
 
+#[test]
+fn migration_from_v2_to_v3_works_fine() {
+    new_test_ext().execute_with(|| {
+        let network_ids = vec![1, 57232, 232, 775];
+
+        StorageVersion::new(2).put::<SlowClap>();
+        System::set_block_number(69);
+
+        for network_id in network_ids.iter() {
+            prepare_evm_network(Some(*network_id), None);
+            let mut fake_old_commits = BTreeMap::new();
+
+            fake_old_commits.insert(
+                0,
+                OldCommitmentDetails {
+                    last_stored_block: (*network_id).into(),
+                    last_updated: 133742069,
+                    commits: 420,
+                },
+            );
+            fake_old_commits.insert(
+                2,
+                OldCommitmentDetails {
+                    last_stored_block: (*network_id).into(),
+                    last_updated: 133742069,
+                    commits: 5,
+                },
+            );
+
+            let key_hash = BlockCommitments::<Runtime>::hashed_key_for(&network_id);
+            frame_support::storage::unhashed::put(&key_hash, &fake_old_commits);
+        }
+
+        type Migrate = crate::migrations::MigrateV2ToV3<Runtime>;
+        <Migrate as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
+
+        let current_block_number = System::current_block_number();
+
+        for network_id in network_ids.iter() {
+            BlockCommitments::<Runtime>::get(&network_id)
+                .iter()
+                .for_each(|(authority_id, details)| match authority_id {
+                    0 => {
+                        assert_eq!(details.last_stored_block, *network_id as u64);
+                        assert_eq!(details.last_updated, current_block_number);
+                        assert_eq!(details.commits, u8::MAX);
+                    }
+                    2 => {
+                        assert_eq!(details.last_stored_block, *network_id as u64);
+                        assert_eq!(details.last_updated, current_block_number);
+                        assert_eq!(details.commits, 5);
+                    }
+                    _ => panic!("non registered block commitment exists after migration"),
+                });
+        }
+    });
+}
+
 fn assert_clapped_amount(
     session_index: &SessionIndex,
     unique_hash: &H256,