From 028afc089f40470ea5316bdd4b1e93c5b1d80ba3 Mon Sep 17 00:00:00 2001 From: Uncle Stinky Date: Sun, 22 Feb 2026 21:56:55 +0300 Subject: [PATCH] use internal block number as last_update for block commitments Signed-off-by: Uncle Stinky --- pallets/slow-clap/Cargo.toml | 2 +- pallets/slow-clap/src/evm_types.rs | 50 ++-- pallets/slow-clap/src/lib.rs | 170 ++++++----- pallets/slow-clap/src/tests.rs | 442 +++++++++++++---------------- 4 files changed, 308 insertions(+), 356 deletions(-) diff --git a/pallets/slow-clap/Cargo.toml b/pallets/slow-clap/Cargo.toml index a7e81ee..969cd34 100644 --- a/pallets/slow-clap/Cargo.toml +++ b/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 diff --git a/pallets/slow-clap/src/evm_types.rs b/pallets/slow-clap/src/evm_types.rs index 0115a4a..1c64b25 100644 --- a/pallets/slow-clap/src/evm_types.rs +++ b/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( - &self, - from_block: u64, - authority_index: AuthIndex, - session_index: SessionIndex, - network_id: NetworkIdOf, - ) -> BlockCommitment> { - 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( &self, authority_index: AuthIndex, @@ -164,30 +144,30 @@ impl EvmResponseType { fn sign_and_submit_block_commitment( &self, + block_now: BlockNumberFor, from_block: u64, authority_index: AuthIndex, authority_key: T::AuthorityId, session_index: SessionIndex, network_id: NetworkIdOf, ) { - let block_commitment = self.prepare_block_commitment::( - from_block, - authority_index, + let block_commitment = BlockCommitment { session_index, + authority_index, network_id, - ); + last_stored_block: from_block, + }; let stored_last_updated = BlockCommitments::::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 - .commitment - .last_updated - .saturating_sub(COMMITMENT_DELAY_MILLIS); - - if current_last_updated < stored_last_updated { + if block_now < stored_last_updated { return; } @@ -229,6 +209,7 @@ impl EvmResponseType { pub fn sign_and_submit( &self, + block_now: BlockNumberFor, 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::( + block_now, from_block, authority_index, authority_key, diff --git a/pallets/slow-clap/src/lib.rs b/pallets/slow-clap/src/lib.rs index 1b85692..119c9e6 100644 --- a/pallets/slow-clap/src/lib.rs +++ b/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 last_stored_block: u64, - pub last_updated: u64, - pub commits: u64, +pub struct CommitmentDetails { + pub last_stored_block: ExternalBlockNumber, + pub last_updated: BlockNumberFor, + pub commits: u8, } #[derive( @@ -111,7 +113,7 @@ pub struct BlockCommitment { 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, - BTreeMap, + BTreeMap>>, 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::::get(&network_id); let validators = Validators::::get(&session_index); - if validators.len() == 0 { + let validators_len = validators.len() as u32; + + if validators_len == 0 { return weight; } let disabled_bitmap = DisabledAuthorityIndexes::::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)> = 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, - &validators, - max_block_deviation, + &mut stored_blocks, + validators_len, + 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 Pallet { fn try_commit_block(new_commitment: &BlockCommitment>) -> 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::::get(&network_id); ensure!( - T::NetworkDataHandler::get(&network_id).is_some(), + T::NetworkDataHandler::contains_key(&network_id), Error::::UnregistedNetwork ); @@ -795,24 +825,36 @@ impl Pallet { ); BlockCommitments::::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, - Error::::TimeWentBackwards + current_block_number >= current_last_updated, + Error::::TimeWentBackwards, ); - new_commitment_details.commits = current_commits; + ensure!( + new_commitment.last_stored_block > latest_executed_block, + Error::::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 Pallet { for (authority_index, authority_key) in Self::local_authorities(&session_index) { parsed_evm_response.sign_and_submit::( + block_number, new_block_range.0, authority_index, authority_key, @@ -1275,6 +1318,7 @@ impl Pallet { Authorities::::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 Pallet { debug_assert!(cursor.maybe_cursor.is_none()); } - fn calculate_median_value(values: &mut Vec<(AuthIndex, u64)>) -> u64 { + fn calculate_median_value(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( - 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( + fn capture_deviation_in_commitments( disabled_bitmap: &BitMap, - block_commitments: &BTreeMap, - validators: &WeakBoundedVec, T::MaxAuthorities>, - max_block_deviation: u64, - ) -> BitMap { - let validators_len = validators.len() as u32; + mut values: &mut Vec<(AuthIndex, InnerValue)>, + validators_len: u32, + max_deviation: InnerValue, + ) -> BitMap + 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); - let mut stored_blocks: Vec<(AuthIndex, u64)> = Vec::new(); - let mut time_updates: Vec<(AuthIndex, u64)> = Vec::new(); + values.iter().for_each(|(authority_index, value)| { + let abs_diff = if *value > median_value { + *value - median_value + } else { + median_value - *value + }; - 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)); - time_updates.push((authority_index, data.last_updated)); - } + if !disabled_bitmap.exists(authority_index) && abs_diff > max_deviation { + delayed_bitmap.set(*authority_index); + } + }); - let stored_block_median = Self::calculate_median_value(&mut stored_blocks); - 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 + delayed_bitmap } fn get_cumulative_missing_clapped_amount( @@ -1508,8 +1526,8 @@ impl OneSessionHandler for Pallet { where I: Iterator, { - for (network_id, _) in BlockCommitments::::iter() { - BlockCommitments::::remove(network_id); + for network_id in T::NetworkDataHandler::iter_indexes() { + BlockCommitments::::remove(&network_id); } let total_exposure = T::ExposureListener::get_total_exposure(); diff --git a/pallets/slow-clap/src/tests.rs b/pallets/slow-clap/src/tests.rs index 1b7a725..cd0870f 100644 --- a/pallets/slow-clap/src/tests.rs +++ b/pallets/slow-clap/src/tests.rs @@ -1,9 +1,6 @@ #![cfg(test)] -use std::{ - collections::HashMap, - time::{SystemTime, UNIX_EPOCH}, -}; +use std::collections::HashMap; 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::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 0); - assert_eq!(commitment_details.last_updated, 0); - assert_eq!(commitment_details.commits, 0); - } + assert_eq!(BlockCommitments::::get(network_id).len(), 0); + + let last_stored_block = 69; + 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::::TimeWentBackwards, ); } - for commitment_details in BlockCommitments::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 69); - assert_eq!(commitment_details.last_updated, 1337); - assert_eq!(commitment_details.commits, 1); - } + let block_commitments = BlockCommitments::::get(network_id); + 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); + }); advance_session(); - for commitment_details in BlockCommitments::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 0); - assert_eq!(commitment_details.last_updated, 0); - assert_eq!(commitment_details.commits, 0); - } + assert_eq!(BlockCommitments::::get(network_id).len(), 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::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 0); - assert_eq!(commitment_details.last_updated, 0); - assert_eq!(commitment_details.commits, 0); + assert_eq!(BlockCommitments::::get(network_id).len(), 0); + System::set_block_number(420); + + let last_stored_block = 1337; + let current_block = SlowClap::current_block_number(); + + for i in 0..3 { + assert_ok!(do_block_commitment( + session_index, + network_id, + i, + last_stored_block, + )); } - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .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 - )); - } - } + 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::::get(network_id) - .values() - .take(2) - { - assert_eq!(commitment_details.commits, 3); + let block_commitments = BlockCommitments::::get(network_id); + assert_eq!(block_commitments.get(&3), None); + block_commitments.values().for_each(|details| { + assert_eq!(details.commits, 1); + 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::::get(network_id), 0); + assert_eq!(BlockCommitments::::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::::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::::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::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 0); - assert_eq!(commitment_details.last_updated, 0); - } + assert_eq!(BlockCommitments::::get(network_id).len(), 0); - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("Time went backwards") - .as_millis() as u64; + let good_last_stored_block = 9_500_000; + let bad_last_stored_block = 9_100_000; - let mut block_commitment = CommitmentDetails { - 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 - )); - } + let current_block = SlowClap::current_block_number(); + for i in 0..3 { assert_ok!(do_block_commitment( session_index, network_id, - 3, - &bad_block_commitment + i, + good_last_stored_block, )); } + assert_ok!(do_block_commitment( + session_index, + network_id, + 3, + 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::::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() - .duration_since(UNIX_EPOCH) - .expect("Time went backwards") - .as_millis() as u64; + assert_eq!(BlockCommitments::::get(network_id).len(), 0); - let mut block_commitment = CommitmentDetails { - last_stored_block: 9_500_000, - commits: 420, - last_updated: timestamp, - }; + let last_stored_block = 9_500_000; + let next_stored_block = 9_600_000; + let current_block = SlowClap::current_block_number(); 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::::TimeWentBackwards, ); - block_commitment.last_updated += COMMITMENT_DELAY_MILLIS / 2; - assert_err!( - do_block_commitment(session_index, network_id, 0, &block_commitment), - Error::::TimeWentBackwards, - ); + BlockCommitments::::get(network_id) + .get(&0) + .map(|details| { + 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; - assert_err!( - do_block_commitment(session_index, network_id, 0, &block_commitment), - Error::::TimeWentBackwards, - ); + System::set_block_number(current_block + BLOCK_COMMITMENT_DELAY); - 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; - assert_err!( - do_block_commitment(session_index, network_id, 0, &block_commitment), - Error::::TimeWentBackwards, - ); - - for commitment_details in BlockCommitments::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 9_500_000); - 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, - )); + BlockCommitments::::get(network_id) + .get(&0) + .map(|details| { + assert_eq!(details.last_stored_block, next_stored_block); + assert_eq!(details.last_updated, new_current_block); + assert_eq!(details.commits, 2); + }) + .expect("authority_index 0 has voted; qed"); }); } @@ -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::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 0); - assert_eq!(commitment_details.last_updated, 0); - } + assert_eq!(BlockCommitments::::get(network_id).len(), 0); - 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, - }; + let last_stored_block = 9_500_000; + let current_block = SlowClap::current_block_number(); + let mut disabled_bitmp = BitMap::new(4); + assert_eq!( + DisabledAuthorityIndexes::::get(&session_index), + Some(disabled_bitmp.clone()) + ); Session::disable_index(3); + disabled_bitmp.set(3); + assert_eq!( + DisabledAuthorityIndexes::::get(&session_index), + Some(disabled_bitmp.clone()) + ); - // 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 - )); - } + for i in 0..=3 { + assert_ok!(do_block_commitment( + session_index, + network_id, + i, + last_stored_block, + )); } SlowClap::on_initialize(CHECK_BLOCK_INTERVAL); System::assert_has_event(RuntimeEvent::SlowClap( crate::Event::AuthoritiesCommitmentEquilibrium, )); + + let block_commitments = BlockCommitments::::get(network_id); + assert_eq!( + DisabledAuthorityIndexes::::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() - .duration_since(UNIX_EPOCH) - .expect("Time went backwards") - .as_millis() as u64; + assert_eq!(BlockCommitments::::get(network_id).len(), 0); - let mut block_commitment = CommitmentDetails { - last_stored_block: 9_500_000, - commits: 420, - last_updated: timestamp, - }; + let very_old_block = 9_499_999; + let last_stored_block = 9_500_000; + let current_block = SlowClap::current_block_number(); 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::::TimeWentBackwards + ); + assert_err!( + do_block_commitment(session_index, network_id, 3, very_old_block), Error::::TimeWentBackwards ); - for commitment_details in BlockCommitments::::get(network_id).values() { - assert_eq!(commitment_details.last_stored_block, 9_500_000); - assert_eq!(commitment_details.last_updated, timestamp); - assert_eq!(commitment_details.commits, 1); - } + BlockCommitments::::get(network_id) + .values() + .for_each(|details| { + 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() - .duration_since(UNIX_EPOCH) - .expect("Time went backwards") - .as_millis() as u64; + assert_eq!(BlockCommitments::::get(network_id).len(), 0); - let mut block_commitment = CommitmentDetails { - last_stored_block: 9_500_000, - commits: 420, - last_updated: timestamp, - }; + let last_stored_block = 9_500_000; + let current_block = SlowClap::current_block_number(); - 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 - )); - } + for i in 0..3 { + assert_ok!(do_block_commitment( + session_index, + network_id, + i, + 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::::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();