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

Conversation

Szegoo
Copy link
Contributor

@Szegoo Szegoo commented Jan 4, 2023

Closes: paritytech/polkadot-sdk#432

Introduces three new extrinsics that can be used by force origin to transition between phases and rounds:

  • force_rotate_round
  • force_start_phase - Accepts a phase that we want to transition into as an argument. Only works when transitioning to the Signed or Emergency phase, otherwise fails. Stores the phase we want to transition into in the ForcePhase storage value. The transition will happen at the start of the next block.

Polkadot companion: paritytech/polkadot#6822

@stale
Copy link

stale bot commented Feb 4, 2023

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Feb 4, 2023
@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Feb 4, 2023
@Szegoo Szegoo marked this pull request as ready for review February 12, 2023 13:47
@Szegoo
Copy link
Contributor Author

Szegoo commented Feb 12, 2023

@Ank4n Could you please review this?

frame/election-provider-multi-phase/src/lib.rs Outdated Show resolved Hide resolved
frame/election-provider-multi-phase/src/lib.rs Outdated Show resolved Hide resolved
frame/election-provider-multi-phase/src/lib.rs Outdated Show resolved Hide resolved
@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Feb 14, 2023
@Ank4n Ank4n self-requested a review February 18, 2023 16:02
Self::do_force_rotate_round();
}

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.

@Szegoo Szegoo changed the title Allow ForceOrigin to force transition phases Allow root origin to force transition phases Mar 19, 2023
@Szegoo
Copy link
Contributor Author

Szegoo commented Mar 28, 2023

@Ank4n @kianenigma Could you please approve if the changes are ok?

@gpestana
Copy link
Contributor

@Szegoo we recently added an integration test crate for staking/EPM and other staking related pallets under frame/election-provider-multi-phase/test-staking-e2e and one of the main goals is to document the different edge cases and behaviour of the pallets as close to production as possible. I wonder if you could add a few e2e tests there for this PS and document them, I think that would be very useful for reviewers and also for posterity.

@Szegoo
Copy link
Contributor Author

Szegoo commented Apr 25, 2023

I wonder if you could add a few e2e tests there for this PS and document them, I think that would be very useful for reviewers and also for posterity.

That seems like a good idea, getting to it 👍

@Szegoo
Copy link
Contributor Author

Szegoo commented Apr 26, 2023

Integration tests are still WIP.

@Szegoo
Copy link
Contributor Author

Szegoo commented Apr 30, 2023

@gpestana I've added two integration tests and made up a scenario for both of them. Please let me know if this seems like something reasonable.

@Szegoo Szegoo requested a review from a team May 5, 2023 20:13
@Szegoo Szegoo requested a review from a team August 18, 2023 10:48
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-each-crate
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3412044

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
Status: ✂️ In progress.
Development

Successfully merging this pull request may close these issues.

Way to enable/disable a phase in EPM
5 participants