Skip to content

Commit

Permalink
Target snapshot in a different page, to reduce PoV even more
Browse files Browse the repository at this point in the history
  • Loading branch information
@kianenigma
kianenigma committed Feb 19, 2025
1 parent 0da20cd commit 2d0a3d7
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 48 deletions.
23 changes: 12 additions & 11 deletions substrate/frame/election-provider-multi-block/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ mod benchmarks {
fn on_initialize_into_snapshot_msp() -> Result<(), BenchmarkError> {
assert!(T::Pages::get() >= 2, "this benchmark only works in a runtime with 2 pages or more, set at least `type Pages = 2` for benchmark run");
T::DataProvider::set_next_election(Pallet::<T>::reasonable_next_election());
// TODO: the results of this benchmark cause too many hits to voters bags list, why???

// roll to next block until we are about to go into the snapshot.
Pallet::<T>::run_until_before_matches(|| {
Expand All @@ -61,19 +60,15 @@ mod benchmarks {
Pallet::<T>::roll_next(true, false);
}

assert_eq!(CurrentPhase::<T>::get(), Phase::Snapshot(T::Pages::get() - 1));
assert_eq!(
Snapshot::<T>::voters_decode_len(T::Pages::get() - 1).unwrap() as u32,
T::VoterSnapshotPerBlock::get(),
"{}",
SNAPSHOT_NOT_BIG_ENOUGH
);
// we have collected the target snapshot only
assert_eq!(CurrentPhase::<T>::get(), Phase::Snapshot(T::Pages::get()));
assert_eq!(
Snapshot::<T>::targets_decode_len().unwrap() as u32,
T::TargetSnapshotPerBlock::get(),
"{}",
SNAPSHOT_NOT_BIG_ENOUGH
);
assert_eq!(Snapshot::<T>::voters_decode_len(T::Pages::get() - 1), None);

Ok(())
}
Expand All @@ -86,17 +81,23 @@ mod benchmarks {
// roll to the first block of the snapshot.
Pallet::<T>::roll_until_matches(|| matches!(CurrentPhase::<T>::get(), Phase::Snapshot(_)));

assert_eq!(CurrentPhase::<T>::get(), Phase::Snapshot(T::Pages::get() - 1));
assert_eq!(CurrentPhase::<T>::get(), Phase::Snapshot(T::Pages::get()));
// we have collected the target snapshot only
assert_eq!(Snapshot::<T>::targets_decode_len().unwrap() as u32, T::TargetSnapshotPerBlock::get());
// and no voters yet.
assert_eq!(Snapshot::<T>::voters_decode_len(T::Pages::get() - 1), None);

// take one more snapshot page.
#[block]
{
Pallet::<T>::roll_next(true, false);
}

assert_eq!(CurrentPhase::<T>::get(), Phase::Snapshot(T::Pages::get() - 2));
// we have now collected the first page of voters.
assert_eq!(CurrentPhase::<T>::get(), Phase::Snapshot(T::Pages::get() - 1));
// it must be full
assert_eq!(
Snapshot::<T>::voters_decode_len(T::Pages::get() - 2).unwrap() as u32,
Snapshot::<T>::voters_decode_len(T::Pages::get() - 1).unwrap() as u32,
T::VoterSnapshotPerBlock::get(),
"{}",
SNAPSHOT_NOT_BIG_ENOUGH
Expand Down
80 changes: 46 additions & 34 deletions substrate/frame/election-provider-multi-block/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,8 @@ pub mod pallet {
let signed_validation_deadline =
T::SignedValidationPhase::get().saturating_add(unsigned_deadline);
let signed_deadline = T::SignedPhase::get().saturating_add(signed_validation_deadline);
let snapshot_deadline = signed_deadline.saturating_add(T::Pages::get().into());
// add one block to take the target snapshot a block ahead of time.
let snapshot_deadline = signed_deadline.saturating_add(T::Pages::get().into()).saturating_add(One::one());

let next_election = T::DataProvider::next_election_prediction(now)
.saturating_sub(T::Lookahead::get())
Expand All @@ -526,10 +527,8 @@ pub mod pallet {
match current_phase {
// start and continue snapshot.
Phase::Off if remaining_blocks <= snapshot_deadline => {
let remaining_pages = Self::msp();
let remaining_pages = Self::msp() + 1;
Self::create_targets_snapshot().defensive_unwrap_or_default();
Self::create_voters_snapshot_paged(remaining_pages)
.defensive_unwrap_or_default();
Self::phase_transition(Phase::Snapshot(remaining_pages));
T::WeightInfo::on_initialize_into_snapshot_msp()
},
Expand Down Expand Up @@ -819,9 +818,6 @@ pub mod pallet {
mut up_to_page: PageIndex,
) -> Result<(), &'static str> {
up_to_page = up_to_page.min(T::Pages::get());
// NOTE: if someday we split the snapshot taking of voters(msp) and targets into two
// different blocks, then this assertion becomes obsolete.
ensure!(up_to_page > 0, "can't check snapshot up to page 0");

// if any number of pages supposed to exist, these must also exist.
ensure!(exists ^ Self::desired_targets().is_none(), "desired target mismatch");
Expand Down Expand Up @@ -1392,17 +1388,19 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 0);

roll_to(13);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, 0));

roll_to(14);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(0));
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, 1));

roll_to(15);
assert_eq!(MultiBlock::current_phase(), Phase::Signed);
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(0) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed }
]
);
Expand All @@ -1419,7 +1417,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(0) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand All @@ -1439,7 +1437,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(0) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand Down Expand Up @@ -1468,9 +1466,12 @@ mod phase_rotation {
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(false, 1));
assert_eq!(MultiBlock::round(), 1);

