From 605c89311007b6dda31495585a6717d51f3713ee Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 6 Mar 2022 11:48:23 +0000 Subject: [PATCH 01/26] Move `sp-npos-elections-solution-type` to `frame-election-provider-support` First stab at it, will need to amend some more stuff --- Cargo.lock | 46 ++++++++++++------- Cargo.toml | 3 +- bin/node/runtime/Cargo.toml | 1 + bin/node/runtime/src/lib.rs | 2 +- .../solution-type/Cargo.toml | 6 +-- .../solution-type/fuzzer/Cargo.toml | 30 ++++++++++++ .../solution-type}/fuzzer/src/compact.rs | 3 +- .../solution-type/src/codec.rs | 0 .../src/from_assignment_helpers.rs | 0 .../solution-type/src/index_assignment.rs | 0 .../solution-type/src/lib.rs | 0 .../solution-type/src/single_page.rs | 0 .../tests/ui/fail/missing_accuracy.rs | 0 .../tests/ui/fail/missing_accuracy.stderr | 0 .../tests/ui/fail/missing_target.rs | 0 .../tests/ui/fail/missing_target.stderr | 0 .../tests/ui/fail/missing_voter.rs | 0 .../tests/ui/fail/missing_voter.stderr | 0 .../tests/ui/fail/no_annotations.rs | 0 .../tests/ui/fail/no_annotations.stderr | 0 .../tests/ui/fail/swap_voter_target.rs | 0 .../tests/ui/fail/swap_voter_target.stderr | 0 .../tests/ui/fail/wrong_attribute.rs | 0 .../tests/ui/fail/wrong_attribute.stderr | 0 primitives/npos-elections/Cargo.toml | 1 - primitives/npos-elections/fuzzer/Cargo.toml | 4 -- primitives/npos-elections/src/lib.rs | 7 ++- primitives/npos-elections/src/phragmms.rs | 2 +- primitives/npos-elections/src/reduce.rs | 1 + 29 files changed, 73 insertions(+), 33 deletions(-) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/Cargo.toml (74%) create mode 100644 frame/election-provider-support/solution-type/fuzzer/Cargo.toml rename {primitives/npos-elections => frame/election-provider-support/solution-type}/fuzzer/src/compact.rs (94%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/src/codec.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/src/from_assignment_helpers.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/src/index_assignment.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/src/lib.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/src/single_page.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/missing_accuracy.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/missing_accuracy.stderr (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/missing_target.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/missing_target.stderr (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/missing_voter.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/missing_voter.stderr (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/no_annotations.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/no_annotations.stderr (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/swap_voter_target.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/swap_voter_target.stderr (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/wrong_attribute.rs (100%) rename {primitives/npos-elections => frame/election-provider-support}/solution-type/tests/ui/fail/wrong_attribute.stderr (100%) diff --git a/Cargo.lock b/Cargo.lock index db6edfa81d539..ab3e1f92e390f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2176,6 +2176,21 @@ dependencies = [ "sp-std", ] +[[package]] +name = "frame-election-solution-type-fuzzer" +version = "2.0.0-alpha.5" +dependencies = [ + "clap 3.0.7", + "frame-npos-elections-solution-type", + "honggfuzz", + "parity-scale-codec", + "rand 0.8.4", + "scale-info", + "sp-arithmetic", + "sp-npos-elections", + "sp-runtime", +] + [[package]] name = "frame-executive" version = "4.0.0-dev" @@ -2208,6 +2223,20 @@ dependencies = [ "serde", ] +[[package]] +name = "frame-npos-elections-solution-type" +version = "4.0.0-dev" +dependencies = [ + "parity-scale-codec", + "proc-macro-crate 1.1.0", + "proc-macro2", + "quote", + "scale-info", + "sp-arithmetic", + "syn", + "trybuild", +] + [[package]] name = "frame-support" version = "4.0.0-dev" @@ -4965,6 +4994,7 @@ dependencies = [ "frame-benchmarking", "frame-election-provider-support", "frame-executive", + "frame-npos-elections-solution-type", "frame-support", "frame-system", "frame-system-benchmarking", @@ -10058,7 +10088,6 @@ dependencies = [ "serde", "sp-arithmetic", "sp-core", - "sp-npos-elections-solution-type", "sp-runtime", "sp-std", "substrate-test-utils", @@ -10077,21 +10106,6 @@ dependencies = [ "sp-runtime", ] -[[package]] -name = "sp-npos-elections-solution-type" -version = "4.0.0-dev" -dependencies = [ - "parity-scale-codec", - "proc-macro-crate 1.1.0", - "proc-macro2", - "quote", - "scale-info", - "sp-arithmetic", - "sp-npos-elections", - "syn", - "trybuild", -] - [[package]] name = "sp-offchain" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index f24ff6d04980a..bce23456b27e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,6 +86,8 @@ members = [ "frame/try-runtime", "frame/election-provider-multi-phase", "frame/election-provider-support", + "frame/election-provider-support/solution-type", + "frame/election-provider-support/solution-type/fuzzer", "frame/examples/basic", "frame/examples/offchain-worker", "frame/examples/parallel", @@ -169,7 +171,6 @@ members = [ "primitives/keystore", "primitives/maybe-compressed-blob", "primitives/npos-elections", - "primitives/npos-elections/solution-type", "primitives/npos-elections/fuzzer", "primitives/offchain", "primitives/panic-handler", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 0572345fa0c89..b9ee7d7d391c0 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -49,6 +49,7 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../.. frame-system = { version = "4.0.0-dev", default-features = false, path = "../../../frame/system" } frame-system-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../../../frame/system/benchmarking", optional = true } frame-election-provider-support = { version = "4.0.0-dev", default-features = false, path = "../../../frame/election-provider-support" } +frame-npos-elections-solution-type = { version = "4.0.0-dev", default-features = false, path = "../../../frame/election-provider-support/solution-type" } frame-system-rpc-runtime-api = { version = "4.0.0-dev", default-features = false, path = "../../../frame/system/rpc/runtime-api/" } frame-try-runtime = { version = "0.10.0-dev", default-features = false, path = "../../../frame/try-runtime", optional = true } pallet-assets = { version = "4.0.0-dev", default-features = false, path = "../../../frame/assets" } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index f12bf8a88365f..cf38123080478 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -588,7 +588,7 @@ parameter_types! { .get(DispatchClass::Normal); } -sp_npos_elections::generate_solution_type!( +frame_npos_elections_solution_type::generate_solution_type!( #[compact] pub struct NposSolution16::< VoterIndex = u32, diff --git a/primitives/npos-elections/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml similarity index 74% rename from primitives/npos-elections/solution-type/Cargo.toml rename to frame/election-provider-support/solution-type/Cargo.toml index 813637e89c338..2679628c7f436 100644 --- a/primitives/npos-elections/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "sp-npos-elections-solution-type" +name = "frame-npos-elections-solution-type" version = "4.0.0-dev" authors = ["Parity Technologies "] edition = "2021" @@ -23,7 +23,5 @@ proc-macro-crate = "1.1.0" [dev-dependencies] parity-scale-codec = "3.0.0" scale-info = "2.0.1" -sp-arithmetic = { path = "../../arithmetic", version = "5.0.0"} -# used by generate_solution_type: -sp-npos-elections = { path = "..", version = "4.0.0-dev" } +sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../../primitives/arithmetic" } trybuild = "1.0.53" diff --git a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml new file mode 100644 index 0000000000000..1b5893f6c94bb --- /dev/null +++ b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "frame-election-solution-type-fuzzer" +version = "2.0.0-alpha.5" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Fuzzer for phragmén solution type implementation." +publish = false + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +clap = { version = "3.0", features = ["derive"] } +honggfuzz = "0.5" +rand = { version = "0.8", features = ["std", "small_rng"] } + +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } +scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } +frame-npos-elections-solution-type = { version = "4.0.0-dev", path = ".." } +sp-arithmetic = { version = "5.0.0", path = "../../../../primitives/arithmetic" } +sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } +# used by generate_solution_type: +sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../../primitives/npos-elections" } + +[[bin]] +name = "compact" +path = "src/compact.rs" diff --git a/primitives/npos-elections/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs similarity index 94% rename from primitives/npos-elections/fuzzer/src/compact.rs rename to frame/election-provider-support/solution-type/fuzzer/src/compact.rs index 595048575d99c..ae40532d71ded 100644 --- a/primitives/npos-elections/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -1,6 +1,7 @@ use honggfuzz::fuzz; -use sp_npos_elections::{generate_solution_type, sp_arithmetic::Percent}; +use sp_arithmetic::Percent; use sp_runtime::codec::{Encode, Error}; +use frame_npos_elections_solution_type::generate_solution_type; fn main() { generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::< diff --git a/primitives/npos-elections/solution-type/src/codec.rs b/frame/election-provider-support/solution-type/src/codec.rs similarity index 100% rename from primitives/npos-elections/solution-type/src/codec.rs rename to frame/election-provider-support/solution-type/src/codec.rs diff --git a/primitives/npos-elections/solution-type/src/from_assignment_helpers.rs b/frame/election-provider-support/solution-type/src/from_assignment_helpers.rs similarity index 100% rename from primitives/npos-elections/solution-type/src/from_assignment_helpers.rs rename to frame/election-provider-support/solution-type/src/from_assignment_helpers.rs diff --git a/primitives/npos-elections/solution-type/src/index_assignment.rs b/frame/election-provider-support/solution-type/src/index_assignment.rs similarity index 100% rename from primitives/npos-elections/solution-type/src/index_assignment.rs rename to frame/election-provider-support/solution-type/src/index_assignment.rs diff --git a/primitives/npos-elections/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs similarity index 100% rename from primitives/npos-elections/solution-type/src/lib.rs rename to frame/election-provider-support/solution-type/src/lib.rs diff --git a/primitives/npos-elections/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs similarity index 100% rename from primitives/npos-elections/solution-type/src/single_page.rs rename to frame/election-provider-support/solution-type/src/single_page.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/missing_accuracy.rs rename to frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/missing_accuracy.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.stderr similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/missing_accuracy.stderr rename to frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.stderr diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/missing_target.rs rename to frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/missing_target.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.stderr similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/missing_target.stderr rename to frame/election-provider-support/solution-type/tests/ui/fail/missing_target.stderr diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/missing_voter.rs rename to frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/missing_voter.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.stderr similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/missing_voter.stderr rename to frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.stderr diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/no_annotations.rs rename to frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/no_annotations.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.stderr similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/no_annotations.stderr rename to frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.stderr diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/swap_voter_target.rs rename to frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/swap_voter_target.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.stderr similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/swap_voter_target.stderr rename to frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.stderr diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.rs rename to frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.stderr similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.stderr rename to frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.stderr diff --git a/primitives/npos-elections/Cargo.toml b/primitives/npos-elections/Cargo.toml index 3facf32196c74..13edbfe90008b 100644 --- a/primitives/npos-elections/Cargo.toml +++ b/primitives/npos-elections/Cargo.toml @@ -17,7 +17,6 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features = scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } serde = { version = "1.0.136", optional = true, features = ["derive"] } sp-std = { version = "4.0.0", default-features = false, path = "../std" } -sp-npos-elections-solution-type = { version = "4.0.0-dev", path = "./solution-type" } sp-arithmetic = { version = "5.0.0", default-features = false, path = "../arithmetic" } sp-core = { version = "6.0.0", default-features = false, path = "../core" } sp-runtime = { version = "6.0.0", path = "../runtime", default-features = false } diff --git a/primitives/npos-elections/fuzzer/Cargo.toml b/primitives/npos-elections/fuzzer/Cargo.toml index afa331b0676e0..157a42478e658 100644 --- a/primitives/npos-elections/fuzzer/Cargo.toml +++ b/primitives/npos-elections/fuzzer/Cargo.toml @@ -35,10 +35,6 @@ path = "src/phragmen_balancing.rs" name = "phragmms_balancing" path = "src/phragmms_balancing.rs" -[[bin]] -name = "compact" -path = "src/compact.rs" - [[bin]] name = "phragmen_pjr" path = "src/phragmen_pjr.rs" diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index 7bd1a4b7f69b6..673036c2d19d1 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -77,7 +77,9 @@ use scale_info::TypeInfo; use sp_arithmetic::{traits::Zero, Normalizable, PerThing, Rational128, ThresholdOrd}; use sp_core::RuntimeDebug; -use sp_std::{cell::RefCell, cmp::Ordering, collections::btree_map::BTreeMap, prelude::*, rc::Rc}; +use sp_std::{ + cell::RefCell, cmp::Ordering, collections::btree_map::BTreeMap, prelude::*, rc::Rc, vec, +}; use codec::{Decode, Encode, MaxEncodedLen}; #[cfg(feature = "std")] @@ -117,9 +119,6 @@ pub use sp_arithmetic; #[doc(hidden)] pub use sp_std; -// re-export the solution type macro. -pub use sp_npos_elections_solution_type::generate_solution_type; - /// The errors that might occur in the this crate and solution-type. #[derive(Eq, PartialEq, RuntimeDebug)] pub enum Error { diff --git a/primitives/npos-elections/src/phragmms.rs b/primitives/npos-elections/src/phragmms.rs index 6220cacd157b2..fff318f590fc6 100644 --- a/primitives/npos-elections/src/phragmms.rs +++ b/primitives/npos-elections/src/phragmms.rs @@ -26,7 +26,7 @@ use crate::{ VoteWeight, Voter, }; use sp_arithmetic::{traits::Bounded, PerThing, Rational128}; -use sp_std::{prelude::*, rc::Rc}; +use sp_std::{prelude::*, rc::Rc, vec}; /// Execute the phragmms method. /// diff --git a/primitives/npos-elections/src/reduce.rs b/primitives/npos-elections/src/reduce.rs index f089a37e3fff3..c5860f7ee41ee 100644 --- a/primitives/npos-elections/src/reduce.rs +++ b/primitives/npos-elections/src/reduce.rs @@ -55,6 +55,7 @@ use sp_arithmetic::traits::{Bounded, Zero}; use sp_std::{ collections::btree_map::{BTreeMap, Entry::*}, prelude::*, + vec, }; /// Map type used for reduce_4. Can be easily swapped with HashMap. From 020cd99e118e6e9168b0cbff691e790c7ee6b3b2 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 6 Mar 2022 13:56:39 +0000 Subject: [PATCH 02/26] Fixing tests --- Cargo.lock | 1 + .../solution-type/Cargo.toml | 2 + .../solution-type/src/lib.rs | 4 +- .../solution-type/src/mock.rs | 178 +++++++++ .../solution-type/src/tests.rs | 350 ++++++++++++++++++ .../tests/ui/fail/missing_accuracy.rs | 2 +- .../tests/ui/fail/missing_target.rs | 2 +- .../tests/ui/fail/missing_voter.rs | 2 +- .../tests/ui/fail/no_annotations.rs | 2 +- .../tests/ui/fail/swap_voter_target.rs | 2 +- .../tests/ui/fail/wrong_attribute.rs | 2 +- primitives/npos-elections/src/mock.rs | 161 -------- primitives/npos-elections/src/tests.rs | 333 +---------------- 13 files changed, 542 insertions(+), 499 deletions(-) create mode 100644 frame/election-provider-support/solution-type/src/mock.rs create mode 100644 frame/election-provider-support/solution-type/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index ab3e1f92e390f..103da539f5b4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2233,6 +2233,7 @@ dependencies = [ "quote", "scale-info", "sp-arithmetic", + "sp-npos-elections", "syn", "trybuild", ] diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index 2679628c7f436..87af6ed08ebed 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -24,4 +24,6 @@ proc-macro-crate = "1.1.0" parity-scale-codec = "3.0.0" scale-info = "2.0.1" sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../../primitives/arithmetic" } +# used by generate_solution_type: +sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/npos-elections" } trybuild = "1.0.53" diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 6e632d19e171e..9bed21cfc2e8f 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -57,7 +57,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// type, `u8` target type and `Perbill` accuracy with maximum of 4 edges per voter. /// /// ``` -/// # use sp_npos_elections_solution_type::generate_solution_type; +/// # use frame_npos_elections_solution_type::generate_solution_type; /// # use sp_arithmetic::per_things::Perbill; /// generate_solution_type!(pub struct TestSolution::< /// VoterIndex = u16, @@ -100,7 +100,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// for numbers will be used, similar to how `parity-scale-codec`'s `Compact` works. /// /// ``` -/// # use sp_npos_elections_solution_type::generate_solution_type; +/// # use frame_npos_elections_solution_type::generate_solution_type; /// # use sp_npos_elections::NposSolution; /// # use sp_arithmetic::per_things::Perbill; /// generate_solution_type!( diff --git a/frame/election-provider-support/solution-type/src/mock.rs b/frame/election-provider-support/solution-type/src/mock.rs new file mode 100644 index 0000000000000..c3d032f2eb257 --- /dev/null +++ b/frame/election-provider-support/solution-type/src/mock.rs @@ -0,0 +1,178 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Mock file for solution-type. + +#![cfg(test)] + +use std::{collections::HashMap, convert::TryInto, hash::Hash, HashSet}; + +use rand::{seq::SliceRandom, Rng}; + +/// The candidate mask allows easy disambiguation between voters and candidates: accounts +/// for which this bit is set are candidates, and without it, are voters. +pub const CANDIDATE_MASK: AccountId = 1 << ((std::mem::size_of::() * 8) - 1); + +pub type TestAccuracy = sp_runtime::Perbill; + +pub fn p(p: u8) -> TestAccuracy { + TestAccuracy::from_percent(p.into()) +} + +pub type MockAssignment = crate::Assignment; +pub type Voter = (AccountId, VoteWeight, Vec); + +crate::generate_solution_type! { + pub struct TestSolution::< + VoterIndex = u32, + TargetIndex = u16, + Accuracy = TestAccuracy, + >(16) +} + +/// Generate voter and assignment lists. Makes no attempt to be realistic about winner or assignment +/// fairness. +/// +/// Maintains these invariants: +/// +/// - candidate ids have `CANDIDATE_MASK` bit set +/// - voter ids do not have `CANDIDATE_MASK` bit set +/// - assignments have the same ordering as voters +/// - `assignments.distribution.iter().map(|(_, frac)| frac).sum() == One::one()` +/// - a coherent set of winners is chosen. +/// - the winner set is a subset of the candidate set. +/// - `assignments.distribution.iter().all(|(who, _)| winners.contains(who))` +pub fn generate_random_votes( + candidate_count: usize, + voter_count: usize, + mut rng: impl Rng, +) -> (Vec, Vec, Vec) { + // cache for fast generation of unique candidate and voter ids + let mut used_ids = HashSet::with_capacity(candidate_count + voter_count); + + // candidates are easy: just a completely random set of IDs + let mut candidates: Vec = Vec::with_capacity(candidate_count); + while candidates.len() < candidate_count { + let mut new = || rng.gen::() | CANDIDATE_MASK; + let mut id = new(); + // insert returns `false` when the value was already present + while !used_ids.insert(id) { + id = new(); + } + candidates.push(id); + } + + // voters are random ids, random weights, random selection from the candidates + let mut voters = Vec::with_capacity(voter_count); + while voters.len() < voter_count { + let mut new = || rng.gen::() & !CANDIDATE_MASK; + let mut id = new(); + // insert returns `false` when the value was already present + while !used_ids.insert(id) { + id = new(); + } + + let vote_weight = rng.gen(); + + // it's not interesting if a voter chooses 0 or all candidates, so rule those cases out. + // also, let's not generate any cases which result in a compact overflow. + let n_candidates_chosen = + rng.gen_range(1, candidates.len().min(::LIMIT)); + + let mut chosen_candidates = Vec::with_capacity(n_candidates_chosen); + chosen_candidates.extend(candidates.choose_multiple(&mut rng, n_candidates_chosen)); + voters.push((id, vote_weight, chosen_candidates)); + } + + // always generate a sensible number of winners: elections are uninteresting if nobody wins, + // or everybody wins + let num_winners = rng.gen_range(1, candidate_count); + let mut winners: HashSet = HashSet::with_capacity(num_winners); + winners.extend(candidates.choose_multiple(&mut rng, num_winners)); + assert_eq!(winners.len(), num_winners); + + let mut assignments = Vec::with_capacity(voters.len()); + for (voter_id, _, votes) in voters.iter() { + let chosen_winners = votes.iter().filter(|vote| winners.contains(vote)).cloned(); + let num_chosen_winners = chosen_winners.clone().count(); + + // distribute the available stake randomly + let stake_distribution = if num_chosen_winners == 0 { + continue + } else { + let mut available_stake = 1000; + let mut stake_distribution = Vec::with_capacity(num_chosen_winners); + for _ in 0..num_chosen_winners - 1 { + let stake = rng.gen_range(0, available_stake).min(1); + stake_distribution.push(TestAccuracy::from_perthousand(stake)); + available_stake -= stake; + } + stake_distribution.push(TestAccuracy::from_perthousand(available_stake)); + stake_distribution.shuffle(&mut rng); + stake_distribution + }; + + assignments.push(MockAssignment { + who: *voter_id, + distribution: chosen_winners.zip(stake_distribution).collect(), + }); + } + + (voters, assignments, candidates) +} + +fn generate_cache(voters: Voters) -> HashMap +where + Voters: Iterator, + Item: Hash + Eq + Copy, +{ + let mut cache = HashMap::new(); + for (idx, voter_id) in voters.enumerate() { + cache.insert(voter_id, idx); + } + cache +} + +/// Create a function that returns the index of a voter in the voters list. +pub fn make_voter_fn(voters: &[Voter]) -> impl Fn(&AccountId) -> Option +where + usize: TryInto, +{ + let cache = generate_cache(voters.iter().map(|(id, _, _)| *id)); + move |who| { + if cache.get(who).is_none() { + println!("WARNING: voter {} will raise InvalidIndex", who); + } + cache.get(who).cloned().and_then(|i| i.try_into().ok()) + } +} + +/// Create a function that returns the index of a candidate in the candidates list. +pub fn make_target_fn( + candidates: &[AccountId], +) -> impl Fn(&AccountId) -> Option +where + usize: TryInto, +{ + let cache = generate_cache(candidates.iter().cloned()); + move |who| { + if cache.get(who).is_none() { + println!("WARNING: target {} will raise InvalidIndex", who); + } + cache.get(who).cloned().and_then(|i| i.try_into().ok()) + } +} diff --git a/frame/election-provider-support/solution-type/src/tests.rs b/frame/election-provider-support/solution-type/src/tests.rs new file mode 100644 index 0000000000000..f173e425b5187 --- /dev/null +++ b/frame/election-provider-support/solution-type/src/tests.rs @@ -0,0 +1,350 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for solution-type. + +#![cfg(test)] + +use crate::{mock::*, IndexAssignment, NposSolution}; +use rand::SeedableRng; +use std::convert::TryInto; + +mod solution_type { + use super::*; + use codec::{Decode, Encode}; + // these need to come from the same dev-dependency `sp-npos-elections`, not from the crate. + use crate::{generate_solution_type, Assignment, Error as NposError, NposSolution}; + use sp_std::{convert::TryInto, fmt::Debug}; + + #[allow(dead_code)] + mod __private { + // This is just to make sure that the solution can be generated in a scope without any + // imports. + use crate::generate_solution_type; + generate_solution_type!( + #[compact] + struct InnerTestSolutionIsolated::(12) + ); + } + + #[test] + fn solution_struct_works_with_and_without_compact() { + // we use u32 size to make sure compact is smaller. + let without_compact = { + generate_solution_type!( + pub struct InnerTestSolution::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = TestAccuracy, + >(16) + ); + let solution = InnerTestSolution { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], + ..Default::default() + }; + + solution.encode().len() + }; + + let with_compact = { + generate_solution_type!( + #[compact] + pub struct InnerTestSolutionCompact::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = TestAccuracy, + >(16) + ); + let compact = InnerTestSolutionCompact { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], + ..Default::default() + }; + + compact.encode().len() + }; + + assert!(with_compact < without_compact); + } + + #[test] + fn solution_struct_is_codec() { + let solution = TestSolution { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], + ..Default::default() + }; + + let encoded = solution.encode(); + + assert_eq!(solution, Decode::decode(&mut &encoded[..]).unwrap()); + assert_eq!(solution.voter_count(), 4); + assert_eq!(solution.edge_count(), 2 + 4); + assert_eq!(solution.unique_targets(), vec![10, 11, 20, 40, 50, 51]); + } + + #[test] + fn remove_voter_works() { + let mut solution = TestSolution { + votes1: vec![(0, 2), (1, 6)], + votes2: vec![(2, [(0, p(80))], 1), (3, [(7, p(85))], 8)], + votes3: vec![(4, [(3, p(50)), (4, p(25))], 5)], + ..Default::default() + }; + + assert!(!solution.remove_voter(11)); + assert!(solution.remove_voter(2)); + assert_eq!( + solution, + TestSolution { + votes1: vec![(0, 2), (1, 6)], + votes2: vec![(3, [(7, p(85))], 8)], + votes3: vec![(4, [(3, p(50)), (4, p(25))], 5,)], + ..Default::default() + }, + ); + + assert!(solution.remove_voter(4)); + assert_eq!( + solution, + TestSolution { + votes1: vec![(0, 2), (1, 6)], + votes2: vec![(3, [(7, p(85))], 8)], + ..Default::default() + }, + ); + + assert!(solution.remove_voter(1)); + assert_eq!( + solution, + TestSolution { + votes1: vec![(0, 2)], + votes2: vec![(3, [(7, p(85))], 8),], + ..Default::default() + }, + ); + } + + #[test] + fn from_and_into_assignment_works() { + let voters = vec![2 as AccountId, 4, 1, 5, 3]; + let targets = vec![ + 10 as AccountId, + 11, + 20, // 2 + 30, + 31, // 4 + 32, + 40, // 6 + 50, + 51, // 8 + ]; + + let assignments = vec![ + Assignment { who: 2 as AccountId, distribution: vec![(20u64, p(100))] }, + Assignment { who: 4, distribution: vec![(40, p(100))] }, + Assignment { who: 1, distribution: vec![(10, p(80)), (11, p(20))] }, + Assignment { who: 5, distribution: vec![(50, p(85)), (51, p(15))] }, + Assignment { who: 3, distribution: vec![(30, p(50)), (31, p(25)), (32, p(25))] }, + ]; + + let voter_index = |a: &AccountId| -> Option { + voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() + }; + let target_index = |a: &AccountId| -> Option { + targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() + }; + + let solution = + TestSolution::from_assignment(&assignments, voter_index, target_index).unwrap(); + + // basically number of assignments that it is encoding. + assert_eq!(solution.voter_count(), assignments.len()); + assert_eq!( + solution.edge_count(), + assignments.iter().fold(0, |a, b| a + b.distribution.len()), + ); + + assert_eq!( + solution, + TestSolution { + votes1: vec![(0, 2), (1, 6)], + votes2: vec![(2, [(0, p(80))], 1), (3, [(7, p(85))], 8)], + votes3: vec![(4, [(3, p(50)), (4, p(25))], 5)], + ..Default::default() + } + ); + + assert_eq!(solution.unique_targets(), vec![0, 1, 2, 3, 4, 5, 6, 7, 8]); + + let voter_at = |a: u32| -> Option { + voters.get(>::try_into(a).unwrap()).cloned() + }; + let target_at = |a: u16| -> Option { + targets.get(>::try_into(a).unwrap()).cloned() + }; + + assert_eq!(solution.into_assignment(voter_at, target_at).unwrap(), assignments); + } + + #[test] + fn unique_targets_len_edge_count_works() { + // we don't really care about voters here so all duplicates. This is not invalid per se. + let solution = TestSolution { + votes1: vec![(99, 1), (99, 2)], + votes2: vec![(99, [(3, p(10))], 7), (99, [(4, p(10))], 8)], + votes3: vec![(99, [(11, p(10)), (12, p(10))], 13)], + // ensure the last one is also counted. + votes16: vec![( + 99, + [ + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + (66, p(10)), + ], + 67, + )], + ..Default::default() + }; + + assert_eq!(solution.unique_targets(), vec![1, 2, 3, 4, 7, 8, 11, 12, 13, 66, 67]); + assert_eq!(solution.edge_count(), 2 + (2 * 2) + 3 + 16); + assert_eq!(solution.voter_count(), 6); + + // this one has some duplicates. + let solution = TestSolution { + votes1: vec![(99, 1), (99, 1)], + votes2: vec![(99, [(3, p(10))], 7), (99, [(4, p(10))], 8)], + votes3: vec![(99, [(11, p(10)), (11, p(10))], 13)], + ..Default::default() + }; + + assert_eq!(solution.unique_targets(), vec![1, 3, 4, 7, 8, 11, 13]); + assert_eq!(solution.edge_count(), 2 + (2 * 2) + 3); + assert_eq!(solution.voter_count(), 5); + } + + #[test] + fn solution_into_assignment_must_report_overflow() { + // in votes2 + let solution = TestSolution { + votes1: Default::default(), + votes2: vec![(0, [(1, p(100))], 2)], + ..Default::default() + }; + + let voter_at = |a: u32| -> Option { Some(a as AccountId) }; + let target_at = |a: u16| -> Option { Some(a as AccountId) }; + + assert_eq!( + solution.into_assignment(&voter_at, &target_at).unwrap_err(), + NposError::SolutionWeightOverflow, + ); + + // in votes3 onwards + let solution = TestSolution { + votes1: Default::default(), + votes2: Default::default(), + votes3: vec![(0, [(1, p(70)), (2, p(80))], 3)], + ..Default::default() + }; + + assert_eq!( + solution.into_assignment(&voter_at, &target_at).unwrap_err(), + NposError::SolutionWeightOverflow, + ); + } + + #[test] + fn target_count_overflow_is_detected() { + let voter_index = |a: &AccountId| -> Option { Some(*a as u32) }; + let target_index = |a: &AccountId| -> Option { Some(*a as u16) }; + + let assignments = vec![Assignment { + who: 1 as AccountId, + distribution: (10..27).map(|i| (i as AccountId, p(i as u8))).collect::>(), + }]; + + let solution = TestSolution::from_assignment(&assignments, voter_index, target_index); + assert_eq!(solution.unwrap_err(), NposError::SolutionTargetOverflow); + } + + #[test] + fn zero_target_count_is_ignored() { + let voters = vec![1 as AccountId, 2]; + let targets = vec![10 as AccountId, 11]; + + let assignments = vec![ + Assignment { who: 1 as AccountId, distribution: vec![(10, p(50)), (11, p(50))] }, + Assignment { who: 2, distribution: vec![] }, + ]; + + let voter_index = |a: &AccountId| -> Option { + voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() + }; + let target_index = |a: &AccountId| -> Option { + targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() + }; + + let solution = + TestSolution::from_assignment(&assignments, voter_index, target_index).unwrap(); + + assert_eq!( + solution, + TestSolution { + votes1: Default::default(), + votes2: vec![(0, [(0, p(50))], 1)], + ..Default::default() + } + ); + } +} + +#[test] +fn index_assignments_generate_same_solution_as_plain_assignments() { + let rng = rand::rngs::SmallRng::seed_from_u64(0); + + let (voters, assignments, candidates) = generate_random_votes(1000, 2500, rng); + let voter_index = make_voter_fn(&voters); + let target_index = make_target_fn(&candidates); + + let solution = + TestSolution::from_assignment(&assignments, &voter_index, &target_index).unwrap(); + + let index_assignments = assignments + .into_iter() + .map(|assignment| IndexAssignment::new(&assignment, &voter_index, &target_index)) + .collect::, _>>() + .unwrap(); + + let index_compact = index_assignments.as_slice().try_into().unwrap(); + + assert_eq!(solution, index_compact); +} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index b74b857e45815..47ec86b85220a 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -1,4 +1,4 @@ -use sp_npos_elections_solution_type::generate_solution_type; +use frame_npos_elections_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index 4c9cd51a32096..17727ad8f8286 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -1,4 +1,4 @@ -use sp_npos_elections_solution_type::generate_solution_type; +use frame_npos_elections_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index b87037f77f1e3..b735477d8cdc8 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -1,4 +1,4 @@ -use sp_npos_elections_solution_type::generate_solution_type; +use frame_npos_elections_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index cfca2841db633..4a1445591527d 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -1,4 +1,4 @@ -use sp_npos_elections_solution_type::generate_solution_type; +use frame_npos_elections_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index 443202d11b39b..bbb5c88568752 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -1,4 +1,4 @@ -use sp_npos_elections_solution_type::generate_solution_type; +use frame_npos_elections_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index 3008277e36b74..3e4786ede7674 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -1,4 +1,4 @@ -use sp_npos_elections_solution_type::generate_solution_type; +use frame_npos_elections_solution_type::generate_solution_type; generate_solution_type!( #[pages(1)] pub struct TestSolution::< diff --git a/primitives/npos-elections/src/mock.rs b/primitives/npos-elections/src/mock.rs index 85c970d7b418f..dd85ce9b6dfae 100644 --- a/primitives/npos-elections/src/mock.rs +++ b/primitives/npos-elections/src/mock.rs @@ -19,13 +19,6 @@ #![cfg(test)] -use std::{ - collections::{HashMap, HashSet}, - convert::TryInto, - hash::Hash, -}; - -use rand::{self, seq::SliceRandom, Rng}; use sp_arithmetic::{ traits::{One, SaturatedConversion, Zero}, PerThing, @@ -37,27 +30,6 @@ use crate::{seq_phragmen, Assignment, ElectionResult, ExtendedBalance, PerThing1 pub type AccountId = u64; -/// The candidate mask allows easy disambiguation between voters and candidates: accounts -/// for which this bit is set are candidates, and without it, are voters. -pub const CANDIDATE_MASK: AccountId = 1 << ((std::mem::size_of::() * 8) - 1); - -pub type TestAccuracy = sp_runtime::Perbill; - -crate::generate_solution_type! { - pub struct TestSolution::< - VoterIndex = u32, - TargetIndex = u16, - Accuracy = TestAccuracy, - >(16) -} - -pub fn p(p: u8) -> TestAccuracy { - TestAccuracy::from_percent(p.into()) -} - -pub type MockAssignment = crate::Assignment; -pub type Voter = (AccountId, VoteWeight, Vec); - #[derive(Default, Debug)] pub(crate) struct _Candidate { who: A, @@ -412,136 +384,3 @@ pub(crate) fn build_support_map_float( } supports } - -/// Generate voter and assignment lists. Makes no attempt to be realistic about winner or assignment -/// fairness. -/// -/// Maintains these invariants: -/// -/// - candidate ids have `CANDIDATE_MASK` bit set -/// - voter ids do not have `CANDIDATE_MASK` bit set -/// - assignments have the same ordering as voters -/// - `assignments.distribution.iter().map(|(_, frac)| frac).sum() == One::one()` -/// - a coherent set of winners is chosen. -/// - the winner set is a subset of the candidate set. -/// - `assignments.distribution.iter().all(|(who, _)| winners.contains(who))` -pub fn generate_random_votes( - candidate_count: usize, - voter_count: usize, - mut rng: impl Rng, -) -> (Vec, Vec, Vec) { - // cache for fast generation of unique candidate and voter ids - let mut used_ids = HashSet::with_capacity(candidate_count + voter_count); - - // candidates are easy: just a completely random set of IDs - let mut candidates: Vec = Vec::with_capacity(candidate_count); - while candidates.len() < candidate_count { - let mut new = || rng.gen::() | CANDIDATE_MASK; - let mut id = new(); - // insert returns `false` when the value was already present - while !used_ids.insert(id) { - id = new(); - } - candidates.push(id); - } - - // voters are random ids, random weights, random selection from the candidates - let mut voters = Vec::with_capacity(voter_count); - while voters.len() < voter_count { - let mut new = || rng.gen::() & !CANDIDATE_MASK; - let mut id = new(); - // insert returns `false` when the value was already present - while !used_ids.insert(id) { - id = new(); - } - - let vote_weight = rng.gen(); - - // it's not interesting if a voter chooses 0 or all candidates, so rule those cases out. - // also, let's not generate any cases which result in a compact overflow. - let n_candidates_chosen = - rng.gen_range(1, candidates.len().min(::LIMIT)); - - let mut chosen_candidates = Vec::with_capacity(n_candidates_chosen); - chosen_candidates.extend(candidates.choose_multiple(&mut rng, n_candidates_chosen)); - voters.push((id, vote_weight, chosen_candidates)); - } - - // always generate a sensible number of winners: elections are uninteresting if nobody wins, - // or everybody wins - let num_winners = rng.gen_range(1, candidate_count); - let mut winners: HashSet = HashSet::with_capacity(num_winners); - winners.extend(candidates.choose_multiple(&mut rng, num_winners)); - assert_eq!(winners.len(), num_winners); - - let mut assignments = Vec::with_capacity(voters.len()); - for (voter_id, _, votes) in voters.iter() { - let chosen_winners = votes.iter().filter(|vote| winners.contains(vote)).cloned(); - let num_chosen_winners = chosen_winners.clone().count(); - - // distribute the available stake randomly - let stake_distribution = if num_chosen_winners == 0 { - continue - } else { - let mut available_stake = 1000; - let mut stake_distribution = Vec::with_capacity(num_chosen_winners); - for _ in 0..num_chosen_winners - 1 { - let stake = rng.gen_range(0, available_stake).min(1); - stake_distribution.push(TestAccuracy::from_perthousand(stake)); - available_stake -= stake; - } - stake_distribution.push(TestAccuracy::from_perthousand(available_stake)); - stake_distribution.shuffle(&mut rng); - stake_distribution - }; - - assignments.push(MockAssignment { - who: *voter_id, - distribution: chosen_winners.zip(stake_distribution).collect(), - }); - } - - (voters, assignments, candidates) -} - -fn generate_cache(voters: Voters) -> HashMap -where - Voters: Iterator, - Item: Hash + Eq + Copy, -{ - let mut cache = HashMap::new(); - for (idx, voter_id) in voters.enumerate() { - cache.insert(voter_id, idx); - } - cache -} - -/// Create a function that returns the index of a voter in the voters list. -pub fn make_voter_fn(voters: &[Voter]) -> impl Fn(&AccountId) -> Option -where - usize: TryInto, -{ - let cache = generate_cache(voters.iter().map(|(id, _, _)| *id)); - move |who| { - if cache.get(who).is_none() { - println!("WARNING: voter {} will raise InvalidIndex", who); - } - cache.get(who).cloned().and_then(|i| i.try_into().ok()) - } -} - -/// Create a function that returns the index of a candidate in the candidates list. -pub fn make_target_fn( - candidates: &[AccountId], -) -> impl Fn(&AccountId) -> Option -where - usize: TryInto, -{ - let cache = generate_cache(candidates.iter().cloned()); - move |who| { - if cache.get(who).is_none() { - println!("WARNING: target {} will raise InvalidIndex", who); - } - cache.get(who).cloned().and_then(|i| i.try_into().ok()) - } -} diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index b199fdd1af77f..b0f8f16ce90fb 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -17,14 +17,13 @@ //! Tests for npos-elections. +#![cfg(test)] + use crate::{ balancing, helpers::*, mock::*, seq_phragmen, seq_phragmen_core, setup_inputs, to_support_map, - Assignment, ElectionResult, ExtendedBalance, IndexAssignment, NposSolution, StakedAssignment, - Support, Voter, + Assignment, ElectionResult, ExtendedBalance, StakedAssignment, Support, Voter, }; -use rand::{self, SeedableRng}; use sp_arithmetic::{PerU16, Perbill, Percent, Permill}; -use std::convert::TryInto; use substrate_test_utils::assert_eq_uvec; #[test] @@ -919,329 +918,3 @@ mod score { assert!(ElectionScore::from([10, 5, 15]) > ElectionScore::from([10, 5, 25])); } } - -mod solution_type { - use super::*; - use codec::{Decode, Encode}; - // these need to come from the same dev-dependency `sp-npos-elections`, not from the crate. - use crate::{generate_solution_type, Assignment, Error as NposError, NposSolution}; - use sp_std::{convert::TryInto, fmt::Debug}; - - #[allow(dead_code)] - mod __private { - // This is just to make sure that the solution can be generated in a scope without any - // imports. - use crate::generate_solution_type; - generate_solution_type!( - #[compact] - struct InnerTestSolutionIsolated::(12) - ); - } - - #[test] - fn solution_struct_works_with_and_without_compact() { - // we use u32 size to make sure compact is smaller. - let without_compact = { - generate_solution_type!( - pub struct InnerTestSolution::< - VoterIndex = u32, - TargetIndex = u32, - Accuracy = TestAccuracy, - >(16) - ); - let solution = InnerTestSolution { - votes1: vec![(2, 20), (4, 40)], - votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], - ..Default::default() - }; - - solution.encode().len() - }; - - let with_compact = { - generate_solution_type!( - #[compact] - pub struct InnerTestSolutionCompact::< - VoterIndex = u32, - TargetIndex = u32, - Accuracy = TestAccuracy, - >(16) - ); - let compact = InnerTestSolutionCompact { - votes1: vec![(2, 20), (4, 40)], - votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], - ..Default::default() - }; - - compact.encode().len() - }; - - assert!(with_compact < without_compact); - } - - #[test] - fn solution_struct_is_codec() { - let solution = TestSolution { - votes1: vec![(2, 20), (4, 40)], - votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], - ..Default::default() - }; - - let encoded = solution.encode(); - - assert_eq!(solution, Decode::decode(&mut &encoded[..]).unwrap()); - assert_eq!(solution.voter_count(), 4); - assert_eq!(solution.edge_count(), 2 + 4); - assert_eq!(solution.unique_targets(), vec![10, 11, 20, 40, 50, 51]); - } - - #[test] - fn remove_voter_works() { - let mut solution = TestSolution { - votes1: vec![(0, 2), (1, 6)], - votes2: vec![(2, [(0, p(80))], 1), (3, [(7, p(85))], 8)], - votes3: vec![(4, [(3, p(50)), (4, p(25))], 5)], - ..Default::default() - }; - - assert!(!solution.remove_voter(11)); - assert!(solution.remove_voter(2)); - assert_eq!( - solution, - TestSolution { - votes1: vec![(0, 2), (1, 6)], - votes2: vec![(3, [(7, p(85))], 8)], - votes3: vec![(4, [(3, p(50)), (4, p(25))], 5,)], - ..Default::default() - }, - ); - - assert!(solution.remove_voter(4)); - assert_eq!( - solution, - TestSolution { - votes1: vec![(0, 2), (1, 6)], - votes2: vec![(3, [(7, p(85))], 8)], - ..Default::default() - }, - ); - - assert!(solution.remove_voter(1)); - assert_eq!( - solution, - TestSolution { - votes1: vec![(0, 2)], - votes2: vec![(3, [(7, p(85))], 8),], - ..Default::default() - }, - ); - } - - #[test] - fn from_and_into_assignment_works() { - let voters = vec![2 as AccountId, 4, 1, 5, 3]; - let targets = vec![ - 10 as AccountId, - 11, - 20, // 2 - 30, - 31, // 4 - 32, - 40, // 6 - 50, - 51, // 8 - ]; - - let assignments = vec![ - Assignment { who: 2 as AccountId, distribution: vec![(20u64, p(100))] }, - Assignment { who: 4, distribution: vec![(40, p(100))] }, - Assignment { who: 1, distribution: vec![(10, p(80)), (11, p(20))] }, - Assignment { who: 5, distribution: vec![(50, p(85)), (51, p(15))] }, - Assignment { who: 3, distribution: vec![(30, p(50)), (31, p(25)), (32, p(25))] }, - ]; - - let voter_index = |a: &AccountId| -> Option { - voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() - }; - let target_index = |a: &AccountId| -> Option { - targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() - }; - - let solution = - TestSolution::from_assignment(&assignments, voter_index, target_index).unwrap(); - - // basically number of assignments that it is encoding. - assert_eq!(solution.voter_count(), assignments.len()); - assert_eq!( - solution.edge_count(), - assignments.iter().fold(0, |a, b| a + b.distribution.len()), - ); - - assert_eq!( - solution, - TestSolution { - votes1: vec![(0, 2), (1, 6)], - votes2: vec![(2, [(0, p(80))], 1), (3, [(7, p(85))], 8)], - votes3: vec![(4, [(3, p(50)), (4, p(25))], 5)], - ..Default::default() - } - ); - - assert_eq!(solution.unique_targets(), vec![0, 1, 2, 3, 4, 5, 6, 7, 8]); - - let voter_at = |a: u32| -> Option { - voters.get(>::try_into(a).unwrap()).cloned() - }; - let target_at = |a: u16| -> Option { - targets.get(>::try_into(a).unwrap()).cloned() - }; - - assert_eq!(solution.into_assignment(voter_at, target_at).unwrap(), assignments); - } - - #[test] - fn unique_targets_len_edge_count_works() { - // we don't really care about voters here so all duplicates. This is not invalid per se. - let solution = TestSolution { - votes1: vec![(99, 1), (99, 2)], - votes2: vec![(99, [(3, p(10))], 7), (99, [(4, p(10))], 8)], - votes3: vec![(99, [(11, p(10)), (12, p(10))], 13)], - // ensure the last one is also counted. - votes16: vec![( - 99, - [ - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - (66, p(10)), - ], - 67, - )], - ..Default::default() - }; - - assert_eq!(solution.unique_targets(), vec![1, 2, 3, 4, 7, 8, 11, 12, 13, 66, 67]); - assert_eq!(solution.edge_count(), 2 + (2 * 2) + 3 + 16); - assert_eq!(solution.voter_count(), 6); - - // this one has some duplicates. - let solution = TestSolution { - votes1: vec![(99, 1), (99, 1)], - votes2: vec![(99, [(3, p(10))], 7), (99, [(4, p(10))], 8)], - votes3: vec![(99, [(11, p(10)), (11, p(10))], 13)], - ..Default::default() - }; - - assert_eq!(solution.unique_targets(), vec![1, 3, 4, 7, 8, 11, 13]); - assert_eq!(solution.edge_count(), 2 + (2 * 2) + 3); - assert_eq!(solution.voter_count(), 5); - } - - #[test] - fn solution_into_assignment_must_report_overflow() { - // in votes2 - let solution = TestSolution { - votes1: Default::default(), - votes2: vec![(0, [(1, p(100))], 2)], - ..Default::default() - }; - - let voter_at = |a: u32| -> Option { Some(a as AccountId) }; - let target_at = |a: u16| -> Option { Some(a as AccountId) }; - - assert_eq!( - solution.into_assignment(&voter_at, &target_at).unwrap_err(), - NposError::SolutionWeightOverflow, - ); - - // in votes3 onwards - let solution = TestSolution { - votes1: Default::default(), - votes2: Default::default(), - votes3: vec![(0, [(1, p(70)), (2, p(80))], 3)], - ..Default::default() - }; - - assert_eq!( - solution.into_assignment(&voter_at, &target_at).unwrap_err(), - NposError::SolutionWeightOverflow, - ); - } - - #[test] - fn target_count_overflow_is_detected() { - let voter_index = |a: &AccountId| -> Option { Some(*a as u32) }; - let target_index = |a: &AccountId| -> Option { Some(*a as u16) }; - - let assignments = vec![Assignment { - who: 1 as AccountId, - distribution: (10..27).map(|i| (i as AccountId, p(i as u8))).collect::>(), - }]; - - let solution = TestSolution::from_assignment(&assignments, voter_index, target_index); - assert_eq!(solution.unwrap_err(), NposError::SolutionTargetOverflow); - } - - #[test] - fn zero_target_count_is_ignored() { - let voters = vec![1 as AccountId, 2]; - let targets = vec![10 as AccountId, 11]; - - let assignments = vec![ - Assignment { who: 1 as AccountId, distribution: vec![(10, p(50)), (11, p(50))] }, - Assignment { who: 2, distribution: vec![] }, - ]; - - let voter_index = |a: &AccountId| -> Option { - voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() - }; - let target_index = |a: &AccountId| -> Option { - targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() - }; - - let solution = - TestSolution::from_assignment(&assignments, voter_index, target_index).unwrap(); - - assert_eq!( - solution, - TestSolution { - votes1: Default::default(), - votes2: vec![(0, [(0, p(50))], 1)], - ..Default::default() - } - ); - } -} - -#[test] -fn index_assignments_generate_same_solution_as_plain_assignments() { - let rng = rand::rngs::SmallRng::seed_from_u64(0); - - let (voters, assignments, candidates) = generate_random_votes(1000, 2500, rng); - let voter_index = make_voter_fn(&voters); - let target_index = make_target_fn(&candidates); - - let solution = - TestSolution::from_assignment(&assignments, &voter_index, &target_index).unwrap(); - - let index_assignments = assignments - .into_iter() - .map(|assignment| IndexAssignment::new(&assignment, &voter_index, &target_index)) - .collect::, _>>() - .unwrap(); - - let index_compact = index_assignments.as_slice().try_into().unwrap(); - - assert_eq!(solution, index_compact); -} From 676923dd079d244749f2332cc2cdf78a5f0187b4 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Mon, 7 Mar 2022 20:50:59 +0000 Subject: [PATCH 03/26] Fixing tests --- Cargo.lock | 1 + frame/election-provider-multi-phase/Cargo.toml | 4 ++-- frame/election-provider-multi-phase/src/mock.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 103da539f5b4f..fce6397b5a91f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5865,6 +5865,7 @@ version = "4.0.0-dev" dependencies = [ "frame-benchmarking", "frame-election-provider-support", + "frame-npos-elections-solution-type", "frame-support", "frame-system", "log 0.4.14", diff --git a/frame/election-provider-multi-phase/Cargo.toml b/frame/election-provider-multi-phase/Cargo.toml index 38039f6926b15..bca40045dc5ed 100644 --- a/frame/election-provider-multi-phase/Cargo.toml +++ b/frame/election-provider-multi-phase/Cargo.toml @@ -46,8 +46,8 @@ sp-core = { version = "6.0.0", default-features = false, path = "../../primitive sp-io = { version = "6.0.0", path = "../../primitives/io" } sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../primitives/npos-elections" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } -frame-election-provider-support = { version = "4.0.0-dev", features = [ -], path = "../election-provider-support" } +frame-election-provider-support = { version = "4.0.0-dev", features = [], path = "../election-provider-support" } +frame-npos-elections-solution-type = { version = "4.0.0-dev", default-features = false, path = "../election-provider-support/solution-type" } pallet-balances = { version = "4.0.0-dev", path = "../balances" } frame-benchmarking = { version = "4.0.0-dev", path = "../benchmarking" } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 7c7034ac91a83..ef54792b12fa9 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -68,7 +68,7 @@ pub(crate) type BlockNumber = u64; pub(crate) type VoterIndex = u32; pub(crate) type TargetIndex = u16; -sp_npos_elections::generate_solution_type!( +frame_npos_elections_solution_type::generate_solution_type!( #[compact] pub struct TestNposSolution::(16) ); From 7c31687e31a227140397304efb343965d79a443e Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Mon, 7 Mar 2022 21:59:05 +0000 Subject: [PATCH 04/26] Fixing cargo.toml for std configuration --- frame/election-provider-support/solution-type/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index 87af6ed08ebed..20c618dbd4a5f 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -23,7 +23,7 @@ proc-macro-crate = "1.1.0" [dev-dependencies] parity-scale-codec = "3.0.0" scale-info = "2.0.1" -sp-arithmetic = { version = "5.0.0", default-features = false, path = "../../../primitives/arithmetic" } +sp-arithmetic = { version = "5.0.0", path = "../../../primitives/arithmetic" } # used by generate_solution_type: -sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/npos-elections" } +sp-npos-elections = { version = "4.0.0-dev", path = "../../../primitives/npos-elections" } trybuild = "1.0.53" From 79538d25be73dbcd5557dae82d62e2a045385be5 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Mon, 7 Mar 2022 23:18:44 +0000 Subject: [PATCH 05/26] Implementing `MaxEncodedLen` on `generate_solution_type` --- Cargo.lock | 2 ++ bin/node/runtime/src/lib.rs | 4 ++- .../election-provider-multi-phase/src/mock.rs | 7 ++++- .../solution-type/Cargo.toml | 1 + .../solution-type/fuzzer/Cargo.toml | 1 + .../solution-type/fuzzer/src/compact.rs | 3 +- .../solution-type/src/lib.rs | 30 +++++++++++++++---- .../solution-type/src/single_page.rs | 8 +++++ .../tests/ui/fail/missing_accuracy.rs | 1 + .../tests/ui/fail/missing_size_bound.rs | 10 +++++++ .../tests/ui/fail/missing_size_bound.stderr | 5 ++++ .../tests/ui/fail/missing_target.rs | 1 + .../tests/ui/fail/missing_voter.rs | 1 + .../tests/ui/fail/no_annotations.rs | 1 + .../tests/ui/fail/swap_voter_target.rs | 1 + .../tests/ui/fail/wrong_attribute.rs | 1 + 16 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs create mode 100644 frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr diff --git a/Cargo.lock b/Cargo.lock index fce6397b5a91f..19becb047cf0e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2182,6 +2182,7 @@ version = "2.0.0-alpha.5" dependencies = [ "clap 3.0.7", "frame-npos-elections-solution-type", + "frame-support", "honggfuzz", "parity-scale-codec", "rand 0.8.4", @@ -2227,6 +2228,7 @@ dependencies = [ name = "frame-npos-elections-solution-type" version = "4.0.0-dev" dependencies = [ + "frame-support", "parity-scale-codec", "proc-macro-crate 1.1.0", "proc-macro2", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index cf38123080478..d205437a8b5a8 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -594,11 +594,13 @@ frame_npos_elections_solution_type::generate_solution_type!( VoterIndex = u32, TargetIndex = u16, Accuracy = sp_runtime::PerU16, + SizeBound = VoterSnapshotPerBlock, >(16) ); parameter_types! { pub MaxNominations: u32 = ::LIMIT as u32; + pub VoterSnapshotPerBlock: u32 = 10_000; } /// The numbers configured here could always be more than the the maximum limits of staking pallet @@ -681,7 +683,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime { // BagsList allows a practically unbounded count of nominators to participate in NPoS elections. // To ensure we respect memory limits when using the BagsList this must be set to a number of // voters we know can fit into a single vec allocation. - type VoterSnapshotPerBlock = ConstU32<10_000>; + type VoterSnapshotPerBlock = VoterSnapshotPerBlock; } parameter_types! { diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index ef54792b12fa9..611bd75fe2653 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -70,7 +70,12 @@ pub(crate) type TargetIndex = u16; frame_npos_elections_solution_type::generate_solution_type!( #[compact] - pub struct TestNposSolution::(16) + pub struct TestNposSolution::< + VoterIndex = VoterIndex, + TargetIndex = TargetIndex, + Accuracy = PerU16, + SizeBound = ConstU32::<10_000> //TODO bound size? + >(16) ); /// All events of this pallet. diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index 20c618dbd4a5f..e11226388c3d9 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -26,4 +26,5 @@ scale-info = "2.0.1" sp-arithmetic = { version = "5.0.0", path = "../../../primitives/arithmetic" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", path = "../../../primitives/npos-elections" } +frame-support = { version = "4.0.0-dev", path = "../../support" } trybuild = "1.0.53" diff --git a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml index 1b5893f6c94bb..659680d57860e 100644 --- a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml +++ b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml @@ -24,6 +24,7 @@ sp-arithmetic = { version = "5.0.0", path = "../../../../primitives/arithmetic" sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../../primitives/npos-elections" } +frame-support = { version = "4.0.0-dev", path = "../../../support" } [[bin]] name = "compact" diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index ae40532d71ded..592e85fb14aba 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -1,13 +1,14 @@ +use frame_npos_elections_solution_type::generate_solution_type; use honggfuzz::fuzz; use sp_arithmetic::Percent; use sp_runtime::codec::{Encode, Error}; -use frame_npos_elections_solution_type::generate_solution_type; fn main() { generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::< VoterIndex = u32, TargetIndex = u32, Accuracy = Percent, + SizeBound = frame_support::traits::ConstU32::<100_000>, //TODO: unsure about size >(16)); loop { fuzz!(|fuzzer_data: &[u8]| { diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 9bed21cfc2e8f..c3b768e43851d 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,6 +49,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. +/// - The maximum number of votes per snapshot. This must be of type `Get`. /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. @@ -59,10 +60,12 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// ``` /// # use frame_npos_elections_solution_type::generate_solution_type; /// # use sp_arithmetic::per_things::Perbill; +/// # use frame_support::traits::ConstU32; /// generate_solution_type!(pub struct TestSolution::< /// VoterIndex = u16, /// TargetIndex = u8, /// Accuracy = Perbill, +/// SizeBound = ConstU32::<10>, /// >(4)); /// ``` /// @@ -103,9 +106,15 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// # use frame_npos_elections_solution_type::generate_solution_type; /// # use sp_npos_elections::NposSolution; /// # use sp_arithmetic::per_things::Perbill; +/// # use frame_support::traits::ConstU32; /// generate_solution_type!( /// #[compact] -/// pub struct TestSolutionCompact::(8) +/// pub struct TestSolutionCompact::< +/// VoterIndex = u16, +/// TargetIndex = u8, +/// Accuracy = Perbill, +/// SizeBound = ConstU32::<10>, +/// >(8) /// ); /// ``` #[proc_macro] @@ -129,6 +138,7 @@ struct SolutionDef { voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, + size_bound: syn::Type, count: usize, compact_encoding: bool, } @@ -167,11 +177,11 @@ impl Parse for SolutionDef { let _ = ::parse(input)?; let generics: syn::AngleBracketedGenericArguments = input.parse()?; - if generics.args.len() != 3 { - return Err(syn_err("Must provide 3 generic args.")) + if generics.args.len() != 4 { + return Err(syn_err("Must provide 4 generic args.")) } - let expected_types = ["VoterIndex", "TargetIndex", "Accuracy"]; + let expected_types = ["VoterIndex", "TargetIndex", "Accuracy", "SizeBound"]; let mut types: Vec = generics .args @@ -197,6 +207,7 @@ impl Parse for SolutionDef { }) .collect::>()?; + let size_bound = types.pop().expect("Vector of length 4 can be popped; qed"); let weight_type = types.pop().expect("Vector of length 3 can be popped; qed"); let target_type = types.pop().expect("Vector of length 2 can be popped; qed"); let voter_type = types.pop().expect("Vector of length 1 can be popped; qed"); @@ -205,7 +216,16 @@ impl Parse for SolutionDef { let count_expr: syn::ExprParen = input.parse()?; let count = parse_parenthesized_number::(count_expr)?; - Ok(Self { vis, ident, voter_type, target_type, weight_type, count, compact_encoding }) + Ok(Self { + vis, + ident, + voter_type, + target_type, + weight_type, + size_bound, + count, + compact_encoding, + }) } } diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 01a8a8eba5dcc..764d974e744e5 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -28,6 +28,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { voter_type, target_type, weight_type, + size_bound, compact_encoding, } = def; @@ -178,6 +179,13 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { <#ident as _npos::NposSolution>::TargetIndex, <#ident as _npos::NposSolution>::Accuracy, >; + impl _npos::codec::MaxEncodedLen for #ident { + fn max_encoded_len() -> usize { + use frame_support::traits::Get; + let s: u32 = #size_bound::get(); + s as usize + } + } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { type Error = _npos::Error; fn try_from(index_assignments: &'a [__IndexAssignment]) -> Result { diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index 47ec86b85220a..3613b024a3a56 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs new file mode 100644 index 0000000000000..0ea93f0fa6306 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs @@ -0,0 +1,10 @@ +use frame_npos_elections_solution_type::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + VoterIndex = u16, + TargetIndex = u8, + Accuracy = Perbill, + frame_support::traits::ConstU32::<10>, +>(8)); + +fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr new file mode 100644 index 0000000000000..8dc3ffa46f6e7 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `SizeBound = ...` + --> tests/ui/fail/missing_size_bound.rs:7:2 + | +7 | frame_support::traits::ConstU32::<10>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index 17727ad8f8286..9ec699709a127 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index b735477d8cdc8..451d02ea2aee1 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, TargetIndex = u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index 4a1445591527d..addc67ee14f86 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, u8, Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index bbb5c88568752..72f280fe2648e 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, VoterIndex = u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index 3e4786ede7674..995b5006d1664 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -5,6 +5,7 @@ generate_solution_type!( VoterIndex = u8, TargetIndex = u16, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8) ); From 4dac1d2d4f2636e1e9091b799d569839cb3d5236 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 8 Mar 2022 22:20:48 +0000 Subject: [PATCH 06/26] Full implementation of `max_encoded_len` --- .../solution-type/src/single_page.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 764d974e744e5..8434c6545a73b 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -182,8 +182,14 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { impl _npos::codec::MaxEncodedLen for #ident { fn max_encoded_len() -> usize { use frame_support::traits::Get; + use _npos::codec::Encode; let s: u32 = #size_bound::get(); - s as usize + let max_element_size = #voter_type::max_encoded_len() + .max(#target_type::max_encoded_len() + .max(#weight_type::max_encoded_len())); + #count.saturating_mul( + _npos::codec::Compact(s).encoded_size() + .saturating_add((s as usize).saturating_mul(max_element_size))) } } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { From 38dc2fa79bc5696015b38cb5a88791ea68fe0761 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 8 Mar 2022 22:53:06 +0000 Subject: [PATCH 07/26] Fixing implementation bug adding some comments and documentation --- .../solution-type/src/lib.rs | 3 ++- .../solution-type/src/single_page.rs | 16 +++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index c3b768e43851d..2b28d6596bfc9 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,7 +49,8 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. -/// - The maximum number of votes per snapshot. This must be of type `Get`. +/// - The maximum number of voters per snapshot. This must be of type `Get`. Check +/// for more details /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 8434c6545a73b..d960bfe954e0a 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -184,12 +184,18 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { use frame_support::traits::Get; use _npos::codec::Encode; let s: u32 = #size_bound::get(); + // The last element of the struct is a vec with 1 voter + // then #count-1 tuple of target with an accuracy + // and then lastly the final target let max_element_size = #voter_type::max_encoded_len() - .max(#target_type::max_encoded_len() - .max(#weight_type::max_encoded_len())); - #count.saturating_mul( - _npos::codec::Compact(s).encoded_size() - .saturating_add((s as usize).saturating_mul(max_element_size))) + .saturating_add((#count - 1) + .saturating_mul(#target_type::max_encoded_len() + .saturating_add(#weight_type::max_encoded_len()))) + .saturating_add(#target_type::max_encoded_len()); + // The assumption is that it contains #count-1 empty elements + // and then last element with full size + #count.saturating_mul(_npos::codec::Compact(0u32).encoded_size()) + .saturating_add((s as usize).saturating_mul(max_element_size)) } } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { From bb71738a460f712df709c9c3692abfa08251a8b0 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Fri, 11 Mar 2022 21:28:10 +0000 Subject: [PATCH 08/26] fmt --- .../solution-type/fuzzer/src/compact.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index ae40532d71ded..0ac4ae2d7ef68 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -1,7 +1,7 @@ +use frame_npos_elections_solution_type::generate_solution_type; use honggfuzz::fuzz; use sp_arithmetic::Percent; use sp_runtime::codec::{Encode, Error}; -use frame_npos_elections_solution_type::generate_solution_type; fn main() { generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::< From 4ba1eea9b58ff290cc0b5d34c76676b197bd4a1b Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sat, 12 Mar 2022 10:30:56 +0000 Subject: [PATCH 09/26] Committing suggested changes renaming, and re exporting macro. --- Cargo.lock | 35 +++++++++---------- bin/node/runtime/Cargo.toml | 1 - bin/node/runtime/src/lib.rs | 2 +- .../election-provider-multi-phase/Cargo.toml | 3 +- .../election-provider-multi-phase/src/mock.rs | 2 +- frame/election-provider-support/Cargo.toml | 1 + .../solution-type/Cargo.toml | 2 +- .../solution-type/fuzzer/Cargo.toml | 2 +- .../solution-type/fuzzer/src/compact.rs | 2 +- .../solution-type/src/lib.rs | 4 +-- .../tests/ui/fail/missing_accuracy.rs | 2 +- .../tests/ui/fail/missing_target.rs | 2 +- .../tests/ui/fail/missing_voter.rs | 2 +- .../tests/ui/fail/no_annotations.rs | 2 +- .../tests/ui/fail/swap_voter_target.rs | 2 +- .../tests/ui/fail/wrong_attribute.rs | 2 +- frame/election-provider-support/src/lib.rs | 1 + frame/staking/src/lib.rs | 6 ++-- primitives/npos-elections/src/tests.rs | 2 -- 19 files changed, 36 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fce6397b5a91f..b87b9cc15942f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2160,10 +2160,26 @@ dependencies = [ "sp-trie", ] +[[package]] +name = "frame-election-provider-solution-type" +version = "4.0.0-dev" +dependencies = [ + "parity-scale-codec", + "proc-macro-crate 1.1.0", + "proc-macro2", + "quote", + "scale-info", + "sp-arithmetic", + "sp-npos-elections", + "syn", + "trybuild", +] + [[package]] name = "frame-election-provider-support" version = "4.0.0-dev" dependencies = [ + "frame-election-provider-solution-type", "frame-support", "frame-system", "parity-scale-codec", @@ -2181,7 +2197,7 @@ name = "frame-election-solution-type-fuzzer" version = "2.0.0-alpha.5" dependencies = [ "clap 3.0.7", - "frame-npos-elections-solution-type", + "frame-election-provider-solution-type", "honggfuzz", "parity-scale-codec", "rand 0.8.4", @@ -2223,21 +2239,6 @@ dependencies = [ "serde", ] -[[package]] -name = "frame-npos-elections-solution-type" -version = "4.0.0-dev" -dependencies = [ - "parity-scale-codec", - "proc-macro-crate 1.1.0", - "proc-macro2", - "quote", - "scale-info", - "sp-arithmetic", - "sp-npos-elections", - "syn", - "trybuild", -] - [[package]] name = "frame-support" version = "4.0.0-dev" @@ -4995,7 +4996,6 @@ dependencies = [ "frame-benchmarking", "frame-election-provider-support", "frame-executive", - "frame-npos-elections-solution-type", "frame-support", "frame-system", "frame-system-benchmarking", @@ -5865,7 +5865,6 @@ version = "4.0.0-dev" dependencies = [ "frame-benchmarking", "frame-election-provider-support", - "frame-npos-elections-solution-type", "frame-support", "frame-system", "log 0.4.14", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index b9ee7d7d391c0..0572345fa0c89 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -49,7 +49,6 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../.. frame-system = { version = "4.0.0-dev", default-features = false, path = "../../../frame/system" } frame-system-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../../../frame/system/benchmarking", optional = true } frame-election-provider-support = { version = "4.0.0-dev", default-features = false, path = "../../../frame/election-provider-support" } -frame-npos-elections-solution-type = { version = "4.0.0-dev", default-features = false, path = "../../../frame/election-provider-support/solution-type" } frame-system-rpc-runtime-api = { version = "4.0.0-dev", default-features = false, path = "../../../frame/system/rpc/runtime-api/" } frame-try-runtime = { version = "0.10.0-dev", default-features = false, path = "../../../frame/try-runtime", optional = true } pallet-assets = { version = "4.0.0-dev", default-features = false, path = "../../../frame/assets" } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 47e2f11b18649..20b718e2fa8f7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -588,7 +588,7 @@ parameter_types! { .get(DispatchClass::Normal); } -frame_npos_elections_solution_type::generate_solution_type!( +frame_election_provider_support::generate_solution_type!( #[compact] pub struct NposSolution16::< VoterIndex = u32, diff --git a/frame/election-provider-multi-phase/Cargo.toml b/frame/election-provider-multi-phase/Cargo.toml index bca40045dc5ed..25f98d965d86b 100644 --- a/frame/election-provider-multi-phase/Cargo.toml +++ b/frame/election-provider-multi-phase/Cargo.toml @@ -46,8 +46,7 @@ sp-core = { version = "6.0.0", default-features = false, path = "../../primitive sp-io = { version = "6.0.0", path = "../../primitives/io" } sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../primitives/npos-elections" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } -frame-election-provider-support = { version = "4.0.0-dev", features = [], path = "../election-provider-support" } -frame-npos-elections-solution-type = { version = "4.0.0-dev", default-features = false, path = "../election-provider-support/solution-type" } +frame-election-provider-support = { version = "4.0.0-dev", path = "../election-provider-support" } pallet-balances = { version = "4.0.0-dev", path = "../balances" } frame-benchmarking = { version = "4.0.0-dev", path = "../benchmarking" } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index ef54792b12fa9..7c4ef5d8055c3 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -68,7 +68,7 @@ pub(crate) type BlockNumber = u64; pub(crate) type VoterIndex = u32; pub(crate) type TargetIndex = u16; -frame_npos_elections_solution_type::generate_solution_type!( +frame_election_provider_support::generate_solution_type!( #[compact] pub struct TestNposSolution::(16) ); diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index b95bd994fa70a..16b79dbb098d4 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -21,6 +21,7 @@ sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = ". sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } +frame-election-provider-solution-type = { version = "4.0.0-dev", path = "solution-type" } [dev-dependencies] sp-npos-elections = { version = "4.0.0-dev", path = "../../primitives/npos-elections" } diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index 20c618dbd4a5f..d489c7b4c10de 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "frame-npos-elections-solution-type" +name = "frame-election-provider-solution-type" version = "4.0.0-dev" authors = ["Parity Technologies "] edition = "2021" diff --git a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml index 1b5893f6c94bb..f52b7f7332620 100644 --- a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml +++ b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml @@ -19,7 +19,7 @@ rand = { version = "0.8", features = ["std", "small_rng"] } codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } -frame-npos-elections-solution-type = { version = "4.0.0-dev", path = ".." } +frame-election-provider-solution-type = { version = "4.0.0-dev", path = ".." } sp-arithmetic = { version = "5.0.0", path = "../../../../primitives/arithmetic" } sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } # used by generate_solution_type: diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index 0ac4ae2d7ef68..501d241b2b80b 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; use honggfuzz::fuzz; use sp_arithmetic::Percent; use sp_runtime::codec::{Encode, Error}; diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 9bed21cfc2e8f..3de923cdffd03 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -57,7 +57,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// type, `u8` target type and `Perbill` accuracy with maximum of 4 edges per voter. /// /// ``` -/// # use frame_npos_elections_solution_type::generate_solution_type; +/// # use frame_election_provider_solution_type::generate_solution_type; /// # use sp_arithmetic::per_things::Perbill; /// generate_solution_type!(pub struct TestSolution::< /// VoterIndex = u16, @@ -100,7 +100,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// for numbers will be used, similar to how `parity-scale-codec`'s `Compact` works. /// /// ``` -/// # use frame_npos_elections_solution_type::generate_solution_type; +/// # use frame_election_provider_solution_type::generate_solution_type; /// # use sp_npos_elections::NposSolution; /// # use sp_arithmetic::per_things::Perbill; /// generate_solution_type!( diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index 47ec86b85220a..22693cd875e17 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index 17727ad8f8286..8d0ca927c5fb3 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index b735477d8cdc8..ad4b7f5217794 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index 4a1445591527d..87673a3823513 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index bbb5c88568752..f1d5d0e7bf99f 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index 3e4786ede7674..d04cc4a7a966b 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!( #[pages(1)] pub struct TestSolution::< diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index ff57eacf68fa0..1bd10ba09346f 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -172,6 +172,7 @@ use sp_runtime::traits::Bounded; use sp_std::{fmt::Debug, prelude::*}; /// Re-export some type as they are used in the interface. +pub use frame_election_provider_solution_type::generate_solution_type; pub use sp_arithmetic::PerThing; pub use sp_npos_elections::{ Assignment, ElectionResult, ExtendedBalance, IdentifierT, PerThing128, Support, Supports, diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index d833ac86fe0bd..4c1bb438457e5 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -302,7 +302,7 @@ mod pallet; use codec::{Decode, Encode, HasCompact}; use frame_support::{ parameter_types, - traits::{ConstU32, Currency, Get}, + traits::{Currency, Get}, weights::Weight, BoundedVec, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; @@ -861,6 +861,6 @@ pub struct TestBenchmarkingConfig; #[cfg(feature = "std")] impl BenchmarkingConfig for TestBenchmarkingConfig { - type MaxValidators = ConstU32<100>; - type MaxNominators = ConstU32<100>; + type MaxValidators = frame_support::traits::ConstU32<100>; + type MaxNominators = frame_support::traits::ConstU32<100>; } diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index b0f8f16ce90fb..1cf5ea8a24920 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -17,8 +17,6 @@ //! Tests for npos-elections. -#![cfg(test)] - use crate::{ balancing, helpers::*, mock::*, seq_phragmen, seq_phragmen_core, setup_inputs, to_support_map, Assignment, ElectionResult, ExtendedBalance, StakedAssignment, Support, Voter, From a076c9b536c41da14d0365ccdc53c2129d8d9039 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sat, 12 Mar 2022 10:37:26 +0000 Subject: [PATCH 10/26] Removing unneeded imports --- primitives/npos-elections/src/phragmms.rs | 2 +- primitives/npos-elections/src/reduce.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/primitives/npos-elections/src/phragmms.rs b/primitives/npos-elections/src/phragmms.rs index fff318f590fc6..6220cacd157b2 100644 --- a/primitives/npos-elections/src/phragmms.rs +++ b/primitives/npos-elections/src/phragmms.rs @@ -26,7 +26,7 @@ use crate::{ VoteWeight, Voter, }; use sp_arithmetic::{traits::Bounded, PerThing, Rational128}; -use sp_std::{prelude::*, rc::Rc, vec}; +use sp_std::{prelude::*, rc::Rc}; /// Execute the phragmms method. /// diff --git a/primitives/npos-elections/src/reduce.rs b/primitives/npos-elections/src/reduce.rs index c5860f7ee41ee..f089a37e3fff3 100644 --- a/primitives/npos-elections/src/reduce.rs +++ b/primitives/npos-elections/src/reduce.rs @@ -55,7 +55,6 @@ use sp_arithmetic::traits::{Bounded, Zero}; use sp_std::{ collections::btree_map::{BTreeMap, Entry::*}, prelude::*, - vec, }; /// Map type used for reduce_4. Can be easily swapped with HashMap. From 505f5aaecedc6c63b4f19f7837a2191e007102c1 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Mon, 7 Mar 2022 23:18:44 +0000 Subject: [PATCH 11/26] Implementing `MaxEncodedLen` on `generate_solution_type` --- Cargo.lock | 2 ++ bin/node/runtime/src/lib.rs | 4 ++- .../election-provider-multi-phase/src/mock.rs | 7 ++++- .../solution-type/Cargo.toml | 1 + .../solution-type/fuzzer/Cargo.toml | 1 + .../solution-type/fuzzer/src/compact.rs | 1 + .../solution-type/src/lib.rs | 30 +++++++++++++++---- .../solution-type/src/single_page.rs | 8 +++++ .../tests/ui/fail/missing_accuracy.rs | 1 + .../tests/ui/fail/missing_size_bound.rs | 10 +++++++ .../tests/ui/fail/missing_size_bound.stderr | 5 ++++ .../tests/ui/fail/missing_target.rs | 1 + .../tests/ui/fail/missing_voter.rs | 1 + .../tests/ui/fail/no_annotations.rs | 1 + .../tests/ui/fail/swap_voter_target.rs | 1 + .../tests/ui/fail/wrong_attribute.rs | 1 + 16 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs create mode 100644 frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr diff --git a/Cargo.lock b/Cargo.lock index b87b9cc15942f..3a560c84646c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2164,6 +2164,7 @@ dependencies = [ name = "frame-election-provider-solution-type" version = "4.0.0-dev" dependencies = [ + "frame-support", "parity-scale-codec", "proc-macro-crate 1.1.0", "proc-macro2", @@ -2198,6 +2199,7 @@ version = "2.0.0-alpha.5" dependencies = [ "clap 3.0.7", "frame-election-provider-solution-type", + "frame-support", "honggfuzz", "parity-scale-codec", "rand 0.8.4", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 20b718e2fa8f7..62257ee3cfbed 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -594,11 +594,13 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = u32, TargetIndex = u16, Accuracy = sp_runtime::PerU16, + SizeBound = VoterSnapshotPerBlock, >(16) ); parameter_types! { pub MaxNominations: u32 = ::LIMIT as u32; + pub VoterSnapshotPerBlock: u32 = 10_000; } /// The numbers configured here could always be more than the the maximum limits of staking pallet @@ -681,7 +683,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime { // BagsList allows a practically unbounded count of nominators to participate in NPoS elections. // To ensure we respect memory limits when using the BagsList this must be set to a number of // voters we know can fit into a single vec allocation. - type VoterSnapshotPerBlock = ConstU32<10_000>; + type VoterSnapshotPerBlock = VoterSnapshotPerBlock; } parameter_types! { diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 7c4ef5d8055c3..f9ff8abb52586 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -70,7 +70,12 @@ pub(crate) type TargetIndex = u16; frame_election_provider_support::generate_solution_type!( #[compact] - pub struct TestNposSolution::(16) + pub struct TestNposSolution::< + VoterIndex = VoterIndex, + TargetIndex = TargetIndex, + Accuracy = PerU16, + SizeBound = ConstU32::<10_000> //TODO bound size? + >(16) ); /// All events of this pallet. diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index d489c7b4c10de..54b43659c7281 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -26,4 +26,5 @@ scale-info = "2.0.1" sp-arithmetic = { version = "5.0.0", path = "../../../primitives/arithmetic" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", path = "../../../primitives/npos-elections" } +frame-support = { version = "4.0.0-dev", path = "../../support" } trybuild = "1.0.53" diff --git a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml index f52b7f7332620..f4d45863233ca 100644 --- a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml +++ b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml @@ -24,6 +24,7 @@ sp-arithmetic = { version = "5.0.0", path = "../../../../primitives/arithmetic" sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../../primitives/npos-elections" } +frame-support = { version = "4.0.0-dev", path = "../../../support" } [[bin]] name = "compact" diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index 501d241b2b80b..a7c7c5d256f74 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -8,6 +8,7 @@ fn main() { VoterIndex = u32, TargetIndex = u32, Accuracy = Percent, + SizeBound = frame_support::traits::ConstU32::<100_000>, //TODO: unsure about size >(16)); loop { fuzz!(|fuzzer_data: &[u8]| { diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 3de923cdffd03..001353b96a4a6 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,6 +49,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. +/// - The maximum number of votes per snapshot. This must be of type `Get`. /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. @@ -59,10 +60,12 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// ``` /// # use frame_election_provider_solution_type::generate_solution_type; /// # use sp_arithmetic::per_things::Perbill; +/// # use frame_support::traits::ConstU32; /// generate_solution_type!(pub struct TestSolution::< /// VoterIndex = u16, /// TargetIndex = u8, /// Accuracy = Perbill, +/// SizeBound = ConstU32::<10>, /// >(4)); /// ``` /// @@ -103,9 +106,15 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// # use frame_election_provider_solution_type::generate_solution_type; /// # use sp_npos_elections::NposSolution; /// # use sp_arithmetic::per_things::Perbill; +/// # use frame_support::traits::ConstU32; /// generate_solution_type!( /// #[compact] -/// pub struct TestSolutionCompact::(8) +/// pub struct TestSolutionCompact::< +/// VoterIndex = u16, +/// TargetIndex = u8, +/// Accuracy = Perbill, +/// SizeBound = ConstU32::<10>, +/// >(8) /// ); /// ``` #[proc_macro] @@ -129,6 +138,7 @@ struct SolutionDef { voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, + size_bound: syn::Type, count: usize, compact_encoding: bool, } @@ -167,11 +177,11 @@ impl Parse for SolutionDef { let _ = ::parse(input)?; let generics: syn::AngleBracketedGenericArguments = input.parse()?; - if generics.args.len() != 3 { - return Err(syn_err("Must provide 3 generic args.")) + if generics.args.len() != 4 { + return Err(syn_err("Must provide 4 generic args.")) } - let expected_types = ["VoterIndex", "TargetIndex", "Accuracy"]; + let expected_types = ["VoterIndex", "TargetIndex", "Accuracy", "SizeBound"]; let mut types: Vec = generics .args @@ -197,6 +207,7 @@ impl Parse for SolutionDef { }) .collect::>()?; + let size_bound = types.pop().expect("Vector of length 4 can be popped; qed"); let weight_type = types.pop().expect("Vector of length 3 can be popped; qed"); let target_type = types.pop().expect("Vector of length 2 can be popped; qed"); let voter_type = types.pop().expect("Vector of length 1 can be popped; qed"); @@ -205,7 +216,16 @@ impl Parse for SolutionDef { let count_expr: syn::ExprParen = input.parse()?; let count = parse_parenthesized_number::(count_expr)?; - Ok(Self { vis, ident, voter_type, target_type, weight_type, count, compact_encoding }) + Ok(Self { + vis, + ident, + voter_type, + target_type, + weight_type, + size_bound, + count, + compact_encoding, + }) } } diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 01a8a8eba5dcc..764d974e744e5 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -28,6 +28,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { voter_type, target_type, weight_type, + size_bound, compact_encoding, } = def; @@ -178,6 +179,13 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { <#ident as _npos::NposSolution>::TargetIndex, <#ident as _npos::NposSolution>::Accuracy, >; + impl _npos::codec::MaxEncodedLen for #ident { + fn max_encoded_len() -> usize { + use frame_support::traits::Get; + let s: u32 = #size_bound::get(); + s as usize + } + } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { type Error = _npos::Error; fn try_from(index_assignments: &'a [__IndexAssignment]) -> Result { diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index 22693cd875e17..9a5b48d3af7cc 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs new file mode 100644 index 0000000000000..0ea93f0fa6306 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs @@ -0,0 +1,10 @@ +use frame_npos_elections_solution_type::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + VoterIndex = u16, + TargetIndex = u8, + Accuracy = Perbill, + frame_support::traits::ConstU32::<10>, +>(8)); + +fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr new file mode 100644 index 0000000000000..8dc3ffa46f6e7 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `SizeBound = ...` + --> tests/ui/fail/missing_size_bound.rs:7:2 + | +7 | frame_support::traits::ConstU32::<10>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index 8d0ca927c5fb3..c6231074aa935 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index ad4b7f5217794..c21a05ec9cd95 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, TargetIndex = u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index 87673a3823513..9442cda0d983b 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, u8, Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index f1d5d0e7bf99f..b17d24aaabeef 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, VoterIndex = u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index d04cc4a7a966b..86c54e9d7d957 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -5,6 +5,7 @@ generate_solution_type!( VoterIndex = u8, TargetIndex = u16, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8) ); From dff988da15000d4b512616a3d88f993807023a41 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 8 Mar 2022 22:20:48 +0000 Subject: [PATCH 12/26] Full implementation of `max_encoded_len` --- .../solution-type/src/single_page.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 764d974e744e5..8434c6545a73b 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -182,8 +182,14 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { impl _npos::codec::MaxEncodedLen for #ident { fn max_encoded_len() -> usize { use frame_support::traits::Get; + use _npos::codec::Encode; let s: u32 = #size_bound::get(); - s as usize + let max_element_size = #voter_type::max_encoded_len() + .max(#target_type::max_encoded_len() + .max(#weight_type::max_encoded_len())); + #count.saturating_mul( + _npos::codec::Compact(s).encoded_size() + .saturating_add((s as usize).saturating_mul(max_element_size))) } } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { From 4c37fbbb4171fc8eb92f3aeeb668aec306dde262 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 8 Mar 2022 22:53:06 +0000 Subject: [PATCH 13/26] Fixing implementation bug adding some comments and documentation --- .../solution-type/src/lib.rs | 3 ++- .../solution-type/src/single_page.rs | 16 +++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 001353b96a4a6..b2bd0af4f66c3 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,7 +49,8 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. -/// - The maximum number of votes per snapshot. This must be of type `Get`. +/// - The maximum number of voters per snapshot. This must be of type `Get`. Check +/// for more details /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 8434c6545a73b..d960bfe954e0a 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -184,12 +184,18 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { use frame_support::traits::Get; use _npos::codec::Encode; let s: u32 = #size_bound::get(); + // The last element of the struct is a vec with 1 voter + // then #count-1 tuple of target with an accuracy + // and then lastly the final target let max_element_size = #voter_type::max_encoded_len() - .max(#target_type::max_encoded_len() - .max(#weight_type::max_encoded_len())); - #count.saturating_mul( - _npos::codec::Compact(s).encoded_size() - .saturating_add((s as usize).saturating_mul(max_element_size))) + .saturating_add((#count - 1) + .saturating_mul(#target_type::max_encoded_len() + .saturating_add(#weight_type::max_encoded_len()))) + .saturating_add(#target_type::max_encoded_len()); + // The assumption is that it contains #count-1 empty elements + // and then last element with full size + #count.saturating_mul(_npos::codec::Compact(0u32).encoded_size()) + .saturating_add((s as usize).saturating_mul(max_element_size)) } } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { From 45d778c63ddd58276f32249b7c43422f97039c37 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 13 Mar 2022 18:18:00 +0000 Subject: [PATCH 14/26] Move `NposSolution` to frame --- Cargo.lock | 2 + bin/node/runtime/src/lib.rs | 2 +- .../election-provider-multi-phase/src/lib.rs | 5 +- .../election-provider-multi-phase/src/mock.rs | 4 +- .../src/signed.rs | 3 +- .../src/unsigned.rs | 6 +- .../solution-type/Cargo.toml | 1 + .../solution-type/fuzzer/Cargo.toml | 1 + .../solution-type/src/lib.rs | 2 +- .../solution-type/src/single_page.rs | 8 +- frame/election-provider-support/src/lib.rs | 2 + frame/election-provider-support/src/traits.rs | 136 ++++++++++++++++++ primitives/npos-elections/src/assignments.rs | 7 - primitives/npos-elections/src/lib.rs | 4 +- primitives/npos-elections/src/traits.rs | 118 +-------------- 15 files changed, 162 insertions(+), 139 deletions(-) create mode 100644 frame/election-provider-support/src/traits.rs diff --git a/Cargo.lock b/Cargo.lock index b87b9cc15942f..eba761c07ebee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2164,6 +2164,7 @@ dependencies = [ name = "frame-election-provider-solution-type" version = "4.0.0-dev" dependencies = [ + "frame-election-provider-support", "parity-scale-codec", "proc-macro-crate 1.1.0", "proc-macro2", @@ -2198,6 +2199,7 @@ version = "2.0.0-alpha.5" dependencies = [ "clap 3.0.7", "frame-election-provider-solution-type", + "frame-election-provider-support", "honggfuzz", "parity-scale-codec", "rand 0.8.4", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 20b718e2fa8f7..ae0809b1475ed 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -598,7 +598,7 @@ frame_election_provider_support::generate_solution_type!( ); parameter_types! { - pub MaxNominations: u32 = ::LIMIT as u32; + pub MaxNominations: u32 = ::LIMIT as u32; } /// The numbers configured here could always be more than the the maximum limits of staking pallet diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 7e211c5ee9211..b366ceb4dafe3 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -231,7 +231,7 @@ use codec::{Decode, Encode}; use frame_election_provider_support::{ - ElectionDataProvider, ElectionProvider, InstantElectionProvider, + ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolution, }; use frame_support::{ dispatch::DispatchResultWithPostInfo, @@ -246,8 +246,7 @@ use sp_arithmetic::{ UpperOf, }; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, NposSolution, Supports, - VoteWeight, + assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, Supports, VoteWeight, }; use sp_runtime::{ traits::Bounded, diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 7c4ef5d8055c3..d09d45784cba4 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -18,7 +18,7 @@ use super::*; use crate as multi_phase; use frame_election_provider_support::{ - data_provider, onchain, ElectionDataProvider, SequentialPhragmen, + data_provider, onchain, ElectionDataProvider, NposSolution, SequentialPhragmen, }; pub use frame_support::{assert_noop, assert_ok}; use frame_support::{ @@ -38,7 +38,7 @@ use sp_core::{ }; use sp_npos_elections::{ assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, ElectionResult, - EvaluateSupport, ExtendedBalance, NposSolution, + EvaluateSupport, ExtendedBalance, }; use sp_runtime::{ testing::Header, diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index a233346b4fd77..82b40e3276036 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -23,12 +23,13 @@ use crate::{ SolutionOrSnapshotSize, Weight, WeightInfo, }; use codec::{Decode, Encode, HasCompact}; +use frame_election_provider_support::NposSolution; use frame_support::{ storage::bounded_btree_map::BoundedBTreeMap, traits::{defensive_prelude::*, Currency, Get, OnUnbalanced, ReservableCurrency}, }; use sp_arithmetic::traits::SaturatedConversion; -use sp_npos_elections::{ElectionScore, NposSolution}; +use sp_npos_elections::ElectionScore; use sp_runtime::{ traits::{Saturating, Zero}, RuntimeDebug, diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 81ea4453d0964..623cf62d6fffe 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -23,12 +23,11 @@ use crate::{ WeightInfo, }; use codec::Encode; -use frame_election_provider_support::{NposSolver, PerThing128}; +use frame_election_provider_support::{NposSolution, NposSolver, PerThing128}; use frame_support::{dispatch::DispatchResult, ensure, traits::Get}; use frame_system::offchain::SubmitTransaction; use sp_npos_elections::{ assignment_ratio_to_staked_normalized, assignment_staked_to_ratio_normalized, ElectionResult, - NposSolution, }; use sp_runtime::{ offchain::storage::{MutateStorageError, StorageValueRef}, @@ -54,7 +53,8 @@ pub type Assignment = /// The [`IndexAssignment`][sp_npos_elections::IndexAssignment] type specialized for a particular /// runtime `T`. -pub type IndexAssignmentOf = sp_npos_elections::IndexAssignmentOf>; +pub type IndexAssignmentOf = + frame_election_provider_support::traits::IndexAssignmentOf>; /// Error type of the pallet's [`crate::Config::Solver`]. pub type SolverErrorOf = <::Solver as NposSolver>::Error; diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index d489c7b4c10de..08c33f20bb259 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -26,4 +26,5 @@ scale-info = "2.0.1" sp-arithmetic = { version = "5.0.0", path = "../../../primitives/arithmetic" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", path = "../../../primitives/npos-elections" } +frame-election-provider-support = { version = "4.0.0-dev", path = ".." } trybuild = "1.0.53" diff --git a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml index f52b7f7332620..f6c2f2fe491e8 100644 --- a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml +++ b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml @@ -20,6 +20,7 @@ rand = { version = "0.8", features = ["std", "small_rng"] } codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } scale-info = { version = "2.0.1", default-features = false, features = ["derive"] } frame-election-provider-solution-type = { version = "4.0.0-dev", path = ".." } +frame-election-provider-support = { version = "4.0.0-dev", path = "../.." } sp-arithmetic = { version = "5.0.0", path = "../../../../primitives/arithmetic" } sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } # used by generate_solution_type: diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 3de923cdffd03..8081d77ae8205 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -101,7 +101,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// /// ``` /// # use frame_election_provider_solution_type::generate_solution_type; -/// # use sp_npos_elections::NposSolution; +/// # use frame_election_provider_support::NposSolution; /// # use sp_arithmetic::per_things::Perbill; /// generate_solution_type!( /// #[compact] diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 01a8a8eba5dcc..6ef9dea0100aa 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -102,7 +102,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { #vis struct #ident { #single #rest } use _npos::__OrInvalidIndex; - impl _npos::NposSolution for #ident { + impl frame_election_provider_support::NposSolution for #ident { const LIMIT: usize = #count; type VoterIndex = #voter_type; type TargetIndex = #target_type; @@ -174,9 +174,9 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { } type __IndexAssignment = _npos::IndexAssignment< - <#ident as _npos::NposSolution>::VoterIndex, - <#ident as _npos::NposSolution>::TargetIndex, - <#ident as _npos::NposSolution>::Accuracy, + <#ident as frame_election_provider_support::NposSolution>::VoterIndex, + <#ident as frame_election_provider_support::NposSolution>::TargetIndex, + <#ident as frame_election_provider_support::NposSolution>::Accuracy, >; impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { type Error = _npos::Error; diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 1bd10ba09346f..b720acd1d0c2b 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -167,6 +167,7 @@ #![cfg_attr(not(feature = "std"), no_std)] pub mod onchain; +pub mod traits; use frame_support::{traits::Get, BoundedVec}; use sp_runtime::traits::Bounded; use sp_std::{fmt::Debug, prelude::*}; @@ -178,6 +179,7 @@ pub use sp_npos_elections::{ Assignment, ElectionResult, ExtendedBalance, IdentifierT, PerThing128, Support, Supports, VoteWeight, }; +pub use traits::NposSolution; /// Types that are used by the data provider trait. pub mod data_provider { diff --git a/frame/election-provider-support/src/traits.rs b/frame/election-provider-support/src/traits.rs new file mode 100644 index 0000000000000..a719b1eef74cb --- /dev/null +++ b/frame/election-provider-support/src/traits.rs @@ -0,0 +1,136 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Traits for the election operations. + +use crate::{Assignment, IdentifierT, PerThing128, VoteWeight}; +use codec::Encode; +use scale_info::TypeInfo; +use sp_arithmetic::traits::{Bounded, UniqueSaturatedInto}; +use sp_npos_elections::{ElectionScore, Error, EvaluateSupport, IndexAssignment}; +use sp_std::{ + convert::{TryFrom, TryInto}, + fmt::Debug, + prelude::*, +}; + +/// An opaque index-based, NPoS solution type. +pub trait NposSolution +where + Self: Sized + for<'a> sp_std::convert::TryFrom<&'a [IndexAssignmentOf], Error = Error>, +{ + /// The maximum number of votes that are allowed. + const LIMIT: usize; + + /// The voter type. Needs to be an index (convert to usize). + type VoterIndex: UniqueSaturatedInto + + TryInto + + TryFrom + + Debug + + Copy + + Clone + + Bounded + + Encode + + TypeInfo; + + /// The target type. Needs to be an index (convert to usize). + type TargetIndex: UniqueSaturatedInto + + TryInto + + TryFrom + + Debug + + Copy + + Clone + + Bounded + + Encode + + TypeInfo; + + /// The weight/accuracy type of each vote. + type Accuracy: PerThing128; + + /// Get the length of all the voters that this type is encoding. + /// + /// This is basically the same as the number of assignments, or number of active voters. + fn voter_count(&self) -> usize; + + /// Get the total count of edges. + /// + /// This is effectively in the range of {[`Self::voter_count`], [`Self::voter_count`] * + /// [`Self::LIMIT`]}. + fn edge_count(&self) -> usize; + + /// Get the number of unique targets in the whole struct. + /// + /// Once presented with a list of winners, this set and the set of winners must be + /// equal. + fn unique_targets(&self) -> Vec; + + /// Get the average edge count. + fn average_edge_count(&self) -> usize { + self.edge_count().checked_div(self.voter_count()).unwrap_or(0) + } + + /// Compute the score of this solution type. + fn score( + self, + stake_of: FS, + voter_at: impl Fn(Self::VoterIndex) -> Option, + target_at: impl Fn(Self::TargetIndex) -> Option, + ) -> Result + where + for<'r> FS: Fn(&'r A) -> VoteWeight, + A: IdentifierT, + { + let ratio = self.into_assignment(voter_at, target_at)?; + let staked = + sp_npos_elections::helpers::assignment_ratio_to_staked_normalized(ratio, stake_of)?; + let supports = sp_npos_elections::to_supports(&staked); + Ok(supports.evaluate()) + } + + /// Remove a certain voter. + /// + /// This will only search until the first instance of `to_remove`, and return true. If + /// no instance is found (no-op), then it returns false. + /// + /// In other words, if this return true, exactly **one** element must have been removed self. + fn remove_voter(&mut self, to_remove: Self::VoterIndex) -> bool; + + /// Build self from a list of assignments. + fn from_assignment( + assignments: &[Assignment], + voter_index: FV, + target_index: FT, + ) -> Result + where + A: IdentifierT, + for<'r> FV: Fn(&'r A) -> Option, + for<'r> FT: Fn(&'r A) -> Option; + + /// Convert self into a `Vec>` + fn into_assignment( + self, + voter_at: impl Fn(Self::VoterIndex) -> Option, + target_at: impl Fn(Self::TargetIndex) -> Option, + ) -> Result>, Error>; +} + +/// A type alias for [`IndexAssignment`] made from [`NposSolution`]. +pub type IndexAssignmentOf = IndexAssignment< + ::VoterIndex, + ::TargetIndex, + ::Accuracy, +>; diff --git a/primitives/npos-elections/src/assignments.rs b/primitives/npos-elections/src/assignments.rs index 5641a33447bad..6fd876bcd5152 100644 --- a/primitives/npos-elections/src/assignments.rs +++ b/primitives/npos-elections/src/assignments.rs @@ -200,10 +200,3 @@ impl IndexAssignment = IndexAssignment< - ::VoterIndex, - ::TargetIndex, - ::Accuracy, ->; diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index 673036c2d19d1..7b3c7cd1598dc 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -100,14 +100,14 @@ pub mod pjr; pub mod reduce; pub mod traits; -pub use assignments::{Assignment, IndexAssignment, IndexAssignmentOf, StakedAssignment}; +pub use assignments::{Assignment, IndexAssignment, StakedAssignment}; pub use balancing::*; pub use helpers::*; pub use phragmen::*; pub use phragmms::*; pub use pjr::*; pub use reduce::reduce; -pub use traits::{IdentifierT, NposSolution, PerThing128, __OrInvalidIndex}; +pub use traits::{IdentifierT, PerThing128, __OrInvalidIndex}; // re-export for the solution macro, with the dependencies of the macro. #[doc(hidden)] diff --git a/primitives/npos-elections/src/traits.rs b/primitives/npos-elections/src/traits.rs index 0723948b62269..966963b5a3368 100644 --- a/primitives/npos-elections/src/traits.rs +++ b/primitives/npos-elections/src/traits.rs @@ -17,22 +17,9 @@ //! Traits for the npos-election operations. -use crate::{ - Assignment, ElectionScore, Error, EvaluateSupport, ExtendedBalance, IndexAssignmentOf, - VoteWeight, -}; -use codec::Encode; -use scale_info::TypeInfo; -use sp_arithmetic::{ - traits::{Bounded, UniqueSaturatedInto}, - PerThing, -}; -use sp_std::{ - convert::{TryFrom, TryInto}, - fmt::Debug, - ops::Mul, - prelude::*, -}; +use crate::{Error, ExtendedBalance}; +use sp_arithmetic::PerThing; +use sp_std::{fmt::Debug, ops::Mul, prelude::*}; /// an aggregator trait for a generic type of a voter/target identifier. This usually maps to /// substrate's account id. @@ -56,102 +43,3 @@ impl __OrInvalidIndex for Option { self.ok_or(Error::SolutionInvalidIndex) } } - -/// An opaque index-based, NPoS solution type. -pub trait NposSolution -where - Self: Sized + for<'a> sp_std::convert::TryFrom<&'a [IndexAssignmentOf], Error = Error>, -{ - /// The maximum number of votes that are allowed. - const LIMIT: usize; - - /// The voter type. Needs to be an index (convert to usize). - type VoterIndex: UniqueSaturatedInto - + TryInto - + TryFrom - + Debug - + Copy - + Clone - + Bounded - + Encode - + TypeInfo; - - /// The target type. Needs to be an index (convert to usize). - type TargetIndex: UniqueSaturatedInto - + TryInto - + TryFrom - + Debug - + Copy - + Clone - + Bounded - + Encode - + TypeInfo; - - /// The weight/accuracy type of each vote. - type Accuracy: PerThing128; - - /// Get the length of all the voters that this type is encoding. - /// - /// This is basically the same as the number of assignments, or number of active voters. - fn voter_count(&self) -> usize; - - /// Get the total count of edges. - /// - /// This is effectively in the range of {[`Self::voter_count`], [`Self::voter_count`] * - /// [`Self::LIMIT`]}. - fn edge_count(&self) -> usize; - - /// Get the number of unique targets in the whole struct. - /// - /// Once presented with a list of winners, this set and the set of winners must be - /// equal. - fn unique_targets(&self) -> Vec; - - /// Get the average edge count. - fn average_edge_count(&self) -> usize { - self.edge_count().checked_div(self.voter_count()).unwrap_or(0) - } - - /// Compute the score of this solution type. - fn score( - self, - stake_of: FS, - voter_at: impl Fn(Self::VoterIndex) -> Option, - target_at: impl Fn(Self::TargetIndex) -> Option, - ) -> Result - where - for<'r> FS: Fn(&'r A) -> VoteWeight, - A: IdentifierT, - { - let ratio = self.into_assignment(voter_at, target_at)?; - let staked = crate::helpers::assignment_ratio_to_staked_normalized(ratio, stake_of)?; - let supports = crate::to_supports(&staked); - Ok(supports.evaluate()) - } - - /// Remove a certain voter. - /// - /// This will only search until the first instance of `to_remove`, and return true. If - /// no instance is found (no-op), then it returns false. - /// - /// In other words, if this return true, exactly **one** element must have been removed self. - fn remove_voter(&mut self, to_remove: Self::VoterIndex) -> bool; - - /// Build self from a list of assignments. - fn from_assignment( - assignments: &[Assignment], - voter_index: FV, - target_index: FT, - ) -> Result - where - A: IdentifierT, - for<'r> FV: Fn(&'r A) -> Option, - for<'r> FT: Fn(&'r A) -> Option; - - /// Convert self into a `Vec>` - fn into_assignment( - self, - voter_at: impl Fn(Self::VoterIndex) -> Option, - target_at: impl Fn(Self::TargetIndex) -> Option, - ) -> Result>, Error>; -} From 626878dbc9e740224925cb48fa1f2cdca81940f1 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Mon, 7 Mar 2022 23:18:44 +0000 Subject: [PATCH 15/26] Implementing `MaxEncodedLen` on `generate_solution_type` --- Cargo.lock | 2 ++ bin/node/runtime/src/lib.rs | 4 ++- .../election-provider-multi-phase/src/mock.rs | 7 ++++- .../solution-type/Cargo.toml | 1 + .../solution-type/fuzzer/Cargo.toml | 1 + .../solution-type/fuzzer/src/compact.rs | 1 + .../solution-type/src/lib.rs | 30 +++++++++++++++---- .../solution-type/src/single_page.rs | 8 +++++ .../tests/ui/fail/missing_accuracy.rs | 1 + .../tests/ui/fail/missing_size_bound.rs | 10 +++++++ .../tests/ui/fail/missing_size_bound.stderr | 5 ++++ .../tests/ui/fail/missing_target.rs | 1 + .../tests/ui/fail/missing_voter.rs | 1 + .../tests/ui/fail/no_annotations.rs | 1 + .../tests/ui/fail/swap_voter_target.rs | 1 + .../tests/ui/fail/wrong_attribute.rs | 1 + 16 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs create mode 100644 frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr diff --git a/Cargo.lock b/Cargo.lock index f938a3522ced8..3a017b6eecaab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2165,6 +2165,7 @@ name = "frame-election-provider-solution-type" version = "4.0.0-dev" dependencies = [ "frame-election-provider-support", + "frame-support", "parity-scale-codec", "proc-macro-crate 1.1.0", "proc-macro2", @@ -2200,6 +2201,7 @@ dependencies = [ "clap 3.1.6", "frame-election-provider-solution-type", "frame-election-provider-support", + "frame-support", "honggfuzz", "parity-scale-codec", "rand 0.8.4", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index ae0809b1475ed..a1bb5d84c3b46 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -594,11 +594,13 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = u32, TargetIndex = u16, Accuracy = sp_runtime::PerU16, + SizeBound = VoterSnapshotPerBlock, >(16) ); parameter_types! { pub MaxNominations: u32 = ::LIMIT as u32; + pub VoterSnapshotPerBlock: u32 = 10_000; } /// The numbers configured here could always be more than the the maximum limits of staking pallet @@ -681,7 +683,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime { // BagsList allows a practically unbounded count of nominators to participate in NPoS elections. // To ensure we respect memory limits when using the BagsList this must be set to a number of // voters we know can fit into a single vec allocation. - type VoterSnapshotPerBlock = ConstU32<10_000>; + type VoterSnapshotPerBlock = VoterSnapshotPerBlock; } parameter_types! { diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index d09d45784cba4..2ec4c51b58ad7 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -70,7 +70,12 @@ pub(crate) type TargetIndex = u16; frame_election_provider_support::generate_solution_type!( #[compact] - pub struct TestNposSolution::(16) + pub struct TestNposSolution::< + VoterIndex = VoterIndex, + TargetIndex = TargetIndex, + Accuracy = PerU16, + SizeBound = ConstU32::<10_000> //TODO bound size? + >(16) ); /// All events of this pallet. diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index 08c33f20bb259..96322454aa670 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -27,4 +27,5 @@ sp-arithmetic = { version = "5.0.0", path = "../../../primitives/arithmetic" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", path = "../../../primitives/npos-elections" } frame-election-provider-support = { version = "4.0.0-dev", path = ".." } +frame-support = { version = "4.0.0-dev", path = "../../support" } trybuild = "1.0.53" diff --git a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml index f6c2f2fe491e8..e2bae3f72a4f7 100644 --- a/frame/election-provider-support/solution-type/fuzzer/Cargo.toml +++ b/frame/election-provider-support/solution-type/fuzzer/Cargo.toml @@ -25,6 +25,7 @@ sp-arithmetic = { version = "5.0.0", path = "../../../../primitives/arithmetic" sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } # used by generate_solution_type: sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../../primitives/npos-elections" } +frame-support = { version = "4.0.0-dev", path = "../../../support" } [[bin]] name = "compact" diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index 501d241b2b80b..a7c7c5d256f74 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -8,6 +8,7 @@ fn main() { VoterIndex = u32, TargetIndex = u32, Accuracy = Percent, + SizeBound = frame_support::traits::ConstU32::<100_000>, //TODO: unsure about size >(16)); loop { fuzz!(|fuzzer_data: &[u8]| { diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 8081d77ae8205..10922e0a6d0aa 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,6 +49,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. +/// - The maximum number of votes per snapshot. This must be of type `Get`. /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. @@ -59,10 +60,12 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// ``` /// # use frame_election_provider_solution_type::generate_solution_type; /// # use sp_arithmetic::per_things::Perbill; +/// # use frame_support::traits::ConstU32; /// generate_solution_type!(pub struct TestSolution::< /// VoterIndex = u16, /// TargetIndex = u8, /// Accuracy = Perbill, +/// SizeBound = ConstU32::<10>, /// >(4)); /// ``` /// @@ -103,9 +106,15 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// # use frame_election_provider_solution_type::generate_solution_type; /// # use frame_election_provider_support::NposSolution; /// # use sp_arithmetic::per_things::Perbill; +/// # use frame_support::traits::ConstU32; /// generate_solution_type!( /// #[compact] -/// pub struct TestSolutionCompact::(8) +/// pub struct TestSolutionCompact::< +/// VoterIndex = u16, +/// TargetIndex = u8, +/// Accuracy = Perbill, +/// SizeBound = ConstU32::<10>, +/// >(8) /// ); /// ``` #[proc_macro] @@ -129,6 +138,7 @@ struct SolutionDef { voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, + size_bound: syn::Type, count: usize, compact_encoding: bool, } @@ -167,11 +177,11 @@ impl Parse for SolutionDef { let _ = ::parse(input)?; let generics: syn::AngleBracketedGenericArguments = input.parse()?; - if generics.args.len() != 3 { - return Err(syn_err("Must provide 3 generic args.")) + if generics.args.len() != 4 { + return Err(syn_err("Must provide 4 generic args.")) } - let expected_types = ["VoterIndex", "TargetIndex", "Accuracy"]; + let expected_types = ["VoterIndex", "TargetIndex", "Accuracy", "SizeBound"]; let mut types: Vec = generics .args @@ -197,6 +207,7 @@ impl Parse for SolutionDef { }) .collect::>()?; + let size_bound = types.pop().expect("Vector of length 4 can be popped; qed"); let weight_type = types.pop().expect("Vector of length 3 can be popped; qed"); let target_type = types.pop().expect("Vector of length 2 can be popped; qed"); let voter_type = types.pop().expect("Vector of length 1 can be popped; qed"); @@ -205,7 +216,16 @@ impl Parse for SolutionDef { let count_expr: syn::ExprParen = input.parse()?; let count = parse_parenthesized_number::(count_expr)?; - Ok(Self { vis, ident, voter_type, target_type, weight_type, count, compact_encoding }) + Ok(Self { + vis, + ident, + voter_type, + target_type, + weight_type, + size_bound, + count, + compact_encoding, + }) } } diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 6ef9dea0100aa..37d04f13051c3 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -28,6 +28,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { voter_type, target_type, weight_type, + size_bound, compact_encoding, } = def; @@ -178,6 +179,13 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { <#ident as frame_election_provider_support::NposSolution>::TargetIndex, <#ident as frame_election_provider_support::NposSolution>::Accuracy, >; + impl _npos::codec::MaxEncodedLen for #ident { + fn max_encoded_len() -> usize { + use frame_support::traits::Get; + let s: u32 = #size_bound::get(); + s as usize + } + } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { type Error = _npos::Error; fn try_from(index_assignments: &'a [__IndexAssignment]) -> Result { diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index 22693cd875e17..9a5b48d3af7cc 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs new file mode 100644 index 0000000000000..0ea93f0fa6306 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs @@ -0,0 +1,10 @@ +use frame_npos_elections_solution_type::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + VoterIndex = u16, + TargetIndex = u8, + Accuracy = Perbill, + frame_support::traits::ConstU32::<10>, +>(8)); + +fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr new file mode 100644 index 0000000000000..8dc3ffa46f6e7 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `SizeBound = ...` + --> tests/ui/fail/missing_size_bound.rs:7:2 + | +7 | frame_support::traits::ConstU32::<10>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index 8d0ca927c5fb3..c6231074aa935 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index ad4b7f5217794..c21a05ec9cd95 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, TargetIndex = u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index 87673a3823513..9442cda0d983b 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, u8, Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index f1d5d0e7bf99f..b17d24aaabeef 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -4,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, VoterIndex = u8, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index d04cc4a7a966b..86c54e9d7d957 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -5,6 +5,7 @@ generate_solution_type!( VoterIndex = u8, TargetIndex = u16, Accuracy = Perbill, + SizeBound = frame_support::traits::ConstU32::<10>, >(8) ); From d57cd90fcadb12b6a1620abe8acb1a37deeba49b Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 8 Mar 2022 22:20:48 +0000 Subject: [PATCH 16/26] Full implementation of `max_encoded_len` --- .../solution-type/src/single_page.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 37d04f13051c3..e9a9b8c6c74bc 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -182,8 +182,14 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { impl _npos::codec::MaxEncodedLen for #ident { fn max_encoded_len() -> usize { use frame_support::traits::Get; + use _npos::codec::Encode; let s: u32 = #size_bound::get(); - s as usize + let max_element_size = #voter_type::max_encoded_len() + .max(#target_type::max_encoded_len() + .max(#weight_type::max_encoded_len())); + #count.saturating_mul( + _npos::codec::Compact(s).encoded_size() + .saturating_add((s as usize).saturating_mul(max_element_size))) } } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { From 6759535ae1a52099092a484984e0ad7a9ba30694 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 8 Mar 2022 22:53:06 +0000 Subject: [PATCH 17/26] Fixing implementation bug adding some comments and documentation --- .../solution-type/src/lib.rs | 3 ++- .../solution-type/src/single_page.rs | 16 +++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 10922e0a6d0aa..4c5638a5f73c0 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,7 +49,8 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. -/// - The maximum number of votes per snapshot. This must be of type `Get`. +/// - The maximum number of voters per snapshot. This must be of type `Get`. Check +/// for more details /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index e9a9b8c6c74bc..03f7fc1aefd63 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -184,12 +184,18 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { use frame_support::traits::Get; use _npos::codec::Encode; let s: u32 = #size_bound::get(); + // The last element of the struct is a vec with 1 voter + // then #count-1 tuple of target with an accuracy + // and then lastly the final target let max_element_size = #voter_type::max_encoded_len() - .max(#target_type::max_encoded_len() - .max(#weight_type::max_encoded_len())); - #count.saturating_mul( - _npos::codec::Compact(s).encoded_size() - .saturating_add((s as usize).saturating_mul(max_element_size))) + .saturating_add((#count - 1) + .saturating_mul(#target_type::max_encoded_len() + .saturating_add(#weight_type::max_encoded_len()))) + .saturating_add(#target_type::max_encoded_len()); + // The assumption is that it contains #count-1 empty elements + // and then last element with full size + #count.saturating_mul(_npos::codec::Compact(0u32).encoded_size()) + .saturating_add((s as usize).saturating_mul(max_element_size)) } } impl<'a> _npos::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { From 64b083a681602f73365f7d7e71095a31bb373f4f Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Mon, 14 Mar 2022 21:42:00 +0000 Subject: [PATCH 18/26] Fixing test --- .../solution-type/tests/ui/fail/missing_size_bound.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs index 0ea93f0fa6306..a21032ba06ea1 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs @@ -1,4 +1,4 @@ -use frame_npos_elections_solution_type::generate_solution_type; +use frame_election_provider_solution_type::generate_solution_type; generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, From 5b01e892f95779d6e56b9df38f1a5777727037b3 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Wed, 16 Mar 2022 22:04:44 +0000 Subject: [PATCH 19/26] Removing unneeded dependencies --- Cargo.lock | 1 - frame/election-provider-support/solution-type/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d58c0d749dbc1..b0b1a6c38a296 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2172,7 +2172,6 @@ dependencies = [ "quote", "scale-info", "sp-arithmetic", - "sp-npos-elections", "syn", "trybuild", ] diff --git a/frame/election-provider-support/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index 96322454aa670..35e8fb7e39c15 100644 --- a/frame/election-provider-support/solution-type/Cargo.toml +++ b/frame/election-provider-support/solution-type/Cargo.toml @@ -25,7 +25,6 @@ parity-scale-codec = "3.0.0" scale-info = "2.0.1" sp-arithmetic = { version = "5.0.0", path = "../../../primitives/arithmetic" } # used by generate_solution_type: -sp-npos-elections = { version = "4.0.0-dev", path = "../../../primitives/npos-elections" } frame-election-provider-support = { version = "4.0.0-dev", path = ".." } frame-support = { version = "4.0.0-dev", path = "../../support" } trybuild = "1.0.53" From 6848ce1b42e08f1d4925d58bd38985281a6aee06 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sat, 19 Mar 2022 21:26:31 +0000 Subject: [PATCH 20/26] `VoterSnapshotPerBlock` -> `MaxElectingVoters` --- bin/node/runtime/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 03419edb13c5c..e142046bdf535 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -597,13 +597,13 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = u32, TargetIndex = u16, Accuracy = sp_runtime::PerU16, - SizeBound = VoterSnapshotPerBlock, + SizeBound = MaxElectingVoters, >(16) ); parameter_types! { pub MaxNominations: u32 = ::LIMIT as u32; - pub VoterSnapshotPerBlock: u32 = 10_000; + pub MaxElectingVoters: u32 = 10_000; } /// The numbers configured here could always be more than the the maximum limits of staking pallet @@ -679,7 +679,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime { >; type ForceOrigin = EnsureRootOrHalfCouncil; type MaxElectableTargets = ConstU16<{ u16::MAX }>; - type MaxElectingVoters = ConstU32<10_000>; + type MaxElectingVoters = MaxElectingVoters; type BenchmarkingConfig = ElectionProviderBenchmarkConfig; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; } From 08833245e3fc2c563622af8e350aaae78c1fdffe Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 20 Mar 2022 10:56:20 +0000 Subject: [PATCH 21/26] rename `SizeBound` to `MaxVoters` --- bin/node/runtime/src/lib.rs | 2 +- frame/election-provider-multi-phase/src/mock.rs | 2 +- .../solution-type/fuzzer/src/compact.rs | 2 +- frame/election-provider-support/solution-type/src/lib.rs | 6 +++--- .../solution-type/tests/ui/fail/missing_accuracy.rs | 2 +- .../solution-type/tests/ui/fail/missing_size_bound.stderr | 2 +- .../solution-type/tests/ui/fail/missing_target.rs | 2 +- .../solution-type/tests/ui/fail/missing_voter.rs | 2 +- .../solution-type/tests/ui/fail/no_annotations.rs | 2 +- .../solution-type/tests/ui/fail/swap_voter_target.rs | 2 +- .../solution-type/tests/ui/fail/wrong_attribute.rs | 2 +- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index e142046bdf535..b7137de48fb15 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -597,7 +597,7 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = u32, TargetIndex = u16, Accuracy = sp_runtime::PerU16, - SizeBound = MaxElectingVoters, + MaxVoters = MaxElectingVoters, >(16) ); diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 7cc08d50fef01..dab82f69b1b66 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -74,7 +74,7 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = VoterIndex, TargetIndex = TargetIndex, Accuracy = PerU16, - SizeBound = ConstU32::<10_000> //TODO bound size? + MaxVoters = ConstU32::<10_000> //TODO bound size? >(16) ); diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index a7c7c5d256f74..e562a7e113b8b 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -8,7 +8,7 @@ fn main() { VoterIndex = u32, TargetIndex = u32, Accuracy = Percent, - SizeBound = frame_support::traits::ConstU32::<100_000>, //TODO: unsure about size + MaxVoters = frame_support::traits::ConstU32::<100_000>, //TODO: unsure about size >(16)); loop { fuzz!(|fuzzer_data: &[u8]| { diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 70f343069c7d7..ede9b1838f2f5 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -66,7 +66,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// VoterIndex = u16, /// TargetIndex = u8, /// Accuracy = Perbill, -/// SizeBound = ConstU32::<10>, +/// MaxVoters = ConstU32::<10>, /// >(4)); /// ``` /// @@ -114,7 +114,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// VoterIndex = u16, /// TargetIndex = u8, /// Accuracy = Perbill, -/// SizeBound = ConstU32::<10>, +/// MaxVoters = ConstU32::<10>, /// >(8) /// ); /// ``` @@ -182,7 +182,7 @@ impl Parse for SolutionDef { return Err(syn_err("Must provide 4 generic args.")) } - let expected_types = ["VoterIndex", "TargetIndex", "Accuracy", "SizeBound"]; + let expected_types = ["VoterIndex", "TargetIndex", "Accuracy", "MaxVoters"]; let mut types: Vec = generics .args diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index 9a5b48d3af7cc..e63c33176eb83 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Perbill, - SizeBound = frame_support::traits::ConstU32::<10>, + MaxVoters = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr index 8dc3ffa46f6e7..bed488821ec44 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr @@ -1,4 +1,4 @@ -error: Expected binding: `SizeBound = ...` +error: Expected binding: `MaxVoters = ...` --> tests/ui/fail/missing_size_bound.rs:7:2 | 7 | frame_support::traits::ConstU32::<10>, diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index c6231074aa935..002eb21e92361 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, u8, Accuracy = Perbill, - SizeBound = frame_support::traits::ConstU32::<10>, + MaxVoters = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index c21a05ec9cd95..c2a258f0a4d1d 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, TargetIndex = u8, Accuracy = Perbill, - SizeBound = frame_support::traits::ConstU32::<10>, + MaxVoters = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index 9442cda0d983b..a66a1b6e7d8a0 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, u8, Perbill, - SizeBound = frame_support::traits::ConstU32::<10>, + MaxVoters = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index b17d24aaabeef..6b87c6620151b 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, VoterIndex = u8, Accuracy = Perbill, - SizeBound = frame_support::traits::ConstU32::<10>, + MaxVoters = frame_support::traits::ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index 86c54e9d7d957..d1bdbd70ce005 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -5,7 +5,7 @@ generate_solution_type!( VoterIndex = u8, TargetIndex = u16, Accuracy = Perbill, - SizeBound = frame_support::traits::ConstU32::<10>, + MaxVoters = frame_support::traits::ConstU32::<10>, >(8) ); From d3b06c646cfe17d2d8489393f4a85adcf7e447a8 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 20 Mar 2022 10:58:16 +0000 Subject: [PATCH 22/26] Removing TODO and change bound --- frame/election-provider-multi-phase/src/mock.rs | 2 +- .../solution-type/fuzzer/src/compact.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index dab82f69b1b66..e2384d2f15761 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -74,7 +74,7 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = VoterIndex, TargetIndex = TargetIndex, Accuracy = PerU16, - MaxVoters = ConstU32::<10_000> //TODO bound size? + MaxVoters = ConstU32::<20> >(16) ); diff --git a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs index e562a7e113b8b..e7ef440ff2195 100644 --- a/frame/election-provider-support/solution-type/fuzzer/src/compact.rs +++ b/frame/election-provider-support/solution-type/fuzzer/src/compact.rs @@ -8,7 +8,7 @@ fn main() { VoterIndex = u32, TargetIndex = u32, Accuracy = Percent, - MaxVoters = frame_support::traits::ConstU32::<100_000>, //TODO: unsure about size + MaxVoters = frame_support::traits::ConstU32::<100_000>, >(16)); loop { fuzz!(|fuzzer_data: &[u8]| { From 0cd8944da3680c1ea045650b02d1a929854eb419 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 20 Mar 2022 20:44:21 +0000 Subject: [PATCH 23/26] renaming `size_bound` to `max_voters` --- frame/election-provider-support/solution-type/src/lib.rs | 6 +++--- .../solution-type/src/single_page.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index ede9b1838f2f5..62e01df3d136c 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -139,7 +139,7 @@ struct SolutionDef { voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, - size_bound: syn::Type, + max_voters: syn::Type, count: usize, compact_encoding: bool, } @@ -208,7 +208,7 @@ impl Parse for SolutionDef { }) .collect::>()?; - let size_bound = types.pop().expect("Vector of length 4 can be popped; qed"); + let max_voters = types.pop().expect("Vector of length 4 can be popped; qed"); let weight_type = types.pop().expect("Vector of length 3 can be popped; qed"); let target_type = types.pop().expect("Vector of length 2 can be popped; qed"); let voter_type = types.pop().expect("Vector of length 1 can be popped; qed"); @@ -223,7 +223,7 @@ impl Parse for SolutionDef { voter_type, target_type, weight_type, - size_bound, + max_voters, count, compact_encoding, }) diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 0932175fb1722..dd7c10ae74c54 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -28,7 +28,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { voter_type, target_type, weight_type, - size_bound, + max_voters, compact_encoding, } = def; @@ -183,7 +183,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { fn max_encoded_len() -> usize { use frame_support::traits::Get; use _feps::codec::Encode; - let s: u32 = #size_bound::get(); + let s: u32 = #max_voters::get(); // The last element of the struct is a vec with 1 voter // then #count-1 tuple of target with an accuracy // and then lastly the final target From 38c0cf56f9b525567790ec72fb81e895996ce628 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 20 Mar 2022 20:52:36 +0000 Subject: [PATCH 24/26] Enabling tests for `solution-type` These got dropped off after the crate was moved from `sp_npos_elections` --- Cargo.lock | 1 + frame/election-provider-support/Cargo.toml | 1 + frame/election-provider-support/src/lib.rs | 5 +++++ .../{solution-type => }/src/mock.rs | 11 +++++++++-- .../{solution-type => }/src/tests.rs | 10 +++++++++- 5 files changed, 25 insertions(+), 3 deletions(-) rename frame/election-provider-support/{solution-type => }/src/mock.rs (96%) rename frame/election-provider-support/{solution-type => }/src/tests.rs (97%) diff --git a/Cargo.lock b/Cargo.lock index 9a6906ba63912..1b9d586afe6fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2186,6 +2186,7 @@ dependencies = [ "frame-support", "frame-system", "parity-scale-codec", + "rand 0.7.3", "scale-info", "sp-arithmetic", "sp-core", diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index 16b79dbb098d4..be0c05e46df32 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -24,6 +24,7 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys frame-election-provider-solution-type = { version = "4.0.0-dev", path = "solution-type" } [dev-dependencies] +rand = "0.7.3" sp-npos-elections = { version = "4.0.0-dev", path = "../../primitives/npos-elections" } sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-io = { version = "6.0.0", path = "../../primitives/io" } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 81fd841a6dc47..2cc27472e8846 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -192,6 +192,11 @@ pub use scale_info; pub use sp_arithmetic; #[doc(hidden)] pub use sp_std; + +#[cfg(test)] +mod mock; +#[cfg(test)] +mod tests; // Simple Extension trait to easily convert `None` from index closures to `Err`. // // This is only generated and re-exported for the solution code to use. diff --git a/frame/election-provider-support/solution-type/src/mock.rs b/frame/election-provider-support/src/mock.rs similarity index 96% rename from frame/election-provider-support/solution-type/src/mock.rs rename to frame/election-provider-support/src/mock.rs index c3d032f2eb257..1ea8dddf7eb17 100644 --- a/frame/election-provider-support/solution-type/src/mock.rs +++ b/frame/election-provider-support/src/mock.rs @@ -19,10 +19,16 @@ #![cfg(test)] -use std::{collections::HashMap, convert::TryInto, hash::Hash, HashSet}; +use std::{ + collections::{HashMap, HashSet}, + convert::TryInto, + hash::Hash, +}; use rand::{seq::SliceRandom, Rng}; +pub type AccountId = u64; + /// The candidate mask allows easy disambiguation between voters and candidates: accounts /// for which this bit is set are candidates, and without it, are voters. pub const CANDIDATE_MASK: AccountId = 1 << ((std::mem::size_of::() * 8) - 1); @@ -34,13 +40,14 @@ pub fn p(p: u8) -> TestAccuracy { } pub type MockAssignment = crate::Assignment; -pub type Voter = (AccountId, VoteWeight, Vec); +pub type Voter = (AccountId, crate::VoteWeight, Vec); crate::generate_solution_type! { pub struct TestSolution::< VoterIndex = u32, TargetIndex = u16, Accuracy = TestAccuracy, + MaxVoters = frame_support::traits::ConstU32::<20>, >(16) } diff --git a/frame/election-provider-support/solution-type/src/tests.rs b/frame/election-provider-support/src/tests.rs similarity index 97% rename from frame/election-provider-support/solution-type/src/tests.rs rename to frame/election-provider-support/src/tests.rs index f173e425b5187..16aaeae7dbcf8 100644 --- a/frame/election-provider-support/solution-type/src/tests.rs +++ b/frame/election-provider-support/src/tests.rs @@ -20,6 +20,7 @@ #![cfg(test)] use crate::{mock::*, IndexAssignment, NposSolution}; +use frame_support::traits::ConstU32; use rand::SeedableRng; use std::convert::TryInto; @@ -37,7 +38,12 @@ mod solution_type { use crate::generate_solution_type; generate_solution_type!( #[compact] - struct InnerTestSolutionIsolated::(12) + struct InnerTestSolutionIsolated::< + VoterIndex = u32, + TargetIndex = u8, + Accuracy = sp_runtime::Percent, + MaxVoters = crate::tests::ConstU32::<20>, + >(12) ); } @@ -50,6 +56,7 @@ mod solution_type { VoterIndex = u32, TargetIndex = u32, Accuracy = TestAccuracy, + MaxVoters = ConstU32::<20>, >(16) ); let solution = InnerTestSolution { @@ -68,6 +75,7 @@ mod solution_type { VoterIndex = u32, TargetIndex = u32, Accuracy = TestAccuracy, + MaxVoters = ConstU32::<20>, >(16) ); let compact = InnerTestSolutionCompact { From 084e586d186668e80e3006a425d27b6c29bb82bd Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Sun, 20 Mar 2022 21:12:13 +0000 Subject: [PATCH 25/26] Adding tests for `MaxEncodedLen` of solution_type --- frame/election-provider-support/src/tests.rs | 74 +++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-support/src/tests.rs b/frame/election-provider-support/src/tests.rs index 16aaeae7dbcf8..7b4e46d836176 100644 --- a/frame/election-provider-support/src/tests.rs +++ b/frame/election-provider-support/src/tests.rs @@ -26,8 +26,9 @@ use std::convert::TryInto; mod solution_type { use super::*; - use codec::{Decode, Encode}; - // these need to come from the same dev-dependency `sp-npos-elections`, not from the crate. + use codec::{Decode, Encode, MaxEncodedLen}; + // these need to come from the same dev-dependency `frame-election-provider-support`, not from + // the crate. use crate::{generate_solution_type, Assignment, Error as NposError, NposSolution}; use sp_std::{convert::TryInto, fmt::Debug}; @@ -90,6 +91,75 @@ mod solution_type { assert!(with_compact < without_compact); } + #[test] + fn max_encoded_len_too_small() { + generate_solution_type!( + pub struct InnerTestSolution::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = TestAccuracy, + MaxVoters = ConstU32::<1>, + >(3) + ); + let solution = InnerTestSolution { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], + ..Default::default() + }; + + // We actually have 4 voters, but the bound is 1 voter, so the implemented bound is too + // small. + assert!(solution.encode().len() > InnerTestSolution::max_encoded_len()); + } + + #[test] + fn max_encoded_len_upper_bound() { + generate_solution_type!( + pub struct InnerTestSolution::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = TestAccuracy, + MaxVoters = ConstU32::<4>, + >(3) + ); + let solution = InnerTestSolution { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![(1, [(10, p(80))], 11), (5, [(50, p(85))], 51)], + ..Default::default() + }; + + // We actually have 4 voters, and the bound is 4 voters, so the implemented bound should be + // larger than the encoded len. + assert!(solution.encode().len() < InnerTestSolution::max_encoded_len()); + } + + #[test] + fn max_encoded_len_exact() { + generate_solution_type!( + pub struct InnerTestSolution::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = TestAccuracy, + MaxVoters = ConstU32::<4>, + >(3) + ); + let solution = InnerTestSolution { + votes1: vec![], + votes2: vec![], + votes3: vec![ + (1, [(10, p(50)), (11, p(20))], 12), + (2, [(20, p(50)), (21, p(20))], 22), + (3, [(30, p(50)), (31, p(20))], 32), + (4, [(40, p(50)), (41, p(20))], 42), + ], + }; + + // We have 4 voters, the bound is 4 voters, and all the voters voted for 3 targets, which is + // the max number of targets. This should represent the upper bound that `max_encoded_len` + // represents. + assert_eq!(solution.encode().len(), InnerTestSolution::max_encoded_len()); + } + #[test] fn solution_struct_is_codec() { let solution = TestSolution { From 05782242fef7eb24c925d6afd8a488169abb7a27 Mon Sep 17 00:00:00 2001 From: Georges Dib Date: Tue, 22 Mar 2022 21:31:32 +0000 Subject: [PATCH 26/26] Better rustdocs. Better indenting and comments. Removing unneeded imports in tests. --- .../solution-type/src/lib.rs | 8 ++++++-- .../solution-type/src/single_page.rs | 18 ++++++++++-------- .../tests/ui/fail/missing_accuracy.rs | 2 +- .../tests/ui/fail/missing_size_bound.rs | 2 +- .../tests/ui/fail/missing_size_bound.stderr | 4 ++-- .../tests/ui/fail/missing_target.rs | 2 +- .../tests/ui/fail/missing_voter.rs | 2 +- .../tests/ui/fail/no_annotations.rs | 2 +- .../tests/ui/fail/swap_voter_target.rs | 2 +- .../tests/ui/fail/wrong_attribute.rs | 2 +- 10 files changed, 25 insertions(+), 19 deletions(-) diff --git a/frame/election-provider-support/solution-type/src/lib.rs b/frame/election-provider-support/solution-type/src/lib.rs index 62e01df3d136c..57d939377b62c 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,8 +49,12 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. -/// - The maximum number of voters per snapshot. This must be of type `Get`. Check -/// for more details +/// - The maximum number of voters. This must be of type `Get`. Check +/// for more details. This is used to bound the struct, by leveraging the fact that `votes1.len() +/// < votes2.len() < ... < votesn.len()` (the details of the struct is explained further below). +/// We know that `sum_i votes_i.len() <= MaxVoters`, and we know that the maximum size of the +/// struct would be achieved if all voters fall in the last bucket. One can also check the tests +/// and more specifically `max_encoded_len_exact` for a concrete example. /// /// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be /// specified. Attempting to convert from/to an assignment with more distributions will fail. diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index dd7c10ae74c54..5a3ddc22f61c8 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -184,17 +184,19 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { use frame_support::traits::Get; use _feps::codec::Encode; let s: u32 = #max_voters::get(); - // The last element of the struct is a vec with 1 voter - // then #count-1 tuple of target with an accuracy - // and then lastly the final target - let max_element_size = #voter_type::max_encoded_len() - .saturating_add((#count - 1) - .saturating_mul(#target_type::max_encoded_len() - .saturating_add(#weight_type::max_encoded_len()))) + let max_element_size = + // the first voter.. + #voter_type::max_encoded_len() + // #count - 1 tuples.. + .saturating_add( + (#count - 1).saturating_mul( + #target_type::max_encoded_len().saturating_add(#weight_type::max_encoded_len()))) + // and the last target. .saturating_add(#target_type::max_encoded_len()); // The assumption is that it contains #count-1 empty elements // and then last element with full size - #count.saturating_mul(_feps::codec::Compact(0u32).encoded_size()) + #count + .saturating_mul(_feps::codec::Compact(0u32).encoded_size()) .saturating_add((s as usize).saturating_mul(max_element_size)) } } diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs index e63c33176eb83..52ae9623fd384 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_accuracy.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Perbill, - MaxVoters = frame_support::traits::ConstU32::<10>, + MaxVoters = ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs index a21032ba06ea1..fe8ac04cc8d61 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Accuracy = Perbill, - frame_support::traits::ConstU32::<10>, + ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr index bed488821ec44..c685ab816d399 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr @@ -1,5 +1,5 @@ error: Expected binding: `MaxVoters = ...` --> tests/ui/fail/missing_size_bound.rs:7:2 | -7 | frame_support::traits::ConstU32::<10>, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +7 | ConstU32::<10>, + | ^^^^^^^^^^^^^^ diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs index 002eb21e92361..b457c4abada6c 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_target.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, u8, Accuracy = Perbill, - MaxVoters = frame_support::traits::ConstU32::<10>, + MaxVoters = ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs index c2a258f0a4d1d..3d12e3e6b5ec4 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_voter.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, TargetIndex = u8, Accuracy = Perbill, - MaxVoters = frame_support::traits::ConstU32::<10>, + MaxVoters = ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs index a66a1b6e7d8a0..9aab15e7ec9a1 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/no_annotations.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, u8, Perbill, - MaxVoters = frame_support::traits::ConstU32::<10>, + MaxVoters = ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs index 6b87c6620151b..4275aae045a60 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/swap_voter_target.rs @@ -4,7 +4,7 @@ generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, VoterIndex = u8, Accuracy = Perbill, - MaxVoters = frame_support::traits::ConstU32::<10>, + MaxVoters = ConstU32::<10>, >(8)); fn main() {} diff --git a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs index d1bdbd70ce005..a51cc724ad158 100644 --- a/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs +++ b/frame/election-provider-support/solution-type/tests/ui/fail/wrong_attribute.rs @@ -5,7 +5,7 @@ generate_solution_type!( VoterIndex = u8, TargetIndex = u16, Accuracy = Perbill, - MaxVoters = frame_support::traits::ConstU32::<10>, + MaxVoters = ConstU32::<10>, >(8) );