diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 2ce19483e5539e972fe5a345cd51994e570688c4..fd7fd4213366f74b752fdd0ba5a3da348747519c 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -73,6 +73,7 @@ use pallet_session::{historical as pallet_session_historical};
use sp_inherents::{InherentData, CheckInherentsResult};
use static_assertions::const_assert;
use pallet_contracts::weights::WeightInfo;
+use pallet_election_provider_multi_phase::FallbackStrategy;
#[cfg(any(feature = "std", test))]
pub use sp_runtime::BuildStorage;
@@ -516,9 +517,14 @@ parameter_types! {
pub const SignedPhase: u32 = EPOCH_DURATION_IN_BLOCKS / 4;
pub const UnsignedPhase: u32 = EPOCH_DURATION_IN_BLOCKS / 4;
- // fallback: no need to do on-chain phragmen initially.
- pub const Fallback: pallet_election_provider_multi_phase::FallbackStrategy =
- pallet_election_provider_multi_phase::FallbackStrategy::Nothing;
+ // signed config
+ pub const SignedMaxSubmissions: u32 = 10;
+ pub const SignedRewardBase: Balance = 1 * DOLLARS;
+ pub const SignedDepositBase: Balance = 1 * DOLLARS;
+ pub const SignedDepositByte: Balance = 1 * CENTS;
+
+ // fallback: no on-chain fallback.
+ pub const Fallback: FallbackStrategy = FallbackStrategy::Nothing;
pub SolutionImprovementThreshold: Perbill = Perbill::from_rational(1u32, 10_000);
@@ -559,6 +565,14 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type MinerMaxWeight = MinerMaxWeight;
type MinerMaxLength = MinerMaxLength;
type MinerTxPriority = MultiPhaseUnsignedPriority;
+ type SignedMaxSubmissions = SignedMaxSubmissions;
+ type SignedRewardBase = SignedRewardBase;
+ type SignedDepositBase = SignedDepositBase;
+ type SignedDepositByte = SignedDepositByte;
+ type SignedDepositWeight = ();
+ type SignedMaxWeight = MinerMaxWeight;
+ type SlashHandler = (); // burn slashes
+ type RewardHandler = (); // nothing to do upon rewards
type DataProvider = Staking;
type OnChainAccuracy = Perbill;
type CompactSolution = NposCompactSolution16;
@@ -1556,6 +1570,7 @@ impl_runtime_apis! {
add_benchmark!(params, batches, pallet_uniques, Uniques);
add_benchmark!(params, batches, pallet_utility, Utility);
add_benchmark!(params, batches, pallet_vesting, Vesting);
+ add_benchmark!(params, batches, pallet_election_provider_multi_phase, ElectionProviderMultiPhase);
if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) }
Ok(batches)
diff --git a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
index 4eade8e184e75e60a8726a8b0d360a34abbe8855..7988163e98f65bad603ad3c3c70dc82b48f43e47 100644
--- a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
+++ b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
@@ -19,7 +19,7 @@
use super::*;
use crate::{Pallet as MultiPhase, unsigned::IndexAssignmentOf};
-use frame_benchmarking::impl_benchmark_test_suite;
+use frame_benchmarking::{account, impl_benchmark_test_suite};
use frame_support::{assert_ok, traits::OnInitialize};
use frame_system::RawOrigin;
use rand::{prelude::SliceRandom, rngs::SmallRng, SeedableRng};
@@ -57,7 +57,7 @@ fn solution_with_size<T: Config>(
let targets: Vec<T::AccountId> =
(0..size.targets).map(|i| frame_benchmarking::account("Targets", i, SEED)).collect();
- let mut rng = SmallRng::seed_from_u64(SEED as u64);
+ let mut rng = SmallRng::seed_from_u64(SEED.into());
// decide who are the winners.
let winners = targets
@@ -176,6 +176,39 @@ frame_benchmarking::benchmarks! {
assert!(<MultiPhase<T>>::current_phase().is_unsigned());
}
+ finalize_signed_phase_accept_solution {
+ let receiver = account("receiver", 0, SEED);
+ let initial_balance = T::Currency::minimum_balance() * 10u32.into();
+ T::Currency::make_free_balance_be(&receiver, initial_balance);
+ let ready: ReadySolution<T::AccountId> = Default::default();
+ let deposit: BalanceOf<T> = 10u32.into();
+ let reward: BalanceOf<T> = 20u32.into();
+
+ assert_ok!(T::Currency::reserve(&receiver, deposit));
+ assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into());
+ }: {
+ <MultiPhase<T>>::finalize_signed_phase_accept_solution(ready, &receiver, deposit, reward)
+ } verify {
+ assert_eq!(T::Currency::free_balance(&receiver), initial_balance + 20u32.into());
+ assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into());
+ }
+
+ finalize_signed_phase_reject_solution {
+ let receiver = account("receiver", 0, SEED);
+ let initial_balance = T::Currency::minimum_balance().max(One::one()) * 10u32.into();
+ let deposit: BalanceOf<T> = 10u32.into();
+ T::Currency::make_free_balance_be(&receiver, initial_balance);
+ assert_ok!(T::Currency::reserve(&receiver, deposit));
+
+ assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into());
+ assert_eq!(T::Currency::reserved_balance(&receiver), 10u32.into());
+ }: {
+ <MultiPhase<T>>::finalize_signed_phase_reject_solution(&receiver, deposit)
+ } verify {
+ assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into());
+ assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into());
+ }
+
on_initialize_open_unsigned_without_snapshot {
// need to assume signed phase was open before
<MultiPhase<T>>::on_initialize_open_signed().unwrap();
@@ -227,6 +260,38 @@ frame_benchmarking::benchmarks! {
assert!(<MultiPhase<T>>::snapshot().is_some());
}
+ submit {
+ let c in 1 .. (T::SignedMaxSubmissions::get() - 1);
+
+ // the solution will be worse than all of them meaning the score need to be checked against
+ // ~ log2(c)
+ let solution = RawSolution {
+ score: [(10_000_000u128 - 1).into(), 0, 0],
+ ..Default::default()
+ };
+
+ MultiPhase::<T>::on_initialize_open_signed().expect("should be ok to start signed phase");
+ <Round<T>>::put(1);
+
+ let mut signed_submissions = SignedSubmissions::<T>::get();
+ for i in 0..c {
+ let solution = RawSolution {
+ score: [(10_000_000 + i).into(), 0, 0],
+ ..Default::default()
+ };
+ let signed_submission = SignedSubmission { solution, ..Default::default() };
+ signed_submissions.insert(signed_submission);
+ }
+ signed_submissions.put();
+
+ let caller = frame_benchmarking::whitelisted_caller();
+ T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() * 10u32.into());
+
+ }: _(RawOrigin::Signed(caller), solution, c)
+ verify {
+ assert!(<MultiPhase<T>>::signed_submissions().len() as u32 == c + 1);
+ }
+
submit_unsigned {
// number of votes in snapshot.
let v in (T::BenchmarkingConfig::VOTERS[0]) .. T::BenchmarkingConfig::VOTERS[1];
@@ -234,9 +299,12 @@ frame_benchmarking::benchmarks! {
let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
// number of assignments, i.e. compact.len(). This means the active nominators, thus must be
// a subset of `v` component.
- let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
+ let a in
+ (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
// number of desired targets. Must be a subset of `t` component.
- let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1];
+ let d in
+ (T::BenchmarkingConfig::DESIRED_TARGETS[0]) ..
+ T::BenchmarkingConfig::DESIRED_TARGETS[1];
let witness = SolutionOrSnapshotSize { voters: v, targets: t };
let raw_solution = solution_with_size::<T>(witness, a, d);
@@ -249,7 +317,8 @@ frame_benchmarking::benchmarks! {
let encoded_call = <Call<T>>::submit_unsigned(raw_solution.clone(), witness).encode();
}: {
assert_ok!(<MultiPhase<T>>::submit_unsigned(RawOrigin::None.into(), raw_solution, witness));
- let _decoded_snap = <RoundSnapshot<T::AccountId> as Decode>::decode(&mut &*encoded_snapshot).unwrap();
+ let _decoded_snap = <RoundSnapshot<T::AccountId> as Decode>::decode(&mut &*encoded_snapshot)
+ .unwrap();
let _decoded_call = <Call<T> as Decode>::decode(&mut &*encoded_call).unwrap();
} verify {
assert!(<MultiPhase<T>>::queued_solution().is_some());
@@ -263,13 +332,17 @@ frame_benchmarking::benchmarks! {
let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
// number of assignments, i.e. compact.len(). This means the active nominators, thus must be
// a subset of `v` component.
- let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
+ let a in
+ (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
// number of desired targets. Must be a subset of `t` component.
- let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1];
+ let d in
+ (T::BenchmarkingConfig::DESIRED_TARGETS[0]) ..
+ T::BenchmarkingConfig::DESIRED_TARGETS[1];
// Subtract this percentage from the actual encoded size
let f in 0 .. 95;
- // Compute a random solution, then work backwards to get the lists of voters, targets, and assignments
+ // Compute a random solution, then work backwards to get the lists of voters, targets, and
+ // assignments
let witness = SolutionOrSnapshotSize { voters: v, targets: t };
let RawSolution { compact, .. } = solution_with_size::<T>(witness, a, d);
let RoundSnapshot { voters, targets } = MultiPhase::<T>::snapshot().unwrap();
@@ -313,7 +386,11 @@ frame_benchmarking::benchmarks! {
} verify {
let compact = CompactOf::<T>::try_from(index_assignments.as_slice()).unwrap();
let encoding = compact.encode();
- log!(trace, "encoded size prediction = {}", encoded_size_of(index_assignments.as_slice()).unwrap());
+ log!(
+ trace,
+ "encoded size prediction = {}",
+ encoded_size_of(index_assignments.as_slice()).unwrap(),
+ );
log!(trace, "actual encoded size = {}", encoding.len());
assert!(encoding.len() <= desired_size);
}
@@ -326,9 +403,12 @@ frame_benchmarking::benchmarks! {
let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
// number of assignments, i.e. compact.len(). This means the active nominators, thus must be
// a subset of `v` component.
- let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
+ let a in
+ (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
// number of desired targets. Must be a subset of `t` component.
- let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1];
+ let d in
+ (T::BenchmarkingConfig::DESIRED_TARGETS[0]) ..
+ T::BenchmarkingConfig::DESIRED_TARGETS[1];
let size = SolutionOrSnapshotSize { voters: v, targets: t };
let raw_solution = solution_with_size::<T>(size, a, d);
@@ -340,7 +420,8 @@ frame_benchmarking::benchmarks! {
let encoded_snapshot = <MultiPhase<T>>::snapshot().unwrap().encode();
}: {
assert_ok!(<MultiPhase<T>>::feasibility_check(raw_solution, ElectionCompute::Unsigned));
- let _decoded_snap = <RoundSnapshot<T::AccountId> as Decode>::decode(&mut &*encoded_snapshot).unwrap();
+ let _decoded_snap = <RoundSnapshot<T::AccountId> as Decode>::decode(&mut &*encoded_snapshot)
+ .unwrap();
}
}
diff --git a/substrate/frame/election-provider-multi-phase/src/helpers.rs b/substrate/frame/election-provider-multi-phase/src/helpers.rs
index bf5b360499cb404ab92c04fecfd9e6efafa641d7..46eeef0a6bf73800e6db878d48f295a59483d108 100644
--- a/substrate/frame/election-provider-multi-phase/src/helpers.rs
+++ b/substrate/frame/election-provider-multi-phase/src/helpers.rs
@@ -47,13 +47,13 @@ pub fn generate_voter_cache<T: Config>(
cache
}
-/// Create a function the returns the index a voter in the snapshot.
+/// Create a function that returns the index of a voter in the snapshot.
///
/// The returning index type is the same as the one defined in `T::CompactSolution::Voter`.
///
/// ## Warning
///
-/// The snapshot must be the same is the one used to create `cache`.
+/// Note that this will represent the snapshot data from which the `cache` is generated.
pub fn voter_index_fn<T: Config>(
cache: &BTreeMap<T::AccountId, usize>,
) -> impl Fn(&T::AccountId) -> Option<CompactVoterIndexOf<T>> + '_ {
@@ -78,7 +78,7 @@ pub fn voter_index_fn_owned<T: Config>(
///
/// ## Warning
///
-/// The snapshot must be the same is the one used to create `cache`.
+/// Note that this will represent the snapshot data from which the `cache` is generated.
pub fn voter_index_fn_usize<T: Config>(
cache: &BTreeMap<T::AccountId, usize>,
) -> impl Fn(&T::AccountId) -> Option<usize> + '_ {
@@ -103,7 +103,7 @@ pub fn voter_index_fn_linear<T: Config>(
}
}
-/// Create a function the returns the index to a target in the snapshot.
+/// Create a function that returns the index of a target in the snapshot.
///
/// The returned index type is the same as the one defined in `T::CompactSolution::Target`.
///
diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs
index 2864ca518d0687e5a0c5e55ea050445210146d62..45e04a757f0b38bc11ff03c2663f860871027d71 100644
--- a/substrate/frame/election-provider-multi-phase/src/lib.rs
+++ b/substrate/frame/election-provider-multi-phase/src/lib.rs
@@ -231,7 +231,7 @@ use codec::{Decode, Encode};
use frame_support::{
dispatch::DispatchResultWithPostInfo,
ensure,
- traits::{Currency, Get, ReservableCurrency},
+ traits::{Currency, Get, ReservableCurrency, OnUnbalanced},
weights::Weight,
};
use frame_system::{ensure_none, offchain::SendTransactionTypes};
@@ -266,10 +266,14 @@ pub mod helpers;
const LOG_TARGET: &'static str = "runtime::election-provider";
+pub mod signed;
pub mod unsigned;
pub mod weights;
-/// The weight declaration of the pallet.
+pub use signed::{
+ BalanceOf, NegativeImbalanceOf, PositiveImbalanceOf, SignedSubmission, SignedSubmissionOf,
+ SignedSubmissions, SubmissionIndicesOf,
+};
pub use weights::WeightInfo;
/// The compact solution type used by this crate.
@@ -411,7 +415,7 @@ impl Default for ElectionCompute {
///
/// Such a solution should never become effective in anyway before being checked by the
/// `Pallet::feasibility_check`
-#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, PartialOrd, Ord)]
pub struct RawSolution<C> {
/// Compact election edges.
pub compact: C,
@@ -583,6 +587,44 @@ pub mod pallet {
/// this value, based on [`WeightInfo::submit_unsigned`].
type MinerMaxWeight: Get<Weight>;
+ /// Maximum number of signed submissions that can be queued.
+ ///
+ /// It is best to avoid adjusting this during an election, as it impacts downstream data
+ /// structures. In particular, `SignedSubmissionIndices<T>` is bounded on this value. If you
+ /// update this value during an election, you _must_ ensure that
+ /// `SignedSubmissionIndices.len()` is less than or equal to the new value. Otherwise,
+ /// attempts to submit new solutions may cause a runtime panic.
+ #[pallet::constant]
+ type SignedMaxSubmissions: Get<u32>;
+
+ /// Maximum weight of a signed solution.
+ ///
+ /// This should probably be similar to [`Config::MinerMaxWeight`].
+ #[pallet::constant]
+ type SignedMaxWeight: Get<Weight>;
+
+ /// Base reward for a signed solution
+ #[pallet::constant]
+ type SignedRewardBase: Get<BalanceOf<Self>>;
+
+ /// Base deposit for a signed solution.
+ #[pallet::constant]
+ type SignedDepositBase: Get<BalanceOf<Self>>;
+
+ /// Per-byte deposit for a signed solution.
+ #[pallet::constant]
+ type SignedDepositByte: Get<BalanceOf<Self>>;
+
+ /// Per-weight deposit for a signed solution.
+ #[pallet::constant]
+ type SignedDepositWeight: Get<BalanceOf<Self>>;
+
+ /// Handler for the slashed deposits.
+ type SlashHandler: OnUnbalanced<NegativeImbalanceOf<Self>>;
+
+ /// Handler for the rewards.
+ type RewardHandler: OnUnbalanced<PositiveImbalanceOf<Self>>;
+
/// Maximum length (bytes) that the mined solution should consume.
///
/// The miner will ensure that the total length of the unsigned solution will not exceed
@@ -599,6 +641,7 @@ pub mod pallet {
+ Eq
+ Clone
+ sp_std::fmt::Debug
+ + Ord
+ CompactSolution;
/// Accuracy used for fallback on-chain election.
@@ -656,11 +699,20 @@ pub mod pallet {
Phase::Signed | Phase::Off
if remaining <= unsigned_deadline && remaining > Zero::zero() =>
{
- // Determine if followed by signed or not.
+ // our needs vary according to whether or not the unsigned phase follows a signed phase
let (need_snapshot, enabled, signed_weight) = if current_phase == Phase::Signed {
- // Followed by a signed phase: close the signed phase, no need for snapshot.
- // TODO: proper weight https://github.com/paritytech/substrate/pull/7910.
- (false, true, Weight::zero())
+ // there was previously a signed phase: close the signed phase, no need for snapshot.
+ //
+ // Notes:
+ //
+ // - `Self::finalize_signed_phase()` also appears in `fn do_elect`. This is
+ // a guard against the case that `elect` is called prematurely. This adds
+ // a small amount of overhead, but that is unfortunately unavoidable.
+ let (_success, weight) = Self::finalize_signed_phase();
+ // In the future we can consider disabling the unsigned phase if the signed
+ // phase completes successfully, but for now we're enabling it unconditionally
+ // as a defensive measure.
+ (false, true, weight)
} else {
// No signed phase: create a new snapshot, definitely `enable` the unsigned
// phase.
@@ -807,8 +859,12 @@ pub mod pallet {
// Store the newly received solution.
log!(info, "queued unsigned solution with score {:?}", ready.score);
+ let ejected_a_solution = <QueuedSolution<T>>::exists();
<QueuedSolution<T>>::put(ready);
- Self::deposit_event(Event::SolutionStored(ElectionCompute::Unsigned));
+ Self::deposit_event(Event::SolutionStored(
+ ElectionCompute::Unsigned,
+ ejected_a_solution,
+ ));
Ok(None.into())
}
@@ -828,6 +884,79 @@ pub mod pallet {
Ok(())
}
+ /// Submit a solution for the signed phase.
+ ///
+ /// The dispatch origin fo this call must be __signed__.
+ ///
+ /// The solution is potentially queued, based on the claimed score and processed at the end
+ /// of the signed phase.
+ ///
+ /// A deposit is reserved and recorded for the solution. Based on the outcome, the solution
+ /// might be rewarded, slashed, or get all or a part of the deposit back.
+ ///
+ /// # <weight>
+ /// Queue size must be provided as witness data.
+ /// # </weight>
+ #[pallet::weight(T::WeightInfo::submit(*num_signed_submissions))]
+ pub fn submit(
+ origin: OriginFor<T>,
+ solution: RawSolution<CompactOf<T>>,
+ num_signed_submissions: u32,
+ ) -> DispatchResult {
+ let who = ensure_signed(origin)?;
+
+ // ensure witness data is correct.
+ ensure!(
+ num_signed_submissions >= <SignedSubmissions<T>>::decode_len().unwrap_or_default() as u32,
+ Error::<T>::SignedInvalidWitness,
+ );
+
+ // ensure solution is timely.
+ ensure!(Self::current_phase().is_signed(), Error::<T>::PreDispatchEarlySubmission);
+
+ // NOTE: this is the only case where having separate snapshot would have been better
+ // because could do just decode_len. But we can create abstractions to do this.
+
+ // build size. Note: this is not needed for weight calc, thus not input.
+ // unlikely to ever return an error: if phase is signed, snapshot will exist.
+ let size = Self::snapshot_metadata().ok_or(Error::<T>::MissingSnapshotMetadata)?;
+
+ ensure!(
+ Self::feasibility_weight_of(&solution, size) < T::SignedMaxWeight::get(),
+ Error::<T>::SignedTooMuchWeight,
+ );
+
+ // create the submission
+ let deposit = Self::deposit_for(&solution, size);
+ let submission = SignedSubmission { who: who.clone(), deposit, solution };
+
+ // insert the submission if the queue has space or it's better than the weakest
+ // eject the weakest if the queue was full
+ let mut signed_submissions = Self::signed_submissions();
+ let maybe_removed = match signed_submissions.insert(submission) {
+ // it's an error if we failed to insert a submission: this indicates the queue was
+ // full but our solution had insufficient score to eject any solution
+ signed::InsertResult::NotInserted => return Err(Error::<T>::SignedQueueFull.into()),
+ signed::InsertResult::Inserted => None,
+ signed::InsertResult::InsertedEjecting(weakest) => Some(weakest),
+ };
+
+ // collect deposit. Thereafter, the function cannot fail.
+ T::Currency::reserve(&who, deposit)
+ .map_err(|_| Error::<T>::SignedCannotPayDeposit)?;
+
+ let ejected_a_solution = maybe_removed.is_some();
+ // if we had to remove the weakest solution, unreserve its deposit
+ if let Some(removed) = maybe_removed {
+ let _remainder = T::Currency::unreserve(&removed.who, removed.deposit);
+ debug_assert!(_remainder.is_zero());
+ }
+
+ signed_submissions.put();
+ Self::deposit_event(Event::SolutionStored(ElectionCompute::Signed, ejected_a_solution));
+ Ok(())
+ }
+
/// Set a solution in the queue, to be handed out to the client of this pallet in the next
/// call to `ElectionProvider::elect`.
///
@@ -860,7 +989,9 @@ pub mod pallet {
///
/// If the solution is signed, this means that it hasn't yet been processed. If the
/// solution is unsigned, this means that it has also been processed.
- SolutionStored(ElectionCompute),
+ ///
+ /// The `bool` is `true` when a previous solution was ejected to make room for this one.
+ SolutionStored(ElectionCompute, bool),
/// The election has been finalized, with `Some` of the given computation, or else if the
/// election failed, `None`.
ElectionFinalized(Option<ElectionCompute>),
@@ -883,8 +1014,20 @@ pub mod pallet {
PreDispatchWrongWinnerCount,
/// Submission was too weak, score-wise.
PreDispatchWeakSubmission,
+ /// The queue was full, and the solution was not better than any of the existing ones.
+ SignedQueueFull,
+ /// The origin failed to pay the deposit.
+ SignedCannotPayDeposit,
+ /// Witness data to dispatchable is invalid.
+ SignedInvalidWitness,
+ /// The signed submission consumes too much weight
+ SignedTooMuchWeight,
/// OCW submitted solution for wrong round
OcwCallWrongEra,
+ /// Snapshot metadata should exist but didn't.
+ MissingSnapshotMetadata,
+ /// `Self::insert_submission` returned an invalid index.
+ InvalidSubmissionIndex,
/// The call is not allowed at this point.
CallNotAllowed,
}
@@ -988,6 +1131,45 @@ pub mod pallet {
#[pallet::getter(fn snapshot_metadata)]
pub type SnapshotMetadata<T: Config> = StorageValue<_, SolutionOrSnapshotSize>;
+ // The following storage items collectively comprise `SignedSubmissions<T>`, and should never be
+ // accessed independently. Instead, get `Self::signed_submissions()`, modify it as desired, and
+ // then do `signed_submissions.put()` when you're done with it.
+
+ /// The next index to be assigned to an incoming signed submission.
+ ///
+ /// Every accepted submission is assigned a unique index; that index is bound to that particular
+ /// submission for the duration of the election. On election finalization, the next index is
+ /// reset to 0.
+ ///
+ /// We can't just use `SignedSubmissionIndices.len()`, because that's a bounded set; past its
+ /// capacity, it will simply saturate. We can't just iterate over `SignedSubmissionsMap`,
+ /// because iteration is slow. Instead, we store the value here.
+ #[pallet::storage]
+ pub(crate) type SignedSubmissionNextIndex<T: Config> = StorageValue<_, u32, ValueQuery>;
+
+ /// A sorted, bounded set of `(score, index)`, where each `index` points to a value in
+ /// `SignedSubmissions`.
+ ///
+ /// We never need to process more than a single signed submission at a time. Signed submissions
+ /// can be quite large, so we're willing to pay the cost of multiple database accesses to access
+ /// them one at a time instead of reading and decoding all of them at once.
+ #[pallet::storage]
+ pub(crate) type SignedSubmissionIndices<T: Config> =
+ StorageValue<_, SubmissionIndicesOf<T>, ValueQuery>;
+
+ /// Unchecked, signed solutions.
+ ///
+ /// Together with `SubmissionIndices`, this stores a bounded set of `SignedSubmissions` while
+ /// allowing us to keep only a single one in memory at a time.
+ ///
+ /// 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]
+ pub(crate) type SignedSubmissionsMap<T: Config> =
+ StorageMap<_, Twox64Concat, u32, SignedSubmissionOf<T>, ValueQuery>;
+
+ // `SignedSubmissions` items end here.
+
/// The minimum score that each 'untrusted' solution must attain in order to be considered
/// feasible.
///
@@ -1223,7 +1405,7 @@ impl<T: Config> Pallet<T> {
/// 3. Clear all snapshot data.
fn rotate_round() {
// Inc round.
- <Round<T>>::mutate(|r| *r = *r + 1);
+ <Round<T>>::mutate(|r| *r += 1);
// Phase is off now.
<CurrentPhase<T>>::put(Phase::Off);
@@ -1242,6 +1424,13 @@ impl<T: Config> Pallet<T> {
}
fn do_elect() -> Result<(Supports<T::AccountId>, Weight), ElectionError> {
+ // We have to unconditionally try finalizing the signed phase here. There are only two
+ // possibilities:
+ //
+ // - signed phase was open, in which case this is essential for correct functioning of the system
+ // - signed phase was complete or not started, in which case finalization is idempotent and
+ // inexpensive (1 read of an empty vector).
+ let (_, signed_finalize_weight) = Self::finalize_signed_phase();
<QueuedSolution<T>>::take()
.map_or_else(
|| match T::Fallback::get() {
@@ -1261,7 +1450,7 @@ impl<T: Config> Pallet<T> {
if Self::round() != 1 {
log!(info, "Finalized election round with compute {:?}.", compute);
}
- (supports, weight)
+ (supports, weight.saturating_add(signed_finalize_weight))
})
.map_err(|err| {
Self::deposit_event(Event::ElectionFinalized(None));
@@ -1309,7 +1498,14 @@ mod feasibility_check {
//! more. The best way to audit and review these tests is to try and come up with a solution
//! that is invalid, but gets through the system as valid.
- use super::{mock::*, *};
+ use super::*;
+ use crate::{
+ mock::{
+ MultiPhase, Runtime, roll_to, TargetIndex, raw_solution, EpochLength, UnsignedPhase,
+ SignedPhase, VoterIndex, ExtBuilder,
+ },
+ };
+ use frame_support::assert_noop;
const COMPUTE: ElectionCompute = ElectionCompute::OnChain;
@@ -1476,16 +1672,24 @@ mod feasibility_check {
#[cfg(test)]
mod tests {
- use super::{mock::*, Event, *};
+ use super::*;
+ use crate::{
+ Phase,
+ mock::{
+ ExtBuilder, MultiPhase, Runtime, roll_to, MockWeightInfo, AccountId, TargetIndex,
+ Targets, multi_phase_events, System, SignedMaxSubmissions,
+ },
+ };
use frame_election_provider_support::ElectionProvider;
+ use frame_support::{assert_noop, assert_ok};
use sp_npos_elections::Support;
#[test]
fn phase_rotation_works() {
ExtBuilder::default().build_and_execute(|| {
// 0 ------- 15 ------- 25 ------- 30 ------- ------- 45 ------- 55 ------- 60
- // | | | |
- // Signed Unsigned Signed Unsigned
+ // | | | | | |
+ // Signed Unsigned Elect Signed Unsigned Elect
assert_eq!(System::block_number(), 0);
assert_eq!(MultiPhase::current_phase(), Phase::Off);
@@ -1644,6 +1848,44 @@ mod tests {
assert!(MultiPhase::snapshot_metadata().is_none());
assert!(MultiPhase::desired_targets().is_none());
assert!(MultiPhase::queued_solution().is_none());
+ assert!(MultiPhase::signed_submissions().is_empty());
+ })
+ }
+
+ #[test]
+ fn early_termination_with_submissions() {
+ // an early termination in the signed phase, with no queued solution.
+ ExtBuilder::default().build_and_execute(|| {
+ // signed phase started at block 15 and will end at 25.
+ roll_to(14);
+ assert_eq!(MultiPhase::current_phase(), Phase::Off);
+
+ roll_to(15);
+ assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted(1)]);
+ assert_eq!(MultiPhase::current_phase(), Phase::Signed);
+ assert_eq!(MultiPhase::round(), 1);
+
+ // fill the queue with signed submissions
+ for s in 0..SignedMaxSubmissions::get() {
+ let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() };
+ assert_ok!(MultiPhase::submit(
+ crate::mock::Origin::signed(99),
+ solution,
+ MultiPhase::signed_submissions().len() as u32
+ ));
+ }
+
+ // an unexpected call to elect.
+ roll_to(20);
+ assert!(MultiPhase::elect().is_ok());
+
+ // all storage items must be cleared.
+ assert_eq!(MultiPhase::round(), 2);
+ assert!(MultiPhase::snapshot().is_none());
+ assert!(MultiPhase::snapshot_metadata().is_none());
+ assert!(MultiPhase::desired_targets().is_none());
+ assert!(MultiPhase::queued_solution().is_none());
+ assert!(MultiPhase::signed_submissions().is_empty());
})
}
diff --git a/substrate/frame/election-provider-multi-phase/src/mock.rs b/substrate/frame/election-provider-multi-phase/src/mock.rs
index bd035aaf829698a6680748d3884ef433938e9344..8840e2b935d35f89b6dd5652c046969b0337e090 100644
--- a/substrate/frame/election-provider-multi-phase/src/mock.rs
+++ b/substrate/frame/election-provider-multi-phase/src/mock.rs
@@ -260,8 +260,13 @@ parameter_types! {
pub static DesiredTargets: u32 = 2;
pub static SignedPhase: u64 = 10;
pub static UnsignedPhase: u64 = 5;
- pub static MaxSignedSubmissions: u32 = 5;
-
+ pub static SignedMaxSubmissions: u32 = 5;
+ pub static SignedDepositBase: Balance = 5;
+ pub static SignedDepositByte: Balance = 0;
+ pub static SignedDepositWeight: Balance = 0;
+ pub static SignedRewardBase: Balance = 7;
+ pub static SignedRewardMax: Balance = 10;
+ pub static SignedMaxWeight: Weight = BlockWeights::get().max_block;
pub static MinerMaxIterations: u32 = 5;
pub static MinerTxPriority: u64 = 100;
pub static SolutionImprovementThreshold: Perbill = Perbill::zero();
@@ -304,6 +309,27 @@ impl multi_phase::weights::WeightInfo for DualMockWeightInfo {
<() as multi_phase::weights::WeightInfo>::on_initialize_open_unsigned_without_snapshot()
}
}
+ fn finalize_signed_phase_accept_solution() -> Weight {
+ if MockWeightInfo::get() {
+ Zero::zero()
+ } else {
+ <() as multi_phase::weights::WeightInfo>::finalize_signed_phase_accept_solution()
+ }
+ }
+ fn finalize_signed_phase_reject_solution() -> Weight {
+ if MockWeightInfo::get() {
+ Zero::zero()
+ } else {
+ <() as multi_phase::weights::WeightInfo>::finalize_signed_phase_reject_solution()
+ }
+ }
+ fn submit(c: u32) -> Weight {
+ if MockWeightInfo::get() {
+ Zero::zero()
+ } else {
+ <() as multi_phase::weights::WeightInfo>::submit(c)
+ }
+ }
fn elect_queued() -> Weight {
if MockWeightInfo::get() {
Zero::zero()
@@ -342,6 +368,14 @@ impl crate::Config for Runtime {
type MinerMaxWeight = MinerMaxWeight;
type MinerMaxLength = MinerMaxLength;
type MinerTxPriority = MinerTxPriority;
+ type SignedRewardBase = SignedRewardBase;
+ type SignedDepositBase = SignedDepositBase;
+ type SignedDepositByte = ();
+ type SignedDepositWeight = ();
+ type SignedMaxWeight = SignedMaxWeight;
+ type SignedMaxSubmissions = SignedMaxSubmissions;
+ type SlashHandler = ();
+ type RewardHandler = ();
type DataProvider = StakingMock;
type WeightInfo = DualMockWeightInfo;
type BenchmarkingConfig = ();
@@ -440,6 +474,20 @@ impl ExtBuilder {
VOTERS.with(|v| v.borrow_mut().push((who, stake, targets)));
self
}
+ pub fn signed_max_submission(self, count: u32) -> Self {
+ <SignedMaxSubmissions>::set(count);
+ self
+ }
+ pub fn signed_deposit(self, base: u64, byte: u64, weight: u64) -> Self {
+ <SignedDepositBase>::set(base);
+ <SignedDepositByte>::set(byte);
+ <SignedDepositWeight>::set(weight);
+ self
+ }
+ pub fn signed_weight(self, weight: Weight) -> Self {
+ <SignedMaxWeight>::set(weight);
+ self
+ }
pub fn build(self) -> sp_io::TestExternalities {
sp_tracing::try_init_simple();
let mut storage =
@@ -481,3 +529,7 @@ impl ExtBuilder {
self.build().execute_with(test)
}
}
+
+pub(crate) fn balances(who: &u64) -> (u64, u64) {
+ (Balances::free_balance(who), Balances::reserved_balance(who))
+}
diff --git a/substrate/frame/election-provider-multi-phase/src/signed.rs b/substrate/frame/election-provider-multi-phase/src/signed.rs
new file mode 100644
index 0000000000000000000000000000000000000000..ba1123c1331ad586b663801c0f87e8f5674ca4db
--- /dev/null
+++ b/substrate/frame/election-provider-multi-phase/src/signed.rs
@@ -0,0 +1,920 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! The signed phase implementation.
+
+use crate::{
+ CompactOf, Config, ElectionCompute, Pallet, RawSolution, ReadySolution, SolutionOrSnapshotSize,
+ Weight, WeightInfo, QueuedSolution, SignedSubmissionsMap, SignedSubmissionIndices,
+ SignedSubmissionNextIndex,
+};
+use codec::{Encode, Decode, 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, CompactSolution, ElectionScore};
+use sp_runtime::{
+ RuntimeDebug,
+ traits::{Saturating, Zero},
+};
+use sp_std::{
+ cmp::Ordering,
+ collections::{btree_map::BTreeMap, btree_set::BTreeSet},
+ ops::Deref,
+};
+
+/// A raw, unchecked signed submission.
+///
+/// This is just a wrapper around [`RawSolution`] and some additional info.
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default)]
+pub struct SignedSubmission<AccountId, Balance: HasCompact, CompactSolution> {
+ /// Who submitted this solution.
+ pub who: AccountId,
+ /// The deposit reserved for storing this solution.
+ pub deposit: Balance,
+ /// The raw solution itself.
+ pub solution: RawSolution<CompactSolution>,
+}
+
+impl<AccountId, Balance, CompactSolution> Ord
+ for SignedSubmission<AccountId, Balance, CompactSolution>
+where
+ AccountId: Ord,
+ Balance: Ord + HasCompact,
+ CompactSolution: Ord,
+ RawSolution<CompactSolution>: Ord,
+{
+ fn cmp(&self, other: &Self) -> Ordering {
+ self.solution
+ .score
+ .cmp(&other.solution.score)
+ .then_with(|| self.solution.cmp(&other.solution))
+ .then_with(|| self.deposit.cmp(&other.deposit))
+ .then_with(|| self.who.cmp(&other.who))
+ }
+}
+
+impl<AccountId, Balance, CompactSolution> PartialOrd
+ for SignedSubmission<AccountId, Balance, CompactSolution>
+where
+ AccountId: Ord,
+ Balance: Ord + HasCompact,
+ CompactSolution: Ord,
+ RawSolution<CompactSolution>: Ord,
+{
+ fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+ Some(self.cmp(other))
+ }
+}
+
+pub type BalanceOf<T> =
+ <<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
+pub type PositiveImbalanceOf<T> = <<T as Config>::Currency as Currency<
+ <T as frame_system::Config>::AccountId,
+>>::PositiveImbalance;
+pub type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<
+ <T as frame_system::Config>::AccountId,
+>>::NegativeImbalance;
+pub type SignedSubmissionOf<T> =
+ SignedSubmission<<T as frame_system::Config>::AccountId, BalanceOf<T>, CompactOf<T>>;
+
+pub type SubmissionIndicesOf<T> =
+ BoundedBTreeMap<ElectionScore, u32, <T as Config>::SignedMaxSubmissions>;
+
+/// Outcome of [`SignedSubmissions::insert`].
+pub enum InsertResult<T: Config> {
+ /// The submission was not inserted because the queue was full and the submission had
+ /// insufficient score to eject a prior solution from the queue.
+ NotInserted,
+ /// The submission was inserted successfully without ejecting a solution.
+ Inserted,
+ /// The submission was inserted successfully. As the queue was full, this operation ejected a
+ /// prior solution, contained in this variant.
+ InsertedEjecting(SignedSubmissionOf<T>),
+}
+
+/// 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))]
+pub struct SignedSubmissions<T: Config> {
+ indices: SubmissionIndicesOf<T>,
+ next_idx: u32,
+ insertion_overlay: BTreeMap<u32, SignedSubmissionOf<T>>,
+ deletion_overlay: BTreeSet<u32>,
+}
+
+impl<T: Config> SignedSubmissions<T> {
+ /// Get the signed submissions from storage.
+ pub fn get() -> Self {
+ let submissions = SignedSubmissions {
+ indices: SignedSubmissionIndices::<T>::get(),
+ next_idx: SignedSubmissionNextIndex::<T>::get(),
+ insertion_overlay: BTreeMap::new(),
+ deletion_overlay: BTreeSet::new(),
+ };
+ // validate that the stored state is sane
+ debug_assert!(submissions.indices.values().copied().max().map_or(
+ true,
+ |max_idx| submissions.next_idx > max_idx,
+ ));
+ submissions
+ }
+
+ /// Put the signed submissions back into storage.
+ pub fn put(mut self) {
+ // validate that we're going to write only sane things to storage
+ debug_assert!(self.insertion_overlay.keys().copied().max().map_or(
+ true,
+ |max_idx| self.next_idx > max_idx,
+ ));
+ debug_assert!(self.indices.values().copied().max().map_or(
+ true,
+ |max_idx| self.next_idx > max_idx,
+ ));
+
+ SignedSubmissionIndices::<T>::put(self.indices);
+ SignedSubmissionNextIndex::<T>::put(self.next_idx);
+ for key in self.deletion_overlay {
+ self.insertion_overlay.remove(&key);
+ SignedSubmissionsMap::<T>::remove(key);
+ }
+ for (key, value) in self.insertion_overlay {
+ SignedSubmissionsMap::<T>::insert(key, value);
+ }
+ }
+
+ /// Get the submission at a particular index.
+ fn get_submission(&self, idx: u32) -> Option<SignedSubmissionOf<T>> {
+ if self.deletion_overlay.contains(&idx) {
+ // Note: can't actually remove the item from the insertion overlay (if present)
+ // because we don't want to use `&mut self` here. There may be some kind of
+ // `RefCell` optimization possible here in the future.
+ None
+ } else {
+ self.insertion_overlay
+ .get(&idx)
+ .cloned()
+ .or_else(|| SignedSubmissionsMap::<T>::try_get(idx).ok())
+ }
+ }
+
+ /// Perform three operations:
+ ///
+ /// - Remove a submission (identified by score)
+ /// - Insert a new submission (identified by score and insertion index)
+ /// - Return the submission which was removed.
+ ///
+ /// Note: in the case that `weakest_score` is not present in `self.indices`, this will return
+ /// `None` without inserting the new submission and without further notice.
+ ///
+ /// Note: this does not enforce any ordering relation between the submission removed and that
+ /// inserted.
+ ///
+ /// Note: this doesn't insert into `insertion_overlay`, the optional new insertion must be
+ /// inserted into `insertion_overlay` to keep the variable `self` in a valid state.
+ fn swap_out_submission(
+ &mut self,
+ remove_score: ElectionScore,
+ insert: Option<(ElectionScore, u32)>,
+ ) -> Option<SignedSubmissionOf<T>> {
+ let remove_idx = self.indices.remove(&remove_score)?;
+ if let Some((insert_score, insert_idx)) = insert {
+ self.indices
+ .try_insert(insert_score, insert_idx)
+ .expect("just removed an item, we must be under capacity; qed");
+ }
+
+ self.insertion_overlay.remove(&remove_idx).or_else(|| {
+ (!self.deletion_overlay.contains(&remove_idx)).then(|| {
+ self.deletion_overlay.insert(remove_idx);
+ SignedSubmissionsMap::<T>::try_get(remove_idx).ok()
+ }).flatten()
+ })
+ }
+
+ /// Iterate through the set of signed submissions in order of increasing score.
+ pub fn iter(&self) -> impl '_ + Iterator<Item = SignedSubmissionOf<T>> {
+ self.indices.iter().filter_map(move |(_score, &idx)| {
+ let maybe_submission = self.get_submission(idx);
+ if maybe_submission.is_none() {
+ log!(
+ error,
+ "SignedSubmissions internal state is invalid (idx {}); \
+ there is a logic error in code handling signed solution submissions",
+ idx,
+ )
+ }
+ maybe_submission
+ })
+ }
+
+ /// Empty the set of signed submissions, returning an iterator of signed submissions in
+ /// arbitrary order.
+ ///
+ /// Note that if the iterator is dropped without consuming all elements, not all may be removed
+ /// from the underlying `SignedSubmissionsMap`, putting the storages into an invalid state.
+ ///
+ /// Note that, like `put`, this function consumes `Self` and modifies storage.
+ fn drain(mut self) -> impl Iterator<Item = SignedSubmissionOf<T>> {
+ SignedSubmissionIndices::<T>::kill();
+ SignedSubmissionNextIndex::<T>::kill();
+ let insertion_overlay = sp_std::mem::take(&mut self.insertion_overlay);
+ SignedSubmissionsMap::<T>::drain()
+ .filter(move |(k, _v)| !self.deletion_overlay.contains(k))
+ .map(|(_k, v)| v)
+ .chain(insertion_overlay.into_iter().map(|(_k, v)| v))
+ }
+
+ /// Decode the length of the signed submissions without actually reading the entire struct into
+ /// memory.
+ ///
+ /// Note that if you hold an instance of `SignedSubmissions`, this function does _not_
+ /// track its current length. This only decodes what is currently stored in memory.
+ pub fn decode_len() -> Option<usize> {
+ SignedSubmissionIndices::<T>::decode_len()
+ }
+
+ /// Insert a new signed submission into the set.
+ ///
+ /// In the event that the new submission is not better than the current weakest according
+ /// to `is_score_better`, we do not change anything.
+ pub fn insert(
+ &mut self,
+ submission: SignedSubmissionOf<T>,
+ ) -> InsertResult<T> {
+ // verify the expectation that we never reuse an index
+ debug_assert!(!self.indices.values().any(|&idx| idx == self.next_idx));
+
+ let weakest = match self.indices.try_insert(submission.solution.score, self.next_idx) {
+ Ok(Some(prev_idx)) => {
+ // a submission of equal score was already present in the set;
+ // no point editing the actual backing map as we know that the newer solution can't
+ // be better than the old. However, we do need to put the old value back.
+ self.indices
+ .try_insert(submission.solution.score, prev_idx)
+ .expect("didn't change the map size; qed");
+ return InsertResult::NotInserted;
+ }
+ Ok(None) => {
+ // successfully inserted into the set; no need to take out weakest member
+ None
+ }
+ Err((insert_score, insert_idx)) => {
+ // could not insert into the set because it is full.
+ // note that we short-circuit return here in case the iteration produces `None`.
+ // If there wasn't a weakest entry to remove, then there must be a capacity of 0,
+ // which means that we can't meaningfully proceed.
+ let weakest_score = match self.indices.iter().next() {
+ None => return InsertResult::NotInserted,
+ Some((score, _)) => *score,
+ };
+ let threshold = T::SolutionImprovementThreshold::get();
+
+ // if we haven't improved on the weakest score, don't change anything.
+ if !is_score_better(insert_score, weakest_score, threshold) {
+ return InsertResult::NotInserted;
+ }
+
+ self.swap_out_submission(weakest_score, Some((insert_score, insert_idx)))
+ }
+ };
+
+ // we've taken out the weakest, so update the storage map and the next index
+ debug_assert!(!self.insertion_overlay.contains_key(&self.next_idx));
+ self.insertion_overlay.insert(self.next_idx, submission);
+ debug_assert!(!self.deletion_overlay.contains(&self.next_idx));
+ self.next_idx += 1;
+ match weakest {
+ Some(weakest) => InsertResult::InsertedEjecting(weakest),
+ None => InsertResult::Inserted,
+ }
+ }
+
+ /// Remove the signed submission with the highest score from the set.
+ pub fn pop_last(&mut self) -> Option<SignedSubmissionOf<T>> {
+ let (score, _) = self.indices.iter().rev().next()?;
+ // deref in advance to prevent mutable-immutable borrow conflict
+ let score = *score;
+ self.swap_out_submission(score, None)
+ }
+}
+
+impl<T: Config> Deref for SignedSubmissions<T> {
+ type Target = SubmissionIndicesOf<T>;
+
+ fn deref(&self) -> &Self::Target {
+ &self.indices
+ }
+}
+
+impl<T: Config> Pallet<T> {
+ /// `Self` accessor for `SignedSubmission<T>`.
+ pub fn signed_submissions() -> SignedSubmissions<T> {
+ SignedSubmissions::<T>::get()
+ }
+
+ /// Finish the signed phase. Process the signed submissions from best to worse until a valid one
+ /// is found, rewarding the best one and slashing the invalid ones along the way.
+ ///
+ /// Returns true if we have a good solution in the signed phase.
+ ///
+ /// This drains the [`SignedSubmissions`], potentially storing the best valid one in
+ /// [`QueuedSolution`].
+ pub fn finalize_signed_phase() -> (bool, Weight) {
+ let mut all_submissions = Self::signed_submissions();
+ let mut found_solution = false;
+ let mut weight = T::DbWeight::get().reads(1);
+
+ let SolutionOrSnapshotSize { voters, targets } =
+ Self::snapshot_metadata().unwrap_or_default();
+
+ let reward = T::SignedRewardBase::get();
+
+ while let Some(best) = all_submissions.pop_last() {
+ let SignedSubmission { solution, who, deposit} = best;
+ let active_voters = solution.compact.voter_count() as u32;
+ let feasibility_weight = {
+ // defensive only: at the end of signed phase, snapshot will exits.
+ let desired_targets = Self::desired_targets().unwrap_or_default();
+ T::WeightInfo::feasibility_check(
+ voters,
+ targets,
+ active_voters,
+ desired_targets,
+ )
+ };
+ // the feasibility check itself has some weight
+ weight = weight.saturating_add(feasibility_weight);
+ match Self::feasibility_check(solution, ElectionCompute::Signed) {
+ Ok(ready_solution) => {
+ Self::finalize_signed_phase_accept_solution(
+ ready_solution,
+ &who,
+ deposit,
+ reward,
+ );
+ found_solution = true;
+
+ weight = weight
+ .saturating_add(T::WeightInfo::finalize_signed_phase_accept_solution());
+ break;
+ }
+ Err(_) => {
+ Self::finalize_signed_phase_reject_solution(&who, deposit);
+ weight = weight
+ .saturating_add(T::WeightInfo::finalize_signed_phase_reject_solution());
+ }
+ }
+ }
+
+ // Any unprocessed solution is pointless to even consider. Feasible or malicious,
+ // they didn't end up being used. Unreserve the bonds.
+ let discarded = all_submissions.len();
+ for SignedSubmission { who, deposit, .. } in all_submissions.drain() {
+ let _remaining = T::Currency::unreserve(&who, deposit);
+ weight = weight.saturating_add(T::DbWeight::get().writes(1));
+ debug_assert!(_remaining.is_zero());
+ }
+
+ debug_assert!(!SignedSubmissionIndices::<T>::exists());
+ debug_assert!(!SignedSubmissionNextIndex::<T>::exists());
+ debug_assert!(SignedSubmissionsMap::<T>::iter().next().is_none());
+
+ log!(debug, "closed signed phase, found solution? {}, discarded {}", found_solution, discarded);
+ (found_solution, weight)
+ }
+
+ /// Helper function for the case where a solution is accepted in the signed phase.
+ ///
+ /// Extracted to facilitate with weight calculation.
+ ///
+ /// Infallible
+ pub fn finalize_signed_phase_accept_solution(
+ ready_solution: ReadySolution<T::AccountId>,
+ who: &T::AccountId,
+ deposit: BalanceOf<T>,
+ reward: BalanceOf<T>,
+ ) {
+ // write this ready solution.
+ <QueuedSolution<T>>::put(ready_solution);
+
+ // emit reward event
+ Self::deposit_event(crate::Event::Rewarded(who.clone()));
+
+ // unreserve deposit.
+ let _remaining = T::Currency::unreserve(who, deposit);
+ debug_assert!(_remaining.is_zero());
+
+ // Reward.
+ let positive_imbalance = T::Currency::deposit_creating(who, reward);
+ T::RewardHandler::on_unbalanced(positive_imbalance);
+ }
+
+ /// Helper function for the case where a solution is accepted in the rejected phase.
+ ///
+ /// Extracted to facilitate with weight calculation.
+ ///
+ /// Infallible
+ pub fn finalize_signed_phase_reject_solution(who: &T::AccountId, deposit: BalanceOf<T>) {
+ Self::deposit_event(crate::Event::Slashed(who.clone()));
+ let (negative_imbalance, _remaining) = T::Currency::slash_reserved(who, deposit);
+ debug_assert!(_remaining.is_zero());
+ T::SlashHandler::on_unbalanced(negative_imbalance);
+ }
+
+ /// The feasibility weight of the given raw solution.
+ pub fn feasibility_weight_of(
+ solution: &RawSolution<CompactOf<T>>,
+ size: SolutionOrSnapshotSize,
+ ) -> Weight {
+ T::WeightInfo::feasibility_check(
+ size.voters,
+ size.targets,
+ solution.compact.voter_count() as u32,
+ solution.compact.unique_targets().len() as u32,
+ )
+ }
+
+ /// Collect a sufficient deposit to store this solution.
+ ///
+ /// The deposit is composed of 3 main elements:
+ ///
+ /// 1. base deposit, fixed for all submissions.
+ /// 2. a per-byte deposit, for renting the state usage.
+ /// 3. a per-weight deposit, for the potential weight usage in an upcoming on_initialize
+ pub fn deposit_for(
+ solution: &RawSolution<CompactOf<T>>,
+ size: SolutionOrSnapshotSize,
+ ) -> BalanceOf<T> {
+ let encoded_len: u32 = solution.encoded_size().saturated_into();
+ let encoded_len: BalanceOf<T> = encoded_len.into();
+ let feasibility_weight = Self::feasibility_weight_of(solution, size);
+
+ let len_deposit = T::SignedDepositByte::get().saturating_mul(encoded_len);
+ let weight_deposit = T::SignedDepositWeight::get().saturating_mul(feasibility_weight.saturated_into());
+
+ T::SignedDepositBase::get().saturating_add(len_deposit).saturating_add(weight_deposit)
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::{
+ Phase, Error,
+ mock::{
+ balances, ExtBuilder, MultiPhase, Origin, raw_solution, roll_to, Runtime,
+ SignedMaxSubmissions, SignedMaxWeight,
+ },
+ };
+ use frame_support::{dispatch::DispatchResult, assert_noop, assert_storage_noop, assert_ok};
+
+ fn submit_with_witness(
+ origin: Origin,
+ solution: RawSolution<CompactOf<Runtime>>,
+ ) -> DispatchResult {
+ MultiPhase::submit(origin, solution, MultiPhase::signed_submissions().len() as u32)
+ }
+
+ #[test]
+ fn cannot_submit_too_early() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(2);
+ assert_eq!(MultiPhase::current_phase(), Phase::Off);
+
+ // create a temp snapshot only for this test.
+ MultiPhase::create_snapshot().unwrap();
+ let solution = raw_solution();
+
+ assert_noop!(
+ submit_with_witness(Origin::signed(10), solution),
+ Error::<Runtime>::PreDispatchEarlySubmission,
+ );
+ })
+ }
+
+ #[test]
+ fn wrong_witness_fails() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let solution = raw_solution();
+ // submit this once correctly
+ assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
+ assert_eq!(MultiPhase::signed_submissions().len(), 1);
+
+ // now try and cheat by passing a lower queue length
+ assert_noop!(
+ MultiPhase::submit(Origin::signed(99), solution, 0),
+ Error::<Runtime>::SignedInvalidWitness,
+ );
+ })
+ }
+
+ #[test]
+ fn should_pay_deposit() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let solution = raw_solution();
+ assert_eq!(balances(&99), (100, 0));
+
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+
+ assert_eq!(balances(&99), (95, 5));
+ assert_eq!(MultiPhase::signed_submissions().iter().next().unwrap().deposit, 5);
+ })
+ }
+
+ #[test]
+ fn good_solution_is_rewarded() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let solution = raw_solution();
+ assert_eq!(balances(&99), (100, 0));
+
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ assert_eq!(balances(&99), (95, 5));
+
+ assert!(MultiPhase::finalize_signed_phase().0);
+ assert_eq!(balances(&99), (100 + 7, 0));
+ })
+ }
+
+ #[test]
+ fn bad_solution_is_slashed() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let mut solution = raw_solution();
+ assert_eq!(balances(&99), (100, 0));
+
+ // make the solution invalid.
+ solution.score[0] += 1;
+
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ assert_eq!(balances(&99), (95, 5));
+
+ // no good solution was stored.
+ assert!(!MultiPhase::finalize_signed_phase().0);
+ // and the bond is gone.
+ assert_eq!(balances(&99), (95, 0));
+ })
+ }
+
+ #[test]
+ fn suppressed_solution_gets_bond_back() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let mut solution = raw_solution();
+ assert_eq!(balances(&99), (100, 0));
+ assert_eq!(balances(&999), (100, 0));
+
+ // submit as correct.
+ assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
+
+ // make the solution invalid and weaker.
+ solution.score[0] -= 1;
+ assert_ok!(submit_with_witness(Origin::signed(999), solution));
+ assert_eq!(balances(&99), (95, 5));
+ assert_eq!(balances(&999), (95, 5));
+
+ // _some_ good solution was stored.
+ assert!(MultiPhase::finalize_signed_phase().0);
+
+ // 99 is rewarded.
+ assert_eq!(balances(&99), (100 + 7, 0));
+ // 999 gets everything back.
+ assert_eq!(balances(&999), (100, 0));
+ })
+ }
+
+ #[test]
+ fn cannot_submit_worse_with_full_queue() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ for s in 0..SignedMaxSubmissions::get() {
+ // score is always getting better
+ let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ }
+
+
+ // weaker.
+ let solution = RawSolution { score: [4, 0, 0], ..Default::default() };
+
+ assert_noop!(
+ submit_with_witness(Origin::signed(99), solution),
+ Error::<Runtime>::SignedQueueFull,
+ );
+ })
+ }
+
+ #[test]
+ fn weakest_is_removed_if_better_provided() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ for s in 0..SignedMaxSubmissions::get() {
+ // score is always getting better
+ let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ }
+
+ assert_eq!(
+ MultiPhase::signed_submissions()
+ .iter()
+ .map(|s| s.solution.score[0])
+ .collect::<Vec<_>>(),
+ vec![5, 6, 7, 8, 9]
+ );
+
+ // better.
+ let solution = RawSolution { score: [20, 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+
+ // the one with score 5 was rejected, the new one inserted.
+ assert_eq!(
+ MultiPhase::signed_submissions()
+ .iter()
+ .map(|s| s.solution.score[0])
+ .collect::<Vec<_>>(),
+ vec![6, 7, 8, 9, 20]
+ );
+ })
+ }
+
+ #[test]
+ fn replace_weakest_works() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ for s in 1..SignedMaxSubmissions::get() {
+ // score is always getting better
+ let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ }
+
+ let solution = RawSolution { score: [4, 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+
+ assert_eq!(
+ MultiPhase::signed_submissions()
+ .iter()
+ .map(|s| s.solution.score[0])
+ .collect::<Vec<_>>(),
+ vec![4, 6, 7, 8, 9],
+ );
+
+ // better.
+ let solution = RawSolution { score: [5, 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+
+ // the one with score 5 was rejected, the new one inserted.
+ assert_eq!(
+ MultiPhase::signed_submissions()
+ .iter()
+ .map(|s| s.solution.score[0])
+ .collect::<Vec<_>>(),
+ vec![5, 6, 7, 8, 9],
+ );
+ })
+ }
+
+ #[test]
+ fn early_ejected_solution_gets_bond_back() {
+ ExtBuilder::default().signed_deposit(2, 0, 0).build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ for s in 0..SignedMaxSubmissions::get() {
+ // score is always getting better
+ let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ }
+
+ assert_eq!(balances(&99).1, 2 * 5);
+ assert_eq!(balances(&999).1, 0);
+
+ // better.
+ let solution = RawSolution { score: [20, 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(999), solution));
+
+ // got one bond back.
+ assert_eq!(balances(&99).1, 2 * 4);
+ assert_eq!(balances(&999).1, 2);
+ })
+ }
+
+ #[test]
+ fn equally_good_solution_is_not_accepted() {
+ ExtBuilder::default().signed_max_submission(3).build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ for i in 0..SignedMaxSubmissions::get() {
+ let solution = RawSolution { score: [(5 + i).into(), 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ }
+ assert_eq!(
+ MultiPhase::signed_submissions()
+ .iter()
+ .map(|s| s.solution.score[0])
+ .collect::<Vec<_>>(),
+ vec![5, 6, 7]
+ );
+
+ // 5 is not accepted. This will only cause processing with no benefit.
+ let solution = RawSolution { score: [5, 0, 0], ..Default::default() };
+ assert_noop!(
+ submit_with_witness(Origin::signed(99), solution),
+ Error::<Runtime>::SignedQueueFull,
+ );
+ })
+ }
+
+ #[test]
+ fn all_in_one_signed_submission_scenario() {
+ // a combination of:
+ // - good_solution_is_rewarded
+ // - bad_solution_is_slashed
+ // - suppressed_solution_gets_bond_back
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ assert_eq!(balances(&99), (100, 0));
+ assert_eq!(balances(&999), (100, 0));
+ assert_eq!(balances(&9999), (100, 0));
+ let solution = raw_solution();
+
+ // submit a correct one.
+ assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
+
+ // make the solution invalidly better and submit. This ought to be slashed.
+ let mut solution_999 = solution.clone();
+ solution_999.score[0] += 1;
+ assert_ok!(submit_with_witness(Origin::signed(999), solution_999));
+
+ // make the solution invalidly worse and submit. This ought to be suppressed and
+ // returned.
+ let mut solution_9999 = solution.clone();
+ solution_9999.score[0] -= 1;
+ assert_ok!(submit_with_witness(Origin::signed(9999), solution_9999));
+
+ assert_eq!(
+ MultiPhase::signed_submissions().iter().map(|x| x.who).collect::<Vec<_>>(),
+ vec![9999, 99, 999]
+ );
+
+ // _some_ good solution was stored.
+ assert!(MultiPhase::finalize_signed_phase().0);
+
+ // 99 is rewarded.
+ assert_eq!(balances(&99), (100 + 7, 0));
+ // 999 is slashed.
+ assert_eq!(balances(&999), (95, 0));
+ // 9999 gets everything back.
+ assert_eq!(balances(&9999), (100, 0));
+ })
+ }
+
+ #[test]
+ fn cannot_consume_too_much_future_weight() {
+ ExtBuilder::default().signed_weight(40).mock_weight_info(true).build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let (solution, witness) = MultiPhase::mine_solution(2).unwrap();
+ let solution_weight = <Runtime as Config>::WeightInfo::feasibility_check(
+ witness.voters,
+ witness.targets,
+ solution.compact.voter_count() as u32,
+ solution.compact.unique_targets().len() as u32,
+ );
+ // default solution will have 5 edges (5 * 5 + 10)
+ assert_eq!(solution_weight, 35);
+ assert_eq!(solution.compact.voter_count(), 5);
+ assert_eq!(<Runtime as Config>::SignedMaxWeight::get(), 40);
+
+ assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
+
+ <SignedMaxWeight>::set(30);
+
+ // note: resubmitting the same solution is technically okay as long as the queue has
+ // space.
+ assert_noop!(
+ submit_with_witness(Origin::signed(99), solution),
+ Error::<Runtime>::SignedTooMuchWeight,
+ );
+ })
+ }
+
+ #[test]
+ fn insufficient_deposit_doesnt_store_submission() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let solution = raw_solution();
+
+ assert_eq!(balances(&123), (0, 0));
+ assert_noop!(
+ submit_with_witness(Origin::signed(123), solution),
+ Error::<Runtime>::SignedCannotPayDeposit,
+ );
+
+ assert_eq!(balances(&123), (0, 0));
+ })
+ }
+
+ // given a full queue, and a solution which _should_ be allowed in, but the proposer of this
+ // new solution has insufficient deposit, we should not modify storage at all
+ #[test]
+ fn insufficient_deposit_with_full_queue_works_properly() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ for s in 0..SignedMaxSubmissions::get() {
+ // score is always getting better
+ let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() };
+ assert_ok!(submit_with_witness(Origin::signed(99), solution));
+ }
+
+ // this solution has a higher score than any in the queue
+ let solution = RawSolution {
+ score: [(5 + SignedMaxSubmissions::get()).into(), 0, 0],
+ ..Default::default()
+ };
+
+ assert_eq!(balances(&123), (0, 0));
+ assert_noop!(
+ submit_with_witness(Origin::signed(123), solution),
+ Error::<Runtime>::SignedCannotPayDeposit,
+ );
+
+ assert_eq!(balances(&123), (0, 0));
+ })
+ }
+
+ #[test]
+ fn finalize_signed_phase_is_idempotent_given_no_submissions() {
+ ExtBuilder::default().build_and_execute(|| {
+ for block_number in 0..25 {
+ roll_to(block_number);
+
+ assert_eq!(SignedSubmissions::<Runtime>::decode_len().unwrap_or_default(), 0);
+ assert_storage_noop!(MultiPhase::finalize_signed_phase());
+ }
+ })
+ }
+
+ #[test]
+ fn finalize_signed_phase_is_idempotent_given_submissions() {
+ ExtBuilder::default().build_and_execute(|| {
+ roll_to(15);
+ assert!(MultiPhase::current_phase().is_signed());
+
+ let solution = raw_solution();
+
+ // submit a correct one.
+ assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
+
+ // _some_ good solution was stored.
+ assert!(MultiPhase::finalize_signed_phase().0);
+
+ // calling it again doesn't change anything
+ assert_storage_noop!(MultiPhase::finalize_signed_phase());
+ })
+ }
+}
diff --git a/substrate/frame/election-provider-multi-phase/src/unsigned.rs b/substrate/frame/election-provider-multi-phase/src/unsigned.rs
index 543883fc035c500daedd7412a3924d7f3b970b1d..52ecae7afa5fc6431e5081fa64777c82f5675fea 100644
--- a/substrate/frame/election-provider-multi-phase/src/unsigned.rs
+++ b/substrate/frame/election-provider-multi-phase/src/unsigned.rs
@@ -30,8 +30,10 @@ use sp_npos_elections::{
assignment_staked_to_ratio_normalized, is_score_better, seq_phragmen,
};
use sp_runtime::{
+ DispatchError,
+ SaturatedConversion,
offchain::storage::{MutateStorageError, StorageValueRef},
- traits::TrailingZeroInput, SaturatedConversion
+ traits::TrailingZeroInput,
};
use sp_std::{cmp::Ordering, convert::TryFrom, vec::Vec};
@@ -57,7 +59,8 @@ pub type Assignment<T> = sp_npos_elections::Assignment<
CompactAccuracyOf<T>,
>;
-/// The [`IndexAssignment`][sp_npos_elections::IndexAssignment] type specialized for a particular runtime `T`.
+/// The [`IndexAssignment`][sp_npos_elections::IndexAssignment] type specialized for a particular
+/// runtime `T`.
pub type IndexAssignmentOf<T> = sp_npos_elections::IndexAssignmentOf<CompactOf<T>>;
#[derive(Debug, Eq, PartialEq)]
@@ -69,7 +72,7 @@ pub enum MinerError {
/// Submitting a transaction to the pool failed.
PoolSubmissionFailed,
/// The pre-dispatch checks failed for the mined solution.
- PreDispatchChecksFailed,
+ PreDispatchChecksFailed(DispatchError),
/// The solution generated from the miner is not feasible.
Feasibility(FeasibilityError),
/// Something went wrong fetching the lock.
@@ -234,7 +237,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), MinerError> {
Self::unsigned_pre_dispatch_checks(raw_solution).map_err(|err| {
log!(debug, "pre-dispatch checks failed for {} solution: {:?}", solution_type, err);
- MinerError::PreDispatchChecksFailed
+ MinerError::PreDispatchChecksFailed(err)
})?;
Self::feasibility_check(raw_solution.clone(), ElectionCompute::Unsigned).map_err(|err| {
@@ -344,7 +347,11 @@ impl<T: Config> Pallet<T> {
// converting to `Compact`.
let mut index_assignments = sorted_assignments
.into_iter()
- .map(|assignment| IndexAssignmentOf::<T>::new(&assignment, &voter_index, &target_index))
+ .map(|assignment| IndexAssignmentOf::<T>::new(
+ &assignment,
+ &voter_index,
+ &target_index,
+ ))
.collect::<Result<Vec<_>, _>>()?;
// trim assignments list for weight and length.
@@ -416,7 +423,9 @@ impl<T: Config> Pallet<T> {
size,
max_weight,
);
- let removing: usize = assignments.len().saturating_sub(maximum_allowed_voters.saturated_into());
+ let removing: usize = assignments.len().saturating_sub(
+ maximum_allowed_voters.saturated_into(),
+ );
log!(
debug,
"from {} assignments, truncating to {} for weight, removing {}",
@@ -464,7 +473,9 @@ impl<T: Config> Pallet<T> {
}
}
let maximum_allowed_voters =
- if low < assignments.len() && encoded_size_of(&assignments[..low + 1])? <= max_allowed_length {
+ if low < assignments.len() &&
+ encoded_size_of(&assignments[..low + 1])? <= max_allowed_length
+ {
low + 1
} else {
low
@@ -674,6 +685,15 @@ mod max_weight {
fn on_initialize_open_unsigned_without_snapshot() -> Weight {
unreachable!()
}
+ fn finalize_signed_phase_accept_solution() -> Weight {
+ unreachable!()
+ }
+ fn finalize_signed_phase_reject_solution() -> Weight {
+ unreachable!()
+ }
+ fn submit(c: u32) -> Weight {
+ unreachable!()
+ }
fn submit_unsigned(v: u32, t: u32, a: u32, d: u32) -> Weight {
(0 * v + 0 * t + 1000 * a + 0 * d) as Weight
}
@@ -994,7 +1014,11 @@ mod tests {
assert_eq!(
MultiPhase::mine_check_save_submit().unwrap_err(),
- MinerError::PreDispatchChecksFailed,
+ MinerError::PreDispatchChecksFailed(DispatchError::Module{
+ index: 2,
+ error: 1,
+ message: Some("PreDispatchWrongWinnerCount"),
+ }),
);
})
}
@@ -1199,11 +1223,17 @@ mod tests {
let mut storage = StorageValueRef::persistent(&OFFCHAIN_LAST_BLOCK);
storage.clear();
- assert!(!ocw_solution_exists::<Runtime>(), "no solution should be present before we mine one");
+ assert!(
+ !ocw_solution_exists::<Runtime>(),
+ "no solution should be present before we mine one",
+ );
// creates and cache a solution
MultiPhase::offchain_worker(25);
- assert!(ocw_solution_exists::<Runtime>(), "a solution must be cached after running the worker");
+ assert!(
+ ocw_solution_exists::<Runtime>(),
+ "a solution must be cached after running the worker",
+ );
// after an election, the solution must be cleared
// we don't actually care about the result of the election
@@ -1329,10 +1359,15 @@ mod tests {
_ => panic!("bad call: unexpected submission"),
};
- // Custom(3) maps to PreDispatchChecksFailed
- let pre_dispatch_check_error = TransactionValidityError::Invalid(InvalidTransaction::Custom(3));
+ // Custom(7) maps to PreDispatchChecksFailed
+ let pre_dispatch_check_error = TransactionValidityError::Invalid(
+ InvalidTransaction::Custom(7),
+ );
assert_eq!(
- <MultiPhase as ValidateUnsigned>::validate_unsigned(TransactionSource::Local, &call)
+ <MultiPhase as ValidateUnsigned>::validate_unsigned(
+ TransactionSource::Local,
+ &call,
+ )
.unwrap_err(),
pre_dispatch_check_error,
);
@@ -1359,7 +1394,11 @@ mod tests {
let compact_clone = compact.clone();
// when
- MultiPhase::trim_assignments_length(encoded_len, &mut assignments, encoded_size_of).unwrap();
+ MultiPhase::trim_assignments_length(
+ encoded_len,
+ &mut assignments,
+ encoded_size_of,
+ ).unwrap();
// then
let compact = CompactOf::<Runtime>::try_from(assignments.as_slice()).unwrap();
@@ -1383,7 +1422,11 @@ mod tests {
let compact_clone = compact.clone();
// when
- MultiPhase::trim_assignments_length(encoded_len as u32 - 1, &mut assignments, encoded_size_of).unwrap();
+ MultiPhase::trim_assignments_length(
+ encoded_len as u32 - 1,
+ &mut assignments,
+ encoded_size_of,
+ ).unwrap();
// then
let compact = CompactOf::<Runtime>::try_from(assignments.as_slice()).unwrap();
@@ -1414,7 +1457,11 @@ mod tests {
.unwrap();
// when
- MultiPhase::trim_assignments_length(encoded_len - 1, &mut assignments, encoded_size_of).unwrap();
+ MultiPhase::trim_assignments_length(
+ encoded_len - 1,
+ &mut assignments,
+ encoded_size_of,
+ ).unwrap();
// then
assert_eq!(assignments.len(), count - 1, "we must have removed exactly one assignment");
diff --git a/substrate/frame/election-provider-multi-phase/src/weights.rs b/substrate/frame/election-provider-multi-phase/src/weights.rs
index 51b99bc962d43d3c711e92b8427281a861595a4c..6a245ebb512545b4194ed39c6d106c36c47a3aa4 100644
--- a/substrate/frame/election-provider-multi-phase/src/weights.rs
+++ b/substrate/frame/election-provider-multi-phase/src/weights.rs
@@ -18,7 +18,7 @@
//! Autogenerated weights for pallet_election_provider_multi_phase
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
-//! DATE: 2021-06-19, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2021-06-20, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128
// Executed Command:
@@ -47,8 +47,11 @@ pub trait WeightInfo {
fn on_initialize_nothing() -> Weight;
fn on_initialize_open_signed() -> Weight;
fn on_initialize_open_unsigned_with_snapshot() -> Weight;
+ fn finalize_signed_phase_accept_solution() -> Weight;
+ fn finalize_signed_phase_reject_solution() -> Weight;
fn on_initialize_open_unsigned_without_snapshot() -> Weight;
fn elect_queued() -> Weight;
+ fn submit(c: u32, ) -> Weight;
fn submit_unsigned(v: u32, t: u32, a: u32, d: u32, ) -> Weight;
fn feasibility_check(v: u32, t: u32, a: u32, d: u32, ) -> Weight;
}
@@ -57,52 +60,69 @@ pub trait WeightInfo {
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
fn on_initialize_nothing() -> Weight {
- (24_579_000 as Weight)
+ (33_392_000 as Weight)
.saturating_add(T::DbWeight::get().reads(8 as Weight))
}
fn on_initialize_open_signed() -> Weight {
- (87_463_000 as Weight)
+ (115_659_000 as Weight)
.saturating_add(T::DbWeight::get().reads(10 as Weight))
.saturating_add(T::DbWeight::get().writes(4 as Weight))
}
fn on_initialize_open_unsigned_with_snapshot() -> Weight {
- (87_381_000 as Weight)
+ (114_970_000 as Weight)
.saturating_add(T::DbWeight::get().reads(10 as Weight))
.saturating_add(T::DbWeight::get().writes(4 as Weight))
}
+ fn finalize_signed_phase_accept_solution() -> Weight {
+ (51_442_000 as Weight)
+ .saturating_add(T::DbWeight::get().reads(1 as Weight))
+ .saturating_add(T::DbWeight::get().writes(2 as Weight))
+ }
+ fn finalize_signed_phase_reject_solution() -> Weight {
+ (23_160_000 as Weight)
+ .saturating_add(T::DbWeight::get().reads(1 as Weight))
+ .saturating_add(T::DbWeight::get().writes(1 as Weight))
+ }
fn on_initialize_open_unsigned_without_snapshot() -> Weight {
- (18_489_000 as Weight)
+ (24_101_000 as Weight)
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
fn elect_queued() -> Weight {
- (6_038_989_000 as Weight)
- .saturating_add(T::DbWeight::get().reads(2 as Weight))
- .saturating_add(T::DbWeight::get().writes(6 as Weight))
+ (6_153_604_000 as Weight)
+ .saturating_add(T::DbWeight::get().reads(5 as Weight))
+ .saturating_add(T::DbWeight::get().writes(8 as Weight))
+ }
+ fn submit(c: u32, ) -> Weight {
+ (78_972_000 as Weight)
+ // Standard Error: 16_000
+ .saturating_add((308_000 as Weight).saturating_mul(c as Weight))
+ .saturating_add(T::DbWeight::get().reads(4 as Weight))
+ .saturating_add(T::DbWeight::get().writes(3 as Weight))
}
fn submit_unsigned(v: u32, t: u32, a: u32, d: u32, ) -> Weight {
(0 as Weight)
// Standard Error: 12_000
- .saturating_add((3_480_000 as Weight).saturating_mul(v as Weight))
+ .saturating_add((3_572_000 as Weight).saturating_mul(v as Weight))
// Standard Error: 42_000
- .saturating_add((194_000 as Weight).saturating_mul(t as Weight))
+ .saturating_add((23_000 as Weight).saturating_mul(t as Weight))
// Standard Error: 12_000
- .saturating_add((10_498_000 as Weight).saturating_mul(a as Weight))
+ .saturating_add((11_529_000 as Weight).saturating_mul(a as Weight))
// Standard Error: 63_000
- .saturating_add((3_074_000 as Weight).saturating_mul(d as Weight))
+ .saturating_add((3_333_000 as Weight).saturating_mul(d as Weight))
.saturating_add(T::DbWeight::get().reads(7 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
fn feasibility_check(v: u32, t: u32, a: u32, d: u32, ) -> Weight {
(0 as Weight)
// Standard Error: 7_000
- .saturating_add((3_481_000 as Weight).saturating_mul(v as Weight))
- // Standard Error: 24_000
- .saturating_add((385_000 as Weight).saturating_mul(t as Weight))
+ .saturating_add((3_647_000 as Weight).saturating_mul(v as Weight))
+ // Standard Error: 23_000
+ .saturating_add((390_000 as Weight).saturating_mul(t as Weight))
// Standard Error: 7_000
- .saturating_add((8_538_000 as Weight).saturating_mul(a as Weight))
- // Standard Error: 36_000
- .saturating_add((3_322_000 as Weight).saturating_mul(d as Weight))
+ .saturating_add((9_614_000 as Weight).saturating_mul(a as Weight))
+ // Standard Error: 35_000
+ .saturating_add((3_405_000 as Weight).saturating_mul(d as Weight))
.saturating_add(T::DbWeight::get().reads(4 as Weight))
}
}
@@ -110,52 +130,69 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// For backwards compatibility and tests
impl WeightInfo for () {
fn on_initialize_nothing() -> Weight {
- (24_579_000 as Weight)
+ (33_392_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(8 as Weight))
}
fn on_initialize_open_signed() -> Weight {
- (87_463_000 as Weight)
+ (115_659_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(10 as Weight))
.saturating_add(RocksDbWeight::get().writes(4 as Weight))
}
fn on_initialize_open_unsigned_with_snapshot() -> Weight {
- (87_381_000 as Weight)
+ (114_970_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(10 as Weight))
.saturating_add(RocksDbWeight::get().writes(4 as Weight))
}
+ fn finalize_signed_phase_accept_solution() -> Weight {
+ (51_442_000 as Weight)
+ .saturating_add(RocksDbWeight::get().reads(1 as Weight))
+ .saturating_add(RocksDbWeight::get().writes(2 as Weight))
+ }
+ fn finalize_signed_phase_reject_solution() -> Weight {
+ (23_160_000 as Weight)
+ .saturating_add(RocksDbWeight::get().reads(1 as Weight))
+ .saturating_add(RocksDbWeight::get().writes(1 as Weight))
+ }
fn on_initialize_open_unsigned_without_snapshot() -> Weight {
- (18_489_000 as Weight)
+ (24_101_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
fn elect_queued() -> Weight {
- (6_038_989_000 as Weight)
- .saturating_add(RocksDbWeight::get().reads(2 as Weight))
- .saturating_add(RocksDbWeight::get().writes(6 as Weight))
+ (6_153_604_000 as Weight)
+ .saturating_add(RocksDbWeight::get().reads(5 as Weight))
+ .saturating_add(RocksDbWeight::get().writes(8 as Weight))
+ }
+ fn submit(c: u32, ) -> Weight {
+ (78_972_000 as Weight)
+ // Standard Error: 16_000
+ .saturating_add((308_000 as Weight).saturating_mul(c as Weight))
+ .saturating_add(RocksDbWeight::get().reads(4 as Weight))
+ .saturating_add(RocksDbWeight::get().writes(3 as Weight))
}
fn submit_unsigned(v: u32, t: u32, a: u32, d: u32, ) -> Weight {
(0 as Weight)
// Standard Error: 12_000
- .saturating_add((3_480_000 as Weight).saturating_mul(v as Weight))
+ .saturating_add((3_572_000 as Weight).saturating_mul(v as Weight))
// Standard Error: 42_000
- .saturating_add((194_000 as Weight).saturating_mul(t as Weight))
+ .saturating_add((23_000 as Weight).saturating_mul(t as Weight))
// Standard Error: 12_000
- .saturating_add((10_498_000 as Weight).saturating_mul(a as Weight))
+ .saturating_add((11_529_000 as Weight).saturating_mul(a as Weight))
// Standard Error: 63_000
- .saturating_add((3_074_000 as Weight).saturating_mul(d as Weight))
+ .saturating_add((3_333_000 as Weight).saturating_mul(d as Weight))
.saturating_add(RocksDbWeight::get().reads(7 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
fn feasibility_check(v: u32, t: u32, a: u32, d: u32, ) -> Weight {
(0 as Weight)
// Standard Error: 7_000
- .saturating_add((3_481_000 as Weight).saturating_mul(v as Weight))
- // Standard Error: 24_000
- .saturating_add((385_000 as Weight).saturating_mul(t as Weight))
+ .saturating_add((3_647_000 as Weight).saturating_mul(v as Weight))
+ // Standard Error: 23_000
+ .saturating_add((390_000 as Weight).saturating_mul(t as Weight))
// Standard Error: 7_000
- .saturating_add((8_538_000 as Weight).saturating_mul(a as Weight))
- // Standard Error: 36_000
- .saturating_add((3_322_000 as Weight).saturating_mul(d as Weight))
+ .saturating_add((9_614_000 as Weight).saturating_mul(a as Weight))
+ // Standard Error: 35_000
+ .saturating_add((3_405_000 as Weight).saturating_mul(d as Weight))
.saturating_add(RocksDbWeight::get().reads(4 as Weight))
}
}
diff --git a/substrate/frame/support/src/storage/bounded_btree_map.rs b/substrate/frame/support/src/storage/bounded_btree_map.rs
index 8c50557618eec9551075a2f4e30eee04aa441e00..0c1994d63a35da0f630346f92fe687891456cba1 100644
--- a/substrate/frame/support/src/storage/bounded_btree_map.rs
+++ b/substrate/frame/support/src/storage/bounded_btree_map.rs
@@ -39,7 +39,8 @@ pub struct BoundedBTreeMap<K, V, S>(BTreeMap<K, V>, PhantomData<S>);
impl<K, V, S> Decode for BoundedBTreeMap<K, V, S>
where
- BTreeMap<K, V>: Decode,
+ K: Decode + Ord,
+ V: Decode,
S: Get<u32>,
{
fn decode<I: codec::Input>(input: &mut I) -> Result<Self, codec::Error> {
@@ -115,14 +116,15 @@ where
self.0.get_mut(key)
}
- /// Exactly the same semantics as [`BTreeMap::insert`], but returns an `Err` (and is a noop) if the
- /// new length of the map exceeds `S`.
- pub fn try_insert(&mut self, key: K, value: V) -> Result<(), ()> {
- if self.len() < Self::bound() {
- self.0.insert(key, value);
- Ok(())
+ /// Exactly the same semantics as [`BTreeMap::insert`], but returns an `Err` (and is a noop) if
+ /// the new length of the map exceeds `S`.
+ ///
+ /// In the `Err` case, returns the inserted pair so it can be further used without cloning.
+ pub fn try_insert(&mut self, key: K, value: V) -> Result<Option<V>, (K, V)> {
+ if self.len() < Self::bound() || self.0.contains_key(&key) {
+ Ok(self.0.insert(key, value))
} else {
- Err(())
+ Err((key, value))
}
}
@@ -407,4 +409,50 @@ pub mod test {
Err("BoundedBTreeMap exceeds its limit".into()),
);
}
+
+ #[test]
+ fn unequal_eq_impl_insert_works() {
+ // given a struct with a strange notion of equality
+ #[derive(Debug)]
+ struct Unequal(u32, bool);
+
+ impl PartialEq for Unequal {
+ fn eq(&self, other: &Self) -> bool {
+ self.0 == other.0
+ }
+ }
+ impl Eq for Unequal {}
+
+ impl Ord for Unequal {
+ fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+ self.0.cmp(&other.0)
+ }
+ }
+
+ impl PartialOrd for Unequal {
+ fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+ Some(self.cmp(other))
+ }
+ }
+
+ let mut map = BoundedBTreeMap::<Unequal, u32, Four>::new();
+
+ // when the set is full
+
+ for i in 0..4 {
+ map.try_insert(Unequal(i, false), i).unwrap();
+ }
+
+ // can't insert a new distinct member
+ map.try_insert(Unequal(5, false), 5).unwrap_err();
+
+ // but _can_ insert a distinct member which compares equal, though per the documentation,
+ // neither the set length nor the actual member are changed, but the value is
+ map.try_insert(Unequal(0, true), 6).unwrap();
+ assert_eq!(map.len(), 4);
+ let (zero_key, zero_value) = map.get_key_value(&Unequal(0, true)).unwrap();
+ assert_eq!(zero_key.0, 0);
+ assert_eq!(zero_key.1, false);
+ assert_eq!(*zero_value, 6);
+ }
}
diff --git a/substrate/frame/support/src/storage/bounded_btree_set.rs b/substrate/frame/support/src/storage/bounded_btree_set.rs
index f551a3cbfa38e2c823794b240f315478272af763..10c2300a08a09fa343da909a1b23652e2fd8118a 100644
--- a/substrate/frame/support/src/storage/bounded_btree_set.rs
+++ b/substrate/frame/support/src/storage/bounded_btree_set.rs
@@ -39,7 +39,7 @@ pub struct BoundedBTreeSet<T, S>(BTreeSet<T>, PhantomData<S>);
impl<T, S> Decode for BoundedBTreeSet<T, S>
where
- BTreeSet<T>: Decode,
+ T: Decode + Ord,
S: Get<u32>,
{
fn decode<I: codec::Input>(input: &mut I) -> Result<Self, codec::Error> {
@@ -103,14 +103,15 @@ where
self.0.clear()
}
- /// Exactly the same semantics as [`BTreeSet::insert`], but returns an `Err` (and is a noop) if the
- /// new length of the set exceeds `S`.
- pub fn try_insert(&mut self, item: T) -> Result<(), ()> {
- if self.len() < Self::bound() {
- self.0.insert(item);
- Ok(())
+ /// Exactly the same semantics as [`BTreeSet::insert`], but returns an `Err` (and is a noop) if
+ /// the new length of the set exceeds `S`.
+ ///
+ /// In the `Err` case, returns the inserted item so it can be further used without cloning.
+ pub fn try_insert(&mut self, item: T) -> Result<bool, T> {
+ if self.len() < Self::bound() || self.0.contains(&item) {
+ Ok(self.0.insert(item))
} else {
- Err(())
+ Err(item)
}
}
@@ -393,4 +394,49 @@ pub mod test {
Err("BoundedBTreeSet exceeds its limit".into()),
);
}
+
+ #[test]
+ fn unequal_eq_impl_insert_works() {
+ // given a struct with a strange notion of equality
+ #[derive(Debug)]
+ struct Unequal(u32, bool);
+
+ impl PartialEq for Unequal {
+ fn eq(&self, other: &Self) -> bool {
+ self.0 == other.0
+ }
+ }
+ impl Eq for Unequal {}
+
+ impl Ord for Unequal {
+ fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+ self.0.cmp(&other.0)
+ }
+ }
+
+ impl PartialOrd for Unequal {
+ fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+ Some(self.cmp(other))
+ }
+ }
+
+ let mut set = BoundedBTreeSet::<Unequal, Four>::new();
+
+ // when the set is full
+
+ for i in 0..4 {
+ set.try_insert(Unequal(i, false)).unwrap();
+ }
+
+ // can't insert a new distinct member
+ set.try_insert(Unequal(5, false)).unwrap_err();
+
+ // but _can_ insert a distinct member which compares equal, though per the documentation,
+ // neither the set length nor the actual member are changed
+ set.try_insert(Unequal(0, true)).unwrap();
+ assert_eq!(set.len(), 4);
+ let zero_item = set.get(&Unequal(0, true)).unwrap();
+ assert_eq!(zero_item.0, 0);
+ assert_eq!(zero_item.1, false);
+ }
}
diff --git a/substrate/primitives/npos-elections/compact/src/lib.rs b/substrate/primitives/npos-elections/compact/src/lib.rs
index e8cde87744539b77c696385fad91c0d231c592a2..0e9fbb34eea17d0518758e7571563862449989de 100644
--- a/substrate/primitives/npos-elections/compact/src/lib.rs
+++ b/substrate/primitives/npos-elections/compact/src/lib.rs
@@ -169,7 +169,7 @@ fn struct_def(
);
quote!{
#compact_impl
- #[derive(Default, PartialEq, Eq, Clone, Debug)]
+ #[derive(Default, PartialEq, Eq, Clone, Debug, PartialOrd, Ord)]
}
} else {
// automatically derived.