From a79c34778c9af04d484a92d37ede9a713c894741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= <123550+andresilva@users.noreply.github.com> Date: Wed, 26 Aug 2020 17:27:44 +0100 Subject: [PATCH] babe: fix report_equivocation weight (#6936) * babe: fix report_equivocation weight * node: bump spec_version * babe: fix floor in report_equivocation weight calculation Co-authored-by: Gavin Wood * grandpa: fix floor in report_equivocation weight calculation * babe, grandpa: add test for weight_for::report_equivocation Co-authored-by: Gavin Wood --- substrate/frame/babe/src/lib.rs | 29 +++++++++++++++++++--------- substrate/frame/babe/src/tests.rs | 23 ++++++++++++++++++++++ substrate/frame/grandpa/src/lib.rs | 2 +- substrate/frame/grandpa/src/tests.rs | 23 ++++++++++++++++++++++ 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/substrate/frame/babe/src/lib.rs b/substrate/frame/babe/src/lib.rs index f80ac18643..891411e8ed 100644 --- a/substrate/frame/babe/src/lib.rs +++ b/substrate/frame/babe/src/lib.rs @@ -255,7 +255,7 @@ decl_module! { /// the equivocation proof and validate the given key ownership proof /// against the extracted offender. If both are valid, the offence will /// be reported. - #[weight = weight::weight_for_report_equivocation::()] + #[weight = weight_for::report_equivocation::(key_owner_proof.validator_count())] fn report_equivocation( origin, equivocation_proof: EquivocationProof, @@ -278,7 +278,7 @@ decl_module! { /// block authors will call it (validated in `ValidateUnsigned`), as such /// if the block author is defined it will be defined as the equivocation /// reporter. - #[weight = weight::weight_for_report_equivocation::()] + #[weight = weight_for::report_equivocation::(key_owner_proof.validator_count())] fn report_equivocation_unsigned( origin, equivocation_proof: EquivocationProof, @@ -295,24 +295,35 @@ decl_module! { } } -mod weight { +mod weight_for { use frame_support::{ traits::Get, - weights::{constants::WEIGHT_PER_MICROS, Weight}, + weights::{ + constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}, + Weight, + }, }; - pub fn weight_for_report_equivocation() -> Weight { + pub fn report_equivocation(validator_count: u32) -> Weight { + // we take the validator set count from the membership proof to + // calculate the weight but we set a floor of 100 validators. + let validator_count = validator_count.max(100) as u64; + + // worst case we are considering is that the given offender + // is backed by 200 nominators + const MAX_NOMINATORS: u64 = 200; + // checking membership proof (35 * WEIGHT_PER_MICROS) + .saturating_add((175 * WEIGHT_PER_NANOS).saturating_mul(validator_count)) .saturating_add(T::DbWeight::get().reads(5)) // check equivocation proof .saturating_add(110 * WEIGHT_PER_MICROS) // report offence .saturating_add(110 * WEIGHT_PER_MICROS) - // worst case we are considering is that the given offender - // is backed by 200 nominators - .saturating_add(T::DbWeight::get().reads(14 + 3 * 200)) - .saturating_add(T::DbWeight::get().writes(10 + 3 * 200)) + .saturating_add(25 * WEIGHT_PER_MICROS * MAX_NOMINATORS) + .saturating_add(T::DbWeight::get().reads(14 + 3 * MAX_NOMINATORS)) + .saturating_add(T::DbWeight::get().writes(10 + 3 * MAX_NOMINATORS)) } } diff --git a/substrate/frame/babe/src/tests.rs b/substrate/frame/babe/src/tests.rs index bdd6748c3b..2b24e1208d 100644 --- a/substrate/frame/babe/src/tests.rs +++ b/substrate/frame/babe/src/tests.rs @@ -585,3 +585,26 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() { ); }); } + +#[test] +fn report_equivocation_has_valid_weight() { + // the weight depends on the size of the validator set, + // but there's a lower bound of 100 validators. + assert!( + (1..=100) + .map(weight_for::report_equivocation::) + .collect::>() + .windows(2) + .all(|w| w[0] == w[1]) + ); + + // after 100 validators the weight should keep increasing + // with every extra validator. + assert!( + (100..=1000) + .map(weight_for::report_equivocation::) + .collect::>() + .windows(2) + .all(|w| w[0] < w[1]) + ); +} diff --git a/substrate/frame/grandpa/src/lib.rs b/substrate/frame/grandpa/src/lib.rs index 961c099460..09d32662d3 100644 --- a/substrate/frame/grandpa/src/lib.rs +++ b/substrate/frame/grandpa/src/lib.rs @@ -376,7 +376,7 @@ mod weight_for { pub fn report_equivocation(validator_count: u32) -> Weight { // we take the validator set count from the membership proof to // calculate the weight but we set a floor of 100 validators. - let validator_count = validator_count.min(100) as u64; + let validator_count = validator_count.max(100) as u64; // worst case we are considering is that the given offender // is backed by 200 nominators diff --git a/substrate/frame/grandpa/src/tests.rs b/substrate/frame/grandpa/src/tests.rs index 9eca2cc381..aa1b48681d 100644 --- a/substrate/frame/grandpa/src/tests.rs +++ b/substrate/frame/grandpa/src/tests.rs @@ -842,3 +842,26 @@ fn always_schedules_a_change_on_new_session_when_stalled() { assert_eq!(Grandpa::current_set_id(), 2); }); } + +#[test] +fn report_equivocation_has_valid_weight() { + // the weight depends on the size of the validator set, + // but there's a lower bound of 100 validators. + assert!( + (1..=100) + .map(weight_for::report_equivocation::) + .collect::>() + .windows(2) + .all(|w| w[0] == w[1]) + ); + + // after 100 validators the weight should keep increasing + // with every extra validator. + assert!( + (100..=1000) + .map(weight_for::report_equivocation::) + .collect::>() + .windows(2) + .all(|w| w[0] < w[1]) + ); +}