Implement wrapping of EPM types #1633
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wirednkod
added
the
T1-FRAME
wirednkod
requested review from
gpestana,
kianenigma,
lexnv and
sergejparity
lexnv
approved these changes
Sep 19, 2023
gpestana
requested changes
Sep 19, 2023
Can you elaborate? The ones in |
…wrap-epm-types-with-snapshotwrapper
…paritytech/polkadot-sdk into nik-wrap-epm-types-with-snapshotwrapper
gpestana
requested changes
Sep 22, 2023
gpestana
approved these changes
Oct 11, 2023
There was a problem hiding this comment.
A few comment nits, if the tests pass it looks great to me.
One more suggestion, could you please a few words on the Snapshot, etc storage types definitions and say that we should use the SnapshotWrapper to mutate these types? thanks! (e.g. /// Note: This storage type must only be mutates thorough[SnapshotWrapper].
…s must only be mutated throught the SnapshotWrapper
gpestana
reviewed
Oct 25, 2023
…paritytech/polkadot-sdk into nik-wrap-epm-types-with-snapshotwrapper
juangirini
approved these changes
Oct 25, 2023
Ank4n
approved these changes
Oct 25, 2023
liamaharon
approved these changes
Oct 26, 2023
Comment on lines
1381
to
1388
| pub fn is_consistent() -> bool { | ||
| (<Snapshot<T>>::exists() && | ||
| <SnapshotMetadata<T>>::exists() && | ||
| <DesiredTargets<T>>::exists()) || | ||
| (!<Snapshot<T>>::exists() && | ||
| !<SnapshotMetadata<T>>::exists() && | ||
| !<DesiredTargets<T>>::exists()) | ||
| } |
There was a problem hiding this comment.
maybe can do something like this for improved readability
Suggested change
| pub fn is_consistent() -> bool { | |
| (<Snapshot<T>>::exists() && | |
| <SnapshotMetadata<T>>::exists() && | |
| <DesiredTargets<T>>::exists()) || | |
| (!<Snapshot<T>>::exists() && | |
| !<SnapshotMetadata<T>>::exists() && | |
| !<DesiredTargets<T>>::exists()) | |
| } | |
| pub fn is_consistent() -> bool { | |
| let snapshots = [ | |
| <Snapshot<T>>::exists(), | |
| <SnapshotMetadata<T>>::exists(), | |
| <DesiredTargets<T>>::exists(), | |
| ]; | |
| // All should either exist or not exist | |
| snapshots.iter().all(|&exists| exists) || snapshots.iter().all(|&exists| !exists) | |
| } |
There was a problem hiding this comment.
snapshots.iter().skip(1).all(|v| snapshots[0] == v) has the same effect.
kianenigma
reviewed
Nov 6, 2023
kianenigma
approved these changes
Nov 6, 2023
…paritytech/polkadot-sdk into nik-wrap-epm-types-with-snapshotwrapper
|
The CI pipeline was cancelled due to failure one of the required jobs. |
bgallois
pushed a commit
to duniter/duniter-polkadot-sdk
that referenced
this pull request
Mar 25, 2024
This PR wraps the `Snapshot`, `SnapshotMetadata` and `DesiredTargets` storage items in the [EPM pallet](https://paritytech.github.io/substrate/master/pallet_election_provider_multi_phase/index.html) in order to keep them in sync throughout the election lifetime and in order to be killed together. Prior to this PR, these storage items were mutated in different places in the pallet; In addition 2 helper `fns` are introduced for chekcing if all the wrapped storage items exist or not; Fixes paritytech#413 ; --------- Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR wraps the
Snapshot,SnapshotMetadataandDesiredTargetsstorage items in the EPM pallet in order to keep them in sync throughout the election lifetime and in order to be killed together.Prior to this PR, these storage items were mutated in different places in the pallet;
In addition 2 helper
fnsare introduced for chekcing if all the wrapped storage items exist or not;Fixes #413 ;