use internal block number as last_update for block commitments

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.18"
+version = "0.4.19"
 description = "Applause protocol for the EVM bridge"
 license.workspace = true
 authors.workspace = true

pallets/slow-clap/src/evm_types.rs

@@ -1,4 +1,4 @@
-use sp_runtime::SaturatedConversion;
+use sp_runtime::{traits::UniqueSaturatedInto, SaturatedConversion, Saturating};
 use sp_staking::SessionIndex;
 
 use crate::{
@@ -6,9 +6,9 @@ use crate::{
         de_string_to_bytes, de_string_to_h256, de_string_to_u64, de_string_to_u64_pure,
         de_string_to_vec_of_bytes,
     },
-    AuthIndex, BalanceOf, BlockCommitment, BlockCommitments, Call, Clap, CommitmentDetails, Config,
+    AuthIndex, BalanceOf, BlockCommitment, BlockCommitments, BlockNumberFor, Call, Clap, Config,
     Decode, Deserialize, Encode, NetworkIdOf, RuntimeAppPublic, RuntimeDebug, SubmitTransaction,
-    Vec, COMMITMENT_DELAY_MILLIS, H256, LOG_TARGET,
+    Vec, BLOCK_COMMITMENT_DELAY, H256, LOG_TARGET,
 };
 
 const NUMBER_OF_TOPICS: usize = 3;
