Full Slash Reversals

[staking4all]

Recently we have seen slashing on validators on Kusama and Polkadot that was a result of a bug.

Slashes are in place to keep the network safe, however when a slash is executed due to a bug and not as a result of malicious behavior it then brings to question if the slash should be reversed.

A slash consists of

• Financial slash by slashing dot staked on the validator, the enactment of this is delayed
• Removing all nominations immediately, this is done immediately to ensure bad actors are halted immediately

The council however should be in the position to reverse a slash if deemed a bug or not a result of malicious behaviour.

However at this stage only the "financial" impact is reverted which halts the enactment.

However the reversal of the LOSS of the validators nominations does not occur.

This means reversal of slash mainly protects the "nominators" and doesn't reinstate the validators nominations status.

The proposal would be that ALL affects of the slash be removed.

I do understand that nominators should be made aware of bad actors and that is why the original design requires them to renominate the validator to get the nominations back. However there is nothing showing bad actors (bad validator) on the UI when the nominators renominate. If there is no financial slash then the nominator will not really ever notice or investigate further.

I believe ensuring nominators are aware of bad actors can only be seen fully when there is a slash.

I would request that when slashes are reversed that both Financial aspect is reversed as well any/all slash side affects which includes reinstating of nominations.

[burdges]

I'd think this depends upon reversal reasoning, so it's unclear one should do this for one-off cases by default.

We intentionally error in favor of slashed nominators needing to redo their nominations, not just of the slashed validator, but all their nominations. Yet, our slashing design in https://github.com/w3f/research/blob/master/docs/polkadot/slashing/npos.md#suppressed-nominators-in-phragmen includes a parameter \xi by which governance can quickly just partially suppress the suppression of nominations so the network continues operating with most stake in play.

I'd think the same concerns arise here: If we've a big bug that require reverting many slashes, then we'd hate to leave the network running on significantly less stake afterwards. If \xi is implemented, then adjusting \xi works here, and this is exactly one case for which it was created, but mass reverting might be preferable under some situations.

We encountered computational complexity issues, not with reverting per se, but with applying the denominations in the first place. I made \xi global to simplify this computation.

cc @AlfonsoCev @jonasW3F

[kianenigma]

I'd like to read the link above at some point but I have a proposal for the time being:

Currently, we do this filtering here:

substrate/frame/staking/src/lib.rs

Lines 3025 to 3030 in d019a66

	targets.retain(|stash| {
	<Self as Store>::SlashingSpans::get(&stash).map_or(
	true,
	|spans| submitted_in >= spans.last_nonzero_slash(),
	)
	});

Which is already a bit a of weird situation because it was not really apparent, and none of the external tools supported this. Even to this day I think polkadot-js-apps either has not added support to warn about this, or has done it very recently.

Instead, we can store 1 bit per nomination on chain to indicate if it is suppressed or not, similar to the suppressed field of Nominations:

#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)]
pub struct Nominations<AccountId> {
	/// The targets of nomination.
- 	pub targets: Vec<AccountId>,
+ 	pub targets: Vec<(AccountId, bool)>,
	/// The era the nominations were submitted.
	///
	/// Except for initial nominations which are considered submitted at era 0.
	pub submitted_in: EraIndex,
	/// Whether the nominations have been suppressed. This can happen due to slashing of the
	/// validators, or other events that might invalidate the nomination.
	///
	/// NOTE: this for future proofing and is thus far not used.
	pub suppressed: bool,
}

and upon non-zero slash we flip this bit. Then, the council's proposal to revert a slash can flip the bit again.

Of course there is a tradeoff:

1. This is much better for the election process because then we don't need to do this filter on the fly and saves us a lot of time.
2. But this makes the following more expensive.
   • Slashing event more expensive because we need to go to Nominators storage of every nominator in the slashee's Exposure.others and flip their bit.
   • the cancel_slash() will also be a lot more expensive because it will be the same.

Nonetheless, I think with correct weight measuring and parameters to facilitate the search, we can implement this in a feasible manner and it will solve this problem in an isolated manner, without the need to touch SlashingSpans or anything else in the slashing sub-module.

cc @rphmeier

[rphmeier]

Slashing event more expensive because we need to go to Nominators storage of every nominator in the slashee's Exposure.others and flip their bit.

Yeah, this was the main reason we avoided that before. The benchmarking infra is a lot better now so we could run some empirical tests with a few tens of thousands of nominators.

[burdges]

I left the \xi parameter global so it should work fine with this bit just like without the bit.

[kianenigma]

Closed finally.

[Polkadot-Forum]

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/kusama-era-4543-slashing/1410/2

[Polkadot-Forum]

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-35/1509/1