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

Integration tests for staking + election-provider-multi-phase #12972

Merged
merged 23 commits into from
Mar 16, 2023

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Dec 19, 2022

This PR creates an integration test crate for EPM and staking that is intended to test as accurately as possible protocol edge cases and critical states. Another goal of the integration test crate is to document how to recover from those edge cases in production and inform future improvements.

The current PR includes two e2e tests:

  1. fn enters_emergency_phase_after_forcing_before_elect, which replicates the Kusama incident of 12/22.
  2. fn continous_slashes_below_offending_threshold, which applies a slash to a % of the active validators below the offending threshold until EPM enters in emergency phase (due to the election min score checks).

Next improvements paritytech/polkadot-sdk#423

Closes #9057

@gpestana gpestana added A3-in_progress Pull request is in progress. No review needed at this stage. 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. labels Dec 19, 2022
@gpestana gpestana self-assigned this Dec 19, 2022
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Dec 19, 2022
@gpestana gpestana marked this pull request as draft December 19, 2022 11:27
@gpestana gpestana requested a review from kianenigma December 19, 2022 11:27
}

pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
self.build().execute_with(test)
Copy link
Contributor

@kianenigma kianenigma Dec 21, 2022

Choose a reason for hiding this comment

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

<AllPallets as TryState>::try_state()


assert_eq!(pallet_staking::ForceEra::<Runtime>::get(), pallet_staking::Forcing::ForceNew);

advance_session();
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, we should plan the slashes to happen EXACTLY at the block where if we do roll_to(current_block + 1), a new session would happen automatically.

Right now, you are kinda simulating that 🙈

Not sure how accurate you want to be. Perhaps I am too strict here.

But, the nice thing is:

Assume the block at which the slashes happen is n, and the block at which the next session+election happens in x. If the difference between n and x is too short, then we fail. But if we have like 5 block between the two, this would not happen.

As a side note, in order to make these tests more accurate, you can inspire from here to also have automatic submission of OCWs: https://github.com/paritytech/substrate/blob/kiz-multi-block-election/frame/election-provider-multi-block/src/mock/mod.rs#L580

This could help tests be more accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah cool! Yes, I just finished my own flavour of fn roll_with_ocw, I should have looked a bit more into the existing code. But yes, I agree. The next commit should have a more accurate flow without simulating emergency etc

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.

Okay for not, looking to see more improvements!

@gpestana gpestana requested a review from kianenigma January 9, 2023 11:48
@gpestana gpestana marked this pull request as ready for review January 9, 2023 11:48
@gpestana gpestana requested review from rossbulat and Ank4n January 9, 2023 11:48
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.

Won't block this, but some improvements can be done.

Hard to judge at this point if the mock file is good, as not so many tests are there.

Hope you can address most my comments, merge, use it to test further PRs, then evaluate.

What I am worried is that a shitty mock file with too much unused functionality will be a headache waiting to happen and since this pallet is focused toward testing, we should make sure the mock build remains AAA.

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@gpestana
Copy link
Contributor Author

Won't block this, but some improvements can be done.

Hard to judge at this point if the mock file is good, as not so many tests are there.

Hope you can address most my comments, merge, use it to test further PRs, then evaluate.

What I am worried is that a shitty mock file with too much unused functionality will be a headache waiting to happen and since this pallet is focused toward testing, we should make sure the mock build remains AAA.

I agree with your comments, I'll address your comments and improve the current PR and ask for another pass before merging.

@gpestana
Copy link
Contributor Author

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 22cca14 into master Mar 16, 2023
@paritytech-processbot paritytech-processbot bot deleted the gpestana/9057-EPM-integration-tests branch March 16, 2023 12:09
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
…tech#12972)

* EPM and staking pallets: Adds new crate for integration tests

a

* Adds ExtBuilder and helpers with initial conditions assertions

* removes account helpers; adds staking, session, etc genesis

* Adds kusama incident test case

* Prepare for slashing test

* Adds solution submission

* slash_through_offending_threshold

* Renames e2e integration tests dir and crate

* consistently slash 10% of validator set

* finishes continous_slashes_below_offending_threshold test

* Update frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* mock fixes

* Additional checks to delayed solution eras and mock fixes

* nits and addresses review comments; splits ext_builder into one per pallet

* helper to set balances ext builder

* bring up mock.rs to master

* integration test fixes and additions

---------

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
…tech#12972)

* EPM and staking pallets: Adds new crate for integration tests

a

* Adds ExtBuilder and helpers with initial conditions assertions

* removes account helpers; adds staking, session, etc genesis

* Adds kusama incident test case

* Prepare for slashing test

* Adds solution submission

* slash_through_offending_threshold

* Renames e2e integration tests dir and crate

* consistently slash 10% of validator set

* finishes continous_slashes_below_offending_threshold test

* Update frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* mock fixes

* Additional checks to delayed solution eras and mock fixes

* nits and addresses review comments; splits ext_builder into one per pallet

* helper to set balances ext builder

* bring up mock.rs to master

* integration test fixes and additions

---------

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
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. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. I5-tests Tests need fixing, improving or augmenting.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Integration tests for staking + election-provider-multi-phase
3 participants