diff --git a/Cargo.lock b/Cargo.lock index 2a06b1f993e59..39c6adebdeeae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6967,6 +6967,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "sp-session", "sp-staking", "sp-std", ] @@ -11373,6 +11374,7 @@ dependencies = [ name = "sp-session" version = "4.0.0-dev" dependencies = [ + "impl-trait-for-tuples", "parity-scale-codec", "scale-info", "sp-api", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index e1434f0504bd3..3bc79e3fbe130 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -533,6 +533,7 @@ impl pallet_session::Config for Runtime { type NextSessionRotation = Babe; type SessionManager = pallet_session::historical::NoteHistoricalRoot; type SessionHandler = ::KeyTypeIdProviders; + type SessionChangeListener = Offences; type Keys = SessionKeys; type WeightInfo = pallet_session::weights::SubstrateWeight; } @@ -1339,10 +1340,16 @@ impl pallet_im_online::Config for Runtime { type MaxPeerInHeartbeats = MaxPeerInHeartbeats; } +parameter_types! { + pub const MaxSessionReportAge: u32 = BondingDuration::get() * SessionsPerEra::get(); +} + impl pallet_offences::Config for Runtime { type RuntimeEvent = RuntimeEvent; type IdentificationTuple = pallet_session::historical::IdentificationTuple; type OnOffenceHandler = Staking; + type MaxSessionReportAge = MaxSessionReportAge; + type SessionInfoProvider = Session; } impl pallet_authority_discovery::Config for Runtime { diff --git a/frame/authority-discovery/src/lib.rs b/frame/authority-discovery/src/lib.rs index 6365c95359472..5d3d35ad27c96 100644 --- a/frame/authority-discovery/src/lib.rs +++ b/frame/authority-discovery/src/lib.rs @@ -213,6 +213,7 @@ mod tests { type ValidatorIdOf = ConvertInto; type NextSessionRotation = pallet_session::PeriodicSessions; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Test { diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index c0ccc9a8acd32..b8a8364afe2d8 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -114,6 +114,7 @@ impl pallet_session::Config for Test { type SessionHandler = ::KeyTypeIdProviders; type Keys = MockSessionKeys; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Test { @@ -166,6 +167,7 @@ parameter_types! { pub const SlashDeferDuration: EraIndex = 0; pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(16); + pub const MaxSessionReportAge: SessionIndex = BondingDuration::get() * SessionsPerEra::get(); } pub struct OnChainSeqPhragmen; @@ -213,6 +215,8 @@ impl pallet_offences::Config for Test { type RuntimeEvent = RuntimeEvent; type IdentificationTuple = pallet_session::historical::IdentificationTuple; type OnOffenceHandler = Staking; + type SessionInfoProvider = Session; + type MaxSessionReportAge = MaxSessionReportAge; } parameter_types! { diff --git a/frame/beefy-mmr/src/mock.rs b/frame/beefy-mmr/src/mock.rs index 31484aaa6be70..d7588f42202a1 100644 --- a/frame/beefy-mmr/src/mock.rs +++ b/frame/beefy-mmr/src/mock.rs @@ -99,6 +99,7 @@ impl pallet_session::Config for Test { type SessionHandler = ::KeyTypeIdProviders; type Keys = MockSessionKeys; type WeightInfo = (); + type SessionChangeListener = (); } pub type MmrLeaf = sp_consensus_beefy::mmr::MmrLeaf< diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 7edf4d339758c..ad561754b60f0 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -139,6 +139,7 @@ impl pallet_session::Config for Test { type SessionHandler = ::KeyTypeIdProviders; type Keys = MockSessionKeys; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Test { @@ -190,6 +191,7 @@ parameter_types! { pub const BondingDuration: EraIndex = 3; pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(17); + pub const MaxSessionReportAge: SessionIndex = BondingDuration::get() * SessionsPerEra::get(); } pub struct OnChainSeqPhragmen; @@ -237,6 +239,8 @@ impl pallet_offences::Config for Test { type RuntimeEvent = RuntimeEvent; type IdentificationTuple = pallet_session::historical::IdentificationTuple; type OnOffenceHandler = Staking; + type SessionInfoProvider = Session; + type MaxSessionReportAge = MaxSessionReportAge; } // Note, that we can't use `UintAuthorityId` here. Reason is that the implementation diff --git a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index da7ccf6dce9ce..9acdb77c9aa0b 100644 --- a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -159,6 +159,7 @@ impl pallet_session::Config for Runtime { type ValidatorId = AccountId; type ValidatorIdOf = pallet_staking::StashOf; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Runtime { type FullIdentification = pallet_staking::Exposure; diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index df012ab9dc6e8..88412714b460a 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -120,6 +120,7 @@ impl pallet_session::Config for Test { type SessionHandler = ::KeyTypeIdProviders; type Keys = TestSessionKeys; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Test { @@ -171,6 +172,7 @@ parameter_types! { pub const BondingDuration: EraIndex = 3; pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(17); + pub const MaxSessionReportAge: SessionIndex = BondingDuration::get() * SessionsPerEra::get(); } pub struct OnChainSeqPhragmen; @@ -218,6 +220,8 @@ impl pallet_offences::Config for Test { type RuntimeEvent = RuntimeEvent; type IdentificationTuple = pallet_session::historical::IdentificationTuple; type OnOffenceHandler = Staking; + type SessionInfoProvider = Session; + type MaxSessionReportAge = MaxSessionReportAge; } parameter_types! { diff --git a/frame/im-online/src/mock.rs b/frame/im-online/src/mock.rs index 78a86edf54598..91f9c2ceb0a02 100644 --- a/frame/im-online/src/mock.rs +++ b/frame/im-online/src/mock.rs @@ -160,6 +160,7 @@ impl pallet_session::Config for Runtime { type RuntimeEvent = RuntimeEvent; type NextSessionRotation = pallet_session::PeriodicSessions; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Runtime { diff --git a/frame/offences/Cargo.toml b/frame/offences/Cargo.toml index 5d418dfa364b2..a09e3b7feb1b9 100644 --- a/frame/offences/Cargo.toml +++ b/frame/offences/Cargo.toml @@ -22,6 +22,7 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys pallet-balances = { version = "4.0.0-dev", default-features = false, path = "../balances" } sp-runtime = { version = "24.0.0", default-features = false, path = "../../primitives/runtime" } sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../primitives/staking" } +sp-session = { version = "4.0.0-dev", default-features = false, path = "../../primitives/session" } sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" } [dev-dependencies] @@ -41,6 +42,7 @@ std = [ "sp-runtime/std", "sp-staking/std", "sp-std/std", + "sp-session/std", ] runtime-benchmarks = [] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 6bb3b9d7f4ee8..b796b5c54533b 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -128,6 +128,7 @@ impl pallet_session::Config for Test { type ValidatorId = AccountId; type ValidatorIdOf = pallet_staking::StashOf; type WeightInfo = (); + type SessionChangeListener = (); } pallet_staking_reward_curve::build! { @@ -203,6 +204,8 @@ impl pallet_offences::Config for Test { type RuntimeEvent = RuntimeEvent; type IdentificationTuple = pallet_session::historical::IdentificationTuple; type OnOffenceHandler = Staking; + type SessionInfoProvider = Session; + type MaxSessionReportAge = ConstU32<6>; } impl frame_system::offchain::SendTransactionTypes for Test diff --git a/frame/offences/src/lib.rs b/frame/offences/src/lib.rs index 1c7ffeca71983..9049883adeccb 100644 --- a/frame/offences/src/lib.rs +++ b/frame/offences/src/lib.rs @@ -17,7 +17,25 @@ //! # Offences Pallet //! -//! Tracks reported offences +//! The offences pallet tracks reported offences using 3 key storage types: +//! +//! - [`Reports`]: A storage map of a report hash to its details +//! - [`ConcurrentReportsIndex`]: A storage double map that stores a vector of reports +//! for a specific [`Kind`] and [`OpaqueTimeSlot`] +//! - [`SessionReports`]: A storage map that keeps a vector of active reports for a +//! [`SessionIndex`]. +//! +//! When a new offence is reported using [`ReportOffence::report_offence`], its `session_index` is +//! first compared against the current `session_index`. +//! +//! If older than [`Config::MaxSessionReportAge`], the report is rejected right away. +//! +//! Else, all concurrent reports are loaded to determine the slash fraction and updated. +//! The report is also inserted in [`SessionReports`] at this time. +//! Finally, [`Config::OnOffenceHandler`] is called to handle any actions for this report. +//! +//! On the start of a new session, `clear_obsolete_reports` clears all reports +//! that are older than [`Config::MaxSessionReportAge`]. // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] @@ -26,25 +44,28 @@ pub mod migration; mod mock; mod tests; -use core::marker::PhantomData; - use codec::Encode; -use frame_support::weights::Weight; +use core::marker::PhantomData; +use frame_support::{ensure, traits::Get, weights::Weight}; use sp_runtime::{traits::Hash, Perbill}; +use sp_session::{SessionChangeListener, SessionInfoProvider}; use sp_staking::{ offence::{Kind, Offence, OffenceDetails, OffenceError, OnOffenceHandler, ReportOffence}, - SessionIndex, + EraIndex, SessionIndex, }; use sp_std::prelude::*; pub use pallet::*; /// A binary blob which represents a SCALE codec-encoded `O::TimeSlot`. -type OpaqueTimeSlot = Vec; +pub type OpaqueTimeSlot = Vec; /// A type alias for a report identifier. type ReportIdOf = ::Hash; +/// A type alias for the data stored for a report in each session. +type SessionReportOf = (Kind, OpaqueTimeSlot, ReportIdOf); + const LOG_TARGET: &str = "runtime::offences"; #[frame_support::pallet] @@ -68,6 +89,14 @@ pub mod pallet { type IdentificationTuple: Parameter; /// A handler called for every offence report. type OnOffenceHandler: OnOffenceHandler; + + /// Number of sessions for which a report is stored. + /// Once it gets older than this value, it gets cleaned up at the start of a new session. + #[pallet::constant] + type MaxSessionReportAge: Get; + + /// A trait that provides information about the current session. + type SessionInfoProvider: SessionInfoProvider; } /// The primary structure that holds all offence records keyed by report identifiers. @@ -92,6 +121,16 @@ pub mod pallet { ValueQuery, >; + /// A map that stores all reports along with their kind & time slot info for a `SessionIndex`. + /// + /// On start of a new session, all reports with a session index older than + /// `MaxSessionReportAge` are removed. + /// + /// Note that `time_slot` is encoded and stored as an opaque type. + #[pallet::storage] + pub type SessionReports = + StorageMap<_, Twox64Concat, SessionIndex, Vec>, ValueQuery>; + /// Events type. #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -103,6 +142,30 @@ pub mod pallet { } } +impl Pallet { + /// This is called at the start of each new session. Hence, only one session becomes + /// obsolete in this call, whose data gets cleared up here. + /// + /// For example, if the `current_session_index` is 10 and `MaxSessionReportAge` is 6, + /// this clears all reports for `obsolete_session_index` 4. + fn clear_obsolete_reports(current_session_index: SessionIndex) { + let Some(obsolete_session_index) = current_session_index.checked_sub(T::MaxSessionReportAge::get()) else { return }; + + let session_reports = SessionReports::::take(obsolete_session_index); + + for (kind, time_slot, report_id) in &session_reports { + ConcurrentReportsIndex::::remove(kind, time_slot); + Reports::::remove(report_id); + } + } +} + +impl SessionChangeListener for Pallet { + fn on_session_change(session_index: SessionIndex) { + Self::clear_obsolete_reports(session_index) + } +} + impl ReportOffence for Pallet where T: Config, @@ -112,10 +175,19 @@ where let offenders = offence.offenders(); let time_slot = offence.time_slot(); + let session_index = offence.session_index(); + let current_session_index = T::SessionInfoProvider::current_session_index(); + + ensure!( + session_index >= current_session_index.saturating_sub(T::MaxSessionReportAge::get()), + OffenceError::ObsoleteReport + ); + // Go through all offenders in the offence report and find all offenders that were spotted // in unique reports. let TriageOutcome { concurrent_offenders } = - match Self::triage_offence_report::(reporters, &time_slot, offenders) { + match Self::triage_offence_report::(reporters, &time_slot, session_index, offenders) + { Some(triage) => triage, // The report contained only duplicates, so there is no need to slash again. None => return Err(OffenceError::DuplicateReport), @@ -167,9 +239,10 @@ impl Pallet { fn triage_offence_report>( reporters: Vec, time_slot: &O::TimeSlot, + session_index: SessionIndex, offenders: Vec, ) -> Option> { - let mut storage = ReportIndexStorage::::load(time_slot); + let mut storage = ReportIndexStorage::::load(time_slot, session_index); let mut any_new = false; for offender in offenders { @@ -182,7 +255,7 @@ impl Pallet { OffenceDetails { offender, reporters: reporters.clone() }, ); - storage.insert(report_id); + storage.insert(report_id, &time_slot); } } @@ -216,28 +289,41 @@ struct TriageOutcome { #[must_use = "The changes are not saved without called `save`"] struct ReportIndexStorage> { opaque_time_slot: OpaqueTimeSlot, + session_index: SessionIndex, concurrent_reports: Vec>, + session_reports: Vec<(Kind, OpaqueTimeSlot, ReportIdOf)>, _phantom: PhantomData, } impl> ReportIndexStorage { /// Preload indexes from the storage for the specific `time_slot` and the kind of the offence. - fn load(time_slot: &O::TimeSlot) -> Self { + fn load(time_slot: &O::TimeSlot, session_index: SessionIndex) -> Self { let opaque_time_slot = time_slot.encode(); + let session_reports = SessionReports::::get(session_index); let concurrent_reports = >::get(&O::ID, &opaque_time_slot); - Self { opaque_time_slot, concurrent_reports, _phantom: Default::default() } + Self { + opaque_time_slot, + session_index, + concurrent_reports, + session_reports, + _phantom: Default::default(), + } } /// Insert a new report to the index. - fn insert(&mut self, report_id: ReportIdOf) { + fn insert(&mut self, report_id: ReportIdOf, time_slot: &O::TimeSlot) { + let opaque_time_slot = time_slot.encode(); + self.session_reports.push((O::ID, opaque_time_slot, report_id)); + // Update the list of concurrent reports. self.concurrent_reports.push(report_id); } /// Dump the indexes to the storage. fn save(self) { + SessionReports::::set(self.session_index, self.session_reports); >::insert( &O::ID, &self.opaque_time_slot, diff --git a/frame/offences/src/mock.rs b/frame/offences/src/mock.rs index 17480be76c1d8..9c6645b2ee8f1 100644 --- a/frame/offences/src/mock.rs +++ b/frame/offences/src/mock.rs @@ -66,6 +66,13 @@ pub fn with_on_offence_fractions) -> R>(f: F) -> OnOffencePerbill::mutate(|fractions| f(fractions)) } +pub struct SessionInfoProvider; +impl sp_session::SessionInfoProvider for SessionInfoProvider { + fn current_session_index() -> SessionIndex { + 10 + } +} + type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -111,6 +118,8 @@ impl Config for Runtime { type RuntimeEvent = RuntimeEvent; type IdentificationTuple = u64; type OnOffenceHandler = OnOffenceHandler; + type MaxSessionReportAge = ConstU32<6>; + type SessionInfoProvider = SessionInfoProvider; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -138,6 +147,7 @@ pub struct Offence { pub validator_set_count: u32, pub offenders: Vec, pub time_slot: u128, + pub session_index: SessionIndex, } impl offence::Offence for Offence { @@ -157,10 +167,15 @@ impl offence::Offence for Offence { } fn session_index(&self) -> SessionIndex { - 1 + self.session_index } fn slash_fraction(&self, offenders_count: u32) -> Perbill { Perbill::from_percent(5 + offenders_count * 100 / self.validator_set_count) } } + +/// Create the report id for the given `offender` and `time_slot` combination. +pub fn report_id(time_slot: u128, offender: u64) -> H256 { + Offences::report_id::(&time_slot, &offender) +} diff --git a/frame/offences/src/tests.rs b/frame/offences/src/tests.rs index d525c7c3ab1d9..7f7c05848f17d 100644 --- a/frame/offences/src/tests.rs +++ b/frame/offences/src/tests.rs @@ -21,12 +21,15 @@ use super::*; use crate::mock::{ - new_test_ext, offence_reports, with_on_offence_fractions, Offence, Offences, RuntimeEvent, - System, KIND, + new_test_ext, offence_reports, report_id, with_on_offence_fractions, Offence, Offences, + Runtime, RuntimeEvent, System, KIND, }; +use frame_support::assert_noop; use frame_system::{EventRecord, Phase}; use sp_runtime::Perbill; +static SESSION_INDEX: u32 = 10; + #[test] fn should_report_an_authority_and_trigger_on_offence() { new_test_ext().execute_with(|| { @@ -34,7 +37,12 @@ fn should_report_an_authority_and_trigger_on_offence() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; + let offence = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; // when Offences::report_offence(vec![], offence).unwrap(); @@ -53,7 +61,12 @@ fn should_not_report_the_same_authority_twice_in_the_same_slot() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; + let offence = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; Offences::report_offence(vec![], offence.clone()).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); @@ -71,6 +84,27 @@ fn should_not_report_the_same_authority_twice_in_the_same_slot() { }); } +#[test] +fn should_not_report_an_obsolete_offence() { + new_test_ext().execute_with(|| { + // given + let time_slot = 42; + assert_eq!(offence_reports(KIND, time_slot), vec![]); + + let offence = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX - 7, + time_slot, + offenders: vec![5], + }; + assert_noop!(Offences::report_offence(vec![], offence), OffenceError::ObsoleteReport); + + with_on_offence_fractions(|f| { + assert_eq!(f.clone(), vec![]); + }); + }); +} + #[test] fn should_report_in_different_time_slot() { new_test_ext().execute_with(|| { @@ -78,7 +112,12 @@ fn should_report_in_different_time_slot() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let mut offence = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; + let mut offence = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; Offences::report_offence(vec![], offence.clone()).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); @@ -104,7 +143,12 @@ fn should_deposit_event() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; + let offence = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; // when Offences::report_offence(vec![], offence).unwrap(); @@ -131,7 +175,12 @@ fn doesnt_deposit_event_for_dups() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; + let offence = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; Offences::report_offence(vec![], offence.clone()).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); @@ -164,8 +213,12 @@ fn reports_if_an_offence_is_dup() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence = - |time_slot, offenders| Offence { validator_set_count: 5, time_slot, offenders }; + let offence = |time_slot, offenders| Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders, + }; let mut test_offence = offence(time_slot, vec![0]); @@ -222,8 +275,18 @@ fn should_properly_count_offences() { let time_slot = 42; assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence1 = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; - let offence2 = Offence { validator_set_count: 5, time_slot, offenders: vec![4] }; + let offence1 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; + let offence2 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![4], + }; Offences::report_offence(vec![], offence1).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); @@ -245,3 +308,156 @@ fn should_properly_count_offences() { ); }); } + +#[test] +fn should_properly_store_offences() { + new_test_ext().execute_with(|| { + // given + let time_slot = 42; + assert_eq!(offence_reports(KIND, time_slot), vec![]); + + let offence1 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; + let offence2 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot: time_slot + 1, + offenders: vec![4], + }; + let offence3 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX + 1, + time_slot: time_slot + 1, + offenders: vec![6, 7], + }; + let offence4 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX - 1, + time_slot: time_slot - 1, + offenders: vec![3], + }; + + // when + Offences::report_offence(vec![], offence1).unwrap(); + + // then + with_on_offence_fractions(|f| { + assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); + f.clear(); + }); + + // when + // report for the second time + Offences::report_offence(vec![], offence2).unwrap(); + Offences::report_offence(vec![], offence3).unwrap(); + Offences::report_offence(vec![], offence4).unwrap(); + + // then + let prev_session_reports = SessionReports::::get(SESSION_INDEX - 1); + assert_eq!( + prev_session_reports, + vec![(KIND, (time_slot - 1).encode(), report_id(time_slot - 1, 3)),] + ); + + let session_reports = SessionReports::::get(SESSION_INDEX); + assert_eq!( + session_reports, + vec![ + (KIND, time_slot.encode(), report_id(time_slot, 5)), + (KIND, (time_slot + 1).encode(), report_id(time_slot + 1, 4)), + ] + ); + + let next_session_reports = SessionReports::::get(SESSION_INDEX + 1); + assert_eq!( + next_session_reports, + vec![ + (KIND, (time_slot + 1).encode(), report_id(time_slot + 1, 6)), + (KIND, (time_slot + 1).encode(), report_id(time_slot + 1, 7)), + ] + ); + }); +} + +#[test] +fn should_properly_clear_obsolete_offences() { + new_test_ext().execute_with(|| { + // given + let time_slot = 42; + assert_eq!(offence_reports(KIND, time_slot), vec![]); + + let offence1 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot, + offenders: vec![5], + }; + let offence2 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX, + time_slot: time_slot + 1, + offenders: vec![4], + }; + let offence3 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX + 1, + time_slot: time_slot + 5, + offenders: vec![6, 7], + }; + let offence4 = Offence { + validator_set_count: 5, + session_index: SESSION_INDEX - 1, + time_slot: time_slot - 1, + offenders: vec![3], + }; + + // when + Offences::report_offence(vec![], offence1).unwrap(); + + // then + with_on_offence_fractions(|f| { + assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); + f.clear(); + }); + + // when + // report for the second time + Offences::report_offence(vec![], offence2).unwrap(); + Offences::report_offence(vec![], offence3).unwrap(); + Offences::report_offence(vec![], offence4).unwrap(); + + assert_eq!( + offence_reports(KIND, time_slot - 1), + vec![OffenceDetails { offender: 3, reporters: vec![] }] + ); + + ::on_session_change(SESSION_INDEX + 5); + + assert_eq!(offence_reports(KIND, time_slot - 1), vec![]); + + // then + let obsolete_session_reports = SessionReports::::get(SESSION_INDEX - 1); + assert_eq!(obsolete_session_reports, vec![]); + + let session_reports = SessionReports::::get(SESSION_INDEX + 1); + assert_eq!( + session_reports, + vec![ + (KIND, (time_slot + 5).encode(), report_id(time_slot + 5, 6)), + (KIND, (time_slot + 5).encode(), report_id(time_slot + 5, 7)), + ] + ); + + assert_eq!( + offence_reports(KIND, time_slot + 5), + vec![ + OffenceDetails { offender: 6, reporters: vec![] }, + OffenceDetails { offender: 7, reporters: vec![] }, + ] + ); + }); +} diff --git a/frame/root-offences/src/mock.rs b/frame/root-offences/src/mock.rs index 8c48e34e4e04a..654cf51ad3123 100644 --- a/frame/root-offences/src/mock.rs +++ b/frame/root-offences/src/mock.rs @@ -222,6 +222,7 @@ impl pallet_session::Config for Test { type ValidatorIdOf = pallet_staking::StashOf; type NextSessionRotation = pallet_session::PeriodicSessions; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_timestamp::Config for Test { diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 3b027492e0a6a..9cf5f92a001a5 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -133,6 +133,7 @@ impl pallet_session::Config for Test { type ValidatorId = AccountId; type ValidatorIdOf = pallet_staking::StashOf; type WeightInfo = (); + type SessionChangeListener = (); } pallet_staking_reward_curve::build! { const I_NPOS: sp_runtime::curve::PiecewiseLinear<'static> = curve!( diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 1219aaaf12a10..9f4bf2e2b555c 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -130,6 +130,7 @@ use sp_runtime::{ traits::{AtLeast32BitUnsigned, Convert, Member, One, OpaqueKeys, Zero}, ConsensusEngineId, KeyTypeId, Permill, RuntimeAppPublic, }; +use sp_session::{SessionChangeListener, SessionInfoProvider}; use sp_staking::SessionIndex; use sp_std::{ marker::PhantomData, @@ -403,9 +404,12 @@ pub mod pallet { /// Handler for managing new session. type SessionManager: SessionManager; - /// Handler when a session has changed. + /// Handler when a session has changed for specific key ids. type SessionHandler: SessionHandler; + /// Handler when a session has changed. + type SessionChangeListener: SessionChangeListener; + /// The keys. type Keys: OpaqueKeys + Member + Parameter + MaybeSerializeDeserialize; @@ -698,6 +702,7 @@ impl Pallet { // Tell everyone about the new session keys. T::SessionHandler::on_new_session::(changed, &session_keys, &queued_amalgamated); + T::SessionChangeListener::on_session_change(session_index); } /// Disable the validator of index `i`, returns `false` if the validator was already disabled. @@ -937,3 +942,9 @@ impl> FindAuthor validators.get(i as usize).cloned() } } + +impl SessionInfoProvider for Pallet { + fn current_session_index() -> SessionIndex { + >::get() + } +} diff --git a/frame/session/src/mock.rs b/frame/session/src/mock.rs index d6b8d9e207e02..c769d7464b965 100644 --- a/frame/session/src/mock.rs +++ b/frame/session/src/mock.rs @@ -297,6 +297,7 @@ impl Config for Test { type RuntimeEvent = RuntimeEvent; type NextSessionRotation = (); type WeightInfo = (); + type SessionChangeListener = (); } #[cfg(feature = "historical")] diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index f9af9f5003b43..1c260456767a8 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -178,6 +178,7 @@ impl pallet_session::Config for Test { type ValidatorIdOf = crate::StashOf; type NextSessionRotation = pallet_session::PeriodicSessions; type WeightInfo = (); + type SessionChangeListener = (); } impl pallet_session::historical::Config for Test { diff --git a/primitives/session/Cargo.toml b/primitives/session/Cargo.toml index f43390f0a1069..3f5842b44aa4a 100644 --- a/primitives/session/Cargo.toml +++ b/primitives/session/Cargo.toml @@ -20,6 +20,7 @@ sp-core = { version = "21.0.0", default-features = false, path = "../core" } sp-runtime = { version = "24.0.0", optional = true, path = "../runtime" } sp-staking = { version = "4.0.0-dev", default-features = false, path = "../staking" } sp-std = { version = "8.0.0", default-features = false, path = "../std" } +impl-trait-for-tuples = { version = "0.2.2", default-features = true } [features] default = [ "std" ] diff --git a/primitives/session/src/lib.rs b/primitives/session/src/lib.rs index 642aa2a21143e..2c085fc89f0a5 100644 --- a/primitives/session/src/lib.rs +++ b/primitives/session/src/lib.rs @@ -130,3 +130,15 @@ where Ok(()) } + +/// A trait to be notified when the session changes. +/// This is in contrast to `SessionHandler` which handles events for specific keys. +#[impl_trait_for_tuples::impl_for_tuples(8)] +pub trait SessionChangeListener { + fn on_session_change(session_index: SessionIndex); +} + +/// A trait get the current session info +pub trait SessionInfoProvider { + fn current_session_index() -> SessionIndex; +} diff --git a/primitives/staking/src/offence.rs b/primitives/staking/src/offence.rs index 8013166374e06..271cb0d737369 100644 --- a/primitives/staking/src/offence.rs +++ b/primitives/staking/src/offence.rs @@ -117,9 +117,12 @@ pub trait Offence { /// Errors that may happen on offence reports. #[derive(PartialEq, sp_runtime::RuntimeDebug)] pub enum OffenceError { - /// The report has already been sumbmitted. + /// The report has already been submitted. DuplicateReport, + /// The report is no longer useful. + ObsoleteReport, + /// Other error has happened. Other(u8), } @@ -129,6 +132,7 @@ impl sp_runtime::traits::Printable for OffenceError { "OffenceError".print(); match self { Self::DuplicateReport => "DuplicateReport".print(), + Self::ObsoleteReport => "ObsoleteReport".print(), Self::Other(e) => { "Other".print(); e.print();