enable disputes for known chains, except for polkadot (#4464)

* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>

.gitlab-ci.yml

@@ -196,7 +196,7 @@ test-build-linux-stable:
     - ./scripts/gitlab/test_linux_stable.sh
     # we're using the bin built here, instead of having a parallel `build-linux-release`
     # disputes feature is needed for zombie-net parachains malus test
-    - time cargo build --release --verbose --bin polkadot --features "disputes"
+    - time cargo build --release --verbose --bin polkadot
     - sccache -s
     # pack artifacts
     - mkdir -p ./artifacts
@@ -277,7 +277,7 @@ build-malus:
     - if: $CI_COMMIT_REF_NAME == "master"
     - if: $CI_COMMIT_REF_NAME =~ /^[0-9]+$/                         # PRs
   script:
-    - time cargo build --release --verbose -p polkadot-test-malus --features disputes
+    - time cargo build --release --verbose -p polkadot-test-malus
     - sccache -s
     # pack artifacts
     - mkdir -p ./artifacts

Cargo.toml

@@ -117,9 +117,9 @@ panic = "unwind"
 [features]
 runtime-benchmarks= [ "polkadot-cli/runtime-benchmarks" ]
 try-runtime = [ "polkadot-cli/try-runtime" ]
-disputes = [ "polkadot-cli/disputes" ]
 runtime-metrics = [ "polkadot-cli/runtime-metrics" ]
 
+
 # Configuration for building a .deb package - for use with `cargo-deb`
 [package.metadata.deb]
 name = "polkadot"

bridges/bin/rialto/node/src/overseer.rs

@@ -30,6 +30,7 @@ use polkadot_node_core_av_store::Config as AvailabilityConfig;
 use polkadot_node_core_candidate_validation::Config as CandidateValidationConfig;
 use polkadot_node_core_chain_selection::Config as ChainSelectionConfig;
 use polkadot_node_core_dispute_coordinator::Config as DisputeCoordinatorConfig;
+use polkadot_node_core_provisioner::ProvisionerConfig;
 use polkadot_node_network_protocol::request_response::{v1 as request_v1, IncomingRequestReceiver};
 use polkadot_overseer::{
 	metrics::Metrics as OverseerMetrics, BlockInfo, MetricsTrait, Overseer, OverseerBuilder,
@@ -108,6 +109,8 @@ where
 	pub chain_selection_config: ChainSelectionConfig,
 	/// Configuration for the dispute coordinator subsystem.
 	pub dispute_coordinator_config: DisputeCoordinatorConfig,
+	/// Configuration for the provisioner subsystem.
+	pub disputes_enabled: bool,
 }
 
 /// Obtain a prepared `OverseerBuilder`, that is initialized
@@ -133,6 +136,7 @@ pub fn prepared_overseer_builder<Spawner, RuntimeClient>(
 		candidate_validation_config,
 		chain_selection_config,
 		dispute_coordinator_config,
+		disputes_enabled,
 	}: OverseerGenArgs<'_, Spawner, RuntimeClient>,
 ) -> Result<
 	OverseerBuilder<
@@ -218,7 +222,7 @@ where
 			Box::new(network_service.clone()),
 			Metrics::register(registry)?,
 		))
-		.provisioner(ProvisionerSubsystem::new(spawner.clone(), (), Metrics::register(registry)?))
+		.provisioner(ProvisionerSubsystem::new(spawner.clone(), ProvisionerConfig { disputes_enabled }, Metrics::register(registry)?))
 		.runtime_api(RuntimeApiSubsystem::new(
 			runtime_client.clone(),
 			Metrics::register(registry)?,

bridges/bin/rialto/node/src/service.rs

@@ -473,6 +473,7 @@ where
 		slot_duration_millis: slot_duration.as_millis() as u64,
 	};
 
+
 	let candidate_validation_config = CandidateValidationConfig {
 		artifacts_cache_path: config
 			.database
@@ -581,6 +582,7 @@ where
 					dispute_req_receiver,
 					pov_req_receiver,
 					statement_req_receiver,
+					disputes_enabled: false,
 				},
 			)?;
 		let handle = Handle::new(overseer_handle);

