No chilling of validators #1962

eskimor · 2023-10-20T16:44:29Z

Validators should always be up for re-election even if they were slashed. This is necessary so any consensus issues will never lead to malicious actors increasing their share on active validators.

eskimor · 2023-11-08T13:30:28Z

@paritytech/staking-core @gpestana any concerns?

eskimor · 2023-11-09T11:00:50Z

Alternatively: Disabling without slashing (not even 0%)? @ordian

eskimor · 2023-11-22T13:58:16Z

@tdimitrov can we disable without slashing? Or please ping @gpestana on this one, thanks!

tdimitrov · 2023-11-22T14:00:46Z

@tdimitrov can we disable without slashing? Or please ping @gpestana on this one, thanks!

Yes. With the new disabling strategy we disable each offender no matter if it was slashed or not.

gpestana · 2023-11-23T11:09:37Z

In the current logic, slashed validators are "chilled" and their intention for validating is dropped. However, the validator can still re-validate its intention to become a validator right away. Is this the expected behaviour?

Recently we even changed the logic so that the nominations of slashed/chilled validators are not dropped. This means that once the chilled validator re-validates its intention to become a validator, its nominations remain the same as pre-slash.

Is this the intended behaviour described by this issue?

eskimor · 2023-11-23T12:38:29Z

Yes. With the new disabling strategy we disable each offender no matter if it was slashed or not.

And do we also do that? So for voting invalid on valid - we disable but don't slash (not even 0%) and thus we get no chilling?

Is this the intended behavior described by this issue?

Not quite. It is fine if we can disable, without slashing (and thus no chilling). We are worried about the particular case that a determinism issue is abused to get honest validators disabled. We fixed all the concerns we had with disabling, but if those validators would not get re-elected we would still have the original problem just delayed: An attacker can increase its share of validators, by kicking out honest validators from the set (abusing some nondeterminism) and then run an attack.

gpestana · 2023-11-23T12:38:36Z

This is necessary so any consensus issues will never lead to malicious actors increasing their share on active validators.

Could you elaborate? Are you mentioning the case where offence have been reported on non-faulty validators for some reason?

eskimor · 2023-11-23T12:41:43Z

Yes. This obviously should not happen, but it is super hard (impossible) to guarantee this from never happening. With #742 in place, we will have removed one important source, but I would sleep better if nondeterminism could never cause security issues (only liveness).

ordian · 2023-11-23T13:32:55Z

So for voting invalid on valid - we disable but don't slash (not even 0%) and thus we get no chilling?

currently we don't disable (no offence is submitted)

eskimor · 2023-11-23T14:48:13Z

@ordian just mentioned that he is 80% sure that there is no chilling on 0% slashes. If that is true, then this is good enough and we can close this ticket. Consequences of doing non 0% slashes should be documented in the guide though.

ordian · 2023-11-23T16:27:18Z

polkadot-sdk/substrate/frame/staking/src/slashing.rs

Lines 236 to 239 in 21f1811

    
           if params.slash * params.exposure.total == Zero::zero() { 
        
           	// kick out the validator even if they won't be slashed, 
        
           	// as long as the misbehavior is from their most recent slashing span. 
        
           	kick_out_if_recent::<T>(params);

This doesn't seem to indicate that I was correct. Especially given that the

polkadot-sdk/substrate/frame/staking/src/slashing.rs

Line 309 in 21f1811

let mut spans = fetch_spans::<T>(

seems to be inserting the span

polkadot-sdk/substrate/frame/staking/src/slashing.rs

Line 449 in 21f1811

crate::SlashingSpans::<T>::insert(stash, &spans);

tdimitrov · 2023-11-24T14:39:17Z

And also here:

polkadot-sdk/substrate/frame/staking/src/slashing.rs

Line 286 in 9f8c598

add_offending_validator::<T>(params.stash);

I don't know how chilling is related to slashing, but in the current implementation we disable based on enum DisableStrategy (attached to the offence). In the new one - we disable unconditionally.

tdimitrov · 2023-12-07T12:19:48Z

Me and @gpestana decided not to touch slashing spans with this issue and only remove validator chilling on offence.

Removing slashing spans is tracked in #2650

* Bump substrate/polkadot/cumulus * sp_finality_grandpa - >sp_consensus_grandpa * sp_beefy -> sp_consensus_beefy * pallet_randomness_collective_flip -> pallet_insecure_randomness_collective_flip * fix * Cumulus parachain stuff * Cumulus parachain stuff one more * Millau/Rialto runtimes * Removed pallet_insecure_randomness_collective_flip * Millau node * Removed session historial * TMP: just try disable all fetches * Docs in gitlab.yml

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]>

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]>

eskimor added this to parachains team board Oct 20, 2023

eskimor converted this from a draft issue Oct 20, 2023

eskimor mentioned this issue Nov 23, 2023

Guide Update with all considerations #1961

Closed

tdimitrov mentioned this issue Dec 7, 2023

Consider removing/simplifying SlashingSpans and SpanSlash from staking pallet #2650

Open

tdimitrov moved this from Backlog to In Progress in parachains team board Dec 7, 2023

tdimitrov mentioned this issue Dec 7, 2023

Implementation of the new validator disabling strategy #2226

Merged

Overkillus linked a pull request Dec 11, 2023 that will close this issue

Implementation of the new validator disabling strategy #2226

Merged

Overkillus moved this from In Progress to Review in progress in parachains team board Dec 11, 2023

Overkillus assigned tdimitrov Dec 11, 2023

ordian closed this as completed in #2226 Apr 26, 2024

github-project-automation bot moved this from Review in progress to Completed in parachains team board Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No chilling of validators #1962

No chilling of validators #1962

eskimor commented Oct 20, 2023 •

edited

Loading

eskimor commented Nov 8, 2023 •

edited

Loading

eskimor commented Nov 9, 2023

eskimor commented Nov 22, 2023 •

edited

Loading

tdimitrov commented Nov 22, 2023

gpestana commented Nov 23, 2023

eskimor commented Nov 23, 2023

gpestana commented Nov 23, 2023

eskimor commented Nov 23, 2023

ordian commented Nov 23, 2023

eskimor commented Nov 23, 2023

ordian commented Nov 23, 2023

tdimitrov commented Nov 24, 2023

tdimitrov commented Dec 7, 2023

No chilling of validators #1962

No chilling of validators #1962

Comments

eskimor commented Oct 20, 2023 • edited Loading

eskimor commented Nov 8, 2023 • edited Loading

eskimor commented Nov 9, 2023

eskimor commented Nov 22, 2023 • edited Loading

tdimitrov commented Nov 22, 2023

gpestana commented Nov 23, 2023

eskimor commented Nov 23, 2023

gpestana commented Nov 23, 2023

eskimor commented Nov 23, 2023

ordian commented Nov 23, 2023

eskimor commented Nov 23, 2023

ordian commented Nov 23, 2023

tdimitrov commented Nov 24, 2023

tdimitrov commented Dec 7, 2023

eskimor commented Oct 20, 2023 •

edited

Loading

eskimor commented Nov 8, 2023 •

edited

Loading

eskimor commented Nov 22, 2023 •

edited

Loading