Skip to content

Commit b6d96c4

Browse files
committed
Add error events for failed target and voter snapshots
The commit adds new events to track when snapshot creation fails. This helps with monitoring and debugging failures in target/voter snapshot operations.
1 parent 63ac3b8 commit b6d96c4

File tree

3 files changed

+125
-9
lines changed

3 files changed

+125
-9
lines changed

prdoc/pr_8687.prdoc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ title: 'Staking (EPMB): Add defensive error handling to voter snapshot creation
33
doc:
44
- audience: Runtime Dev
55
description: |-
6-
Replace unwrap() with defensive_unwrap_or_default() in the on_initialize path of the
6+
- Replace unwrap() with defensive_unwrap_or_default() in the on_initialize path of the
77
election-provider-multiblock pallet to gracefully handle runtime misconfigurations where voter
88
snapshot conversion fails, preventing potential panics during block initialization.
9-
10-
Replace unwrap() with defensive_unwrap_or(u32::MAX) to ensure solution fails verification
9+
- Replace unwrap() with defensive_unwrap_or(u32::MAX) to ensure solution fails verification
1110
gracefully when desired_targets is unavailable rather than panicking.
11+
- Add error events for failed target and voter snapshots
1212
crates:
1313
- name: pallet-election-provider-multi-block
14-
bump: minor
14+
bump: major

substrate/frame/election-provider-multi-block/src/lib.rs

Lines changed: 115 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -660,12 +660,20 @@ pub mod pallet {
660660
let weight1 = match current_phase {
661661
Phase::Snapshot(x) if x == T::Pages::get() => {
662662
// create the target snapshot
663-
Self::create_targets_snapshot().defensive_unwrap_or_default();
663+
let result = Self::create_targets_snapshot();
664+
if result.is_err() {
665+
Self::deposit_event(Event::TargetSnapshotFailed);
666+
}
667+
result.defensive_unwrap_or_default();
664668
T::WeightInfo::on_initialize_into_snapshot_msp()
665669
},
666670
Phase::Snapshot(x) => {
667671
// create voter snapshot
668-
Self::create_voters_snapshot_paged(x).defensive_unwrap_or_default();
672+
let result = Self::create_voters_snapshot_paged(x);
673+
if result.is_err() {
674+
Self::deposit_event(Event::VoterSnapshotFailed);
675+
}
676+
result.defensive_unwrap_or_default();
669677
T::WeightInfo::on_initialize_into_snapshot_rest()
670678
},
671679
_ => T::WeightInfo::on_initialize_nothing(),
@@ -778,6 +786,10 @@ pub mod pallet {
778786
/// The target phase
779787
to: Phase<T>,
780788
},
789+
/// Target snapshot creation failed
790+
TargetSnapshotFailed,
791+
/// Voter snapshot creation failed
792+
VoterSnapshotFailed,
781793
}
782794

783795
/// Error of the pallet that can be returned in response to dispatches.
@@ -1635,8 +1647,9 @@ mod phase_rotation {
16351647
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, 1));
16361648
assert_eq!(MultiBlock::round(), 0);
16371649

1650+
let events = multi_block_events_since_last_call();
16381651
assert_eq!(
1639-
multi_block_events_since_last_call(),
1652+
events,
16401653
vec![
16411654
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(1) },
16421655
Event::PhaseTransitioned {
@@ -1645,6 +1658,9 @@ mod phase_rotation {
16451658
}
16461659
]
16471660
);
1661+
// Verify no snapshot failure events during successful operation
1662+
assert!(!events.contains(&Event::TargetSnapshotFailed));
1663+
assert!(!events.contains(&Event::VoterSnapshotFailed));
16481664

