diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index ae8251f620966b33b40adedd1892bc962325abc7..85889a50c20b98116317e4238ac5733f1e599b89 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 226, + spec_version: 227, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; diff --git a/substrate/frame/im-online/src/lib.rs b/substrate/frame/im-online/src/lib.rs index d6d13c66c250152b38cfaa651be176cd3287daf9..9aa8d2d67f6feb6d1ef5c0d18b2f30280d60b678 100644 --- a/substrate/frame/im-online/src/lib.rs +++ b/substrate/frame/im-online/src/lib.rs @@ -608,7 +608,9 @@ impl<T: Trait> pallet_session::OneSessionHandler<T::AccountId> for Module<T> { let validator_set_count = keys.len() as u32; let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders }; - T::ReportUnresponsiveness::report_offence(vec![], offence); + if let Err(e) = T::ReportUnresponsiveness::report_offence(vec![], offence) { + sp_runtime::print(e); + } } } diff --git a/substrate/frame/im-online/src/mock.rs b/substrate/frame/im-online/src/mock.rs index a703b24629c0f8ef5cc2c60894fb8109903c6748..97a0e7eb844c4794776ecd26cba869bdb6168e09 100644 --- a/substrate/frame/im-online/src/mock.rs +++ b/substrate/frame/im-online/src/mock.rs @@ -22,7 +22,7 @@ use std::cell::RefCell; use crate::{Module, Trait}; use sp_runtime::Perbill; -use sp_staking::{SessionIndex, offence::ReportOffence}; +use sp_staking::{SessionIndex, offence::{ReportOffence, OffenceError}}; use sp_runtime::testing::{Header, UintAuthorityId, TestXt}; use sp_runtime::traits::{IdentityLookup, BlakeTwo256, ConvertInto}; use sp_core::H256; @@ -77,8 +77,9 @@ thread_local! { /// A mock offence report handler. pub struct OffenceHandler; impl ReportOffence<u64, IdentificationTuple, Offence> for OffenceHandler { - fn report_offence(reporters: Vec<u64>, offence: Offence) { + fn report_offence(reporters: Vec<u64>, offence: Offence) -> Result<(), OffenceError> { OFFENCES.with(|l| l.borrow_mut().push((reporters, offence))); + Ok(()) } } diff --git a/substrate/frame/offences/src/lib.rs b/substrate/frame/offences/src/lib.rs index 7831ba65a3b067b4b68746d4272073e3a79b77c8..27983cbb5332e57e95724999338e850dfd77f7c9 100644 --- a/substrate/frame/offences/src/lib.rs +++ b/substrate/frame/offences/src/lib.rs @@ -30,7 +30,7 @@ use frame_support::{ }; use sp_runtime::traits::Hash; use sp_staking::{ - offence::{Offence, ReportOffence, Kind, OnOffenceHandler, OffenceDetails}, + offence::{Offence, ReportOffence, Kind, OnOffenceHandler, OffenceDetails, OffenceError}, }; use codec::{Encode, Decode}; use frame_system as system; @@ -90,7 +90,7 @@ impl<T: Trait, O: Offence<T::IdentificationTuple>> where T::IdentificationTuple: Clone, { - fn report_offence(reporters: Vec<T::AccountId>, offence: O) { + fn report_offence(reporters: Vec<T::AccountId>, offence: O) -> Result<(), OffenceError> { let offenders = offence.offenders(); let time_slot = offence.time_slot(); let validator_set_count = offence.validator_set_count(); @@ -104,7 +104,7 @@ where ) { Some(triage) => triage, // The report contained only duplicates, so there is no need to slash again. - None => return, + None => return Err(OffenceError::DuplicateReport), }; // Deposit the event. @@ -123,6 +123,8 @@ where &slash_perbill, offence.session_index(), ); + + Ok(()) } } diff --git a/substrate/frame/offences/src/tests.rs b/substrate/frame/offences/src/tests.rs index f2f82cf7a87eee70677d28dc81cb88808ef19451..0ed98427c65f8eba4d02be1e679dc1e8db52d791 100644 --- a/substrate/frame/offences/src/tests.rs +++ b/substrate/frame/offences/src/tests.rs @@ -40,7 +40,7 @@ fn should_report_an_authority_and_trigger_on_offence() { }; // when - Offences::report_offence(vec![], offence); + Offences::report_offence(vec![], offence).unwrap(); // then with_on_offence_fractions(|f| { @@ -61,7 +61,7 @@ fn should_not_report_the_same_authority_twice_in_the_same_slot() { time_slot, offenders: vec![5], }; - Offences::report_offence(vec![], offence.clone()); + Offences::report_offence(vec![], offence.clone()).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); f.clear(); @@ -69,7 +69,7 @@ fn should_not_report_the_same_authority_twice_in_the_same_slot() { // when // report for the second time - Offences::report_offence(vec![], offence); + assert_eq!(Offences::report_offence(vec![], offence), Err(OffenceError::DuplicateReport)); // then with_on_offence_fractions(|f| { @@ -91,7 +91,7 @@ fn should_report_in_different_time_slot() { time_slot, offenders: vec![5], }; - Offences::report_offence(vec![], offence.clone()); + Offences::report_offence(vec![], offence.clone()).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); f.clear(); @@ -100,7 +100,7 @@ fn should_report_in_different_time_slot() { // when // report for the second time offence.time_slot += 1; - Offences::report_offence(vec![], offence); + Offences::report_offence(vec![], offence).unwrap(); // then with_on_offence_fractions(|f| { @@ -123,7 +123,7 @@ fn should_deposit_event() { }; // when - Offences::report_offence(vec![], offence); + Offences::report_offence(vec![], offence).unwrap(); // then assert_eq!( @@ -149,7 +149,7 @@ fn doesnt_deposit_event_for_dups() { time_slot, offenders: vec![5], }; - Offences::report_offence(vec![], offence.clone()); + Offences::report_offence(vec![], offence.clone()).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); f.clear(); @@ -157,7 +157,7 @@ fn doesnt_deposit_event_for_dups() { // when // report for the second time - Offences::report_offence(vec![], offence); + assert_eq!(Offences::report_offence(vec![], offence), Err(OffenceError::DuplicateReport)); // then // there is only one event. @@ -191,7 +191,7 @@ fn should_properly_count_offences() { time_slot, offenders: vec![4], }; - Offences::report_offence(vec![], offence1); + Offences::report_offence(vec![], offence1).unwrap(); with_on_offence_fractions(|f| { assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); f.clear(); @@ -199,7 +199,7 @@ fn should_properly_count_offences() { // when // report for the second time - Offences::report_offence(vec![], offence2); + Offences::report_offence(vec![], offence2).unwrap(); // then // the 1st authority should have count 2 and the 2nd one should be reported only once. diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index ee708dabd3c6e447b5f9741e7d89d62de166f9f4..81bce3b5487a19e433bf212bc104725d96af9823 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -276,7 +276,7 @@ use sp_runtime::{ }; use sp_staking::{ SessionIndex, - offence::{OnOffenceHandler, OffenceDetails, Offence, ReportOffence}, + offence::{OnOffenceHandler, OffenceDetails, Offence, ReportOffence, OffenceError}, }; #[cfg(feature = "std")] use sp_runtime::{Serialize, Deserialize}; @@ -1828,7 +1828,7 @@ impl<T, Reporter, Offender, R, O> ReportOffence<Reporter, Offender, O> R: ReportOffence<Reporter, Offender, O>, O: Offence<Offender>, { - fn report_offence(reporters: Vec<Reporter>, offence: O) { + fn report_offence(reporters: Vec<Reporter>, offence: O) -> Result<(), OffenceError> { <Module<T>>::ensure_storage_upgraded(); // disallow any slashing from before the current bonding period. @@ -1840,7 +1840,8 @@ impl<T, Reporter, Offender, R, O> ReportOffence<Reporter, Offender, O> } else { <Module<T>>::deposit_event( RawEvent::OldSlashingReportDiscarded(offence_session) - ) + ); + Ok(()) } } } diff --git a/substrate/primitives/staking/src/offence.rs b/substrate/primitives/staking/src/offence.rs index cac1ed065ceb7916242f45cf564eb29a08697b10..06e73f018b7656a691801b93b2178631cacd5ddd 100644 --- a/substrate/primitives/staking/src/offence.rs +++ b/substrate/primitives/staking/src/offence.rs @@ -91,14 +91,37 @@ pub trait Offence<Offender> { ) -> Perbill; } +/// Errors that may happen on offence reports. +#[derive(PartialEq, sp_runtime::RuntimeDebug)] +pub enum OffenceError { + /// The report has already been sumbmitted. + DuplicateReport, + + /// Other error has happened. + Other(u8), +} + +impl sp_runtime::traits::Printable for OffenceError { + fn print(&self) { + "OffenceError".print(); + match self { + Self::DuplicateReport => "DuplicateReport".print(), + Self::Other(e) => { + "Other".print(); + e.print(); + } + } + } +} + /// A trait for decoupling offence reporters from the actual handling of offence reports. pub trait ReportOffence<Reporter, Offender, O: Offence<Offender>> { /// Report an `offence` and reward given `reporters`. - fn report_offence(reporters: Vec<Reporter>, offence: O); + fn report_offence(reporters: Vec<Reporter>, offence: O) -> Result<(), OffenceError>; } impl<Reporter, Offender, O: Offence<Offender>> ReportOffence<Reporter, Offender, O> for () { - fn report_offence(_reporters: Vec<Reporter>, _offence: O) {} + fn report_offence(_reporters: Vec<Reporter>, _offence: O) -> Result<(), OffenceError> { Ok(()) } } /// A trait to take action on an offence.