roll_to(43);
roll_to(42);
assert_eq!(MultiBlock::current_phase(), Phase::Off);

roll_to(43);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));

roll_to(44);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(0));

Expand Down Expand Up @@ -1505,7 +1506,8 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 0);

roll_to(12);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, 0));

roll_to(13);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));
Expand All @@ -1520,7 +1522,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed }
]
);
Expand All @@ -1537,7 +1539,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand All @@ -1557,7 +1559,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand Down Expand Up @@ -1591,7 +1593,7 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 1);

roll_to(42);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));

roll_to(43);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));
Expand Down Expand Up @@ -1630,7 +1632,8 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 0);

roll_to(11);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(3));
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, 0));

roll_to(12);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
Expand All @@ -1649,7 +1652,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed }
]
);
Expand All @@ -1664,7 +1667,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand All @@ -1682,7 +1685,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand Down Expand Up @@ -1713,7 +1716,7 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 1);

roll_to(41);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(3));

roll_to(42);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
Expand Down Expand Up @@ -1756,7 +1759,9 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 0);

roll_to(9);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(3));
// ensure only target snapshot has been taken, and no voter snapshot.
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, 0));

roll_to(10);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
Expand All @@ -1775,7 +1780,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed }
]
);
Expand All @@ -1791,7 +1796,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand All @@ -1810,7 +1815,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand Down Expand Up @@ -1841,7 +1846,7 @@ mod phase_rotation {
assert_eq!(MultiBlock::round(), 1);

roll_to(41 - 2);
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(3));

roll_to(42 - 2);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
Expand Down Expand Up @@ -1883,10 +1888,15 @@ mod phase_rotation {
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::round(), 0);

roll_to(16);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(3));

roll_to(17);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));

roll_to(18);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));

roll_to(19);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(0));

Expand All @@ -1901,7 +1911,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand Down Expand Up @@ -1947,6 +1957,8 @@ mod phase_rotation {
assert_eq!(MultiBlock::current_phase(), Phase::Off);
assert_eq!(MultiBlock::round(), 0);

roll_to(21);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(3));
roll_to(22);
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
roll_to(23);
Expand All @@ -1962,7 +1974,7 @@ mod phase_rotation {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned {
from: Phase::Snapshot(0),
to: Phase::Unsigned(25)
Expand Down Expand Up @@ -2036,7 +2048,7 @@ mod election_provider {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned {
from: Phase::Signed,
Expand Down Expand Up @@ -2357,7 +2369,7 @@ mod election_provider {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::Export(2) },
Event::PhaseTransitioned { from: Phase::Export(1), to: Phase::Off }
Expand All @@ -2381,7 +2393,7 @@ mod election_provider {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::Off }
]
Expand Down Expand Up @@ -2439,7 +2451,7 @@ mod admin_ops {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::Emergency },
Event::PhaseTransitioned { from: Phase::Emergency, to: Phase::Off }
Expand Down Expand Up @@ -2484,7 +2496,7 @@ mod admin_ops {
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::Emergency },
Event::PhaseTransitioned { from: Phase::Emergency, to: Phase::Off }
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/election-provider-multi-block/src/mock/signed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ pub fn load_signed_for_verification_and_start(
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::SignedValidation(20) }
]
Expand All @@ -200,7 +200,7 @@ pub fn load_signed_for_verification_and_start_and_roll_to_verified(
assert_eq!(
multi_block_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
Event::PhaseTransitioned { from: Phase::Snapshot(0), to: Phase::Signed },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::SignedValidation(20) }
]
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/election-provider-multi-block/src/unsigned/miner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1929,7 +1929,7 @@ mod offchain_worker_miner {
assert_eq!(
multi_block_events(),
vec![
crate::Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
crate::Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
crate::Event::PhaseTransitioned {
from: Phase::Snapshot(0),
to: Phase::Unsigned(25)
Expand Down

0 comments on commit 2d0a3d7

Please sign in to comment.