From 846828f61cdbd0c309f9b8678f4ea1fdee6e2681 Mon Sep 17 00:00:00 2001 From: Robert Klotzner Date: Thu, 23 Dec 2021 17:41:34 +0100 Subject: [PATCH] First step in implementing #4386 (#4437) * First step in implementing https://github.com/paritytech/polkadot/issues/4386 This PR: - Reduces MAX_UNSHARED_UPLOAD_TIME to 150ms - Increases timeout on collation fetching to 1200ms - Reduces limit on needed backing votes in the runtime This PR does not yet reduce the number of needed backing votes on the node as this can only be meaningfully enacted once the changed limit in the runtime is live. * Fix tests. * Guide updates. * Review remarks. * Bump minimum required backing votes to 2 in runtime. * Make sure node side code won't make runtime vomit. * cargo +nightly fmt --- polkadot/node/core/backing/src/lib.rs | 14 ++++++++--- .../src/collator_side/mod.rs | 8 ++----- .../protocol/src/request_response/mod.rs | 2 +- .../src/node/backing/candidate-backing.md | 4 ++-- .../runtime/parachains/src/inclusion/mod.rs | 11 ++++++++- .../runtime/parachains/src/inclusion/tests.rs | 24 ++++++++++++++----- 6 files changed, 44 insertions(+), 19 deletions(-) diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index e790f411d6..f3ce935362 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -190,8 +190,16 @@ struct AttestingData { backing: Vec, } -const fn group_quorum(n_validators: usize) -> usize { - (n_validators / 2) + 1 +/// How many votes we need to consider a candidate backed. +fn minimum_votes(n_validators: usize) -> usize { + // Runtime change going live, see: https://github.com/paritytech/polkadot/pull/4437 + let old_runtime_value = n_validators / 2 + 1; + let new_runtime_value = std::cmp::min(2, n_validators); + + // Until new runtime is live everywhere and we don't yet have + // https://github.com/paritytech/polkadot/issues/4576, we want to err on the higher value for + // secured block production: + std::cmp::max(old_runtime_value, new_runtime_value) } #[derive(Default)] @@ -223,7 +231,7 @@ impl TableContextTrait for TableContext { } fn requisite_votes(&self, group: &ParaId) -> usize { - self.groups.get(group).map_or(usize::MAX, |g| group_quorum(g.len())) + self.groups.get(group).map_or(usize::MAX, |g| minimum_votes(g.len())) } } diff --git a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs index 7fc1422c41..4252de837e 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -66,12 +66,8 @@ const COST_APPARENT_FLOOD: Rep = /// /// This is to protect from a single slow validator preventing collations from happening. /// -/// With a collation size of 5MB and bandwidth of 500Mbit/s (requirement for Kusama validators), -/// the transfer should be possible within 0.1 seconds. 400 milliseconds should therefore be -/// plenty and should be low enough for later validators to still be able to finish on time. -/// -/// There is debug logging output, so we can adjust this value based on production results. -const MAX_UNSHARED_UPLOAD_TIME: Duration = Duration::from_millis(400); +/// For considerations on this value, see: https://github.com/paritytech/polkadot/issues/4386 +const MAX_UNSHARED_UPLOAD_TIME: Duration = Duration::from_millis(150); #[derive(Clone, Default)] pub struct Metrics(Option); diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index f8d414cd35..b4e013b5b1 100644 --- a/polkadot/node/network/protocol/src/request_response/mod.rs +++ b/polkadot/node/network/protocol/src/request_response/mod.rs @@ -95,7 +95,7 @@ pub const CHUNK_REQUEST_TIMEOUT: Duration = DEFAULT_REQUEST_TIMEOUT_CONNECTED; /// This timeout is based on what seems sensible from a time budget perspective, considering 6 /// second block time. This is going to be tough, if we have multiple forks and large PoVs, but we /// only have so much time. -const POV_REQUEST_TIMEOUT_CONNECTED: Duration = Duration::from_millis(1000); +const POV_REQUEST_TIMEOUT_CONNECTED: Duration = Duration::from_millis(1200); /// We want timeout statement requests fast, so we don't waste time on slow nodes. Responders will /// try their best to either serve within that timeout or return an error immediately. (We need to diff --git a/polkadot/roadmap/implementers-guide/src/node/backing/candidate-backing.md b/polkadot/roadmap/implementers-guide/src/node/backing/candidate-backing.md index 4b25a89cb1..6637ef431b 100644 --- a/polkadot/roadmap/implementers-guide/src/node/backing/candidate-backing.md +++ b/polkadot/roadmap/implementers-guide/src/node/backing/candidate-backing.md @@ -1,6 +1,6 @@ # Candidate Backing -The Candidate Backing subsystem ensures every parablock considered for relay block inclusion has been seconded by at least one validator, and approved by a quorum. Parablocks for which no validator will assert correctness are discarded. If the block later proves invalid, the initial backers are slashable; this gives polkadot a rational threat model during subsequent stages. +The Candidate Backing subsystem ensures every parablock considered for relay block inclusion has been seconded by at least one validator, and approved by a quorum. Parablocks for which not enough validators will assert correctness are discarded. If the block later proves invalid, the initial backers are slashable; this gives polkadot a rational threat model during subsequent stages. Its role is to produce backable candidates for inclusion in new relay-chain blocks. It does so by issuing signed [`Statement`s][Statement] and tracking received statements signed by other validators. Once enough statements are received, they can be combined into backing for specific candidates. @@ -91,7 +91,7 @@ match msg { } ``` -Add `Seconded` statements and `Valid` statements to a quorum. If quorum reaches validator-group majority, send a [`ProvisionerMessage`][PM]`::ProvisionableData(ProvisionableData::BackedCandidate(CandidateReceipt))` message. +Add `Seconded` statements and `Valid` statements to a quorum. If the quorum reaches a pre-defined threshold, send a [`ProvisionerMessage`][PM]`::ProvisionableData(ProvisionableData::BackedCandidate(CandidateReceipt))` message. `Invalid` statements that conflict with already witnessed `Seconded` and `Valid` statements for the given candidate, statements that are double-votes, self-contradictions and so on, should result in issuing a [`ProvisionerMessage`][PM]`::MisbehaviorReport` message for each newly detected case of this kind. On each incoming statement, [`DisputeCoordinatorMessage::ImportStatement`][DCM] should be issued. diff --git a/polkadot/runtime/parachains/src/inclusion/mod.rs b/polkadot/runtime/parachains/src/inclusion/mod.rs index 8ef66cbc81..9632e2b98b 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -169,6 +169,15 @@ impl Default for ProcessedCandidates { } } +/// Number of backing votes we need for a valid backing. +pub fn minimum_backing_votes(n_validators: usize) -> usize { + // For considerations on this value see: + // https://github.com/paritytech/polkadot/pull/1656#issuecomment-999734650 + // and + // https://github.com/paritytech/polkadot/issues/4386 + sp_std::cmp::min(n_validators, 2) +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -578,7 +587,7 @@ impl Pallet { match maybe_amount_validated { Ok(amount_validated) => ensure!( - amount_validated * 2 > group_vals.len(), + amount_validated >= minimum_backing_votes(group_vals.len()), Error::::InsufficientBacking, ), Err(()) => { diff --git a/polkadot/runtime/parachains/src/inclusion/tests.rs b/polkadot/runtime/parachains/src/inclusion/tests.rs index 73d0bb83b6..12dbb76640 100644 --- a/polkadot/runtime/parachains/src/inclusion/tests.rs +++ b/polkadot/runtime/parachains/src/inclusion/tests.rs @@ -113,7 +113,7 @@ pub(crate) async fn back_candidate( kind: BackingKind, ) -> BackedCandidate { let mut validator_indices = bitvec::bitvec![BitOrderLsb0, u8; 0; group.len()]; - let threshold = (group.len() / 2) + 1; + let threshold = minimum_backing_votes(group.len()); let signing = match kind { BackingKind::Unanimous => group.len(), @@ -151,8 +151,8 @@ pub(crate) async fn back_candidate( Some(validators[group[i].0 as usize].public().into()) }) .ok() - .unwrap_or(0) * - 2 > group.len(); + .unwrap_or(0) >= + threshold; match kind { BackingKind::Unanimous | BackingKind::Threshold => assert!(successfully_backed), @@ -1651,6 +1651,10 @@ fn backing_works() { assure_candidate_sorting(candidate_receipt_with_backing_validator_indices) ); + let backers = { + let num_backers = minimum_backing_votes(group_validators(GroupIndex(0)).unwrap().len()); + backing_bitfield(&(0..num_backers).collect::>()) + }; assert_eq!( >::get(&chain_a), Some(CandidatePendingAvailability { @@ -1660,7 +1664,7 @@ fn backing_works() { availability_votes: default_availability_votes(), relay_parent_number: System::block_number() - 1, backed_in_number: System::block_number(), - backers: backing_bitfield(&[0, 1]), + backers, backing_group: GroupIndex::from(0), }) ); @@ -1669,6 +1673,10 @@ fn backing_works() { Some(candidate_a.commitments), ); + let backers = { + let num_backers = minimum_backing_votes(group_validators(GroupIndex(0)).unwrap().len()); + backing_bitfield(&(0..num_backers).map(|v| v + 2).collect::>()) + }; assert_eq!( >::get(&chain_b), Some(CandidatePendingAvailability { @@ -1678,7 +1686,7 @@ fn backing_works() { availability_votes: default_availability_votes(), relay_parent_number: System::block_number() - 1, backed_in_number: System::block_number(), - backers: backing_bitfield(&[2, 3]), + backers, backing_group: GroupIndex::from(1), }) ); @@ -1790,6 +1798,10 @@ fn can_include_candidate_with_ok_code_upgrade() { assert_eq!(occupied_cores, vec![CoreIndex::from(0)]); + let backers = { + let num_backers = minimum_backing_votes(group_validators(GroupIndex(0)).unwrap().len()); + backing_bitfield(&(0..num_backers).collect::>()) + }; assert_eq!( >::get(&chain_a), Some(CandidatePendingAvailability { @@ -1799,7 +1811,7 @@ fn can_include_candidate_with_ok_code_upgrade() { availability_votes: default_availability_votes(), relay_parent_number: System::block_number() - 1, backed_in_number: System::block_number(), - backers: backing_bitfield(&[0, 1, 2]), + backers, backing_group: GroupIndex::from(0), }) );