From b9aa8817cf93847caf36a450df1c654ca04933ef Mon Sep 17 00:00:00 2001
From: kianenigma <kian@parity.io>
Date: Mon, 27 Jan 2025 10:41:29 +0000
Subject: [PATCH] stateless data provider so that EPM and EPMB can work
together
---
substrate/bin/node/runtime/src/lib.rs | 28 +++++-
.../election-provider-multi-block/src/lib.rs | 4 +-
.../election-provider-multi-phase/src/lib.rs | 5 +-
.../election-provider-support/src/lib.rs | 16 ++++
.../election-provider-support/src/onchain.rs | 5 +-
substrate/frame/staking/src/benchmarking.rs | 6 +-
substrate/frame/staking/src/pallet/impls.rs | 93 ++++++++++---------
substrate/frame/staking/src/tests.rs | 4 +-
8 files changed, 103 insertions(+), 58 deletions(-)
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 8f9c2d2e295..efb7cabdfd0 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -831,6 +831,31 @@ impl pallet_staking::BenchmarkingConfig for StakingBenchmarkingConfig {
type MaxValidators = ConstU32<1000>;
}
+use frame_election_provider_support::{BoundedSupportsOf, ElectionProvider, PageIndex};
+pub struct MultiElectionProvider;
+impl ElectionProvider for MultiElectionProvider {
+ type AccountId = <MultiBlock as ElectionProvider>::AccountId;
+ type BlockNumber = <MultiBlock as ElectionProvider>::BlockNumber;
+ type DataProvider = <MultiBlock as ElectionProvider>::DataProvider;
+ type Error = <MultiBlock as ElectionProvider>::Error;
+ type Pages = <MultiBlock as ElectionProvider>::Pages;
+ type MaxBackersPerWinner = <MultiBlock as ElectionProvider>::MaxBackersPerWinner;
+ type MaxWinnersPerPage = <MultiBlock as ElectionProvider>::MaxWinnersPerPage;
+
+ fn elect(page: PageIndex) -> Result<BoundedSupportsOf<Self>, Self::Error> {
+ if page == 0 {
+ // TODO: later on, we can even compare the results of the multi-page and multi-block
+ // election like this.
+ let _ = ElectionProviderMultiPhase::elect(page);
+ }
+ MultiBlock::elect(page)
+ }
+
+ fn ongoing() -> bool {
+ MultiBlock::ongoing()
+ }
+}
+
impl pallet_staking::Config for Runtime {
type OldCurrency = Balances;
type Currency = Balances;
@@ -855,8 +880,7 @@ impl pallet_staking::Config for Runtime {
type NextNewSession = Session;
type MaxExposurePageSize = MaxExposurePageSize;
type MaxValidatorSet = MaxActiveValidators;
- type ElectionProvider = MultiBlock;
- // type ElectionProvider = ElectionProviderMultiPhase;
+ type ElectionProvider = MultiElectionProvider;
type GenesisElectionProvider = onchain::OnChainExecution<OnChainSeqPhragmen>;
type VoterList = VoterList;
type NominationsQuota = pallet_staking::FixedNominationsQuota<MAX_QUOTA_NOMINATIONS>;
diff --git a/substrate/frame/election-provider-multi-block/src/lib.rs b/substrate/frame/election-provider-multi-block/src/lib.rs
index 6ba447726da..282c45b76c3 100644
--- a/substrate/frame/election-provider-multi-block/src/lib.rs
+++ b/substrate/frame/election-provider-multi-block/src/lib.rs
@@ -60,6 +60,8 @@
//! 3. signed
//! 4. unsigned
//!
+//! This is critical for the phase transition to work.
+//!
//! This should be manually checked, there is not automated way to test it.
//!
//! ## Pagination
@@ -552,8 +554,6 @@ pub mod pallet {
Self::phase_transition(Phase::SignedValidation(now));
// we don't do anything else here. We expect the signed sub-pallet to handle
// whatever else needs to be done.
- // TODO: this notification system based on block numbers is 100% based on the
- // on_initialize of the parent pallet is called before the rest of them.
todo_weight
},
diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs
index 0f4a5934196..09bb54221b5 100644
--- a/substrate/frame/election-provider-multi-phase/src/lib.rs
+++ b/substrate/frame/election-provider-multi-phase/src/lib.rs
@@ -768,7 +768,6 @@ pub mod pallet {
#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_initialize(now: BlockNumberFor<T>) -> Weight {
- return Default::default();
let next_election = T::DataProvider::next_election_prediction(now).max(now);
let signed_deadline = T::SignedPhase::get() + T::UnsignedPhase::get();
@@ -1525,7 +1524,7 @@ impl<T: Config> Pallet<T> {
fn create_snapshot_external(
) -> Result<(Vec<T::AccountId>, Vec<VoterOf<T>>, u32), ElectionError<T>> {
let election_bounds = T::ElectionBounds::get();
- let targets = T::DataProvider::electable_targets(election_bounds.targets, SINGLE_PAGE)
+ let targets = T::DataProvider::electable_targets_stateless(election_bounds.targets)
.and_then(|t| {
election_bounds.ensure_targets_limits(
CountBound(t.len() as u32),
@@ -1535,7 +1534,7 @@ impl<T: Config> Pallet<T> {
})
.map_err(ElectionError::DataProvider)?;
- let voters = T::DataProvider::electing_voters(election_bounds.voters, SINGLE_PAGE)
+ let voters = T::DataProvider::electing_voters_stateless(election_bounds.voters)
.and_then(|v| {
election_bounds.ensure_voters_limits(
CountBound(v.len() as u32),
diff --git a/substrate/frame/election-provider-support/src/lib.rs b/substrate/frame/election-provider-support/src/lib.rs
index e9055d456c4..234314d21d3 100644
--- a/substrate/frame/election-provider-support/src/lib.rs
+++ b/substrate/frame/election-provider-support/src/lib.rs
@@ -330,6 +330,15 @@ pub trait ElectionDataProvider {
page: PageIndex,
) -> data_provider::Result<Vec<Self::AccountId>>;
+ /// A state-less version of [`Self::electing_targets`].
+ ///
+ /// An election-provider that only uses 1 page should use this.
+ fn electable_targets_stateless(
+ bounds: DataProviderBounds,
+ ) -> data_provider::Result<Vec<Self::AccountId>> {
+ Self::electable_targets(bounds, 0)
+ }
+
/// All the voters that participate in the election associated with page `page`, thus
/// "electing".
///
@@ -342,6 +351,13 @@ pub trait ElectionDataProvider {
page: PageIndex,
) -> data_provider::Result<Vec<VoterOf<Self>>>;
+ /// A state-less version of [`Self::electing_voters`].
+ fn electing_voters_stateless(
+ bounds: DataProviderBounds,
+ ) -> data_provider::Result<Vec<VoterOf<Self>>> {
+ Self::electing_voters(bounds, 0)
+ }
+
/// The number of targets to elect.
///
/// This should be implemented as a self-weighing function. The implementor should register its
diff --git a/substrate/frame/election-provider-support/src/onchain.rs b/substrate/frame/election-provider-support/src/onchain.rs
index 65f5bc05cf6..3fe8f3b4bc3 100644
--- a/substrate/frame/election-provider-support/src/onchain.rs
+++ b/substrate/frame/election-provider-support/src/onchain.rs
@@ -184,8 +184,9 @@ impl<T: Config> ElectionProvider for OnChainExecution<T> {
type Error = Error;
type MaxWinnersPerPage = T::MaxWinnersPerPage;
type MaxBackersPerWinner = T::MaxBackersPerWinner;
- // can support any number of pages, as this is meant to be called "instantly".
- type Pages = sp_core::ConstU32<{ u32::MAX }>;
+ // can support any number of pages, as this is meant to be called "instantly". We don't care
+ // about this value here.
+ type Pages = sp_core::ConstU32<1>;
type DataProvider = T::DataProvider;
fn elect(page: PageIndex) -> Result<BoundedSupportsOf<Self>, Self::Error> {
diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs
index 984c4aadcd7..4538556f56c 100644
--- a/substrate/frame/staking/src/benchmarking.rs
+++ b/substrate/frame/staking/src/benchmarking.rs
@@ -42,7 +42,6 @@ use frame_system::RawOrigin;
const SEED: u32 = 0;
const MAX_SPANS: u32 = 100;
const MAX_SLASHES: u32 = 1000;
-const SINGLE_PAGE: u32 = 0;
type MaxValidators<T> = <<T as Config>::BenchmarkingConfig as BenchmarkingConfig>::MaxValidators;
type MaxNominators<T> = <<T as Config>::BenchmarkingConfig as BenchmarkingConfig>::MaxNominators;
@@ -1031,7 +1030,10 @@ mod benchmarks {
let voters;
#[block]
{
- voters = <Staking<T>>::get_npos_voters(DataProviderBounds::default(), SINGLE_PAGE);
+ voters = <Staking<T>>::get_npos_voters(
+ DataProviderBounds::default(),
+ &SnapshotStatus::<T::AccountId>::Waiting,
+ );
}
assert_eq!(voters.len(), num_voters);
diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs
index 96336362334..a08f15b47b7 100644
--- a/substrate/frame/staking/src/pallet/impls.rs
+++ b/substrate/frame/staking/src/pallet/impls.rs
@@ -1038,7 +1038,7 @@ impl<T: Config> Pallet<T> {
/// This function is self-weighing as [`DispatchClass::Mandatory`].
pub(crate) fn get_npos_voters(
bounds: DataProviderBounds,
- page: PageIndex,
+ status: &SnapshotStatus<T::AccountId>,
) -> Vec<VoterOf<Self>> {
let mut voters_size_tracker: StaticTracker<Self> = StaticTracker::default();
@@ -1057,7 +1057,7 @@ impl<T: Config> Pallet<T> {
let mut nominators_taken = 0u32;
let mut min_active_stake = u64::MAX;
- let mut sorted_voters = match VoterSnapshotStatus::<T>::get() {
+ let mut sorted_voters = match status {
// start the snapshot processing from the beginning.
SnapshotStatus::Waiting => T::VoterList::iter(),
// snapshot continues, start from the last iterated voter in the list.
@@ -1140,29 +1140,6 @@ impl<T: Config> Pallet<T> {
}
}
- // update the voter snapshot status.
- VoterSnapshotStatus::<T>::mutate(|status| {
- match (page, status.clone()) {
- // last page, reset status for next round.
- (0, _) => *status = SnapshotStatus::Waiting,
-
- (_, SnapshotStatus::Waiting) | (_, SnapshotStatus::Ongoing(_)) => {
- let maybe_last = all_voters.last().map(|(x, _, _)| x).cloned();
-
- if let Some(ref last) = maybe_last {
- if maybe_last == T::VoterList::iter().last() {
- // all voters in the voter list have been consumed.
- *status = SnapshotStatus::Consumed;
- } else {
- *status = SnapshotStatus::Ongoing(last.clone());
- }
- }
- },
- // do nothing.
- (_, SnapshotStatus::Consumed) => (),
- }
- });
-
// all_voters should have not re-allocated.
debug_assert!(all_voters.capacity() == page_len_prediction as usize);
@@ -1173,17 +1150,6 @@ impl<T: Config> Pallet<T> {
MinimumActiveStake::<T>::put(min_active_stake);
- log!(
- info,
- "[page {}, status {:?}, bounds {:?}] generated {} npos voters, {} from validators and {} nominators",
- page,
- VoterSnapshotStatus::<T>::get(),
- bounds,
- all_voters.len(),
- validators_taken,
- nominators_taken
- );
-
all_voters
}
@@ -1463,13 +1429,58 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
bounds: DataProviderBounds,
page: PageIndex,
) -> data_provider::Result<Vec<VoterOf<Self>>> {
- let voters = Self::get_npos_voters(bounds, page);
+ let mut status = VoterSnapshotStatus::<T>::get();
+ let voters = Self::get_npos_voters(bounds, &status);
+
+ // update the voter snapshot status.
+ match (page, &status) {
+ // last page, reset status for next round.
+ (0, _) => status = SnapshotStatus::Waiting,
+
+ (_, SnapshotStatus::Waiting) | (_, SnapshotStatus::Ongoing(_)) => {
+ let maybe_last = voters.last().map(|(x, _, _)| x).cloned();
+
+ if let Some(ref last) = maybe_last {
+ if maybe_last == T::VoterList::iter().last() {
+ // all voters in the voter list have been consumed.
+ status = SnapshotStatus::Consumed;
+ } else {
+ status = SnapshotStatus::Ongoing(last.clone());
+ }
+ }
+ },
+ // do nothing.
+ (_, SnapshotStatus::Consumed) => (),
+ }
+ log!(
+ info,
+ "[page {}, status {:?}, bounds {:?}] generated {} npos voters",
+ page,
+ VoterSnapshotStatus::<T>::get(),
+ bounds,
+ voters.len(),
+ );
+ VoterSnapshotStatus::<T>::put(status);
debug_assert!(!bounds.slice_exhausted(&voters));
Ok(voters)
}
+ fn electing_voters_stateless(
+ bounds: DataProviderBounds,
+ ) -> data_provider::Result<Vec<VoterOf<Self>>> {
+ let voters = Self::get_npos_voters(bounds, &SnapshotStatus::Waiting);
+ log!(
+ info,
+ "[stateless, status {:?}, bounds {:?}] generated {} npos voters",
+ VoterSnapshotStatus::<T>::get(),
+ bounds,
+ voters.len(),
+ );
+ Ok(voters)
+ }
+
fn electable_targets(
bounds: DataProviderBounds,
page: PageIndex,
@@ -2295,7 +2306,6 @@ impl<T: Config> Pallet<T> {
///
/// - `NextElectionPage`: should only be set if pages > 1 and if we are within `pages-election
/// -> election`
- /// - `ElectableStashes`: should only be set in `pages-election -> election block`
/// - `VoterSnapshotStatus`: cannot be argued about as we don't know when we get a call to data
/// provider, but we know it should never be set if we have 1 page.
///
@@ -2304,13 +2314,6 @@ impl<T: Config> Pallet<T> {
let next_election = Self::next_election_prediction(now);
let pages = Self::election_pages().saturated_into::<BlockNumberFor<T>>();
let election_prep_start = next_election - pages;
- let is_mid_election = now >= election_prep_start && now < next_election;
- if !is_mid_election {
- ensure!(
- ElectableStashes::<T>::get().is_empty(),
- "ElectableStashes should not be empty mid election"
- );
- }
if pages > One::one() && now >= election_prep_start {
ensure!(
diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs
index 200d4e2fd8d..152d623330e 100644
--- a/substrate/frame/staking/src/tests.rs
+++ b/substrate/frame/staking/src/tests.rs
@@ -5625,7 +5625,7 @@ mod election_data_provider {
fn estimate_next_election_single_page_works() {
ExtBuilder::default().session_per_era(5).period(5).build_and_execute(|| {
// first session is always length 0.
- for b in 1..20 {
+ for b in 1..19 {
run_to_block(b);
assert_eq!(Staking::next_election_prediction(System::block_number()), 20);
}
@@ -5635,7 +5635,7 @@ mod election_data_provider {
assert_eq!(Staking::next_election_prediction(System::block_number()), 45);
assert_eq!(*staking_events().last().unwrap(), Event::StakersElected);
- for b in 21..45 {
+ for b in 21..44 {
run_to_block(b);
assert_eq!(Staking::next_election_prediction(System::block_number()), 45);
}
--
GitLab