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

Implements try_state hook in elections and EPM pallets #13718

Closed
wants to merge 61 commits into from

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Mar 27, 2023

This PR implements the try_state hook in elections and EPM pallets and uses it in the tests. An improvement to this PR would be to work on paritytech/polkadot-sdk#210 to automatically include calling the try_state hook in all tests.

Related to paritytech/polkadot-sdk#239

@gpestana gpestana requested a review from a team March 27, 2023 09:11
@gpestana gpestana self-assigned this Mar 27, 2023
@gpestana gpestana marked this pull request as draft March 27, 2023 09:12
@gpestana gpestana changed the title (wip) Implements try_state hook in elections pallet (wip) Implements try_state hook in elections and EPM pallets Mar 28, 2023
@gpestana gpestana added A0-please_review Pull request needs code review. I5-tests Tests need fixing, improving or augmenting. 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. T1-runtime This PR/Issue is related to the topic “runtime”. labels Mar 30, 2023
@gpestana gpestana marked this pull request as ready for review March 30, 2023 16:10
@gpestana gpestana added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Mar 30, 2023
@gpestana gpestana changed the title (wip) Implements try_state hook in elections and EPM pallets Implements try_state hook in elections and EPM pallets Mar 30, 2023
frame/election-provider-multi-phase/src/lib.rs Outdated Show resolved Hide resolved
Comment on lines +1588 to +1590
// [`Snapshot`] state check. Invariants:
// - [`DesiredTargets`] exist IFF [`Snapshot`] is present.
// - [`SnapshotMetadata`] exist IFF [`Snapshot`] is present.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be more thorough to check this: if Snapshot or DesiredTargets or SnapshotMetadata exist, then they must all exist?

Otherwise this check would pass if we're in a weird state like only DesiredTargets is set but Snapshot and SnapshotMetadata are not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one more check to ensure DesiredTargets and SnapshotMetadata are not set if Snapshot is not set.


// [`SignedSubmissionsMap`] state check. Invariants:
// - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more;
// - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`];
Copy link
Contributor

@liamaharon liamaharon Apr 12, 2023

Choose a reason for hiding this comment

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

I wonder if we can be more specific with this invariant. Should SignedSubmissionNextIndex be equal to the size of SignedSubmissionsMap?

Copy link
Contributor Author

@gpestana gpestana Apr 23, 2023

Choose a reason for hiding this comment

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

Should SignedSubmissionNextIndex be equal to the size of SignedSubmissionsMap?

Not necessarily, that is the reason why we keep the index in storage (also to avoid iterating over the map).

frame/election-provider-multi-phase/src/lib.rs Outdated Show resolved Hide resolved
frame/elections-phragmen/src/lib.rs Outdated Show resolved Hide resolved
frame/elections-phragmen/src/lib.rs Outdated Show resolved Hide resolved
gpestana and others added 13 commits April 20, 2023 16:24
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
* chore(sc-cli): improve runner and signals

* Update client/cli/src/runner.rs

* fmt

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
* Fix the Referenda confirming alarm

* Add minimal regression test

This fails on bf395c8 since the
downwards rounding voids the curve delay.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* reward pool migration fix

* comment

* remove generic

* rm max

* foramtting

* Add note to V4 migration

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add more asserts to the V5 migration

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make compile

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/nomination-pools/src/migration.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* Make V4 migration re-usable

Otherwise it wont chain together with the V5.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add MigrateV3ToV5 wrapper

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
* Change node-template license from Unlicense to MIT-0

* Change frame examples license from Unlicense to MIT-0

* Update bin/node-template/LICENSE

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
* updating labels descriptions

* delete milestones

* Update docs/CONTRIBUTING.adoc

Co-authored-by: Bastian Köcher <git@kchr.de>

* link to label docs