16491665
roll_to(19);
16501666
assert_eq!(MultiBlock::current_phase(), Phase::Signed(0));
@@ -1749,8 +1765,9 @@ mod phase_rotation {
17491765
assert_eq!(MultiBlock::round(), 0);
17501766
assert_eq!(MultiBlock::current_phase(), Phase::Signed(SignedPhase::get() - 1));
17511767

1768+
let events = multi_block_events_since_last_call();
17521769
assert_eq!(
1753-
multi_block_events_since_last_call(),
1770+
events,
17541771
vec![
17551772
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(2) },
17561773
Event::PhaseTransitioned {
@@ -1759,6 +1776,9 @@ mod phase_rotation {
17591776
}
17601777
]
17611778
);
1779+
// Verify no snapshot failure events during successful operation
1780+
assert!(!events.contains(&Event::TargetSnapshotFailed));
1781+
assert!(!events.contains(&Event::VoterSnapshotFailed));
17621782

17631783
roll_to(19);
17641784
assert_eq!(MultiBlock::current_phase(), Phase::Signed(0));
@@ -1860,8 +1880,9 @@ mod phase_rotation {
18601880
roll_to(15);
18611881
assert_ok!(Snapshot::<Runtime>::ensure_snapshot(true, Pages::get()));
18621882
assert_eq!(MultiBlock::current_phase(), Phase::Signed(4));
1883+
let events = multi_block_events_since_last_call();
18631884
assert_eq!(
1864-
multi_block_events_since_last_call(),
1885+
events,
18651886
vec![
18661887
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Snapshot(3) },
18671888
Event::PhaseTransitioned {
@@ -1870,6 +1891,9 @@ mod phase_rotation {
18701891
}
18711892
]
18721893
);
1894+
// Verify no snapshot failure events during successful operation
1895+
assert!(!events.contains(&Event::TargetSnapshotFailed));
1896+
assert!(!events.contains(&Event::VoterSnapshotFailed));
18731897
assert_eq!(MultiBlock::round(), 0);
18741898

18751899
roll_to(19);
@@ -2228,6 +2252,92 @@ mod phase_rotation {
22282252
assert_eq!(MultiBlock::current_phase(), Phase::Export(0));
22292253
});
22302254
}
2255+
2256+
#[test]
2257+
#[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))]
2258+
fn target_snapshot_failed_event_emitted() {
2259+
ExtBuilder::full()
2260+
.pages(2)
2261+
.election_start(13)
2262+
.build_and_execute(|| {
2263+
// Create way more targets than the TargetSnapshotPerBlock limit (4)
2264+
// This will cause bounds.slice_exhausted(&targets) to return true
2265+
let too_many_targets: Vec<AccountId> = (1..=100).collect();
2266+
Targets::set(too_many_targets);
2267+
2268+
roll_to(13);
2269+
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(2));
2270+
2271+
// Clear any existing events
2272+
let _ = multi_block_events_since_last_call();
2273+
2274+
// Roll to next block - on_initialize will be in Phase::Snapshot(2) where x == T::Pages::get()
2275+
// This triggers target snapshot creation, which should fail due to too many targets
2276+
roll_to(14);
2277+
2278+
// Verify that TargetSnapshotFailed event was emitted
2279+
let events = multi_block_events_since_last_call();
2280+
assert!(
2281+
events.contains(&Event::TargetSnapshotFailed),
2282+
"TargetSnapshotFailed event should have been emitted when target snapshot creation fails. Events: {:?}",
2283+
events
2284+
);
2285+
2286+
// Verify phase transition still happened despite the failure
2287+
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));
2288+
});
2289+
}
2290+
2291+
#[test]
2292+
#[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))]
2293+
fn voter_snapshot_failed_event_emitted() {
2294+
ExtBuilder::full()
2295+
.pages(2)
2296+
.election_start(13)
2297+
.build_and_execute(|| {
2298+
// Enable voter snapshot failure for this test
2299+
FailVotersSnapshot::set(true);
2300+
2301+
// Set normal targets (no failure for target snapshot)
2302+
Targets::set(vec![1, 2, 3, 4]);
2303+
2304+
// Create way more voters than the VoterSnapshotPerBlock limit (4)
2305+
// This will cause bounds.slice_exhausted(&voters) to return true in the mock
2306+
let too_many_voters: Vec<(AccountId, VoteWeight, BoundedVec<AccountId, MaxVotesPerVoter>)> =
2307+
(1..=100)
2308+
.map(|i| (i as AccountId, 100, vec![1].try_into().unwrap()))
2309+
.collect();
2310+
Voters::set(too_many_voters);
2311+
2312+
// Start election and move past target snapshot creation
2313+
roll_to(13); // Phase::Snapshot(2)
2314+
roll_to(14); // Target snapshot created, now Phase::Snapshot(1)
2315+
2316+
// We should now be in Phase::Snapshot(1) where voter snapshot will be created
2317+
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(1));
2318+
2319+
// Clear any existing events
2320+
let _ = multi_block_events_since_last_call();
2321+
2322+
// Roll to next block - on_initialize will be in Phase::Snapshot(1) where x < T::Pages::get()
2323+
// This triggers voter snapshot creation for page 1, which should fail due to too many voters
2324+
roll_to(15);
2325+
2326+
// Verify that VoterSnapshotFailed event was emitted
2327+
let events = multi_block_events_since_last_call();
2328+
assert!(
2329+
events.contains(&Event::VoterSnapshotFailed),
2330+
"VoterSnapshotFailed event should have been emitted when voter snapshot creation fails. Events: {:?}",
2331+
events
2332+
);
2333+
2334+
// Verify phase transition still happened despite the failure
2335+
assert_eq!(MultiBlock::current_phase(), Phase::Snapshot(0));
2336+
2337+
// Clean up: disable the failure flag
2338+
FailVotersSnapshot::set(false);
2339+
});
2340+
}
22312341
}
22322342

22332343
#[cfg(test)]

substrate/frame/election-provider-multi-block/src/mock/staking.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ frame_support::parameter_types! {
4949
pub static EpochLength: u64 = 30;
5050

5151
pub static LastIteratedVoterIndex: Option<usize> = None;
52+
pub static FailVotersSnapshot: bool = false;
5253
}
5354

5455
pub struct MockStaking;
@@ -84,6 +85,11 @@ impl ElectionDataProvider for MockStaking {
8485
> {
8586
let mut voters = Voters::get();
8687

88+
// Add conditional bounds check for testing voter snapshot failures
89+
if FailVotersSnapshot::get() && bounds.slice_exhausted(&voters) {
90+
return Err("Voters too big")
91+
}
92+
8793
// jump to the first non-iterated, if this is a follow up.
8894
if let Some(index) = LastIteratedVoterIndex::get() {
8995
voters = voters.iter().skip(index).cloned().collect::<Vec<_>>();

0 commit comments

Comments
 (0)