cli/Cargo.toml

@@ -66,5 +66,4 @@ westend-native = [ "service/westend-native" ]
 rococo-native = [ "service/rococo-native" ]
 
 malus = [ "full-node", "service/malus" ]
-disputes = [ "service/disputes" ]
 runtime-metrics = ["service/runtime-metrics", "polkadot-node-metrics/runtime-metrics"]

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

@@ -16,45 +16,29 @@
 
 //! Implements the dispute coordinator subsystem (dummy implementation).
 
-use std::sync::Arc;
-
 use polkadot_node_subsystem::{
-	errors::{ChainApiError, RuntimeApiError},
-	messages::DisputeCoordinatorMessage,
-	overseer, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError,
+	messages::DisputeCoordinatorMessage, overseer, FromOverseer, OverseerSignal, SpawnedSubsystem,
+	SubsystemContext, SubsystemError,
 };
 use polkadot_primitives::v1::BlockNumber;
 
-use futures::{channel::oneshot, prelude::*};
-use kvdb::KeyValueDB;
-use parity_scale_codec::{Decode, Encode, Error as CodecError};
-use sc_keystore::LocalKeystore;
+use futures::prelude::*;
 
-use crate::metrics::Metrics;
+use crate::error::{Error, Result};
 
 const LOG_TARGET: &str = "parachain::dispute-coordinator";
 
-/// Timestamp based on the 1 Jan 1970 UNIX base, which is persistent across node restarts and OS reboots.
-type Timestamp = u64;
-
 #[derive(Eq, PartialEq)]
 enum Participation {}
 
 struct State {}
 
-/// Configuration for the dispute coordinator subsystem.
-#[derive(Debug, Clone, Copy)]
-pub struct Config {
-	/// The data column in the store to use for dispute data.
-	pub col_data: u32,
-}
-
 /// An implementation of the dispute coordinator subsystem.
 pub struct DisputeCoordinatorSubsystem {}
 
 impl DisputeCoordinatorSubsystem {
 	/// Create a new instance of the subsystem.
-	pub fn new(_: Arc<dyn KeyValueDB>, _: Config, _: Arc<LocalKeystore>, _: Metrics) -> Self {
+	pub fn new() -> Self {
 		DisputeCoordinatorSubsystem {}
 	}
 }
@@ -71,109 +55,6 @@ where
 	}
 }
 
