diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs index 04325a40d0ad..4f43f89abed2 100644 --- a/substrate/frame/election-provider-multi-phase/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/src/lib.rs @@ -274,14 +274,14 @@ pub mod migrations; pub mod signed; pub mod unsigned; pub mod weights; -use unsigned::VoterOf; -pub use weights::WeightInfo; pub use signed::{ BalanceOf, GeometricDepositBase, NegativeImbalanceOf, PositiveImbalanceOf, SignedSubmission, SignedSubmissionOf, SignedSubmissions, SubmissionIndicesOf, }; +use unsigned::VoterOf; pub use unsigned::{Miner, MinerConfig}; +pub use weights::WeightInfo; /// The solution type used by this crate. pub type SolutionOf = ::Solution; @@ -1275,6 +1275,7 @@ pub mod pallet { /// Snapshot data of the round. /// /// This is created at the beginning of the signed phase and cleared upon calling `elect`. + /// Note: This storage type must only be mutated through [`SnapshotWrapper`]. #[pallet::storage] #[pallet::getter(fn snapshot)] pub type Snapshot = StorageValue<_, RoundSnapshot>>; @@ -1282,6 +1283,7 @@ pub mod pallet { /// Desired number of targets to elect for this round. /// /// Only exists when [`Snapshot`] is present. + /// Note: This storage type must only be mutated through [`SnapshotWrapper`]. #[pallet::storage] #[pallet::getter(fn desired_targets)] pub type DesiredTargets = StorageValue<_, u32>; @@ -1289,6 +1291,7 @@ pub mod pallet { /// The metadata of the [`RoundSnapshot`] /// /// Only exists when [`Snapshot`] is present. + /// Note: This storage type must only be mutated through [`SnapshotWrapper`]. #[pallet::storage] #[pallet::getter(fn snapshot_metadata)] pub type SnapshotMetadata = StorageValue<_, SolutionOrSnapshotSize>; @@ -1351,6 +1354,39 @@ pub mod pallet { pub struct Pallet(_); } +/// This wrapper is created for handling the synchronization of [`Snapshot`], [`SnapshotMetadata`] +/// and [`DesiredTargets`] storage items. +pub struct SnapshotWrapper(sp_std::marker::PhantomData); + +impl SnapshotWrapper { + /// Kill all snapshot related storage items at the same time. + pub fn kill() { + >::kill(); + >::kill(); + >::kill(); + } + /// Set all snapshot related storage items at the same time. + pub fn set(metadata: SolutionOrSnapshotSize, desired_targets: u32, buffer: &[u8]) { + >::put(metadata); + >::put(desired_targets); + sp_io::storage::set(&>::hashed_key(), &buffer); + } + + /// Check if all of the storage items exist at the same time or all of the storage items do not + /// exist. + #[cfg(feature = "try-runtime")] + pub fn is_consistent() -> bool { + let snapshots = [ + >::exists(), + >::exists(), + >::exists(), + ]; + + // All should either exist or not exist + snapshots.iter().skip(1).all(|v| snapshots[0] == *v) + } +} + impl Pallet { /// Internal logic of the offchain worker, to be executed only when the offchain lock is /// acquired with success. @@ -1402,9 +1438,6 @@ impl Pallet { SolutionOrSnapshotSize { voters: voters.len() as u32, targets: targets.len() as u32 }; log!(info, "creating a snapshot with metadata {:?}", metadata); - >::put(metadata); - >::put(desired_targets); - // instead of using storage APIs, we do a manual encoding into a fixed-size buffer. // `encoded_size` encodes it without storing it anywhere, this should not cause any // allocation. @@ -1419,7 +1452,7 @@ impl Pallet { // buffer should have not re-allocated since. debug_assert!(buffer.len() == size && size == buffer.capacity()); - sp_io::storage::set(&>::hashed_key(), &buffer); + SnapshotWrapper::::set(metadata, desired_targets, &buffer); } /// Parts of [`create_snapshot`] that happen outside of this pallet. @@ -1500,13 +1533,6 @@ impl Pallet { ); } - /// Kill everything created by [`Pallet::create_snapshot`]. - pub fn kill_snapshot() { - >::kill(); - >::kill(); - >::kill(); - } - /// Checks the feasibility of a solution. pub fn feasibility_check( raw_solution: RawSolution>, @@ -1541,8 +1567,8 @@ impl Pallet { // Phase is off now. Self::phase_transition(Phase::Off); - // Kill snapshots. - Self::kill_snapshot(); + // Kill snapshot and relevant metadata (everything created by [`SnapshotMetadata::set`]). + SnapshotWrapper::::kill(); } fn do_elect() -> Result, ElectionError> { @@ -1613,15 +1639,7 @@ impl Pallet { // - [`DesiredTargets`] exists if and only if [`Snapshot`] is present. // - [`SnapshotMetadata`] exist if and only if [`Snapshot`] is present. fn try_state_snapshot() -> Result<(), TryRuntimeError> { - if >::exists() && - >::exists() && - >::exists() - { - Ok(()) - } else if !>::exists() && - !>::exists() && - !>::exists() - { + if SnapshotWrapper::::is_consistent() { Ok(()) } else { Err("If snapshot exists, metadata and desired targets should be set too. Otherwise, none should be set.".into()) @@ -1756,18 +1774,14 @@ mod feasibility_check { assert!(MultiPhase::current_phase().is_signed()); let solution = raw_solution(); - // For whatever reason it might be: - >::kill(); + // kill `Snapshot`, `SnapshotMetadata` and `DesiredTargets` for the storage state to + // be consistent, by using the `SnapshotWrapper` for the try_state checks to pass. + >::kill(); assert_noop!( MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::SnapshotUnavailable ); - - // kill also `SnapshotMetadata` and `DesiredTargets` for the storage state to be - // consistent for the try_state checks to pass. - >::kill(); - >::kill(); }) }