Skip to content

Commit

Permalink
fix tests and a bug
Browse files Browse the repository at this point in the history
  • Loading branch information
kianenigma committed Jan 16, 2025
1 parent 1b5ddc6 commit c5efc26
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 177 deletions.
8 changes: 7 additions & 1 deletion substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,13 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type SlashHandler = (); // burn slashes
type RewardHandler = (); // rewards are minted from the void
type DataProvider = Staking;
type Fallback = onchain::OnChainExecution<OnChainSeqPhragmen>;
type Fallback = frame_election_provider_support::NoElection<(
AccountId,
BlockNumber,
Staking,
MaxActiveValidators,
MaxBackersPerWinner,
)>;
type GovernanceFallback = onchain::OnChainExecution<OnChainSeqPhragmen>;
type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Self>, OffchainRandomBalancing>;
type ForceOrigin = EnsureRootOrHalfCouncil;
Expand Down
19 changes: 10 additions & 9 deletions substrate/frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,9 +778,10 @@ pub mod pallet {

log!(
trace,
"current phase {:?}, next election {:?}, metadata: {:?}",
"current phase {:?}, next election {:?}, queued? {:?}, metadata: {:?}",
current_phase,
next_election,
QueuedSolution::<T>::get().map(|rs| (rs.supports.len(), rs.compute, rs.score)),
SnapshotMetadata::<T>::get()
);
match current_phase {
Expand Down Expand Up @@ -1652,6 +1653,7 @@ impl<T: Config> Pallet<T> {
QueuedSolution::<T>::take()
.ok_or(ElectionError::<T>::NothingQueued)
.or_else(|_| {
log!(warn, "No solution queued, falling back to instant fallback.",);
// default data provider bounds are unbounded. calling `instant_elect` with
// unbounded data provider bounds means that the on-chain `T:Bounds` configs will
// *not* be overwritten.
Expand All @@ -1670,16 +1672,12 @@ impl<T: Config> Pallet<T> {
})
.map(|ReadySolution { compute, score, supports }| {
Self::deposit_event(Event::ElectionFinalized { compute, score });
if Round::<T>::get() != 1 {
log!(info, "Finalized election round with compute {:?}.", compute);
}
log!(info, "Finalized election round with compute {:?}.", compute);
supports
})
.map_err(|err| {
Self::deposit_event(Event::ElectionFailed);
if Round::<T>::get() != 1 {
log!(warn, "Failed to finalize election round. reason {:?}", err);
}
log!(warn, "Failed to finalize election round. reason {:?}", err);
err
})
}
Expand Down Expand Up @@ -1790,7 +1788,7 @@ impl<T: Config> ElectionProvider for Pallet<T> {
// Note: this pallet **MUST** only by used in the single-page mode.
ensure!(page == SINGLE_PAGE, ElectionError::<T>::MultiPageNotSupported);

match Self::do_elect() {
let res = match Self::do_elect() {
Ok(bounded_supports) => {
// All went okay, record the weight, put sign to be Off, clean snapshot, etc.
Self::weigh_supports(&bounded_supports);
Expand All @@ -1802,7 +1800,10 @@ impl<T: Config> ElectionProvider for Pallet<T> {
Self::phase_transition(Phase::Emergency);
Err(why)
},
}
};

log!(info, "ElectionProvider::elect({}) => {:?}", page, res.as_ref().map(|s| s.len()));
res
}

fn ongoing() -> bool {
Expand Down
13 changes: 6 additions & 7 deletions substrate/frame/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,15 @@ mod benchmarks {
#[benchmark]
fn on_initialize_noop() {
assert!(ElectableStashes::<T>::get().is_empty());
assert_eq!(ElectingStartedAt::<T>::get(), None);
assert_eq!(NextElectionPage::<T>::get(), None);

#[block]
{
Pallet::<T>::on_initialize(1_u32.into());
}

assert!(ElectableStashes::<T>::get().is_empty());
assert_eq!(ElectingStartedAt::<T>::get(), None);
assert_eq!(NextElectionPage::<T>::get(), None);
}

#[benchmark]
Expand Down Expand Up @@ -272,14 +272,14 @@ mod benchmarks {
}

ElectableStashes::<T>::set(stashes);
ElectingStartedAt::<T>::set(Some(10u32.into()));
NextElectionPage::<T>::set(Some(10u32.into()));

#[block]
{
Pallet::<T>::clear_election_metadata()
}

assert!(ElectingStartedAt::<T>::get().is_none());
assert!(NextElectionPage::<T>::get().is_none());
assert!(ElectableStashes::<T>::get().is_empty());

Ok(())
Expand Down Expand Up @@ -1245,20 +1245,19 @@ mod tests {
ExtBuilder::default().build_and_execute(|| {
let n = 10;

let current_era = CurrentEra::<Test>::get().unwrap();
let (validator_stash, nominators) = create_validator_with_nominators::<Test>(
n,
<<Test as Config>::MaxExposurePageSize as Get<_>>::get(),
false,
false,
RewardDestination::Staked,
CurrentEra::<Test>::get().unwrap(),
current_era,
)
.unwrap();

assert_eq!(nominators.len() as u32, n);

let current_era = CurrentEra::<Test>::get().unwrap();

let original_stakeable_balance = asset::stakeable_balance::<Test>(&validator_stash);
assert_ok!(Staking::payout_stakers_by_page(
RuntimeOrigin::signed(1337),
Expand Down
90 changes: 14 additions & 76 deletions substrate/frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ impl<T: Config> Pallet<T> {

/// Clears up all election preparation metadata in storage.
pub(crate) fn clear_election_metadata() {
ElectingStartedAt::<T>::kill();
// TODO: voter snapshot status should also be killed?
NextElectionPage::<T>::kill();
ElectableStashes::<T>::kill();
}

Expand Down Expand Up @@ -640,10 +641,9 @@ impl<T: Config> Pallet<T> {

// Clean old era information.
if let Some(old_era) = new_planned_era.checked_sub(T::HistoryDepth::get() + 1) {
log!(trace, "Removing era information for {:?}", old_era);
Self::clear_era_information(old_era);
}
// Including election prep metadata.
Self::clear_election_metadata();
}

/// Potentially plan a new era.
Expand Down Expand Up @@ -713,12 +713,13 @@ impl<T: Config> Pallet<T> {
_ => {},
}
// election failed, clear election prep metadata.
Self::clear_election_metadata();
Self::deposit_event(Event::StakingElectionFailed);
Self::clear_election_metadata();

None
} else {
Self::deposit_event(Event::StakersElected);
Self::clear_election_metadata();
Self::trigger_new_era(start_session_index);

Some(validators)
Expand All @@ -737,7 +738,7 @@ impl<T: Config> Pallet<T> {
/// result of the election. We ensure that only the winners that are part of the electable
/// stashes have exposures collected for the next era.
pub(crate) fn do_elect_paged(page: PageIndex) -> Weight {
let paged_result = match <T::ElectionProvider>::elect(page) {
let paged_result = match T::ElectionProvider::elect(page) {
Ok(result) => result,
Err(e) => {
log!(warn, "election provider page failed due to {:?} (page: {})", e, page);
Expand Down Expand Up @@ -2166,77 +2167,14 @@ impl<T: Config> Pallet<T> {
}

/// Invariants:
/// * If the election preparation has started (i.e. `now` >= `expected_election - n_pages`):
/// * The election preparation metadata should be set (`ElectingStartedAt`);
/// * The electable stashes should not be empty;
/// * The exposures for the current electable stashes should have been collected;
/// * If the election preparation has not started yet:
/// * The election preparation metadata is empty;
/// * The electable stashes for this era is empty;
pub fn ensure_snapshot_metadata_state(now: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
let pages: BlockNumberFor<T> = Self::election_pages().into();
let next_election = <Self as ElectionDataProvider>::next_election_prediction(now);
let expect_election_start_at = next_election.saturating_sub(pages);

let election_prep_started = now >= expect_election_start_at;

if !election_prep_started {
// election prep should have not been started yet, no metadata in storage.
ensure!(
ElectableStashes::<T>::get().is_empty(),
"unexpected electable stashes in storage while election prep hasn't started."
);
ensure!(
ElectingStartedAt::<T>::get().is_none(),
"unexpected election metadata while election prep hasn't started.",
);
ensure!(
VoterSnapshotStatus::<T>::get() == SnapshotStatus::Waiting,
"unexpected voter snapshot status in storage."
);

return Ok(())
}

// from now on, we expect the election to have started. check election metadata, electable
// targets and era exposures.
let maybe_electing_started = ElectingStartedAt::<T>::get();

if maybe_electing_started.is_none() {
return Err(
"election prep should have started already, but no election metadata in storage."
.into(),
);
}

let started_at = maybe_electing_started.unwrap();

ensure!(
started_at == expect_election_start_at,
"unexpected electing_started_at block number in storage."
);
ensure!(
!ElectableStashes::<T>::get().is_empty(),
"election should have been started and the electable stashes non empty."
);

// all the current electable stashes exposures should have been collected and
// stored for the next era, and their total exposure should be > 0.
for s in ElectableStashes::<T>::get().iter() {
ensure!(
EraInfo::<T>::get_paged_exposure(
Self::current_era().unwrap_or_default().saturating_add(1),
s,
0
)
.defensive_proof("electable stash exposure does not exist, unexpected.")
.unwrap()
.exposure_metadata
.total != Zero::zero(),
"no exposures collected for an electable stash."
);
}

///
/// Test invariants of:
///
/// - `NextElectionPage`
/// - `ElectableStashes`
/// - `VoterSnapshotStatus`
pub fn ensure_snapshot_metadata_state(_now: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
// TODO:
Ok(())
}

Expand Down
35 changes: 22 additions & 13 deletions substrate/frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use frame_support::{
};
use frame_system::{ensure_root, ensure_signed, pallet_prelude::*};
use sp_runtime::{
traits::{One, SaturatedConversion, StaticLookup, Zero},
traits::{SaturatedConversion, StaticLookup, Zero},
ArithmeticError, Perbill, Percent, Saturating,
};

Expand Down Expand Up @@ -739,10 +739,12 @@ pub mod pallet {

/// Keeps track of an ongoing multi-page election solution request.
///
/// Stores the block number of when the first election page was requested. `None` indicates
/// that the election results haven't started to be fetched.
/// If `Some(_)``, it is the next page that we intend to elect. If `None`, we are not in the
/// election process.
///
/// This is only set in multi-block elections. Should always be `None` otherwise.
#[pallet::storage]
pub(crate) type ElectingStartedAt<T: Config> = StorageValue<_, BlockNumberFor<T>, OptionQuery>;
pub(crate) type NextElectionPage<T: Config> = StorageValue<_, PageIndex, OptionQuery>;

/// A bounded list of the "electable" stashes that resulted from a successful election.
#[pallet::storage]
Expand Down Expand Up @@ -1016,27 +1018,34 @@ pub mod pallet {
/// that the `ElectableStashes` has been populated with all validators from all pages at
/// the time of the election.
fn on_initialize(now: BlockNumberFor<T>) -> Weight {
let pages: BlockNumberFor<T> = Self::election_pages().into();
let pages = Self::election_pages();
crate::log!(
trace,
"now: {:?}, NextElectionPage: {:?}",
now,
NextElectionPage::<T>::get()
);

// election ongoing, fetch the next page.
let inner_weight = if let Some(started_at) = ElectingStartedAt::<T>::get() {
let next_page =
pages.saturating_sub(One::one()).saturating_sub(now.saturating_sub(started_at));

Self::do_elect_paged(next_page.saturated_into::<PageIndex>())
let inner_weight = if let Some(next_page) = NextElectionPage::<T>::get() {
let next_next_page = next_page.checked_sub(1);
NextElectionPage::<T>::set(next_next_page);
Self::do_elect_paged(next_page)
} else {
// election isn't ongoing yet, check if it should start.
let next_election = <Self as ElectionDataProvider>::next_election_prediction(now);

if now == (next_election.saturating_sub(pages)) {
if now == (next_election.saturating_sub(pages.into())) {
crate::log!(
trace,
"elect(): start fetching solution pages. expected pages: {:?}",
pages
);

ElectingStartedAt::<T>::set(Some(now));
Self::do_elect_paged(pages.saturated_into::<PageIndex>().saturating_sub(1))
let current_page = pages.saturating_sub(1);
let next_page = current_page.checked_sub(1);
NextElectionPage::<T>::set(next_page);
Self::do_elect_paged(current_page)
} else {
Weight::default()
}
Expand Down
Loading

0 comments on commit c5efc26

Please sign in to comment.