-#[derive(Debug, thiserror::Error)]
-#[allow(missing_docs)]
-pub enum Error {
-	#[error(transparent)]
-	RuntimeApi(#[from] RuntimeApiError),
-
-	#[error(transparent)]
-	ChainApi(#[from] ChainApiError),
-
-	#[error(transparent)]
-	Io(#[from] std::io::Error),
-
-	#[error(transparent)]
-	Oneshot(#[from] oneshot::Canceled),
-
-	#[error("Oneshot send failed")]
-	OneshotSend,
-
-	#[error(transparent)]
-	Subsystem(#[from] SubsystemError),
-
-	#[error(transparent)]
-	Codec(#[from] CodecError),
-}
-
-impl Error {
-	fn trace(&self) {
-		match self {
-			// don't spam the log with spurious errors
-			Self::RuntimeApi(_) | Self::Oneshot(_) =>
-				tracing::debug!(target: LOG_TARGET, err = ?self),
-			// it's worth reporting otherwise
-			_ => tracing::warn!(target: LOG_TARGET, err = ?self),
-		}
-	}
-}
-
-/// The status of dispute. This is a state machine which can be altered by the
-/// helper methods.
-#[derive(Debug, Clone, Copy, Encode, Decode, PartialEq)]
-pub enum DisputeStatus {
-	/// The dispute is active and unconcluded.
-	#[codec(index = 0)]
-	Active,
-	/// The dispute has been concluded in favor of the candidate
-	/// since the given timestamp.
-	#[codec(index = 1)]
-	ConcludedFor(Timestamp),
-	/// The dispute has been concluded against the candidate
-	/// since the given timestamp.
-	///
-	/// This takes precedence over `ConcludedFor` in the case that
-	/// both are true, which is impossible unless a large amount of
-	/// validators are participating on both sides.
-	#[codec(index = 2)]
-	ConcludedAgainst(Timestamp),
-}
-
-impl DisputeStatus {
-	/// Initialize the status to the active state.
-	pub fn active() -> DisputeStatus {
-		DisputeStatus::Active
-	}
-
-	/// Transition the status to a new status after observing the dispute has concluded for the candidate.
-	/// This may be a no-op if the status was already concluded.
-	pub fn concluded_for(self, now: Timestamp) -> DisputeStatus {
-		match self {
-			DisputeStatus::Active => DisputeStatus::ConcludedFor(now),
-			DisputeStatus::ConcludedFor(at) => DisputeStatus::ConcludedFor(std::cmp::min(at, now)),
-			against => against,
-		}
-	}
-
-	/// Transition the status to a new status after observing the dispute has concluded against the candidate.
-	/// This may be a no-op if the status was already concluded.
-	pub fn concluded_against(self, now: Timestamp) -> DisputeStatus {
-		match self {
-			DisputeStatus::Active => DisputeStatus::ConcludedAgainst(now),
-			DisputeStatus::ConcludedFor(at) =>
-				DisputeStatus::ConcludedAgainst(std::cmp::min(at, now)),
-			DisputeStatus::ConcludedAgainst(at) =>
-				DisputeStatus::ConcludedAgainst(std::cmp::min(at, now)),
-		}
-	}
-
-	/// Whether the disputed candidate is possibly invalid.
-	pub fn is_possibly_invalid(&self) -> bool {
-		match self {
-			DisputeStatus::Active | DisputeStatus::ConcludedAgainst(_) => true,
-			DisputeStatus::ConcludedFor(_) => false,
-		}
-	}
-
-	/// Yields the timestamp this dispute concluded at, if any.
-	pub fn concluded_at(&self) -> Option<Timestamp> {
-		match self {
-			DisputeStatus::Active => None,
-			DisputeStatus::ConcludedFor(at) | DisputeStatus::ConcludedAgainst(at) => Some(*at),
-		}
-	}
-}
-
 async fn run<Context>(subsystem: DisputeCoordinatorSubsystem, mut ctx: Context)
 where
 	Context: overseer::SubsystemContext<Message = DisputeCoordinatorMessage>,
@@ -182,13 +63,10 @@ where
 	loop {
 		let res = run_until_error(&mut ctx, &subsystem).await;
 		match res {
-			Err(e) => {
-				e.trace();
-
-				if let Error::Subsystem(SubsystemError::Context(_)) = e {
+			Err(e) =>
+				if let Error::Fatal(_) = e {
 					break
-				}
-			},
+				},
 			Ok(()) => {
 				tracing::info!(target: LOG_TARGET, "received `Conclude` signal, exiting");
 				break
@@ -197,10 +75,7 @@ where
 	}
 }
 
-async fn run_until_error<Context>(
-	ctx: &mut Context,
-	_: &DisputeCoordinatorSubsystem,
-) -> Result<(), Error>
+async fn run_until_error<Context>(ctx: &mut Context, _: &DisputeCoordinatorSubsystem) -> Result<()>
 where
 	Context: overseer::SubsystemContext<Message = DisputeCoordinatorMessage>,
 	Context: SubsystemContext<Message = DisputeCoordinatorMessage>,
@@ -221,7 +96,7 @@ async fn handle_incoming(
 	_: &mut impl SubsystemContext,
 	_: &mut State,
 	message: DisputeCoordinatorMessage,
-) -> Result<(), Error> {
+) -> Result<()> {
 	match message {
 		DisputeCoordinatorMessage::ImportStatements { .. } => { /* just drop confirmation */ },
 		DisputeCoordinatorMessage::RecentDisputes(tx) => {

node/core/dispute-coordinator/src/real/error.rs → node/core/dispute-coordinator/src/error.rs

@@ -23,8 +23,8 @@ use polkadot_node_subsystem::{
 };
 use polkadot_node_subsystem_util::{rolling_session_window::SessionsUnavailable, runtime};
 
-use super::{db, participation};
-use crate::real::{CodecError, LOG_TARGET};
+use crate::LOG_TARGET;
+use parity_scale_codec::Error as CodecError;
 
 /// Errors for this subsystem.
 #[derive(Debug, Error)]
@@ -126,16 +126,7 @@ pub enum NonFatal {
 	Runtime(#[from] runtime::NonFatal),
 
 	#[error(transparent)]
-	QueueError(#[from] participation::QueueError),
-}
-
-impl From<db::v1::Error> for Error {
-	fn from(err: db::v1::Error) -> Self {
-		match err {
-			db::v1::Error::Io(io) => Self::NonFatal(NonFatal::Io(io)),
-			db::v1::Error::Codec(e) => Self::NonFatal(NonFatal::Codec(e)),
-		}
-	}
+	QueueError(#[from] crate::real::participation::QueueError),
 }
 
 /// Utility for eating top level errors and log them.

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

@@ -25,14 +25,67 @@
 //! another node, this will trigger the dispute participation subsystem to recover and validate the block and call
 //! back to this subsystem.
 
+/// Metrics types.
 mod metrics;
 
-#[cfg(feature = "disputes")]
-mod real;
-#[cfg(feature = "disputes")]
-pub use real::*;
+/// Common error types for this subsystem.
+mod error;
+
+/// Status tracking of disputes (`DisputeStatus`).
+mod status;
 
-#[cfg(not(feature = "disputes"))]
+/// Dummy implementation.
 mod dummy;
-#[cfg(not(feature = "disputes"))]
-pub use dummy::*;
+/// The real implementation.
+mod real;
+
+use kvdb::KeyValueDB;
+use metrics::Metrics;
+use polkadot_node_subsystem::{
+	messages::DisputeCoordinatorMessage, overseer, SpawnedSubsystem, SubsystemContext,
+	SubsystemError,
+};
+use sc_keystore::LocalKeystore;
+use std::sync::Arc;
+
+pub use self::real::Config;
+
+pub(crate) const LOG_TARGET: &str = "parachain::dispute-coordinator";
+
+/// The disputes coordinator subsystem, abstracts `dummy` and `real` implementations.
+pub enum DisputeCoordinatorSubsystem {
+	Dummy(dummy::DisputeCoordinatorSubsystem),
+	Real(real::DisputeCoordinatorSubsystem),
+}
+
+impl DisputeCoordinatorSubsystem {
+	/// Create a new dummy instance.
+	pub fn dummy() -> Self {
+		DisputeCoordinatorSubsystem::Dummy(dummy::DisputeCoordinatorSubsystem::new())
+	}
+
+	/// Create a new instance of the subsystem.
+	pub fn new(
+		store: Arc<dyn KeyValueDB>,
+		config: real::Config,
+		keystore: Arc<LocalKeystore>,
+		metrics: Metrics,
+	) -> Self {
+		DisputeCoordinatorSubsystem::Real(real::DisputeCoordinatorSubsystem::new(
+			store, config, keystore, metrics,
+		))
+	}
+}
+
+impl<Context> overseer::Subsystem<Context, SubsystemError> for DisputeCoordinatorSubsystem
+where
+	Context: SubsystemContext<Message = DisputeCoordinatorMessage>,
+	Context: overseer::SubsystemContext<Message = DisputeCoordinatorMessage>,
+{
+	fn start(self, ctx: Context) -> SpawnedSubsystem {
+		match self {
+			DisputeCoordinatorSubsystem::Dummy(dummy) => dummy.start(ctx),
+			DisputeCoordinatorSubsystem::Real(real) => real.start(ctx),
+		}
+	}
+}