diff --git a/polkadot/.gitlab-ci.yml b/polkadot/.gitlab-ci.yml index 728c48c933..5c9343cdbe 100644 --- a/polkadot/.gitlab-ci.yml +++ b/polkadot/.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 diff --git a/polkadot/Cargo.toml b/polkadot/Cargo.toml index a88c72f7ca..db2d4093b2 100644 --- a/polkadot/Cargo.toml +++ b/polkadot/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" diff --git a/polkadot/bridges/bin/rialto/node/src/overseer.rs b/polkadot/bridges/bin/rialto/node/src/overseer.rs index 488a664904..517f83f206 100644 --- a/polkadot/bridges/bin/rialto/node/src/overseer.rs +++ b/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( 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)?, diff --git a/polkadot/bridges/bin/rialto/node/src/service.rs b/polkadot/bridges/bin/rialto/node/src/service.rs index 06830e7080..549d00c5af 100644 --- a/polkadot/bridges/bin/rialto/node/src/service.rs +++ b/polkadot/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); diff --git a/polkadot/cli/Cargo.toml b/polkadot/cli/Cargo.toml index 11848c8e00..4c133de6fe 100644 --- a/polkadot/cli/Cargo.toml +++ b/polkadot/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"] diff --git a/polkadot/node/core/dispute-coordinator/src/dummy.rs b/polkadot/node/core/dispute-coordinator/src/dummy.rs index d151af60e9..4f56a0fb9b 100644 --- a/polkadot/node/core/dispute-coordinator/src/dummy.rs +++ b/polkadot/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, _: Config, _: Arc, _: 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 { - match self { - DisputeStatus::Active => None, - DisputeStatus::ConcludedFor(at) | DisputeStatus::ConcludedAgainst(at) => Some(*at), - } - } -} - async fn run(subsystem: DisputeCoordinatorSubsystem, mut ctx: Context) where Context: overseer::SubsystemContext, @@ -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( - ctx: &mut Context, - _: &DisputeCoordinatorSubsystem, -) -> Result<(), Error> +async fn run_until_error(ctx: &mut Context, _: &DisputeCoordinatorSubsystem) -> Result<()> where Context: overseer::SubsystemContext, Context: SubsystemContext, @@ -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) => { diff --git a/polkadot/node/core/dispute-coordinator/src/real/error.rs b/polkadot/node/core/dispute-coordinator/src/error.rs similarity index 92% rename from polkadot/node/core/dispute-coordinator/src/real/error.rs rename to polkadot/node/core/dispute-coordinator/src/error.rs index 0a1b47fcc4..ad7115db45 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/error.rs +++ b/polkadot/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 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. diff --git a/polkadot/node/core/dispute-coordinator/src/lib.rs b/polkadot/node/core/dispute-coordinator/src/lib.rs index 38e4f5d66d..6b12ab8fd1 100644 --- a/polkadot/node/core/dispute-coordinator/src/lib.rs +++ b/polkadot/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; -#[cfg(not(feature = "disputes"))] +/// Status tracking of disputes (`DisputeStatus`). +mod status; + +/// 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, + config: real::Config, + keystore: Arc, + metrics: Metrics, + ) -> Self { + DisputeCoordinatorSubsystem::Real(real::DisputeCoordinatorSubsystem::new( + store, config, keystore, metrics, + )) + } +} + +impl overseer::Subsystem for DisputeCoordinatorSubsystem +where + Context: SubsystemContext, + Context: overseer::SubsystemContext, +{ + fn start(self, ctx: Context) -> SpawnedSubsystem { + match self { + DisputeCoordinatorSubsystem::Dummy(dummy) => dummy.start(ctx), + DisputeCoordinatorSubsystem::Real(real) => real.start(ctx), + } + } +} diff --git a/polkadot/node/core/dispute-coordinator/src/metrics.rs b/polkadot/node/core/dispute-coordinator/src/metrics.rs index a683c15329..83810df67a 100644 --- a/polkadot/node/core/dispute-coordinator/src/metrics.rs +++ b/polkadot/node/core/dispute-coordinator/src/metrics.rs @@ -19,16 +19,12 @@ use polkadot_node_subsystem_util::metrics::{self, prometheus}; #[derive(Clone)] struct MetricsInner { /// Number of opened disputes. - #[cfg(feature = "disputes")] open: prometheus::Counter, /// Votes of all disputes. - #[cfg(feature = "disputes")] votes: prometheus::CounterVec, /// Conclusion across all disputes. - #[cfg(feature = "disputes")] concluded: prometheus::CounterVec, /// Number of participations that have been queued. - #[cfg(feature = "disputes")] queued_participations: prometheus::CounterVec, } @@ -36,7 +32,6 @@ struct MetricsInner { #[derive(Default, Clone)] pub struct Metrics(Option); -#[cfg(feature = "disputes")] impl Metrics { pub(crate) fn on_open(&self) { if let Some(metrics) = &self.0 { @@ -82,13 +77,6 @@ impl Metrics { } impl metrics::Metrics for Metrics { - #[cfg(not(feature = "disputes"))] - fn try_register(_registry: &prometheus::Registry) -> Result { - let metrics = MetricsInner {}; - Ok(Metrics(Some(metrics))) - } - - #[cfg(feature = "disputes")] fn try_register(registry: &prometheus::Registry) -> Result { let metrics = MetricsInner { open: prometheus::register( diff --git a/polkadot/node/core/dispute-coordinator/src/real/backend.rs b/polkadot/node/core/dispute-coordinator/src/real/backend.rs index 71d2dd35cc..469e9fb018 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/backend.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/backend.rs @@ -26,10 +26,8 @@ use polkadot_primitives::v1::{CandidateHash, SessionIndex}; use std::collections::HashMap; -use super::{ - db::v1::{CandidateVotes, RecentDisputes}, - error::FatalResult, -}; +use super::db::v1::{CandidateVotes, RecentDisputes}; +use crate::error::FatalResult; #[derive(Debug)] pub enum BackendWriteOp { diff --git a/polkadot/node/core/dispute-coordinator/src/real/db/v1.rs b/polkadot/node/core/dispute-coordinator/src/real/db/v1.rs index b7035d593f..05d58e88f2 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/db/v1.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/db/v1.rs @@ -27,10 +27,13 @@ use std::sync::Arc; use kvdb::{DBTransaction, KeyValueDB}; use parity_scale_codec::{Decode, Encode}; -use crate::real::{ - backend::{Backend, BackendWriteOp, OverlayedBackend}, +use crate::{ error::{Fatal, FatalResult}, status::DisputeStatus, +}; + +use crate::real::{ + backend::{Backend, BackendWriteOp, OverlayedBackend}, DISPUTE_WINDOW, }; @@ -162,6 +165,15 @@ pub enum Error { Codec(#[from] parity_scale_codec::Error), } +impl From for crate::error::Error { + fn from(err: Error) -> Self { + match err { + Error::Io(io) => Self::NonFatal(crate::error::NonFatal::Io(io)), + Error::Codec(e) => Self::NonFatal(crate::error::NonFatal::Codec(e)), + } + } +} + /// Result alias for DB errors. pub type Result = std::result::Result; diff --git a/polkadot/node/core/dispute-coordinator/src/real/initialized.rs b/polkadot/node/core/dispute-coordinator/src/real/initialized.rs index f3c99a86d0..67e1400120 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/initialized.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/initialized.rs @@ -45,19 +45,22 @@ use polkadot_primitives::v1::{ ValidDisputeStatementKind, ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature, }; -use crate::{metrics::Metrics, DisputeCoordinatorSubsystem}; +use crate::{metrics::Metrics, real::DisputeCoordinatorSubsystem, LOG_TARGET}; + +use crate::{ + error::{log_error, Fatal, FatalResult, NonFatal, NonFatalResult, Result}, + status::{get_active_with_status, Clock, DisputeStatus, Timestamp}, +}; use super::{ backend::Backend, db, - error::{log_error, Fatal, FatalResult, NonFatal, NonFatalResult, Result}, ordering::{CandidateComparator, OrderingProvider}, participation::{ self, Participation, ParticipationRequest, ParticipationStatement, WorkerMessageReceiver, }, spam_slots::SpamSlots, - status::{get_active_with_status, Clock, DisputeStatus, Timestamp}, - OverlayedBackend, LOG_TARGET, + OverlayedBackend, }; /// After the first active leaves update we transition to `Initialized` state. diff --git a/polkadot/node/core/dispute-coordinator/src/real/mod.rs b/polkadot/node/core/dispute-coordinator/src/real/mod.rs index 551d4cedae..345735ed39 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/mod.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/mod.rs @@ -28,7 +28,6 @@ use std::{collections::HashSet, sync::Arc}; use futures::FutureExt; use kvdb::KeyValueDB; -use parity_scale_codec::Error as CodecError; use sc_keystore::LocalKeystore; @@ -40,24 +39,23 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::rolling_session_window::RollingSessionWindow; use polkadot_primitives::v1::{ValidatorIndex, ValidatorPair}; -use crate::metrics::Metrics; -use backend::{Backend, OverlayedBackend}; -use db::v1::DbBackend; -use error::{FatalResult, Result}; - -use self::{ - error::{Error, NonFatal}, - ordering::CandidateComparator, - participation::ParticipationRequest, - spam_slots::{SpamSlots, UnconfirmedDisputes}, +use crate::{ + error::{Error, FatalResult, NonFatal, Result}, + metrics::Metrics, status::{get_active_with_status, SystemClock}, }; -mod backend; -mod db; +use backend::{Backend, OverlayedBackend}; +use db::v1::DbBackend; -/// Common error types for this subsystem. -mod error; +use self::{ + ordering::CandidateComparator, + participation::ParticipationRequest, + spam_slots::{SpamSlots, UnconfirmedDisputes}, +}; + +pub(crate) mod backend; +pub(crate) mod db; /// Subsystem after receiving the first active leaf. mod initialized; @@ -90,17 +88,15 @@ mod spam_slots; /// participation requests, such that most important/urgent disputes will be resolved and processed /// first and more importantly it will order requests in a way so disputes will get resolved, even /// if there are lots of them. -mod participation; +pub(crate) mod participation; -/// Status tracking of disputes (`DisputeStatus`). -mod status; -use status::Clock; +use crate::status::Clock; + +use crate::LOG_TARGET; #[cfg(test)] mod tests; -const LOG_TARGET: &str = "parachain::dispute-coordinator"; - /// An implementation of the dispute coordinator subsystem. pub struct DisputeCoordinatorSubsystem { config: Config, diff --git a/polkadot/node/core/dispute-coordinator/src/real/ordering/mod.rs b/polkadot/node/core/dispute-coordinator/src/real/ordering/mod.rs index bfd6298ddf..b8eea21134 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/ordering/mod.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/ordering/mod.rs @@ -28,7 +28,7 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::runtime::get_candidate_events; use polkadot_primitives::v1::{BlockNumber, CandidateEvent, CandidateHash, CandidateReceipt, Hash}; -use super::{ +use crate::{ error::{Fatal, FatalResult, Result}, LOG_TARGET, }; diff --git a/polkadot/node/core/dispute-coordinator/src/real/participation/mod.rs b/polkadot/node/core/dispute-coordinator/src/real/participation/mod.rs index dcea701de0..f2f3862ab5 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/participation/mod.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/participation/mod.rs @@ -29,13 +29,13 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::runtime::get_validation_code_by_hash; use polkadot_primitives::v1::{BlockNumber, CandidateHash, CandidateReceipt, Hash, SessionIndex}; -use crate::real::LOG_TARGET; - -use super::{ +use crate::{ error::{Fatal, FatalResult, NonFatal, Result}, - ordering::CandidateComparator, + LOG_TARGET, }; +use super::ordering::CandidateComparator; + #[cfg(test)] mod tests; #[cfg(test)] diff --git a/polkadot/node/core/dispute-coordinator/src/real/tests.rs b/polkadot/node/core/dispute-coordinator/src/real/tests.rs index 62f81db2a8..cf264572e2 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/tests.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/tests.rs @@ -65,15 +65,12 @@ use crate::{ real::{ backend::Backend, participation::{participation_full_happy_path, participation_missing_availability}, - status::ACTIVE_DURATION_SECS, + Config, DisputeCoordinatorSubsystem, }, - Config, DisputeCoordinatorSubsystem, + status::{Clock, Timestamp, ACTIVE_DURATION_SECS}, }; -use super::{ - db::v1::DbBackend, - status::{Clock, Timestamp}, -}; +use super::db::v1::DbBackend; const TEST_TIMEOUT: Duration = Duration::from_secs(2); diff --git a/polkadot/node/core/dispute-coordinator/src/real/status.rs b/polkadot/node/core/dispute-coordinator/src/status.rs similarity index 99% rename from polkadot/node/core/dispute-coordinator/src/real/status.rs rename to polkadot/node/core/dispute-coordinator/src/status.rs index c0b01e9b96..5be1e7b390 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/status.rs +++ b/polkadot/node/core/dispute-coordinator/src/status.rs @@ -19,7 +19,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use parity_scale_codec::{Decode, Encode}; use polkadot_primitives::v1::{CandidateHash, SessionIndex}; -use crate::real::LOG_TARGET; +use crate::LOG_TARGET; /// The choice here is fairly arbitrary. But any dispute that concluded more than a few minutes ago /// is not worth considering anymore. Changing this value has little to no bearing on consensus, diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs index 2b6dfb564d..f9375f094c 100644 --- a/polkadot/node/core/provisioner/src/lib.rs +++ b/polkadot/node/core/provisioner/src/lib.rs @@ -148,10 +148,19 @@ pub enum Error { BackedCandidateOrderingProblem, } +/// Provisioner run arguments. +#[derive(Debug, Clone, Copy)] +pub struct ProvisionerConfig { + /// If enabled, dispute votes will be provided to `fn create_inherent`, otherwise not. + /// Long term we will obviously always want disputes to be enabled, this option exists for testing purposes + /// and will be removed in the near future. + pub disputes_enabled: bool, +} + impl JobTrait for ProvisionerJob { type ToJob = ProvisionerMessage; type Error = Error; - type RunArgs = (); + type RunArgs = ProvisionerConfig; type Metrics = Metrics; const NAME: &'static str = "provisioner-job"; @@ -161,17 +170,21 @@ impl JobTrait for ProvisionerJob { // this function is in charge of creating and executing the job's main loop fn run( leaf: ActivatedLeaf, - _run_args: Self::RunArgs, + run_args: Self::RunArgs, metrics: Self::Metrics, receiver: mpsc::Receiver, mut sender: JobSender, ) -> Pin> + Send>> { + let span = leaf.span.clone(); async move { - let span = leaf.span.clone(); let job = ProvisionerJob::new(leaf, metrics, receiver); - job.run_loop(sender.subsystem_sender(), PerLeafSpan::new(span, "provisioner")) - .await + job.run_loop( + sender.subsystem_sender(), + run_args.disputes_enabled, + PerLeafSpan::new(span, "provisioner"), + ) + .await } .boxed() } @@ -197,6 +210,7 @@ impl ProvisionerJob { async fn run_loop( mut self, sender: &mut impl SubsystemSender, + disputes_enabled: bool, span: PerLeafSpan, ) -> Result<(), Error> { use ProvisionerMessage::{ProvisionableData, RequestInherentData}; @@ -208,7 +222,7 @@ impl ProvisionerJob { let _timer = self.metrics.time_request_inherent_data(); if self.inherent_after.is_ready() { - self.send_inherent_data(sender, vec![return_sender]).await; + self.send_inherent_data(sender, vec![return_sender], disputes_enabled).await; } else { self.awaiting_inherent.push(return_sender); } @@ -225,7 +239,7 @@ impl ProvisionerJob { let _span = span.child("send-inherent-data"); let return_senders = std::mem::take(&mut self.awaiting_inherent); if !return_senders.is_empty() { - self.send_inherent_data(sender, return_senders).await; + self.send_inherent_data(sender, return_senders, disputes_enabled).await; } } } @@ -238,6 +252,7 @@ impl ProvisionerJob { &mut self, sender: &mut impl SubsystemSender, return_senders: Vec>, + disputes_enabled: bool, ) { if let Err(err) = send_inherent_data( &self.leaf, @@ -245,6 +260,7 @@ impl ProvisionerJob { &self.backed_candidates, return_senders, sender, + disputes_enabled, &self.metrics, ) .await @@ -300,6 +316,7 @@ async fn send_inherent_data( candidates: &[CandidateReceipt], return_senders: Vec>, from_job: &mut impl SubsystemSender, + disputes_enabled: bool, metrics: &Metrics, ) -> Result<(), Error> { let availability_cores = request_availability_cores(leaf.hash, from_job) @@ -307,7 +324,9 @@ async fn send_inherent_data( .await .map_err(|err| Error::CanceledAvailabilityCores(err))??; - let disputes = select_disputes(from_job, metrics).await?; + let disputes = + if disputes_enabled { select_disputes(from_job, metrics).await? } else { vec![] }; + // Only include bitfields on fresh leaves. On chain reversions, we want to make sure that // there will be at least one block, which cannot get disputed, so the chain can make progress. let bitfields = match leaf.status { diff --git a/polkadot/node/malus/Cargo.toml b/polkadot/node/malus/Cargo.toml index 7225846e31..29549d1722 100644 --- a/polkadot/node/malus/Cargo.toml +++ b/polkadot/node/malus/Cargo.toml @@ -34,8 +34,7 @@ futures-timer = "3.0.2" tracing = "0.1.26" [features] -default = [] # we do not enable disputes by default to avoid feature leak -disputes = ["polkadot-cli/disputes"] +default = [] [dev-dependencies] polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } diff --git a/polkadot/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs b/polkadot/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs index c631eef369..a5c8c3a4a5 100644 --- a/polkadot/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs +++ b/polkadot/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs @@ -202,7 +202,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { Fn(SubsystemInitFn), /// Directly initialize the subsystem with the given subsystem type `T`. Value(T), - /// Subsystem field does not have value just yet. + /// Subsystem field does not have a value just yet. Uninitialized } diff --git a/polkadot/node/service/Cargo.toml b/polkadot/node/service/Cargo.toml index 2c27cdbae1..cab30c9682 100644 --- a/polkadot/node/service/Cargo.toml +++ b/polkadot/node/service/Cargo.toml @@ -178,12 +178,11 @@ try-runtime = [ "rococo-runtime/try-runtime", ] malus = ["full-node"] -disputes = ["polkadot-node-core-dispute-coordinator/disputes"] runtime-metrics = [ - "polkadot-client/runtime-metrics", + "polkadot-client/runtime-metrics", "rococo-runtime/runtime-metrics", "westend-runtime/runtime-metrics", "kusama-runtime/runtime-metrics", "polkadot-runtime/runtime-metrics", "polkadot-runtime-parachains/runtime-metrics" -] \ No newline at end of file +] diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs index 4252ff7360..4cb9ad13df 100644 --- a/polkadot/node/service/src/lib.rs +++ b/polkadot/node/service/src/lib.rs @@ -29,7 +29,7 @@ pub mod overseer; #[cfg(feature = "full-node")] pub use self::overseer::{OverseerGen, OverseerGenArgs, RealOverseerGen}; -#[cfg(all(test, feature = "disputes"))] +#[cfg(test)] mod tests; #[cfg(feature = "full-node")] @@ -721,6 +721,12 @@ where let auth_or_collator = role.is_authority() || is_collator.is_collator(); let requires_overseer_for_chain_sel = local_keystore.is_some() && auth_or_collator; + let disputes_enabled = chain_spec.is_rococo() || + chain_spec.is_kusama() || + chain_spec.is_westend() || + chain_spec.is_versi() || + chain_spec.is_wococo(); + let select_chain = if requires_overseer_for_chain_sel { let metrics = polkadot_node_subsystem_util::metrics::Metrics::register(prometheus_registry.as_ref())?; @@ -729,6 +735,7 @@ where basics.backend.clone(), overseer_handle.clone(), metrics, + disputes_enabled, ) } else { SelectRelayChain::new_longest_chain(basics.backend.clone()) @@ -952,6 +959,7 @@ where candidate_validation_config, chain_selection_config, dispute_coordinator_config, + disputes_enabled, }, ) .map_err(|e| { diff --git a/polkadot/node/service/src/overseer.rs b/polkadot/node/service/src/overseer.rs index 2441c688ba..0d42673f21 100644 --- a/polkadot/node/service/src/overseer.rs +++ b/polkadot/node/service/src/overseer.rs @@ -22,6 +22,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}; #[cfg(any(feature = "malus", test))] pub use polkadot_overseer::{ @@ -106,6 +107,8 @@ where pub chain_selection_config: ChainSelectionConfig, /// Configuration for the dispute coordinator subsystem. pub dispute_coordinator_config: DisputeCoordinatorConfig, + /// Enable to disputes. + pub disputes_enabled: bool, } /// Obtain a prepared `OverseerBuilder`, that is initialized @@ -132,6 +135,7 @@ pub fn prepared_overseer_builder<'a, Spawner, RuntimeClient>( candidate_validation_config, chain_selection_config, dispute_coordinator_config, + disputes_enabled, }: OverseerGenArgs<'a, Spawner, RuntimeClient>, ) -> Result< OverseerBuilder< @@ -228,7 +232,11 @@ 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)?, @@ -251,12 +259,16 @@ where keystore.clone(), authority_discovery_service.clone(), )) - .dispute_coordinator(DisputeCoordinatorSubsystem::new( - parachains_db.clone(), - dispute_coordinator_config, - keystore.clone(), - Metrics::register(registry)?, - )) + .dispute_coordinator(if disputes_enabled { + DisputeCoordinatorSubsystem::new( + parachains_db.clone(), + dispute_coordinator_config, + keystore.clone(), + Metrics::register(registry)?, + ) + } else { + DisputeCoordinatorSubsystem::dummy() + }) .dispute_distribution(DisputeDistributionSubsystem::new( keystore.clone(), dispute_req_receiver, @@ -319,8 +331,15 @@ impl OverseerGen for RealOverseerGen { RuntimeClient::Api: ParachainHost + BabeApi + AuthorityDiscoveryApi, Spawner: 'static + SpawnNamed + Clone + Unpin, { - prepared_overseer_builder(args)? - .build_with_connector(connector) - .map_err(|e| e.into()) + let disputes_enabled = args.disputes_enabled; + let builder = prepared_overseer_builder(args)?; + if disputes_enabled { + builder + .dispute_coordinator(DisputeCoordinatorSubsystem::dummy()) + .build_with_connector(connector) + } else { + builder.build_with_connector(connector) + } + .map_err(|e| e.into()) } } diff --git a/polkadot/node/service/src/relay_chain_selection.rs b/polkadot/node/service/src/relay_chain_selection.rs index e9a67f2336..e0bdda348d 100644 --- a/polkadot/node/service/src/relay_chain_selection.rs +++ b/polkadot/node/service/src/relay_chain_selection.rs @@ -161,11 +161,16 @@ where /// Create a new [`SelectRelayChain`] wrapping the given chain backend /// and a handle to the overseer. - pub fn new_disputes_aware(backend: Arc, overseer: Handle, metrics: Metrics) -> Self { + pub fn new_disputes_aware( + backend: Arc, + overseer: Handle, + metrics: Metrics, + disputes_enabled: bool, + ) -> Self { tracing::debug!( target: LOG_TARGET, "Using {} chain selection algorithm", - if cfg!(feature = "disputes") { + if disputes_enabled { "dispute aware relay" } else { // no disputes are queried, that logic is disabled @@ -176,7 +181,10 @@ where SelectRelayChain { longest_chain: sc_consensus::LongestChain::new(backend.clone()), selection: IsDisputesAwareWithOverseer::Yes(SelectRelayChainInner::new( - backend, overseer, metrics, + backend, + overseer, + metrics, + disputes_enabled, )), } } @@ -233,6 +241,7 @@ where pub struct SelectRelayChainInner { backend: Arc, overseer: OH, + disputes_enabled: bool, metrics: Metrics, } @@ -243,8 +252,8 @@ where { /// Create a new [`SelectRelayChainInner`] wrapping the given chain backend /// and a handle to the overseer. - pub fn new(backend: Arc, overseer: OH, metrics: Metrics) -> Self { - SelectRelayChainInner { backend, overseer, metrics } + pub fn new(backend: Arc, overseer: OH, metrics: Metrics, disputes_enabled: bool) -> Self { + SelectRelayChainInner { backend, overseer, metrics, disputes_enabled } } fn block_header(&self, hash: Hash) -> Result { @@ -282,6 +291,7 @@ where backend: self.backend.clone(), overseer: self.overseer.clone(), metrics: self.metrics.clone(), + disputes_enabled: self.disputes_enabled, } } } @@ -371,7 +381,7 @@ where let mut overseer = self.overseer.clone(); tracing::trace!(target: LOG_TARGET, ?best_leaf, "Longest chain"); - let subchain_head = if cfg!(feature = "disputes") { + let subchain_head = if self.disputes_enabled { let (tx, rx) = oneshot::channel(); overseer .send_msg( @@ -476,7 +486,7 @@ where let lag = initial_leaf_number.saturating_sub(subchain_number); self.metrics.note_approval_checking_finality_lag(lag); - let (lag, subchain_head) = if cfg!(feature = "disputes") { + let (lag, subchain_head) = if self.disputes_enabled { // Prevent sending flawed data to the dispute-coordinator. if Some(subchain_block_descriptions.len() as _) != subchain_number.checked_sub(target_number) diff --git a/polkadot/node/service/src/tests.rs b/polkadot/node/service/src/tests.rs index 22eeeacb05..cb96a5b0eb 100644 --- a/polkadot/node/service/src/tests.rs +++ b/polkadot/node/service/src/tests.rs @@ -83,6 +83,7 @@ fn test_harness>( Arc::new(case_vars.chain.clone()), context.sender().clone(), Default::default(), + true, ); let target_hash = case_vars.target_block.clone(); diff --git a/polkadot/scripts/gitlab/test_linux_stable.sh b/polkadot/scripts/gitlab/test_linux_stable.sh index 6234cdbf6e..7f11e08d9e 100755 --- a/polkadot/scripts/gitlab/test_linux_stable.sh +++ b/polkadot/scripts/gitlab/test_linux_stable.sh @@ -4,6 +4,5 @@ set -eux #shellcheck source=../common/lib.sh source "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )/../common/lib.sh" -time cargo test --release --locked -p polkadot-node-core-dispute-coordinator --features disputes # Builds with the runtime benchmarks/metrics features are only to be used for testing. time cargo test --workspace --release --verbose --locked --features=runtime-benchmarks,runtime-metrics