From b73e4274c246e232480454ed24b1d9b3d034eaf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Sun, 9 Jul 2023 12:35:06 +0200 Subject: [PATCH 01/11] removes without_storage_info annotation from the pallet and replaces with unbounded annotation per storage item that needs to be bounded in the future --- frame/election-provider-multi-phase/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 1960c458b983e..8f8e00f8a0786 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1256,6 +1256,7 @@ pub mod pallet { /// Current phase. #[pallet::storage] + #[pallet::unbounded] #[pallet::getter(fn current_phase)] pub type CurrentPhase = StorageValue<_, Phase, ValueQuery>; @@ -1263,6 +1264,7 @@ pub mod pallet { /// /// Always sorted by score. #[pallet::storage] + #[pallet::unbounded] #[pallet::getter(fn queued_solution)] pub type QueuedSolution = StorageValue<_, ReadySolution>; @@ -1271,6 +1273,7 @@ pub mod pallet { /// /// This is created at the beginning of the signed phase and cleared upon calling `elect`. #[pallet::storage] + #[pallet::unbounded] #[pallet::getter(fn snapshot)] pub type Snapshot = StorageValue<_, RoundSnapshot>>; @@ -1285,6 +1288,7 @@ pub mod pallet { /// /// Only exists when [`Snapshot`] is present. #[pallet::storage] + #[pallet::unbounded] #[pallet::getter(fn snapshot_metadata)] pub type SnapshotMetadata = StorageValue<_, SolutionOrSnapshotSize>; @@ -1322,6 +1326,7 @@ pub mod pallet { /// Twox note: the key of the map is an auto-incrementing index which users cannot inspect or /// affect; we shouldn't need a cryptographically secure hasher. #[pallet::storage] + #[pallet::unbounded] pub type SignedSubmissionsMap = StorageMap<_, Twox64Concat, u32, SignedSubmissionOf, OptionQuery>; @@ -1341,7 +1346,6 @@ pub mod pallet { const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); #[pallet::pallet] - #[pallet::without_storage_info] #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); } From 9fa1aa36e898d22d513c5b01ba1d2e6b2ebc72cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Sun, 9 Jul 2023 12:37:27 +0200 Subject: [PATCH 02/11] bound CurrenPhase storage type by deriving MaxEncodedLen on the Phase type --- frame/election-provider-multi-phase/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 8f8e00f8a0786..4749c49f48482 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -229,7 +229,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::{ BoundedSupportsOf, ElectionDataProvider, ElectionProvider, ElectionProviderBase, InstantElectionProvider, NposSolution, @@ -314,7 +314,7 @@ pub trait BenchmarkingConfig { } /// Current phase of the pallet. -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)] pub enum Phase { /// Nothing, the election is not happening. Off, @@ -1256,7 +1256,6 @@ pub mod pallet { /// Current phase. #[pallet::storage] - #[pallet::unbounded] #[pallet::getter(fn current_phase)] pub type CurrentPhase = StorageValue<_, Phase, ValueQuery>; From c7ac0474ac9ccf8ca8765e5445559029168cc8bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Sun, 9 Jul 2023 12:39:13 +0200 Subject: [PATCH 03/11] bound SnapshotMetadata storage type by deriving MaxEncodedLen on the SolutionOrSnapshotSize type --- frame/election-provider-multi-phase/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 4749c49f48482..b5687b2fa8df8 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -467,7 +467,7 @@ pub struct RoundSnapshot { /// This is stored automatically on-chain, and it contains the **size of the entire snapshot**. /// This is also used in dispatchables as weight witness data and should **only contain the size of /// the presented solution**, not the entire snapshot. -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, Default, TypeInfo)] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, Default, TypeInfo, MaxEncodedLen)] pub struct SolutionOrSnapshotSize { /// The length of voters. #[codec(compact)] @@ -1287,7 +1287,6 @@ pub mod pallet { /// /// Only exists when [`Snapshot`] is present. #[pallet::storage] - #[pallet::unbounded] #[pallet::getter(fn snapshot_metadata)] pub type SnapshotMetadata = StorageValue<_, SolutionOrSnapshotSize>; From 3c5308c823119bed2fdbb02c0e5ee33d013943df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Sun, 9 Jul 2023 21:44:57 +0200 Subject: [PATCH 04/11] starts experimenting with bounding the snapshot storage type; minerconfig missing --- .../election-provider-multi-phase/src/lib.rs | 56 +++++++++++++------ .../src/unsigned.rs | 2 +- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index b5687b2fa8df8..1732c7745a42f 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -239,7 +239,7 @@ use frame_support::{ ensure, traits::{Currency, DefensiveResult, Get, OnUnbalanced, ReservableCurrency}, weights::Weight, - DefaultNoBound, EqNoBound, PartialEqNoBound, + BoundedVec, DefaultNoBound, EqNoBound, PartialEqNoBound, }; use frame_system::{ensure_none, offchain::SendTransactionTypes}; use scale_info::TypeInfo; @@ -453,13 +453,14 @@ where /// [`ElectionDataProvider`] and are kept around until the round is finished. /// /// These are stored together because they are often accessed together. -#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, TypeInfo)] -#[scale_info(skip_type_params(T))] -pub struct RoundSnapshot { +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, MaxEncodedLen, TypeInfo)] +#[codec(mel_bound(skip_type_params(VotersBond, TargetsBound)))] +#[scale_info(skip_type_params(VotersBound, TargetsBound))] +pub struct RoundSnapshot, TargetsBound: Get> { /// All of the voters. - pub voters: Vec, + pub voters: BoundedVec, /// All of the targets. - pub targets: Vec, + pub targets: BoundedVec, } /// Encodes the length of a solution or a snapshot. @@ -663,11 +664,11 @@ pub mod pallet { /// are only over a single block, but once multi-block elections are introduced they will /// take place over multiple blocks. #[pallet::constant] - type MaxElectingVoters: Get>; + type MaxElectingVoters: Get> + Get; /// The maximum number of electable targets to put in the snapshot. #[pallet::constant] - type MaxElectableTargets: Get>; + type MaxElectableTargets: Get> + Get; /// The maximum number of winners that can be elected by this `ElectionProvider` /// implementation. @@ -1272,9 +1273,11 @@ pub mod pallet { /// /// This is created at the beginning of the signed phase and cleared upon calling `elect`. #[pallet::storage] - #[pallet::unbounded] #[pallet::getter(fn snapshot)] - pub type Snapshot = StorageValue<_, RoundSnapshot>>; + pub type Snapshot = StorageValue< + _, + RoundSnapshot, T::MaxElectingVoters, T::MaxElectableTargets>, + >; /// Desired number of targets to elect for this round. /// @@ -1391,8 +1394,8 @@ impl Pallet { /// /// Extracted for easier weight calculation. fn create_snapshot_internal( - targets: Vec, - voters: Vec>, + targets: BoundedVec, + voters: BoundedVec, T::MaxElectingVoters>, desired_targets: u32, ) { let metadata = @@ -1405,7 +1408,15 @@ impl Pallet { // instead of using storage APIs, we do a manual encoding into a fixed-size buffer. // `encoded_size` encodes it without storing it anywhere, this should not cause any // allocation. - let snapshot = RoundSnapshot::> { voters, targets }; + let snapshot = RoundSnapshot::< + T::AccountId, + VoterOf, + T::MaxElectingVoters, + T::MaxElectableTargets, + > { + voters, + targets, + }; let size = snapshot.encoded_size(); log!(debug, "snapshot pre-calculated size {:?}", size); let mut buffer = Vec::with_capacity(size); @@ -1422,10 +1433,16 @@ impl Pallet { /// Parts of [`create_snapshot`] that happen outside of this pallet. /// /// Extracted for easier weight calculation. - fn create_snapshot_external( - ) -> Result<(Vec, Vec>, u32), ElectionError> { - let target_limit = T::MaxElectableTargets::get().saturated_into::(); - let voter_limit = T::MaxElectingVoters::get().saturated_into::(); + fn create_snapshot_external() -> Result< + ( + BoundedVec, + BoundedVec, T::MaxElectingVoters>, + u32, + ), + ElectionError, + > { + let target_limit = >::get().saturated_into::(); + let voter_limit = >::get().saturated_into::(); let targets = T::DataProvider::electable_targets(Some(target_limit)) .map_err(ElectionError::DataProvider)?; @@ -1437,6 +1454,11 @@ impl Pallet { return Err(ElectionError::DataProvider("Snapshot too big for submission.")) } + // TODO: data provider must return a bounded vec. this is just to experiment at this level + // without having to touch the data provider trait and impl for now. + let targets: BoundedVec<_, T::MaxElectableTargets> = BoundedVec::truncate_from(targets); + let voters: BoundedVec<_, T::MaxElectingVoters> = BoundedVec::truncate_from(voters); + let mut desired_targets = as ElectionProviderBase>::desired_targets_checked() .map_err(|e| ElectionError::DataProvider(e))?; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 9c09cb48c7c05..b967882422d46 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -702,7 +702,7 @@ impl Miner { raw_solution: RawSolution>, compute: ElectionCompute, desired_targets: u32, - snapshot: RoundSnapshot>, + snapshot: RoundSnapshot, T::MaxWinners, T::MaxWinners>, current_round: u32, minimum_untrusted_score: Option, ) -> Result, FeasibilityError> { From 6eed63bf4bc3c7c375d6f19effa353a2e1fd9dfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Mon, 10 Jul 2023 10:50:03 +0200 Subject: [PATCH 05/11] (draft) bound Snapshot storage type without refactoring external dependencies (solved, sq_phragmen, etc) --- .../election-provider-multi-phase/src/lib.rs | 29 ++++++++++++------ .../election-provider-multi-phase/src/mock.rs | 19 ++++++++++-- .../src/unsigned.rs | 30 +++++++++++++++---- .../test-staking-e2e/src/mock.rs | 2 ++ 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 1732c7745a42f..97843dd217876 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -294,6 +294,10 @@ pub type SolutionAccuracyOf = ::MinerConfig> as NposSolution>::Accuracy; /// The fallback election type. pub type FallbackErrorOf = <::Fallback as ElectionProviderBase>::Error; +/// The maximum electable targets, as seen by the miner config. +pub type MaxElectableTargetsOf = ::MaxElectableTargets; +/// The maximum electing voters, as seen by the miner config. +pub type MaxElectingVotersOf = ::MaxElectingVoters; /// Configuration for the benchmarks of the pallet. pub trait BenchmarkingConfig { @@ -454,13 +458,13 @@ where /// /// These are stored together because they are often accessed together. #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, MaxEncodedLen, TypeInfo)] -#[codec(mel_bound(skip_type_params(VotersBond, TargetsBound)))] -#[scale_info(skip_type_params(VotersBound, TargetsBound))] -pub struct RoundSnapshot, TargetsBound: Get> { +#[codec(mel_bound(skip_type_params(V, T)))] +#[scale_info(skip_type_params(V, T))] +pub struct RoundSnapshot, T: Get> { /// All of the voters. - pub voters: BoundedVec, + pub voters: BoundedVec, /// All of the targets. - pub targets: BoundedVec, + pub targets: BoundedVec, } /// Encodes the length of a solution or a snapshot. @@ -620,6 +624,8 @@ pub mod pallet { AccountId = Self::AccountId, MaxVotesPerVoter = ::MaxVotesPerVoter, MaxWinners = Self::MaxWinners, + MaxElectingVoters = Self::MaxElectingVoters, + MaxElectableTargets = Self::MaxElectableTargets, >; /// Maximum number of signed submissions that can be queued. @@ -1276,7 +1282,12 @@ pub mod pallet { #[pallet::getter(fn snapshot)] pub type Snapshot = StorageValue< _, - RoundSnapshot, T::MaxElectingVoters, T::MaxElectableTargets>, + RoundSnapshot< + T::AccountId, + VoterOf, + MaxElectingVotersOf, + MaxElectableTargetsOf, + >, >; /// Desired number of targets to elect for this round. @@ -1454,8 +1465,8 @@ impl Pallet { return Err(ElectionError::DataProvider("Snapshot too big for submission.")) } - // TODO: data provider must return a bounded vec. this is just to experiment at this level - // without having to touch the data provider trait and impl for now. + // TODO(gpestana): data provider must return a bounded vec. this is just to experiment at + // this level without having to touch the data provider trait and impl for now. let targets: BoundedVec<_, T::MaxElectableTargets> = BoundedVec::truncate_from(targets); let voters: BoundedVec<_, T::MaxElectingVoters> = BoundedVec::truncate_from(voters); @@ -1482,7 +1493,7 @@ impl Pallet { /// /// 1. [`SnapshotMetadata`] /// 2. [`RoundSnapshot`] - /// 3. [`DesiredTargets`] + /// 3. [`DesiredTargets`]1536 /// /// Returns `Ok(())` if operation is okay. /// diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index cf797aea845f4..3b35d0a721ab1 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -151,8 +151,12 @@ pub fn trim_helpers() -> TrimHelpers { let desired_targets = MultiPhase::desired_targets().unwrap(); + // TODO(gpestana): change seq_phragmen inputs to bounded vec + let targets_unbounded: Vec<_> = targets.clone().into(); + let voters_unbounded: Vec<_> = voters.clone().into(); + let ElectionResult::<_, SolutionAccuracyOf> { mut assignments, .. } = - seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap(); + seq_phragmen(desired_targets as usize, targets_unbounded, voters_unbounded, None).unwrap(); // sort by decreasing order of stake assignments.sort_by_key(|assignment| { @@ -168,7 +172,12 @@ pub fn trim_helpers() -> TrimHelpers { .collect::, _>>() .expect("test assignments don't contain any voters with too many votes"); - TrimHelpers { voters, assignments, encoded_size_of, voter_index: Box::new(voter_index) } + TrimHelpers { + voters: voters.to_vec(), + assignments, + encoded_size_of, + voter_index: Box::new(voter_index), + } } /// Spit out a verifiable raw solution. @@ -178,6 +187,10 @@ pub fn raw_solution() -> RawSolution> { let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap(); let desired_targets = MultiPhase::desired_targets().unwrap(); + // TODO(gpestana): change seq_phragmen inputs to bounded vec + let targets: Vec<_> = targets.into(); + let voters: Vec<_> = voters.into(); + let ElectionResult::<_, SolutionAccuracyOf> { winners: _, assignments } = seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap(); @@ -364,6 +377,8 @@ impl MinerConfig for Runtime { type AccountId = AccountId; type MaxLength = MinerMaxLength; type MaxWeight = MinerMaxWeight; + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxVotesPerVoter = ::MaxVotesPerVoter; type MaxWinners = MaxWinners; type Solution = TestNposSolution; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index b967882422d46..ee3cea21b52f4 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -167,6 +167,7 @@ impl Pallet { let RoundSnapshot { voters, targets } = Self::snapshot().ok_or(MinerError::SnapshotUnAvailable)?; let desired_targets = Self::desired_targets().ok_or(MinerError::SnapshotUnAvailable)?; + let (solution, score, size) = Miner::::mine_solution_with_snapshot::< T::Solver, >(voters, targets, desired_targets)?; @@ -380,6 +381,11 @@ pub trait MinerConfig { + Ord + NposSolution + TypeInfo; + + /// Maximum number of electing voters in the snaphots. + type MaxElectingVoters: Get; + /// Maximum number of elecatble targets in the snapshots. + type MaxElectableTargets: Get; /// Maximum number of votes per voter in the snapshots. type MaxVotesPerVoter; /// Maximum length of the solution that the miner is allowed to generate. @@ -405,14 +411,18 @@ pub struct Miner(sp_std::marker::PhantomData); impl Miner { /// Same as [`Pallet::mine_solution`], but the input snapshot data must be given. pub fn mine_solution_with_snapshot( - voters: Vec<(T::AccountId, VoteWeight, BoundedVec)>, - targets: Vec, + voters: BoundedVec< + (T::AccountId, VoteWeight, BoundedVec), + T::MaxElectingVoters, + >, + targets: BoundedVec, desired_targets: u32, ) -> Result<(SolutionOf, ElectionScore, SolutionOrSnapshotSize), MinerError> where S: NposSolver, { - S::solve(desired_targets as usize, targets.clone(), voters.clone()) + // TODO(gpestana): change the Solver::solve signature to accept bounded vec instead. + S::solve(desired_targets as usize, targets.clone().to_vec(), voters.clone().to_vec()) .map_err(|e| { log_no_system!(error, "solver error: {:?}", e); MinerError::Solver @@ -433,8 +443,11 @@ impl Miner { /// Will always reduce the solution as well. pub fn prepare_election_result_with_snapshot( election_result: ElectionResult, - voters: Vec<(T::AccountId, VoteWeight, BoundedVec)>, - targets: Vec, + voters: BoundedVec< + (T::AccountId, VoteWeight, BoundedVec), + T::MaxElectingVoters, + >, + targets: BoundedVec, desired_targets: u32, ) -> Result<(SolutionOf, ElectionScore, SolutionOrSnapshotSize), MinerError> { // now make some helper closures. @@ -702,7 +715,12 @@ impl Miner { raw_solution: RawSolution>, compute: ElectionCompute, desired_targets: u32, - snapshot: RoundSnapshot, T::MaxWinners, T::MaxWinners>, + snapshot: RoundSnapshot< + T::AccountId, + MinerVoterOf, + T::MaxElectingVoters, + T::MaxElectableTargets, + >, current_round: u32, minimum_untrusted_score: Option, ) -> Result, FeasibilityError> { diff --git a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index da7ccf6dce9ce..1ec377fe1b1ec 100644 --- a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -229,6 +229,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime { impl MinerConfig for Runtime { type AccountId = AccountId; type Solution = MockNposSolution; + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxVotesPerVoter = <::DataProvider as ElectionDataProvider>::MaxVotesPerVoter; type MaxLength = MinerMaxLength; From 0e592166556a8d1f26fed232fcadb2a4cd9ed559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Mon, 10 Jul 2023 14:10:27 +0200 Subject: [PATCH 06/11] fix todos --- .../election-provider-multi-phase/src/lib.rs | 2 +- .../election-provider-multi-phase/src/mock.rs | 25 ++++++++------- .../src/unsigned.rs | 31 ++++++++++--------- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 97843dd217876..6acff4bc16e78 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1493,7 +1493,7 @@ impl Pallet { /// /// 1. [`SnapshotMetadata`] /// 2. [`RoundSnapshot`] - /// 3. [`DesiredTargets`]1536 + /// 3. [`DesiredTargets`] /// /// Returns `Ok(())` if operation is okay. /// diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 3b35d0a721ab1..8d96efe10f03b 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -151,12 +151,13 @@ pub fn trim_helpers() -> TrimHelpers { let desired_targets = MultiPhase::desired_targets().unwrap(); - // TODO(gpestana): change seq_phragmen inputs to bounded vec - let targets_unbounded: Vec<_> = targets.clone().into(); - let voters_unbounded: Vec<_> = voters.clone().into(); - - let ElectionResult::<_, SolutionAccuracyOf> { mut assignments, .. } = - seq_phragmen(desired_targets as usize, targets_unbounded, voters_unbounded, None).unwrap(); + let ElectionResult::<_, SolutionAccuracyOf> { mut assignments, .. } = seq_phragmen( + desired_targets as usize, + targets.clone().into_inner(), + voters.clone().into_inner(), + None, + ) + .unwrap(); // sort by decreasing order of stake assignments.sort_by_key(|assignment| { @@ -187,12 +188,14 @@ pub fn raw_solution() -> RawSolution> { let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap(); let desired_targets = MultiPhase::desired_targets().unwrap(); - // TODO(gpestana): change seq_phragmen inputs to bounded vec - let targets: Vec<_> = targets.into(); - let voters: Vec<_> = voters.into(); - let ElectionResult::<_, SolutionAccuracyOf> { winners: _, assignments } = - seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap(); + seq_phragmen( + desired_targets as usize, + targets.clone().into_inner(), + voters.clone().into_inner(), + None, + ) + .unwrap(); // closures let cache = helpers::generate_voter_cache::(&voters); diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index ee3cea21b52f4..463893a4f56cf 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -421,20 +421,23 @@ impl Miner { where S: NposSolver, { - // TODO(gpestana): change the Solver::solve signature to accept bounded vec instead. - S::solve(desired_targets as usize, targets.clone().to_vec(), voters.clone().to_vec()) - .map_err(|e| { - log_no_system!(error, "solver error: {:?}", e); - MinerError::Solver - }) - .and_then(|e| { - Self::prepare_election_result_with_snapshot::( - e, - voters, - targets, - desired_targets, - ) - }) + S::solve( + desired_targets as usize, + targets.clone().into_inner(), + voters.clone().into_inner(), + ) + .map_err(|e| { + log_no_system!(error, "solver error: {:?}", e); + MinerError::Solver + }) + .and_then(|e| { + Self::prepare_election_result_with_snapshot::( + e, + voters, + targets, + desired_targets, + ) + }) } /// Convert a raw solution from [`sp_npos_elections::ElectionResult`] to [`RawSolution`], which From 6d97eb51f0b38aa2d4a4f9be46c1a976a15232b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Fri, 14 Jul 2023 15:12:25 +0200 Subject: [PATCH 07/11] Bounds the results returned by ElectionDataProvider --- .../election-provider-multi-phase/src/lib.rs | 9 +- .../election-provider-multi-phase/src/mock.rs | 25 +++--- .../src/signed.rs | 30 ------- .../test-staking-e2e/src/mock.rs | 12 ++- frame/election-provider-support/src/lib.rs | 88 ++++++++++++++++--- .../election-provider-support/src/onchain.rs | 39 ++++++-- frame/staking/src/pallet/impls.rs | 18 ++-- 7 files changed, 149 insertions(+), 72 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 6acff4bc16e78..192d62ea1b13b 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -693,6 +693,8 @@ pub mod pallet { type DataProvider: ElectionDataProvider< AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, + MaxElectingVoters = Self::MaxElectingVoters, + MaxElectableTargets = Self::MaxElectableTargets, >; /// Configuration for the fallback. @@ -1465,11 +1467,6 @@ impl Pallet { return Err(ElectionError::DataProvider("Snapshot too big for submission.")) } - // TODO(gpestana): data provider must return a bounded vec. this is just to experiment at - // this level without having to touch the data provider trait and impl for now. - let targets: BoundedVec<_, T::MaxElectableTargets> = BoundedVec::truncate_from(targets); - let voters: BoundedVec<_, T::MaxElectingVoters> = BoundedVec::truncate_from(voters); - let mut desired_targets = as ElectionProviderBase>::desired_targets_checked() .map_err(|e| ElectionError::DataProvider(e))?; @@ -1709,6 +1706,8 @@ impl ElectionProviderBase for Pallet { type AccountId = T::AccountId; type BlockNumber = T::BlockNumber; type Error = ElectionError; + type MaxElectingVoters = T::MaxElectingVoters; + type MaxElectableTargets = T::MaxElectableTargets; type MaxWinners = T::MaxWinners; type DataProvider = T::DataProvider; } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 8d96efe10f03b..c37842ab122d1 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -331,6 +331,8 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen, Balancing>; type DataProvider = StakingMock; type WeightInfo = (); + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxWinners = MaxWinners; type VotersBound = ConstU32<{ u32::MAX }>; type TargetsBound = ConstU32<{ u32::MAX }>; @@ -342,6 +344,8 @@ impl ElectionProviderBase for MockFallback { type BlockNumber = u64; type Error = &'static str; type DataProvider = StakingMock; + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxWinners = MaxWinners; } @@ -457,8 +461,12 @@ impl ElectionDataProvider for StakingMock { type AccountId = AccountId; type BlockNumber = u64; type MaxVotesPerVoter = MaxNominations; + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; - fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { + fn electable_targets( + maybe_max_len: Option, + ) -> data_provider::Result> { let targets = Targets::get(); if !DataProviderAllowBadData::get() && @@ -467,20 +475,15 @@ impl ElectionDataProvider for StakingMock { return Err("Targets too big") } - Ok(targets) + Ok(BoundedVec::truncate_from(targets)) } fn electing_voters( - maybe_max_len: Option, - ) -> data_provider::Result>> { - let mut voters = Voters::get(); - if !DataProviderAllowBadData::get() { - if let Some(max_len) = maybe_max_len { - voters.truncate(max_len) - } - } + _maybe_max_len: Option, + ) -> data_provider::Result, Self::MaxElectingVoters>> { + let voters = Voters::get(); - Ok(voters) + Ok(BoundedVec::truncate_from(voters)) } fn desired_targets() -> data_provider::Result { diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index bde985518d53e..4d7e1af3ad0e3 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -562,36 +562,6 @@ mod tests { }) } - #[test] - fn data_provider_should_respect_target_limits() { - ExtBuilder::default().build_and_execute(|| { - // given a reduced expectation of maximum electable targets - MaxElectableTargets::set(2); - // and a data provider that does not respect limits - DataProviderAllowBadData::set(true); - - assert_noop!( - MultiPhase::create_snapshot(), - ElectionError::DataProvider("Snapshot too big for submission."), - ); - }) - } - - #[test] - fn data_provider_should_respect_voter_limits() { - ExtBuilder::default().build_and_execute(|| { - // given a reduced expectation of maximum electing voters - MaxElectingVoters::set(2); - // and a data provider that does not respect limits - DataProviderAllowBadData::set(true); - - assert_noop!( - MultiPhase::create_snapshot(), - ElectionError::DataProvider("Snapshot too big for submission."), - ); - }) - } - #[test] fn desired_targets_greater_than_max_winners() { ExtBuilder::default().build_and_execute(|| { diff --git a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index 1ec377fe1b1ec..446c14b38250a 100644 --- a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -214,8 +214,14 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type SlashHandler = (); type RewardHandler = (); type DataProvider = Staking; - type Fallback = - frame_election_provider_support::NoElection<(AccountId, BlockNumber, Staking, MaxWinners)>; + type Fallback = frame_election_provider_support::NoElection<( + AccountId, + BlockNumber, + Staking, + MaxElectingVoters, + MaxElectableTargets, + MaxWinners, + )>; type GovernanceFallback = onchain::OnChainExecution; type Solver = SequentialPhragmen, ()>; type ForceOrigin = EnsureRoot; @@ -315,6 +321,8 @@ impl onchain::Config for OnChainSeqPhragmen { >; type DataProvider = Staking; type WeightInfo = (); + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxWinners = MaxWinners; type VotersBound = VotersBound; type TargetsBound = TargetsBound; diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 978a5df427244..23276a4548c53 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -81,7 +81,7 @@ //! ```rust //! # use frame_election_provider_support::{*, data_provider}; //! # use sp_npos_elections::{Support, Assignment}; -//! # use frame_support::traits::ConstU32; +//! # use frame_support::{BoundedVec, traits::ConstU32}; //! # use frame_support::bounded_vec; //! //! type AccountId = u64; @@ -105,17 +105,19 @@ //! type AccountId = AccountId; //! type BlockNumber = BlockNumber; //! type MaxVotesPerVoter = ConstU32<1>; +//! type MaxElectableTargets = ConstU32<400>; +//! type MaxElectingVoters = ConstU32<400>; //! //! fn desired_targets() -> data_provider::Result { //! Ok(1) //! } //! fn electing_voters(maybe_max_len: Option) -//! -> data_provider::Result>> +//! -> data_provider::Result, Self::MaxElectingVoters>> //! { //! Ok(Default::default()) //! } -//! fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { -//! Ok(vec![10, 20, 30]) +//! fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { +//! Ok(bounded_vec![10, 20, 30]) //! } //! fn next_election_prediction(now: BlockNumber) -> BlockNumber { //! 0 @@ -138,6 +140,8 @@ //! type BlockNumber = BlockNumber; //! type Error = &'static str; //! type DataProvider = T::DataProvider; +//! type MaxElectingVoters = ConstU32<{ u32::MAX }>; +//! type MaxElectableTargets = ConstU32<{ u32::MAX }>; //! type MaxWinners = ConstU32<{ u32::MAX }>; //! //! } @@ -280,6 +284,12 @@ pub trait ElectionDataProvider { /// The block number type. type BlockNumber; + /// Maximum number of targets that this data provider is providing. + type MaxElectableTargets: Get; + + /// Maximum number of voters that this data provider is providing. + type MaxElectingVoters: Get; + /// Maximum number of votes per voter that this data provider is providing. type MaxVotesPerVoter: Get; @@ -293,7 +303,7 @@ pub trait ElectionDataProvider { /// appropriate weight at the end of execution with the system pallet directly. fn electable_targets( maybe_max_len: Option, - ) -> data_provider::Result>; + ) -> data_provider::Result>; /// All the voters that participate in the election, thus "electing". /// @@ -304,7 +314,9 @@ pub trait ElectionDataProvider { /// /// This should be implemented as a self-weighing function. The implementor should register its /// appropriate weight at the end of execution with the system pallet directly. - fn electing_voters(maybe_max_len: Option) -> data_provider::Result>>; + fn electing_voters( + maybe_max_len: Option, + ) -> data_provider::Result, Self::MaxElectingVoters>>; /// The number of targets to elect. /// @@ -371,6 +383,26 @@ pub trait ElectionProviderBase { /// The error type that is returned by the provider. type Error: Debug; + /// The upper bound on electing voters that can be returned. + /// + /// # WARNING + /// + /// when communicating with the data provider, one must ensure that + /// `DataProvider::electing_voters` returns a value less than this bound. An + /// implementation can chose to either return an error and/or sort and + /// truncate the output to meet this bound. + type MaxElectingVoters: Get; + + /// The upper bound on the electable targets that can be returned. + /// + /// # WARNING + /// + /// when communicating with the data provider, one must ensure that + /// `DataProvider::electable_targets` returns a value less than this bound. An + /// implementation can chose to either return an error and/or sort and + /// truncate the output to meet this bound. + type MaxElectableTargets: Get; + /// The upper bound on election winners that can be returned. /// /// # WARNING @@ -433,23 +465,45 @@ pub trait InstantElectionProvider: ElectionProviderBase { /// An election provider that does nothing whatsoever. pub struct NoElection(sp_std::marker::PhantomData); -impl ElectionProviderBase - for NoElection<(AccountId, BlockNumber, DataProvider, MaxWinners)> +impl + ElectionProviderBase + for NoElection<( + AccountId, + BlockNumber, + DataProvider, + MaxElectingVoters, + MaxElectableTargets, + MaxWinners, + )> where DataProvider: ElectionDataProvider, + MaxElectingVoters: Get, + MaxElectableTargets: Get, MaxWinners: Get, { type AccountId = AccountId; type BlockNumber = BlockNumber; type Error = &'static str; + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxWinners = MaxWinners; type DataProvider = DataProvider; } -impl ElectionProvider - for NoElection<(AccountId, BlockNumber, DataProvider, MaxWinners)> +impl + ElectionProvider + for NoElection<( + AccountId, + BlockNumber, + DataProvider, + MaxElectingVoters, + MaxElectableTargets, + MaxWinners, + )> where DataProvider: ElectionDataProvider, + MaxElectingVoters: Get, + MaxElectableTargets: Get, MaxWinners: Get, { fn ongoing() -> bool { @@ -461,10 +515,20 @@ where } } -impl InstantElectionProvider - for NoElection<(AccountId, BlockNumber, DataProvider, MaxWinners)> +impl + InstantElectionProvider + for NoElection<( + AccountId, + BlockNumber, + DataProvider, + MaxElectingVoters, + MaxElectableTargets, + MaxWinners, + )> where DataProvider: ElectionDataProvider, + MaxElectingVoters: Get, + MaxElectableTargets: Get, MaxWinners: Get, { fn instant_elect( diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index a312562d4944c..19ca6ebd3a4a0 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -79,6 +79,18 @@ pub trait Config { /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; + /// Upper bound on maximum electing voters. + /// + /// As noted in the documentation of [`ElectionProviderBase::MaxElectingVoters`], this value + /// should always be more than `DataProvider::electing_voters`. + type MaxElectingVoters: Get; + + /// Upper bound on electable targets. + /// + /// As noted in the documentation of [`ElectionProviderBase::MaxElectableTargets`], this value + /// should always be more than `DataProvider::electable_targets`. + type MaxElectableTargets: Get; + /// Upper bound on maximum winners from electable targets. /// /// As noted in the documentation of [`ElectionProviderBase::MaxWinners`], this value should @@ -127,7 +139,8 @@ fn elect_with_input_bounds( }; let ElectionResult { winners: _, assignments } = - T::Solver::solve(desired_targets as usize, targets, voters).map_err(Error::from)?; + T::Solver::solve(desired_targets as usize, targets.into_inner(), voters.into_inner()) + .map_err(Error::from)?; let staked = assignment_ratio_to_staked_normalized(assignments, &stake_of)?; @@ -153,6 +166,8 @@ impl ElectionProviderBase for OnChainExecution { type AccountId = ::AccountId; type BlockNumber = ::BlockNumber; type Error = Error; + type MaxElectingVoters = T::MaxElectingVoters; + type MaxElectableTargets = T::MaxElectableTargets; type MaxWinners = T::MaxWinners; type DataProvider = T::DataProvider; } @@ -186,7 +201,7 @@ impl ElectionProvider for OnChainExecution { mod tests { use super::*; use crate::{ElectionProvider, PhragMMS, SequentialPhragmen}; - use frame_support::{assert_noop, parameter_types, traits::ConstU32}; + use frame_support::{assert_noop, parameter_types, traits::ConstU32, BoundedVec}; use sp_npos_elections::Support; use sp_runtime::Perbill; type AccountId = u64; @@ -239,6 +254,8 @@ mod tests { parameter_types! { pub static MaxWinners: u32 = 10; pub static DesiredTargets: u32 = 2; + pub static MaxElectableTargets: u32 = 400; + pub static MaxElectingVoters: u32 = 600; } impl Config for PhragmenParams { @@ -246,6 +263,8 @@ mod tests { type Solver = SequentialPhragmen; type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxWinners = MaxWinners; type VotersBound = ConstU32<600>; type TargetsBound = ConstU32<400>; @@ -256,6 +275,8 @@ mod tests { type Solver = PhragMMS; type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type MaxWinners = MaxWinners; type VotersBound = ConstU32<600>; type TargetsBound = ConstU32<400>; @@ -272,16 +293,22 @@ mod tests { type AccountId = AccountId; type BlockNumber = BlockNumber; type MaxVotesPerVoter = ConstU32<2>; - fn electing_voters(_: Option) -> data_provider::Result>> { - Ok(vec![ + type MaxElectableTargets = MaxElectableTargets; + type MaxElectingVoters = MaxElectingVoters; + fn electing_voters( + _: Option, + ) -> data_provider::Result, Self::MaxElectingVoters>> { + Ok(bounded_vec![ (1, 10, bounded_vec![10, 20]), (2, 20, bounded_vec![30, 20]), (3, 30, bounded_vec![10, 30]), ]) } - fn electable_targets(_: Option) -> data_provider::Result> { - Ok(vec![10, 20, 30]) + fn electable_targets( + _: Option, + ) -> data_provider::Result> { + Ok(bounded_vec![10, 20, 30]) } fn desired_targets() -> data_provider::Result { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 44d5674f2f816..3160a36899472 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -18,8 +18,8 @@ //! Implementations for the Staking FRAME Pallet. use frame_election_provider_support::{ - data_provider, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, ScoreProvider, - SortedListProvider, VoteWeight, VoterOf, + data_provider, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, ElectionProviderBase, + ScoreProvider, SortedListProvider, VoteWeight, VoterOf, }; use frame_support::{ defensive, @@ -996,21 +996,27 @@ impl ElectionDataProvider for Pallet { type AccountId = T::AccountId; type BlockNumber = BlockNumberFor; type MaxVotesPerVoter = T::MaxNominations; + type MaxElectingVoters = ::MaxElectingVoters; + type MaxElectableTargets = ::MaxElectableTargets; fn desired_targets() -> data_provider::Result { Self::register_weight(T::DbWeight::get().reads(1)); Ok(Self::validator_count()) } - fn electing_voters(maybe_max_len: Option) -> data_provider::Result>> { + fn electing_voters( + maybe_max_len: Option, + ) -> data_provider::Result, Self::MaxElectingVoters>> { // This can never fail -- if `maybe_max_len` is `Some(_)` we handle it. let voters = Self::get_npos_voters(maybe_max_len); debug_assert!(maybe_max_len.map_or(true, |max| voters.len() <= max)); - Ok(voters) + Ok(BoundedVec::truncate_from(voters)) } - fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { + fn electable_targets( + maybe_max_len: Option, + ) -> data_provider::Result> { let target_count = T::TargetList::count(); // We can't handle this case yet -- return an error. @@ -1018,7 +1024,7 @@ impl ElectionDataProvider for Pallet { return Err("Target snapshot too big") } - Ok(Self::get_npos_targets(None)) + Ok(BoundedVec::truncate_from(Self::get_npos_targets(None))) } fn next_election_prediction(now: T::BlockNumber) -> T::BlockNumber { From e59f57f0e43bb6225d90860c9571ba7e01c0a83d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Wed, 9 Aug 2023 17:11:46 +0200 Subject: [PATCH 08/11] Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com> --- frame/election-provider-multi-phase/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 97719b28b5613..7ce9a8ce08bd7 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1291,8 +1291,8 @@ pub mod pallet { RoundSnapshot< T::AccountId, VoterOf, - MaxElectingVotersOf, - MaxElectableTargetsOf, + T::MaxElectingVoters, + T::MaxElectableTargets, >, >; From 0f4d03d71d4274a1cb6d5777e277dea65a3fc6e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Wed, 9 Aug 2023 18:09:56 +0200 Subject: [PATCH 09/11] removes need to truncate from get_npos_X implementations --- frame/staking/src/mock.rs | 4 ++++ frame/staking/src/pallet/impls.rs | 40 +++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 232b37de7c1b8..350d9dcec9389 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -233,6 +233,8 @@ parameter_types! { pub static MaxUnlockingChunks: u32 = 32; pub static RewardOnUnbalanceWasCalled: bool = false; pub static MaxWinners: u32 = 100; + pub static MaxElectingVoters: u32 = u32::max_value(); + pub static MaxElectableTargets: u32 = u32::max_value(); } type VoterBagsListInstance = pallet_bags_list::Instance1; @@ -252,6 +254,8 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = Staking; type WeightInfo = (); type MaxWinners = MaxWinners; + type MaxElectingVoters = MaxElectingVoters; + type MaxElectableTargets = MaxElectableTargets; type VotersBound = ConstU32<{ u32::MAX }>; type TargetsBound = ConstU32<{ u32::MAX }>; } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index bc808efd7fcf7..53c1c17e947d5 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -57,6 +57,11 @@ use frame_support::ensure; #[cfg(any(test, feature = "try-runtime"))] use sp_runtime::TryRuntimeError; +/// Bounds for electing voters defined by the election provider. +type MaxElectingVotersOf = ::MaxElectingVoters; +/// Bounds for electable targets defined by the election provider. +type MaxElectabletargetsOf = ::MaxElectableTargets; + /// The maximum number of iterations that we do whilst iterating over `T::VoterList` in /// `get_npos_voters`. /// @@ -761,13 +766,16 @@ impl Pallet { /// nominators. /// /// This function is self-weighing as [`DispatchClass::Mandatory`]. - pub fn get_npos_voters(maybe_max_len: Option) -> Vec> { + pub fn get_npos_voters( + maybe_max_len: Option, + ) -> BoundedVec, MaxElectingVotersOf> { let max_allowed_len = { let all_voter_count = T::VoterList::count() as usize; maybe_max_len.unwrap_or(all_voter_count).min(all_voter_count) }; - let mut all_voters = Vec::<_>::with_capacity(max_allowed_len); + let mut all_voters = + BoundedVec::<_, MaxElectingVotersOf>::with_max_capacity(); // cache a few things. let weight_of = Self::weight_of_fn(); @@ -798,7 +806,10 @@ impl Pallet { if let Some(Nominations { targets, .. }) = >::get(&voter) { if !targets.is_empty() { - all_voters.push((voter.clone(), voter_weight, targets)); + if all_voters.try_push((voter.clone(), voter_weight, targets)).is_err() { + // bounds saturated, break loop. + break + } nominators_taken.saturating_inc(); } else { // Technically should never happen, but not much we can do about it. @@ -814,7 +825,10 @@ impl Pallet { .try_into() .expect("`MaxVotesPerVoter` must be greater than or equal to 1"), ); - all_voters.push(self_vote); + if all_voters.try_push(self_vote).is_err() { + // bounds saturated, break loop. + break + } validators_taken.saturating_inc(); } else { // this can only happen if: 1. there a bug in the bags-list (or whatever is the @@ -830,9 +844,6 @@ impl Pallet { } } - // all_voters should have not re-allocated. - debug_assert!(all_voters.capacity() == max_allowed_len); - Self::register_weight(T::WeightInfo::get_npos_voters(validators_taken, nominators_taken)); let min_active_stake: T::CurrencyBalance = @@ -854,9 +865,11 @@ impl Pallet { /// Get the targets for an upcoming npos election. /// /// This function is self-weighing as [`DispatchClass::Mandatory`]. - pub fn get_npos_targets(maybe_max_len: Option) -> Vec { + pub fn get_npos_targets( + maybe_max_len: Option, + ) -> BoundedVec> { let max_allowed_len = maybe_max_len.unwrap_or_else(|| T::TargetList::count() as usize); - let mut all_targets = Vec::::with_capacity(max_allowed_len); + let mut all_targets = BoundedVec::>::with_max_capacity(); let mut targets_seen = 0; let mut targets_iter = T::TargetList::iter(); @@ -872,7 +885,10 @@ impl Pallet { }; if Validators::::contains_key(&target) { - all_targets.push(target); + if all_targets.try_push(target).is_err() { + // bounds saturated, break loop. + break + } } } @@ -1018,7 +1034,7 @@ impl ElectionDataProvider for Pallet { let voters = Self::get_npos_voters(maybe_max_len); debug_assert!(maybe_max_len.map_or(true, |max| voters.len() <= max)); - Ok(BoundedVec::truncate_from(voters)) + Ok(voters) } fn electable_targets( @@ -1031,7 +1047,7 @@ impl ElectionDataProvider for Pallet { return Err("Target snapshot too big") } - Ok(BoundedVec::truncate_from(Self::get_npos_targets(None))) + Ok(Self::get_npos_targets(None)) } fn next_election_prediction(now: BlockNumberFor) -> BlockNumberFor { From 5962c989c7d4d395c2333827775af99433803ba9 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Wed, 9 Aug 2023 16:13:12 +0000 Subject: [PATCH 10/11] ".git/.scripts/commands/fmt/fmt.sh" --- frame/election-provider-multi-phase/src/lib.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 7ce9a8ce08bd7..180efa49a4cfa 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1288,12 +1288,7 @@ pub mod pallet { #[pallet::getter(fn snapshot)] pub type Snapshot = StorageValue< _, - RoundSnapshot< - T::AccountId, - VoterOf, - T::MaxElectingVoters, - T::MaxElectableTargets, - >, + RoundSnapshot, T::MaxElectingVoters, T::MaxElectableTargets>, >; /// Desired number of targets to elect for this round. From 4df72e33ce683f8240b884182ff146bf9ddde031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Wed, 9 Aug 2023 18:26:43 +0200 Subject: [PATCH 11/11] Adds debug_assert for hard/soft bounds --- frame/staking/src/pallet/impls.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 53c1c17e947d5..b5d7b93e64716 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -777,6 +777,9 @@ impl Pallet { let mut all_voters = BoundedVec::<_, MaxElectingVotersOf>::with_max_capacity(); + debug_assert!(all_voters.capacity() > max_allowed_len, + "voter soft bounds are larger than hard bounds from bounded vec, result may be truncated to hard bounds."); + // cache a few things. let weight_of = Self::weight_of_fn(); @@ -872,6 +875,9 @@ impl Pallet { let mut all_targets = BoundedVec::>::with_max_capacity(); let mut targets_seen = 0; + debug_assert!(all_targets.capacity() > max_allowed_len, + "target soft bounds are larger than hard bounds from bounded vec, result may be truncated to hard bounds."); + let mut targets_iter = T::TargetList::iter(); while all_targets.len() < max_allowed_len && targets_seen < (NPOS_MAX_ITERATIONS_COEFFICIENT * max_allowed_len as u32)