Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove Offence delay #8414

Merged
34 commits merged into from
May 3, 2021
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e58f008
Removed can_report api from OnOffenceHandler
Lohann Mar 22, 2021
a00589c
Removed DeferredOffences and create a storage migration
Lohann Mar 22, 2021
cfacecf
Removed missing comments
Lohann Mar 22, 2021
9654509
Mock set_deferred_offences and deferred_offences methods
Lohann Mar 22, 2021
50136e0
OnOffenceHandler::on_offence always succeed
Lohann Mar 23, 2021
2869ecf
Fix benchmark tests
Lohann Mar 23, 2021
1b65516
Fix runtime-benchmark cfg methods
Lohann Mar 23, 2021
61cadeb
Removed 'applied' attribute from Offence event
Lohann Mar 24, 2021
5befe38
refactor deprecated deferred offences getter
Lohann Mar 24, 2021
7ae4c5b
Validate if offences are submited after on_runtime_upgrade
Lohann Mar 24, 2021
8599172
update changelog
Lohann Mar 24, 2021
fa61742
Remove empty lines
Lohann Mar 25, 2021
bdbf5c3
Fix remove_deferred_storage weights
Lohann Mar 25, 2021
3bc1e2f
Remove Offence::on_runtime_upgrade benchmark
Lohann Mar 25, 2021
d4de2df
Revert CHANGELOG.md update
Lohann Mar 25, 2021
f9ec009
Deprecate DeferredOffenceOf type
Lohann Mar 25, 2021
f19d8ee
Update copyright
Lohann Mar 30, 2021
dbc8010
Add migration logs
Lohann Mar 30, 2021
19c5ca2
Merge 'master' into 'lpcf/issue-8343-remove-offence-delay'
Lohann Mar 30, 2021
658192b
Fix migration log
Lohann Mar 31, 2021
67c0b4f
Remove unused import
Lohann Apr 5, 2021
4f78f43
Add migration tests
Lohann Apr 19, 2021
30297b9
rustfmt
Lohann Apr 19, 2021
ca099af
Merge branch 'master' into lpcf/issue-8343-remove-offence-delay
Lohann Apr 19, 2021
be4f69f
use generate_storage_alias! macro
Lohann Apr 27, 2021
9c49fe5
Refactor should_resubmit_deferred_offences test
Lohann Apr 27, 2021
4cce1f7
Replace spaces by tabs
Lohann Apr 27, 2021
000f80f
Refactor should_resubmit_deferred_offences test
Lohann Apr 27, 2021
66916f7
Merge branch 'master' into lpcf/issue-8343-remove-offence-delay
Lohann Apr 29, 2021
3b32a51
Removed WeightSoftLimit
Lohann Apr 29, 2021
d2c1865
Removed WeightSoftLimit from tests and mocks
Lohann Apr 29, 2021
a1eae7b
Remove unused imports
Lohann Apr 29, 2021
6047123
Merge branch 'master' into lpcf/issue-8343-remove-offence-delay
Lohann May 2, 2021
f53b798
Apply suggestions from code review
kianenigma May 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,16 +896,10 @@ impl pallet_im_online::Config for Runtime {
type WeightInfo = pallet_im_online::weights::SubstrateWeight<Runtime>;
}

parameter_types! {
pub OffencesWeightSoftLimit: Weight = Perbill::from_percent(60) *
RuntimeBlockWeights::get().max_block;
}

impl pallet_offences::Config for Runtime {
type Event = Event;
type IdentificationTuple = pallet_session::historical::IdentificationTuple<Self>;
type OnOffenceHandler = Staking;
type WeightSoftLimit = OffencesWeightSoftLimit;
}

impl pallet_authority_discovery::Config for Runtime {}
Expand Down
7 changes: 0 additions & 7 deletions frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use frame_system::InitKind;
use frame_support::{
parameter_types,
traits::{KeyOwnerProofSystem, OnInitialize},
weights::Weight,
};
use sp_io;
use sp_core::{H256, U256, crypto::{IsWrappedBy, KeyTypeId, Pair}};
Expand Down Expand Up @@ -214,16 +213,10 @@ impl pallet_staking::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub OffencesWeightSoftLimit: Weight = Perbill::from_percent(60)
* BlockWeights::get().max_block;
}

