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

clean the interface of supports map #9674

Merged
5 commits merged into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion frame/election-provider-multi-phase/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn solution_with_size<T: Config>(

let solution =
<SolutionOf<T>>::from_assignment(&assignments, &voter_index, &target_index).unwrap();
let score = solution.clone().score(&winners, stake_of, voter_at, target_at).unwrap();
let score = solution.clone().score(stake_of, voter_at, target_at).unwrap();
let round = <MultiPhase<T>>::round();

assert!(score[0] > 0, "score is zero, this probably means that the stakes are not set.");
Expand Down
25 changes: 5 additions & 20 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,6 @@ pub enum FeasibilityError {
InvalidVote,
/// A voter is invalid.
InvalidVoter,
/// A winner is invalid.
InvalidWinner,
/// The given score was invalid.
InvalidScore,
/// The provided round is incorrect.
Expand Down Expand Up @@ -1395,17 +1393,8 @@ impl<T: Config> Pallet<T> {
let target_at = helpers::target_at_fn::<T>(&snapshot_targets);
let voter_index = helpers::voter_index_fn_usize::<T>(&cache);

// First, make sure that all the winners are sane.
// OPTIMIZATION: we could first build the assignments, and then extract the winners directly
// from that, as that would eliminate a little bit of duplicate work. For now, we keep them
// separate: First extract winners separately from solution, and then assignments. This is
// also better, because we can reject solutions that don't meet `desired_targets` early on.
let winners = winners
.into_iter()
.map(|i| target_at(i).ok_or(FeasibilityError::InvalidWinner))
.collect::<Result<Vec<T::AccountId>, FeasibilityError>>()?;

// Then convert solution -> assignment. This will fail if any of the indices are gibberish.
// Then convert solution -> assignment. This will fail if any of the indices are gibberish,
// namely any of the voters or targets.
let assignments = solution
.into_assignment(voter_at, target_at)
.map_err::<FeasibilityError, _>(Into::into)?;
Expand Down Expand Up @@ -1441,14 +1430,10 @@ impl<T: Config> Pallet<T> {
// This might fail if the normalization fails. Very unlikely. See `integrity_test`.
let staked_assignments = assignment_ratio_to_staked_normalized(assignments, stake_of)
.map_err::<FeasibilityError, _>(Into::into)?;

// This might fail if one of the voter edges is pointing to a non-winner, which is not
// really possible anymore because all the winners come from the same `solution`.
let supports = sp_npos_elections::to_supports(&winners, &staked_assignments)
.map_err::<FeasibilityError, _>(Into::into)?;
let supports = sp_npos_elections::to_supports(&staked_assignments);

// Finally, check that the claimed score was indeed correct.
let known_score = (&supports).evaluate();
let known_score = supports.evaluate();
ensure!(known_score == score, FeasibilityError::InvalidScore);

Ok(ReadySolution { supports, compute, score })
Expand Down Expand Up @@ -1653,7 +1638,7 @@ mod feasibility_check {
});
assert_noop!(
MultiPhase::feasibility_check(raw, COMPUTE),
FeasibilityError::InvalidWinner
FeasibilityError::NposElection(sp_npos_elections::Error::SolutionInvalidIndex)
);
})
}
Expand Down
23 changes: 11 additions & 12 deletions frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use sp_core::{
H256,
};
use sp_npos_elections::{
assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, to_without_backing,
ElectionResult, EvaluateSupport, ExtendedBalance, NposSolution,
assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, ElectionResult,
EvaluateSupport, ExtendedBalance, NposSolution,
};
use sp_runtime::{
testing::Header,
Expand Down Expand Up @@ -157,25 +157,24 @@ pub fn raw_solution() -> RawSolution<SolutionOf<Runtime>> {
let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap();
let desired_targets = MultiPhase::desired_targets().unwrap();

let ElectionResult { winners, assignments } = seq_phragmen::<_, SolutionAccuracyOf<Runtime>>(
desired_targets as usize,
targets.clone(),
voters.clone(),
None,
)
.unwrap();
let ElectionResult { winners: _, assignments } =
seq_phragmen::<_, SolutionAccuracyOf<Runtime>>(
desired_targets as usize,
targets.clone(),
voters.clone(),
None,
)
.unwrap();

// closures
let cache = helpers::generate_voter_cache::<Runtime>(&voters);
let voter_index = helpers::voter_index_fn_linear::<Runtime>(&voters);
let target_index = helpers::target_index_fn_linear::<Runtime>(&targets);
let stake_of = helpers::stake_of_fn::<Runtime>(&voters, &cache);

let winners = to_without_backing(winners);

let score = {
let staked = assignment_ratio_to_staked_normalized(assignments.clone(), &stake_of).unwrap();
to_supports(&winners, &staked).unwrap().evaluate()
to_supports(&staked).evaluate()
};
let solution =
<SolutionOf<Runtime>>::from_assignment(&assignments, &voter_index, &target_index).unwrap();
Expand Down
3 changes: 1 addition & 2 deletions frame/election-provider-multi-phase/src/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use codec::{Decode, Encode, HasCompact};
use frame_support::{
storage::bounded_btree_map::BoundedBTreeMap,
traits::{Currency, Get, OnUnbalanced, ReservableCurrency},
DebugNoBound,
};
use sp_arithmetic::traits::SaturatedConversion;
use sp_npos_elections::{is_score_better, ElectionScore, NposSolution};
Expand Down Expand Up @@ -113,7 +112,7 @@ pub enum InsertResult<T: Config> {
/// Mask type which pretends to be a set of `SignedSubmissionOf<T>`, while in fact delegating to the
/// actual implementations in `SignedSubmissionIndices<T>`, `SignedSubmissionsMap<T>`, and
/// `SignedSubmissionNextIndex<T>`.
#[cfg_attr(feature = "std", derive(DebugNoBound))]
#[cfg_attr(feature = "std", derive(frame_support::DebugNoBound))]
pub struct SignedSubmissions<T: Config> {
indices: SubmissionIndicesOf<T>,
next_idx: u32,
Expand Down
5 changes: 2 additions & 3 deletions frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ impl<T: Config> Pallet<T> {
SolutionOf::<T>::try_from(assignments).map(|s| s.encoded_size())
};

let ElectionResult { assignments, winners } = election_result;
let ElectionResult { assignments, winners: _ } = election_result;

// Reduce (requires round-trip to staked form)
let sorted_assignments = {
Expand Down Expand Up @@ -374,8 +374,7 @@ impl<T: Config> Pallet<T> {
let solution = SolutionOf::<T>::try_from(&index_assignments)?;

// re-calc score.
let winners = sp_npos_elections::to_without_backing(winners);
let score = solution.clone().score(&winners, stake_of, voter_at, target_at)?;
let score = solution.clone().score(stake_of, voter_at, target_at)?;

let round = Self::round();
Ok((RawSolution { solution, score, round }, size))
Expand Down
5 changes: 2 additions & 3 deletions frame/election-provider-support/src/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,13 @@ impl<T: Config> ElectionProvider<T::AccountId, T::BlockNumber> for OnChainSequen
let stake_of =
|w: &T::AccountId| -> VoteWeight { stake_map.get(w).cloned().unwrap_or_default() };

let ElectionResult { winners, assignments } =
let ElectionResult { winners: _, assignments } =
seq_phragmen::<_, T::Accuracy>(desired_targets as usize, targets, voters, None)
.map_err(Error::from)?;

let staked = assignment_ratio_to_staked_normalized(assignments, &stake_of)?;
let winners = to_without_backing(winners);

to_supports(&winners, &staked).map_err(Error::from)
Ok(to_supports(&staked))
}
}

Expand Down
11 changes: 6 additions & 5 deletions frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ pub use impls::*;
use crate::{
migrations, slashing, weights::WeightInfo, ActiveEraInfo, BalanceOf, EraIndex, EraPayout,
EraRewardPoints, Exposure, Forcing, NegativeImbalanceOf, Nominations, PositiveImbalanceOf,
Releases, RewardDestination, SessionInterface, StakerStatus, StakingLedger, UnappliedSlash,
UnlockChunk, ValidatorPrefs,
Releases, RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk,
ValidatorPrefs,
};

pub const MAX_UNLOCKING_CHUNKS: usize = 32;
Expand Down Expand Up @@ -453,7 +453,8 @@ pub mod pallet {
pub force_era: Forcing,
pub slash_reward_fraction: Perbill,
pub canceled_payout: BalanceOf<T>,
pub stakers: Vec<(T::AccountId, T::AccountId, BalanceOf<T>, StakerStatus<T::AccountId>)>,
pub stakers:
Vec<(T::AccountId, T::AccountId, BalanceOf<T>, crate::StakerStatus<T::AccountId>)>,
pub min_nominator_bond: BalanceOf<T>,
pub min_validator_bond: BalanceOf<T>,
}
Expand Down Expand Up @@ -502,11 +503,11 @@ pub mod pallet {
RewardDestination::Staked,
));
frame_support::assert_ok!(match status {
StakerStatus::Validator => <Pallet<T>>::validate(
crate::StakerStatus::Validator => <Pallet<T>>::validate(
T::Origin::from(Some(controller.clone()).into()),
Default::default(),
),
StakerStatus::Nominator(votes) => <Pallet<T>>::nominate(
crate::StakerStatus::Nominator(votes) => <Pallet<T>>::nominate(
T::Origin::from(Some(controller.clone()).into()),
votes.iter().map(|l| T::Lookup::unlookup(l.clone())).collect(),
),
Expand Down
8 changes: 3 additions & 5 deletions primitives/npos-elections/fuzzer/src/phragmen_balancing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use honggfuzz::fuzz;
use rand::{self, SeedableRng};
use sp_npos_elections::{
assignment_ratio_to_staked_normalized, is_score_better, seq_phragmen, to_supports,
to_without_backing, EvaluateSupport, VoteWeight,
EvaluateSupport, VoteWeight,
};
use sp_runtime::Perbill;

Expand Down Expand Up @@ -58,8 +58,7 @@ fn main() {
&stake_of,
)
.unwrap();
let winners = to_without_backing(unbalanced.winners.clone());
let score = to_supports(winners.as_ref(), staked.as_ref()).unwrap().evaluate();
let score = to_supports(staked.as_ref()).evaluate();

if score[0] == 0 {
// such cases cannot be improved by balancing.
Expand All @@ -83,8 +82,7 @@ fn main() {
&stake_of,
)
.unwrap();
let winners = to_without_backing(balanced.winners);
to_supports(winners.as_ref(), staked.as_ref()).unwrap().evaluate()
to_supports(staked.as_ref()).evaluate()
};

let enhance = is_score_better(balanced_score, unbalanced_score, Perbill::zero());
Expand Down
10 changes: 4 additions & 6 deletions primitives/npos-elections/fuzzer/src/phragmms_balancing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use common::*;
use honggfuzz::fuzz;
use rand::{self, SeedableRng};
use sp_npos_elections::{
assignment_ratio_to_staked_normalized, is_score_better, phragmms, to_supports,
to_without_backing, EvaluateSupport, VoteWeight,
assignment_ratio_to_staked_normalized, is_score_better, phragmms, to_supports, EvaluateSupport,
VoteWeight,
};
use sp_runtime::Perbill;

Expand Down Expand Up @@ -58,8 +58,7 @@ fn main() {
&stake_of,
)
.unwrap();
let winners = to_without_backing(unbalanced.winners.clone());
let score = to_supports(&winners, &staked).unwrap().evaluate();
let score = to_supports(&staked).evaluate();

if score[0] == 0 {
// such cases cannot be improved by balancing.
Expand All @@ -80,8 +79,7 @@ fn main() {
let staked =
assignment_ratio_to_staked_normalized(balanced.assignments.clone(), &stake_of)
.unwrap();
let winners = to_without_backing(balanced.winners);
to_supports(winners.as_ref(), staked.as_ref()).unwrap().evaluate()
to_supports(staked.as_ref()).evaluate()
};

let enhance = is_score_better(balanced_score, unbalanced_score, Perbill::zero());
Expand Down
8 changes: 3 additions & 5 deletions primitives/npos-elections/fuzzer/src/reduce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,11 @@ fn generate_random_phragmen_assignment(
}

fn assert_assignments_equal(
winners: &Vec<AccountId>,
ass1: &Vec<StakedAssignment<AccountId>>,
ass2: &Vec<StakedAssignment<AccountId>>,
) {
let support_1 = to_support_map::<AccountId>(winners, ass1).unwrap();
let support_2 = to_support_map::<AccountId>(winners, ass2).unwrap();

let support_1 = to_support_map::<AccountId>(ass1);
let support_2 = to_support_map::<AccountId>(ass2);
for (who, support) in support_1.iter() {
assert_eq!(support.total, support_2.get(who).unwrap().total);
}
Expand All @@ -134,7 +132,7 @@ fn reduce_and_compare(assignment: &Vec<StakedAssignment<AccountId>>, winners: &V
num_changed,
);

assert_assignments_equal(winners, &assignment, &altered_assignment);
assert_assignments_equal(&assignment, &altered_assignment);
}

fn assignment_len(assignments: &[StakedAssignment<AccountId>]) -> u32 {
Expand Down
9 changes: 1 addition & 8 deletions primitives/npos-elections/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@

//! Helper methods for npos-elections.

use crate::{
Assignment, Error, IdentifierT, PerThing128, StakedAssignment, VoteWeight, WithApprovalOf,
};
use crate::{Assignment, Error, IdentifierT, PerThing128, StakedAssignment, VoteWeight};
use sp_arithmetic::PerThing;
use sp_std::prelude::*;

Expand Down Expand Up @@ -81,11 +79,6 @@ pub fn assignment_staked_to_ratio_normalized<A: IdentifierT, P: PerThing128>(
Ok(ratio)
}

/// consumes a vector of winners with backing stake to just winners.
pub fn to_without_backing<A: IdentifierT>(winners: Vec<WithApprovalOf<A>>) -> Vec<A> {
winners.into_iter().map(|(who, _)| who).collect::<Vec<A>>()
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
Loading