Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of the new validator disabling strategy #2226

Merged
merged 99 commits into from
Apr 26, 2024

Conversation

tdimitrov
Copy link
Contributor

@tdimitrov tdimitrov commented Nov 8, 2023

Closes #1966, #1963 and #1962.

Disabling strategy specification here. (Updated 13/02/2024)

Implements:

  • validator disabling for a whole era instead of just a session (been the case even before)
  • no more than 1/3 of the validators in the active set are disabled
    Removes:
  • DisableStrategy enum - now each validator committing an offence is disabled.
  • New era is not forced if too many validators are disabled.

Before this PR not all offenders were disabled. A decision was made based on enum DisableStrategy. Some offenders were disabled for a whole era, some just for a session, some were not disabled at all.

This PR changes the disabling behaviour. Now a validator committing an offense is disabled immediately till the end of the current era.

Some implementation notes:

  • OffendingValidators in pallet session keeps all offenders (this is not changed). However its type is changed from Vec<(u32, bool)> to Vec<u32>. The reason is simple - each offender is getting disabled so the bool doesn't make sense anymore.
  • When a validator is disabled it is first added to OffendingValidators and then to DisabledValidators. This is done in add_offending_validator from staking pallet.
  • In rotate_session the end_session also calls end_era when an era ends. In this case OffendingValidators are cleared (1).
  • Then in rotate_session DisabledValidators are cleared (2)
  • And finally (still in rotate_session) a call to start_session repopulates the disabled validators (3).
  • The reason for this complication is that session pallet knows nothing abut eras. To overcome this on each new session the disabled list is repopulated (points 2 and 3). Staking pallet knows when a new era starts so with point 1 it ensures that the offenders list is cleared.

tdimitrov and others added 6 commits October 20, 2023 16:13
Handles validator disabling in the backing subsystem. The PR introduces
two changes:
1. Don't import statements from disabled validators.
2. Don't validate and second if the local validator is disabled.

The purpose of this change is first to ignore statements from disabled
validators on the node side. This is an optimisation as these statements
are supposed to be cleared in the runtime too (still not implemented at
the moment). Additionally if the local node is a validator which is
disabled - don't process any new candidates.

---------

