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

EPM and staking events improvement #13035

Merged
merged 9 commits into from
Jan 9, 2023

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Dec 29, 2022

This PR refactors some events in the EPM pallet and staking with the goal of making it easier to trace runtime events related to the election provider and staking pallets:

EPM pallet

  • SolutionStored { compute, prev_ejected, origin: Option<T::AccountId> } (modified).
  • PhaseTransitioned { from: Phase, to: Phase, round: u32}: All the phase transition events (Event::SignedPhaseStarted, Event::UnsighedPhaseStarted) are replaced by a state transition event that keep the previous and next phase. This will also allow us to easily start emitting when the emergency and phase off start and all new future phases that we may need.

Staking pallet:

  • ForceEra { mode: Forcing }: signals that a new Forcing mode was set.

In addition to refactoring and adding new events, this PR also adds helper functions to transition EPM phases and set a new force era mode.


Further discussion in #13026
Closes #13026

@gpestana gpestana 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 29, 2022
@gpestana gpestana requested a review from Ank4n December 29, 2022 22:49
@gpestana gpestana self-assigned this Dec 29, 2022
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
@gpestana gpestana added B3-apinoteworthy and removed B0-silent Changes should not be mentioned in any release notes labels Dec 30, 2022
@gpestana gpestana requested a review from Ank4n January 2, 2023 10:43
@gpestana gpestana added the J2-unconfirmed Issue might be valid, but it’s not yet known. label Jan 2, 2023
@@ -1130,10 +1139,8 @@ pub mod pallet {
Rewarded { account: <T as frame_system::Config>::AccountId, value: BalanceOf<T> },
/// An account has been slashed for submitting an invalid signed submission.
Slashed { account: <T as frame_system::Config>::AccountId, value: BalanceOf<T> },
/// The signed phase of the given round has started.
SignedPhaseStarted { round: u32 },
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes might break some UIs depending on these events. May be we should label it as breaking changes (E5?, B7 ) with some release notes in description.

@kianenigma might have better insights.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think any UI reads these events, so it should be okay.

Nonetheless, I would love to see Ankan's suggestions in PRs that actually do break UIs that we care about.

The broader meta-discussion here is what degree of release-note do we want to have for breaking frame changes.

I think there is a LOT LOT more that we can do here, but the current labels are barely enough to reflect changes that are relevant to parachain teams and polkadot, and for this audience I don't think this matters.

Copy link
Contributor

Choose a reason for hiding this comment

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

hi,
currently the only thing you can do to highlight a code change on the release notes is elevating its upgrade criticality with the C* labels. This will change though, we're slowly implementing new labels on polkadot/substrate/cumulus that have a clear description and in future the C* labels should be used in combination with other labels so that you can distinguish what it is about upfront. Here is the documentation of the new labels explaining how they should be used: https://github.com/paritytech/labels/blob/main/docs/doc_cumulus.md

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Good stuff, but there are a few minor issues.

@gpestana gpestana removed the J2-unconfirmed Issue might be valid, but it’s not yet known. label Jan 3, 2023
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

LGTM!

@kianenigma kianenigma added B0-silent Changes should not be mentioned in any release notes and removed B3-apinoteworthy labels Jan 4, 2023
@gpestana
Copy link
Contributor Author

gpestana commented Jan 9, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@gpestana
Copy link
Contributor Author

gpestana commented Jan 9, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 6c7e941 into master Jan 9, 2023
@paritytech-processbot paritytech-processbot bot deleted the gpestana/13026-epm-staking-events branch January 9, 2023 16:06
rossbulat pushed a commit that referenced this pull request Jan 11, 2023
* EPM and staking events improvement

* Uses RawOrigin in ElectionCompute event

* Refactors new phase events to PhaseTransition event

* PhaseTransitioned and remove RawOrigin from event

* Adds helpers for epm phase transition and staking force new

* addresses review comments

* nit: removes unecessary clone

* fixes benchmarks

Co-authored-by: parity-processbot <>
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* EPM and staking events improvement

* Uses RawOrigin in ElectionCompute event

* Refactors new phase events to PhaseTransition event

* PhaseTransitioned and remove RawOrigin from event

* Adds helpers for epm phase transition and staking force new

* addresses review comments

* nit: removes unecessary clone

* fixes benchmarks

Co-authored-by: parity-processbot <>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* EPM and staking events improvement

* Uses RawOrigin in ElectionCompute event

* Refactors new phase events to PhaseTransition event

* PhaseTransitioned and remove RawOrigin from event

* Adds helpers for epm phase transition and staking force new

* addresses review comments

* nit: removes unecessary clone

* fixes benchmarks

Co-authored-by: parity-processbot <>
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

EPM and staking events improvement
4 participants