Unverified Commit d488955d authored by Robert Klotzner's avatar Robert Klotzner Committed by GitHub
Browse files

Dispute spam protection (#4134)



* Mostly notes.

* Better error messages.

* Introduce Fatal/NonFatal + drop back channel participation

- Fatal/NonFatal - in order to make it easier to use utility functions.
- We drop the back channel in dispute participation as it won't be
needed any more.

* Better error messages.

* Utility function for receiving `CandidateEvent`s.

* Ordering module typechecks.

* cargo fmt

* Prepare spam slots module.

* Implement SpamSlots mechanism.

* Implement queues.

* cargo fmt

* Participation.

* Participation taking shape.

* Finish participation.

* cargo fmt

* Cleanup.

* WIP: Cleanup + Integration.

* Make `RollingSessionWindow` initialized by default.

* Make approval voting typecheck.

* Get rid of lazy_static & fix approval voting tests

* Move `SessionWindowSize` to node primitives.

* Implement dispute coordinator initialization.

* cargo fmt

* Make queues return error instead of boolean.

* Initialized: WIP

* Introduce chain api for getting finalized block.

* Fix ordering to only prune candidates on finalized events.

* Pruning of old sessions in spam slots.

* New import logic.

* Make everything typecheck.

* Fix warnings.

* Get rid of obsolete dispute-participation.

* Fixes.

* Add back accidentelly deleted Cargo.lock

* Deliver disputes in an ordered fashion.

* Add module docs for errors

* Use type synonym.

* hidden docs.

* Fix overseer tests.

* Ordering provider taking `CandidateReceipt`.

... To be kicked on one next commit.

* Fix ordering to use relay_parent

as included block is not unique per candidate.

* Add comment in ordering.rs.

* Take care of duplicate entries in queues.

* Better spam slots.

* Review remarks + docs.

* Fix db tests.

* Participation tests.

* Also scrape votes on first leaf for good measure.

* Make tests typecheck.

* Spelling.

* Only participate in actual disputes, not on every import.

* Don't account backing votes to spam slots.

* Fix more tests.

* Don't participate if we don't have keys.

* Fix tests, typos and warnings.

* Fix merge error.

* Spelling fixes.

* Add missing docs.

* Queue tests.

* More tests.

* Add metrics + don't short circuit import.

* Basic test for ordering provider.

* Import fix.

* Remove dead link.

* One more dead link.
Co-authored-by: Lldenaurois's avatarLldenaurois <Ljdenaurois@gmail.com>
parent 9f059fb1
Pipeline #167012 canceled with stages
in 15 minutes and 9 seconds
...@@ -6368,22 +6368,6 @@ dependencies = [ ...@@ -6368,22 +6368,6 @@ dependencies = [
"tracing", "tracing",
] ]
[[package]]
name = "polkadot-node-core-dispute-participation"
version = "0.9.13"
dependencies = [
"assert_matches",
"futures 0.3.17",
"parity-scale-codec",
"polkadot-node-primitives",
"polkadot-node-subsystem",
"polkadot-node-subsystem-test-helpers",
"polkadot-primitives",
"sp-core",
"thiserror",
"tracing",
]
[[package]] [[package]]
name = "polkadot-node-core-parachains-inherent" name = "polkadot-node-core-parachains-inherent"
version = "0.9.13" version = "0.9.13"
...@@ -6591,6 +6575,7 @@ dependencies = [ ...@@ -6591,6 +6575,7 @@ dependencies = [
"env_logger 0.9.0", "env_logger 0.9.0",
"futures 0.3.17", "futures 0.3.17",
"itertools", "itertools",
"lazy_static",
"log", "log",
"lru 0.7.0", "lru 0.7.0",
"metered-channel", "metered-channel",
...@@ -6599,6 +6584,7 @@ dependencies = [ ...@@ -6599,6 +6584,7 @@ dependencies = [
"polkadot-node-jaeger", "polkadot-node-jaeger",
"polkadot-node-metrics", "polkadot-node-metrics",
"polkadot-node-network-protocol", "polkadot-node-network-protocol",
"polkadot-node-primitives",
"polkadot-node-subsystem", "polkadot-node-subsystem",
"polkadot-node-subsystem-test-helpers", "polkadot-node-subsystem-test-helpers",
"polkadot-overseer", "polkadot-overseer",
...@@ -6955,7 +6941,6 @@ dependencies = [ ...@@ -6955,7 +6941,6 @@ dependencies = [
"polkadot-node-core-chain-api", "polkadot-node-core-chain-api",
"polkadot-node-core-chain-selection", "polkadot-node-core-chain-selection",
"polkadot-node-core-dispute-coordinator", "polkadot-node-core-dispute-coordinator",
"polkadot-node-core-dispute-participation",
"polkadot-node-core-parachains-inherent", "polkadot-node-core-parachains-inherent",
"polkadot-node-core-provisioner", "polkadot-node-core-provisioner",
"polkadot-node-core-runtime-api", "polkadot-node-core-runtime-api",
......
...@@ -56,7 +56,6 @@ members = [ ...@@ -56,7 +56,6 @@ members = [
"node/core/chain-api", "node/core/chain-api",
"node/core/chain-selection", "node/core/chain-selection",
"node/core/dispute-coordinator", "node/core/dispute-coordinator",
"node/core/dispute-participation",
"node/core/parachains-inherent", "node/core/parachains-inherent",
"node/core/provisioner", "node/core/provisioner",
"node/core/pvf", "node/core/pvf",
......
...@@ -76,7 +76,7 @@ struct ImportedBlockInfo { ...@@ -76,7 +76,7 @@ struct ImportedBlockInfo {
} }
struct ImportedBlockInfoEnv<'a> { struct ImportedBlockInfoEnv<'a> {
session_window: &'a RollingSessionWindow, session_window: &'a Option<RollingSessionWindow>,
assignment_criteria: &'a (dyn AssignmentCriteria + Send + Sync), assignment_criteria: &'a (dyn AssignmentCriteria + Send + Sync),
keystore: &'a LocalKeystore, keystore: &'a LocalKeystore,
} }
...@@ -133,7 +133,11 @@ async fn imported_block_info( ...@@ -133,7 +133,11 @@ async fn imported_block_info(
Err(_) => return Ok(None), Err(_) => return Ok(None),
}; };
if env.session_window.earliest_session().map_or(true, |e| session_index < e) { if env
.session_window
.as_ref()
.map_or(true, |s| session_index < s.earliest_session())
{
tracing::debug!( tracing::debug!(
target: LOG_TARGET, target: LOG_TARGET,
"Block {} is from ancient session {}. Skipping", "Block {} is from ancient session {}. Skipping",
...@@ -180,7 +184,8 @@ async fn imported_block_info( ...@@ -180,7 +184,8 @@ async fn imported_block_info(
} }
}; };
let session_info = match env.session_window.session_info(session_index) { let session_info = match env.session_window.as_ref().and_then(|s| s.session_info(session_index))
{
Some(s) => s, Some(s) => s,
None => { None => {
tracing::debug!( tracing::debug!(
...@@ -324,7 +329,7 @@ pub(crate) async fn handle_new_head( ...@@ -324,7 +329,7 @@ pub(crate) async fn handle_new_head(
} }
}; };
match state.session_window.cache_session_info_for_head(ctx, head).await { match state.cache_session_info_for_head(ctx, head).await {
Err(e) => { Err(e) => {
tracing::debug!( tracing::debug!(
target: LOG_TARGET, target: LOG_TARGET,
...@@ -335,7 +340,7 @@ pub(crate) async fn handle_new_head( ...@@ -335,7 +340,7 @@ pub(crate) async fn handle_new_head(
return Ok(Vec::new()) return Ok(Vec::new())
}, },
Ok(a @ SessionWindowUpdate::Advanced { .. }) => { Ok(Some(a @ SessionWindowUpdate::Advanced { .. })) => {
tracing::info!( tracing::info!(
target: LOG_TARGET, target: LOG_TARGET,
update = ?a, update = ?a,
...@@ -431,8 +436,9 @@ pub(crate) async fn handle_new_head( ...@@ -431,8 +436,9 @@ pub(crate) async fn handle_new_head(
let session_info = state let session_info = state
.session_window .session_window
.session_info(session_index) .as_ref()
.expect("imported_block_info requires session to be available; qed"); .and_then(|s| s.session_info(session_index))
.expect("imported_block_info requires session info to be available; qed");
let (block_tick, no_show_duration) = { let (block_tick, no_show_duration) = {
let block_tick = slot_number_to_tick(state.slot_duration_millis, slot); let block_tick = slot_number_to_tick(state.slot_duration_millis, slot);
...@@ -608,7 +614,7 @@ pub(crate) mod tests { ...@@ -608,7 +614,7 @@ pub(crate) mod tests {
fn blank_state() -> State { fn blank_state() -> State {
State { State {
session_window: RollingSessionWindow::new(APPROVAL_SESSIONS), session_window: None,
keystore: Arc::new(LocalKeystore::in_memory()), keystore: Arc::new(LocalKeystore::in_memory()),
slot_duration_millis: 6_000, slot_duration_millis: 6_000,
clock: Box::new(MockClock::default()), clock: Box::new(MockClock::default()),
...@@ -618,11 +624,11 @@ pub(crate) mod tests { ...@@ -618,11 +624,11 @@ pub(crate) mod tests {
fn single_session_state(index: SessionIndex, info: SessionInfo) -> State { fn single_session_state(index: SessionIndex, info: SessionInfo) -> State {
State { State {
session_window: RollingSessionWindow::with_session_info( session_window: Some(RollingSessionWindow::with_session_info(
APPROVAL_SESSIONS, APPROVAL_SESSIONS,
index, index,
vec![info], vec![info],
), )),
..blank_state() ..blank_state()
} }
} }
...@@ -740,7 +746,7 @@ pub(crate) mod tests { ...@@ -740,7 +746,7 @@ pub(crate) mod tests {
let header = header.clone(); let header = header.clone();
Box::pin(async move { Box::pin(async move {
let env = ImportedBlockInfoEnv { let env = ImportedBlockInfoEnv {
session_window: &session_window, session_window: &Some(session_window),
assignment_criteria: &MockAssignmentCriteria, assignment_criteria: &MockAssignmentCriteria,
keystore: &LocalKeystore::in_memory(), keystore: &LocalKeystore::in_memory(),
}; };
...@@ -849,7 +855,7 @@ pub(crate) mod tests { ...@@ -849,7 +855,7 @@ pub(crate) mod tests {
let header = header.clone(); let header = header.clone();
Box::pin(async move { Box::pin(async move {
let env = ImportedBlockInfoEnv { let env = ImportedBlockInfoEnv {
session_window: &session_window, session_window: &Some(session_window),
assignment_criteria: &MockAssignmentCriteria, assignment_criteria: &MockAssignmentCriteria,
keystore: &LocalKeystore::in_memory(), keystore: &LocalKeystore::in_memory(),
}; };
...@@ -942,7 +948,7 @@ pub(crate) mod tests { ...@@ -942,7 +948,7 @@ pub(crate) mod tests {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let test_fut = { let test_fut = {
let session_window = RollingSessionWindow::new(APPROVAL_SESSIONS); let session_window = None;
let header = header.clone(); let header = header.clone();
Box::pin(async move { Box::pin(async move {
...@@ -1037,11 +1043,11 @@ pub(crate) mod tests { ...@@ -1037,11 +1043,11 @@ pub(crate) mod tests {
.map(|(r, c, g)| (r.hash(), r.clone(), *c, *g)) .map(|(r, c, g)| (r.hash(), r.clone(), *c, *g))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let session_window = RollingSessionWindow::with_session_info( let session_window = Some(RollingSessionWindow::with_session_info(
APPROVAL_SESSIONS, APPROVAL_SESSIONS,
session, session,
vec![session_info], vec![session_info],
); ));
let header = header.clone(); let header = header.clone();
Box::pin(async move { Box::pin(async move {
......
...@@ -44,7 +44,10 @@ use polkadot_node_subsystem::{ ...@@ -44,7 +44,10 @@ use polkadot_node_subsystem::{
}; };
use polkadot_node_subsystem_util::{ use polkadot_node_subsystem_util::{
metrics::{self, prometheus}, metrics::{self, prometheus},
rolling_session_window::RollingSessionWindow, rolling_session_window::{
new_session_window_size, RollingSessionWindow, SessionWindowSize, SessionWindowUpdate,
SessionsUnavailable,
},
TimeoutExt, TimeoutExt,
}; };
use polkadot_primitives::v1::{ use polkadot_primitives::v1::{
...@@ -92,7 +95,8 @@ use crate::{ ...@@ -92,7 +95,8 @@ use crate::{
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
const APPROVAL_SESSIONS: SessionIndex = 6; pub const APPROVAL_SESSIONS: SessionWindowSize = new_session_window_size!(6);
const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120); const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120);
const APPROVAL_CACHE_SIZE: usize = 1024; const APPROVAL_CACHE_SIZE: usize = 1024;
const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds. const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds.
...@@ -568,7 +572,7 @@ impl CurrentlyCheckingSet { ...@@ -568,7 +572,7 @@ impl CurrentlyCheckingSet {
} }
struct State { struct State {
session_window: RollingSessionWindow, session_window: Option<RollingSessionWindow>,
keystore: Arc<LocalKeystore>, keystore: Arc<LocalKeystore>,
slot_duration_millis: u64, slot_duration_millis: u64,
clock: Box<dyn Clock + Send + Sync>, clock: Box<dyn Clock + Send + Sync>,
...@@ -577,9 +581,30 @@ struct State { ...@@ -577,9 +581,30 @@ struct State {
impl State { impl State {
fn session_info(&self, i: SessionIndex) -> Option<&SessionInfo> { fn session_info(&self, i: SessionIndex) -> Option<&SessionInfo> {
self.session_window.session_info(i) self.session_window.as_ref().and_then(|w| w.session_info(i))
} }
/// Bring `session_window` up to date.
pub async fn cache_session_info_for_head(
&mut self,
ctx: &mut (impl SubsystemContext + overseer::SubsystemContext),
head: Hash,
) -> Result<Option<SessionWindowUpdate>, SessionsUnavailable> {
let session_window = self.session_window.take();
match session_window {
None => {
self.session_window =
Some(RollingSessionWindow::new(ctx, APPROVAL_SESSIONS, head).await?);
Ok(None)
},
Some(mut session_window) => {
let r =
session_window.cache_session_info_for_head(ctx, head).await.map(Option::Some);
self.session_window = Some(session_window);
r
},
}
}
// Compute the required tranches for approval for this block and candidate combo. // Compute the required tranches for approval for this block and candidate combo.
// Fails if there is no approval entry for the block under the candidate or no candidate entry // Fails if there is no approval entry for the block under the candidate or no candidate entry
// under the block, or if the session is out of bounds. // under the block, or if the session is out of bounds.
...@@ -671,7 +696,7 @@ where ...@@ -671,7 +696,7 @@ where
B: Backend, B: Backend,
{ {
let mut state = State { let mut state = State {
session_window: RollingSessionWindow::new(APPROVAL_SESSIONS), session_window: None,
keystore: subsystem.keystore, keystore: subsystem.keystore,
slot_duration_millis: subsystem.slot_duration_millis, slot_duration_millis: subsystem.slot_duration_millis,
clock, clock,
......
...@@ -24,6 +24,8 @@ struct MetricsInner { ...@@ -24,6 +24,8 @@ struct MetricsInner {
votes: prometheus::CounterVec<prometheus::U64>, votes: prometheus::CounterVec<prometheus::U64>,
/// Conclusion across all disputes. /// Conclusion across all disputes.
concluded: prometheus::CounterVec<prometheus::U64>, concluded: prometheus::CounterVec<prometheus::U64>,
/// Number of participations that have been queued.
queued_participations: prometheus::CounterVec<prometheus::U64>,
} }
/// Candidate validation metrics. /// Candidate validation metrics.
...@@ -61,6 +63,18 @@ impl Metrics { ...@@ -61,6 +63,18 @@ impl Metrics {
metrics.concluded.with_label_values(&["invalid"]).inc(); metrics.concluded.with_label_values(&["invalid"]).inc();
} }
} }
pub(crate) fn on_queued_priority_participation(&self) {
if let Some(metrics) = &self.0 {
metrics.queued_participations.with_label_values(&["priority"]).inc();
}
}
pub(crate) fn on_queued_best_effort_participation(&self) {
if let Some(metrics) = &self.0 {
metrics.queued_participations.with_label_values(&["best-effort"]).inc();
}
}
} }
impl metrics::Metrics for Metrics { impl metrics::Metrics for Metrics {
...@@ -93,6 +107,16 @@ impl metrics::Metrics for Metrics { ...@@ -93,6 +107,16 @@ impl metrics::Metrics for Metrics {
)?, )?,
registry, registry,
)?, )?,
queued_participations: prometheus::register(
prometheus::CounterVec::new(
prometheus::Opts::new(
"parachain_dispute_participations",
"Total number of queued participations, grouped by priority and best-effort. (Not every queueing will necessarily lead to an actual participation because of duplicates.)",
),
&["priority"],
)?,
registry,
)?,
}; };
Ok(Metrics(Some(metrics))) Ok(Metrics(Some(metrics)))
} }
......
...@@ -26,7 +26,10 @@ use polkadot_primitives::v1::{CandidateHash, SessionIndex}; ...@@ -26,7 +26,10 @@ use polkadot_primitives::v1::{CandidateHash, SessionIndex};
use std::collections::HashMap; use std::collections::HashMap;
use super::db::v1::{CandidateVotes, RecentDisputes}; use super::{
db::v1::{CandidateVotes, RecentDisputes},
error::FatalResult,
};
#[derive(Debug)] #[derive(Debug)]
pub enum BackendWriteOp { pub enum BackendWriteOp {
...@@ -53,7 +56,7 @@ pub trait Backend { ...@@ -53,7 +56,7 @@ pub trait Backend {
/// Atomically writes the list of operations, with later operations taking precedence over /// Atomically writes the list of operations, with later operations taking precedence over
/// prior. /// prior.
fn write<I>(&mut self, ops: I) -> SubsystemResult<()> fn write<I>(&mut self, ops: I) -> FatalResult<()>
where where
I: IntoIterator<Item = BackendWriteOp>; I: IntoIterator<Item = BackendWriteOp>;
} }
......
...@@ -27,12 +27,11 @@ use std::sync::Arc; ...@@ -27,12 +27,11 @@ use std::sync::Arc;
use kvdb::{DBTransaction, KeyValueDB}; use kvdb::{DBTransaction, KeyValueDB};
use parity_scale_codec::{Decode, Encode}; use parity_scale_codec::{Decode, Encode};
use crate::{ use crate::real::{
real::{ backend::{Backend, BackendWriteOp, OverlayedBackend},
backend::{Backend, BackendWriteOp, OverlayedBackend}, error::{Fatal, FatalResult},
DISPUTE_WINDOW, status::DisputeStatus,
}, DISPUTE_WINDOW,
DisputeStatus,
}; };
const RECENT_DISPUTES_KEY: &[u8; 15] = b"recent-disputes"; const RECENT_DISPUTES_KEY: &[u8; 15] = b"recent-disputes";
...@@ -72,7 +71,7 @@ impl Backend for DbBackend { ...@@ -72,7 +71,7 @@ impl Backend for DbBackend {
/// Atomically writes the list of operations, with later operations taking precedence over /// Atomically writes the list of operations, with later operations taking precedence over
/// prior. /// prior.
fn write<I>(&mut self, ops: I) -> SubsystemResult<()> fn write<I>(&mut self, ops: I) -> FatalResult<()>
where where
I: IntoIterator<Item = BackendWriteOp>, I: IntoIterator<Item = BackendWriteOp>,
{ {
...@@ -98,7 +97,7 @@ impl Backend for DbBackend { ...@@ -98,7 +97,7 @@ impl Backend for DbBackend {
} }
} }
self.inner.write(tx).map_err(Into::into) self.inner.write(tx).map_err(Fatal::DbWriteFailed)
} }
} }
...@@ -214,7 +213,7 @@ pub(crate) fn note_current_session( ...@@ -214,7 +213,7 @@ pub(crate) fn note_current_session(
overlay_db: &mut OverlayedBackend<'_, impl Backend>, overlay_db: &mut OverlayedBackend<'_, impl Backend>,
current_session: SessionIndex, current_session: SessionIndex,
) -> SubsystemResult<()> { ) -> SubsystemResult<()> {
let new_earliest = current_session.saturating_sub(DISPUTE_WINDOW); let new_earliest = current_session.saturating_sub(DISPUTE_WINDOW.get());
match overlay_db.load_earliest_session()? { match overlay_db.load_earliest_session()? {
None => { None => {
// First launch - write new-earliest. // First launch - write new-earliest.
...@@ -421,7 +420,7 @@ mod tests { ...@@ -421,7 +420,7 @@ mod tests {
let prev_earliest_session = 0; let prev_earliest_session = 0;
let new_earliest_session = 5; let new_earliest_session = 5;
let current_session = 5 + DISPUTE_WINDOW; let current_session = 5 + DISPUTE_WINDOW.get();
let very_old = 3; let very_old = 3;
let slightly_old = 4; let slightly_old = 4;
......
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Polkadot.
// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
use futures::channel::oneshot;
use thiserror::Error;
use polkadot_node_subsystem::{
errors::{ChainApiError, RuntimeApiError},
SubsystemError,
};
use polkadot_node_subsystem_util::{rolling_session_window::SessionsUnavailable, runtime};
use super::{db, participation};
use crate::real::{CodecError, LOG_TARGET};
/// Errors for this subsystem.
#[derive(Debug, Error)]
#[error(transparent)]
pub enum Error {
/// All fatal errors.
Fatal(#[from] Fatal),
/// All nonfatal/potentially recoverable errors.
NonFatal(#[from] NonFatal),
}
/// General `Result` type for dispute coordinator.
pub type Result<R> = std::result::Result<R, Error>;
/// Result type with only fatal errors.
pub type FatalResult<R> = std::result::Result<R, Fatal>;
/// Result type with only non fatal errors.
pub type NonFatalResult<R> = std::result::Result<R, NonFatal>;
impl From<runtime::Error> for Error {
fn from(o: runtime::Error) -> Self {
match o {
runtime::Error::Fatal(f) => Self::Fatal(Fatal::Runtime(f)),
runtime::Error::NonFatal(f) => Self::NonFatal(NonFatal::Runtime(f)),
}
}
}
impl From<SubsystemError> for Error {
fn from(o: SubsystemError) -> Self {
match o {
SubsystemError::Context(msg) => Self::Fatal(Fatal::SubsystemContext(msg)),
_ => Self::NonFatal(NonFatal::Subsystem(o)),
}
}
}
/// Fatal errors of this subsystem.
#[derive(Debug, Error)]
pub enum Fatal {
/// Errors coming from runtime::Runtime.
#[error("Error while accessing runtime information {0}")]
Runtime(#[from] runtime::Fatal),
/// We received a legacy `SubystemError::Context` error which is considered fatal.
#[error("SubsystemError::Context error: {0}")]
SubsystemContext(String),
/// `ctx.spawn` failed with an error.
#[error("Spawning a task failed: {0}")]
SpawnFailed(SubsystemError),
#[error("Participation worker receiver exhausted.")]
ParticipationWorkerReceiverExhausted,
/// Receiving subsystem message from overseer failed.
#[error("Receiving message from overseer failed: {0}")]
SubsystemReceive(#[source] SubsystemError),