From 48cb7d2706f531c0bbf54683854a13cc7b2e1436 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sun, 19 Apr 2020 17:23:29 +0200 Subject: [PATCH 01/19] really rough mock runtime --- Cargo.lock | 5 + frame/offences/benchmarking/Cargo.toml | 8 + frame/offences/benchmarking/src/lib.rs | 40 +++-- frame/offences/benchmarking/src/mock.rs | 218 ++++++++++++++++++++++++ frame/session/benchmarking/src/mock.rs | 2 +- 5 files changed, 257 insertions(+), 16 deletions(-) create mode 100644 frame/offences/benchmarking/src/mock.rs diff --git a/Cargo.lock b/Cargo.lock index f7f85106414ac..d61a6373bb726 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4362,11 +4362,16 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "pallet-balances", "pallet-im-online", "pallet-offences", "pallet-session", "pallet-staking", + "pallet-staking-reward-curve", + "pallet-timestamp", "parity-scale-codec", + "serde", + "sp-core", "sp-io", "sp-runtime", "sp-staking", diff --git a/frame/offences/benchmarking/Cargo.toml b/frame/offences/benchmarking/Cargo.toml index de9d68dc80485..a94efb2b2a470 100644 --- a/frame/offences/benchmarking/Cargo.toml +++ b/frame/offences/benchmarking/Cargo.toml @@ -26,6 +26,14 @@ pallet-staking = { version = "2.0.0-dev", default-features = false, features = [ pallet-session = { version = "2.0.0-dev", default-features = false, path = "../../session" } sp-io = { path = "../../../primitives/io", default-features = false, version = "2.0.0-dev"} +[dev-dependencies] +serde = { version = "1.0.101" } +codec = { package = "parity-scale-codec", version = "1.3.0", features = ["derive"] } +sp-core = { version = "2.0.0-dev", path = "../../../primitives/core" } +pallet-staking-reward-curve = { version = "2.0.0-dev", path = "../../staking/reward-curve" } +sp-io ={ path = "../../../primitives/io", version = "2.0.0-dev"} +pallet-timestamp = { version = "2.0.0-dev", path = "../../timestamp" } +pallet-balances = { version = "2.0.0-dev", path = "../../balances" } [features] default = ["std"] diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index a88714a89a7fa..74efc663532b7 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -18,6 +18,8 @@ #![cfg_attr(not(feature = "std"), no_std)] +mod mock; + use sp_std::prelude::*; use sp_std::vec; @@ -39,7 +41,6 @@ use pallet_session::historical::{Trait as HistoricalTrait, IdentificationTuple}; const SEED: u32 = 0; -const MAX_USERS: u32 = 1000; const MAX_REPORTERS: u32 = 100; const MAX_OFFENDERS: u32 = 100; const MAX_NOMINATORS: u32 = 100; @@ -123,26 +124,20 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result (); - let r in 1 .. MAX_REPORTERS => (); - let o in 1 .. MAX_OFFENDERS => (); - let n in 1 .. MAX_NOMINATORS => (); - let d in 1 .. MAX_DEFERRED_OFFENCES => (); - } + _ { } report_offence { - let r in ...; - let o in ...; - let n in ...; + let r in 1 .. MAX_REPORTERS; + let o in 1 .. MAX_OFFENDERS; + let n in 1 .. MAX_NOMINATORS; + // Make r reporters let mut reporters = vec![]; - for i in 0 .. r { let reporter = account("reporter", i, SEED); reporters.push(reporter); } - + let offenders = make_offenders::(o, n).expect("failed to create offenders"); let keys = ImOnline::::keys(); @@ -157,7 +152,7 @@ benchmarks! { } on_initialize { - let d in ...; + let d in 1 .. MAX_DEFERRED_OFFENCES; Staking::::put_election_status(ElectionStatus::Closed); @@ -170,6 +165,21 @@ benchmarks! { Offences::::set_deferred_offences(deferred_offences); }: { - Offences::::on_initialize(u.into()); + Offences::::on_initialize(0.into()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{new_test_ext, Test}; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks() { + new_test_ext().execute_with(|| { + assert_ok!(test_benchmark_report_offence::()); + assert_ok!(test_benchmark_on_initialize::()); + }); } } diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs new file mode 100644 index 0000000000000..77af17214a4e6 --- /dev/null +++ b/frame/offences/benchmarking/src/mock.rs @@ -0,0 +1,218 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Mock file for session benchmarking. + +#![cfg(test)] + +use super::*; +use frame_support::{Hashable, assert_ok, assert_noop, parameter_types, weights::Weight}; +use frame_system::{self as system, EventRecord, Phase}; +use sp_core::H256; +use sp_runtime::{ + Perbill, traits::{BlakeTwo256, IdentityLookup, Block as BlockT}, testing::Header, + BuildStorage, +}; +use crate as collective; +use sp_runtime::testing::{UintAuthorityId, TestXt}; +use sp_runtime::SaturatedConversion; + + +type AccountId = u64; +type AccountIndex = u32; +type BlockNumber = u64; +type Balance = u64; + +impl frame_system::Trait for Test { + type Origin = Origin; + type Index = AccountIndex; + type BlockNumber = BlockNumber; + type Call = Call; + type Hash = sp_core::H256; + type Hashing = ::sp_runtime::traits::BlakeTwo256; + type AccountId = AccountId; + type Lookup = IdentityLookup; + type Header = sp_runtime::testing::Header; + type Event = (); + type BlockHashCount = (); + type MaximumBlockWeight = (); + type DbWeight = (); + type AvailableBlockRatio = (); + type MaximumBlockLength = (); + type Version = (); + type ModuleToIndex = (); + type AccountData = pallet_balances::AccountData; + type OnNewAccount = (); + type OnKilledAccount = (Balances,); +} +parameter_types! { + pub const ExistentialDeposit: Balance = 10; +} +impl pallet_balances::Trait for Test { + type Balance = Balance; + type Event = (); + type DustRemoval = (); + type ExistentialDeposit = ExistentialDeposit; + type AccountStore = System; +} + +parameter_types! { + pub const MinimumPeriod: u64 = 5; +} +impl pallet_timestamp::Trait for Test { + type Moment = u64; + type OnTimestampSet = (); + type MinimumPeriod = MinimumPeriod; +} +impl pallet_session::historical::Trait for Test { + type FullIdentification = pallet_staking::Exposure; + type FullIdentificationOf = pallet_staking::ExposureOf; +} + +sp_runtime::impl_opaque_keys! { + pub struct SessionKeys { + pub foo: sp_runtime::testing::UintAuthorityId, + } +} + +pub struct TestSessionHandler; +impl pallet_session::SessionHandler for TestSessionHandler { + const KEY_TYPE_IDS: &'static [sp_runtime::KeyTypeId] = &[]; + + fn on_genesis_session(_validators: &[(AccountId, Ks)]) {} + + fn on_new_session( + _: bool, + _: &[(AccountId, Ks)], + _: &[(AccountId, Ks)], + ) {} + + fn on_disabled(_: usize) {} +} + +parameter_types! { + pub const Period: u64 = 1; + pub const Offset: u64 = 0; +} + +impl pallet_session::Trait for Test { + type SessionManager = pallet_session::historical::NoteHistoricalRoot; + type Keys = SessionKeys; + type ShouldEndSession = pallet_session::PeriodicSessions; + type NextSessionRotation = pallet_session::PeriodicSessions; + type SessionHandler = TestSessionHandler; + type Event = (); + type ValidatorId = AccountId; + type ValidatorIdOf = pallet_staking::StashOf; + type DisabledValidatorsThreshold = (); +} +pallet_staking_reward_curve::build! { + const I_NPOS: sp_runtime::curve::PiecewiseLinear<'static> = curve!( + min_inflation: 0_025_000, + max_inflation: 0_100_000, + ideal_stake: 0_500_000, + falloff: 0_050_000, + max_piece_count: 40, + test_precision: 0_005_000, + ); +} +parameter_types! { + pub const RewardCurve: &'static sp_runtime::curve::PiecewiseLinear<'static> = &I_NPOS; + pub const MaxNominatorRewardedPerValidator: u32 = 64; + pub const UnsignedPriority: u64 = 1 << 20; +} + +pub type Extrinsic = sp_runtime::testing::TestXt; +type SubmitTransaction = frame_system::offchain::TransactionSubmitter< + sp_runtime::testing::UintAuthorityId, + Test, + Extrinsic, +>; + +pub struct CurrencyToVoteHandler; +impl Convert for CurrencyToVoteHandler { + fn convert(x: u64) -> u64 { + x + } +} +impl Convert for CurrencyToVoteHandler { + fn convert(x: u128) -> u64 { + x.saturated_into() + } +} + +impl pallet_staking::Trait for Test { + type Currency = Balances; + type UnixTime = pallet_timestamp::Module; + type CurrencyToVote = CurrencyToVoteHandler; + type RewardRemainder = (); + type Event = (); + type Slash = (); + type Reward = (); + type SessionsPerEra = (); + type SlashDeferDuration = (); + type SlashCancelOrigin = frame_system::EnsureRoot; + type BondingDuration = (); + type SessionInterface = Self; + type RewardCurve = RewardCurve; + type NextNewSession = Session; + type ElectionLookahead = (); + type Call = Call; + type SubmitTransaction = SubmitTransaction; + type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; + type UnsignedPriority = UnsignedPriority; +} + +impl pallet_im_online::Trait for Test { + type AuthorityId = UintAuthorityId; + type Event = (); + type Call = Call; + type SubmitTransaction = SubmitTransaction; + type SessionDuration = Period; + type ReportUnresponsiveness = Offences; + type UnsignedPriority = UnsignedPriority; +} + +impl pallet_offences::Trait for Test { + type Event = (); + type IdentificationTuple = pallet_session::historical::IdentificationTuple; + type OnOffenceHandler = Staking; +} + +impl crate::Trait for Test {} + +pub type Block = sp_runtime::generic::Block; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + +frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Module, Call, Event}, + Balances: pallet_balances::{Module, Call, Storage, Config, Event}, + Staking: pallet_staking::{Module, Call, Config, Storage, Event, ValidateUnsigned}, + Session: pallet_session::{Module, Call, Storage, Event, Config}, + ImOnline: pallet_im_online::{Module, Call, Storage, Event, ValidateUnsigned, Config}, + Offences: pallet_offences::{Module, Call, Storage, Event}, + } +); + +pub fn new_test_ext() -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + sp_io::TestExternalities::new(t) +} diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 4c022eb8b89b6..4344cd12647b6 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Mock file for staking fuzzing. +//! Mock file for session benchmarks. #![cfg(test)] From bd532c9d158524b287b0a354e8a50166d85c521e Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sun, 19 Apr 2020 20:47:00 +0200 Subject: [PATCH 02/19] start to work on offences --- frame/offences/benchmarking/src/lib.rs | 8 ++++++-- frame/offences/benchmarking/src/mock.rs | 15 ++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 74efc663532b7..f79ded07ebc6c 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -34,7 +34,7 @@ use pallet_im_online::{Trait as ImOnlineTrait, Module as ImOnline, Unresponsiven use pallet_offences::{Trait as OffencesTrait, Module as Offences}; use pallet_staking::{ Module as Staking, Trait as StakingTrait, RewardDestination, ValidatorPrefs, - Exposure, IndividualExposure, ElectionStatus + Exposure, IndividualExposure, ElectionStatus, MAX_NOMINATIONS, }; use pallet_session::Trait as SessionTrait; use pallet_session::historical::{Trait as HistoricalTrait, IdentificationTuple}; @@ -152,8 +152,12 @@ benchmarks! { } on_initialize { + let n in 1 .. MAX_NOMINATIONS as u32; let d in 1 .. MAX_DEFERRED_OFFENCES; + + pallet_staking::benchmarking::create_validator_with_nominators::(n, MAX_NOMINATIONS as u32)?; + Staking::::put_election_status(ElectionStatus::Closed); let mut deferred_offences = vec![]; @@ -178,7 +182,7 @@ mod tests { #[test] fn test_benchmarks() { new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_report_offence::()); + //assert_ok!(test_benchmark_report_offence::()); assert_ok!(test_benchmark_on_initialize::()); }); } diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 77af17214a4e6..06a9e38745485 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -14,21 +14,18 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Mock file for session benchmarking. +//! Mock file for offences benchmarking. #![cfg(test)] use super::*; -use frame_support::{Hashable, assert_ok, assert_noop, parameter_types, weights::Weight}; -use frame_system::{self as system, EventRecord, Phase}; -use sp_core::H256; +use frame_support::parameter_types; +use frame_system as system; use sp_runtime::{ - Perbill, traits::{BlakeTwo256, IdentityLookup, Block as BlockT}, testing::Header, - BuildStorage, + SaturatedConversion, + traits::{IdentityLookup, Block as BlockT}, + testing::{Header, UintAuthorityId}, }; -use crate as collective; -use sp_runtime::testing::{UintAuthorityId, TestXt}; -use sp_runtime::SaturatedConversion; type AccountId = u64; From 18e9a4fba3bd1f557e025633d288268e716c62b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 24 Apr 2020 15:20:54 +0200 Subject: [PATCH 03/19] Make sure to start the session. --- frame/offences/benchmarking/src/lib.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index f79ded07ebc6c..6e3bb42b18c58 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -36,7 +36,7 @@ use pallet_staking::{ Module as Staking, Trait as StakingTrait, RewardDestination, ValidatorPrefs, Exposure, IndividualExposure, ElectionStatus, MAX_NOMINATIONS, }; -use pallet_session::Trait as SessionTrait; +use pallet_session::{Trait as SessionTrait, SessionManager}; use pallet_session::historical::{Trait as HistoricalTrait, IdentificationTuple}; const SEED: u32 = 0; @@ -105,13 +105,16 @@ fn create_offender(n: u32, nominators: u32) -> Result(num_offenders: u32, num_nominators: u32) -> Result>, &'static str> { - let mut offenders: Vec = vec![]; + Staking::::new_session(0); + let mut offenders: Vec = vec![]; for i in 0 .. num_offenders { let offender = create_offender::(i, num_nominators)?; offenders.push(offender); } + Staking::::start_session(0); + Ok(offenders.iter() .map(|id| ::ValidatorIdOf::convert(id.clone()) @@ -182,7 +185,7 @@ mod tests { #[test] fn test_benchmarks() { new_test_ext().execute_with(|| { - //assert_ok!(test_benchmark_report_offence::()); + assert_ok!(test_benchmark_report_offence::()); assert_ok!(test_benchmark_on_initialize::()); }); } From 181bd7d0976b5b86b4de4dbfddbf69a30d69fc29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 29 Apr 2020 12:24:25 +0200 Subject: [PATCH 04/19] Update to latest master. --- frame/offences/benchmarking/src/mock.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 06a9e38745485..4bb8053c734e2 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -19,7 +19,7 @@ #![cfg(test)] use super::*; -use frame_support::parameter_types; +use frame_support::{parameter_types, weights::Weight}; use frame_system as system; use sp_runtime::{ SaturatedConversion, @@ -33,6 +33,10 @@ type AccountIndex = u32; type BlockNumber = u64; type Balance = u64; +parameter_types! { + pub const ExtrinsicBaseWeight: Weight = 10_000_000; +} + impl frame_system::Trait for Test { type Origin = Origin; type Index = AccountIndex; @@ -54,6 +58,8 @@ impl frame_system::Trait for Test { type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (Balances,); + type BlockExecutionWeight = (); + type ExtrinsicBaseWeight = ExtrinsicBaseWeight; } parameter_types! { pub const ExistentialDeposit: Balance = 10; @@ -130,14 +136,10 @@ parameter_types! { pub const RewardCurve: &'static sp_runtime::curve::PiecewiseLinear<'static> = &I_NPOS; pub const MaxNominatorRewardedPerValidator: u32 = 64; pub const UnsignedPriority: u64 = 1 << 20; + pub const MaxIterations: u32 = 5; } pub type Extrinsic = sp_runtime::testing::TestXt; -type SubmitTransaction = frame_system::offchain::TransactionSubmitter< - sp_runtime::testing::UintAuthorityId, - Test, - Extrinsic, ->; pub struct CurrencyToVoteHandler; impl Convert for CurrencyToVoteHandler { @@ -168,16 +170,14 @@ impl pallet_staking::Trait for Test { type NextNewSession = Session; type ElectionLookahead = (); type Call = Call; - type SubmitTransaction = SubmitTransaction; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type UnsignedPriority = UnsignedPriority; + type MaxIterations = MaxIterations; } impl pallet_im_online::Trait for Test { type AuthorityId = UintAuthorityId; type Event = (); - type Call = Call; - type SubmitTransaction = SubmitTransaction; type SessionDuration = Period; type ReportUnresponsiveness = Offences; type UnsignedPriority = UnsignedPriority; @@ -189,10 +189,15 @@ impl pallet_offences::Trait for Test { type OnOffenceHandler = Staking; } +impl frame_system::offchain::SendTransactionTypes for Test where Call: From { + type Extrinsic = Extrinsic; + type OverarchingCall = Call; +} + impl crate::Trait for Test {} pub type Block = sp_runtime::generic::Block; -pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; frame_support::construct_runtime!( pub enum Test where From f29eaf33047ebdcd087b89721a985ce7e871d7d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 29 Apr 2020 18:10:32 +0200 Subject: [PATCH 05/19] Add verify. --- frame/offences/benchmarking/Cargo.toml | 12 +++--- frame/offences/benchmarking/src/lib.rs | 58 +++++++++++++++++++++----- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/frame/offences/benchmarking/Cargo.toml b/frame/offences/benchmarking/Cargo.toml index a94efb2b2a470..6120d2f8f08ef 100644 --- a/frame/offences/benchmarking/Cargo.toml +++ b/frame/offences/benchmarking/Cargo.toml @@ -14,17 +14,18 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "1.3.0", default-features = false } -sp-std = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/std" } -sp-staking = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/staking" } -sp-runtime = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/runtime" } frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../../benchmarking" } -frame-system = { version = "2.0.0-dev", default-features = false, path = "../../system" } frame-support = { version = "2.0.0-dev", default-features = false, path = "../../support" } +frame-system = { version = "2.0.0-dev", default-features = false, path = "../../system" } +pallet-balances = { version = "2.0.0-dev", path = "../../balances" } pallet-im-online = { version = "2.0.0-dev", default-features = false, path = "../../im-online" } pallet-offences = { version = "2.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../offences" } -pallet-staking = { version = "2.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../staking" } pallet-session = { version = "2.0.0-dev", default-features = false, path = "../../session" } +pallet-staking = { version = "2.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../staking" } sp-io = { path = "../../../primitives/io", default-features = false, version = "2.0.0-dev"} +sp-runtime = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/runtime" } +sp-staking = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/staking" } +sp-std = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/std" } [dev-dependencies] serde = { version = "1.0.101" } @@ -33,7 +34,6 @@ sp-core = { version = "2.0.0-dev", path = "../../../primitives/core" } pallet-staking-reward-curve = { version = "2.0.0-dev", path = "../../staking/reward-curve" } sp-io ={ path = "../../../primitives/io", version = "2.0.0-dev"} pallet-timestamp = { version = "2.0.0-dev", path = "../../timestamp" } -pallet-balances = { version = "2.0.0-dev", path = "../../balances" } [features] default = ["std"] diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 6e3bb42b18c58..87718b574c591 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -23,13 +23,14 @@ mod mock; use sp_std::prelude::*; use sp_std::vec; -use frame_system::RawOrigin; +use frame_system::{RawOrigin, Module as System}; use frame_benchmarking::{benchmarks, account}; use frame_support::traits::{Currency, OnInitialize}; use sp_runtime::{Perbill, traits::{Convert, StaticLookup}}; use sp_staking::offence::ReportOffence; +use pallet_balances::{Trait as BalancesTrait, Module as Balances}; use pallet_im_online::{Trait as ImOnlineTrait, Module as ImOnline, UnresponsivenessOffence}; use pallet_offences::{Trait as OffencesTrait, Module as Offences}; use pallet_staking::{ @@ -48,15 +49,18 @@ const MAX_DEFERRED_OFFENCES: u32 = 100; pub struct Module(Offences); -pub trait Trait: SessionTrait + StakingTrait + OffencesTrait + ImOnlineTrait + HistoricalTrait {} +pub trait Trait: + SessionTrait + StakingTrait + OffencesTrait + ImOnlineTrait + HistoricalTrait + BalancesTrait {} fn create_offender(n: u32, nominators: u32) -> Result { let stash: T::AccountId = account("stash", n, SEED); + let stash_lookup: ::Source = T::Lookup::unlookup(stash.clone()); let controller: T::AccountId = account("controller", n, SEED); let controller_lookup: ::Source = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; - let amount = T::Currency::minimum_balance(); - + let raw_amount = 1_000_000; + Balances::::set_balance(RawOrigin::Root.into(), stash_lookup, raw_amount.into(), raw_amount.into())?; + let amount: >::Balance = raw_amount.into(); Staking::::bond( RawOrigin::Signed(stash.clone()).into(), controller_lookup.clone(), @@ -74,13 +78,19 @@ fn create_offender(n: u32, nominators: u32) -> Result::Source = + T::Lookup::unlookup(nominator_stash.clone()); let nominator_controller: T::AccountId = account("nominator controller", n * MAX_NOMINATORS + i, SEED); - let nominator_controller_lookup: ::Source = T::Lookup::unlookup(nominator_controller.clone()); + let nominator_controller_lookup: ::Source = + T::Lookup::unlookup(nominator_controller.clone()); + Balances::::set_balance( + RawOrigin::Root.into(), nominator_stash_lookup, raw_amount.into(), raw_amount.into() + )?; Staking::::bond( RawOrigin::Signed(nominator_stash.clone()).into(), nominator_controller_lookup.clone(), - amount, + amount.clone(), reward_destination, )?; @@ -88,7 +98,7 @@ fn create_offender(n: u32, nominators: u32) -> Result::nominate(RawOrigin::Signed(nominator_controller.clone()).into(), selected_validators)?; individual_exposures.push(IndividualExposure { - who: nominator_controller.clone(), + who: nominator_stash.clone(), value: amount.clone(), }); } @@ -109,7 +119,7 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result = vec![]; for i in 0 .. num_offenders { - let offender = create_offender::(i, num_nominators)?; + let offender = create_offender::(i + 1, num_nominators)?; offenders.push(offender); } @@ -126,13 +136,16 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result>>()) } +// TODO Add other offences: BabeEquivocation and GrandpaEquivocation + benchmarks! { _ { } report_offence { let r in 1 .. MAX_REPORTERS; - let o in 1 .. MAX_OFFENDERS; - let n in 1 .. MAX_NOMINATORS; + // we skip 1 offender, because in such case there is no slashing + let o in 2 .. MAX_OFFENDERS; + let n in 0 .. MAX_NOMINATORS; // Make r reporters let mut reporters = vec![]; @@ -141,6 +154,9 @@ benchmarks! { reporters.push(reporter); } + // make sure reporters actually get rewarded + Staking::::set_slash_reward_fraction(Perbill::one()); + let offenders = make_offenders::(o, n).expect("failed to create offenders"); let keys = ImOnline::::keys(); @@ -149,10 +165,22 @@ benchmarks! { validator_set_count: keys.len() as u32, offenders, }; - + assert_eq!(System::::event_count(), 0); }: { let _ = ::ReportUnresponsiveness::report_offence(reporters, offence); } + verify { + // make sure the report was not deferred + assert!(Offences::::deferred_offences().is_empty()); + // make sure that all slashes have been applied + assert_eq!( + System::::event_count(), 0 + + 1 // offence + + 2 * r // reporter (reward + endowment) + + o // offenders slashed + + o * n // nominators slashed + ); + } on_initialize { let n in 1 .. MAX_NOMINATIONS as u32; @@ -165,6 +193,10 @@ benchmarks! { let mut deferred_offences = vec![]; + + // TODO [ToDr] add a bunch of concurrent_offenders to deferred_offences + // Most likely creaate offenders and convert them to OffenceDetails (reporters are not + // relevant) for i in 0 .. d { deferred_offences.push((vec![], vec![], 0u32)); } @@ -174,6 +206,10 @@ benchmarks! { }: { Offences::::on_initialize(0.into()); } + verify { + // make sure that all deferred offences were reported with Ok status. + assert!(Offences::::deferred_offences().is_empty()); + } } #[cfg(test)] From c2ceaf4c04a6e1fe9bf60f68e9c6a2e3941943d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 30 Apr 2020 13:17:39 +0200 Subject: [PATCH 06/19] Fix on_initialize benchmark. --- frame/balances/src/lib.rs | 2 +- frame/offences/benchmarking/src/lib.rs | 52 ++++++++++++++++++++------ frame/staking/src/lib.rs | 6 +++ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 94dbd3730f163..41edf3ca9b87d 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -458,7 +458,7 @@ decl_module! { /// - Contains a limited number of reads and writes. /// # #[weight = T::DbWeight::get().reads_writes(1, 1) + 100_000_000] - fn set_balance( + pub fn set_balance( origin, who: ::Source, #[compact] new_free: T::Balance, diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 87718b574c591..ae62156a6bde3 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -50,7 +50,20 @@ const MAX_DEFERRED_OFFENCES: u32 = 100; pub struct Module(Offences); pub trait Trait: - SessionTrait + StakingTrait + OffencesTrait + ImOnlineTrait + HistoricalTrait + BalancesTrait {} + SessionTrait + + StakingTrait + + OffencesTrait + + ImOnlineTrait + + HistoricalTrait + + BalancesTrait + + IdTupleConvert +{} + +/// A helper trait to make sure we can convert `IdentificationTuple` coming from historical +/// and the one required by offences. +pub trait IdTupleConvert { + fn convert(id: IdentificationTuple) -> ::IdentificationTuple; +} fn create_offender(n: u32, nominators: u32) -> Result { let stash: T::AccountId = account("stash", n, SEED); @@ -145,7 +158,7 @@ benchmarks! { let r in 1 .. MAX_REPORTERS; // we skip 1 offender, because in such case there is no slashing let o in 2 .. MAX_OFFENDERS; - let n in 0 .. MAX_NOMINATORS; + let n in 0 .. MAX_NOMINATORS.min(MAX_NOMINATIONS as u32); // Make r reporters let mut reporters = vec![]; @@ -183,32 +196,41 @@ benchmarks! { } on_initialize { - let n in 1 .. MAX_NOMINATIONS as u32; let d in 1 .. MAX_DEFERRED_OFFENCES; - - - pallet_staking::benchmarking::create_validator_with_nominators::(n, MAX_NOMINATIONS as u32)?; + let o = 10; + let n = 100; Staking::::put_election_status(ElectionStatus::Closed); let mut deferred_offences = vec![]; + let offenders = make_offenders::(o, n).expect("failed to create offenders"); + let offence_details = offenders.into_iter() + .map(|offender| sp_staking::offence::OffenceDetails { + offender: T::convert(offender), + reporters: vec![], + }) + .collect::>(); - - // TODO [ToDr] add a bunch of concurrent_offenders to deferred_offences - // Most likely creaate offenders and convert them to OffenceDetails (reporters are not - // relevant) for i in 0 .. d { - deferred_offences.push((vec![], vec![], 0u32)); + let fractions = offence_details.iter() + .map(|_| Perbill::from_percent(100 * (i + 1) / MAX_DEFERRED_OFFENCES)) + .collect::>(); + deferred_offences.push((offence_details.clone(), fractions.clone(), 0u32)); } Offences::::set_deferred_offences(deferred_offences); - + assert!(!Offences::::deferred_offences().is_empty()); }: { Offences::::on_initialize(0.into()); } verify { // make sure that all deferred offences were reported with Ok status. assert!(Offences::::deferred_offences().is_empty()); + assert_eq!( + System::::event_count(), d * (0 + + o // offenders slashed + + o * n // nominators slashed + )); } } @@ -218,6 +240,12 @@ mod tests { use crate::mock::{new_test_ext, Test}; use frame_support::assert_ok; + impl IdTupleConvert for Test { + fn convert(id: IdentificationTuple) -> ::IdentificationTuple { + id + } + } + #[test] fn test_benchmarks() { new_test_ext().execute_with(|| { diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 67240d8d34b2a..15ca2b7a5ca4e 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -2941,6 +2941,12 @@ impl Module { pub fn put_election_status(status: ElectionStatus::) { >::put(status); } + + #[cfg(feature = "runtime-benchmarks")] + pub fn set_slash_reward_fraction(fraction: Perbill) { + SlashRewardFraction::put(fraction); + } + } /// In this implementation `new_session(session)` must be called before `end_session(session-1)` From 0ca03e8b007d6bfbc291a2eb126ad731a0abf619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 30 Apr 2020 15:12:27 +0200 Subject: [PATCH 07/19] Add grandpa offence. --- Cargo.lock | 1 + frame/grandpa/src/lib.rs | 19 +++++----- frame/offences/benchmarking/Cargo.toml | 19 +++++----- frame/offences/benchmarking/src/lib.rs | 51 ++++++++++++++++++++++++-- primitives/staking/src/offence.rs | 2 - 5 files changed, 68 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b6aeabfd6d149..372617376d10a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4386,6 +4386,7 @@ dependencies = [ "frame-support", "frame-system", "pallet-balances", + "pallet-grandpa", "pallet-im-online", "pallet-offences", "pallet-session", diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index aa5db8849fe3b..5827d2f195b3c 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -456,23 +456,24 @@ impl pallet_finality_tracker::OnFinalizationStalled fo /// A round number and set id which point on the time of an offence. #[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Encode, Decode)] -struct GrandpaTimeSlot { +pub struct GrandpaTimeSlot { // The order of these matters for `derive(Ord)`. - set_id: SetId, - round: RoundNumber, + /// Grandpa Set ID. + pub set_id: SetId, + /// Round number. + pub round: RoundNumber, } -// TODO [slashing]: Integrate this. /// A grandpa equivocation offence report. -struct GrandpaEquivocationOffence { +pub struct GrandpaEquivocationOffence { /// Time slot at which this incident happened. - time_slot: GrandpaTimeSlot, + pub time_slot: GrandpaTimeSlot, /// The session index in which the incident happened. - session_index: SessionIndex, + pub session_index: SessionIndex, /// The size of the validator set at the time of the offence. - validator_set_count: u32, + pub validator_set_count: u32, /// The authority which produced this equivocation. - offender: FullIdentification, + pub offender: FullIdentification, } impl Offence for GrandpaEquivocationOffence { diff --git a/frame/offences/benchmarking/Cargo.toml b/frame/offences/benchmarking/Cargo.toml index 6120d2f8f08ef..d45d8c847686b 100644 --- a/frame/offences/benchmarking/Cargo.toml +++ b/frame/offences/benchmarking/Cargo.toml @@ -13,11 +13,11 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "1.3.0", default-features = false } - frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../../benchmarking" } frame-support = { version = "2.0.0-dev", default-features = false, path = "../../support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../../system" } pallet-balances = { version = "2.0.0-dev", path = "../../balances" } +pallet-grandpa = { version = "2.0.0-dev", default-features = false, path = "../../grandpa" } pallet-im-online = { version = "2.0.0-dev", default-features = false, path = "../../im-online" } pallet-offences = { version = "2.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../offences" } pallet-session = { version = "2.0.0-dev", default-features = false, path = "../../session" } @@ -28,24 +28,25 @@ sp-staking = { version = "2.0.0-dev", default-features = false, path = "../../.. sp-std = { version = "2.0.0-dev", default-features = false, path = "../../../primitives/std" } [dev-dependencies] -serde = { version = "1.0.101" } codec = { package = "parity-scale-codec", version = "1.3.0", features = ["derive"] } -sp-core = { version = "2.0.0-dev", path = "../../../primitives/core" } pallet-staking-reward-curve = { version = "2.0.0-dev", path = "../../staking/reward-curve" } -sp-io ={ path = "../../../primitives/io", version = "2.0.0-dev"} pallet-timestamp = { version = "2.0.0-dev", path = "../../timestamp" } +serde = { version = "1.0.101" } +sp-core = { version = "2.0.0-dev", path = "../../../primitives/core" } +sp-io ={ path = "../../../primitives/io", version = "2.0.0-dev"} [features] default = ["std"] std = [ - "sp-runtime/std", - "sp-std/std", - "sp-staking/std", "frame-benchmarking/std", "frame-support/std", "frame-system/std", - "pallet-offences/std", + "pallet-grandpa/std", "pallet-im-online/std", - "pallet-staking/std", + "pallet-offences/std", "pallet-session/std", + "pallet-staking/std", + "sp-runtime/std", + "sp-staking/std", + "sp-std/std", ] diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index ae62156a6bde3..842d82d8068a3 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -31,14 +31,15 @@ use sp_runtime::{Perbill, traits::{Convert, StaticLookup}}; use sp_staking::offence::ReportOffence; use pallet_balances::{Trait as BalancesTrait, Module as Balances}; +use pallet_grandpa::{GrandpaEquivocationOffence, GrandpaTimeSlot}; use pallet_im_online::{Trait as ImOnlineTrait, Module as ImOnline, UnresponsivenessOffence}; use pallet_offences::{Trait as OffencesTrait, Module as Offences}; +use pallet_session::historical::{Trait as HistoricalTrait, IdentificationTuple}; +use pallet_session::{Trait as SessionTrait, SessionManager}; use pallet_staking::{ Module as Staking, Trait as StakingTrait, RewardDestination, ValidatorPrefs, Exposure, IndividualExposure, ElectionStatus, MAX_NOMINATIONS, }; -use pallet_session::{Trait as SessionTrait, SessionManager}; -use pallet_session::historical::{Trait as HistoricalTrait, IdentificationTuple}; const SEED: u32 = 0; @@ -154,7 +155,7 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result::set_slash_reward_fraction(Perbill::one()); + + let mut offenders = make_offenders::(o, n).expect("failed to create offenders"); + let keys = ImOnline::::keys(); + + let offence = GrandpaEquivocationOffence { + time_slot: GrandpaTimeSlot { set_id: 0, round: 0 }, + session_index: 0, + validator_set_count: keys.len() as u32, + offender: T::convert(offenders.pop().unwrap()), + }; + assert_eq!(System::::event_count(), 0); + }: { + let _ = Offences::::report_offence(reporters, offence); + } + verify { + // make sure the report was not deferred + assert!(Offences::::deferred_offences().is_empty()); + // make sure that all slashes have been applied + assert_eq!( + System::::event_count(), 0 + + 1 // offence + + 2 * r // reporter (reward + endowment) + + o // offenders slashed + + o * n // nominators slashed + ); + } + on_initialize { let d in 1 .. MAX_DEFERRED_OFFENCES; let o = 10; @@ -249,7 +291,8 @@ mod tests { #[test] fn test_benchmarks() { new_test_ext().execute_with(|| { - assert_ok!(test_benchmark_report_offence::()); + assert_ok!(test_benchmark_report_offence_im_online::()); + assert_ok!(test_benchmark_report_offence_grandpa::()); assert_ok!(test_benchmark_on_initialize::()); }); } diff --git a/primitives/staking/src/offence.rs b/primitives/staking/src/offence.rs index 584f3a75ea3ab..5becfeab75c4e 100644 --- a/primitives/staking/src/offence.rs +++ b/primitives/staking/src/offence.rs @@ -26,8 +26,6 @@ use crate::SessionIndex; /// The kind of an offence, is a byte string representing some kind identifier /// e.g. `b"im-online:offlin"`, `b"babe:equivocatio"` -// TODO [slashing]: Is there something better we can have here that is more natural but still -// flexible? as you see in examples, they get cut off with long names. pub type Kind = [u8; 16]; /// Number of times the offence of this authority was already reported in the past. From f525478170c7d9890f8f3d5eecb92ec9635e585f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 30 Apr 2020 15:21:54 +0200 Subject: [PATCH 08/19] Add Babe offence benchmarking. --- Cargo.lock | 1 + frame/babe/src/lib.rs | 13 ++++---- frame/offences/benchmarking/Cargo.toml | 2 ++ frame/offences/benchmarking/src/lib.rs | 44 ++++++++++++++++++++++++-- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 372617376d10a..9bba41e61f0a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4385,6 +4385,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "pallet-babe", "pallet-balances", "pallet-grandpa", "pallet-im-online", diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index 7357ef75ffaf9..55a6b96dc81fd 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -18,7 +18,7 @@ //! from VRF outputs and manages epoch transitions. #![cfg_attr(not(feature = "std"), no_std)] -#![forbid(unused_must_use, unsafe_code, unused_variables, unused_must_use)] +#![warn(unused_must_use, unsafe_code, unused_variables, unused_must_use)] use pallet_timestamp; @@ -267,19 +267,18 @@ impl pallet_session::ShouldEndSession for Module { } } -// TODO [slashing]: @marcio use this, remove the dead_code annotation. /// A BABE equivocation offence report. /// /// When a validator released two or more blocks at the same slot. -struct BabeEquivocationOffence { +pub struct BabeEquivocationOffence { /// A babe slot number in which this incident happened. - slot: u64, + pub slot: u64, /// The session index in which the incident happened. - session_index: SessionIndex, + pub session_index: SessionIndex, /// The size of the validator set at the time of the offence. - validator_set_count: u32, + pub validator_set_count: u32, /// The authority that produced the equivocation. - offender: FullIdentification, + pub offender: FullIdentification, } impl Offence for BabeEquivocationOffence { diff --git a/frame/offences/benchmarking/Cargo.toml b/frame/offences/benchmarking/Cargo.toml index d45d8c847686b..1529d978d3b1a 100644 --- a/frame/offences/benchmarking/Cargo.toml +++ b/frame/offences/benchmarking/Cargo.toml @@ -16,6 +16,7 @@ codec = { package = "parity-scale-codec", version = "1.3.0", default-features = frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../../benchmarking" } frame-support = { version = "2.0.0-dev", default-features = false, path = "../../support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../../system" } +pallet-babe = { version = "2.0.0-dev", default-features = false, path = "../../babe" } pallet-balances = { version = "2.0.0-dev", path = "../../balances" } pallet-grandpa = { version = "2.0.0-dev", default-features = false, path = "../../grandpa" } pallet-im-online = { version = "2.0.0-dev", default-features = false, path = "../../im-online" } @@ -41,6 +42,7 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", + "pallet-babe/std", "pallet-grandpa/std", "pallet-im-online/std", "pallet-offences/std", diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 842d82d8068a3..9580a2f01862c 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -31,6 +31,7 @@ use sp_runtime::{Perbill, traits::{Convert, StaticLookup}}; use sp_staking::offence::ReportOffence; use pallet_balances::{Trait as BalancesTrait, Module as Balances}; +use pallet_babe::BabeEquivocationOffence; use pallet_grandpa::{GrandpaEquivocationOffence, GrandpaTimeSlot}; use pallet_im_online::{Trait as ImOnlineTrait, Module as ImOnline, UnresponsivenessOffence}; use pallet_offences::{Trait as OffencesTrait, Module as Offences}; @@ -150,8 +151,6 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result>>()) } -// TODO Add other offences: BabeEquivocation and GrandpaEquivocation - benchmarks! { _ { } @@ -237,6 +236,47 @@ benchmarks! { ); } + report_offence_babe { + let r in 1 .. MAX_REPORTERS; + let n in 0 .. MAX_NOMINATORS.min(MAX_NOMINATIONS as u32); + let o = 1; + + // Make r reporters + let mut reporters = vec![]; + for i in 0 .. r { + let reporter = account("reporter", i, SEED); + reporters.push(reporter); + } + + // make sure reporters actually get rewarded + Staking::::set_slash_reward_fraction(Perbill::one()); + + let mut offenders = make_offenders::(o, n).expect("failed to create offenders"); + let keys = ImOnline::::keys(); + + let offence = BabeEquivocationOffence { + slot: 0, + session_index: 0, + validator_set_count: keys.len() as u32, + offender: T::convert(offenders.pop().unwrap()), + }; + assert_eq!(System::::event_count(), 0); + }: { + let _ = Offences::::report_offence(reporters, offence); + } + verify { + // make sure the report was not deferred + assert!(Offences::::deferred_offences().is_empty()); + // make sure that all slashes have been applied + assert_eq!( + System::::event_count(), 0 + + 1 // offence + + 2 * r // reporter (reward + endowment) + + o // offenders slashed + + o * n // nominators slashed + ); + } + on_initialize { let d in 1 .. MAX_DEFERRED_OFFENCES; let o = 10; From 7dec75d462e9cf37fc47f09e89ebf02e9beef38d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 30 Apr 2020 16:19:31 +0200 Subject: [PATCH 09/19] Enable babe test. --- frame/offences/benchmarking/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 9580a2f01862c..68b92e06988df 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -333,6 +333,7 @@ mod tests { new_test_ext().execute_with(|| { assert_ok!(test_benchmark_report_offence_im_online::()); assert_ok!(test_benchmark_report_offence_grandpa::()); + assert_ok!(test_benchmark_report_offence_babe::()); assert_ok!(test_benchmark_on_initialize::()); }); } From d778c1c3973c36543a444f53b6ec0033535ef838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 30 Apr 2020 16:35:51 +0200 Subject: [PATCH 10/19] Address review grumbles. --- frame/offences/benchmarking/src/lib.rs | 21 +++++++++++---------- frame/offences/benchmarking/src/mock.rs | 16 +++++----------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 68b92e06988df..8ea9c36985e10 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -23,7 +23,7 @@ mod mock; use sp_std::prelude::*; use sp_std::vec; -use frame_system::{RawOrigin, Module as System}; +use frame_system::{RawOrigin, Module as System, Trait as SystemTrait}; use frame_benchmarking::{benchmarks, account}; use frame_support::traits::{Currency, OnInitialize}; @@ -67,15 +67,18 @@ pub trait IdTupleConvert { fn convert(id: IdentificationTuple) -> ::IdentificationTuple; } +type LookupSourceOf = <::Lookup as StaticLookup>::Source; +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; + fn create_offender(n: u32, nominators: u32) -> Result { let stash: T::AccountId = account("stash", n, SEED); - let stash_lookup: ::Source = T::Lookup::unlookup(stash.clone()); + let stash_lookup: LookupSourceOf = T::Lookup::unlookup(stash.clone()); let controller: T::AccountId = account("controller", n, SEED); - let controller_lookup: ::Source = T::Lookup::unlookup(controller.clone()); + let controller_lookup: LookupSourceOf = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; let raw_amount = 1_000_000; Balances::::set_balance(RawOrigin::Root.into(), stash_lookup, raw_amount.into(), raw_amount.into())?; - let amount: >::Balance = raw_amount.into(); + let amount: BalanceOf = raw_amount.into(); Staking::::bond( RawOrigin::Signed(stash.clone()).into(), controller_lookup.clone(), @@ -93,11 +96,9 @@ fn create_offender(n: u32, nominators: u32) -> Result::Source = - T::Lookup::unlookup(nominator_stash.clone()); + let nominator_stash_lookup: LookupSourceOf = T::Lookup::unlookup(nominator_stash.clone()); let nominator_controller: T::AccountId = account("nominator controller", n * MAX_NOMINATORS + i, SEED); - let nominator_controller_lookup: ::Source = - T::Lookup::unlookup(nominator_controller.clone()); + let nominator_controller_lookup: LookupSourceOf = T::Lookup::unlookup(nominator_controller.clone()); Balances::::set_balance( RawOrigin::Root.into(), nominator_stash_lookup, raw_amount.into(), raw_amount.into() )?; @@ -109,7 +110,7 @@ fn create_offender(n: u32, nominators: u32) -> Result::Source> = vec![controller_lookup.clone()]; + let selected_validators: Vec> = vec![controller_lookup.clone()]; Staking::::nominate(RawOrigin::Signed(nominator_controller.clone()).into(), selected_validators)?; individual_exposures.push(IndividualExposure { @@ -211,7 +212,7 @@ benchmarks! { Staking::::set_slash_reward_fraction(Perbill::one()); let mut offenders = make_offenders::(o, n).expect("failed to create offenders"); - let keys = ImOnline::::keys(); + let keys = ImOnline::::keys(); let offence = GrandpaEquivocationOffence { time_slot: GrandpaTimeSlot { set_id: 0, round: 0 }, diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 4bb8053c734e2..d55d41c91a89c 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -19,7 +19,7 @@ #![cfg(test)] use super::*; -use frame_support::{parameter_types, weights::Weight}; +use frame_support::parameter_types; use frame_system as system; use sp_runtime::{ SaturatedConversion, @@ -33,10 +33,6 @@ type AccountIndex = u32; type BlockNumber = u64; type Balance = u64; -parameter_types! { - pub const ExtrinsicBaseWeight: Weight = 10_000_000; -} - impl frame_system::Trait for Test { type Origin = Origin; type Index = AccountIndex; @@ -59,7 +55,7 @@ impl frame_system::Trait for Test { type OnNewAccount = (); type OnKilledAccount = (Balances,); type BlockExecutionWeight = (); - type ExtrinsicBaseWeight = ExtrinsicBaseWeight; + type ExtrinsicBaseWeight = (); } parameter_types! { pub const ExistentialDeposit: Balance = 10; @@ -135,8 +131,6 @@ pallet_staking_reward_curve::build! { parameter_types! { pub const RewardCurve: &'static sp_runtime::curve::PiecewiseLinear<'static> = &I_NPOS; pub const MaxNominatorRewardedPerValidator: u32 = 64; - pub const UnsignedPriority: u64 = 1 << 20; - pub const MaxIterations: u32 = 5; } pub type Extrinsic = sp_runtime::testing::TestXt; @@ -171,8 +165,8 @@ impl pallet_staking::Trait for Test { type ElectionLookahead = (); type Call = Call; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; - type UnsignedPriority = UnsignedPriority; - type MaxIterations = MaxIterations; + type UnsignedPriority = (); + type MaxIterations = (); } impl pallet_im_online::Trait for Test { @@ -180,7 +174,7 @@ impl pallet_im_online::Trait for Test { type Event = (); type SessionDuration = Period; type ReportUnresponsiveness = Offences; - type UnsignedPriority = UnsignedPriority; + type UnsignedPriority = (); } impl pallet_offences::Trait for Test { From 1d2a55d4ca60ef05ba8a7a85704f17540a3c9e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 30 Apr 2020 16:35:51 +0200 Subject: [PATCH 11/19] Address review grumbles. --- frame/offences/benchmarking/src/lib.rs | 29 +++++++++++++------------ frame/offences/benchmarking/src/mock.rs | 16 +++++--------- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 68b92e06988df..d3aecb7e38383 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -23,7 +23,7 @@ mod mock; use sp_std::prelude::*; use sp_std::vec; -use frame_system::{RawOrigin, Module as System}; +use frame_system::{RawOrigin, Module as System, Trait as SystemTrait}; use frame_benchmarking::{benchmarks, account}; use frame_support::traits::{Currency, OnInitialize}; @@ -67,15 +67,18 @@ pub trait IdTupleConvert { fn convert(id: IdentificationTuple) -> ::IdentificationTuple; } +type LookupSourceOf = <::Lookup as StaticLookup>::Source; +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; + fn create_offender(n: u32, nominators: u32) -> Result { let stash: T::AccountId = account("stash", n, SEED); - let stash_lookup: ::Source = T::Lookup::unlookup(stash.clone()); + let stash_lookup: LookupSourceOf = T::Lookup::unlookup(stash.clone()); let controller: T::AccountId = account("controller", n, SEED); - let controller_lookup: ::Source = T::Lookup::unlookup(controller.clone()); + let controller_lookup: LookupSourceOf = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; let raw_amount = 1_000_000; Balances::::set_balance(RawOrigin::Root.into(), stash_lookup, raw_amount.into(), raw_amount.into())?; - let amount: >::Balance = raw_amount.into(); + let amount: BalanceOf = raw_amount.into(); Staking::::bond( RawOrigin::Signed(stash.clone()).into(), controller_lookup.clone(), @@ -93,11 +96,9 @@ fn create_offender(n: u32, nominators: u32) -> Result::Source = - T::Lookup::unlookup(nominator_stash.clone()); + let nominator_stash_lookup: LookupSourceOf = T::Lookup::unlookup(nominator_stash.clone()); let nominator_controller: T::AccountId = account("nominator controller", n * MAX_NOMINATORS + i, SEED); - let nominator_controller_lookup: ::Source = - T::Lookup::unlookup(nominator_controller.clone()); + let nominator_controller_lookup: LookupSourceOf = T::Lookup::unlookup(nominator_controller.clone()); Balances::::set_balance( RawOrigin::Root.into(), nominator_stash_lookup, raw_amount.into(), raw_amount.into() )?; @@ -109,7 +110,7 @@ fn create_offender(n: u32, nominators: u32) -> Result::Source> = vec![controller_lookup.clone()]; + let selected_validators: Vec> = vec![controller_lookup.clone()]; Staking::::nominate(RawOrigin::Signed(nominator_controller.clone()).into(), selected_validators)?; individual_exposures.push(IndividualExposure { @@ -170,7 +171,7 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let offenders = make_offenders::(o, n).expect("failed to create offenders"); + let offenders = make_offenders::(o, n)?; let keys = ImOnline::::keys(); let offence = UnresponsivenessOffence { @@ -210,8 +211,8 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let mut offenders = make_offenders::(o, n).expect("failed to create offenders"); - let keys = ImOnline::::keys(); + let mut offenders = make_offenders::(o, n)?; + let keys = ImOnline::::keys(); let offence = GrandpaEquivocationOffence { time_slot: GrandpaTimeSlot { set_id: 0, round: 0 }, @@ -251,7 +252,7 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let mut offenders = make_offenders::(o, n).expect("failed to create offenders"); + let mut offenders = make_offenders::(o, n)?; let keys = ImOnline::::keys(); let offence = BabeEquivocationOffence { @@ -285,7 +286,7 @@ benchmarks! { Staking::::put_election_status(ElectionStatus::Closed); let mut deferred_offences = vec![]; - let offenders = make_offenders::(o, n).expect("failed to create offenders"); + let offenders = make_offenders::(o, n)?; let offence_details = offenders.into_iter() .map(|offender| sp_staking::offence::OffenceDetails { offender: T::convert(offender), diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 4bb8053c734e2..d55d41c91a89c 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -19,7 +19,7 @@ #![cfg(test)] use super::*; -use frame_support::{parameter_types, weights::Weight}; +use frame_support::parameter_types; use frame_system as system; use sp_runtime::{ SaturatedConversion, @@ -33,10 +33,6 @@ type AccountIndex = u32; type BlockNumber = u64; type Balance = u64; -parameter_types! { - pub const ExtrinsicBaseWeight: Weight = 10_000_000; -} - impl frame_system::Trait for Test { type Origin = Origin; type Index = AccountIndex; @@ -59,7 +55,7 @@ impl frame_system::Trait for Test { type OnNewAccount = (); type OnKilledAccount = (Balances,); type BlockExecutionWeight = (); - type ExtrinsicBaseWeight = ExtrinsicBaseWeight; + type ExtrinsicBaseWeight = (); } parameter_types! { pub const ExistentialDeposit: Balance = 10; @@ -135,8 +131,6 @@ pallet_staking_reward_curve::build! { parameter_types! { pub const RewardCurve: &'static sp_runtime::curve::PiecewiseLinear<'static> = &I_NPOS; pub const MaxNominatorRewardedPerValidator: u32 = 64; - pub const UnsignedPriority: u64 = 1 << 20; - pub const MaxIterations: u32 = 5; } pub type Extrinsic = sp_runtime::testing::TestXt; @@ -171,8 +165,8 @@ impl pallet_staking::Trait for Test { type ElectionLookahead = (); type Call = Call; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; - type UnsignedPriority = UnsignedPriority; - type MaxIterations = MaxIterations; + type UnsignedPriority = (); + type MaxIterations = (); } impl pallet_im_online::Trait for Test { @@ -180,7 +174,7 @@ impl pallet_im_online::Trait for Test { type Event = (); type SessionDuration = Period; type ReportUnresponsiveness = Offences; - type UnsignedPriority = UnsignedPriority; + type UnsignedPriority = (); } impl pallet_offences::Trait for Test { From be69494d0f8683b1c23b1cd712e5f243edb8e952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 4 May 2020 11:37:16 +0200 Subject: [PATCH 12/19] Address review grumbles part 1/2 --- frame/balances/src/lib.rs | 2 +- frame/offences/benchmarking/src/lib.rs | 11 +++++------ frame/offences/benchmarking/src/mock.rs | 12 ++++++------ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 41edf3ca9b87d..94dbd3730f163 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -458,7 +458,7 @@ decl_module! { /// - Contains a limited number of reads and writes. /// # #[weight = T::DbWeight::get().reads_writes(1, 1) + 100_000_000] - pub fn set_balance( + fn set_balance( origin, who: ::Source, #[compact] new_free: T::Balance, diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index d3aecb7e38383..bbf21633df276 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -72,12 +72,14 @@ type BalanceOf = <::Currency as Currency<(n: u32, nominators: u32) -> Result { let stash: T::AccountId = account("stash", n, SEED); - let stash_lookup: LookupSourceOf = T::Lookup::unlookup(stash.clone()); let controller: T::AccountId = account("controller", n, SEED); let controller_lookup: LookupSourceOf = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; + // let raw_amount = crate::mock::ExistentialDeposit::get(); let raw_amount = 1_000_000; - Balances::::set_balance(RawOrigin::Root.into(), stash_lookup, raw_amount.into(), raw_amount.into())?; + // make twice as much balance to prevent the account from being killed. + let free_amount = 2 * raw_amount; + Balances::::make_free_balance_be(&stash, free_amount.into()); let amount: BalanceOf = raw_amount.into(); Staking::::bond( RawOrigin::Signed(stash.clone()).into(), @@ -96,12 +98,9 @@ fn create_offender(n: u32, nominators: u32) -> Result = T::Lookup::unlookup(nominator_stash.clone()); let nominator_controller: T::AccountId = account("nominator controller", n * MAX_NOMINATORS + i, SEED); let nominator_controller_lookup: LookupSourceOf = T::Lookup::unlookup(nominator_controller.clone()); - Balances::::set_balance( - RawOrigin::Root.into(), nominator_stash_lookup, raw_amount.into(), raw_amount.into() - )?; + Balances::::make_free_balance_be(&nominator_stash, free_amount.into()); Staking::::bond( RawOrigin::Signed(nominator_stash.clone()).into(), diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index d55d41c91a89c..20cf337d442b9 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -43,7 +43,7 @@ impl frame_system::Trait for Test { type AccountId = AccountId; type Lookup = IdentityLookup; type Header = sp_runtime::testing::Header; - type Event = (); + type Event = Event; type BlockHashCount = (); type MaximumBlockWeight = (); type DbWeight = (); @@ -62,7 +62,7 @@ parameter_types! { } impl pallet_balances::Trait for Test { type Balance = Balance; - type Event = (); + type Event = Event; type DustRemoval = (); type ExistentialDeposit = ExistentialDeposit; type AccountStore = System; @@ -113,7 +113,7 @@ impl pallet_session::Trait for Test { type ShouldEndSession = pallet_session::PeriodicSessions; type NextSessionRotation = pallet_session::PeriodicSessions; type SessionHandler = TestSessionHandler; - type Event = (); + type Event = Event; type ValidatorId = AccountId; type ValidatorIdOf = pallet_staking::StashOf; type DisabledValidatorsThreshold = (); @@ -152,7 +152,7 @@ impl pallet_staking::Trait for Test { type UnixTime = pallet_timestamp::Module; type CurrencyToVote = CurrencyToVoteHandler; type RewardRemainder = (); - type Event = (); + type Event = Event; type Slash = (); type Reward = (); type SessionsPerEra = (); @@ -171,14 +171,14 @@ impl pallet_staking::Trait for Test { impl pallet_im_online::Trait for Test { type AuthorityId = UintAuthorityId; - type Event = (); + type Event = Event; type SessionDuration = Period; type ReportUnresponsiveness = Offences; type UnsignedPriority = (); } impl pallet_offences::Trait for Test { - type Event = (); + type Event = Event; type IdentificationTuple = pallet_session::historical::IdentificationTuple; type OnOffenceHandler = Staking; } From 6b5b8980bfd863aa877db85c4cd77ba50972ab6f Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 4 May 2020 12:09:58 +0200 Subject: [PATCH 13/19] use currency trait --- frame/offences/benchmarking/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index bbf21633df276..5421ffcbb9580 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -27,10 +27,10 @@ use frame_system::{RawOrigin, Module as System, Trait as SystemTrait}; use frame_benchmarking::{benchmarks, account}; use frame_support::traits::{Currency, OnInitialize}; -use sp_runtime::{Perbill, traits::{Convert, StaticLookup}}; +use sp_runtime::{Perbill, traits::{Convert, StaticLookup, Saturating}}; use sp_staking::offence::ReportOffence; -use pallet_balances::{Trait as BalancesTrait, Module as Balances}; +use pallet_balances::{Trait as BalancesTrait}; use pallet_babe::BabeEquivocationOffence; use pallet_grandpa::{GrandpaEquivocationOffence, GrandpaTimeSlot}; use pallet_im_online::{Trait as ImOnlineTrait, Module as ImOnline, UnresponsivenessOffence}; @@ -75,11 +75,10 @@ fn create_offender(n: u32, nominators: u32) -> Result = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; - // let raw_amount = crate::mock::ExistentialDeposit::get(); - let raw_amount = 1_000_000; + let raw_amount = T::Currency::minimum_balance().saturating_mul(10_000.into()); // make twice as much balance to prevent the account from being killed. - let free_amount = 2 * raw_amount; - Balances::::make_free_balance_be(&stash, free_amount.into()); + let free_amount = raw_amount.saturating_mul(2.into()); + T::Currency::make_free_balance_be(&stash, free_amount); let amount: BalanceOf = raw_amount.into(); Staking::::bond( RawOrigin::Signed(stash.clone()).into(), @@ -100,7 +99,7 @@ fn create_offender(n: u32, nominators: u32) -> Result = T::Lookup::unlookup(nominator_controller.clone()); - Balances::::make_free_balance_be(&nominator_stash, free_amount.into()); + T::Currency::make_free_balance_be(&nominator_stash, free_amount.into()); Staking::::bond( RawOrigin::Signed(nominator_stash.clone()).into(), From 08bcf294f0bff110dbd0b05829125bc2bae71b9b Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 4 May 2020 13:27:02 +0200 Subject: [PATCH 14/19] features --- frame/offences/benchmarking/Cargo.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/offences/benchmarking/Cargo.toml b/frame/offences/benchmarking/Cargo.toml index 1529d978d3b1a..7b998176eb0de 100644 --- a/frame/offences/benchmarking/Cargo.toml +++ b/frame/offences/benchmarking/Cargo.toml @@ -17,7 +17,7 @@ frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = " frame-support = { version = "2.0.0-dev", default-features = false, path = "../../support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../../system" } pallet-babe = { version = "2.0.0-dev", default-features = false, path = "../../babe" } -pallet-balances = { version = "2.0.0-dev", path = "../../balances" } +pallet-balances = { version = "2.0.0-dev", default-features = false, path = "../../balances" } pallet-grandpa = { version = "2.0.0-dev", default-features = false, path = "../../grandpa" } pallet-im-online = { version = "2.0.0-dev", default-features = false, path = "../../im-online" } pallet-offences = { version = "2.0.0-dev", default-features = false, features = ["runtime-benchmarks"], path = "../../offences" } @@ -43,6 +43,7 @@ std = [ "frame-support/std", "frame-system/std", "pallet-babe/std", + "pallet-balances/std", "pallet-grandpa/std", "pallet-im-online/std", "pallet-offences/std", @@ -51,4 +52,5 @@ std = [ "sp-runtime/std", "sp-staking/std", "sp-std/std", + "sp-io/std", ] From 3e7024d039e7b85ce850754ec0c1be7bdf29e480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 4 May 2020 18:16:38 +0200 Subject: [PATCH 15/19] Check events explicitly. --- frame/offences/benchmarking/src/lib.rs | 129 +++++++++++++++++++------ 1 file changed, 102 insertions(+), 27 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 5421ffcbb9580..fad12116ac785 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -27,8 +27,8 @@ use frame_system::{RawOrigin, Module as System, Trait as SystemTrait}; use frame_benchmarking::{benchmarks, account}; use frame_support::traits::{Currency, OnInitialize}; -use sp_runtime::{Perbill, traits::{Convert, StaticLookup, Saturating}}; -use sp_staking::offence::ReportOffence; +use sp_runtime::{Perbill, traits::{Convert, StaticLookup, Saturating, UniqueSaturatedInto}}; +use sp_staking::offence::{ReportOffence, Offence, OffenceDetails}; use pallet_balances::{Trait as BalancesTrait}; use pallet_babe::BabeEquivocationOffence; @@ -39,7 +39,7 @@ use pallet_session::historical::{Trait as HistoricalTrait, IdentificationTuple}; use pallet_session::{Trait as SessionTrait, SessionManager}; use pallet_staking::{ Module as Staking, Trait as StakingTrait, RewardDestination, ValidatorPrefs, - Exposure, IndividualExposure, ElectionStatus, MAX_NOMINATIONS, + Exposure, IndividualExposure, ElectionStatus, MAX_NOMINATIONS, Event as StakingEvent }; const SEED: u32 = 0; @@ -70,13 +70,23 @@ pub trait IdTupleConvert { type LookupSourceOf = <::Lookup as StaticLookup>::Source; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; -fn create_offender(n: u32, nominators: u32) -> Result { +struct Offender { + pub controller: T::AccountId, + pub stash: T::AccountId, + pub nominator_stashes: Vec, +} + +fn bond_amount() -> BalanceOf { + T::Currency::minimum_balance().saturating_mul(10_000.into()) +} + +fn create_offender(n: u32, nominators: u32) -> Result, &'static str> { let stash: T::AccountId = account("stash", n, SEED); let controller: T::AccountId = account("controller", n, SEED); let controller_lookup: LookupSourceOf = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; - let raw_amount = T::Currency::minimum_balance().saturating_mul(10_000.into()); - // make twice as much balance to prevent the account from being killed. + let raw_amount = bond_amount::(); + // add twice as much balance to prevent the account from being killed. let free_amount = raw_amount.saturating_mul(2.into()); T::Currency::make_free_balance_be(&stash, free_amount); let amount: BalanceOf = raw_amount.into(); @@ -93,7 +103,7 @@ fn create_offender(n: u32, nominators: u32) -> Result::validate(RawOrigin::Signed(controller.clone()).into(), validator_prefs)?; let mut individual_exposures = vec![]; - + let mut nominator_stashes = vec![]; // Create n nominators for i in 0 .. nominators { let nominator_stash: T::AccountId = account("nominator stash", n * MAX_NOMINATORS + i, SEED); @@ -115,6 +125,7 @@ fn create_offender(n: u32, nominators: u32) -> Result(n: u32, nominators: u32) -> Result::add_era_stakers(current_era.into(), stash.clone().into(), exposure); - Ok(controller) + Ok(Offender { controller, stash, nominator_stashes }) } -fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result>, &'static str> { +fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result< + (Vec>, Vec>), + &'static str +> { Staking::::new_session(0); - let mut offenders: Vec = vec![]; + let mut offenders = vec![]; for i in 0 .. num_offenders { let offender = create_offender::(i + 1, num_nominators)?; offenders.push(offender); @@ -139,15 +153,42 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result::start_session(0); - Ok(offenders.iter() - .map(|id| - ::ValidatorIdOf::convert(id.clone()) + let id_tuples = offenders.iter() + .map(|offender| + ::ValidatorIdOf::convert(offender.controller.clone()) .expect("failed to get validator id from account id")) .map(|validator_id| ::FullIdentificationOf::convert(validator_id.clone()) .map(|full_id| (validator_id, full_id)) .expect("failed to convert validator id to full identification")) - .collect::>>()) + .collect::>>(); + Ok((id_tuples, offenders)) +} + +fn check_events::Event>>(expected: I) { + let events = System::::events() .into_iter() + .map(|frame_system::EventRecord { event, .. }| event).collect::>(); + let expected = expected.collect::>(); + let lengths = (events.len(), expected.len()); + let length_mismatch = if lengths.0 != lengths.1 { + fn pretty(header: &str, ev: &[D]) { + println!("{}", header); + for (idx, ev) in ev.iter().enumerate() { + println!("\t[{:04}] {:?}", idx, ev); + } + } + pretty("--Got:", &events); + pretty("--Expected:", &expected); + format!("Mismatching length. Got: {}, expected: {}", lengths.0, lengths.1) + } else { Default::default() }; + + for (idx, (a, b)) in events.into_iter().zip(expected).enumerate() { + assert_eq!(a, b, "Mismatch at: {}. {}", idx, length_mismatch); + } + + if !length_mismatch.is_empty() { + panic!(length_mismatch); + } } benchmarks! { @@ -169,28 +210,62 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let offenders = make_offenders::(o, n)?; + let (offenders, raw_offenders) = make_offenders::(o, n)?; let keys = ImOnline::::keys(); + let validator_set_count = keys.len() as u32; + let slash_fraction = UnresponsivenessOffence::::slash_fraction( + offenders.len() as u32, validator_set_count, + ); let offence = UnresponsivenessOffence { session_index: 0, - validator_set_count: keys.len() as u32, + validator_set_count, offenders, }; assert_eq!(System::::event_count(), 0); }: { - let _ = ::ReportUnresponsiveness::report_offence(reporters, offence); + let _ = ::ReportUnresponsiveness::report_offence( + reporters.clone(), + offence + ); } verify { // make sure the report was not deferred assert!(Offences::::deferred_offences().is_empty()); + let slash_amount = slash_fraction * bond_amount::().unique_saturated_into() as u32; + let reward_amount = slash_amount * (1 + n) / 2; + let mut slash_events = raw_offenders.into_iter() + .flat_map(|offender| { + std::iter::once(offender.stash).chain(offender.nominator_stashes.into_iter()) + }) + .map(|stash| ::Event::from( + StakingEvent::::Slash(stash, BalanceOf::::from(slash_amount)) + )) + .collect::>(); + let reward_events = reporters.into_iter() + .flat_map(|reporter| vec![ + frame_system::Event::::NewAccount(reporter.clone()).into(), + ::Event::from( + pallet_balances::Event::::Endowed(reporter.clone(), (reward_amount / r).into()) + ).into() + ]); + + // rewards are applied after first offender and it's nominators + let slash_rest = slash_events.split_off(1 + n as usize); + // make sure that all slashes have been applied - assert_eq!( - System::::event_count(), 0 - + 1 // offence - + 2 * r // reporter (reward + endowment) - + o // offenders slashed - + o * n // nominators slashed + check_events::( + std::iter::empty() + .chain(slash_events.into_iter().map(Into::into)) + .chain(reward_events) + .chain(slash_rest.into_iter().map(Into::into)) + .chain(std::iter::once(::Event::from( + pallet_offences::Event::Offence( + UnresponsivenessOffence::::ID, + 0_u32.to_le_bytes().to_vec(), + true + ) + ).into())) ); } @@ -209,7 +284,7 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let mut offenders = make_offenders::(o, n)?; + let (mut offenders, raw_offenders) = make_offenders::(o, n)?; let keys = ImOnline::::keys(); let offence = GrandpaEquivocationOffence { @@ -250,7 +325,7 @@ benchmarks! { // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let mut offenders = make_offenders::(o, n)?; + let (mut offenders, raw_offenders) = make_offenders::(o, n)?; let keys = ImOnline::::keys(); let offence = BabeEquivocationOffence { @@ -284,9 +359,9 @@ benchmarks! { Staking::::put_election_status(ElectionStatus::Closed); let mut deferred_offences = vec![]; - let offenders = make_offenders::(o, n)?; + let offenders = make_offenders::(o, n)?.0; let offence_details = offenders.into_iter() - .map(|offender| sp_staking::offence::OffenceDetails { + .map(|offender| OffenceDetails { offender: T::convert(offender), reporters: vec![], }) From 1fedd84f4b6e8fccd4241550d82343536c3e6d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 4 May 2020 18:22:30 +0200 Subject: [PATCH 16/19] Auto-impl tuple converter. --- frame/offences/benchmarking/src/lib.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index fad12116ac785..d4ad32f2005a3 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -67,6 +67,14 @@ pub trait IdTupleConvert { fn convert(id: IdentificationTuple) -> ::IdentificationTuple; } +impl IdTupleConvert for T where + ::IdentificationTuple: From> +{ + fn convert(id: IdentificationTuple) -> ::IdentificationTuple { + id.into() + } +} + type LookupSourceOf = <::Lookup as StaticLookup>::Source; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; @@ -395,12 +403,12 @@ mod tests { use super::*; use crate::mock::{new_test_ext, Test}; use frame_support::assert_ok; - - impl IdTupleConvert for Test { - fn convert(id: IdentificationTuple) -> ::IdentificationTuple { - id - } - } + // + // impl IdTupleConvert for Test { + // fn convert(id: IdentificationTuple) -> ::IdentificationTuple { + // id + // } + // } #[test] fn test_benchmarks() { From 0615188e03a8e5fd048cb534fc6669b6fb666d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 4 May 2020 18:23:26 +0200 Subject: [PATCH 17/19] Removed dead code. --- frame/offences/benchmarking/src/lib.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index d4ad32f2005a3..3e331640e3d71 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -64,6 +64,7 @@ pub trait Trait: /// A helper trait to make sure we can convert `IdentificationTuple` coming from historical /// and the one required by offences. pub trait IdTupleConvert { + /// Convert identification tuple from `historical` trait to the one expected by `offences`. fn convert(id: IdentificationTuple) -> ::IdentificationTuple; } @@ -403,12 +404,6 @@ mod tests { use super::*; use crate::mock::{new_test_ext, Test}; use frame_support::assert_ok; - // - // impl IdTupleConvert for Test { - // fn convert(id: IdentificationTuple) -> ::IdentificationTuple { - // id - // } - // } #[test] fn test_benchmarks() { From e756563cceacd9c2218db9f7baeed5e2f55ca8e3 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 4 May 2020 18:40:04 +0200 Subject: [PATCH 18/19] add test feature flag --- frame/offences/benchmarking/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 3e331640e3d71..71fab183a928d 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -174,6 +174,7 @@ fn make_offenders(num_offenders: u32, num_nominators: u32) -> Result< Ok((id_tuples, offenders)) } +#[cfg(test)] fn check_events::Event>>(expected: I) { let events = System::::events() .into_iter() .map(|frame_system::EventRecord { event, .. }| event).collect::>(); @@ -263,6 +264,7 @@ benchmarks! { let slash_rest = slash_events.split_off(1 + n as usize); // make sure that all slashes have been applied + #[cfg(test)] check_events::( std::iter::empty() .chain(slash_events.into_iter().map(Into::into)) From 79dbc4927cf0e3af54ae2831741b63dad4aca00d Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 4 May 2020 19:02:00 +0200 Subject: [PATCH 19/19] dont use std --- frame/offences/benchmarking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 71fab183a928d..a0e05a74d58db 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -246,7 +246,7 @@ benchmarks! { let reward_amount = slash_amount * (1 + n) / 2; let mut slash_events = raw_offenders.into_iter() .flat_map(|offender| { - std::iter::once(offender.stash).chain(offender.nominator_stashes.into_iter()) + core::iter::once(offender.stash).chain(offender.nominator_stashes.into_iter()) }) .map(|stash| ::Event::from( StakingEvent::::Slash(stash, BalanceOf::::from(slash_amount))