Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow root origin to force transition phases #13070

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
f233e7b
force_transition_phase
Szegoo Jan 4, 2023
974f6bd
separate into different extrinsics
Szegoo Jan 5, 2023
b8cc054
Merge branch 'paritytech:master' into force-transition
Szegoo Feb 4, 2023
91b7a56
update
Szegoo Feb 7, 2023
e30df1d
tests & fix
Szegoo Feb 8, 2023
c0a6ed6
remove nonsense
Szegoo Feb 8, 2023
d128084
fix logic
Szegoo Feb 9, 2023
9f9c639
calculate weights
Szegoo Feb 12, 2023
1e0a673
require root origin
Szegoo Feb 13, 2023
59abf6c
make extrinsics operational
Szegoo Feb 15, 2023
1fab484
better solution
Szegoo Feb 17, 2023
6464caa
fmt
Szegoo Feb 17, 2023
fcbe75d
refactor
Szegoo Mar 1, 2023
9b2e7f2
add comment
Szegoo Mar 1, 2023
00b0d23
correct weight
Szegoo Mar 1, 2023
fa4a22b
clean up
Szegoo Mar 4, 2023
f5a0979
Merge branch 'master' into force-transition
Szegoo Mar 17, 2023
6c2660e
disallow force transition from signed & unsigned
Szegoo Mar 19, 2023
1931103
allow transition to signed from unsigned
Szegoo Mar 19, 2023
b1bc6c5
remove unused code
Szegoo Mar 19, 2023
d3bb8df
Merge branch 'paritytech:master' into force-transition
Szegoo Mar 19, 2023
944c5e2
Update frame/election-provider-multi-phase/src/lib.rs
Szegoo Mar 29, 2023
75c50c4
fixes
Szegoo Mar 29, 2023
5cd9fd1
Merge branch 'paritytech:master' into force-transition
Szegoo Mar 29, 2023
465d885
remove unused argument
Szegoo Mar 29, 2023
acca1fc
remove old code
Szegoo Apr 15, 2023
440072c
Merge branch 'paritytech:master' into force-transition
Szegoo Apr 15, 2023
f8d3b63
Merge branch 'paritytech:master' into force-transition
Szegoo Apr 25, 2023
8da9aa6
integration tests
Szegoo Apr 26, 2023
c3a6d55
improve integration test
Szegoo Apr 30, 2023
90eed95
docs
Szegoo Apr 30, 2023
f3cba4b
merge
Szegoo May 5, 2023
bef191c
merge fix
Szegoo May 5, 2023
6123d80
Merge branch 'master' into force-transition
Szegoo May 21, 2023
1bd8c84
merge fixes
Szegoo May 21, 2023
f08a775
Merge branch 'master' into force-transition
Szegoo Aug 18, 2023
c87549c
merge fix
Szegoo Aug 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions frame/election-provider-multi-phase/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,20 @@ frame_benchmarking::benchmarks! {
assert!(<MultiPhase<T>>::queued_solution().is_some());
}

force_rotate_round {

}: _(RawOrigin::Root, Phase::Off)
verify {
assert_eq!(<MultiPhase<T>>::round(), 2);
}

force_rotate_round_from_signed {
<CurrentPhase<T>>::put(Phase::Signed);
}: force_rotate_round(RawOrigin::Root, Phase::Signed)
verify {
assert_eq!(<MultiPhase<T>>::round(), 2);
}

