dispute-coordinator: disabling in participation #2637
Conversation
|
Looks very good! |
| .map_err(Error::UtilError)?; | ||
| let disabled_validators = | ||
| get_disabled_validators_with_fallback(ctx.sender(), parent).await.map_err(|e| { | ||
| Error::UtilError(TryFrom::try_from(e).expect("the conversion is infallible; qed")) |
There was a problem hiding this comment.
the error conversion is a bit ugly, but I'm open for suggestions how to improve it
ordian
added
the
T8-polkadot
| @@ -2542,6 +2583,353 @@ fn issue_local_statement_does_cause_distribution_but_not_duplicate_participation | |||
| }); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn participation_with_onchain_disabling() { | |||
There was a problem hiding this comment.
You have 2 scenarios. First of all I'd consider splitting scenarios into separate tests.
Scenario 1: unconfirmed dispute with onchain disabled validator against -> expected that we import but not participate
✔️
Scenario 2: confirmed dispute with disabled validator
This one might need to be split into 2 cases. I think it mixes up 2 different behaviours that we need to verify independently.
Firstly confirmation should work even if ALL the votes are coming from disabled guys simply as a defence in depth against something going really badly within the system. So instead of importing votes from normal validators push into the confirmation only with disabled and ensure it works correctly.
Secondly, even if the dispute is unconfirmed once we receive an invalid vote from a non-disabled validator we should participate.
Then we have all important cases.
There was a problem hiding this comment.
Firstly confirmation should work even if ALL the votes are coming from disabled guys simply as a defence in depth against something going really badly within the system.
Does this mean that if we have got a dispute storm, as the last incident on Kusama, disabling won't save us?
There was a problem hiding this comment.
Re: confirmation trumping disablement I've opened a new issue because there is some extra analysis needed there.
Nevertheless.... generally what happened in the recent kusama parachain stall would not be fully handled by the disabling strategy. We can deal with some dispute storms but not all dispute storms are alike. The recent one was caused by the faulty pruning logic which was present in all honest nodes. All honest nodes participated in something they shouldn't have. No disabling strategy can save us from that point so the best we could have done is freeze parachains and fix the critical error before the security is fully compromised.
If every node malfunctions no disabling will ever save us. The goal of the disabling strat is reducing the damages from a few (up to 1/3) faulty nodes but do it in such a way that even if those punished are honest the system does not collapse or reduce it's security significantly.
There was a problem hiding this comment.
All honest nodes participated in something they shouldn't have.
Valid point.
polkadot/roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md
Outdated
Show resolved
Hide resolved
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]>
ordian
force-pushed
the
ao-disabling-disputes
branch
from
49c066f to
95f8163
Compare
ordian
requested review from
andresilva,
jsidorenko,
cheme,
koute and
a team
as code owners
ordian
changed the base branch from
tsv-disabling
to
ao-backport-backing-disabling
ordian
removed request for
a team,
acatangiu,
sorpaas,
andresilva,
jsidorenko,
cheme and
koute
Co-authored-by: Tsvetomir Dimitrov <[email protected]>
Closes #2225.
potential_spam