Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use proper bounded vector type for nominations #10601

Merged
merged 27 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5a990c4
Use proper bounded vector type for nominations
kianenigma Jan 6, 2022
fb60d30
add docs and tweak chill_other for cleanup purposes
kianenigma Jan 6, 2022
b96f3af
Fix the build
kianenigma Jan 6, 2022
1b8376d
remove TODO
kianenigma Jan 6, 2022
b93f109
add a bit more doc
kianenigma Jan 6, 2022
d3cb930
even more docs
kianenigma Jan 6, 2022
b55f9e2
Update frame/staking/src/pallet/mod.rs
kianenigma Jan 7, 2022
85ff60f
Update frame/staking/src/pallet/mod.rs
kianenigma Jan 7, 2022
ef51ec4
Fix the nasty bug
kianenigma Jan 7, 2022
6f5db81
Merge branch 'kiz-bound-nominator-votes' of github.com:paritytech/sub…
kianenigma Jan 7, 2022
5ec63fb
Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…
kianenigma Jan 7, 2022
477079c
also bound the Snapshot type
kianenigma Jan 7, 2022
419354c
fix doc test
kianenigma Jan 7, 2022
ec6b11b
document bounded_vec
kianenigma Jan 7, 2022
2864d22
self-review
kianenigma Jan 7, 2022
7ca67ee
remove unused
kianenigma Jan 8, 2022
c868c41
Fix build
kianenigma Jan 8, 2022
104ba4a
frame-support: repetition overload for bounded_vec
ggwpez Jan 12, 2022
eb8904e
fix
kianenigma Jan 19, 2022
587f715
remove the need to allocate into unbounded voters etc etc
kianenigma Jan 19, 2022
88cb4a6
Don't expect
kianenigma Jan 19, 2022
17bad6a
Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…
kianenigma Jan 19, 2022
437722a
Master.into()
kianenigma Jan 19, 2022
a831b08
unbreal the build again
kianenigma Jan 19, 2022
ac7fc12
Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…
kianenigma Jan 20, 2022
5e1b8f0
handle macro a bit better
kianenigma Jan 20, 2022
7fe85d2
Merge branch 'master' of github.com:paritytech/substrate into kiz-bou…
kianenigma Jan 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions bin/node/cli/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use hex_literal::hex;
use node_runtime::{
constants::currency::*, wasm_binary_unwrap, AuthorityDiscoveryConfig, BabeConfig,
BalancesConfig, Block, CouncilConfig, DemocracyConfig, ElectionsConfig, GrandpaConfig,
ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, SocietyConfig, StakerStatus,
StakingConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig, MAX_NOMINATIONS,
ImOnlineConfig, IndicesConfig, MaxNominations, SessionConfig, SessionKeys, SocietyConfig,
StakerStatus, StakingConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig,
};
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use sc_chain_spec::ChainSpecExtension;
Expand Down Expand Up @@ -278,7 +278,7 @@ pub fn testnet_genesis(
.map(|x| (x.0.clone(), x.1.clone(), STASH, StakerStatus::Validator))
.chain(initial_nominators.iter().map(|x| {
use rand::{seq::SliceRandom, Rng};
let limit = (MAX_NOMINATIONS as usize).min(initial_authorities.len());
let limit = (MaxNominations::get() as usize).min(initial_authorities.len());
let count = rng.gen::<usize>() % limit;
let nominations = initial_authorities
.as_slice()
Expand Down
8 changes: 5 additions & 3 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ impl pallet_staking::BenchmarkingConfig for StakingBenchmarkingConfig {
}

impl pallet_staking::Config for Runtime {
const MAX_NOMINATIONS: u32 = MAX_NOMINATIONS;
type MaxNominations = MaxNominations;
type Currency = Balances;
type UnixTime = Timestamp;
type CurrencyToVote = U128CurrencyToVote;
Expand Down Expand Up @@ -605,7 +605,9 @@ sp_npos_elections::generate_solution_type!(
>(16)
);

pub const MAX_NOMINATIONS: u32 = <NposSolution16 as sp_npos_elections::NposSolution>::LIMIT as u32;
parameter_types! {
pub MaxNominations: u32 = <NposSolution16 as sp_npos_elections::NposSolution>::LIMIT as u32;
}

/// The numbers configured here could always be more than the the maximum limits of staking pallet
/// to ensure election snapshot will not run out of memory. For now, we set them to smaller values
Expand Down Expand Up @@ -1792,7 +1794,7 @@ mod tests {
#[test]
fn perbill_as_onchain_accuracy() {
type OnChainAccuracy = <Runtime as onchain::Config>::Accuracy;
let maximum_chain_accuracy: Vec<UpperOf<OnChainAccuracy>> = (0..MAX_NOMINATIONS)
let maximum_chain_accuracy: Vec<UpperOf<OnChainAccuracy>> = (0..MaxNominations::get())
.map(|_| <UpperOf<OnChainAccuracy>>::from(OnChainAccuracy::one().deconstruct()))
.collect();
let _: UpperOf<OnChainAccuracy> =
Expand Down
2 changes: 1 addition & 1 deletion frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl onchain::Config for Test {
}

impl pallet_staking::Config for Test {
const MAX_NOMINATIONS: u32 = 16;
type MaxNominations = ConstU32<16>;
type RewardRemainder = ();
type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote;
type Event = Event;
Expand Down
26 changes: 17 additions & 9 deletions frame/election-provider-multi-phase/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
use super::*;
use crate::{unsigned::IndexAssignmentOf, Pallet as MultiPhase};
use frame_benchmarking::account;
use frame_support::{assert_ok, traits::Hooks};
use frame_support::{
assert_ok,
traits::{Hooks, TryCollect},
BoundedVec,
};
use frame_system::RawOrigin;
use rand::{prelude::SliceRandom, rngs::SmallRng, SeedableRng};
use sp_arithmetic::{per_things::Percent, traits::One};
Expand Down Expand Up @@ -69,11 +73,12 @@ fn solution_with_size<T: Config>(
let active_voters = (0..active_voters_count)
.map(|i| {
// chose a random subset of winners.
let winner_votes = winners
let winner_votes: BoundedVec<_, _> = winners
.as_slice()
.choose_multiple(&mut rng, <SolutionOf<T>>::LIMIT)
.cloned()
.collect::<Vec<_>>();
.try_collect()
.expect("<SolutionOf<T>>::LIMIT is the correct bound; qed.");
let voter = frame_benchmarking::account::<T::AccountId>("Voter", i, SEED);
(voter, stake, winner_votes)
})
Expand All @@ -87,10 +92,11 @@ fn solution_with_size<T: Config>(
.collect::<Vec<T::AccountId>>();
let rest_voters = (active_voters_count..size.voters)
.map(|i| {
let votes = (&non_winners)
let votes: BoundedVec<_, _> = (&non_winners)
.choose_multiple(&mut rng, <SolutionOf<T>>::LIMIT)
.cloned()
.collect::<Vec<T::AccountId>>();
.try_collect()
.expect("<SolutionOf<T>>::LIMIT is the correct bound; qed.");
let voter = frame_benchmarking::account::<T::AccountId>("Voter", i, SEED);
(voter, stake, votes)
})
Expand Down Expand Up @@ -152,7 +158,7 @@ fn set_up_data_provider<T: Config>(v: u32, t: u32) {
info,
"setting up with voters = {} [degree = {}], targets = {}",
v,
T::DataProvider::MAXIMUM_VOTES_PER_VOTER,
<T::DataProvider as ElectionDataProvider>::MaxVotesPerVoter::get(),
t
);

Expand All @@ -165,14 +171,16 @@ fn set_up_data_provider<T: Config>(v: u32, t: u32) {
})
.collect::<Vec<_>>();
// we should always have enough voters to fill.
assert!(targets.len() > T::DataProvider::MAXIMUM_VOTES_PER_VOTER as usize);
targets.truncate(T::DataProvider::MAXIMUM_VOTES_PER_VOTER as usize);
assert!(
targets.len() > <T::DataProvider as ElectionDataProvider>::MaxVotesPerVoter::get() as usize
);
targets.truncate(<T::DataProvider as ElectionDataProvider>::MaxVotesPerVoter::get() as usize);

// fill voters.
(0..v).for_each(|i| {
let voter = frame_benchmarking::account::<T::AccountId>("Voter", i, SEED);
let weight = T::Currency::minimum_balance().saturated_into::<u64>() * 1000;
T::DataProvider::add_voter(voter, weight, targets.clone());
T::DataProvider::add_voter(voter, weight, targets.clone().try_into().unwrap());
});
}

Expand Down
12 changes: 6 additions & 6 deletions frame/election-provider-multi-phase/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! Some helper functions/macros for this crate.

use super::{Config, SolutionTargetIndexOf, SolutionVoterIndexOf, VoteWeight};
use crate::{unsigned::VoterOf, Config, SolutionTargetIndexOf, SolutionVoterIndexOf, VoteWeight};
use sp_std::{collections::btree_map::BTreeMap, prelude::*};

#[macro_export]
Expand All @@ -34,7 +34,7 @@ macro_rules! log {
///
/// This can be used to efficiently build index getter closures.
pub fn generate_voter_cache<T: Config>(
snapshot: &Vec<(T::AccountId, VoteWeight, Vec<T::AccountId>)>,
snapshot: &Vec<VoterOf<T>>,
) -> BTreeMap<T::AccountId, usize> {
let mut cache: BTreeMap<T::AccountId, usize> = BTreeMap::new();
snapshot.iter().enumerate().for_each(|(i, (x, _, _))| {
Expand Down Expand Up @@ -97,7 +97,7 @@ pub fn voter_index_fn_usize<T: Config>(
/// Not meant to be used in production.
#[cfg(test)]
pub fn voter_index_fn_linear<T: Config>(
snapshot: &Vec<(T::AccountId, VoteWeight, Vec<T::AccountId>)>,
snapshot: &Vec<VoterOf<T>>,
) -> impl Fn(&T::AccountId) -> Option<SolutionVoterIndexOf<T>> + '_ {
move |who| {
snapshot
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn target_index_fn_linear<T: Config>(
/// Create a function that can map a voter index ([`SolutionVoterIndexOf`]) to the actual voter
/// account using a linearly indexible snapshot.
pub fn voter_at_fn<T: Config>(
snapshot: &Vec<(T::AccountId, VoteWeight, Vec<T::AccountId>)>,
snapshot: &Vec<VoterOf<T>>,
) -> impl Fn(SolutionVoterIndexOf<T>) -> Option<T::AccountId> + '_ {
move |i| {
<SolutionVoterIndexOf<T> as TryInto<usize>>::try_into(i)
Expand All @@ -174,7 +174,7 @@ pub fn target_at_fn<T: Config>(
/// This is not optimized and uses a linear search.
#[cfg(test)]
pub fn stake_of_fn_linear<T: Config>(
snapshot: &Vec<(T::AccountId, VoteWeight, Vec<T::AccountId>)>,
snapshot: &Vec<VoterOf<T>>,
) -> impl Fn(&T::AccountId) -> VoteWeight + '_ {
move |who| {
snapshot
Expand All @@ -192,7 +192,7 @@ pub fn stake_of_fn_linear<T: Config>(
/// The cache need must be derived from the same snapshot. Zero is returned if a voter is
/// non-existent.
pub fn stake_of_fn<'a, T: Config>(
snapshot: &'a Vec<(T::AccountId, VoteWeight, Vec<T::AccountId>)>,
snapshot: &'a Vec<VoterOf<T>>,
cache: &'a BTreeMap<T::AccountId, usize>,
) -> impl Fn(&T::AccountId) -> VoteWeight + 'a {
move |who| {
Expand Down
19 changes: 11 additions & 8 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ const LOG_TARGET: &'static str = "runtime::election-provider";
pub mod signed;
pub mod unsigned;
pub mod weights;
use unsigned::VoterOf;
pub use weights::WeightInfo;

pub use signed::{
Expand Down Expand Up @@ -448,11 +449,13 @@ pub struct ReadySolution<A> {
///
/// These are stored together because they are often accessed together.
#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, TypeInfo)]
pub struct RoundSnapshot<A> {
#[codec(mel_bound(T: Config))]
#[scale_info(skip_type_params(T))]
pub struct RoundSnapshot<T: Config> {
/// All of the voters.
pub voters: Vec<(A, VoteWeight, Vec<A>)>,
pub voters: Vec<VoterOf<T>>,
/// All of the targets.
pub targets: Vec<A>,
pub targets: Vec<T::AccountId>,
}

/// Encodes the length of a solution or a snapshot.
Expand Down Expand Up @@ -820,7 +823,7 @@ pub mod pallet {
// NOTE that this pallet does not really need to enforce this in runtime. The
// solution cannot represent any voters more than `LIMIT` anyhow.
assert_eq!(
<T::DataProvider as ElectionDataProvider>::MAXIMUM_VOTES_PER_VOTER,
<T::DataProvider as ElectionDataProvider>::MaxVotesPerVoter::get(),
<SolutionOf<T> as NposSolution>::LIMIT as u32,
);
}
Expand Down Expand Up @@ -1140,7 +1143,7 @@ pub mod pallet {
/// This is created at the beginning of the signed phase and cleared upon calling `elect`.
#[pallet::storage]
#[pallet::getter(fn snapshot)]
pub type Snapshot<T: Config> = StorageValue<_, RoundSnapshot<T::AccountId>>;
pub type Snapshot<T: Config> = StorageValue<_, RoundSnapshot<T>>;

/// Desired number of targets to elect for this round.
///
Expand Down Expand Up @@ -1256,7 +1259,7 @@ impl<T: Config> Pallet<T> {
/// Extracted for easier weight calculation.
fn create_snapshot_internal(
targets: Vec<T::AccountId>,
voters: Vec<crate::unsigned::Voter<T>>,
voters: Vec<VoterOf<T>>,
desired_targets: u32,
) {
let metadata =
Expand All @@ -1269,7 +1272,7 @@ impl<T: Config> Pallet<T> {
// 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> { voters, targets };
let size = snapshot.encoded_size();
log!(debug, "snapshot pre-calculated size {:?}", size);
let mut buffer = Vec::with_capacity(size);
Expand All @@ -1287,7 +1290,7 @@ impl<T: Config> Pallet<T> {
///
/// Extracted for easier weight calculation.
fn create_snapshot_external(
) -> Result<(Vec<T::AccountId>, Vec<crate::unsigned::Voter<T>>, u32), ElectionError<T>> {
) -> Result<(Vec<T::AccountId>, Vec<VoterOf<T>>, u32), ElectionError<T>> {
let target_limit = <SolutionTargetIndexOf<T>>::max_value().saturated_into::<usize>();
// for now we have just a single block snapshot.
let voter_limit = T::VoterSnapshotPerBlock::get().saturated_into::<usize>();
Expand Down
Loading