Co-authored-by: ordian <[email protected]>
Co-authored-by: ordian <[email protected]>
* master:
  Removed TODO from test-case for hard-coded delivery fee estimation (#2042)
  Expose collection attributes from `Inspect` trait (#1914)
  `polkadot-parachain-primitives` should not depend on `frame-support`. (#1897)
  [testnet] Align testnet system parachain runtimes using `RelayTreasuryLocation` and `SystemParachains` in the same way (#2023)
  Sort the benchmarks before listing them (#2026)
  publish pallet-root-testing (#2017)
  Contracts: Add benchmarks to include files (#2022)
  Small optimisation to `--profile dev` wasm builds (#1851)
  basic-authorship: Improve time recording and logging (#2010)
  Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto (#1815)
  [ci] Run check-rust-feature-propagation in pr and master (#2012)
  Improve features dev-ex (#1831)
  Remove obsolete comment. (#2008)
@tdimitrov tdimitrov changed the base branch from master to tsv-disabling November 8, 2023 13:57
@tdimitrov
Copy link
Contributor Author

And offenders list is already being cleared here:

<OffendingValidators<T>>::kill();

@ordian
Copy link
Member

ordian commented Nov 8, 2023

And offenders list is already being cleared here:

<OffendingValidators<T>>::kill();

But this is the state of offenders, not disabled state in session pallet. Although, it's currently cleared on session changes, so it's fine.

@tdimitrov
Copy link
Contributor Author

But this is the state of offenders, not disabled state in session pallet. Although, it's currently cleared on session changes, so it's fine.

Exactly. DisabledValidators are cleared on each session. The code I linked is called in end_session just before that. I guess it should work :)

I had a concern why the validator indices are not recalculated but as far as I can see in the code, the active set is not reshuffled between sessions within an era.

@tdimitrov tdimitrov added the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Nov 9, 2023
@tdimitrov tdimitrov mentioned this pull request Nov 9, 2023
4 tasks
@tdimitrov tdimitrov linked an issue Nov 15, 2023 that may be closed by this pull request
@tdimitrov tdimitrov changed the title Validators are disabled for a whole era on any offence Implementation of the new validator disabling strategy Nov 15, 2023
@ordian ordian enabled auto-merge April 26, 2024 13:26
@ordian ordian dismissed kianenigma’s stale review April 26, 2024 13:28

The technical comments have been resolved and the PR is audited and tested

@ordian ordian added this pull request to the merge queue Apr 26, 2024
Merged via the queue into master with commit 988e30f Apr 26, 2024
139 of 140 checks passed
@ordian ordian deleted the tsv-disabling-for-era branch April 26, 2024 14:02
Morganamilo pushed a commit that referenced this pull request May 2, 2024
Closes #1966,
#1963 and
#1962.

Disabling strategy specification
[here](#2955). (Updated
13/02/2024)

Implements:
* validator disabling for a whole era instead of just a session
* no more than 1/3 of the validators in the active set are disabled
Removes:
* `DisableStrategy` enum - now each validator committing an offence is
disabled.
* New era is not forced if too many validators are disabled.

Before this PR not all offenders were disabled. A decision was made
based on [`enum
DisableStrategy`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/primitives/staking/src/offence.rs#L54).
Some offenders were disabled for a whole era, some just for a session,
some were not disabled at all.

This PR changes the disabling behaviour. Now a validator committing an
offense is disabled immediately till the end of the current era.

Some implementation notes:
* `OffendingValidators` in pallet session keeps all offenders (this is
not changed). However its type is changed from `Vec<(u32, bool)>` to
`Vec<u32>`. The reason is simple - each offender is getting disabled so
the bool doesn't make sense anymore.
* When a validator is disabled it is first added to
`OffendingValidators` and then to `DisabledValidators`. This is done in
[`add_offending_validator`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/slashing.rs#L325)
from staking pallet.
* In
[`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623)
the `end_session` also calls
[`end_era`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L490)
when an era ends. In this case `OffendingValidators` are cleared
**(1)**.
* Then in
[`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623)
`DisabledValidators` are cleared **(2)**
* And finally (still in `rotate_session`) a call to
[`start_session`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L430)
repopulates the disabled validators **(3)**.
* The reason for this complication is that session pallet knows nothing
abut eras. To overcome this on each new session the disabled list is
repopulated (points 2 and 3). Staking pallet knows when a new era starts
so with point 1 it ensures that the offenders list is cleared.

---------

Co-authored-by: ordian <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: Maciej <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Ankan <[email protected]>
bkontur pushed a commit that referenced this pull request May 15, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 16, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 17, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 17, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 17, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 20, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 21, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 22, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 23, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request May 30, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request Jun 4, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request Jun 5, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request Jun 7, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
bkontur pushed a commit that referenced this pull request Jun 7, 2024
* StorageProofSize -> StorageSize

* Rename benchmarks

* StorageSize -> UnverifiedStorageProofParams

* Fix clippy

* Fix priority boost
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Closes paritytech#1966,
paritytech#1963 and
paritytech#1962.

Disabling strategy specification
[here](paritytech#2955). (Updated
13/02/2024)

Implements:
* validator disabling for a whole era instead of just a session
* no more than 1/3 of the validators in the active set are disabled
Removes:
* `DisableStrategy` enum - now each validator committing an offence is
disabled.
* New era is not forced if too many validators are disabled.

Before this PR not all offenders were disabled. A decision was made
based on [`enum
DisableStrategy`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/primitives/staking/src/offence.rs#L54).
Some offenders were disabled for a whole era, some just for a session,
some were not disabled at all.

This PR changes the disabling behaviour. Now a validator committing an
offense is disabled immediately till the end of the current era.

Some implementation notes:
* `OffendingValidators` in pallet session keeps all offenders (this is
not changed). However its type is changed from `Vec<(u32, bool)>` to
`Vec<u32>`. The reason is simple - each offender is getting disabled so
the bool doesn't make sense anymore.
* When a validator is disabled it is first added to
`OffendingValidators` and then to `DisabledValidators`. This is done in
[`add_offending_validator`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/slashing.rs#L325)
from staking pallet.
* In
[`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623)
the `end_session` also calls
[`end_era`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L490)
when an era ends. In this case `OffendingValidators` are cleared
**(1)**.
* Then in
[`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623)
`DisabledValidators` are cleared **(2)**
* And finally (still in `rotate_session`) a call to
[`start_session`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L430)
repopulates the disabled validators **(3)**.
* The reason for this complication is that session pallet knows nothing
abut eras. To overcome this on each new session the disabled list is
repopulated (points 2 and 3). Staking pallet knows when a new era starts
so with point 1 it ensures that the offenders list is cleared.

---------

Co-authored-by: ordian <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: Maciej <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Ankan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
Status: Audited
10 participants