From 33a79d9d4f45097c085f1a9c5e1d3a3b3b407c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 21 Aug 2020 21:13:22 +0100 Subject: [PATCH 1/5] babe: fix report_equivocation weight --- frame/babe/src/lib.rs | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index f80ac186434d0..202d6b718227a 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.min(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)) } } From de1d1171ff2e973c15483b8164038e727f8ba9f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Fri, 21 Aug 2020 21:41:22 +0100 Subject: [PATCH 2/5] node: bump spec_version --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 9595ef424d8c1..19963ef6cc9f1 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -109,7 +109,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 257, + spec_version: 258, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From f896f421bdc27929dec71e69c1f50d93dc8c5edc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= <123550+andresilva@users.noreply.github.com> Date: Mon, 24 Aug 2020 14:26:55 +0100 Subject: [PATCH 3/5] babe: fix floor in report_equivocation weight calculation Co-authored-by: Gavin Wood --- frame/babe/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index 202d6b718227a..891411e8ede57 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -307,7 +307,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 From 02c7fef0a0cccfd4dc2f4933e71a5e30aadc2cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Mon, 24 Aug 2020 14:28:11 +0100 Subject: [PATCH 4/5] grandpa: fix floor in report_equivocation weight calculation --- frame/grandpa/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 9faa6a238305d7bf9d36e977cfdcf0c8f3d79f32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Mon, 24 Aug 2020 14:28:38 +0100 Subject: [PATCH 5/5] babe, grandpa: add test for weight_for::report_equivocation --- frame/babe/src/tests.rs | 23 +++++++++++++++++++++++ frame/grandpa/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+) 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/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]) + ); +}