// This is checking a valid solution. The worse case is indeed a valid solution.
feasibility_check {
// number of votes in snapshot.
Expand Down
201 changes: 198 additions & 3 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,28 @@ pub mod pallet {
#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_initialize(now: T::BlockNumber) -> Weight {
if Self::force_phase().is_some() {
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
match Self::force_phase() {
Some(Phase::Signed) => {
// If the current phase is not `Phase::Off` proceed to the next round.
if !Self::current_phase().is_off() {
Self::do_force_rotate_round();
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
}

match Self::create_snapshot() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we reuse code either by

  • extracting in a separate fn start_signed()
  • adding the check for force phase inside the existing arm
						match current_phase {
				Phase::Off if force_phase == Phase::Signed || remaining <= signed_deadline && remaining > unsigned_deadline => {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good reason to extract logic into functions start_<some>_phase is that we can potentially add it to trait ElectionProvider such that other pallets can tell EPM to force phase as well.

Ok(_) => Self::phase_transition(Phase::Signed),
Err(why) => {
log!(warn, "failed to open signed phase due to {:?}", why);
return T::WeightInfo::on_initialize_nothing()
},
}
<ForcePhase<T>>::kill();
return T::WeightInfo::on_initialize_open_signed()
},
_ => {},
}
}

let next_election = T::DataProvider::next_election_prediction(now).max(now);

let signed_deadline = T::SignedPhase::get() + T::UnsignedPhase::get();
Expand Down Expand Up @@ -880,7 +902,7 @@ pub mod pallet {
impl<T: Config> Pallet<T> {
/// Submit a solution for the unsigned phase.
///
/// The dispatch origin fo this call must be __none__.
/// The dispatch origin for this call must be __none__.
///
/// This submission is checked on the fly. Moreover, this unsigned solution is only
/// validated when submitted to the pool from the **local** node. Effectively, this means
Expand Down Expand Up @@ -994,7 +1016,7 @@ pub mod pallet {

/// Submit a solution for the signed phase.
///
/// The dispatch origin fo this call must be __signed__.
/// The dispatch origin for this call must be __signed__.
///
/// The solution is potentially queued, based on the claimed score and processed at the end
/// of the signed phase.
Expand Down Expand Up @@ -1112,6 +1134,57 @@ pub mod pallet {
<QueuedSolution<T>>::put(solution);
Ok(())
}

/// Rotates the round which results in setting the current phase to
/// `Phase::Off` and removes all the current solutions.
///
/// The dispatch origin for this call must be `ForceOrigin`.
#[pallet::call_index(5)]
#[pallet::weight(({
if current_phase.is_signed() {
T::WeightInfo::force_rotate_round_from_signed()
}else {
T::WeightInfo::force_rotate_round()
}
}, DispatchClass::Operational))]
pub fn force_rotate_round(
origin: OriginFor<T>,
current_phase: Phase<T::BlockNumber>,
) -> DispatchResult {
T::ForceOrigin::ensure_origin(origin)?;
ensure!(current_phase == Self::current_phase(), <Error<T>>::CallNotAllowed);
Szegoo marked this conversation as resolved.
Show resolved Hide resolved

Self::do_force_rotate_round();

Ok(())
}

/// Initializes an open signed phase. If the current phase is not
/// `Phase::Off` proceeds to the next round.
///
/// The dispatch origin for this call must be root.
#[pallet::call_index(6)]
#[pallet::weight((T::DbWeight::get().writes(1), DispatchClass::Operational))]
pub fn force_start_signed_phase(origin: OriginFor<T>) -> DispatchResult {
ensure_root(origin)?;

<ForcePhase<T>>::put(Phase::Signed);
Ok(())
}

/// Sets the current phase to `Phase::Emergency` and removes the current
/// queued solution.
///
/// The dispatch origin for this call must be `ForceOrigin`.
#[pallet::call_index(8)]
#[pallet::weight((T::DbWeight::get().writes(2), DispatchClass::Operational))]
pub fn force_start_emergency_phase(origin: OriginFor<T>) -> DispatchResult {
T::ForceOrigin::ensure_origin(origin)?;

<QueuedSolution<T>>::kill();
<CurrentPhase<T>>::put(Phase::Emergency);
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
}
}

#[pallet::event]
Expand Down Expand Up @@ -1174,6 +1247,8 @@ pub mod pallet {
BoundNotMet,
/// Submitted solution has too many winners
TooManyWinners,
/// Failed to create a snapshot.
SnapshotCreationFailed,
}

#[pallet::validate_unsigned]
Expand Down Expand Up @@ -1317,6 +1392,11 @@ pub mod pallet {
#[pallet::getter(fn minimum_untrusted_score)]
pub type MinimumUntrustedScore<T: Config> = StorageValue<_, ElectionScore>;

/// Stores the phase we want to force during `on_initialize`.
#[pallet::storage]
#[pallet::getter(fn force_phase)]
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
pub type ForcePhase<T: Config> = StorageValue<_, Phase<T::BlockNumber>, OptionQuery>;

/// The current storage version.
///
/// v1: https://github.com/paritytech/substrate/pull/12237/
Expand Down Expand Up @@ -1356,6 +1436,16 @@ impl<T: Config> Pallet<T> {
}
}

/// Remove the current solutions and proceed to the following round.
fn do_force_rotate_round() {
if Self::current_phase().is_signed() {
Self::finalize_signed_phase();
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
}

<QueuedSolution<T>>::kill();
Self::rotate_round();
}

/// Phase transition helper.
pub(crate) fn phase_transition(to: Phase<T::BlockNumber>) {
log!(info, "Starting phase {:?}, round {}.", to, Self::round());
Expand Down Expand Up @@ -1894,7 +1984,7 @@ mod tests {
assert_eq!(MultiPhase::current_phase(), Phase::Off);
assert_eq!(MultiPhase::round(), 1);

roll_to(4);
roll_to(14);
assert_eq!(MultiPhase::current_phase(), Phase::Off);
assert!(MultiPhase::snapshot().is_none());
assert_eq!(MultiPhase::round(), 1);
Expand Down Expand Up @@ -1989,6 +2079,111 @@ mod tests {
})
}

#[test]
fn force_rotate_round_works() {
ExtBuilder::default().build_and_execute(|| {
roll_to(1);
assert_eq!(System::block_number(), 1);

assert_eq!(MultiPhase::current_phase(), Phase::Off);
assert_eq!(MultiPhase::round(), 1);

assert_noop!(
MultiPhase::force_rotate_round(crate::mock::RuntimeOrigin::none(), Phase::Off),
DispatchError::BadOrigin
);
assert_noop!(
MultiPhase::force_rotate_round(crate::mock::RuntimeOrigin::signed(1), Phase::Off),
DispatchError::BadOrigin
);

// The provided current phase is incorrect.
assert_noop!(
MultiPhase::force_rotate_round(crate::mock::RuntimeOrigin::root(), Phase::Signed),
Error::<Runtime>::CallNotAllowed
);

assert_ok!(MultiPhase::force_rotate_round(
crate::mock::RuntimeOrigin::root(),
Phase::Off
));
assert_eq!(MultiPhase::round(), 2);

assert_eq!(
multi_phase_events(),
vec![Event::PhaseTransitioned { from: Phase::Off, to: Phase::Off, round: 2 },]
);
})
}

#[test]
fn force_start_signed_phase_works() {
ExtBuilder::default().build_and_execute(|| {
roll_to(1);
assert_eq!(System::block_number(), 1);

assert_eq!(MultiPhase::current_phase(), Phase::Off);
assert_eq!(MultiPhase::round(), 1);

assert_noop!(
MultiPhase::force_start_signed_phase(crate::mock::RuntimeOrigin::none(),),
DispatchError::BadOrigin
);
assert_noop!(
MultiPhase::force_start_signed_phase(crate::mock::RuntimeOrigin::signed(1),),
DispatchError::BadOrigin
);

assert_ok!(MultiPhase::force_start_signed_phase(crate::mock::RuntimeOrigin::root(),));
assert_eq!(MultiPhase::force_phase(), Some(Phase::Signed));

roll_to(2);
assert!(MultiPhase::current_phase().is_signed());
assert!(MultiPhase::snapshot().is_some());
assert!(MultiPhase::force_phase().is_none());
// Didn't proceed to following round since the previos phase was `Phase::Off`.
assert_eq!(MultiPhase::round(), 1);

assert_ok!(MultiPhase::force_start_signed_phase(crate::mock::RuntimeOrigin::root()));
assert_eq!(MultiPhase::force_phase(), Some(Phase::Signed));

roll_to(3);
assert!(MultiPhase::current_phase().is_signed());
assert!(MultiPhase::snapshot().is_some());
// Proceeded to the following roung because the previous phase was signed.
assert_eq!(MultiPhase::round(), 2);
assert_eq!(
multi_phase_events(),
vec![
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Signed, round: 1 },
Event::PhaseTransitioned { from: Phase::Signed, to: Phase::Off, round: 2 },
Event::PhaseTransitioned { from: Phase::Off, to: Phase::Signed, round: 2 },
]
);
})
}

#[test]
fn force_start_emergency_works() {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(System::block_number(), 0);
assert_eq!(MultiPhase::current_phase(), Phase::Off);
assert_eq!(MultiPhase::round(), 1);

assert_noop!(
MultiPhase::force_start_emergency_phase(crate::mock::RuntimeOrigin::none()),
DispatchError::BadOrigin
);
assert_noop!(
MultiPhase::force_start_emergency_phase(crate::mock::RuntimeOrigin::signed(1)),
DispatchError::BadOrigin
);

assert_ok!(MultiPhase::force_start_emergency_phase(crate::mock::RuntimeOrigin::root()));
assert!(MultiPhase::current_phase().is_emergency());
})
}

#[test]
fn signed_phase_void() {
ExtBuilder::default().phases(0, 10).build_and_execute(|| {
Expand Down
Loading