From 0e0d0a677af136319d637cf47fd9e20e637d158e Mon Sep 17 00:00:00 2001 From: Georges Date: Thu, 31 Mar 2022 13:54:44 +0100 Subject: [PATCH] Enforce `MaxEncodedLen` impl for `NposSolution` (#11103) * Fail if `MaxVoters` too small * Fixing benchmarking test, better naming of error * reverting accidental change * use fully qualified syntax no need to interate to calculate len * Fail directly if too many voters --- .../election-provider-multi-phase/src/mock.rs | 2 +- .../solution-type/src/single_page.rs | 6 +++++ .../election-provider-support/src/lib.rs | 3 ++- .../election-provider-support/src/mock.rs | 2 +- .../election-provider-support/src/tests.rs | 27 +++++++++++++++++++ .../primitives/npos-elections/src/lib.rs | 4 ++- 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/substrate/frame/election-provider-multi-phase/src/mock.rs b/substrate/frame/election-provider-multi-phase/src/mock.rs index 1b3c4d9306..d6f040363d 100644 --- a/substrate/frame/election-provider-multi-phase/src/mock.rs +++ b/substrate/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::<20> + MaxVoters = ConstU32::<2_000> >(16) ); diff --git a/substrate/frame/election-provider-support/solution-type/src/single_page.rs b/substrate/frame/election-provider-support/solution-type/src/single_page.rs index 5a3ddc22f6..a20f054298 100644 --- a/substrate/frame/election-provider-support/solution-type/src/single_page.rs +++ b/substrate/frame/election-provider-support/solution-type/src/single_page.rs @@ -124,6 +124,11 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { for<'r> FV: Fn(&'r A) -> Option, for<'r> FT: Fn(&'r A) -> Option, { + // Make sure that the voter bound is binding. + // `assignments.len()` actually represents the number of voters + if assignments.len() as u32 > <#max_voters as _feps::Get>::get() { + return Err(_feps::Error::TooManyVoters); + } let mut #struct_name: #ident = Default::default(); for _feps::Assignment { who, distribution } in assignments { match distribution.len() { @@ -134,6 +139,7 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { } } }; + Ok(#struct_name) } diff --git a/substrate/frame/election-provider-support/src/lib.rs b/substrate/frame/election-provider-support/src/lib.rs index d79b5289df..453cef8956 100644 --- a/substrate/frame/election-provider-support/src/lib.rs +++ b/substrate/frame/election-provider-support/src/lib.rs @@ -170,12 +170,13 @@ pub mod onchain; pub mod traits; #[cfg(feature = "std")] use codec::{Decode, Encode}; -use frame_support::{traits::Get, BoundedVec, RuntimeDebug}; +use frame_support::{BoundedVec, RuntimeDebug}; use sp_runtime::traits::Bounded; use sp_std::{fmt::Debug, prelude::*}; /// Re-export the solution generation macro. pub use frame_election_provider_solution_type::generate_solution_type; +pub use frame_support::traits::Get; /// Re-export some type as they are used in the interface. pub use sp_arithmetic::PerThing; pub use sp_npos_elections::{ diff --git a/substrate/frame/election-provider-support/src/mock.rs b/substrate/frame/election-provider-support/src/mock.rs index 1ea8dddf7e..d10b1724c9 100644 --- a/substrate/frame/election-provider-support/src/mock.rs +++ b/substrate/frame/election-provider-support/src/mock.rs @@ -47,7 +47,7 @@ crate::generate_solution_type! { VoterIndex = u32, TargetIndex = u16, Accuracy = TestAccuracy, - MaxVoters = frame_support::traits::ConstU32::<20>, + MaxVoters = frame_support::traits::ConstU32::<2_500>, >(16) } diff --git a/substrate/frame/election-provider-support/src/tests.rs b/substrate/frame/election-provider-support/src/tests.rs index 7b4e46d836..f88f3653c6 100644 --- a/substrate/frame/election-provider-support/src/tests.rs +++ b/substrate/frame/election-provider-support/src/tests.rs @@ -91,6 +91,33 @@ mod solution_type { assert!(with_compact < without_compact); } + #[test] + fn from_assignment_fail_too_many_voters() { + let rng = rand::rngs::SmallRng::seed_from_u64(0); + + // This will produce 24 voters.. + let (voters, assignments, candidates) = generate_random_votes(10, 25, rng); + let voter_index = make_voter_fn(&voters); + let target_index = make_target_fn(&candidates); + + // Limit the voters to 20.. + generate_solution_type!( + pub struct InnerTestSolution::< + VoterIndex = u32, + TargetIndex = u16, + Accuracy = TestAccuracy, + MaxVoters = frame_support::traits::ConstU32::<20>, + >(16) + ); + + // 24 > 20, so this should fail. + assert_eq!( + InnerTestSolution::from_assignment(&assignments, &voter_index, &target_index) + .unwrap_err(), + NposError::TooManyVoters, + ); + } + #[test] fn max_encoded_len_too_small() { generate_solution_type!( diff --git a/substrate/primitives/npos-elections/src/lib.rs b/substrate/primitives/npos-elections/src/lib.rs index 11d531fa56..93fb24eb4a 100644 --- a/substrate/primitives/npos-elections/src/lib.rs +++ b/substrate/primitives/npos-elections/src/lib.rs @@ -119,12 +119,14 @@ pub enum Error { SolutionTargetOverflow, /// One of the index functions returned none. SolutionInvalidIndex, - /// One of the page indices was invalid + /// One of the page indices was invalid. SolutionInvalidPageIndex, /// An error occurred in some arithmetic operation. ArithmeticError(&'static str), /// The data provided to create support map was invalid. InvalidSupportEdge, + /// The number of voters is bigger than the `MaxVoters` bound. + TooManyVoters, } /// A type which is used in the API of this crate as a numeric weight of a vote, most often the