From 5153e2b5636a176c0b0d2313f95bfce32e481289 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Fri, 13 Dec 2024 10:58:19 +0200 Subject: [PATCH] Collation fetching fairness (#4880) Related to https://github.com/paritytech/polkadot-sdk/issues/1797 # The problem When fetching collations in collator protocol/validator side we need to ensure that each parachain has got a fair core time share depending on its assignments in the claim queue. This means that the number of collations fetched per parachain should ideally be equal to (but definitely not bigger than) the number of claims for the particular parachain in the claim queue. # Why the current implementation is not good enough The current implementation doesn't guarantee such fairness. For each relay parent there is a `waiting_queue` (PerRelayParent -> Collations -> waiting_queue) which holds any unfetched collations advertised to the validator. The collations are fetched on first in first out principle which means that if two parachains share a core and one of the parachains is more aggressive it might starve the second parachain. How? At each relay parent up to `max_candidate_depth` candidates are accepted (enforced in `fn is_seconded_limit_reached`) so if one of the parachains is quick enough to fill in the queue with its advertisements the validator will never fetch anything from the rest of the parachains despite they are scheduled. This doesn't mean that the aggressive parachain will occupy all the core time (this is guaranteed by the runtime) but it will deny the rest of the parachains sharing the same core to have collations backed. # How to fix it The solution I am proposing is to limit fetches and advertisements based on the state of the claim queue. At each relay parent the claim queue for the core assigned to the validator is fetched. For each parachain a fetch limit is calculated (equal to the number of entries in the claim queue). Advertisements are not fetched for a parachain which has exceeded its claims in the claim queue. This solves the problem with aggressive parachains advertising too much collations. The second part is in collation fetching logic. The collator will keep track on which collations it has fetched so far. When a new collation needs to be fetched instead of popping the first entry from the `waiting_queue` the validator examines the claim queue and looks for the earliest claim which hasn't got a corresponding fetch. This way the collator will always try to prioritise the most urgent entries. ## How the 'fair share of coretime' for each parachain is determined? Thanks to async backing we can accept more than one candidate per relay parent (with some constraints). We also have got the claim queue which gives us a hint which parachain will be scheduled next on each core. So thanks to the claim queue we can determine the maximum number of claims per parachain. For example the claim queue is [A A A] at relay parent X so we know that at relay parent X we can accept three candidates for parachain A. There are two things to consider though: 1. If we accept more than one candidate at relay parent X we are claiming the slot of a future relay parent. So accepting two candidates for relay parent X means that we are claiming the slot at rp X+1 or rp X+2. 2. At the same time the slot at relay parent X could have been claimed by a previous relay parent(s). This means that we need to accept less candidates at X or even no candidates. There are a few cases worth considering: 1. Slot claimed by previous relay parent. CQ @ rp X: [A A A] Advertisements at X-1 for para A: 2 Advertisements at X-2 for para A: 2 Outcome - at rp X we can accept only 1 advertisement since our slots were already claimed. 2. Slot in our claim queue already claimed at future relay parent CQ @ rp X: [A A A] Advertisements at X+1 for para A: 1 Advertisements at X+2 for para A: 1 Outcome: at rp X we can accept only 1 advertisement since the slots in our relay parents were already claimed. The situation becomes more complicated with multiple leaves (forks). Imagine we have got a fork at rp X: ``` CQ @ rp X: [A A A] (rp X) -> (rp X+1) -> rp(X+2) \-> (rp X+1') ``` Now when we examine the claim queue at RP X we need to consider both forks. This means that accepting a candidate at X means that we should have a slot for it in *BOTH* leaves. If for example there are three candidates accepted at rp X+1' we can't accept any candidates at rp X because there will be no slot for it in one of the leaves. ## How the claims are counted There are two solutions for counting the claims at relay parent X: 1. Keep a state for the claim queue (number of claims and which of them are claimed) and look it up when accepting a collation. With this approach we need to keep the state up to date with each new advertisement and each new leaf update. 2. Calculate the state of the claim queue on the fly at each advertisement. This way we rebuild the state of the claim queue at each advertisements. Solution 1 is hard to implement with forks. There are too many variants to keep track of (different state for each leaf) and at the same time we might never need to use them. So I decided to go with option 2 - building claim queue state on the fly. To achieve this I've extended `View` from backing_implicit_view to keep track of the outer leaves. I've also added a method which accepts a relay parent and return all paths from an outer leaf to it. Let's call it `paths_to_relay_parent`. So how the counting works for relay parent X? First we examine the number of seconded and pending advertisements (more on pending in a second) from relay parent X to relay parent X-N (inclusive) where N is the length of the claim queue. Then we use `paths_to_relay_parent` to obtain all paths from outer leaves to relay parent X. We calculate the claims at relay parents X+1 to X+N (inclusive) for each leaf and get the maximum value. This way we guarantee that the candidate at rp X can be included in each leaf. This is the state of the claim queue which we use to decide if we can fetch one more advertisement at rp X or not. ## What is a pending advertisement I mentioned that we count seconded and pending advertisements at relay parent X. A pending advertisement is: 1. An advertisement which is being fetched right now. 2. An advertisement pending validation at backing subsystem. 3. An advertisement blocked for seconding by backing because we don't know on of its parent heads. Any of these is considered a 'pending fetch' and a slot for it is kept. All of them are already tracked in `State`. --------- Co-authored-by: Maciej Co-authored-by: command-bot <> Co-authored-by: Alin Dima --- .gitlab/pipeline/zombienet/polkadot.yml | 11 + .../network/collator-protocol/src/error.rs | 3 + .../src/validator_side/claim_queue_state.rs | 1055 +++++++++++++++ .../src/validator_side/collation.rs | 184 +-- .../src/validator_side/mod.rs | 599 +++++---- .../src/validator_side/tests/mod.rs | 570 ++------ .../tests/prospective_parachains.rs | 1192 +++++++++++++++-- .../src/backing_implicit_view.rs | 435 ++++-- ...astic-scaling-doesnt-break-parachains.toml | 2 +- ...stic-scaling-doesnt-break-parachains.zndsl | 2 +- .../0018-shared-core-idle-parachain.toml | 2 +- ...-coretime-collation-fetching-fairness.toml | 58 + ...coretime-collation-fetching-fairness.zndsl | 16 + .../functional/0019-verify-included-events.js | 51 + prdoc/pr_4880.prdoc | 31 + 15 files changed, 3196 insertions(+), 1015 deletions(-) create mode 100644 polkadot/node/network/collator-protocol/src/validator_side/claim_queue_state.rs create mode 100644 polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.toml create mode 100644 polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.zndsl create mode 100644 polkadot/zombienet_tests/functional/0019-verify-included-events.js create mode 100644 prdoc/pr_4880.prdoc diff --git a/.gitlab/pipeline/zombienet/polkadot.yml b/.gitlab/pipeline/zombienet/polkadot.yml index ac4bdac7ad15..e722239d890c 100644 --- a/.gitlab/pipeline/zombienet/polkadot.yml +++ b/.gitlab/pipeline/zombienet/polkadot.yml @@ -252,6 +252,17 @@ zombienet-polkadot-functional-0018-shared-core-idle-parachain: --local-dir="${LOCAL_DIR}/functional" --test="0018-shared-core-idle-parachain.zndsl" +zombienet-polkadot-functional-0019-coretime-collation-fetching-fairness: + extends: + - .zombienet-polkadot-common + before_script: + - !reference [ .zombienet-polkadot-common, before_script ] + - cp --remove-destination ${LOCAL_DIR}/assign-core.js ${LOCAL_DIR}/functional + script: + - /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh + --local-dir="${LOCAL_DIR}/functional" + --test="0019-coretime-collation-fetching-fairness.zndsl" + zombienet-polkadot-smoke-0001-parachains-smoke-test: extends: - .zombienet-polkadot-common diff --git a/polkadot/node/network/collator-protocol/src/error.rs b/polkadot/node/network/collator-protocol/src/error.rs index 598cdcf43900..97fd4076bb8f 100644 --- a/polkadot/node/network/collator-protocol/src/error.rs +++ b/polkadot/node/network/collator-protocol/src/error.rs @@ -70,6 +70,9 @@ pub enum Error { #[error("Response receiver for claim queue request cancelled")] CancelledClaimQueue(oneshot::Canceled), + + #[error("No state for the relay parent")] + RelayParentStateNotFound, } /// An error happened on the validator side of the protocol when attempting diff --git a/polkadot/node/network/collator-protocol/src/validator_side/claim_queue_state.rs b/polkadot/node/network/collator-protocol/src/validator_side/claim_queue_state.rs new file mode 100644 index 000000000000..3a34cf52fec6 --- /dev/null +++ b/polkadot/node/network/collator-protocol/src/validator_side/claim_queue_state.rs @@ -0,0 +1,1055 @@ +// Copyright (C) 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 . + +//! `ClaimQueueState` tracks the state of the claim queue over a set of relay blocks. Refer to +//! [`ClaimQueueState`] for more details. + +use std::collections::VecDeque; + +use crate::LOG_TARGET; +use polkadot_primitives::{Hash, Id as ParaId}; + +/// Represents a single claim from the claim queue, mapped to the relay chain block where it could +/// be backed on-chain. +#[derive(Debug, PartialEq)] +struct ClaimInfo { + // Hash of the relay chain block. Can be `None` if it is still not known (a future block). + hash: Option, + /// Represents the `ParaId` scheduled for the block. Can be `None` if nothing is scheduled. + claim: Option, + /// The length of the claim queue at the block. It is used to determine the 'block window' + /// where a claim can be made. + claim_queue_len: usize, + /// A flag that indicates if the slot is claimed or not. + claimed: bool, +} + +/// Tracks the state of the claim queue over a set of relay blocks. +/// +/// Generally the claim queue represents the `ParaId` that should be scheduled at the current block +/// (the first element of the claim queue) and N other `ParaId`s which are supposed to be scheduled +/// on the next relay blocks. In other words the claim queue is a rolling window giving a hint what +/// should be built/fetched/accepted (depending on the context) at each block. +/// +/// Since the claim queue peeks into the future blocks there is a relation between the claim queue +/// state between the current block and the future blocks. +/// Let's see an example with 2 co-scheduled parachains: +/// - relay parent 1; Claim queue: [A, B, A] +/// - relay parent 2; Claim queue: [B, A, B] +/// - relay parent 3; Claim queue: [A, B, A] +/// - and so on +/// +/// Note that at rp1 the second element in the claim queue is equal to the first one in rp2. Also +/// the third element of the claim queue at rp1 is equal to the second one in rp2 and the first one +/// in rp3. +/// +/// So if we want to claim the third slot at rp 1 we are also claiming the second at rp2 and first +/// at rp3. To track this in a simple way we can project the claim queue onto the relay blocks like +/// this: +/// [A] [B] [A] -> this is the claim queue at rp3 +/// [B] [A] [B] -> this is the claim queue at rp2 +/// [A] [B] [A] -> this is the claim queue at rp1 +/// [RP 1][RP 2][RP 3][RP X][RP Y] -> relay blocks, RP x and RP Y are future blocks +/// +/// Note that the claims at each column are the same so we can simplify this by just projecting a +/// single claim over a block: +/// [A] [B] [A] [B] [A] -> claims effectively are the same +/// [RP 1][RP 2][RP 3][RP X][RP Y] -> relay blocks, RP x and RP Y are future blocks +/// +/// Basically this is how `ClaimQueueState` works. It keeps track of claims at each block by mapping +/// claims to relay blocks. +/// +/// How making a claim works? +/// At each relay block we keep track how long is the claim queue. This is a 'window' where we can +/// make a claim. So adding a claim just looks for a free spot at this window and claims it. +/// +/// Note on adding a new leaf. +/// When a new leaf is added we check if the first element in its claim queue matches with the +/// projection on the first element in 'future blocks'. If yes - the new relay block inherits this +/// claim. If not - this means that the claim queue changed for some reason so the claim can't be +/// inherited. This should not happen under normal circumstances. But if it happens it means that we +/// have got one claim which won't be satisfied in the worst case scenario. +pub(crate) struct ClaimQueueState { + block_state: VecDeque, + future_blocks: VecDeque, +} + +impl ClaimQueueState { + pub(crate) fn new() -> Self { + Self { block_state: VecDeque::new(), future_blocks: VecDeque::new() } + } + + // Appends a new leaf + pub(crate) fn add_leaf(&mut self, hash: &Hash, claim_queue: &Vec) { + if self.block_state.iter().any(|s| s.hash == Some(*hash)) { + return + } + + // First check if our view for the future blocks is consistent with the one in the claim + // queue of the new block. If not - the claim queue has changed for some reason and we need + // to readjust our view. + for (idx, expected_claim) in claim_queue.iter().enumerate() { + match self.future_blocks.get_mut(idx) { + Some(future_block) => + if future_block.claim.as_ref() != Some(expected_claim) { + // There is an inconsistency. Update our view with the one from the claim + // queue. `claimed` can't be true anymore since the `ParaId` has changed. + future_block.claimed = false; + future_block.claim = Some(*expected_claim); + }, + None => { + self.future_blocks.push_back(ClaimInfo { + hash: None, + claim: Some(*expected_claim), + // For future blocks we don't know the size of the claim queue. + // `claim_queue_len` could be an option but there is not much benefit from + // the extra boilerplate code to handle it. We set it to one since we + // usually know about one claim at each future block but this value is not + // used anywhere in the code. + claim_queue_len: 1, + claimed: false, + }); + }, + } + } + + // Now pop the first future block and add it as a leaf + let claim_info = if let Some(new_leaf) = self.future_blocks.pop_front() { + ClaimInfo { + hash: Some(*hash), + claim: claim_queue.first().copied(), + claim_queue_len: claim_queue.len(), + claimed: new_leaf.claimed, + } + } else { + // maybe the claim queue was empty but we still need to add a leaf + ClaimInfo { + hash: Some(*hash), + claim: claim_queue.first().copied(), + claim_queue_len: claim_queue.len(), + claimed: false, + } + }; + + // `future_blocks` can't be longer than the length of the claim queue at the last block - 1. + // For example this can happen if at relay block N we have got a claim queue of a length 4 + // and it's shrunk to 2. + self.future_blocks.truncate(claim_queue.len().saturating_sub(1)); + + self.block_state.push_back(claim_info); + } + + fn get_window<'a>( + &'a mut self, + relay_parent: &'a Hash, + ) -> impl Iterator + 'a { + let mut window = self + .block_state + .iter_mut() + .skip_while(|b| b.hash != Some(*relay_parent)) + .peekable(); + let cq_len = window.peek().map_or(0, |b| b.claim_queue_len); + window.chain(self.future_blocks.iter_mut()).take(cq_len) + } + + pub(crate) fn claim_at(&mut self, relay_parent: &Hash, para_id: &ParaId) -> bool { + gum::trace!( + target: LOG_TARGET, + ?para_id, + ?relay_parent, + "claim_at" + ); + self.find_a_claim(relay_parent, para_id, true) + } + + pub(crate) fn can_claim_at(&mut self, relay_parent: &Hash, para_id: &ParaId) -> bool { + gum::trace!( + target: LOG_TARGET, + ?para_id, + ?relay_parent, + "can_claim_at" + ); + + self.find_a_claim(relay_parent, para_id, false) + } + + // Returns `true` if there is a claim within `relay_parent`'s view of the claim queue for + // `para_id`. If `claim_it` is set to `true` the slot is claimed. Otherwise the function just + // reports the availability of the slot. + fn find_a_claim(&mut self, relay_parent: &Hash, para_id: &ParaId, claim_it: bool) -> bool { + let window = self.get_window(relay_parent); + + for w in window { + gum::trace!( + target: LOG_TARGET, + ?para_id, + ?relay_parent, + claim_info=?w, + ?claim_it, + "Checking claim" + ); + + if !w.claimed && w.claim == Some(*para_id) { + w.claimed = claim_it; + return true + } + } + + false + } + + pub(crate) fn unclaimed_at(&mut self, relay_parent: &Hash) -> Vec { + let window = self.get_window(relay_parent); + + window.filter(|b| !b.claimed).filter_map(|b| b.claim).collect() + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn sane_initial_state() { + let mut state = ClaimQueueState::new(); + let relay_parent = Hash::from_low_u64_be(1); + let para_id = ParaId::new(1); + + assert!(!state.can_claim_at(&relay_parent, ¶_id)); + assert!(!state.claim_at(&relay_parent, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent), vec![]); + } + + #[test] + fn add_leaf_works() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id = ParaId::new(1); + let claim_queue = vec![para_id, para_id, para_id]; + + state.add_leaf(&relay_parent_a, &claim_queue); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id, para_id, para_id]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: false, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false } + ]) + ); + + // should be no op + state.add_leaf(&relay_parent_a, &claim_queue); + assert_eq!(state.block_state.len(), 1); + assert_eq!(state.future_blocks.len(), 2); + + // add another leaf + let relay_parent_b = Hash::from_low_u64_be(2); + state.add_leaf(&relay_parent_b, &claim_queue); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: false, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id), + claim_queue_len: 3, + claimed: false, + } + ]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false } + ]) + ); + + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id, para_id, para_id]); + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![para_id, para_id, para_id]); + } + + #[test] + fn claims_at_separate_relay_parents_work() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let relay_parent_b = Hash::from_low_u64_be(2); + let para_id = ParaId::new(1); + let claim_queue = vec![para_id, para_id, para_id]; + + state.add_leaf(&relay_parent_a, &claim_queue); + state.add_leaf(&relay_parent_b, &claim_queue); + + // add one claim for a + assert!(state.can_claim_at(&relay_parent_a, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id, para_id, para_id]); + assert!(state.claim_at(&relay_parent_a, ¶_id)); + + // and one for b + assert!(state.can_claim_at(&relay_parent_b, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![para_id, para_id, para_id]); + assert!(state.claim_at(&relay_parent_b, ¶_id)); + + // a should have one claim since the one for b was claimed + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id]); + // and two more for b + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![para_id, para_id]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + } + ]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false } + ]) + ); + } + + #[test] + fn claims_are_transferred_to_next_slot() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id = ParaId::new(1); + let claim_queue = vec![para_id, para_id, para_id]; + + state.add_leaf(&relay_parent_a, &claim_queue); + + // add two claims, 2nd should be transferred to a new leaf + assert!(state.can_claim_at(&relay_parent_a, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id, para_id, para_id]); + assert!(state.claim_at(&relay_parent_a, ¶_id)); + + assert!(state.can_claim_at(&relay_parent_a, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id, para_id]); + assert!(state.claim_at(&relay_parent_a, ¶_id)); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false } + ]) + ); + + // one more + assert!(state.can_claim_at(&relay_parent_a, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id]); + assert!(state.claim_at(&relay_parent_a, ¶_id)); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true } + ]) + ); + + // no more claims + assert!(!state.can_claim_at(&relay_parent_a, ¶_id)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + } + + #[test] + fn claims_are_transferred_to_new_leaves() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id = ParaId::new(1); + let claim_queue = vec![para_id, para_id, para_id]; + + state.add_leaf(&relay_parent_a, &claim_queue); + + for _ in 0..3 { + assert!(state.can_claim_at(&relay_parent_a, ¶_id)); + assert!(state.claim_at(&relay_parent_a, ¶_id)); + } + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true } + ]) + ); + + // no more claims + assert!(!state.can_claim_at(&relay_parent_a, ¶_id)); + + // new leaf + let relay_parent_b = Hash::from_low_u64_be(2); + state.add_leaf(&relay_parent_b, &claim_queue); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + } + ]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: false } + ]) + ); + + // still no claims for a + assert!(!state.can_claim_at(&relay_parent_a, ¶_id)); + + // but can accept for b + assert!(state.can_claim_at(&relay_parent_b, ¶_id)); + assert!(state.claim_at(&relay_parent_b, ¶_id)); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id), + claim_queue_len: 3, + claimed: true, + } + ]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id), claim_queue_len: 1, claimed: true } + ]) + ); + } + + #[test] + fn two_paras() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id_a = ParaId::new(1); + let para_id_b = ParaId::new(2); + let claim_queue = vec![para_id_a, para_id_b, para_id_a]; + + state.add_leaf(&relay_parent_a, &claim_queue); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_b)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_a, para_id_b, para_id_a]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: false, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + } + ]) + ); + + assert!(state.claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_b)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_b, para_id_a]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + } + ]) + ); + + assert!(state.claim_at(&relay_parent_a, ¶_id_a)); + assert!(!state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_b)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_b]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { hash: None, claim: Some(para_id_a), claim_queue_len: 1, claimed: true } + ]) + ); + + assert!(state.claim_at(&relay_parent_a, ¶_id_b)); + assert!(!state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(!state.can_claim_at(&relay_parent_a, ¶_id_b)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id_b), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id_a), claim_queue_len: 1, claimed: true } + ]) + ); + } + + #[test] + fn claim_queue_changes_unexpectedly() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id_a = ParaId::new(1); + let para_id_b = ParaId::new(2); + let claim_queue_a = vec![para_id_a, para_id_b, para_id_a]; + + state.add_leaf(&relay_parent_a, &claim_queue_a); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_b)); + assert!(state.claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.claim_at(&relay_parent_a, ¶_id_b)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id_b), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id_a), claim_queue_len: 1, claimed: true } + ]) + ); + + let relay_parent_b = Hash::from_low_u64_be(2); + let claim_queue_b = vec![para_id_a, para_id_a, para_id_a]; // should be [b, a, ...] + state.add_leaf(&relay_parent_b, &claim_queue_b); + + // because of the unexpected change in claim queue we lost the claim for paraB and have one + // unclaimed for paraA + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_a]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: false, + } + ]) + ); + assert_eq!( + state.future_blocks, + // since the 3rd slot of the claim queue at rp1 is equal to the second one in rp2, this + // claim still exists + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id_a), claim_queue_len: 1, claimed: true }, + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + } + ]) + ); + } + + #[test] + fn claim_queue_changes_unexpectedly_with_two_blocks() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id_a = ParaId::new(1); + let para_id_b = ParaId::new(2); + let claim_queue_a = vec![para_id_a, para_id_b, para_id_b]; + + state.add_leaf(&relay_parent_a, &claim_queue_a); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.can_claim_at(&relay_parent_a, ¶_id_b)); + assert!(state.claim_at(&relay_parent_a, ¶_id_a)); + assert!(state.claim_at(&relay_parent_a, ¶_id_b)); + assert!(state.claim_at(&relay_parent_a, ¶_id_b)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { hash: None, claim: Some(para_id_b), claim_queue_len: 1, claimed: true }, + ClaimInfo { hash: None, claim: Some(para_id_b), claim_queue_len: 1, claimed: true } + ]) + ); + + let relay_parent_b = Hash::from_low_u64_be(2); + let claim_queue_b = vec![para_id_a, para_id_a, para_id_a]; // should be [b, b, ...] + state.add_leaf(&relay_parent_b, &claim_queue_b); + + // because of the unexpected change in claim queue we lost both claims for paraB and have + // two unclaimed for paraA + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_a, para_id_a]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: true, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: false, + } + ]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + } + ]) + ); + } + + #[test] + fn empty_claim_queue() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id_a = ParaId::new(1); + let claim_queue_a = vec![]; + + state.add_leaf(&relay_parent_a, &claim_queue_a); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: None, + claim_queue_len: 0, + claimed: false, + },]) + ); + // no claim queue so we know nothing about future blocks + assert!(state.future_blocks.is_empty()); + + assert!(!state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(!state.claim_at(&relay_parent_a, ¶_id_a)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + let relay_parent_b = Hash::from_low_u64_be(2); + let claim_queue_b = vec![para_id_a]; + state.add_leaf(&relay_parent_b, &claim_queue_b); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: None, + claim_queue_len: 0, + claimed: false, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false, + }, + ]) + ); + // claim queue with length 1 doesn't say anything about future blocks + assert!(state.future_blocks.is_empty()); + + assert!(!state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(!state.claim_at(&relay_parent_a, ¶_id_a)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + assert!(state.can_claim_at(&relay_parent_b, ¶_id_a)); + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![para_id_a]); + assert!(state.claim_at(&relay_parent_b, ¶_id_a)); + + let relay_parent_c = Hash::from_low_u64_be(3); + let claim_queue_c = vec![para_id_a, para_id_a]; + state.add_leaf(&relay_parent_c, &claim_queue_c); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: None, + claim_queue_len: 0, + claimed: false, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: true, + }, + ClaimInfo { + hash: Some(relay_parent_c), + claim: Some(para_id_a), + claim_queue_len: 2, + claimed: false, + }, + ]) + ); + // claim queue with length 2 fills only one future block + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false, + },]) + ); + + assert!(!state.can_claim_at(&relay_parent_a, ¶_id_a)); + assert!(!state.claim_at(&relay_parent_a, ¶_id_a)); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![]); + + // already claimed + assert!(!state.can_claim_at(&relay_parent_b, ¶_id_a)); + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![]); + assert!(!state.claim_at(&relay_parent_b, ¶_id_a)); + + assert!(state.can_claim_at(&relay_parent_c, ¶_id_a)); + assert_eq!(state.unclaimed_at(&relay_parent_c), vec![para_id_a, para_id_a]); + } + + #[test] + fn claim_queue_becomes_shorter() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id_a = ParaId::new(1); + let para_id_b = ParaId::new(2); + let claim_queue_a = vec![para_id_a, para_id_b, para_id_a]; + + state.add_leaf(&relay_parent_a, &claim_queue_a); + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_a, para_id_b, para_id_a]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: false, + },]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + } + ]) + ); + + let relay_parent_b = Hash::from_low_u64_be(2); + let claim_queue_b = vec![para_id_a, para_id_b]; // should be [b, a] + state.add_leaf(&relay_parent_b, &claim_queue_b); + + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![para_id_a, para_id_b]); + // claims for `relay_parent_a` has changed. + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_a, para_id_a, para_id_b]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 3, + claimed: false, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id_a), + claim_queue_len: 2, + claimed: false, + } + ]) + ); + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + },]) + ); + } + + #[test] + fn claim_queue_becomes_shorter_and_drops_future_claims() { + let mut state = ClaimQueueState::new(); + let relay_parent_a = Hash::from_low_u64_be(1); + let para_id_a = ParaId::new(1); + let para_id_b = ParaId::new(2); + let claim_queue_a = vec![para_id_a, para_id_b, para_id_a, para_id_b]; + + state.add_leaf(&relay_parent_a, &claim_queue_a); + + assert_eq!( + state.unclaimed_at(&relay_parent_a), + vec![para_id_a, para_id_b, para_id_a, para_id_b] + ); + + // We start with claim queue len 4. + assert_eq!( + state.block_state, + VecDeque::from(vec![ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 4, + claimed: false, + },]) + ); + // we have got three future blocks + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ + ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + }, + ClaimInfo { + hash: None, + claim: Some(para_id_b), + claim_queue_len: 1, + claimed: false + } + ]) + ); + + // The next claim len is 2, so we loose one future block + let relay_parent_b = Hash::from_low_u64_be(2); + let para_id_a = ParaId::new(1); + let para_id_b = ParaId::new(2); + let claim_queue_b = vec![para_id_b, para_id_a]; + state.add_leaf(&relay_parent_b, &claim_queue_b); + + assert_eq!(state.unclaimed_at(&relay_parent_a), vec![para_id_a, para_id_b, para_id_a]); + assert_eq!(state.unclaimed_at(&relay_parent_b), vec![para_id_b, para_id_a]); + + assert_eq!( + state.block_state, + VecDeque::from(vec![ + ClaimInfo { + hash: Some(relay_parent_a), + claim: Some(para_id_a), + claim_queue_len: 4, + claimed: false, + }, + ClaimInfo { + hash: Some(relay_parent_b), + claim: Some(para_id_b), + claim_queue_len: 2, + claimed: false, + } + ]) + ); + + assert_eq!( + state.future_blocks, + VecDeque::from(vec![ClaimInfo { + hash: None, + claim: Some(para_id_a), + claim_queue_len: 1, + claimed: false + },]) + ); + } +} diff --git a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs index cc0de1cb70f6..625140a73966 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs @@ -18,16 +18,28 @@ //! //! Usually a path of collations is as follows: //! 1. First, collation must be advertised by collator. -//! 2. If the advertisement was accepted, it's queued for fetch (per relay parent). -//! 3. Once it's requested, the collation is said to be Pending. -//! 4. Pending collation becomes Fetched once received, we send it to backing for validation. -//! 5. If it turns to be invalid or async backing allows seconding another candidate, carry on +//! 2. The validator inspects the claim queue and decides if the collation should be fetched +//! based on the entries there. A parachain can't have more fetched collations than the +//! entries in the claim queue at a specific relay parent. When calculating this limit the +//! validator counts all advertisements within its view not just at the relay parent. +//! 3. If the advertisement was accepted, it's queued for fetch (per relay parent). +//! 4. Once it's requested, the collation is said to be pending fetch +//! (`CollationStatus::Fetching`). +//! 5. Pending fetch collation becomes pending validation +//! (`CollationStatus::WaitingOnValidation`) once received, we send it to backing for +//! validation. +//! 6. If it turns to be invalid or async backing allows seconding another candidate, carry on //! with the next advertisement, otherwise we're done with this relay parent. //! -//! ┌──────────────────────────────────────────┐ -//! └─▶Advertised ─▶ Pending ─▶ Fetched ─▶ Validated - -use std::{collections::VecDeque, future::Future, pin::Pin, task::Poll}; +//! ┌───────────────────────────────────┐ +//! └─▶Waiting ─▶ Fetching ─▶ WaitingOnValidation + +use std::{ + collections::{BTreeMap, VecDeque}, + future::Future, + pin::Pin, + task::Poll, +}; use futures::{future::BoxFuture, FutureExt}; use polkadot_node_network_protocol::{ @@ -36,9 +48,7 @@ use polkadot_node_network_protocol::{ PeerId, }; use polkadot_node_primitives::PoV; -use polkadot_node_subsystem_util::{ - metrics::prometheus::prometheus::HistogramTimer, runtime::ProspectiveParachainsMode, -}; +use polkadot_node_subsystem_util::metrics::prometheus::prometheus::HistogramTimer; use polkadot_primitives::{ vstaging::CandidateReceiptV2 as CandidateReceipt, CandidateHash, CollatorId, Hash, HeadData, Id as ParaId, PersistedValidationData, @@ -187,12 +197,10 @@ pub struct PendingCollationFetch { pub enum CollationStatus { /// We are waiting for a collation to be advertised to us. Waiting, - /// We are currently fetching a collation. - Fetching, + /// We are currently fetching a collation for the specified `ParaId`. + Fetching(ParaId), /// We are waiting that a collation is being validated. WaitingOnValidation, - /// We have seconded a collation. - Seconded, } impl Default for CollationStatus { @@ -202,22 +210,22 @@ impl Default for CollationStatus { } impl CollationStatus { - /// Downgrades to `Waiting`, but only if `self != Seconded`. - fn back_to_waiting(&mut self, relay_parent_mode: ProspectiveParachainsMode) { - match self { - Self::Seconded => - if relay_parent_mode.is_enabled() { - // With async backing enabled it's allowed to - // second more candidates. - *self = Self::Waiting - }, - _ => *self = Self::Waiting, - } + /// Downgrades to `Waiting` + pub fn back_to_waiting(&mut self) { + *self = Self::Waiting } } +/// The number of claims in the claim queue and seconded candidates count for a specific `ParaId`. +#[derive(Default, Debug)] +struct CandidatesStatePerPara { + /// How many collations have been seconded. + pub seconded_per_para: usize, + // Claims in the claim queue for the `ParaId`. + pub claims_per_para: usize, +} + /// Information about collations per relay parent. -#[derive(Default)] pub struct Collations { /// What is the current status in regards to a collation for this relay parent? pub status: CollationStatus, @@ -226,75 +234,89 @@ pub struct Collations { /// This is the currently last started fetch, which did not exceed `MAX_UNSHARED_DOWNLOAD_TIME` /// yet. pub fetching_from: Option<(CollatorId, Option)>, - /// Collation that were advertised to us, but we did not yet fetch. - pub waiting_queue: VecDeque<(PendingCollation, CollatorId)>, - /// How many collations have been seconded. - pub seconded_count: usize, + /// Collation that were advertised to us, but we did not yet request or fetch. Grouped by + /// `ParaId`. + waiting_queue: BTreeMap>, + /// Number of seconded candidates and claims in the claim queue per `ParaId`. + candidates_state: BTreeMap, } impl Collations { + pub(super) fn new(group_assignments: &Vec) -> Self { + let mut candidates_state = BTreeMap::::new(); + + for para_id in group_assignments { + candidates_state.entry(*para_id).or_default().claims_per_para += 1; + } + + Self { + status: Default::default(), + fetching_from: None, + waiting_queue: Default::default(), + candidates_state, + } + } + /// Note a seconded collation for a given para. - pub(super) fn note_seconded(&mut self) { - self.seconded_count += 1 + pub(super) fn note_seconded(&mut self, para_id: ParaId) { + self.candidates_state.entry(para_id).or_default().seconded_per_para += 1; + gum::trace!( + target: LOG_TARGET, + ?para_id, + new_count=self.candidates_state.entry(para_id).or_default().seconded_per_para, + "Note seconded." + ); + self.status.back_to_waiting(); } - /// Returns the next collation to fetch from the `waiting_queue`. + /// Adds a new collation to the waiting queue for the relay parent. This function doesn't + /// perform any limits check. The caller should assure that the collation limit is respected. + pub(super) fn add_to_waiting_queue(&mut self, collation: (PendingCollation, CollatorId)) { + self.waiting_queue.entry(collation.0.para_id).or_default().push_back(collation); + } + + /// Picks a collation to fetch from the waiting queue. + /// When fetching collations we need to ensure that each parachain has got a fair core time + /// share depending on its assignments in the claim queue. This means that the number of + /// collations seconded per parachain should ideally be equal to the number of claims for the + /// particular parachain in the claim queue. /// - /// This will reset the status back to `Waiting` using [`CollationStatus::back_to_waiting`]. + /// To achieve this each seconded collation is mapped to an entry from the claim queue. The next + /// fetch is the first unfulfilled entry from the claim queue for which there is an + /// advertisement. /// - /// Returns `Some(_)` if there is any collation to fetch, the `status` is not `Seconded` and - /// the passed in `finished_one` is the currently `waiting_collation`. - pub(super) fn get_next_collation_to_fetch( + /// `unfulfilled_claim_queue_entries` represents all claim queue entries which are still not + /// fulfilled. + pub(super) fn pick_a_collation_to_fetch( &mut self, - finished_one: &(CollatorId, Option), - relay_parent_mode: ProspectiveParachainsMode, + unfulfilled_claim_queue_entries: Vec, ) -> Option<(PendingCollation, CollatorId)> { - // If finished one does not match waiting_collation, then we already dequeued another fetch - // to replace it. - if let Some((collator_id, maybe_candidate_hash)) = self.fetching_from.as_ref() { - // If a candidate hash was saved previously, `finished_one` must include this too. - if collator_id != &finished_one.0 && - maybe_candidate_hash.map_or(true, |hash| Some(&hash) != finished_one.1.as_ref()) + gum::trace!( + target: LOG_TARGET, + waiting_queue=?self.waiting_queue, + candidates_state=?self.candidates_state, + "Pick a collation to fetch." + ); + + for assignment in unfulfilled_claim_queue_entries { + // if there is an unfulfilled assignment - return it + if let Some(collation) = self + .waiting_queue + .get_mut(&assignment) + .and_then(|collations| collations.pop_front()) { - gum::trace!( - target: LOG_TARGET, - waiting_collation = ?self.fetching_from, - ?finished_one, - "Not proceeding to the next collation - has already been done." - ); - return None + return Some(collation) } } - self.status.back_to_waiting(relay_parent_mode); - - match self.status { - // We don't need to fetch any other collation when we already have seconded one. - CollationStatus::Seconded => None, - CollationStatus::Waiting => - if self.is_seconded_limit_reached(relay_parent_mode) { - None - } else { - self.waiting_queue.pop_front() - }, - CollationStatus::WaitingOnValidation | CollationStatus::Fetching => - unreachable!("We have reset the status above!"), - } + + None } - /// Checks the limit of seconded candidates. - pub(super) fn is_seconded_limit_reached( - &self, - relay_parent_mode: ProspectiveParachainsMode, - ) -> bool { - let seconded_limit = - if let ProspectiveParachainsMode::Enabled { max_candidate_depth, .. } = - relay_parent_mode - { - max_candidate_depth + 1 - } else { - 1 - }; - self.seconded_count >= seconded_limit + pub(super) fn seconded_for_para(&self, para_id: &ParaId) -> usize { + self.candidates_state + .get(¶_id) + .map(|state| state.seconded_per_para) + .unwrap_or_default() } } diff --git a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs index 36ec959c3406..5f5effcde9a8 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -49,22 +49,25 @@ use polkadot_node_subsystem::{ use polkadot_node_subsystem_util::{ backing_implicit_view::View as ImplicitView, reputation::{ReputationAggregator, REPUTATION_CHANGE_INTERVAL}, - request_claim_queue, request_session_index_for_child, - runtime::{prospective_parachains_mode, request_node_features, ProspectiveParachainsMode}, + request_async_backing_params, request_claim_queue, request_session_index_for_child, + runtime::{recv_runtime, request_node_features}, }; use polkadot_primitives::{ node_features, - vstaging::{CandidateDescriptorV2, CandidateDescriptorVersion, CoreState}, - CandidateHash, CollatorId, CoreIndex, Hash, HeadData, Id as ParaId, OccupiedCoreAssumption, - PersistedValidationData, SessionIndex, + vstaging::{CandidateDescriptorV2, CandidateDescriptorVersion}, + AsyncBackingParams, CandidateHash, CollatorId, CoreIndex, Hash, HeadData, Id as ParaId, + OccupiedCoreAssumption, PersistedValidationData, SessionIndex, }; use crate::error::{Error, FetchError, Result, SecondingError}; use self::collation::BlockedCollationId; +use self::claim_queue_state::ClaimQueueState; + use super::{modify_reputation, tick_stream, LOG_TARGET}; +mod claim_queue_state; mod collation; mod metrics; @@ -163,27 +166,19 @@ impl PeerData { fn update_view( &mut self, implicit_view: &ImplicitView, - active_leaves: &HashMap, - per_relay_parent: &HashMap, + active_leaves: &HashMap, new_view: View, ) { let old_view = std::mem::replace(&mut self.view, new_view); if let PeerState::Collating(ref mut peer_state) = self.state { for removed in old_view.difference(&self.view) { - // Remove relay parent advertisements if it went out - // of our (implicit) view. - let keep = per_relay_parent - .get(removed) - .map(|s| { - is_relay_parent_in_implicit_view( - removed, - s.prospective_parachains_mode, - implicit_view, - active_leaves, - peer_state.para_id, - ) - }) - .unwrap_or(false); + // Remove relay parent advertisements if it went out of our (implicit) view. + let keep = is_relay_parent_in_implicit_view( + removed, + implicit_view, + active_leaves, + peer_state.para_id, + ); if !keep { peer_state.advertisements.remove(&removed); @@ -196,8 +191,7 @@ impl PeerData { fn prune_old_advertisements( &mut self, implicit_view: &ImplicitView, - active_leaves: &HashMap, - per_relay_parent: &HashMap, + active_leaves: &HashMap, ) { if let PeerState::Collating(ref mut peer_state) = self.state { peer_state.advertisements.retain(|hash, _| { @@ -205,36 +199,30 @@ impl PeerData { // - Relay parent is an active leaf // - It belongs to allowed ancestry under some leaf // Discard otherwise. - per_relay_parent.get(hash).map_or(false, |s| { - is_relay_parent_in_implicit_view( - hash, - s.prospective_parachains_mode, - implicit_view, - active_leaves, - peer_state.para_id, - ) - }) + is_relay_parent_in_implicit_view( + hash, + implicit_view, + active_leaves, + peer_state.para_id, + ) }); } } - /// Note an advertisement by the collator. Returns `true` if the advertisement was imported - /// successfully. Fails if the advertisement is duplicate, out of view, or the peer has not - /// declared itself a collator. + /// Performs sanity check for an advertisement and notes it as advertised. fn insert_advertisement( &mut self, on_relay_parent: Hash, - relay_parent_mode: ProspectiveParachainsMode, candidate_hash: Option, implicit_view: &ImplicitView, - active_leaves: &HashMap, + active_leaves: &HashMap, + per_relay_parent: &PerRelayParent, ) -> std::result::Result<(CollatorId, ParaId), InsertAdvertisementError> { match self.state { PeerState::Connected(_) => Err(InsertAdvertisementError::UndeclaredCollator), PeerState::Collating(ref mut state) => { if !is_relay_parent_in_implicit_view( &on_relay_parent, - relay_parent_mode, implicit_view, active_leaves, state.para_id, @@ -242,53 +230,41 @@ impl PeerData { return Err(InsertAdvertisementError::OutOfOurView) } - match (relay_parent_mode, candidate_hash) { - (ProspectiveParachainsMode::Disabled, candidate_hash) => { - if state.advertisements.contains_key(&on_relay_parent) { - return Err(InsertAdvertisementError::Duplicate) - } - state - .advertisements - .insert(on_relay_parent, HashSet::from_iter(candidate_hash)); - }, - ( - ProspectiveParachainsMode::Enabled { max_candidate_depth, .. }, - candidate_hash, - ) => { - if let Some(candidate_hash) = candidate_hash { - if state - .advertisements - .get(&on_relay_parent) - .map_or(false, |candidates| candidates.contains(&candidate_hash)) - { - return Err(InsertAdvertisementError::Duplicate) - } - - let candidates = - state.advertisements.entry(on_relay_parent).or_default(); - - if candidates.len() > max_candidate_depth { - return Err(InsertAdvertisementError::PeerLimitReached) - } - candidates.insert(candidate_hash); - } else { - if self.version != CollationVersion::V1 { - gum::error!( - target: LOG_TARGET, - "Programming error, `candidate_hash` can not be `None` \ - for non `V1` networking.", - ); - } - - if state.advertisements.contains_key(&on_relay_parent) { - return Err(InsertAdvertisementError::Duplicate) - } - state - .advertisements - .insert(on_relay_parent, HashSet::from_iter(candidate_hash)); - }; - }, - } + if let Some(candidate_hash) = candidate_hash { + if state + .advertisements + .get(&on_relay_parent) + .map_or(false, |candidates| candidates.contains(&candidate_hash)) + { + return Err(InsertAdvertisementError::Duplicate) + } + + let candidates = state.advertisements.entry(on_relay_parent).or_default(); + + // Current assignments is equal to the length of the claim queue. No honest + // collator should send that many advertisements. + if candidates.len() > per_relay_parent.assignment.current.len() { + return Err(InsertAdvertisementError::PeerLimitReached) + } + + candidates.insert(candidate_hash); + } else { + if self.version != CollationVersion::V1 { + gum::error!( + target: LOG_TARGET, + "Programming error, `candidate_hash` can not be `None` \ + for non `V1` networking.", + ); + } + + if state.advertisements.contains_key(&on_relay_parent) { + return Err(InsertAdvertisementError::Duplicate) + } + + state + .advertisements + .insert(on_relay_parent, HashSet::from_iter(candidate_hash)); + }; state.last_active = Instant::now(); Ok((state.collator_id.clone(), state.para_id)) @@ -369,7 +345,6 @@ struct GroupAssignments { } struct PerRelayParent { - prospective_parachains_mode: ProspectiveParachainsMode, assignment: GroupAssignments, collations: Collations, v2_receipts: bool, @@ -390,11 +365,10 @@ struct State { /// ancestry of some active leaf, then it does support prospective parachains. implicit_view: ImplicitView, - /// All active leaves observed by us, including both that do and do not - /// support prospective parachains. This mapping works as a replacement for + /// All active leaves observed by us. This mapping works as a replacement for /// [`polkadot_node_network_protocol::View`] and can be dropped once the transition /// to asynchronous backing is done. - active_leaves: HashMap, + active_leaves: HashMap, /// State tracked per relay parent. per_relay_parent: HashMap, @@ -437,23 +411,69 @@ struct State { reputation: ReputationAggregator, } +impl State { + // Returns the number of seconded and pending collations for a specific `ParaId`. Pending + // collations are: + // 1. Collations being fetched from a collator. + // 2. Collations waiting for validation from backing subsystem. + // 3. Collations blocked from seconding due to parent not being known by backing subsystem. + fn seconded_and_pending_for_para(&self, relay_parent: &Hash, para_id: &ParaId) -> usize { + let seconded = self + .per_relay_parent + .get(relay_parent) + .map_or(0, |per_relay_parent| per_relay_parent.collations.seconded_for_para(para_id)); + + let pending_fetch = self.per_relay_parent.get(relay_parent).map_or(0, |rp_state| { + match rp_state.collations.status { + CollationStatus::Fetching(pending_para_id) if pending_para_id == *para_id => 1, + _ => 0, + } + }); + + let waiting_for_validation = self + .fetched_candidates + .keys() + .filter(|fc| fc.relay_parent == *relay_parent && fc.para_id == *para_id) + .count(); + + let blocked_from_seconding = + self.blocked_from_seconding.values().fold(0, |acc, blocked_collations| { + acc + blocked_collations + .iter() + .filter(|pc| { + pc.candidate_receipt.descriptor.para_id() == *para_id && + pc.candidate_receipt.descriptor.relay_parent() == *relay_parent + }) + .count() + }); + + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + ?para_id, + seconded, + pending_fetch, + waiting_for_validation, + blocked_from_seconding, + "Seconded and pending collations for para", + ); + + seconded + pending_fetch + waiting_for_validation + blocked_from_seconding + } +} + fn is_relay_parent_in_implicit_view( relay_parent: &Hash, - relay_parent_mode: ProspectiveParachainsMode, implicit_view: &ImplicitView, - active_leaves: &HashMap, + active_leaves: &HashMap, para_id: ParaId, ) -> bool { - match relay_parent_mode { - ProspectiveParachainsMode::Disabled => active_leaves.contains_key(relay_parent), - ProspectiveParachainsMode::Enabled { .. } => active_leaves.iter().any(|(hash, mode)| { - mode.is_enabled() && - implicit_view - .known_allowed_relay_parents_under(hash, Some(para_id)) - .unwrap_or_default() - .contains(relay_parent) - }), - } + active_leaves.iter().any(|(hash, _)| { + implicit_view + .known_allowed_relay_parents_under(hash, Some(para_id)) + .unwrap_or_default() + .contains(relay_parent) + }) } async fn construct_per_relay_parent( @@ -461,7 +481,6 @@ async fn construct_per_relay_parent( current_assignments: &mut HashMap, keystore: &KeystorePtr, relay_parent: Hash, - relay_parent_mode: ProspectiveParachainsMode, v2_receipts: bool, session_index: SessionIndex, ) -> Result> @@ -479,39 +498,24 @@ where .await .map_err(Error::CancelledValidatorGroups)??; - let cores = polkadot_node_subsystem_util::request_availability_cores(relay_parent, sender) - .await - .await - .map_err(Error::CancelledAvailabilityCores)??; - let core_now = if let Some(group) = polkadot_node_subsystem_util::signing_key_and_index(&validators, keystore).and_then( |(_, index)| polkadot_node_subsystem_util::find_validator_group(&groups, index), ) { - rotation_info.core_for_group(group, cores.len()) + rotation_info.core_for_group(group, groups.len()) } else { gum::trace!(target: LOG_TARGET, ?relay_parent, "Not a validator"); return Ok(None) }; - let claim_queue = request_claim_queue(relay_parent, sender) + let mut claim_queue = request_claim_queue(relay_parent, sender) .await .await .map_err(Error::CancelledClaimQueue)??; - let paras_now = cores - .get(core_now.0 as usize) - .and_then(|c| match (c, relay_parent_mode) { - (CoreState::Occupied(_), ProspectiveParachainsMode::Disabled) => None, - ( - CoreState::Occupied(_), - ProspectiveParachainsMode::Enabled { max_candidate_depth: 0, .. }, - ) => None, - _ => claim_queue.get(&core_now).cloned(), - }) - .unwrap_or_else(|| VecDeque::new()); - - for para_id in paras_now.iter() { + let assigned_paras = claim_queue.remove(&core_now).unwrap_or_else(|| VecDeque::new()); + + for para_id in assigned_paras.iter() { let entry = current_assignments.entry(*para_id).or_default(); *entry += 1; if *entry == 1 { @@ -524,10 +528,12 @@ where } } + let assignment = GroupAssignments { current: assigned_paras.into_iter().collect() }; + let collations = Collations::new(&assignment.current); + Ok(Some(PerRelayParent { - prospective_parachains_mode: relay_parent_mode, - assignment: GroupAssignments { current: paras_now.into_iter().collect() }, - collations: Collations::default(), + assignment, + collations, v2_receipts, session_index, current_core: core_now, @@ -655,12 +661,7 @@ fn handle_peer_view_change(state: &mut State, peer_id: PeerId, view: View) { None => return, }; - peer_data.update_view( - &state.implicit_view, - &state.active_leaves, - &state.per_relay_parent, - view, - ); + peer_data.update_view(&state.implicit_view, &state.active_leaves, view); state.collation_requests_cancel_handles.retain(|pc, handle| { let keep = pc.peer_id != peer_id || peer_data.has_advertised(&pc.relay_parent, None); if !keep { @@ -693,7 +694,6 @@ async fn request_collation( .get_mut(&relay_parent) .ok_or(FetchError::RelayParentOutOfView)?; - // Relay parent mode is checked in `handle_advertisement`. let (requests, response_recv) = match (peer_protocol_version, prospective_candidate) { (CollationVersion::V1, _) => { let (req, response_recv) = OutgoingRequest::new( @@ -739,7 +739,7 @@ async fn request_collation( let maybe_candidate_hash = prospective_candidate.as_ref().map(ProspectiveCandidate::candidate_hash); - per_relay_parent.collations.status = CollationStatus::Fetching; + per_relay_parent.collations.status = CollationStatus::Fetching(para_id); per_relay_parent .collations .fetching_from @@ -1050,6 +1050,62 @@ async fn second_unblocked_collations( } } +fn ensure_seconding_limit_is_respected( + relay_parent: &Hash, + para_id: ParaId, + state: &State, +) -> std::result::Result<(), AdvertisementError> { + let paths = state.implicit_view.paths_via_relay_parent(relay_parent); + + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + ?para_id, + ?paths, + "Checking seconding limit", + ); + + let mut has_claim_at_some_path = false; + for path in paths { + let mut cq_state = ClaimQueueState::new(); + for ancestor in &path { + let seconded_and_pending = state.seconded_and_pending_for_para(&ancestor, ¶_id); + cq_state.add_leaf( + &ancestor, + &state + .per_relay_parent + .get(ancestor) + .ok_or(AdvertisementError::RelayParentUnknown)? + .assignment + .current, + ); + for _ in 0..seconded_and_pending { + cq_state.claim_at(ancestor, ¶_id); + } + } + + if cq_state.can_claim_at(relay_parent, ¶_id) { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + ?para_id, + ?path, + "Seconding limit respected at path", + ); + has_claim_at_some_path = true; + break + } + } + + // If there is a place in the claim queue for the candidate at at least one path we will accept + // it. + if has_claim_at_some_path { + Ok(()) + } else { + Err(AdvertisementError::SecondedLimitReached) + } +} + async fn handle_advertisement( sender: &mut Sender, state: &mut State, @@ -1072,7 +1128,6 @@ where .get(&relay_parent) .ok_or(AdvertisementError::RelayParentUnknown)?; - let relay_parent_mode = per_relay_parent.prospective_parachains_mode; let assignment = &per_relay_parent.assignment; let collator_para_id = @@ -1088,32 +1143,29 @@ where let (collator_id, para_id) = peer_data .insert_advertisement( relay_parent, - relay_parent_mode, candidate_hash, &state.implicit_view, &state.active_leaves, + &per_relay_parent, ) .map_err(AdvertisementError::Invalid)?; - if per_relay_parent.collations.is_seconded_limit_reached(relay_parent_mode) { - return Err(AdvertisementError::SecondedLimitReached) - } + ensure_seconding_limit_is_respected(&relay_parent, para_id, state)?; if let Some((candidate_hash, parent_head_data_hash)) = prospective_candidate { // Check if backing subsystem allows to second this candidate. // // This is also only important when async backing or elastic scaling is enabled. - let seconding_not_allowed = relay_parent_mode.is_enabled() && - !can_second( - sender, - collator_para_id, - relay_parent, - candidate_hash, - parent_head_data_hash, - ) - .await; + let can_second = can_second( + sender, + collator_para_id, + relay_parent, + candidate_hash, + parent_head_data_hash, + ) + .await; - if seconding_not_allowed { + if !can_second { return Err(AdvertisementError::BlockedByBacking) } } @@ -1143,8 +1195,8 @@ where Ok(()) } -/// Enqueue collation for fetching. The advertisement is expected to be -/// validated. +/// Enqueue collation for fetching. The advertisement is expected to be validated and the seconding +/// limit checked. async fn enqueue_collation( sender: &mut Sender, state: &mut State, @@ -1179,7 +1231,6 @@ where return Ok(()) }, }; - let relay_parent_mode = per_relay_parent.prospective_parachains_mode; let prospective_candidate = prospective_candidate.map(|(candidate_hash, parent_head_data_hash)| ProspectiveCandidate { candidate_hash, @@ -1187,22 +1238,11 @@ where }); let collations = &mut per_relay_parent.collations; - if collations.is_seconded_limit_reached(relay_parent_mode) { - gum::trace!( - target: LOG_TARGET, - peer_id = ?peer_id, - %para_id, - ?relay_parent, - "Limit of seconded collations reached for valid advertisement", - ); - return Ok(()) - } - let pending_collation = PendingCollation::new(relay_parent, para_id, &peer_id, prospective_candidate); match collations.status { - CollationStatus::Fetching | CollationStatus::WaitingOnValidation => { + CollationStatus::Fetching(_) | CollationStatus::WaitingOnValidation => { gum::trace!( target: LOG_TARGET, peer_id = ?peer_id, @@ -1210,26 +1250,13 @@ where ?relay_parent, "Added collation to the pending list" ); - collations.waiting_queue.push_back((pending_collation, collator_id)); + collations.add_to_waiting_queue((pending_collation, collator_id)); }, CollationStatus::Waiting => { + // We were waiting for a collation to be advertised to us (we were idle) so we can fetch + // the new collation immediately fetch_collation(sender, state, pending_collation, collator_id).await?; }, - CollationStatus::Seconded if relay_parent_mode.is_enabled() => { - // Limit is not reached, it's allowed to second another - // collation. - fetch_collation(sender, state, pending_collation, collator_id).await?; - }, - CollationStatus::Seconded => { - gum::trace!( - target: LOG_TARGET, - peer_id = ?peer_id, - %para_id, - ?relay_parent, - ?relay_parent_mode, - "A collation has already been seconded", - ); - }, } Ok(()) @@ -1255,7 +1282,10 @@ where .await .await .map_err(Error::CancelledSessionIndex)??; - let mode = prospective_parachains_mode(sender, *leaf).await?; + + let async_backing_params = + recv_runtime(request_async_backing_params(*leaf, sender).await).await?; + let v2_receipts = request_node_features(*leaf, session_index, sender) .await? .unwrap_or_default() @@ -1268,7 +1298,6 @@ where &mut state.current_assignments, keystore, *leaf, - mode, v2_receipts, session_index, ) @@ -1277,53 +1306,53 @@ where continue }; - state.active_leaves.insert(*leaf, mode); + state.active_leaves.insert(*leaf, async_backing_params); state.per_relay_parent.insert(*leaf, per_relay_parent); - if mode.is_enabled() { - state - .implicit_view - .activate_leaf(sender, *leaf) - .await - .map_err(Error::ImplicitViewFetchError)?; - - // Order is always descending. - let allowed_ancestry = state - .implicit_view - .known_allowed_relay_parents_under(leaf, None) - .unwrap_or_default(); - for block_hash in allowed_ancestry { - if let Entry::Vacant(entry) = state.per_relay_parent.entry(*block_hash) { - // Safe to use the same v2 receipts config for the allowed relay parents as well - // as the same session index since they must be in the same session. - if let Some(per_relay_parent) = construct_per_relay_parent( - sender, - &mut state.current_assignments, - keystore, - *block_hash, - mode, - v2_receipts, - session_index, - ) - .await? - { - entry.insert(per_relay_parent); - } + state + .implicit_view + .activate_leaf(sender, *leaf) + .await + .map_err(Error::ImplicitViewFetchError)?; + + // Order is always descending. + let allowed_ancestry = state + .implicit_view + .known_allowed_relay_parents_under(leaf, None) + .unwrap_or_default(); + for block_hash in allowed_ancestry { + if let Entry::Vacant(entry) = state.per_relay_parent.entry(*block_hash) { + // Safe to use the same v2 receipts config for the allowed relay parents as well + // as the same session index since they must be in the same session. + if let Some(per_relay_parent) = construct_per_relay_parent( + sender, + &mut state.current_assignments, + keystore, + *block_hash, + v2_receipts, + session_index, + ) + .await? + { + entry.insert(per_relay_parent); } } } } - for (removed, mode) in removed { + for (removed, _) in removed { + gum::trace!( + target: LOG_TARGET, + ?view, + ?removed, + "handle_our_view_change - removed", + ); + state.active_leaves.remove(removed); // If the leaf is deactivated it still may stay in the view as a part // of implicit ancestry. Only update the state after the hash is actually // pruned from the block info storage. - let pruned = if mode.is_enabled() { - state.implicit_view.deactivate_leaf(*removed) - } else { - vec![*removed] - }; + let pruned = state.implicit_view.deactivate_leaf(*removed); for removed in pruned { if let Some(per_relay_parent) = state.per_relay_parent.remove(&removed) { @@ -1353,11 +1382,7 @@ where }); for (peer_id, peer_data) in state.peer_data.iter_mut() { - peer_data.prune_old_advertisements( - &state.implicit_view, - &state.active_leaves, - &state.per_relay_parent, - ); + peer_data.prune_old_advertisements(&state.implicit_view, &state.active_leaves); // Disconnect peers who are not relevant to our current or next para. // @@ -1490,8 +1515,9 @@ async fn process_msg( if let Some(CollationEvent { collator_id, pending_collation, .. }) = state.fetched_candidates.remove(&fetched_collation) { - let PendingCollation { relay_parent, peer_id, prospective_candidate, .. } = - pending_collation; + let PendingCollation { + relay_parent, peer_id, prospective_candidate, para_id, .. + } = pending_collation; note_good_collation( &mut state.reputation, ctx.sender(), @@ -1511,8 +1537,7 @@ async fn process_msg( } if let Some(rp_state) = state.per_relay_parent.get_mut(&parent) { - rp_state.collations.status = CollationStatus::Seconded; - rp_state.collations.note_seconded(); + rp_state.collations.note_seconded(para_id); } // See if we've unblocked other collations for seconding. @@ -1641,6 +1666,7 @@ async fn run_inner( disconnect_inactive_peers(ctx.sender(), &eviction_policy, &state.peer_data).await; }, resp = state.collation_requests.select_next_some() => { + let relay_parent = resp.0.pending_collation.relay_parent; let res = match handle_collation_fetch_response( &mut state, resp, @@ -1649,9 +1675,17 @@ async fn run_inner( ).await { Err(Some((peer_id, rep))) => { modify_reputation(&mut state.reputation, ctx.sender(), peer_id, rep).await; + // Reset the status for the relay parent + state.per_relay_parent.get_mut(&relay_parent).map(|rp| { + rp.collations.status.back_to_waiting(); + }); continue }, Err(None) => { + // Reset the status for the relay parent + state.per_relay_parent.get_mut(&relay_parent).map(|rp| { + rp.collations.status.back_to_waiting(); + }); continue }, Ok(res) => res @@ -1730,11 +1764,7 @@ async fn dequeue_next_collation_and_fetch( // The collator we tried to fetch from last, optionally which candidate. previous_fetch: (CollatorId, Option), ) { - while let Some((next, id)) = state.per_relay_parent.get_mut(&relay_parent).and_then(|state| { - state - .collations - .get_next_collation_to_fetch(&previous_fetch, state.prospective_parachains_mode) - }) { + while let Some((next, id)) = get_next_collation_to_fetch(&previous_fetch, relay_parent, state) { gum::debug!( target: LOG_TARGET, ?relay_parent, @@ -1843,9 +1873,7 @@ async fn kick_off_seconding( collation_event.collator_protocol_version, collation_event.pending_collation.prospective_candidate, ) { - (CollationVersion::V2, Some(ProspectiveCandidate { parent_head_data_hash, .. })) - if per_relay_parent.prospective_parachains_mode.is_enabled() => - { + (CollationVersion::V2, Some(ProspectiveCandidate { parent_head_data_hash, .. })) => { let pvd = request_prospective_validation_data( ctx.sender(), relay_parent, @@ -1857,8 +1885,7 @@ async fn kick_off_seconding( (pvd, maybe_parent_head_data, Some(parent_head_data_hash)) }, - // Support V2 collators without async backing enabled. - (CollationVersion::V2, Some(_)) | (CollationVersion::V1, _) => { + (CollationVersion::V1, _) => { let pvd = request_persisted_validation_data( ctx.sender(), candidate_receipt.descriptor().relay_parent(), @@ -2107,6 +2134,106 @@ async fn handle_collation_fetch_response( result } +// Returns the claim queue without fetched or pending advertisement. The resulting `Vec` keeps the +// order in the claim queue so the earlier an element is located in the `Vec` the higher its +// priority is. +fn unfulfilled_claim_queue_entries(relay_parent: &Hash, state: &State) -> Result> { + let relay_parent_state = state + .per_relay_parent + .get(relay_parent) + .ok_or(Error::RelayParentStateNotFound)?; + let scheduled_paras = relay_parent_state.assignment.current.iter().collect::>(); + let paths = state.implicit_view.paths_via_relay_parent(relay_parent); + + let mut claim_queue_states = Vec::new(); + for path in paths { + let mut cq_state = ClaimQueueState::new(); + for ancestor in &path { + cq_state.add_leaf( + &ancestor, + &state + .per_relay_parent + .get(&ancestor) + .ok_or(Error::RelayParentStateNotFound)? + .assignment + .current, + ); + + for para_id in &scheduled_paras { + let seconded_and_pending = state.seconded_and_pending_for_para(&ancestor, ¶_id); + for _ in 0..seconded_and_pending { + cq_state.claim_at(&ancestor, ¶_id); + } + } + } + claim_queue_states.push(cq_state); + } + + // From the claim queue state for each leaf we have to return a combined single one. Go for a + // simple solution and return the longest one. In theory we always prefer the earliest entries + // in the claim queue so there is a good chance that the longest path is the one with + // unsatisfied entries in the beginning. This is not guaranteed as we might have fetched 2nd or + // 3rd spot from the claim queue but it should be good enough. + let unfulfilled_entries = claim_queue_states + .iter_mut() + .map(|cq| cq.unclaimed_at(relay_parent)) + .max_by(|a, b| a.len().cmp(&b.len())) + .unwrap_or_default(); + + Ok(unfulfilled_entries) +} + +/// Returns the next collation to fetch from the `waiting_queue` and reset the status back to +/// `Waiting`. +fn get_next_collation_to_fetch( + finished_one: &(CollatorId, Option), + relay_parent: Hash, + state: &mut State, +) -> Option<(PendingCollation, CollatorId)> { + let unfulfilled_entries = match unfulfilled_claim_queue_entries(&relay_parent, &state) { + Ok(entries) => entries, + Err(err) => { + gum::error!( + target: LOG_TARGET, + ?relay_parent, + ?err, + "Failed to get unfulfilled claim queue entries" + ); + return None + }, + }; + let rp_state = match state.per_relay_parent.get_mut(&relay_parent) { + Some(rp_state) => rp_state, + None => { + gum::error!( + target: LOG_TARGET, + ?relay_parent, + "Failed to get relay parent state" + ); + return None + }, + }; + + // If finished one does not match waiting_collation, then we already dequeued another fetch + // to replace it. + if let Some((collator_id, maybe_candidate_hash)) = rp_state.collations.fetching_from.as_ref() { + // If a candidate hash was saved previously, `finished_one` must include this too. + if collator_id != &finished_one.0 && + maybe_candidate_hash.map_or(true, |hash| Some(&hash) != finished_one.1.as_ref()) + { + gum::trace!( + target: LOG_TARGET, + waiting_collation = ?rp_state.collations.fetching_from, + ?finished_one, + "Not proceeding to the next collation - has already been done." + ); + return None + } + } + rp_state.collations.status.back_to_waiting(); + rp_state.collations.pick_a_collation_to_fetch(unfulfilled_entries) +} + // Sanity check the candidate descriptor version. fn descriptor_version_sanity_check( descriptor: &CandidateDescriptorV2, diff --git a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs index f2f23c188a66..5a2e135419dd 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs @@ -28,28 +28,24 @@ use std::{ time::Duration, }; +use self::prospective_parachains::update_view; use polkadot_node_network_protocol::{ - our_view, peer_set::CollationVersion, request_response::{Requests, ResponseSender}, ObservedRole, }; use polkadot_node_primitives::{BlockData, PoV}; -use polkadot_node_subsystem::{ - errors::RuntimeApiError, - messages::{AllMessages, ReportPeerMessage, RuntimeApiMessage, RuntimeApiRequest}, +use polkadot_node_subsystem::messages::{ + AllMessages, ReportPeerMessage, RuntimeApiMessage, RuntimeApiRequest, }; use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_node_subsystem_util::{reputation::add_reputation, TimeoutExt}; use polkadot_primitives::{ - node_features, - vstaging::{CandidateReceiptV2 as CandidateReceipt, CoreState, OccupiedCore}, - CollatorPair, CoreIndex, GroupIndex, GroupRotationInfo, HeadData, NodeFeatures, - PersistedValidationData, ScheduledCore, ValidatorId, ValidatorIndex, -}; -use polkadot_primitives_test_helpers::{ - dummy_candidate_descriptor, dummy_candidate_receipt_bad_sig, dummy_hash, + node_features, vstaging::CandidateReceiptV2 as CandidateReceipt, AsyncBackingParams, + CollatorPair, CoreIndex, GroupRotationInfo, HeadData, NodeFeatures, PersistedValidationData, + ValidatorId, ValidatorIndex, }; +use polkadot_primitives_test_helpers::{dummy_candidate_receipt_bad_sig, dummy_hash}; mod prospective_parachains; @@ -57,9 +53,6 @@ const ACTIVITY_TIMEOUT: Duration = Duration::from_millis(500); const DECLARE_TIMEOUT: Duration = Duration::from_millis(25); const REPUTATION_CHANGE_TEST_INTERVAL: Duration = Duration::from_millis(10); -const ASYNC_BACKING_DISABLED_ERROR: RuntimeApiError = - RuntimeApiError::NotSupported { runtime_api_name: "test-runtime" }; - fn dummy_pvd() -> PersistedValidationData { PersistedValidationData { parent_head: HeadData(vec![7, 8, 9]), @@ -77,19 +70,17 @@ struct TestState { validator_public: Vec, validator_groups: Vec>, group_rotation_info: GroupRotationInfo, - cores: Vec, claim_queue: BTreeMap>, + async_backing_params: AsyncBackingParams, node_features: NodeFeatures, session_index: SessionIndex, + // Used by `update_view` to keep track of latest requested ancestor + last_known_block: Option, } impl Default for TestState { fn default() -> Self { - let chain_a = ParaId::from(1); - let chain_b = ParaId::from(2); - - let chain_ids = vec![chain_a, chain_b]; - let relay_parent = Hash::repeat_byte(0x05); + let relay_parent = Hash::from_low_u64_be(0x05); let collators = iter::repeat(()).map(|_| CollatorPair::generate().0).take(5).collect(); let validators = vec![ @@ -110,50 +101,103 @@ impl Default for TestState { let group_rotation_info = GroupRotationInfo { session_start_block: 0, group_rotation_frequency: 1, now: 0 }; - let cores = vec![ - CoreState::Scheduled(ScheduledCore { para_id: chain_ids[0], collator: None }), - CoreState::Free, - CoreState::Occupied(OccupiedCore { - next_up_on_available: Some(ScheduledCore { para_id: chain_ids[1], collator: None }), - occupied_since: 0, - time_out_at: 1, - next_up_on_time_out: None, - availability: Default::default(), - group_responsible: GroupIndex(0), - candidate_hash: Default::default(), - candidate_descriptor: { - let mut d = dummy_candidate_descriptor(dummy_hash()); - d.para_id = chain_ids[1]; - - d.into() - }, - }), - ]; - let mut claim_queue = BTreeMap::new(); - claim_queue.insert(CoreIndex(0), [chain_ids[0]].into_iter().collect()); + claim_queue.insert( + CoreIndex(0), + iter::repeat(ParaId::from(Self::CHAIN_IDS[0])) + .take(Self::ASYNC_BACKING_PARAMS.allowed_ancestry_len as usize) + .collect(), + ); claim_queue.insert(CoreIndex(1), VecDeque::new()); - claim_queue.insert(CoreIndex(2), [chain_ids[1]].into_iter().collect()); + claim_queue.insert( + CoreIndex(2), + iter::repeat(ParaId::from(Self::CHAIN_IDS[1])) + .take(Self::ASYNC_BACKING_PARAMS.allowed_ancestry_len as usize) + .collect(), + ); let mut node_features = NodeFeatures::EMPTY; node_features.resize(node_features::FeatureIndex::CandidateReceiptV2 as usize + 1, false); node_features.set(node_features::FeatureIndex::CandidateReceiptV2 as u8 as usize, true); Self { - chain_ids, + chain_ids: Self::CHAIN_IDS.map(|id| ParaId::from(id)).to_vec(), relay_parent, collators, validator_public, validator_groups, group_rotation_info, - cores, claim_queue, + async_backing_params: Self::ASYNC_BACKING_PARAMS, node_features, session_index: 1, + last_known_block: None, } } } +impl TestState { + const CHAIN_IDS: [u32; 2] = [1, 2]; + const ASYNC_BACKING_PARAMS: AsyncBackingParams = + AsyncBackingParams { max_candidate_depth: 4, allowed_ancestry_len: 3 }; + + fn with_shared_core() -> Self { + let mut state = Self::default(); + + let mut claim_queue = BTreeMap::new(); + claim_queue.insert( + CoreIndex(0), + VecDeque::from_iter( + [ + ParaId::from(Self::CHAIN_IDS[1]), + ParaId::from(Self::CHAIN_IDS[0]), + ParaId::from(Self::CHAIN_IDS[0]), + ] + .into_iter(), + ), + ); + state.validator_groups.truncate(1); + + assert!( + claim_queue.get(&CoreIndex(0)).unwrap().len() == + Self::ASYNC_BACKING_PARAMS.allowed_ancestry_len as usize + ); + + state.claim_queue = claim_queue; + + state + } + + fn with_one_scheduled_para() -> Self { + let mut state = Self::default(); + + let validator_groups = vec![vec![ValidatorIndex(0), ValidatorIndex(1)]]; + + let mut claim_queue = BTreeMap::new(); + claim_queue.insert( + CoreIndex(0), + VecDeque::from_iter( + [ + ParaId::from(Self::CHAIN_IDS[0]), + ParaId::from(Self::CHAIN_IDS[0]), + ParaId::from(Self::CHAIN_IDS[0]), + ] + .into_iter(), + ), + ); + + assert!( + claim_queue.get(&CoreIndex(0)).unwrap().len() == + Self::ASYNC_BACKING_PARAMS.allowed_ancestry_len as usize + ); + + state.validator_groups = validator_groups; + state.claim_queue = claim_queue; + + state + } +} + type VirtualOverseer = polkadot_node_subsystem_test_helpers::TestSubsystemContextHandle; @@ -246,91 +290,6 @@ async fn overseer_signal(overseer: &mut VirtualOverseer, signal: OverseerSignal) .expect(&format!("{:?} is more than enough for sending signals.", TIMEOUT)); } -async fn respond_to_runtime_api_queries( - virtual_overseer: &mut VirtualOverseer, - test_state: &TestState, - hash: Hash, -) { - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - rp, - RuntimeApiRequest::SessionIndexForChild(tx) - )) => { - assert_eq!(rp, hash); - tx.send(Ok(test_state.session_index)).unwrap(); - } - ); - - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - rp, - RuntimeApiRequest::AsyncBackingParams(tx) - )) => { - assert_eq!(rp, hash); - tx.send(Err(ASYNC_BACKING_DISABLED_ERROR)).unwrap(); - } - ); - - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - rp, - RuntimeApiRequest::NodeFeatures(_, tx) - )) => { - assert_eq!(rp, hash); - tx.send(Ok(test_state.node_features.clone())).unwrap(); - } - ); - - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - _, - RuntimeApiRequest::Validators(tx), - )) => { - let _ = tx.send(Ok(test_state.validator_public.clone())); - } - ); - - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - rp, - RuntimeApiRequest::ValidatorGroups(tx), - )) => { - assert_eq!(rp, hash); - let _ = tx.send(Ok(( - test_state.validator_groups.clone(), - test_state.group_rotation_info.clone(), - ))); - } - ); - - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - rp, - RuntimeApiRequest::AvailabilityCores(tx), - )) => { - assert_eq!(rp, hash); - let _ = tx.send(Ok(test_state.cores.clone())); - } - ); - - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - rp, - RuntimeApiRequest::ClaimQueue(tx), - )) => { - assert_eq!(rp, hash); - let _ = tx.send(Ok(test_state.claim_queue.clone())); - } - ); -} - /// Assert that the next message is a `CandidateBacking(Second())`. async fn assert_candidate_backing_second( virtual_overseer: &mut VirtualOverseer, @@ -506,138 +465,6 @@ async fn advertise_collation( .await; } -// As we receive a relevant advertisement act on it and issue a collation request. -#[test] -fn act_on_advertisement() { - let test_state = TestState::default(); - - test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { - let TestHarness { mut virtual_overseer, .. } = test_harness; - - let pair = CollatorPair::generate().0; - gum::trace!("activating"); - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; - - let peer_b = PeerId::random(); - - connect_and_declare_collator( - &mut virtual_overseer, - peer_b, - pair.clone(), - test_state.chain_ids[0], - CollationVersion::V1, - ) - .await; - - advertise_collation(&mut virtual_overseer, peer_b, test_state.relay_parent, None).await; - - assert_fetch_collation_request( - &mut virtual_overseer, - test_state.relay_parent, - test_state.chain_ids[0], - None, - ) - .await; - - virtual_overseer - }); -} - -/// Tests that validator side works with v2 network protocol -/// before async backing is enabled. -#[test] -fn act_on_advertisement_v2() { - let test_state = TestState::default(); - - test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { - let TestHarness { mut virtual_overseer, .. } = test_harness; - - let pair = CollatorPair::generate().0; - gum::trace!("activating"); - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; - - let peer_b = PeerId::random(); - - connect_and_declare_collator( - &mut virtual_overseer, - peer_b, - pair.clone(), - test_state.chain_ids[0], - CollationVersion::V2, - ) - .await; - - let pov = PoV { block_data: BlockData(vec![]) }; - let mut candidate_a = - dummy_candidate_receipt_bad_sig(dummy_hash(), Some(Default::default())); - candidate_a.descriptor.para_id = test_state.chain_ids[0]; - candidate_a.descriptor.relay_parent = test_state.relay_parent; - candidate_a.descriptor.persisted_validation_data_hash = dummy_pvd().hash(); - - let candidate_hash = candidate_a.hash(); - let parent_head_data_hash = Hash::zero(); - // v2 advertisement. - advertise_collation( - &mut virtual_overseer, - peer_b, - test_state.relay_parent, - Some((candidate_hash, parent_head_data_hash)), - ) - .await; - - let response_channel = assert_fetch_collation_request( - &mut virtual_overseer, - test_state.relay_parent, - test_state.chain_ids[0], - Some(candidate_hash), - ) - .await; - - response_channel - .send(Ok(( - request_v1::CollationFetchingResponse::Collation( - candidate_a.clone().into(), - pov.clone(), - ) - .encode(), - ProtocolName::from(""), - ))) - .expect("Sending response should succeed"); - - assert_candidate_backing_second( - &mut virtual_overseer, - test_state.relay_parent, - test_state.chain_ids[0], - &pov, - // Async backing isn't enabled and thus it should do it the old way. - CollationVersion::V1, - ) - .await; - - virtual_overseer - }); -} - // Test that we verify the signatures on `Declare` and `AdvertiseCollation` messages. #[test] fn collator_authentication_verification_works() { @@ -687,31 +514,18 @@ fn collator_authentication_verification_works() { }); } -/// Tests that a validator fetches only one collation at any moment of time -/// per relay parent and ignores other advertisements once a candidate gets -/// seconded. +/// Tests that on a V1 Advertisement a validator fetches only one collation at any moment of time +/// per relay parent and ignores other V1 advertisements once a candidate gets seconded. #[test] -fn fetch_one_collation_at_a_time() { - let test_state = TestState::default(); +fn fetch_one_collation_at_a_time_for_v1_advertisement() { + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; - let second = Hash::random(); - - let our_view = our_view![test_state.relay_parent, second]; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view.clone(), - )), - ) - .await; - - // Iter over view since the order may change due to sorted invariant. - for hash in our_view.iter() { - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, *hash).await; - } + let second = Hash::from_low_u64_be(test_state.relay_parent.to_low_u64_be() - 1); + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0), (second, 1)]) + .await; let peer_b = PeerId::random(); let peer_c = PeerId::random(); @@ -734,8 +548,8 @@ fn fetch_one_collation_at_a_time() { ) .await; - advertise_collation(&mut virtual_overseer, peer_b, test_state.relay_parent, None).await; - advertise_collation(&mut virtual_overseer, peer_c, test_state.relay_parent, None).await; + advertise_collation(&mut virtual_overseer, peer_b, relay_parent, None).await; + advertise_collation(&mut virtual_overseer, peer_c, relay_parent, None).await; let response_channel = assert_fetch_collation_request( &mut virtual_overseer, @@ -790,26 +604,14 @@ fn fetch_one_collation_at_a_time() { /// timeout and in case of an error. #[test] fn fetches_next_collation() { - let test_state = TestState::default(); + let mut test_state = TestState::with_one_scheduled_para(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; + let first = test_state.relay_parent; let second = Hash::random(); - - let our_view = our_view![test_state.relay_parent, second]; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view.clone(), - )), - ) - .await; - - for hash in our_view.iter() { - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, *hash).await; - } + update_view(&mut virtual_overseer, &mut test_state, vec![(first, 0), (second, 1)]).await; let peer_b = PeerId::random(); let peer_c = PeerId::random(); @@ -919,21 +721,13 @@ fn fetches_next_collation() { #[test] fn reject_connection_to_next_group() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; let peer_b = PeerId::random(); @@ -966,26 +760,13 @@ fn reject_connection_to_next_group() { // invalid. #[test] fn fetch_next_collation_on_invalid_collation() { - let test_state = TestState::default(); + let mut test_state = TestState::with_one_scheduled_para(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; - let second = Hash::random(); - - let our_view = our_view![test_state.relay_parent, second]; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view.clone(), - )), - ) - .await; - - for hash in our_view.iter() { - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, *hash).await; - } + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; let peer_b = PeerId::random(); let peer_c = PeerId::random(); @@ -1008,12 +789,12 @@ fn fetch_next_collation_on_invalid_collation() { ) .await; - advertise_collation(&mut virtual_overseer, peer_b, test_state.relay_parent, None).await; - advertise_collation(&mut virtual_overseer, peer_c, test_state.relay_parent, None).await; + advertise_collation(&mut virtual_overseer, peer_b, relay_parent, None).await; + advertise_collation(&mut virtual_overseer, peer_c, relay_parent, None).await; let response_channel = assert_fetch_collation_request( &mut virtual_overseer, - test_state.relay_parent, + relay_parent, test_state.chain_ids[0], None, ) @@ -1023,7 +804,7 @@ fn fetch_next_collation_on_invalid_collation() { let mut candidate_a = dummy_candidate_receipt_bad_sig(dummy_hash(), Some(Default::default())); candidate_a.descriptor.para_id = test_state.chain_ids[0]; - candidate_a.descriptor.relay_parent = test_state.relay_parent; + candidate_a.descriptor.relay_parent = relay_parent; candidate_a.descriptor.persisted_validation_data_hash = dummy_pvd().hash(); response_channel .send(Ok(( @@ -1038,7 +819,7 @@ fn fetch_next_collation_on_invalid_collation() { let receipt = assert_candidate_backing_second( &mut virtual_overseer, - test_state.relay_parent, + relay_parent, test_state.chain_ids[0], &pov, CollationVersion::V1, @@ -1048,7 +829,7 @@ fn fetch_next_collation_on_invalid_collation() { // Inform that the candidate was invalid. overseer_send( &mut virtual_overseer, - CollatorProtocolMessage::Invalid(test_state.relay_parent, receipt), + CollatorProtocolMessage::Invalid(relay_parent, receipt), ) .await; @@ -1065,7 +846,7 @@ fn fetch_next_collation_on_invalid_collation() { // We should see a request for another collation. assert_fetch_collation_request( &mut virtual_overseer, - test_state.relay_parent, + relay_parent, test_state.chain_ids[0], None, ) @@ -1077,25 +858,15 @@ fn fetch_next_collation_on_invalid_collation() { #[test] fn inactive_disconnected() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; let pair = CollatorPair::generate().0; - let hash_a = test_state.relay_parent; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![hash_a], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; let peer_b = PeerId::random(); @@ -1107,11 +878,11 @@ fn inactive_disconnected() { CollationVersion::V1, ) .await; - advertise_collation(&mut virtual_overseer, peer_b, test_state.relay_parent, None).await; + advertise_collation(&mut virtual_overseer, peer_b, relay_parent, None).await; assert_fetch_collation_request( &mut virtual_overseer, - test_state.relay_parent, + relay_parent, test_state.chain_ids[0], None, ) @@ -1126,31 +897,24 @@ fn inactive_disconnected() { #[test] fn activity_extends_life() { - let test_state = TestState::default(); + let mut test_state = TestState::with_one_scheduled_para(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; let pair = CollatorPair::generate().0; - let hash_a = test_state.relay_parent; - let hash_b = Hash::repeat_byte(1); - let hash_c = Hash::repeat_byte(2); + let hash_a = Hash::from_low_u64_be(12); + let hash_b = Hash::from_low_u64_be(11); + let hash_c = Hash::from_low_u64_be(10); - let our_view = our_view![hash_a, hash_b, hash_c]; - - overseer_send( + update_view( &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view.clone(), - )), + &mut test_state, + vec![(hash_a, 0), (hash_b, 1), (hash_c, 2)], ) .await; - for hash in our_view.iter() { - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, *hash).await; - } - let peer_b = PeerId::random(); connect_and_declare_collator( @@ -1208,21 +972,13 @@ fn activity_extends_life() { #[test] fn disconnect_if_no_declare() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; let peer_b = PeerId::random(); @@ -1245,26 +1001,16 @@ fn disconnect_if_no_declare() { #[test] fn disconnect_if_wrong_declare() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; - let pair = CollatorPair::generate().0; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; - let peer_b = PeerId::random(); + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; + overseer_send( &mut virtual_overseer, CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected( @@ -1307,26 +1053,16 @@ fn disconnect_if_wrong_declare() { #[test] fn delay_reputation_change() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| false), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; - let pair = CollatorPair::generate().0; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; - let peer_b = PeerId::random(); + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; + overseer_send( &mut virtual_overseer, CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerConnected( @@ -1400,42 +1136,24 @@ fn view_change_clears_old_collators() { let pair = CollatorPair::generate().0; - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; - - let peer_b = PeerId::random(); + let peer = PeerId::random(); + let relay_parent = test_state.relay_parent; + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent, 0)]).await; connect_and_declare_collator( &mut virtual_overseer, - peer_b, + peer, pair.clone(), test_state.chain_ids[0], CollationVersion::V1, ) .await; - let hash_b = Hash::repeat_byte(69); - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![hash_b], - )), - ) - .await; - test_state.group_rotation_info = test_state.group_rotation_info.bump_rotation(); - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, hash_b).await; - assert_collator_disconnect(&mut virtual_overseer, peer_b).await; + update_view(&mut virtual_overseer, &mut test_state, vec![]).await; + + assert_collator_disconnect(&mut virtual_overseer, peer).await; virtual_overseer }) diff --git a/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs index eda26e8539a1..fac63aeb2097 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs @@ -21,14 +21,11 @@ use super::*; use polkadot_node_subsystem::messages::ChainApiMessage; use polkadot_primitives::{ vstaging::{CommittedCandidateReceiptV2 as CommittedCandidateReceipt, MutateDescriptorV2}, - AsyncBackingParams, BlockNumber, CandidateCommitments, Header, SigningContext, ValidatorId, + BlockNumber, CandidateCommitments, Header, SigningContext, ValidatorId, }; use polkadot_primitives_test_helpers::dummy_committed_candidate_receipt_v2; use rstest::rstest; -const ASYNC_BACKING_PARAMETERS: AsyncBackingParams = - AsyncBackingParams { max_candidate_depth: 4, allowed_ancestry_len: 3 }; - fn get_parent_hash(hash: Hash) -> Hash { Hash::from_low_u64_be(hash.to_low_u64_be() + 1) } @@ -48,7 +45,8 @@ async fn assert_construct_per_relay_parent( msg, AllMessages::RuntimeApi( RuntimeApiMessage::Request(parent, RuntimeApiRequest::Validators(tx)) - ) if parent == hash => { + ) => { + assert_eq!(parent, hash); tx.send(Ok(test_state.validator_public.clone())).unwrap(); } ); @@ -65,15 +63,6 @@ async fn assert_construct_per_relay_parent( } ); - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request(parent, RuntimeApiRequest::AvailabilityCores(tx)) - ) if parent == hash => { - tx.send(Ok(test_state.cores.clone())).unwrap(); - } - ); - assert_matches!( overseer_recv(virtual_overseer).await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( @@ -88,12 +77,11 @@ async fn assert_construct_per_relay_parent( /// Handle a view update. pub(super) async fn update_view( virtual_overseer: &mut VirtualOverseer, - test_state: &TestState, + test_state: &mut TestState, new_view: Vec<(Hash, u32)>, // Hash and block number. - activated: u8, // How many new heads does this update contain? ) -> Option { + let last_block_from_view = new_view.last().map(|t| t.1); let new_view: HashMap = HashMap::from_iter(new_view); - let our_view = OurView::new(new_view.keys().map(|hash| *hash), 0); overseer_send( @@ -103,9 +91,14 @@ pub(super) async fn update_view( .await; let mut next_overseer_message = None; - for _ in 0..activated { + for _ in 0..new_view.len() { + let msg = match next_overseer_message.take() { + Some(msg) => msg, + None => overseer_recv(virtual_overseer).await, + }; + let (leaf_hash, leaf_number) = assert_matches!( - overseer_recv(virtual_overseer).await, + msg, AllMessages::RuntimeApi(RuntimeApiMessage::Request( parent, RuntimeApiRequest::SessionIndexForChild(tx) @@ -121,7 +114,7 @@ pub(super) async fn update_view( _, RuntimeApiRequest::AsyncBackingParams(tx), )) => { - tx.send(Ok(ASYNC_BACKING_PARAMETERS)).unwrap(); + tx.send(Ok(test_state.async_backing_params)).unwrap(); } ); @@ -144,7 +137,8 @@ pub(super) async fn update_view( ) .await; - let min_number = leaf_number.saturating_sub(ASYNC_BACKING_PARAMETERS.allowed_ancestry_len); + let min_number = + leaf_number.saturating_sub(test_state.async_backing_params.allowed_ancestry_len); let ancestry_len = leaf_number + 1 - min_number; let ancestry_hashes = std::iter::successors(Some(leaf_hash), |h| Some(get_parent_hash(*h))) @@ -157,6 +151,10 @@ pub(super) async fn update_view( { let mut ancestry_iter = ancestry_iter.clone(); while let Some((hash, number)) = ancestry_iter.next() { + if Some(number) == test_state.last_known_block { + break; + } + // May be `None` for the last element. let parent_hash = ancestry_iter.peek().map(|(h, _)| *h).unwrap_or_else(|| get_parent_hash(hash)); @@ -204,6 +202,9 @@ pub(super) async fn update_view( // Skip the leaf. for (hash, number) in ancestry_iter.skip(1).take(requested_len.saturating_sub(1)) { + if Some(number) == test_state.last_known_block { + break; + } assert_construct_per_relay_parent( virtual_overseer, test_state, @@ -214,6 +215,9 @@ pub(super) async fn update_view( .await; } } + + test_state.last_known_block = last_block_from_view; + next_overseer_message } @@ -337,9 +341,140 @@ async fn assert_persisted_validation_data( } } +// Combines dummy candidate creation, advertisement and fetching in a single call +async fn submit_second_and_assert( + virtual_overseer: &mut VirtualOverseer, + keystore: KeystorePtr, + para_id: ParaId, + relay_parent: Hash, + collator: PeerId, + candidate_head_data: HeadData, +) { + let (candidate, commitments) = + create_dummy_candidate_and_commitments(para_id, candidate_head_data, relay_parent); + + let candidate_hash = candidate.hash(); + let parent_head_data_hash = Hash::zero(); + + assert_advertise_collation( + virtual_overseer, + collator, + relay_parent, + para_id, + (candidate_hash, parent_head_data_hash), + ) + .await; + + let response_channel = assert_fetch_collation_request( + virtual_overseer, + relay_parent, + para_id, + Some(candidate_hash), + ) + .await; + + let pov = PoV { block_data: BlockData(vec![1]) }; + + send_collation_and_assert_processing( + virtual_overseer, + keystore, + relay_parent, + para_id, + collator, + response_channel, + candidate, + commitments, + pov, + ) + .await; +} + +fn create_dummy_candidate_and_commitments( + para_id: ParaId, + candidate_head_data: HeadData, + relay_parent: Hash, +) -> (CandidateReceipt, CandidateCommitments) { + let mut candidate = dummy_candidate_receipt_bad_sig(relay_parent, Some(Default::default())); + candidate.descriptor.para_id = para_id; + candidate.descriptor.persisted_validation_data_hash = dummy_pvd().hash(); + let commitments = CandidateCommitments { + head_data: candidate_head_data, + horizontal_messages: Default::default(), + upward_messages: Default::default(), + new_validation_code: None, + processed_downward_messages: 0, + hrmp_watermark: 0, + }; + candidate.commitments_hash = commitments.hash(); + + (candidate.into(), commitments) +} + +async fn assert_advertise_collation( + virtual_overseer: &mut VirtualOverseer, + peer: PeerId, + relay_parent: Hash, + expected_para_id: ParaId, + candidate: (CandidateHash, Hash), +) { + advertise_collation(virtual_overseer, peer, relay_parent, Some(candidate)).await; + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::CandidateBacking( + CandidateBackingMessage::CanSecond(request, tx), + ) => { + assert_eq!(request.candidate_hash, candidate.0); + assert_eq!(request.candidate_para_id, expected_para_id); + assert_eq!(request.parent_head_data_hash, candidate.1); + tx.send(true).expect("receiving side should be alive"); + } + ); +} + +async fn send_collation_and_assert_processing( + virtual_overseer: &mut VirtualOverseer, + keystore: KeystorePtr, + relay_parent: Hash, + expected_para_id: ParaId, + expected_peer_id: PeerId, + response_channel: ResponseSender, + candidate: CandidateReceipt, + commitments: CandidateCommitments, + pov: PoV, +) { + response_channel + .send(Ok(( + request_v2::CollationFetchingResponse::Collation(candidate.clone(), pov.clone()) + .encode(), + ProtocolName::from(""), + ))) + .expect("Sending response should succeed"); + + assert_candidate_backing_second( + virtual_overseer, + relay_parent, + expected_para_id, + &pov, + CollationVersion::V2, + ) + .await; + + let candidate = CommittedCandidateReceipt { descriptor: candidate.descriptor, commitments }; + + send_seconded_statement(virtual_overseer, keystore.clone(), &candidate).await; + + assert_collation_seconded( + virtual_overseer, + relay_parent, + expected_peer_id, + CollationVersion::V2, + ) + .await; +} + #[test] fn v1_advertisement_accepted_and_seconded() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, keystore } = test_harness; @@ -349,7 +484,7 @@ fn v1_advertisement_accepted_and_seconded() { let head_b = Hash::from_low_u64_be(128); let head_b_num: u32 = 0; - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); @@ -377,7 +512,7 @@ fn v1_advertisement_accepted_and_seconded() { candidate.descriptor.para_id = test_state.chain_ids[0]; candidate.descriptor.persisted_validation_data_hash = dummy_pvd().hash(); let commitments = CandidateCommitments { - head_data: HeadData(vec![1 as u8]), + head_data: HeadData(vec![1u8]), horizontal_messages: Default::default(), upward_messages: Default::default(), new_validation_code: None, @@ -418,7 +553,7 @@ fn v1_advertisement_accepted_and_seconded() { #[test] fn v1_advertisement_rejected_on_non_active_leaf() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; @@ -428,7 +563,7 @@ fn v1_advertisement_rejected_on_non_active_leaf() { let head_b = Hash::from_low_u64_be(128); let head_b_num: u32 = 5; - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); @@ -460,7 +595,7 @@ fn v1_advertisement_rejected_on_non_active_leaf() { #[test] fn accept_advertisements_from_implicit_view() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; @@ -478,7 +613,7 @@ fn accept_advertisements_from_implicit_view() { let head_d = get_parent_hash(head_c); // Activated leaf is `b`, but the collation will be based on `c`. - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); let peer_b = PeerId::random(); @@ -563,24 +698,26 @@ fn accept_advertisements_from_implicit_view() { #[test] fn second_multiple_candidates_per_relay_parent() { - let test_state = TestState::default(); + let mut test_state = TestState::with_one_scheduled_para(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, keystore } = test_harness; let pair = CollatorPair::generate().0; - // Grandparent of head `a`. + let head_a = Hash::from_low_u64_be(130); + let head_a_num: u32 = 0; + let head_b = Hash::from_low_u64_be(128); let head_b_num: u32 = 2; - // Grandparent of head `b`. - // Group rotation frequency is 1 by default, at `c` we're assigned - // to the first para. - let head_c = Hash::from_low_u64_be(130); - - // Activated leaf is `b`, but the collation will be based on `c`. - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + // Activated leaf is `a` and `b`.The collation will be based on `b`. + update_view( + &mut virtual_overseer, + &mut test_state, + vec![(head_a, head_a_num), (head_b, head_b_num)], + ) + .await; let peer_a = PeerId::random(); @@ -593,80 +730,17 @@ fn second_multiple_candidates_per_relay_parent() { ) .await; - for i in 0..(ASYNC_BACKING_PARAMETERS.max_candidate_depth + 1) { - let mut candidate = dummy_candidate_receipt_bad_sig(head_c, Some(Default::default())); - candidate.descriptor.para_id = test_state.chain_ids[0]; - candidate.descriptor.persisted_validation_data_hash = dummy_pvd().hash(); - let commitments = CandidateCommitments { - head_data: HeadData(vec![i as u8]), - horizontal_messages: Default::default(), - upward_messages: Default::default(), - new_validation_code: None, - processed_downward_messages: 0, - hrmp_watermark: 0, - }; - candidate.commitments_hash = commitments.hash(); - let candidate: CandidateReceipt = candidate.into(); - - let candidate_hash = candidate.hash(); - let parent_head_data_hash = Hash::zero(); - - advertise_collation( - &mut virtual_overseer, - peer_a, - head_c, - Some((candidate_hash, parent_head_data_hash)), - ) - .await; - assert_matches!( - overseer_recv(&mut virtual_overseer).await, - AllMessages::CandidateBacking( - CandidateBackingMessage::CanSecond(request, tx), - ) => { - assert_eq!(request.candidate_hash, candidate_hash); - assert_eq!(request.candidate_para_id, test_state.chain_ids[0]); - assert_eq!(request.parent_head_data_hash, parent_head_data_hash); - tx.send(true).expect("receiving side should be alive"); - } - ); - - let response_channel = assert_fetch_collation_request( - &mut virtual_overseer, - head_c, - test_state.chain_ids[0], - Some(candidate_hash), - ) - .await; - - let pov = PoV { block_data: BlockData(vec![1]) }; - - response_channel - .send(Ok(( - request_v2::CollationFetchingResponse::Collation( - candidate.clone(), - pov.clone(), - ) - .encode(), - ProtocolName::from(""), - ))) - .expect("Sending response should succeed"); - - assert_candidate_backing_second( + // `allowed_ancestry_len` equals the size of the claim queue + for i in 0..test_state.async_backing_params.allowed_ancestry_len { + submit_second_and_assert( &mut virtual_overseer, - head_c, + keystore.clone(), test_state.chain_ids[0], - &pov, - CollationVersion::V2, + head_a, + peer_a, + HeadData(vec![i as u8]), ) .await; - - let candidate = - CommittedCandidateReceipt { descriptor: candidate.descriptor, commitments }; - - send_seconded_statement(&mut virtual_overseer, keystore.clone(), &candidate).await; - - assert_collation_seconded(&mut virtual_overseer, head_c, peer_a, CollationVersion::V2) - .await; } // No more advertisements can be made for this relay parent. @@ -674,21 +748,14 @@ fn second_multiple_candidates_per_relay_parent() { advertise_collation( &mut virtual_overseer, peer_a, - head_c, + head_a, Some((candidate_hash, Hash::zero())), ) .await; - // Reported because reached the limit of advertisements per relay parent. - assert_matches!( - overseer_recv(&mut virtual_overseer).await, - AllMessages::NetworkBridgeTx( - NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(peer_id, rep)), - ) => { - assert_eq!(peer_a, peer_id); - assert_eq!(rep.value, COST_UNEXPECTED_MESSAGE.cost_or_benefit()); - } - ); + // Rejected but not reported because reached the limit of advertisements for the para_id + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); // By different peer too (not reported). let pair_b = CollatorPair::generate().0; @@ -707,7 +774,7 @@ fn second_multiple_candidates_per_relay_parent() { advertise_collation( &mut virtual_overseer, peer_b, - head_c, + head_a, Some((candidate_hash, Hash::zero())), ) .await; @@ -721,7 +788,7 @@ fn second_multiple_candidates_per_relay_parent() { #[test] fn fetched_collation_sanity_check() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; @@ -738,7 +805,7 @@ fn fetched_collation_sanity_check() { let head_c = Hash::from_low_u64_be(130); // Activated leaf is `b`, but the collation will be based on `c`. - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); @@ -832,7 +899,7 @@ fn fetched_collation_sanity_check() { #[test] fn sanity_check_invalid_parent_head_data() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; @@ -842,7 +909,7 @@ fn sanity_check_invalid_parent_head_data() { let head_c = Hash::from_low_u64_be(130); let head_c_num = 3; - update_view(&mut virtual_overseer, &test_state, vec![(head_c, head_c_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_c, head_c_num)]).await; let peer_a = PeerId::random(); @@ -952,7 +1019,7 @@ fn sanity_check_invalid_parent_head_data() { #[test] fn advertisement_spam_protection() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; @@ -965,7 +1032,7 @@ fn advertisement_spam_protection() { let head_c = get_parent_hash(head_b); // Activated leaf is `b`, but the collation will be based on `c`. - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); connect_and_declare_collator( @@ -1026,7 +1093,7 @@ fn advertisement_spam_protection() { #[case(true)] #[case(false)] fn child_blocked_from_seconding_by_parent(#[case] valid_parent: bool) { - let test_state = TestState::default(); + let mut test_state = TestState::with_one_scheduled_para(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, keystore } = test_harness; @@ -1043,7 +1110,7 @@ fn child_blocked_from_seconding_by_parent(#[case] valid_parent: bool) { let head_c = Hash::from_low_u64_be(130); // Activated leaf is `b`, but the collation will be based on `c`. - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); @@ -1344,7 +1411,7 @@ fn v2_descriptor(#[case] v2_feature_enabled: bool) { let head_b = Hash::from_low_u64_be(128); let head_b_num: u32 = 0; - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); @@ -1442,7 +1509,7 @@ fn v2_descriptor(#[case] v2_feature_enabled: bool) { #[test] fn invalid_v2_descriptor() { - let test_state = TestState::default(); + let mut test_state = TestState::default(); test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { let TestHarness { mut virtual_overseer, .. } = test_harness; @@ -1452,7 +1519,7 @@ fn invalid_v2_descriptor() { let head_b = Hash::from_low_u64_be(128); let head_b_num: u32 = 0; - update_view(&mut virtual_overseer, &test_state, vec![(head_b, head_b_num)], 1).await; + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; let peer_a = PeerId::random(); @@ -1545,3 +1612,868 @@ fn invalid_v2_descriptor() { virtual_overseer }); } + +#[test] +fn fair_collation_fetches() { + let mut test_state = TestState::with_shared_core(); + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let head_b = Hash::from_low_u64_be(128); + let head_b_num: u32 = 2; + + update_view(&mut virtual_overseer, &mut test_state, vec![(head_b, head_b_num)]).await; + + let peer_a = PeerId::random(); + let pair_a = CollatorPair::generate().0; + + connect_and_declare_collator( + &mut virtual_overseer, + peer_a, + pair_a.clone(), + test_state.chain_ids[0], + CollationVersion::V2, + ) + .await; + + let peer_b = PeerId::random(); + let pair_b = CollatorPair::generate().0; + + connect_and_declare_collator( + &mut virtual_overseer, + peer_b, + pair_b.clone(), + test_state.chain_ids[1], + CollationVersion::V2, + ) + .await; + + // `peer_a` sends two advertisements (its claim queue limit) + for i in 0..2u8 { + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + head_b, + peer_a, + HeadData(vec![i]), + ) + .await; + } + + // `peer_a` sends another advertisement and it is ignored + let candidate_hash = CandidateHash(Hash::repeat_byte(0xAA)); + advertise_collation( + &mut virtual_overseer, + peer_a, + head_b, + Some((candidate_hash, Hash::zero())), + ) + .await; + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // `peer_b` should still be able to advertise its collation + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[1]), + head_b, + peer_b, + HeadData(vec![0u8]), + ) + .await; + + // And no more advertisements can be made for this relay parent. + + // verify for peer_a + let candidate_hash = CandidateHash(Hash::repeat_byte(0xBB)); + advertise_collation( + &mut virtual_overseer, + peer_a, + head_b, + Some((candidate_hash, Hash::zero())), + ) + .await; + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // verify for peer_b + let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + advertise_collation( + &mut virtual_overseer, + peer_b, + head_b, + Some((candidate_hash, Hash::zero())), + ) + .await; + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + virtual_overseer + }); +} + +#[test] +fn collation_fetching_prefer_entries_earlier_in_claim_queue() { + let mut test_state = TestState::with_shared_core(); + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let pair_a = CollatorPair::generate().0; + let collator_a = PeerId::random(); + let para_id_a = test_state.chain_ids[0]; + + let pair_b = CollatorPair::generate().0; + let collator_b = PeerId::random(); + let para_id_b = test_state.chain_ids[1]; + + let head = Hash::from_low_u64_be(128); + let head_num: u32 = 2; + + update_view(&mut virtual_overseer, &mut test_state, vec![(head, head_num)]).await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_a, + pair_a.clone(), + para_id_a, + CollationVersion::V2, + ) + .await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_b, + pair_b.clone(), + para_id_b, + CollationVersion::V2, + ) + .await; + + let (candidate_a1, commitments_a1) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![0u8]), head); + let (candidate_b1, commitments_b1) = + create_dummy_candidate_and_commitments(para_id_b, HeadData(vec![1u8]), head); + let (candidate_a2, commitments_a2) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![2u8]), head); + let (candidate_a3, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![3u8]), head); + let parent_head_data_a1 = HeadData(vec![0u8]); + let parent_head_data_b1 = HeadData(vec![1u8]); + let parent_head_data_a2 = HeadData(vec![2u8]); + let parent_head_data_a3 = HeadData(vec![3u8]); + + // advertise a collation for `para_id_a` but don't send the collation. This will be a + // pending fetch. + assert_advertise_collation( + &mut virtual_overseer, + collator_a, + head, + para_id_a, + (candidate_a1.hash(), parent_head_data_a1.hash()), + ) + .await; + + let response_channel_a1 = assert_fetch_collation_request( + &mut virtual_overseer, + head, + para_id_a, + Some(candidate_a1.hash()), + ) + .await; + + // advertise another collation for `para_id_a`. This one should be fetched last. + assert_advertise_collation( + &mut virtual_overseer, + collator_a, + head, + para_id_a, + (candidate_a2.hash(), parent_head_data_a2.hash()), + ) + .await; + + // There is a pending collation so nothing should be fetched + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // Advertise a collation for `para_id_b`. This should be fetched second + assert_advertise_collation( + &mut virtual_overseer, + collator_b, + head, + para_id_b, + (candidate_b1.hash(), parent_head_data_b1.hash()), + ) + .await; + + // Again - no fetch because of the pending collation + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + //Now send a response for the first fetch and examine the second fetch + send_collation_and_assert_processing( + &mut virtual_overseer, + keystore.clone(), + head, + para_id_a, + collator_a, + response_channel_a1, + candidate_a1, + commitments_a1, + PoV { block_data: BlockData(vec![1]) }, + ) + .await; + + // The next fetch should be for `para_id_b` + let response_channel_b = assert_fetch_collation_request( + &mut virtual_overseer, + head, + para_id_b, + Some(candidate_b1.hash()), + ) + .await; + + send_collation_and_assert_processing( + &mut virtual_overseer, + keystore.clone(), + head, + para_id_b, + collator_b, + response_channel_b, + candidate_b1, + commitments_b1, + PoV { block_data: BlockData(vec![2]) }, + ) + .await; + + // and the final one for `para_id_a` + let response_channel_a2 = assert_fetch_collation_request( + &mut virtual_overseer, + head, + para_id_a, + Some(candidate_a2.hash()), + ) + .await; + + // Advertise another collation for `para_id_a`. This should be rejected as there is no slot + // in the claim queue for it. One is fetched and one is pending. + advertise_collation( + &mut virtual_overseer, + collator_a, + head, + Some((candidate_a3.hash(), parent_head_data_a3.hash())), + ) + .await; + + // `CanSecond` shouldn't be sent as the advertisement should be ignored + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // Fetch the pending collation + send_collation_and_assert_processing( + &mut virtual_overseer, + keystore.clone(), + head, + para_id_a, + collator_a, + response_channel_a2, + candidate_a2, + commitments_a2, + PoV { block_data: BlockData(vec![3]) }, + ) + .await; + + virtual_overseer + }); +} + +#[test] +fn collation_fetching_considers_advertisements_from_the_whole_view() { + let mut test_state = TestState::with_shared_core(); + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let pair_a = CollatorPair::generate().0; + let collator_a = PeerId::random(); + let para_id_a = test_state.chain_ids[0]; + + let pair_b = CollatorPair::generate().0; + let collator_b = PeerId::random(); + let para_id_b = test_state.chain_ids[1]; + + let relay_parent_2 = Hash::from_low_u64_be(test_state.relay_parent.to_low_u64_be() - 1); + + assert_eq!( + *test_state.claim_queue.get(&CoreIndex(0)).unwrap(), + VecDeque::from([para_id_b, para_id_a, para_id_a]) + ); + + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent_2, 2)]).await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_a, + pair_a.clone(), + para_id_a, + CollationVersion::V2, + ) + .await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_b, + pair_b.clone(), + para_id_b, + CollationVersion::V2, + ) + .await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_a, + relay_parent_2, + collator_a, + HeadData(vec![0u8]), + ) + .await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_b, + relay_parent_2, + collator_b, + HeadData(vec![1u8]), + ) + .await; + + let relay_parent_3 = Hash::from_low_u64_be(relay_parent_2.to_low_u64_be() - 1); + *test_state.claim_queue.get_mut(&CoreIndex(0)).unwrap() = + VecDeque::from([para_id_a, para_id_a, para_id_b]); + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent_3, 3)]).await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_b, + relay_parent_3, + collator_b, + HeadData(vec![3u8]), + ) + .await; + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_a, + relay_parent_3, + collator_a, + HeadData(vec![3u8]), + ) + .await; + + // At this point the claim queue is satisfied and any advertisement at `relay_parent_4` + // must be ignored + + let (candidate_a, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![5u8]), relay_parent_3); + let parent_head_data_a = HeadData(vec![5u8]); + + advertise_collation( + &mut virtual_overseer, + collator_a, + relay_parent_3, + Some((candidate_a.hash(), parent_head_data_a.hash())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + let (candidate_b, _) = + create_dummy_candidate_and_commitments(para_id_b, HeadData(vec![6u8]), relay_parent_3); + let parent_head_data_b = HeadData(vec![6u8]); + + advertise_collation( + &mut virtual_overseer, + collator_b, + relay_parent_3, + Some((candidate_b.hash(), parent_head_data_b.hash())), + ) + .await; + + // `CanSecond` shouldn't be sent as the advertisement should be ignored + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // At `relay_parent_6` the advertisement for `para_id_b` falls out of the view so a new one + // can be accepted + let relay_parent_6 = Hash::from_low_u64_be(relay_parent_3.to_low_u64_be() - 2); + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent_6, 6)]).await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_a, + relay_parent_6, + collator_a, + HeadData(vec![3u8]), + ) + .await; + + virtual_overseer + }); +} + +#[test] +fn collation_fetching_fairness_handles_old_claims() { + let mut test_state = TestState::with_shared_core(); + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let pair_a = CollatorPair::generate().0; + let collator_a = PeerId::random(); + let para_id_a = test_state.chain_ids[0]; + + let pair_b = CollatorPair::generate().0; + let collator_b = PeerId::random(); + let para_id_b = test_state.chain_ids[1]; + + let relay_parent_2 = Hash::from_low_u64_be(test_state.relay_parent.to_low_u64_be() - 1); + + *test_state.claim_queue.get_mut(&CoreIndex(0)).unwrap() = + VecDeque::from([para_id_a, para_id_b, para_id_a]); + + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent_2, 2)]).await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_a, + pair_a.clone(), + para_id_a, + CollationVersion::V2, + ) + .await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_b, + pair_b.clone(), + para_id_b, + CollationVersion::V2, + ) + .await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_a, + relay_parent_2, + collator_a, + HeadData(vec![0u8]), + ) + .await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_b, + relay_parent_2, + collator_b, + HeadData(vec![1u8]), + ) + .await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_a, + relay_parent_2, + collator_a, + HeadData(vec![2u8]), + ) + .await; + + let relay_parent_3 = Hash::from_low_u64_be(relay_parent_2.to_low_u64_be() - 1); + + *test_state.claim_queue.get_mut(&CoreIndex(0)).unwrap() = + VecDeque::from([para_id_b, para_id_a, para_id_b]); + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent_3, 3)]).await; + + // nothing is advertised here + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + let relay_parent_4 = Hash::from_low_u64_be(relay_parent_3.to_low_u64_be() - 1); + + *test_state.claim_queue.get_mut(&CoreIndex(0)).unwrap() = + VecDeque::from([para_id_a, para_id_b, para_id_a]); + update_view(&mut virtual_overseer, &mut test_state, vec![(relay_parent_4, 4)]).await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_b, + relay_parent_4, + collator_b, + HeadData(vec![3u8]), + ) + .await; + + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + para_id_a, + relay_parent_4, + collator_a, + HeadData(vec![4u8]), + ) + .await; + + // At this point the claim queue is satisfied and any advertisement at `relay_parent_4` + // must be ignored + + // Advertisement for `para_id_a` at `relay_parent_4` which must be ignored + let (candidate_a, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![5u8]), relay_parent_4); + let parent_head_data_a = HeadData(vec![5u8]); + + advertise_collation( + &mut virtual_overseer, + collator_a, + relay_parent_4, + Some((candidate_a.hash(), parent_head_data_a.hash())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // Advertisement for `para_id_b` at `relay_parent_4` which must be ignored + let (candidate_b, _) = + create_dummy_candidate_and_commitments(para_id_b, HeadData(vec![6u8]), relay_parent_4); + let parent_head_data_b = HeadData(vec![6u8]); + + advertise_collation( + &mut virtual_overseer, + collator_b, + relay_parent_4, + Some((candidate_b.hash(), parent_head_data_b.hash())), + ) + .await; + + // `CanSecond` shouldn't be sent as the advertisement should be ignored + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + virtual_overseer + }); +} + +#[test] +fn claims_below_are_counted_correctly() { + let mut test_state = TestState::with_one_scheduled_para(); + + // Shorten the claim queue to make the test smaller + let mut claim_queue = BTreeMap::new(); + claim_queue.insert( + CoreIndex(0), + VecDeque::from_iter( + [ParaId::from(test_state.chain_ids[0]), ParaId::from(test_state.chain_ids[0])] + .into_iter(), + ), + ); + test_state.claim_queue = claim_queue; + test_state.async_backing_params.max_candidate_depth = 3; + test_state.async_backing_params.allowed_ancestry_len = 2; + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let hash_a = Hash::from_low_u64_be(test_state.relay_parent.to_low_u64_be() - 1); + let hash_b = Hash::from_low_u64_be(hash_a.to_low_u64_be() - 1); + let hash_c = Hash::from_low_u64_be(hash_b.to_low_u64_be() - 1); + + let pair_a = CollatorPair::generate().0; + let collator_a = PeerId::random(); + let para_id_a = test_state.chain_ids[0]; + + update_view(&mut virtual_overseer, &mut test_state, vec![(hash_c, 2)]).await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_a, + pair_a.clone(), + para_id_a, + CollationVersion::V2, + ) + .await; + + // A collation at hash_a claims the spot at hash_a + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_a, + collator_a, + HeadData(vec![0u8]), + ) + .await; + + // Another collation at hash_a claims the spot at hash_b + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_a, + collator_a, + HeadData(vec![1u8]), + ) + .await; + + // Collation at hash_c claims its own spot + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_c, + collator_a, + HeadData(vec![2u8]), + ) + .await; + + // Collation at hash_b should be ignored because the claim queue is satisfied + let (ignored_candidate, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![3u8]), hash_b); + + advertise_collation( + &mut virtual_overseer, + collator_a, + hash_b, + Some((ignored_candidate.hash(), Hash::random())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + virtual_overseer + }); +} + +#[test] +fn claims_above_are_counted_correctly() { + let mut test_state = TestState::with_one_scheduled_para(); + + // Shorten the claim queue to make the test smaller + let mut claim_queue = BTreeMap::new(); + claim_queue.insert( + CoreIndex(0), + VecDeque::from_iter( + [ParaId::from(test_state.chain_ids[0]), ParaId::from(test_state.chain_ids[0])] + .into_iter(), + ), + ); + test_state.claim_queue = claim_queue; + test_state.async_backing_params.max_candidate_depth = 3; + test_state.async_backing_params.allowed_ancestry_len = 2; + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let hash_a = Hash::from_low_u64_be(test_state.relay_parent.to_low_u64_be() - 1); // block 0 + let hash_b = Hash::from_low_u64_be(hash_a.to_low_u64_be() - 1); // block 1 + let hash_c = Hash::from_low_u64_be(hash_b.to_low_u64_be() - 1); // block 2 + + let pair_a = CollatorPair::generate().0; + let collator_a = PeerId::random(); + let para_id_a = test_state.chain_ids[0]; + + update_view(&mut virtual_overseer, &mut test_state, vec![(hash_c, 2)]).await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_a, + pair_a.clone(), + para_id_a, + CollationVersion::V2, + ) + .await; + + // A collation at hash_b claims the spot at hash_b + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_b, + collator_a, + HeadData(vec![0u8]), + ) + .await; + + // Another collation at hash_b claims the spot at hash_c + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_b, + collator_a, + HeadData(vec![1u8]), + ) + .await; + + // Collation at hash_a claims its own spot + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_a, + collator_a, + HeadData(vec![0u8]), + ) + .await; + + // Another Collation at hash_a should be ignored because the claim queue is satisfied + let (ignored_candidate, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![2u8]), hash_a); + + advertise_collation( + &mut virtual_overseer, + collator_a, + hash_a, + Some((ignored_candidate.hash(), Hash::random())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // Same for hash_b + let (ignored_candidate, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![3u8]), hash_b); + + advertise_collation( + &mut virtual_overseer, + collator_a, + hash_b, + Some((ignored_candidate.hash(), Hash::random())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + virtual_overseer + }); +} + +#[test] +fn claim_fills_last_free_slot() { + let mut test_state = TestState::with_one_scheduled_para(); + + // Shorten the claim queue to make the test smaller + let mut claim_queue = BTreeMap::new(); + claim_queue.insert( + CoreIndex(0), + VecDeque::from_iter( + [ParaId::from(test_state.chain_ids[0]), ParaId::from(test_state.chain_ids[0])] + .into_iter(), + ), + ); + test_state.claim_queue = claim_queue; + test_state.async_backing_params.max_candidate_depth = 3; + test_state.async_backing_params.allowed_ancestry_len = 2; + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, keystore } = test_harness; + + let hash_a = Hash::from_low_u64_be(test_state.relay_parent.to_low_u64_be() - 1); // block 0 + let hash_b = Hash::from_low_u64_be(hash_a.to_low_u64_be() - 1); // block 1 + let hash_c = Hash::from_low_u64_be(hash_b.to_low_u64_be() - 1); // block 2 + + let pair_a = CollatorPair::generate().0; + let collator_a = PeerId::random(); + let para_id_a = test_state.chain_ids[0]; + + update_view(&mut virtual_overseer, &mut test_state, vec![(hash_c, 2)]).await; + + connect_and_declare_collator( + &mut virtual_overseer, + collator_a, + pair_a.clone(), + para_id_a, + CollationVersion::V2, + ) + .await; + + // A collation at hash_a claims its spot + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_a, + collator_a, + HeadData(vec![0u8]), + ) + .await; + + // Collation at hash_b claims its own spot + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_b, + collator_a, + HeadData(vec![3u8]), + ) + .await; + + // Collation at hash_c claims its own spot + submit_second_and_assert( + &mut virtual_overseer, + keystore.clone(), + ParaId::from(test_state.chain_ids[0]), + hash_c, + collator_a, + HeadData(vec![2u8]), + ) + .await; + + // Another Collation at hash_a should be ignored because the claim queue is satisfied + let (ignored_candidate, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![3u8]), hash_a); + + advertise_collation( + &mut virtual_overseer, + collator_a, + hash_a, + Some((ignored_candidate.hash(), Hash::random())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + // Same for hash_b + let (ignored_candidate, _) = + create_dummy_candidate_and_commitments(para_id_a, HeadData(vec![4u8]), hash_b); + + advertise_collation( + &mut virtual_overseer, + collator_a, + hash_b, + Some((ignored_candidate.hash(), Hash::random())), + ) + .await; + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + virtual_overseer + }); +} diff --git a/polkadot/node/subsystem-util/src/backing_implicit_view.rs b/polkadot/node/subsystem-util/src/backing_implicit_view.rs index 6f2191e7add2..67f5dad518e1 100644 --- a/polkadot/node/subsystem-util/src/backing_implicit_view.rs +++ b/polkadot/node/subsystem-util/src/backing_implicit_view.rs @@ -22,12 +22,13 @@ use polkadot_node_subsystem::{ }; use polkadot_primitives::{AsyncBackingParams, BlockNumber, Hash, Id as ParaId}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use crate::{ inclusion_emulator::RelayChainBlockInfo, request_async_backing_params, request_session_index_for_child, runtime::{self, recv_runtime}, + LOG_TARGET, }; // Always aim to retain 1 block before the active leaves. @@ -173,13 +174,7 @@ impl View { return Err(FetchError::AlreadyKnown) } - let res = fetch_fresh_leaf_and_insert_ancestry( - leaf_hash, - &mut self.block_info_storage, - &mut *sender, - self.collating_for, - ) - .await; + let res = self.fetch_fresh_leaf_and_insert_ancestry(leaf_hash, &mut *sender).await; match res { Ok(fetched) => { @@ -323,6 +318,205 @@ impl View { .as_ref() .map(|mins| mins.allowed_relay_parents_for(para_id, block_info.block_number)) } + + /// Returns all paths from each leaf to the last block in state containing `relay_parent`. If no + /// paths exist the function will return an empty `Vec`. + pub fn paths_via_relay_parent(&self, relay_parent: &Hash) -> Vec> { + gum::trace!( + target: LOG_TARGET, + ?relay_parent, + leaves=?self.leaves, + block_info_storage=?self.block_info_storage, + "Finding paths via relay parent" + ); + + if self.leaves.is_empty() { + // No leaves so the view should be empty. Don't return any paths. + return vec![] + }; + + if !self.block_info_storage.contains_key(relay_parent) { + // `relay_parent` is not in the view - don't return any paths + return vec![] + } + + // Find all paths from each leaf to `relay_parent`. + let mut paths = Vec::new(); + for (leaf, _) in &self.leaves { + let mut path = Vec::new(); + let mut current_leaf = *leaf; + let mut visited = HashSet::new(); + let mut path_contains_target = false; + + // Start from the leaf and traverse all known blocks + loop { + if visited.contains(¤t_leaf) { + // There is a cycle - abandon this path + break + } + + current_leaf = match self.block_info_storage.get(¤t_leaf) { + Some(info) => { + // `current_leaf` is a known block - add it to the path and mark it as + // visited + path.push(current_leaf); + visited.insert(current_leaf); + + // `current_leaf` is the target `relay_parent`. Mark the path so that it's + // included in the result + if current_leaf == *relay_parent { + path_contains_target = true; + } + + // update `current_leaf` with the parent + info.parent_hash + }, + None => { + // path is complete + if path_contains_target { + // we want the path ordered from oldest to newest so reverse it + paths.push(path.into_iter().rev().collect()); + } + break + }, + }; + } + } + + paths + } + + async fn fetch_fresh_leaf_and_insert_ancestry( + &mut self, + leaf_hash: Hash, + sender: &mut Sender, + ) -> Result + where + Sender: SubsystemSender + + SubsystemSender + + SubsystemSender, + { + let leaf_header = { + let (tx, rx) = oneshot::channel(); + sender.send_message(ChainApiMessage::BlockHeader(leaf_hash, tx)).await; + + match rx.await { + Ok(Ok(Some(header))) => header, + Ok(Ok(None)) => + return Err(FetchError::BlockHeaderUnavailable( + leaf_hash, + BlockHeaderUnavailableReason::Unknown, + )), + Ok(Err(e)) => + return Err(FetchError::BlockHeaderUnavailable( + leaf_hash, + BlockHeaderUnavailableReason::Internal(e), + )), + Err(_) => + return Err(FetchError::BlockHeaderUnavailable( + leaf_hash, + BlockHeaderUnavailableReason::SubsystemUnavailable, + )), + } + }; + + // If the node is a collator, bypass prospective-parachains. We're only interested in the + // one paraid and the subsystem is not present. + let min_relay_parents = if let Some(para_id) = self.collating_for { + fetch_min_relay_parents_for_collator(leaf_hash, leaf_header.number, sender) + .await? + .map(|x| vec![(para_id, x)]) + .unwrap_or_default() + } else { + fetch_min_relay_parents_from_prospective_parachains(leaf_hash, sender).await? + }; + + let min_min = min_relay_parents.iter().map(|x| x.1).min().unwrap_or(leaf_header.number); + let expected_ancestry_len = (leaf_header.number.saturating_sub(min_min) as usize) + 1; + + let ancestry = if leaf_header.number > 0 { + let mut next_ancestor_number = leaf_header.number - 1; + let mut next_ancestor_hash = leaf_header.parent_hash; + + let mut ancestry = Vec::with_capacity(expected_ancestry_len); + ancestry.push(leaf_hash); + + // Ensure all ancestors up to and including `min_min` are in the + // block storage. When views advance incrementally, everything + // should already be present. + while next_ancestor_number >= min_min { + let parent_hash = if let Some(info) = + self.block_info_storage.get(&next_ancestor_hash) + { + info.parent_hash + } else { + // load the header and insert into block storage. + let (tx, rx) = oneshot::channel(); + sender.send_message(ChainApiMessage::BlockHeader(next_ancestor_hash, tx)).await; + + let header = match rx.await { + Ok(Ok(Some(header))) => header, + Ok(Ok(None)) => + return Err(FetchError::BlockHeaderUnavailable( + next_ancestor_hash, + BlockHeaderUnavailableReason::Unknown, + )), + Ok(Err(e)) => + return Err(FetchError::BlockHeaderUnavailable( + next_ancestor_hash, + BlockHeaderUnavailableReason::Internal(e), + )), + Err(_) => + return Err(FetchError::BlockHeaderUnavailable( + next_ancestor_hash, + BlockHeaderUnavailableReason::SubsystemUnavailable, + )), + }; + + self.block_info_storage.insert( + next_ancestor_hash, + BlockInfo { + block_number: next_ancestor_number, + parent_hash: header.parent_hash, + maybe_allowed_relay_parents: None, + }, + ); + + header.parent_hash + }; + + ancestry.push(next_ancestor_hash); + if next_ancestor_number == 0 { + break + } + + next_ancestor_number -= 1; + next_ancestor_hash = parent_hash; + } + + ancestry + } else { + vec![leaf_hash] + }; + + let fetched_ancestry = + FetchSummary { minimum_ancestor_number: min_min, leaf_number: leaf_header.number }; + + let allowed_relay_parents = AllowedRelayParents { + minimum_relay_parents: min_relay_parents.into_iter().collect(), + allowed_relay_parents_contiguous: ancestry, + }; + + let leaf_block_info = BlockInfo { + parent_hash: leaf_header.parent_hash, + block_number: leaf_header.number, + maybe_allowed_relay_parents: Some(allowed_relay_parents), + }; + + self.block_info_storage.insert(leaf_hash, leaf_block_info); + + Ok(fetched_ancestry) + } } /// Errors when fetching a leaf and associated ancestry. @@ -437,137 +631,6 @@ where Ok(Some(min)) } -async fn fetch_fresh_leaf_and_insert_ancestry( - leaf_hash: Hash, - block_info_storage: &mut HashMap, - sender: &mut Sender, - collating_for: Option, -) -> Result -where - Sender: SubsystemSender - + SubsystemSender - + SubsystemSender, -{ - let leaf_header = { - let (tx, rx) = oneshot::channel(); - sender.send_message(ChainApiMessage::BlockHeader(leaf_hash, tx)).await; - - match rx.await { - Ok(Ok(Some(header))) => header, - Ok(Ok(None)) => - return Err(FetchError::BlockHeaderUnavailable( - leaf_hash, - BlockHeaderUnavailableReason::Unknown, - )), - Ok(Err(e)) => - return Err(FetchError::BlockHeaderUnavailable( - leaf_hash, - BlockHeaderUnavailableReason::Internal(e), - )), - Err(_) => - return Err(FetchError::BlockHeaderUnavailable( - leaf_hash, - BlockHeaderUnavailableReason::SubsystemUnavailable, - )), - } - }; - - // If the node is a collator, bypass prospective-parachains. We're only interested in the one - // paraid and the subsystem is not present. - let min_relay_parents = if let Some(para_id) = collating_for { - fetch_min_relay_parents_for_collator(leaf_hash, leaf_header.number, sender) - .await? - .map(|x| vec![(para_id, x)]) - .unwrap_or_default() - } else { - fetch_min_relay_parents_from_prospective_parachains(leaf_hash, sender).await? - }; - - let min_min = min_relay_parents.iter().map(|x| x.1).min().unwrap_or(leaf_header.number); - let expected_ancestry_len = (leaf_header.number.saturating_sub(min_min) as usize) + 1; - - let ancestry = if leaf_header.number > 0 { - let mut next_ancestor_number = leaf_header.number - 1; - let mut next_ancestor_hash = leaf_header.parent_hash; - - let mut ancestry = Vec::with_capacity(expected_ancestry_len); - ancestry.push(leaf_hash); - - // Ensure all ancestors up to and including `min_min` are in the - // block storage. When views advance incrementally, everything - // should already be present. - while next_ancestor_number >= min_min { - let parent_hash = if let Some(info) = block_info_storage.get(&next_ancestor_hash) { - info.parent_hash - } else { - // load the header and insert into block storage. - let (tx, rx) = oneshot::channel(); - sender.send_message(ChainApiMessage::BlockHeader(next_ancestor_hash, tx)).await; - - let header = match rx.await { - Ok(Ok(Some(header))) => header, - Ok(Ok(None)) => - return Err(FetchError::BlockHeaderUnavailable( - next_ancestor_hash, - BlockHeaderUnavailableReason::Unknown, - )), - Ok(Err(e)) => - return Err(FetchError::BlockHeaderUnavailable( - next_ancestor_hash, - BlockHeaderUnavailableReason::Internal(e), - )), - Err(_) => - return Err(FetchError::BlockHeaderUnavailable( - next_ancestor_hash, - BlockHeaderUnavailableReason::SubsystemUnavailable, - )), - }; - - block_info_storage.insert( - next_ancestor_hash, - BlockInfo { - block_number: next_ancestor_number, - parent_hash: header.parent_hash, - maybe_allowed_relay_parents: None, - }, - ); - - header.parent_hash - }; - - ancestry.push(next_ancestor_hash); - if next_ancestor_number == 0 { - break - } - - next_ancestor_number -= 1; - next_ancestor_hash = parent_hash; - } - - ancestry - } else { - vec![leaf_hash] - }; - - let fetched_ancestry = - FetchSummary { minimum_ancestor_number: min_min, leaf_number: leaf_header.number }; - - let allowed_relay_parents = AllowedRelayParents { - minimum_relay_parents: min_relay_parents.into_iter().collect(), - allowed_relay_parents_contiguous: ancestry, - }; - - let leaf_block_info = BlockInfo { - parent_hash: leaf_header.parent_hash, - block_number: leaf_header.number, - maybe_allowed_relay_parents: Some(allowed_relay_parents), - }; - - block_info_storage.insert(leaf_hash, leaf_block_info); - - Ok(fetched_ancestry) -} - #[cfg(test)] mod tests { use super::*; @@ -798,6 +861,23 @@ mod tests { assert_eq!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_A)), Some(&expected_ancestry[..(PARA_A_MIN_PARENT - 1) as usize])); assert_eq!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_B)), Some(&expected_ancestry[..])); assert!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_C)).unwrap().is_empty()); + + assert_eq!(view.leaves.len(), 1); + assert!(view.leaves.contains_key(leaf)); + assert!(view.paths_via_relay_parent(&CHAIN_B[0]).is_empty()); + assert!(view.paths_via_relay_parent(&CHAIN_A[0]).is_empty()); + assert_eq!( + view.paths_via_relay_parent(&CHAIN_B[min_min_idx]), + vec![CHAIN_B[min_min_idx..].to_vec()] + ); + assert_eq!( + view.paths_via_relay_parent(&CHAIN_B[min_min_idx + 1]), + vec![CHAIN_B[min_min_idx..].to_vec()] + ); + assert_eq!( + view.paths_via_relay_parent(&leaf), + vec![CHAIN_B[min_min_idx..].to_vec()] + ); } ); @@ -918,6 +998,12 @@ mod tests { assert!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_B)).unwrap().is_empty()); assert!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_C)).unwrap().is_empty()); + + assert!(view.paths_via_relay_parent(&CHAIN_A[0]).is_empty()); + assert_eq!( + view.paths_via_relay_parent(&CHAIN_B[min_min_idx]), + vec![CHAIN_B[min_min_idx..].to_vec()] + ); } ); @@ -986,6 +1072,12 @@ mod tests { assert!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_B)).unwrap().is_empty()); assert!(view.known_allowed_relay_parents_under(&leaf, Some(PARA_C)).unwrap().is_empty()); + + assert!(view.paths_via_relay_parent(&GENESIS_HASH).is_empty()); + assert_eq!( + view.paths_via_relay_parent(&CHAIN_A[0]), + vec![CHAIN_A.to_vec()] + ); } ); } @@ -1160,4 +1252,69 @@ mod tests { Some(hashes) if hashes == &[GENESIS_HASH] ); } + + #[test] + fn path_with_fork() { + let pool = TaskExecutor::new(); + let (mut ctx, mut ctx_handle) = make_subsystem_context::(pool); + + let mut view = View::default(); + + assert_eq!(view.collating_for, None); + + // Chain A + let prospective_response = vec![(PARA_A, 0)]; // was PARA_A_MIN_PARENT + let leaf = CHAIN_A.last().unwrap(); + let blocks = [&[GENESIS_HASH], CHAIN_A].concat(); + let leaf_idx = blocks.len() - 1; + + let fut = view.activate_leaf(ctx.sender(), *leaf).timeout(TIMEOUT).map(|res| { + res.expect("`activate_leaf` timed out").unwrap(); + }); + let overseer_fut = async { + assert_block_header_requests(&mut ctx_handle, CHAIN_A, &blocks[leaf_idx..]).await; + assert_min_relay_parents_request(&mut ctx_handle, leaf, prospective_response).await; + assert_block_header_requests(&mut ctx_handle, CHAIN_A, &blocks[..leaf_idx]).await; + }; + futures::executor::block_on(join(fut, overseer_fut)); + + // Chain B + let prospective_response = vec![(PARA_A, 1)]; + + let leaf = CHAIN_B.last().unwrap(); + let leaf_idx = CHAIN_B.len() - 1; + + let fut = view.activate_leaf(ctx.sender(), *leaf).timeout(TIMEOUT).map(|res| { + res.expect("`activate_leaf` timed out").unwrap(); + }); + let overseer_fut = async { + assert_block_header_requests(&mut ctx_handle, CHAIN_B, &CHAIN_B[leaf_idx..]).await; + assert_min_relay_parents_request(&mut ctx_handle, leaf, prospective_response).await; + assert_block_header_requests(&mut ctx_handle, CHAIN_B, &CHAIN_B[0..leaf_idx]).await; + }; + futures::executor::block_on(join(fut, overseer_fut)); + + assert_eq!(view.leaves.len(), 2); + + let mut paths_to_genesis = view.paths_via_relay_parent(&GENESIS_HASH); + paths_to_genesis.sort(); + let mut expected_paths_to_genesis = vec![ + [GENESIS_HASH].iter().chain(CHAIN_A.iter()).copied().collect::>(), + [GENESIS_HASH].iter().chain(CHAIN_B.iter()).copied().collect::>(), + ]; + expected_paths_to_genesis.sort(); + assert_eq!(paths_to_genesis, expected_paths_to_genesis); + + let path_to_leaf_in_a = view.paths_via_relay_parent(&CHAIN_A[1]); + let expected_path_to_leaf_in_a = + vec![[GENESIS_HASH].iter().chain(CHAIN_A.iter()).copied().collect::>()]; + assert_eq!(path_to_leaf_in_a, expected_path_to_leaf_in_a); + + let path_to_leaf_in_b = view.paths_via_relay_parent(&CHAIN_B[4]); + let expected_path_to_leaf_in_b = + vec![[GENESIS_HASH].iter().chain(CHAIN_B.iter()).copied().collect::>()]; + assert_eq!(path_to_leaf_in_b, expected_path_to_leaf_in_b); + + assert_eq!(view.paths_via_relay_parent(&Hash::repeat_byte(0x0A)), Vec::>::new()); + } } diff --git a/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.toml b/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.toml index 9b3576eaa3c2..046d707cc1e8 100644 --- a/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.toml +++ b/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.toml @@ -37,4 +37,4 @@ onboard_as_parachain = false [parachains.collator] name = "collator2000" command = "polkadot-parachain" - args = [ "-lparachain=debug" ] + args = [ "-lparachain=debug", "--experimental-use-slot-based" ] diff --git a/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.zndsl b/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.zndsl index 7ba896e1c903..0cfc29f532d1 100644 --- a/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.zndsl +++ b/polkadot/zombienet_tests/elastic_scaling/0002-elastic-scaling-doesnt-break-parachains.zndsl @@ -12,7 +12,7 @@ validator: parachain 2000 block height is at least 10 within 200 seconds # Register the second core assigned to this parachain. alice: js-script ./assign-core.js with "0,2000,57600" return is 0 within 600 seconds -alice: js-script ./assign-core.js with "0,2000,57600" return is 0 within 600 seconds +alice: js-script ./assign-core.js with "1,2000,57600" return is 0 within 600 seconds validator: reports substrate_block_height{status="finalized"} is at least 35 within 100 seconds diff --git a/polkadot/zombienet_tests/functional/0018-shared-core-idle-parachain.toml b/polkadot/zombienet_tests/functional/0018-shared-core-idle-parachain.toml index 745c4f9e24b1..d3ff00002242 100644 --- a/polkadot/zombienet_tests/functional/0018-shared-core-idle-parachain.toml +++ b/polkadot/zombienet_tests/functional/0018-shared-core-idle-parachain.toml @@ -36,4 +36,4 @@ chain = "glutton-westend-local-2000" name = "collator-2000" image = "{{CUMULUS_IMAGE}}" command = "polkadot-parachain" - args = ["-lparachain=debug"] + args = ["-lparachain=debug", "--experimental-use-slot-based"] diff --git a/polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.toml b/polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.toml new file mode 100644 index 000000000000..43f3ef8f9e55 --- /dev/null +++ b/polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.toml @@ -0,0 +1,58 @@ +[settings] +timeout = 1000 + +[relaychain.genesis.runtimeGenesis.patch.configuration.config.async_backing_params] + max_candidate_depth = 3 + allowed_ancestry_len = 2 + +[relaychain.genesis.runtimeGenesis.patch.configuration.config.scheduler_params] + max_validators_per_core = 4 + num_cores = 1 + lookahead = 2 + +[relaychain.genesis.runtimeGenesis.patch.configuration.config.approval_voting_params] + needed_approvals = 3 + +[relaychain] +default_image = "{{ZOMBIENET_INTEGRATION_TEST_IMAGE}}" +chain = "rococo-local" +command = "polkadot" + + [[relaychain.node_groups]] + name = "validator" + args = ["-lparachain=debug,parachain::collator-protocol=trace" ] + count = 4 + +[[parachains]] +id = 2000 +register_para = false +onboard_as_parachain = false +add_to_genesis = false +chain = "glutton-westend-local-2000" + [parachains.genesis.runtimeGenesis.patch.glutton] + compute = "50000000" + storage = "2500000000" + trashDataCount = 5120 + + [parachains.collator] + name = "collator-2000" + image = "{{CUMULUS_IMAGE}}" + command = "polkadot-parachain" + args = ["-lparachain=debug,parachain::collator-protocol=trace", "--experimental-use-slot-based"] + +[[parachains]] +id = 2001 +register_para = false +onboard_as_parachain = false +add_to_genesis = false +chain = "glutton-westend-local-2001" + [parachains.genesis.runtimeGenesis.patch.glutton] + compute = "50000000" + storage = "2500000000" + trashDataCount = 5120 + + [parachains.collator] + name = "collator-2001" + image = "{{CUMULUS_IMAGE}}" + command = "polkadot-parachain" + args = ["-lparachain=debug"] diff --git a/polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.zndsl b/polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.zndsl new file mode 100644 index 000000000000..8892b03ac29c --- /dev/null +++ b/polkadot/zombienet_tests/functional/0019-coretime-collation-fetching-fairness.zndsl @@ -0,0 +1,16 @@ +Description: CT shared core fairness test +Network: ./0019-coretime-collation-fetching-fairness.toml +Creds: config + +validator: reports node_roles is 4 + +validator-0: js-script ./force-register-paras.js with "2000,2001" return is 0 within 600 seconds +# core 0 is shared 3:1 between paras +validator-0: js-script ./assign-core.js with "0,2000,43200,2001,14400" return is 0 within 600 seconds + +collator-2000: reports block height is at least 9 within 200 seconds +collator-2001: reports block height is at least 3 within 10 seconds + +# hardcoded check to verify that included onchain events are indeed 3:1 +validator-0: js-script ./0019-verify-included-events.js return is 1 within 120 seconds + diff --git a/polkadot/zombienet_tests/functional/0019-verify-included-events.js b/polkadot/zombienet_tests/functional/0019-verify-included-events.js new file mode 100644 index 000000000000..6557a5a80e6b --- /dev/null +++ b/polkadot/zombienet_tests/functional/0019-verify-included-events.js @@ -0,0 +1,51 @@ +function parse_pjs_int(input) { + return parseInt(input.replace(/,/g, '')); +} + +async function run(nodeName, networkInfo) { + const { wsUri, userDefinedTypes } = networkInfo.nodesByName[nodeName]; + const api = await zombie.connect(wsUri, userDefinedTypes); + + let blocks_per_para = {}; + + await new Promise(async (resolve, _) => { + let block_count = 0; + const unsubscribe = await api.query.system.events(async (events, block_hash) => { + block_count++; + + events.forEach((record) => { + const event = record.event; + + if (event.method != 'CandidateIncluded') { + return; + } + + let included_para_id = parse_pjs_int(event.toHuman().data[0].descriptor.paraId); + let relay_parent = event.toHuman().data[0].descriptor.relayParent; + if (blocks_per_para[included_para_id] == undefined) { + blocks_per_para[included_para_id] = 1; + } else { + blocks_per_para[included_para_id]++; + } + console.log(`CandidateIncluded for ${included_para_id}: block_offset=${block_count} relay_parent=${relay_parent}`); + }); + + if (block_count == 12) { + unsubscribe(); + return resolve(); + } + }); + }); + + console.log(`Result: 2000: ${blocks_per_para[2000]}, 2001: ${blocks_per_para[2001]}`); + // This check assumes that para 2000 runs slot based collator which respects its claim queue + // and para 2001 runs lookahead which generates blocks for each relay parent. + // + // For 12 blocks there will be one session change. One block won't have anything backed/included. + // In the next there will be one backed so for 12 blocks we should expect 10 included events - no + // more than 4 for para 2001 and at least 6 for para 2000. This should also cover the unlucky + // case when we observe two session changes during the 12 block period. + return (blocks_per_para[2000] >= 6) && (blocks_per_para[2001] <= 4); +} + +module.exports = { run }; diff --git a/prdoc/pr_4880.prdoc b/prdoc/pr_4880.prdoc new file mode 100644 index 000000000000..1bcd09088b5f --- /dev/null +++ b/prdoc/pr_4880.prdoc @@ -0,0 +1,31 @@ +title: Collation fetching fairness in collator protocol + +doc: + - audience: "Node Dev" + description: | + Implements collation fetching fairness in the validator side of the collator protocol. With + core time in place if two (or more) parachains share a single core no fairness was guaranteed + between them in terms of collation fetching. The current implementation was accepting up to + `max_candidate_depth + 1` seconded collations per relay parent and once this limit is reached + no new collations are accepted. A misbehaving collator can abuse this fact and prevent other + collators/parachains from advertising collations by advertising `max_candidate_depth + 1` + collations of its own. + To address this issue two changes are made: + 1. For each parachain id the validator accepts advertisements until the number of entries in + the claim queue equals the number of seconded candidates. + 2. When new collation should be fetched the validator inspects what was seconded so far, + what's in the claim queue and picks the first slot which hasn't got a collation seconded + and there is no candidate pending seconding for it. If there is an advertisement in the + waiting queue for it it is fetched. Otherwise the next free slot is picked. + These two changes guarantee that: + 1. Validator doesn't accept more collations than it can actually back. + 2. Each parachain has got a fair share of core time based on its allocations in the claim + queue. + +crates: + - name: polkadot-collator-protocol + bump: patch + - name: polkadot + bump: patch + - name: polkadot-node-subsystem-util + bump: minor \ No newline at end of file