Dispute Coordinator Subsystem by rphmeier · Pull Request #3150 · paritytech/polkadot

rphmeier · 2021-06-01T03:14:49Z

The dispute coordinator keeps track of all votes received on all candidates in a rolling window of sessions. It uses these to detect disputes between validators. It also provides interfaces to inform the provisioner about what to include in the chain.

More information can be found in the implementers' guide: https://w3f.github.io/parachain-implementers-guide/node/disputes/dispute-coordinator.html

This does not yet implement chain rollbacks or integrate the subsystem into the overseer. That will require a parachains-db version upgrade and will be done in a follow-on pull request.

node/core/dispute-coordinator/src/lib.rs

node/subsystem/src/messages.rs

node/core/dispute-coordinator/src/db/v1.rs

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

as of #3222 we can do most of the work within the approval voting subsystem

node/core/dispute-coordinator/src/lib.rs

ordian · 2021-06-12T18:42:14Z

node/core/dispute-coordinator/src/lib.rs

+
+// It would be nice to draw this from the chain state, but we have no tools for it right now.
+// On Polkadot this is 1 day, and on Kusama it's 6 hours.
+const DISPUTE_WINDOW: SessionIndex = 6;


but we have no tools for it right now

could you elaborate why can't we use SessionInfo::dispute_period from the runtime like we did previously?

There is no dispute_period in struct SessionInfo. That is exactly what I mean by 'no tools for it' - we'd need to change the type definition or add a new runtime API.

Sorry, I meant in HostConfiguration.

But we can probably rely on the fact that session_info will store sessions based on dispute_period?

#3227 <- We need to expose it, and that's beyond scope here IMO

drahnr

A few nits, generally looks very good and seems to do what is described in the Dispute Coordinator in the guide 👍

node/core/dispute-coordinator/src/db/v1.rs

node/core/dispute-coordinator/src/lib.rs

node/primitives/src/lib.rs

node/core/dispute-coordinator/src/lib.rs

This reverts commit 9cd615e.

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

node/primitives/src/lib.rs

node/core/dispute-coordinator/src/lib.rs

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

andresilva · 2021-06-13T10:32:33Z

node/subsystem-util/src/rolling_session_window.rs

+	/// When inspecting a new import notification, updates the session info cache to match
+	/// the session of the imported block.
+	///
+	/// this only needs to be called on heads where we are directly notified about import, as sessions do


Suggested change

/// this only needs to be called on heads where we are directly notified about import, as sessions do

/// This only needs to be called on heads where we are directly notified about import, as sessions do

andresilva · 2021-06-13T10:32:42Z

node/subsystem-util/src/rolling_session_window.rs

+	/// this only needs to be called on heads where we are directly notified about import, as sessions do
+	/// not change often and import notifications are expected to be typically increasing in session number.
+	///
+	/// some backwards drift in session index is acceptable.


Suggested change

/// some backwards drift in session index is acceptable.

/// Some backwards drift in session index is acceptable.

andresilva · 2021-06-13T10:54:33Z

node/subsystem-util/src/rolling_session_window.rs

