Remove implicit approval chilling upon slash.

[kianenigma]

related to #6835 and paritytech/polkadot#5974 and paritytech/polkadot#5978

(NEW) Context

This was discussed extensively with @burdges and other w3f researchers and the conclusion was that in the short term:

• the chilling that we do now is too restrictive.
• chilling upon slash enactment (what this PR initially wanted to do) is quite useless.

the ideal behavior we want is for nominators to be immediately chilled, perhaps based on some preference that they set, and then re-activated if the slash is canceled.

But, for now, and in the short term, we just remove the slashing behavior.

(OUTDATED) Context

The objective is to achieve this statement of the slashing spec:

We first post a slashing transaction to the chain, which drops the offending validator from the active validator list by invalidating their controller key, or maybe just their session keys. In consequence, all nodes ignore for the remainder of the era. It invalidates any future blocks that do not ignore too. We also remove all nomination approval votes by any nominator for , even those who currently allocate zero stake.

Historically, @rphmeier implemented this by reading the slashing spans upon the staking election. Over the years, the staking system has grown and the number of nominators has grown a lot, ergo the election process has become more weight/memory intense.

I don't see any reason for to think that the original approach had a particular advantage, but I might be missing something.

Moreover, the SlashingSpans of a staker are only removed ONLY when the staker is fully removed. This means the size of this storage item is potentially very large (see below for ) and it is being read every era upon staking election.

This changes the logic such that this chilling happens upon slash, and it is explicit, meaning that we actually remove the nomination. We still record SlashingSpans::last_nonzoro_slash and Nominations::submitted_in, which are no longer used onchain, but a UI can still use it to speculate if/when you need to re-send your nominations to a slashed validator (cc @jacogr @rossbulat).

This will basically make the slashing and npos election part decoupled, and reduces our worries about memory issues when creating the snapshot.

Regarding #6835, now it is pretty easy to move this chilling from compute_slash (when slash is reported) to do_slash (when slash is applied), so that validators who get slashed and are alter forgiven via governance don't lose nominations.

Review notes

The diff is bit noisy because I fixed some comments according to #12404 and a few other random things. The main diff is only in slashing.rs and pallet/impls.rs

I also added an event for when the slash is reported. Might help UIs. cc @wpank

Current slashing spans

// polkadot: 
Len = 455 / encoded_size = 22537
// kusama
Len = 2477 / encoded_size = 124903

As you see, we also have a problem of storing too much slashing spans, which I will formulate in a separate issue.

paritytech/polkadot-sdk#449 Would come very handy here.

polkadot companion: paritytech/polkadot#6424

[burdges]

We should likely remove the slashing spans after the unbonding period, perhaps by making stakers remove them whenever they touch their account, like unbonding, etc. We could make their controller key similarly remove the old spans.

We (Rob & I) designed the slashing spans system to make slashing more fair. We never implemented the whole scheme We threw this out the window for 100% soundness slashes, if not before then once I noticed that gambler's ruin gives us an actual security proof for the whole system. We also now know how to make it much harder to mistakenly triggering these non-soundness slashes. We could therefore discuss more radical simplifications if anyone dares. ;)

[kianenigma]

@ordian @rphmeier have either of you had the time to look at this?

[ordian]

Generally I'm in favor of this change. However, before approving, I'd like to get clarification on two things:

1. Why was slashing designed so that validators lose nominators even if the slash is reversed?

We intentionally error in favor of slashed nominators needing to redo their nominations, not just of the slashed validator, but all their nominations.

2. On zero slashes, validators should not lose nominators IMHO.

[burdges]

I'll claim naivety/idealism on that one. We really did not know all the practical concerns when designing this.

We ideally do want to chill nominators like this eventually, but only after considerable damage. As discussed, we'll presumably do slashes immediately but accrue "slashed balance" in nominators' accounts, and then disable them when this grows above 5% or something.

We should choose if the slashing spans computation stays, which provides similar defenses for retrospective slashing, or long range attacks. I'm think retrospective slashing is either a strange bug or governance or a real long range attack. We trust governance to be fair in the first two, and the third is pretty bad, so I'm tempted to say fuck it and forget about the spans..

[kianenigma]

The decision with @burdges and @ordian and @eskimor was to remove this chilling for the time being.

[kianenigma]

/cmd queue -c bench-bot $ pallet dev pallet_staking

[command-bot]

@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2147939 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_staking. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 56-353cc009-9d45-4c95-9e42-8e86090d03d2 to cancel this command or /cmd cancel to cancel all commands in this pull request.

[command-bot]

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_staking has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2147939 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2147939/artifacts/download.

[kianenigma]

bot merge

[paritytech-processbot]

Error: "Check reviews" status is not passing for paritytech/polkadot#6424

[kianenigma]

bot merge

[Polkadot-Forum]

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

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

[Polkadot-Forum]

This pull request 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 pull request 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