impl pallet_offences::Config for Test {
type Event = Event;
type IdentificationTuple = pallet_session::historical::IdentificationTuple<Self>;
type OnOffenceHandler = Staking;
type WeightSoftLimit = OffencesWeightSoftLimit;
}

parameter_types! {
Expand Down
6 changes: 0 additions & 6 deletions frame/grandpa/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use codec::Encode;
use frame_support::{
parameter_types,
traits::{KeyOwnerProofSystem, OnFinalize, OnInitialize},
weights::Weight,
};
use pallet_staking::EraIndex;
use sp_core::{crypto::KeyTypeId, H256};
Expand Down Expand Up @@ -220,15 +219,10 @@ impl pallet_staking::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub OffencesWeightSoftLimit: Weight = Perbill::from_percent(60) * BlockWeights::get().max_block;
}

impl pallet_offences::Config for Test {
type Event = Event;
type IdentificationTuple = pallet_session::historical::IdentificationTuple<Self>;
type OnOffenceHandler = Staking;
type WeightSoftLimit = OffencesWeightSoftLimit;
}

parameter_types! {
Expand Down
48 changes: 2 additions & 46 deletions frame/offences/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ use sp_std::vec;

use frame_system::{RawOrigin, Pallet as System, Config as SystemConfig};
use frame_benchmarking::{benchmarks, account, impl_benchmark_test_suite};
use frame_support::traits::{Currency, OnInitialize, ValidatorSet, ValidatorSetWithIdentification};
use frame_support::traits::{Currency, ValidatorSet, ValidatorSetWithIdentification};

use sp_runtime::{
Perbill,
traits::{Convert, StaticLookup, Saturating, UniqueSaturatedInto},
};
use sp_staking::offence::{ReportOffence, Offence, OffenceDetails};
use sp_staking::offence::{ReportOffence, Offence};

use pallet_balances::Config as BalancesConfig;
use pallet_babe::BabeEquivocationOffence;
Expand All @@ -51,7 +51,6 @@ const SEED: u32 = 0;
const MAX_REPORTERS: u32 = 100;
const MAX_OFFENDERS: u32 = 100;
const MAX_NOMINATORS: u32 = 100;
const MAX_DEFERRED_OFFENCES: u32 = 100;

pub struct Pallet<T: Config>(Offences<T>);

Expand Down Expand Up @@ -271,8 +270,6 @@ benchmarks! {
);
}
verify {
// make sure the report was not deferred
assert!(Offences::<T>::deferred_offences().is_empty());
let bond_amount: u32 = UniqueSaturatedInto::<u32>::unique_saturated_into(bond_amount::<T>());
let slash_amount = slash_fraction * bond_amount;
let reward_amount = slash_amount * (1 + n) / 2;
Expand Down Expand Up @@ -306,7 +303,6 @@ benchmarks! {
pallet_offences::Event::Offence(
UnresponsivenessOffence::<T>::ID,
0_u32.to_le_bytes().to_vec(),
true
)
).into()))
);
Expand Down Expand Up @@ -336,8 +332,6 @@ benchmarks! {
let _ = Offences::<T>::report_offence(reporters, offence);
}
verify {
// make sure the report was not deferred
assert!(Offences::<T>::deferred_offences().is_empty());
// make sure that all slashes have been applied
assert_eq!(
System::<T>::event_count(), 0
Expand Down Expand Up @@ -372,8 +366,6 @@ benchmarks! {
let _ = Offences::<T>::report_offence(reporters, offence);
}
verify {
// make sure the report was not deferred
assert!(Offences::<T>::deferred_offences().is_empty());
// make sure that all slashes have been applied
assert_eq!(
System::<T>::event_count(), 0
Expand All @@ -383,42 +375,6 @@ benchmarks! {
+ n // nominators slashed
);
}

on_initialize {
Lohann marked this conversation as resolved.
Show resolved Hide resolved
let d in 1 .. MAX_DEFERRED_OFFENCES;
let o = 10;
let n = 100;

let mut deferred_offences = vec![];
let offenders = make_offenders::<T>(o, n)?.0;
let offence_details = offenders.into_iter()
.map(|offender| OffenceDetails {
offender: T::convert(offender),
reporters: vec![],
})
.collect::<Vec<_>>();

for i in 0 .. d {
let fractions = offence_details.iter()
.map(|_| Perbill::from_percent(100 * (i + 1) / MAX_DEFERRED_OFFENCES))
.collect::<Vec<_>>();
deferred_offences.push((offence_details.clone(), fractions.clone(), 0u32));
}

Offences::<T>::set_deferred_offences(deferred_offences);
assert!(!Offences::<T>::deferred_offences().is_empty());
}: {
Offences::<T>::on_initialize(0u32.into());
}
verify {
// make sure that all deferred offences were reported with Ok status.
assert!(Offences::<T>::deferred_offences().is_empty());
assert_eq!(
System::<T>::event_count(), d * (0
+ o // offenders slashed
+ o * n // nominators slashed
));
}
}

impl_benchmark_test_suite!(
Expand Down
7 changes: 1 addition & 6 deletions frame/offences/benchmarking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use super::*;
use frame_support::{
parameter_types,
weights::{Weight, constants::WEIGHT_PER_SECOND},
weights::constants::WEIGHT_PER_SECOND,
};
use frame_system as system;
use sp_runtime::{
Expand Down Expand Up @@ -189,15 +189,10 @@ impl pallet_im_online::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub OffencesWeightSoftLimit: Weight = Perbill::from_percent(60) * BlockWeights::get().max_block;
}

impl pallet_offences::Config for Test {
type Event = Event;
type IdentificationTuple = pallet_session::historical::IdentificationTuple<Self>;
type OnOffenceHandler = Staking;
type WeightSoftLimit = OffencesWeightSoftLimit;
}

impl<T> frame_system::offchain::SendTransactionTypes<T> for Test where Call: From<T> {
Expand Down
96 changes: 10 additions & 86 deletions frame/offences/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@

mod mock;
mod tests;
mod migration;

use sp_std::vec::Vec;
use frame_support::{
decl_module, decl_event, decl_storage, Parameter, traits::Get, weights::Weight,
decl_module, decl_event, decl_storage, Parameter, weights::Weight,
};
use sp_runtime::{traits::{Hash, Zero}, Perbill};
use sp_runtime::{traits::Hash, Perbill};
use sp_staking::{
SessionIndex,
offence::{Offence, ReportOffence, Kind, OnOffenceHandler, OffenceDetails, OffenceError},
Expand All @@ -42,13 +43,6 @@ type OpaqueTimeSlot = Vec<u8>;
/// A type alias for a report identifier.
type ReportIdOf<T> = <T as frame_system::Config>::Hash;

/// Type of data stored as a deferred offence
pub type DeferredOffenceOf<T> = (
Vec<OffenceDetails<<T as frame_system::Config>::AccountId, <T as Config>::IdentificationTuple>>,
Vec<Perbill>,
SessionIndex,
);

pub trait WeightInfo {
fn report_offence_im_online(r: u32, o: u32, n: u32, ) -> Weight;
fn report_offence_grandpa(r: u32, n: u32, ) -> Weight;
Expand All @@ -71,10 +65,6 @@ pub trait Config: frame_system::Config {
type IdentificationTuple: Parameter + Ord;
/// A handler called for every offence report.
type OnOffenceHandler: OnOffenceHandler<Self::AccountId, Self::IdentificationTuple, Weight>;
/// The a soft limit on maximum weight that may be consumed while dispatching deferred offences in
/// `on_initialize`.
/// Note it's going to be exceeded before we stop adding to it, so it has to be set conservatively.
type WeightSoftLimit: Get<Weight>;
}

decl_storage! {
Expand All @@ -84,10 +74,6 @@ decl_storage! {
map hasher(twox_64_concat) ReportIdOf<T>
=> Option<OffenceDetails<T::AccountId, T::IdentificationTuple>>;

/// Deferred reports that have been rejected by the offence handler and need to be submitted
/// at a later time.
DeferredOffences get(fn deferred_offences): Vec<DeferredOffenceOf<T>>;

/// A vector of reports of the same kind that happened at the same time slot.
ConcurrentReportsIndex:
double_map hasher(twox_64_concat) Kind, hasher(twox_64_concat) OpaqueTimeSlot
Expand All @@ -106,53 +92,18 @@ decl_storage! {
decl_event!(
pub enum Event {
/// There is an offence reported of the given `kind` happened at the `session_index` and
/// (kind-specific) time slot. This event is not deposited for duplicate slashes. last
/// element indicates of the offence was applied (true) or queued (false)
/// \[kind, timeslot, applied\].
Offence(Kind, OpaqueTimeSlot, bool),
/// (kind-specific) time slot. This event is not deposited for duplicate slashes.
/// \[kind, timeslot\].
Offence(Kind, OpaqueTimeSlot),
}
);

decl_module! {
pub struct Module<T: Config> for enum Call where origin: T::Origin {
fn deposit_event() = default;

fn on_initialize(now: T::BlockNumber) -> Weight {
// only decode storage if we can actually submit anything again.
if !T::OnOffenceHandler::can_report() {
return 0;
}

let limit = T::WeightSoftLimit::get();
let mut consumed = Weight::zero();

<DeferredOffences<T>>::mutate(|deferred| {
deferred.retain(|(offences, perbill, session)| {
if consumed >= limit {
true
} else {
// keep those that fail to be reported again. An error log is emitted here; this
// should not happen if staking's `can_report` is implemented properly.
match T::OnOffenceHandler::on_offence(&offences, &perbill, *session) {
Ok(weight) => {
consumed += weight;
false
},
Err(_) => {
log::error!(
target: "runtime::offences",
"re-submitting a deferred slash returned Err at {:?}. \
This should not happen with pallet-staking",
now,
);
true
},
}
}
})
});

consumed
fn on_runtime_upgrade() -> Weight {
migration::remove_deferred_storage::<T>()
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -187,14 +138,14 @@ where
let slash_perbill: Vec<_> = (0..concurrent_offenders.len())
.map(|_| new_fraction.clone()).collect();

let applied = Self::report_or_store_offence(
T::OnOffenceHandler::on_offence(
&concurrent_offenders,
&slash_perbill,
offence.session_index(),
);

// Deposit the event.
Self::deposit_event(Event::Offence(O::ID, time_slot.encode(), applied));
Self::deposit_event(Event::Offence(O::ID, time_slot.encode()));

Ok(())
}
Expand All @@ -210,28 +161,6 @@ where
}

impl<T: Config> Module<T> {
/// Tries (without checking) to report an offence. Stores them in [`DeferredOffences`] in case
/// it fails. Returns false in case it has to store the offence.
fn report_or_store_offence(
concurrent_offenders: &[OffenceDetails<T::AccountId, T::IdentificationTuple>],
slash_perbill: &[Perbill],
session_index: SessionIndex,
) -> bool {
match T::OnOffenceHandler::on_offence(
&concurrent_offenders,
&slash_perbill,
session_index,
) {
Ok(_) => true,
Err(_) => {
<DeferredOffences<T>>::mutate(|d|
d.push((concurrent_offenders.to_vec(), slash_perbill.to_vec(), session_index))
);
false
}
}
}

/// Compute the ID for the given report properties.
///
/// The report id depends on the offence kind, time slot and the id of offender.
Expand Down Expand Up @@ -285,11 +214,6 @@ impl<T: Config> Module<T> {
None
}
}

#[cfg(feature = "runtime-benchmarks")]
pub fn set_deferred_offences(offences: Vec<DeferredOffenceOf<T>>) {
<DeferredOffences<T>>::put(offences);
}
}

struct TriageOutcome<T: Config> {
Expand Down
Loading