@@ -46,26 +46,6 @@ pub struct Log {
 }
 
 impl EvmResponseType {
-    fn prepare_block_commitment<T: Config>(
-        &self,
-        from_block: u64,
-        authority_index: AuthIndex,
-        session_index: SessionIndex,
-        network_id: NetworkIdOf<T>,
-    ) -> BlockCommitment<NetworkIdOf<T>> {
-        let last_updated = sp_io::offchain::timestamp().unix_millis();
-        BlockCommitment {
-            session_index,
-            authority_index,
-            network_id,
-            commitment: CommitmentDetails {
-                last_stored_block: from_block,
-                last_updated,
-                commits: 420,
-            },
-        }
-    }
-
     fn prepare_clap<T: Config>(
         &self,
         authority_index: AuthIndex,
@@ -164,30 +144,30 @@ impl EvmResponseType {
 
     fn sign_and_submit_block_commitment<T: Config>(
         &self,
+        block_now: BlockNumberFor<T>,
         from_block: u64,
         authority_index: AuthIndex,
         authority_key: T::AuthorityId,
         session_index: SessionIndex,
         network_id: NetworkIdOf<T>,
     ) {
-        let block_commitment = self.prepare_block_commitment::<T>(
+        let block_commitment = BlockCommitment {
-            from_block,
-            authority_index,
             session_index,
+            authority_index,
             network_id,
-        );
+            last_stored_block: from_block,
+        };
 
         let stored_last_updated = BlockCommitments::<T>::get(&network_id)
             .get(&authority_index)
-            .map(|details| details.last_updated)
+            .map(|details| {
+                details
+                    .last_updated
+                    .saturating_add(BLOCK_COMMITMENT_DELAY.unique_saturated_into())
+            })
             .unwrap_or_default();
 
-        let current_last_updated = block_commitment
+        if block_now < stored_last_updated {
-            .commitment
-            .last_updated
-            .saturating_sub(COMMITMENT_DELAY_MILLIS);
-
-        if current_last_updated < stored_last_updated {
             return;
         }
 
@@ -229,6 +209,7 @@ impl EvmResponseType {
 
     pub fn sign_and_submit<T: Config>(
         &self,
+        block_now: BlockNumberFor<T>,
         from_block: u64,
         authority_index: AuthIndex,
         authority_key: T::AuthorityId,
@@ -243,6 +224,7 @@ impl EvmResponseType {
                 network_id,
             ),
             EvmResponseType::BlockNumber(_) => self.sign_and_submit_block_commitment::<T>(
+                block_now,
                 from_block,
                 authority_index,
                 authority_key,

pallets/slow-clap/src/lib.rs

@@ -28,7 +28,7 @@ use sp_runtime::{
         storage::StorageValueRef,
         storage_lock::{StorageLock, Time},
     },
-    traits::{BlockNumberProvider, Convert, Saturating, UniqueSaturatedInto},
+    traits::{AtLeast32BitUnsigned, BlockNumberProvider, Convert, Saturating, UniqueSaturatedInto},
     Perbill, RuntimeAppPublic, RuntimeDebug,
 };
 use sp_staking::{
@@ -78,11 +78,13 @@ const MIN_LOCK_GUARD_PERIOD: u64 = 15_000;
 const FETCH_TIMEOUT_PERIOD: u64 = 3_000;
 const LOCK_BLOCK_EXPIRATION: u64 = 20;
 
-const COMMITMENT_DELAY_MILLIS: u64 = 600_000;
 const ONE_HOUR_MILLIS: u64 = 3_600_000;
+
 const CHECK_BLOCK_INTERVAL: u64 = 300;
+const BLOCK_COMMITMENT_DELAY: u64 = 100;
 
 pub type AuthIndex = u32;
+pub type ExternalBlockNumber = u64;
 
 #[derive(
     RuntimeDebug,
@@ -98,10 +100,10 @@ pub type AuthIndex = u32;
     TypeInfo,
     MaxEncodedLen,
 )]
-pub struct CommitmentDetails {
+pub struct CommitmentDetails<BlockNumberFor> {
-    pub last_stored_block: u64,
+    pub last_stored_block: ExternalBlockNumber,
-    pub last_updated: u64,
+    pub last_updated: BlockNumberFor,
-    pub commits: u64,
+    pub commits: u8,
 }
 
 #[derive(
@@ -111,7 +113,7 @@ pub struct BlockCommitment<NetworkId> {
     pub session_index: SessionIndex,
     pub authority_index: AuthIndex,
     pub network_id: NetworkId,
-    pub commitment: CommitmentDetails,
+    pub last_stored_block: ExternalBlockNumber,
 }
 
 #[derive(
@@ -366,6 +368,7 @@ pub mod pallet {
         CouldNotIncreaseGatekeeperAmount,
         NonExistentAuthorityIndex,
         TimeWentBackwards,
+        InnerTimeWentBackwards,
         DisabledAuthority,
         ExecutedBlockIsHigher,
         CommitInWrongSession,
@@ -386,7 +389,7 @@ pub mod pallet {
         _,
         Twox64Concat,
         NetworkIdOf<T>,
-        BTreeMap<AuthIndex, CommitmentDetails>,
+        BTreeMap<AuthIndex, CommitmentDetails<BlockNumberFor<T>>>,
         ValueQuery,
     >;
 
@@ -495,28 +498,53 @@ pub mod pallet {
             {
                 weight = weight.saturating_add(T::DbWeight::get().reads_writes(3, 1));
 
+                // TODO: put wrapper function with precalculated weight here
                 let session_index = T::ValidatorSet::session_index();
                 let block_commitments = BlockCommitments::<T>::get(&network_id);
                 let validators = Validators::<T>::get(&session_index);
 
-                if validators.len() == 0 {
+                let validators_len = validators.len() as u32;
+
+                if validators_len == 0 {
                     return weight;
                 }
 
                 let disabled_bitmap = DisabledAuthorityIndexes::<T>::get(&session_index)
                     .unwrap_or(BitMap::new(validators.len() as u32));
 
-                let max_block_deviation = ONE_HOUR_MILLIS
+                let max_external_block_deviation = ONE_HOUR_MILLIS
                     .saturating_mul(6) // TODO: make it constant or calculate
                     .saturating_div(network_data.avg_block_speed);
 
-                let offence_bitmap = Self::capture_deviation_in_commitments_and_remove(
+                let mut stored_blocks: Vec<(AuthIndex, ExternalBlockNumber)> = Vec::new();
+                let mut block_updates: Vec<(AuthIndex, BlockNumberFor<T>)> = Vec::new();
+
+                for authority_index in 0..validators_len {
+                    let data = block_commitments
+                        .get(&authority_index)
+                        .copied()
+                        .unwrap_or_default();
+                    stored_blocks.push((authority_index, data.last_stored_block));
+                    block_updates.push((authority_index, data.last_updated));
+                }
+
+                let stored_blocks_offence_bitmap = Self::capture_deviation_in_commitments(
                     &disabled_bitmap,
-                    &block_commitments,
+                    &mut stored_blocks,
-                    &validators,
+                    validators_len,
-                    max_block_deviation,
+                    max_external_block_deviation,
                 );
 
+                let block_updates_offence_bitmap = Self::capture_deviation_in_commitments(
+                    &disabled_bitmap,
+                    &mut block_updates,
+                    validators_len,
+                    CHECK_BLOCK_INTERVAL.unique_saturated_into(), // TODO: revisit
+                );
+
+                let offence_bitmap =
+                    stored_blocks_offence_bitmap.bitor(block_updates_offence_bitmap);
+
                 let offence_type = OffenceType::CommitmentOffence;
                 let extra_weight = Self::try_offend_validators(
                     &session_index,
@@ -572,7 +600,7 @@ pub mod pallet {
 
                     ValidTransaction::with_tag_prefix("SlowClap")
                         .priority(T::UnsignedPriority::get())
-                        .and_provides(block_commitment.commitment.encode())
+                        .and_provides(block_commitment.encode())
                         .longevity(LOCK_BLOCK_EXPIRATION)
                         .propagate(true)
                         .build()
@@ -782,10 +810,12 @@ impl<T: Config> Pallet<T> {
     fn try_commit_block(new_commitment: &BlockCommitment<NetworkIdOf<T>>) -> DispatchResult {
         let authority_index = new_commitment.authority_index;
         let network_id = new_commitment.network_id;
+
         let session_index = T::ValidatorSet::session_index();
+        let latest_executed_block = LatestExecutedBlock::<T>::get(&network_id);
 
         ensure!(
-            T::NetworkDataHandler::get(&network_id).is_some(),
+            T::NetworkDataHandler::contains_key(&network_id),
             Error::<T>::UnregistedNetwork
         );
 
@@ -795,24 +825,36 @@ impl<T: Config> Pallet<T> {
         );
 
         BlockCommitments::<T>::try_mutate(&network_id, |current_commitments| -> DispatchResult {
-            let mut new_commitment_details = new_commitment.commitment;
+            let current_block_number = Self::current_block_number();
 
             let (current_commits, current_last_updated) = current_commitments
                 .get(&authority_index)
                 .map(|details| {
                     (
                         details.commits.saturating_add(1),
-                        details.last_updated.saturating_add(COMMITMENT_DELAY_MILLIS),
+                        details
+                            .last_updated
+                            .saturating_add(BLOCK_COMMITMENT_DELAY.unique_saturated_into()),
                     )
                 })
-                .unwrap_or((1, 0));
+                .unwrap_or((1, Default::default()));
 
             ensure!(
-                new_commitment_details.last_updated > current_last_updated,
+                current_block_number >= current_last_updated,
-                Error::<T>::TimeWentBackwards
+                Error::<T>::TimeWentBackwards,
             );
 
-            new_commitment_details.commits = current_commits;
+            ensure!(
+                new_commitment.last_stored_block > latest_executed_block,
+                Error::<T>::InnerTimeWentBackwards,
+            );
+
+            let new_commitment_details = CommitmentDetails {
+                last_stored_block: new_commitment.last_stored_block,
+                last_updated: Self::current_block_number(),
+                commits: current_commits,
+            };
+
             current_commitments.insert(authority_index, new_commitment_details);
 
             Ok(())
@@ -959,6 +1001,7 @@ impl<T: Config> Pallet<T> {
 
                 for (authority_index, authority_key) in Self::local_authorities(&session_index) {
                     parsed_evm_response.sign_and_submit::<T>(
+                        block_number,
                         new_block_range.0,
                         authority_index,
                         authority_key,
@@ -1275,6 +1318,7 @@ 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);
         }
@@ -1290,73 +1334,47 @@ impl<T: Config> Pallet<T> {
         debug_assert!(cursor.maybe_cursor.is_none());
     }
 
-    fn calculate_median_value(values: &mut Vec<(AuthIndex, u64)>) -> u64 {
+    fn calculate_median_value<InnerValue>(values: &mut Vec<(AuthIndex, InnerValue)>) -> InnerValue
+    where
+        InnerValue: AtLeast32BitUnsigned + Copy,
+    {
         values.sort_by_key(|data| data.1);
 
         let length = values.len();
         if length % 2 == 0 {
             let mid_left = values[length / 2 - 1].1;
             let mid_right = values[length / 2].1;
-            (mid_left + mid_right) / 2
+            (mid_left + mid_right) / 2u32.into()
         } else {
             values[length / 2].1
         }
     }
 
-    fn apply_median_deviation(
+    fn capture_deviation_in_commitments<InnerValue>(
-        bitmap: &mut BitMap,
-        disabled: &BitMap,
-        values: &Vec<(AuthIndex, u64)>,
-        median: u64,
-        max_deviation: u64,
-    ) {
-        values.iter().for_each(|(authority_index, value)| {
-            if !disabled.exists(authority_index) && value.abs_diff(median) > max_deviation {
-                bitmap.set(*authority_index);
-            }
-        })
-    }
-
-    fn capture_deviation_in_commitments_and_remove(
         disabled_bitmap: &BitMap,
-        block_commitments: &BTreeMap<AuthIndex, CommitmentDetails>,
+        mut values: &mut Vec<(AuthIndex, InnerValue)>,
-        validators: &WeakBoundedVec<ValidatorId<T>, T::MaxAuthorities>,
+        validators_len: u32,
-        max_block_deviation: u64,
+        max_deviation: InnerValue,
-    ) -> BitMap {
+    ) -> BitMap
-        let validators_len = validators.len() as u32;
+    where
+        InnerValue: AtLeast32BitUnsigned + Copy,
+    {
+        let mut delayed_bitmap = BitMap::new(validators_len);
+        let median_value = Self::calculate_median_value(&mut values);
 
-        let mut delayed = BitMap::new(validators_len);
+        values.iter().for_each(|(authority_index, value)| {
-        let mut stored_blocks: Vec<(AuthIndex, u64)> = Vec::new();
+            let abs_diff = if *value > median_value {
-        let mut time_updates: Vec<(AuthIndex, u64)> = Vec::new();
+                *value - median_value
+            } else {
+                median_value - *value
+            };
 
-        for authority_index in 0..validators_len {
+            if !disabled_bitmap.exists(authority_index) && abs_diff > max_deviation {
-            let data = block_commitments
+                delayed_bitmap.set(*authority_index);
-                .get(&authority_index)
-                .copied()
-                .unwrap_or_default();
-            stored_blocks.push((authority_index, data.last_stored_block));
-            time_updates.push((authority_index, data.last_updated));
             }
+        });
 
-        let stored_block_median = Self::calculate_median_value(&mut stored_blocks);
+        delayed_bitmap
-        let time_update_median = Self::calculate_median_value(&mut time_updates);
-
-        Self::apply_median_deviation(
-            &mut delayed,
-            disabled_bitmap,
-            &stored_blocks,
-            stored_block_median,
-            max_block_deviation,
-        );
-        Self::apply_median_deviation(
-            &mut delayed,
-            disabled_bitmap,
-            &time_updates,
-            time_update_median,
-            ONE_HOUR_MILLIS,
-        );
-
-        delayed
     }
 
     fn get_cumulative_missing_clapped_amount(
@@ -1508,8 +1526,8 @@ impl<T: Config> OneSessionHandler<T::AccountId> for Pallet<T> {
     where
         I: Iterator<Item = (&'a T::AccountId, T::AuthorityId)>,
     {
-        for (network_id, _) in BlockCommitments::<T>::iter() {
+        for network_id in T::NetworkDataHandler::iter_indexes() {
-            BlockCommitments::<T>::remove(network_id);
+            BlockCommitments::<T>::remove(&network_id);
         }
 
         let total_exposure = T::ExposureListener::get_total_exposure();

pallets/slow-clap/src/tests.rs

@@ -1,9 +1,6 @@
 #![cfg(test)]
 
-use std::{
+use std::collections::HashMap;
-    collections::HashMap,
-    time::{SystemTime, UNIX_EPOCH},
-};
 
 use super::*;
 use crate::evm_types::Log;
@@ -1064,47 +1061,38 @@ fn should_register_block_commitments() {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            assert_eq!(commitment_details.last_stored_block, 0);
+
-            assert_eq!(commitment_details.last_updated, 0);
+        let last_stored_block = 69;
-            assert_eq!(commitment_details.commits, 0);
+        let current_block = SlowClap::current_block_number();
-        }
 
-        let mut block_commitment = CommitmentDetails {
-            last_stored_block: 69,
-            commits: 420,
-            last_updated: 1337,
-        };
         for i in 0..=3 {
             assert_ok!(do_block_commitment(
                 session_index,
                 network_id,
                 i,
-                &block_commitment
+                last_stored_block,
             ));
         }
 
-        block_commitment.last_updated = 1000;
         for i in 0..=3 {
             assert_err!(
-                do_block_commitment(session_index, network_id, i, &block_commitment),
+                do_block_commitment(session_index, network_id, i, last_stored_block),
                 Error::<Runtime>::TimeWentBackwards,
             );
         }
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        let block_commitments = BlockCommitments::<Runtime>::get(network_id);
-            assert_eq!(commitment_details.last_stored_block, 69);
+        assert_eq!(block_commitments.len(), 4);
-            assert_eq!(commitment_details.last_updated, 1337);
+        block_commitments.values().for_each(|details| {
-            assert_eq!(commitment_details.commits, 1);
+            assert_eq!(details.last_stored_block, last_stored_block);
-        }
+            assert_eq!(details.last_updated, current_block);
+            assert_eq!(details.commits, 1);
+        });
 
         advance_session();
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            assert_eq!(commitment_details.last_stored_block, 0);
-            assert_eq!(commitment_details.last_updated, 0);
-            assert_eq!(commitment_details.commits, 0);
-        }
     });
 }
 
@@ -1116,35 +1104,23 @@ fn should_disable_on_commitment_inactivity() {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            assert_eq!(commitment_details.last_stored_block, 0);
+        System::set_block_number(420);
-            assert_eq!(commitment_details.last_updated, 0);
-            assert_eq!(commitment_details.commits, 0);
-        }
 
-        let timestamp = SystemTime::now()
+        let last_stored_block = 1337;
-            .duration_since(UNIX_EPOCH)
+        let current_block = SlowClap::current_block_number();
-            .expect("Time went backwards")
-            .as_millis() as u64;
 
-        let mut block_commitment = CommitmentDetails {
-            last_stored_block: 69,
-            commits: 420,
-            last_updated: timestamp,
-        };
-
-        // two block commitments
-        for extra_time in 1..=3 {
-            block_commitment.last_updated += COMMITMENT_DELAY_MILLIS + extra_time;
         for i in 0..3 {
             assert_ok!(do_block_commitment(
                 session_index,
                 network_id,
                 i,
-                    &block_commitment
+                last_stored_block,
             ));
         }
-        }
+
+        let delay = ONE_HOUR_MILLIS.saturating_mul(6);
+        System::set_block_number(current_block + delay + 1);
 
         SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
         System::assert_has_event(RuntimeEvent::SlowClap(
@@ -1153,12 +1129,50 @@ fn should_disable_on_commitment_inactivity() {
             },
         ));
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id)
+        let block_commitments = BlockCommitments::<Runtime>::get(network_id);
-            .values()
+        assert_eq!(block_commitments.get(&3), None);
-            .take(2)
+        block_commitments.values().for_each(|details| {
-        {
+            assert_eq!(details.commits, 1);
-            assert_eq!(commitment_details.commits, 3);
+            assert_eq!(details.last_stored_block, last_stored_block);
+            assert_eq!(details.last_updated, current_block);
+        })
+    });
+}
+
+#[test]
+fn should_ignore_commitments_below_latest_executed_block() {
+    let (network_id, _, _) = generate_unique_hash(None, None, None, None, None);
+    let (_, _, _, block_number) = get_mocked_metadata();
+
+    new_test_ext().execute_with(|| {
+        let session_index = advance_session_and_get_index();
+        prepare_evm_network(None, None);
+
+        assert_eq!(LatestExecutedBlock::<Runtime>::get(network_id), 0);
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
+
+        assert_ok!(do_clap_from(session_index, network_id, 0, false));
+        assert_ok!(do_clap_from(session_index, network_id, 1, false));
+        assert_ok!(do_clap_from(session_index, network_id, 2, false));
+        assert_ok!(do_clap_from(session_index, network_id, 3, false));
+
+        let latest_executed_block = LatestExecutedBlock::<Runtime>::get(&network_id);
+        assert_eq!(latest_executed_block, block_number);
+
+        for low_block in 0..=block_number {
+            assert_err!(
+                do_block_commitment(session_index, network_id, 0, low_block),
+                Error::<Runtime>::InnerTimeWentBackwards,
+            );
         }
+
+        let next_block_number = block_number.saturating_add(1);
+        assert_ok!(do_block_commitment(
+            session_index,
+            network_id,
+            0,
+            next_block_number,
+        ));
     });
 }
 
@@ -1170,47 +1184,26 @@ fn should_disable_on_commitment_block_deviation() {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            assert_eq!(commitment_details.last_stored_block, 0);
-            assert_eq!(commitment_details.last_updated, 0);
-        }
 
-        let timestamp = SystemTime::now()
+        let good_last_stored_block = 9_500_000;
-            .duration_since(UNIX_EPOCH)
+        let bad_last_stored_block = 9_100_000;
-            .expect("Time went backwards")
-            .as_millis() as u64;
 
-        let mut block_commitment = CommitmentDetails {
+        let current_block = SlowClap::current_block_number();
-            last_stored_block: 9_500_000,
-            commits: 420,
-            last_updated: timestamp,
-        };
-
-        let mut bad_block_commitment = CommitmentDetails {
-            last_stored_block: 9_100_000,
-            commits: 420,
-            last_updated: timestamp,
-        };
-
-        // two block commitments
-        for extra_time in 1..=3 {
-            block_commitment.last_updated += COMMITMENT_DELAY_MILLIS + extra_time;
-            bad_block_commitment.last_updated += COMMITMENT_DELAY_MILLIS + extra_time;
         for i in 0..3 {
             assert_ok!(do_block_commitment(
                 session_index,
                 network_id,
                 i,
-                    &block_commitment
+                good_last_stored_block,
             ));
         }
         assert_ok!(do_block_commitment(
             session_index,
             network_id,
             3,
-                &bad_block_commitment
+            bad_last_stored_block,
         ));
-        }
 
         SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
         System::assert_has_event(RuntimeEvent::SlowClap(
@@ -1218,6 +1211,20 @@ fn should_disable_on_commitment_block_deviation() {
                 delayed: vec![(3, 3)],
             },
         ));
+
+        BlockCommitments::<Runtime>::get(network_id)
+            .iter()
+            .for_each(|(account_id, details)| {
+                let block_to_be_stored = if *account_id == 3 {
+                    bad_last_stored_block
+                } else {
+                    good_last_stored_block
+                };
+
+                assert_eq!(details.commits, 1);
+                assert_eq!(details.last_stored_block, block_to_be_stored);
+                assert_eq!(details.last_updated, current_block);
+            })
     });
 }
 
@@ -1229,107 +1236,50 @@ fn should_throw_error_on_fast_commitments() {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        let timestamp = SystemTime::now()
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            .duration_since(UNIX_EPOCH)
-            .expect("Time went backwards")
-            .as_millis() as u64;
 
-        let mut block_commitment = CommitmentDetails {
+        let last_stored_block = 9_500_000;
-            last_stored_block: 9_500_000,
+        let next_stored_block = 9_600_000;
-            commits: 420,
+        let current_block = SlowClap::current_block_number();
-            last_updated: timestamp,
-        };
 
         assert_ok!(do_block_commitment(
             session_index,
             network_id,
             0,
-            &block_commitment
+            last_stored_block,
         ));
         assert_err!(
-            do_block_commitment(session_index, network_id, 0, &block_commitment),
+            do_block_commitment(session_index, network_id, 0, next_stored_block),
             Error::<Runtime>::TimeWentBackwards,
         );
 
-        block_commitment.last_updated += COMMITMENT_DELAY_MILLIS / 2;
+        BlockCommitments::<Runtime>::get(network_id)
-        assert_err!(
+            .get(&0)
-            do_block_commitment(session_index, network_id, 0, &block_commitment),
+            .map(|details| {
-            Error::<Runtime>::TimeWentBackwards,
+                assert_eq!(details.last_stored_block, last_stored_block);
-        );
+                assert_eq!(details.last_updated, current_block);
+                assert_eq!(details.commits, 1);
+            })
+            .expect("authority_index 0 has voted; qed");
 
-        block_commitment.last_updated += COMMITMENT_DELAY_MILLIS / 2;
+        System::set_block_number(current_block + BLOCK_COMMITMENT_DELAY);
-        assert_err!(
-            do_block_commitment(session_index, network_id, 0, &block_commitment),
-            Error::<Runtime>::TimeWentBackwards,
-        );
 
-        block_commitment.last_updated += 1;
+        let new_current_block = SlowClap::current_block_number();
         assert_ok!(do_block_commitment(
             session_index,
             network_id,
             0,
-            &block_commitment
+            next_stored_block,
         ));
 
-        block_commitment.last_updated = timestamp;
+        BlockCommitments::<Runtime>::get(network_id)
-        assert_err!(
+            .get(&0)
-            do_block_commitment(session_index, network_id, 0, &block_commitment),
+            .map(|details| {
-            Error::<Runtime>::TimeWentBackwards,
+                assert_eq!(details.last_stored_block, next_stored_block);
-        );
+                assert_eq!(details.last_updated, new_current_block);
-
+                assert_eq!(details.commits, 2);
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+            })
-            assert_eq!(commitment_details.last_stored_block, 9_500_000);
+            .expect("authority_index 0 has voted; qed");
-            assert_eq!(
-                commitment_details.last_updated,
-                timestamp + COMMITMENT_DELAY_MILLIS + 1
-            );
-            assert_eq!(commitment_details.commits, 2);
-        }
-    });
-}
-
-#[test]
-fn should_not_offend_disabled_authorities() {
-    let (network_id, _, _) = generate_unique_hash(None, None, None, None, None);
-
-    new_test_ext().execute_with(|| {
-        let session_index = advance_session_and_get_index();
-        prepare_evm_network(None, None);
-
-        assert_ok!(do_clap_from(session_index, network_id, 0, false));
-        assert_ok!(do_clap_from(session_index, network_id, 1, false));
-        assert_ok!(do_clap_from(session_index, network_id, 2, false));
-
-        let timestamp = SystemTime::now()
-            .duration_since(UNIX_EPOCH)
-            .expect("Time went backwards")
-            .as_millis() as u64;
-
-        let mut block_commitment = CommitmentDetails {
-            last_stored_block: 9_500_000,
-            commits: 420,
-            last_updated: timestamp,
-        };
-
-        Session::disable_index(3);
-
-        // two block commitments
-        for extra_time in 1..=3 {
-            block_commitment.last_updated += COMMITMENT_DELAY_MILLIS + extra_time;
-            for i in 0..3 {
-                assert_ok!(do_block_commitment(
-                    session_index,
-                    network_id,
-                    i,
-                    &block_commitment
-                ));
-            }
-        }
-
-        SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
-        System::assert_has_event(RuntimeEvent::SlowClap(
-            crate::Event::AuthoritiesCommitmentEquilibrium,
-        ));
     });
 }
 
@@ -1341,83 +1291,90 @@ fn should_not_slash_by_applause_if_disabled_by_commitment() {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            assert_eq!(commitment_details.last_stored_block, 0);
-            assert_eq!(commitment_details.last_updated, 0);
-        }
 
-        let timestamp = SystemTime::now()
+        let last_stored_block = 9_500_000;
-            .duration_since(UNIX_EPOCH)
+        let current_block = SlowClap::current_block_number();
-            .expect("Time went backwards")
+        let mut disabled_bitmp = BitMap::new(4);
-            .as_millis() as u64;
-
-        let mut block_commitment = CommitmentDetails {
-            last_stored_block: 9_500_000,
-            commits: 420,
-            last_updated: timestamp,
-        };
 
+        assert_eq!(
+            DisabledAuthorityIndexes::<Runtime>::get(&session_index),
+            Some(disabled_bitmp.clone())
+        );
         Session::disable_index(3);
+        disabled_bitmp.set(3);
+        assert_eq!(
+            DisabledAuthorityIndexes::<Runtime>::get(&session_index),
+            Some(disabled_bitmp.clone())
+        );
 
-        // two block commitments
+        for i in 0..=3 {
-        for extra_time in 1..=3 {
-            block_commitment.last_updated += COMMITMENT_DELAY_MILLIS + extra_time;
-            for i in 0..3 {
             assert_ok!(do_block_commitment(
                 session_index,
                 network_id,
                 i,
-                    &block_commitment
+                last_stored_block,
             ));
         }
-        }
 
         SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
         System::assert_has_event(RuntimeEvent::SlowClap(
             crate::Event::AuthoritiesCommitmentEquilibrium,
         ));
+
+        let block_commitments = BlockCommitments::<Runtime>::get(network_id);
+        assert_eq!(
+            DisabledAuthorityIndexes::<Runtime>::get(&session_index),
+            Some(disabled_bitmp)
+        );
+        assert_eq!(block_commitments.len(), 4);
+        block_commitments.values().for_each(|details| {
+            assert_eq!(details.last_stored_block, last_stored_block);
+            assert_eq!(details.last_updated, current_block);
+            assert_eq!(details.commits, 1);
+        });
     });
 }
 
 #[test]
-fn should_not_nullify_on_incorrect_block_commitment() {
+fn should_not_register_block_commitment_on_old_blocks() {
     let (network_id, _, _) = generate_unique_hash(None, None, None, None, None);
 
     new_test_ext().execute_with(|| {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        let timestamp = SystemTime::now()
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            .duration_since(UNIX_EPOCH)
-            .expect("Time went backwards")
-            .as_millis() as u64;
 
-        let mut block_commitment = CommitmentDetails {
+        let very_old_block = 9_499_999;
-            last_stored_block: 9_500_000,
+        let last_stored_block = 9_500_000;
-            commits: 420,
+        let current_block = SlowClap::current_block_number();
-            last_updated: timestamp,
-        };
 
         for i in 0..4 {
             assert_ok!(do_block_commitment(
                 session_index,
                 network_id,
                 i,
-                &block_commitment
+                last_stored_block,
             ));
         }
 
-        block_commitment.last_updated = 0;
         assert_err!(
-            do_block_commitment(session_index, network_id, 3, &block_commitment),
+            do_block_commitment(session_index, network_id, 3, last_stored_block),
+            Error::<Runtime>::TimeWentBackwards
+        );
+        assert_err!(
+            do_block_commitment(session_index, network_id, 3, very_old_block),
             Error::<Runtime>::TimeWentBackwards
         );
 
-        for commitment_details in BlockCommitments::<Runtime>::get(network_id).values() {
+        BlockCommitments::<Runtime>::get(network_id)
-            assert_eq!(commitment_details.last_stored_block, 9_500_000);
+            .values()
-            assert_eq!(commitment_details.last_updated, timestamp);
+            .for_each(|details| {
-            assert_eq!(commitment_details.commits, 1);
+                assert_eq!(details.commits, 1);
-        }
+                assert_eq!(details.last_stored_block, last_stored_block);
+                assert_eq!(details.last_updated, current_block);
+            })
     });
 }
 
@@ -1429,33 +1386,21 @@ fn should_split_commit_slash_between_active_validators() {
         let session_index = advance_session_and_get_index();
         prepare_evm_network(None, None);
 
-        let timestamp = SystemTime::now()
+        assert_eq!(BlockCommitments::<Runtime>::get(network_id).len(), 0);
-            .duration_since(UNIX_EPOCH)
-            .expect("Time went backwards")
-            .as_millis() as u64;
 
-        let mut block_commitment = CommitmentDetails {
+        let last_stored_block = 9_500_000;
-            last_stored_block: 9_500_000,
+        let current_block = SlowClap::current_block_number();
-            commits: 420,
-            last_updated: timestamp,
-        };
 
-        for extra_time in 1..=3 {
-            if extra_time < 3 {
-                let offences = Offences::get();
-                assert_eq!(offences.len(), 0);
-            }
-
-            block_commitment.last_updated += COMMITMENT_DELAY_MILLIS + extra_time;
         for i in 0..3 {
             assert_ok!(do_block_commitment(
                 session_index,
                 network_id,
                 i,
-                    &block_commitment
+                last_stored_block,
             ));
         }
-        }
+        let offences = Offences::get();
+        assert_eq!(offences.len(), 0);
 
         SlowClap::on_initialize(CHECK_BLOCK_INTERVAL);
         System::assert_has_event(RuntimeEvent::SlowClap(
@@ -1471,9 +1416,16 @@ fn should_split_commit_slash_between_active_validators() {
             assert_eq!(offence.1.session_index, session_index);
             assert_eq!(offence.1.validator_set_count, 4);
             assert_eq!(offence.1.offenders, vec![(3, 3)]);
-            assert_eq!(offence.1.validator_set_count, 4);
             assert_eq!(offence.1.offence_type, OffenceType::CommitmentOffence);
         }
+
+        let block_commitments = BlockCommitments::<Runtime>::get(network_id);
+        assert_eq!(block_commitments.get(&3), None);
+        block_commitments.values().for_each(|details| {
+            assert_eq!(details.last_stored_block, last_stored_block);
+            assert_eq!(details.last_updated, current_block);
+            assert_eq!(details.commits, 1);
+        });
     });
 }
 
@@ -1720,13 +1672,13 @@ fn do_block_commitment(
     session_index: u32,
     network_id: u32,
     authority_index: u32,
-    commitment: &CommitmentDetails,
+    last_stored_block: ExternalBlockNumber,
 ) -> dispatch::DispatchResult {
     let block_commitment = BlockCommitment {
         session_index,
         authority_index,
         network_id,
-        commitment: *commitment,
+        last_stored_block,
     };
     let authority = UintAuthorityId::from(authority_index as u64);
     let signature = authority.sign(&block_commitment.encode()).unwrap();