From 35fe3cd1bc4b64cadb0bc6196ae40173db65bb28 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 --- frame/babe/src/lib.rs | 29 ++++++++++++++++++++--------- frame/babe/src/tests.rs | 23 +++++++++++++++++++++++ frame/grandpa/src/lib.rs | 2 +- frame/grandpa/src/tests.rs | 23 +++++++++++++++++++++++ 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index f80ac186434d0..891411e8ede57 100644 --- a/frame/babe/src/lib.rs +++ b/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/frame/babe/src/tests.rs b/frame/babe/src/tests.rs index bdd6748c3b351..2b24e1208de1d 100644 --- a/frame/babe/src/tests.rs +++ b/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/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 961c099460752..09d32662d349a 100644 --- a/frame/grandpa/src/lib.rs +++ b/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/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 9eca2cc381371..aa1b48681d402 100644 --- a/frame/grandpa/src/tests.rs +++ b/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]) + ); +}