* Update docs/CONTRIBUTING.adoc

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: parity-processbot <>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
…orker (#13730)

* Remove `HeaderBackend` requirement from `NetworkWorker`

* Remove HeaderBackend from authority-discovery
muharem and others added 18 commits April 20, 2023 17:27
Co-authored-by: parity-processbot <>
* refactor: inconsistent BalanceConversion fn

* Revert "refactor: inconsistent BalanceConversion fn"

This reverts commit 1177877.

* refactor: rename BalanceConversion trait

* feat: add ConversionFromAssetBalance
This removes the deprecated batch verification. This was actually never really activated.
Nevertheless, we need to keep the host functions around to support old runtimes which may import
these host functions. However, we do not give access to these functions anymore. This means that any new
runtime can not call them anymore. The host function implementations we keep will not do batch verification and will
instead fall back to the always existing option of directly verifying the passed signature.
`finish_batch_verification` will return the combined result of all the batch verify calls.

This removes the `TaskExecutorExt` which only existed to support the batch verification. So, any
code that used this extension can just remove the registration of them. It also removes
`SignatureBatching` that was used by `frame-executive` to control the batch verification.
However, there wasn't any `Verify` implementation that called the batch verification functions.
* Use proc-macro-warning crate

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fixup

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix pallet_ui tests

Also renamed some of the odd-named ones.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update dep

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Ignore hardcoded weight warning

To be fixed in https://github.com/paritytech/substrate/issues/13813

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix test pallet

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix more tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
…es to collections and items in the Uniques pallet in order to follow the commonly accepted NFTs terminology.) (#13322)

* Update documentation for uniques

The documentation was outdated after merge of #11389
Using the widely spread term collections and item instead
of the previous class and instance.

* Update README.md

---------

Co-authored-by: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com>
Co-authored-by: Jegor Sidorenko <jegor@parity.io>
* Implement #[pallet::hold_reason]

* Appease clippy

* cargo fmt

* Update test expectations

* Update test expectations

* Support composite_enum attribute instead

* Update test expectations

* Change hold_reason to composite_enum

* Add UI test for unsupported identifier when using composite_enum

* Fix comment

* Add documentation for pallet::composable_enum

* More docs

* cargo fmt
* Expose WASM extensions in executor semantics

* Fix benches

* Remove redundant extensions
* Remove deprecated pallet calls

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Deprecate old weight

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update Runtime API

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Delete shitty code

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix doctest

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/alliance/src/lib.rs

Co-authored-by: Koute <koute@users.noreply.github.com>

* Add doc

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* contracts: Use u64 as old weight type

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/contracts/src/lib.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Koute <koute@users.noreply.github.com>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: parity-processbot <>
…13820)

Instead of registering `ReadRuntimeVersionExt` in `sp-state-machine` it is moved to
`ExecutionExtension` which provides the default extensions.
* Adds on_idle round-robin logic to trait Hooks docs

* Makes the docs more concise

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* fmt

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
* TrieRecorder: Start adding support for transactions

* Adds `transactions` functions and some test

* More tests

* Docs

* Ensure that we rollback failed transactions in the storage proof

* FMT

* Update primitives/trie/src/recorder.rs

Co-authored-by: Dmitry Markin <dmitry@markin.tech>

* Review comments

* Update primitives/trie/src/recorder.rs

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>

* ".git/.scripts/commands/fmt/fmt.sh"

* For the holy clippy!

* Update primitives/trie/src/recorder.rs

Co-authored-by: Anton <anton.kalyaev@gmail.com>

---------

Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Co-authored-by: command-bot <>
Co-authored-by: Anton <anton.kalyaev@gmail.com>
* Add HoldReason to the NIS pallet

* Rename composable_enum to composite_enum

* Add encoding test

* Add more doc comments
…test utils (#13794)

* new test for try-runtime tuple stuff

* fix

* remove development comment

* formatting

* remove todo comment

* follow-chain working test

* refactor common cli testing utils

* fix comment

* revert Cargo.lock changes

* update Cargo.lock

* improve doc comment

* fix error typo

* update Cargo.lock

* feature gate try-runtime test

* build_substrate cli test util

* feature gate follow_chain tests

* move fn start_node to test-utils

* improve test pkg name

* use tokio Child and Command

* remove redundant import

* fix ci

* fix ci

* don't leave hanging processes

* improved child process cleanup

* use existing KillChildOnDrop

* remove redundant comment

* Update test-utils/cli/src/lib.rs

Co-authored-by: Koute <koute@users.noreply.github.com>

---------

Co-authored-by: kianenigma <kian@parity.io>
Co-authored-by: Koute <koute@users.noreply.github.com>
* benchmarking to generate weights file

* add the calculated weights in the extrinsics

* use benchmarking v2 syntax to generate the weights

* minor syntax change when benchmarking

* added WeightInfo in the mock to pass tests

* minor cargo fmt format changes
@gpestana gpestana requested review from athei, andresilva, koute and a team as code owners April 23, 2023 15:45
@paritytech-cicd-pr
Copy link

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

@gpestana
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@gpestana
Copy link
Contributor Author

Closing in lieu of #13979

@gpestana gpestana closed this Apr 23, 2023
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 I5-tests Tests need fixing, improving or augmenting. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.