-
Notifications
You must be signed in to change notification settings - Fork 1.6k
dispute-coordinator: past session dispute slashing #6811
Conversation
Co-authored-by: Marcin S. <marcin@bytedude.com>
7689006
to
5223ef3
Compare
* master: (27 commits) bump `zombienet` version to v1.3.37 (#6773) Bump `blake2b_simd` to 1.0.1 (#6829) changelog: update template for new label behavior (E3/E4) (#6804) Companion for paritytech/substrate#12828 (#6380) Don't send `ActiveLeaves` from leaves in db on startup in Overseer (#6727) Polkadot XCM Body constants (#6788) Decrease expected peer count in zombinenet tests (#6826) Additional tracing in `provisioner`, `vote_selection` and `dispute-coordinator` (#6775) Change node-key for bootnodes (#6772) Change handle_import_statements to FatalResult (#6820) Introduce XCM matcher for writing barriers (#6756) Freeze note on `SessionInfo`. (#6818) Bump parity-db (#6816) Removing Outdated References to Misbehavior Arbitration Subsystem (#6814) Forgotten re-export for `MatchedConvertedConcreteId` (#6815) Companion for substrate#13509: bump API versions of {Beefy,Mmr}Api (#6809) Migrate to `Weight::from_parts` (#6794) [XCM] Multiple `FungiblesAdapter`s support + `WeightTrader::buy_weight` more accurate error (#6739) Get rid of unnecessary cloning and work. (#6808) changelog: fix migration listing (#6806) ...
* master: (27 commits) bump `zombienet` version to v1.3.37 (#6773) Bump `blake2b_simd` to 1.0.1 (#6829) changelog: update template for new label behavior (E3/E4) (#6804) Companion for paritytech/substrate#12828 (#6380) Don't send `ActiveLeaves` from leaves in db on startup in Overseer (#6727) Polkadot XCM Body constants (#6788) Decrease expected peer count in zombinenet tests (#6826) Additional tracing in `provisioner`, `vote_selection` and `dispute-coordinator` (#6775) Change node-key for bootnodes (#6772) Change handle_import_statements to FatalResult (#6820) Introduce XCM matcher for writing barriers (#6756) Freeze note on `SessionInfo`. (#6818) Bump parity-db (#6816) Removing Outdated References to Misbehavior Arbitration Subsystem (#6814) Forgotten re-export for `MatchedConvertedConcreteId` (#6815) Companion for substrate#13509: bump API versions of {Beefy,Mmr}Api (#6809) Migrate to `Weight::from_parts` (#6794) [XCM] Multiple `FungiblesAdapter`s support + `WeightTrader::buy_weight` more accurate error (#6739) Get rid of unnecessary cloning and work. (#6808) changelog: fix migration listing (#6806) ...
…slashing-client * ao-past-session-slashing-runtime:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @ordian 💯
* master: PVF: instantiate runtime from bytes (#7270)
* master: [companion] Fix request-response protocols backpressure mechanism (#7276)
* master: XCM: Tools for uniquely referencing messages (#7234) Companion: Substrate#13869 (#7119) Companion for Substrate#14214 (#7283) Fix flaky test and error reporting (#7282) impl guide: Update Collator Generation (#7250) Add staking-miner bin (#7273) metrics: tests: Fix flaky runtime_can_publish_metrics (#7279)
…slashing-client * ao-past-session-slashing-runtime: XCM: Tools for uniquely referencing messages (#7234) Companion: Substrate#13869 (#7119) Companion for Substrate#14214 (#7283) Fix flaky test and error reporting (#7282) impl guide: Update Collator Generation (#7250) Add staking-miner bin (#7273) metrics: tests: Fix flaky runtime_can_publish_metrics (#7279) [companion] Fix request-response protocols backpressure mechanism (#7276) PVF: instantiate runtime from bytes (#7270)
"Processing unapplied validator slashes", | ||
); | ||
|
||
let inclusions = self.scraper.get_blocks_including_candidate(&candidate_hash); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only keep that information for the unfinalized chain (and a bit more). Is that enough for this purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. For the scraped included blocks this is fine. If we finalized a different block at the same height, that probably means the dispute has concluded before, so we should have called this already on some block producing node and DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION
helps with this race condition.
There's a potential problem with runtime API calls to key_ownership_proof
though since this block will be pruned once we reorg to and finalize the undisputed chain. But again, there's a time window between dispute concluding and that happening, so we should be able to slash with some high probability in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am uneasy about this. The raciness for participation already bothers me, but here it is even worse. I am not blocking this PR on this, as racy past session slashing is better than no past session slashing, but let's discuss this further.
@the-right-joyce worth mentioning this does not yet enable past session slashes on Kusama or Polkadot, only Westend and Rococo. |
Yes, enabling it is easy though: we need to implement v5 of ParachainHost and do the runtime upgrade: polkadot/runtime/westend/src/lib.rs Line 1477 in cf0ecdd
polkadot/runtime/kusama/src/lib.rs Line 1720 in cf0ecdd
|
Working on making this available on Polkadot and Kusama. Versioning was a bit messed up in places, fixing. @ordian was this tested with a runtime not yet supporting the calls? Seems we just debug log any error anyway. |
Ouch, I think I tested it before #6885 was merged, but yeah, the only trouble is we will get a different type of error in debug logs. These requirements need to be bumped to 5 to fix that indeed. |
Already on it. |
Closes #5947.
On top of #6667.
TODO:
WARN tokio-runtime-worker txpool::api: (offchain call) Error submitting a transaction to the pool: Transaction pool error: Invalid transaction validity: InvalidTransaction::ExhaustsResources