diff --git a/Cargo.lock b/Cargo.lock index e847dcd23d325..1b9d586afe6fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2167,6 +2167,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.3", "proc-macro2", @@ -2185,6 +2186,7 @@ dependencies = [ "frame-support", "frame-system", "parity-scale-codec", + "rand 0.7.3", "scale-info", "sp-arithmetic", "sp-core", @@ -2201,6 +2203,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 64bdcfc870205..b7137de48fb15 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -597,11 +597,13 @@ frame_election_provider_support::generate_solution_type!( VoterIndex = u32, TargetIndex = u16, Accuracy = sp_runtime::PerU16, + MaxVoters = MaxElectingVoters, >(16) ); parameter_types! { pub MaxNominations: u32 = ::LIMIT as u32; + pub MaxElectingVoters: u32 = 10_000; } /// The numbers configured here could always be more than the the maximum limits of staking pallet @@ -677,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; } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 89b5b72565dcb..e2384d2f15761 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, + MaxVoters = ConstU32::<20> + >(16) ); /// All events of this pallet. 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/solution-type/Cargo.toml b/frame/election-provider-support/solution-type/Cargo.toml index e59bbcc8e7b38..ca3038c9145ce 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: 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..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,6 +8,7 @@ fn main() { VoterIndex = u32, TargetIndex = u32, Accuracy = Percent, + MaxVoters = frame_support::traits::ConstU32::<100_000>, >(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 6e2788f06007f..57d939377b62c 100644 --- a/frame/election-provider-support/solution-type/src/lib.rs +++ b/frame/election-provider-support/solution-type/src/lib.rs @@ -49,6 +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. 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. @@ -59,10 +65,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, +/// MaxVoters = ConstU32::<10>, /// >(4)); /// ``` /// @@ -103,9 +111,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, +/// MaxVoters = ConstU32::<10>, +/// >(8) /// ); /// ``` #[proc_macro] @@ -129,6 +143,7 @@ struct SolutionDef { voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, + max_voters: syn::Type, count: usize, compact_encoding: bool, } @@ -167,11 +182,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", "MaxVoters"]; let mut types: Vec = generics .args @@ -197,6 +212,7 @@ impl Parse for SolutionDef { }) .collect::>()?; + 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"); @@ -205,7 +221,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, + 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 c1d897444da31..5a3ddc22f61c8 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, + max_voters, compact_encoding, } = def; @@ -178,6 +179,27 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { <#ident as _feps::NposSolution>::TargetIndex, <#ident as _feps::NposSolution>::Accuracy, >; + impl _feps::codec::MaxEncodedLen for #ident { + fn max_encoded_len() -> usize { + use frame_support::traits::Get; + use _feps::codec::Encode; + let s: u32 = #max_voters::get(); + 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()) + .saturating_add((s as usize).saturating_mul(max_element_size)) + } + } impl<'a> _feps::sp_std::convert::TryFrom<&'a [__IndexAssignment]> for #ident { type Error = _feps::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..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,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, TargetIndex = u8, Perbill, + 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 new file mode 100644 index 0000000000000..fe8ac04cc8d61 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.rs @@ -0,0 +1,10 @@ +use frame_election_provider_solution_type::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + VoterIndex = u16, + TargetIndex = u8, + Accuracy = Perbill, + 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..c685ab816d399 --- /dev/null +++ b/frame/election-provider-support/solution-type/tests/ui/fail/missing_size_bound.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `MaxVoters = ...` + --> tests/ui/fail/missing_size_bound.rs:7:2 + | +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 8d0ca927c5fb3..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,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< VoterIndex = u16, u8, Accuracy = Perbill, + 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 ad4b7f5217794..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,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, TargetIndex = u8, Accuracy = Perbill, + 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 87673a3823513..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,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< u16, u8, Perbill, + 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 f1d5d0e7bf99f..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,6 +4,7 @@ generate_solution_type!(pub struct TestSolution::< TargetIndex = u16, VoterIndex = u8, Accuracy = Perbill, + 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 d04cc4a7a966b..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,6 +5,7 @@ generate_solution_type!( VoterIndex = u8, TargetIndex = u16, Accuracy = Perbill, + MaxVoters = ConstU32::<10>, >(8) ); 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 80% rename from frame/election-provider-support/solution-type/src/tests.rs rename to frame/election-provider-support/src/tests.rs index f173e425b5187..7b4e46d836176 100644 --- a/frame/election-provider-support/solution-type/src/tests.rs +++ b/frame/election-provider-support/src/tests.rs @@ -20,13 +20,15 @@ #![cfg(test)] use crate::{mock::*, IndexAssignment, NposSolution}; +use frame_support::traits::ConstU32; 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 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}; @@ -37,7 +39,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 +57,7 @@ mod solution_type { VoterIndex = u32, TargetIndex = u32, Accuracy = TestAccuracy, + MaxVoters = ConstU32::<20>, >(16) ); let solution = InnerTestSolution { @@ -68,6 +76,7 @@ mod solution_type { VoterIndex = u32, TargetIndex = u32, Accuracy = TestAccuracy, + MaxVoters = ConstU32::<20>, >(16) ); let compact = InnerTestSolutionCompact { @@ -82,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 {