+						Err(SessionsUnavailable {
+							kind,
+							info: Some(SessionsUnavailableInfo {
+								window_start: latest +1,


Suggested change

window_start: latest +1,

window_start: fresh_start,

AFAIU this could also be window_start since that's what we wanted the full window to be, but we only started requesting from fresh_start and so that's the only data that we're missing.

andresilva · 2021-06-13T11:06:39Z

node/subsystem-util/src/rolling_session_window.rs

+						let update = SessionWindowUpdate::Advanced {
+							prev_window_start: old_window_start,
+							prev_window_end: old_window_end,
+							new_window_start: window_start,


We should use earliest_session from below here to account for the edge case described.

andresilva · 2021-06-13T11:12:09Z

node/subsystem/src/messages.rs

+		/// The indices of validators who have already voted on this candidate.
+		voted_indices: Vec<ValidatorIndex>,


Not sure this will be needed.

andresilva · 2021-06-13T11:15:50Z

node/network/bridge/src/tests.rs

+            // Add variants here as needed, `{ cnt += 1; }` for those that need to be
+            // notified, `unreachable!()` for those that should not.
+        }
+    }
+    assert_eq!(cnt, EXPECTED_COUNT);


Formatted with spaces.

andresilva · 2021-06-13T11:22:58Z

node/primitives/src/lib.rs

+	}
+}
+
+


Suggested change

andresilva · 2021-06-13T11:28:04Z

node/core/dispute-coordinator/src/lib.rs

+
+		// TODO [after https://github.com/paritytech/polkadot/issues/3160]: chain rollbacks


Suggested change

// TODO [after https://github.com/paritytech/polkadot/issues/3160]: chain rollbacks

I guess this won't be happening anymore as we'll go with using SelectChain instead.

rphmeier added 11 commits May 28, 2021 18:42

skeleton for dispute-coordinator

b9ed6d9

add coordinator and participation message types

e0cf708

begin dispute-coordinator DB

2301bf7

functions for loading

d81155a

implement strongly-typed DB transaction

707fe90

add some tests for DB transaction

a26f138

core logic for pruning

4304d70

guide: update candidate-votes key for coordinator

535234d

update candidate-votes key

34e3c09

use big-endian encoding for session, and implement upper bound generator

aa9b06c

finish implementing pruning

9931719

rphmeier added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 1, 2021

andresilva reviewed Jun 1, 2021

View reviewed changes

node/core/dispute-coordinator/src/lib.rs Outdated Show resolved Hide resolved

node/subsystem/src/messages.rs Outdated Show resolved Hide resolved

node/subsystem/src/messages.rs Show resolved Hide resolved

node/core/dispute-coordinator/src/db/v1.rs Show resolved Hide resolved

rphmeier and others added 15 commits June 1, 2021 12:50

add a test for note_current_session

bc8f31d

define state of the subsystem itself

da98ac1

barebones subsystem definition

bb5e26e

control flow

2f926cf

more control flow

f7ec29c

implement session-updating logic

5c371cc

trace

71483a0

control flow for message handling

15783d1

Update node/core/dispute-coordinator/src/lib.rs

b1e6ad3

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

Update node/subsystem/src/messages.rs

493430d

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

some more control flow

465db42

guide: remove overlay

5883f85

more control flow

af3de52

implement some DB getters

b677355

make progress on importing statements

0ec4727

add more tests

571a00c

rphmeier marked this pull request as ready for review June 11, 2021 15:03

rphmeier added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jun 11, 2021

add variants to network bridge

42e5951

rphmeier added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Jun 11, 2021

rphmeier added 2 commits June 11, 2021 19:33

Merge branch 'master' into rh-dispute-coordinator

ebe7a08

fix test compilation

eac3bb4

rphmeier requested review from andresilva and eskimor June 12, 2021 12:08

rphmeier mentioned this pull request Jun 12, 2021

Dispute distribution guide #3158

Merged

rphmeier added 2 commits June 12, 2021 15:37

remove most dispute coordinator functionality

9cd615e

as of #3222 we can do most of the work within the approval voting subsystem

Merge branch 'master' into rh-dispute-coordinator

9dea8ee

Lldenaurois self-requested a review June 12, 2021 17:59

ordian reviewed Jun 12, 2021

View reviewed changes

node/core/dispute-coordinator/src/lib.rs Outdated Show resolved Hide resolved

ordian reviewed Jun 12, 2021

View reviewed changes

drahnr approved these changes Jun 12, 2021

View reviewed changes

rphmeier and others added 2 commits June 12, 2021 21:31

Revert "remove most dispute coordinator functionality"

4042c5a

This reverts commit 9cd615e.

Use thiserror

f0d3179

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

Lldenaurois reviewed Jun 12, 2021

View reviewed changes

node/primitives/src/lib.rs Show resolved Hide resolved

node/core/dispute-coordinator/src/lib.rs Show resolved Hide resolved

rphmeier and others added 4 commits June 13, 2021 12:30

Update node/core/dispute-coordinator/src/lib.rs

8c3af58

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

extract tests to separate module

9647f65

address nit

6a3ea3f

adjust run_iteration API

ad8f990

rphmeier merged commit 19c1d29 into master Jun 13, 2021

rphmeier deleted the rh-dispute-coordinator branch June 13, 2021 11:35

andresilva reviewed Jun 13, 2021

View reviewed changes

rphmeier mentioned this pull request Jun 15, 2021

Tracking issue for Parachain disputes #3255

Closed

28 tasks

jacogr mentioned this pull request Aug 19, 2021

Adjust ValidDisputeStatementKind enum polkadot-js/api#3884

Merged

rphmeier mentioned this pull request Sep 26, 2022

Member Request polkadot-fellows/seeding#4

Merged

	/// this only needs to be called on heads where we are directly notified about import, as sessions do
	/// This only needs to be called on heads where we are directly notified about import, as sessions do

	/// some backwards drift in session index is acceptable.
	/// Some backwards drift in session index is acceptable.

		/// The indices of validators who have already voted on this candidate.
		voted_indices: Vec<ValidatorIndex>,


		// TODO [after https://github.com/paritytech/polkadot/issues/3160]: chain rollbacks

Conversation

rphmeier commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drahnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rphmeier commented Jun 1, 2021 •

edited

Loading