From 9bfdc59e3c74bfc97a14e0a5083bc82b235cbad5 Mon Sep 17 00:00:00 2001 From: alindima Date: Wed, 31 Jan 2024 18:02:41 +0200 Subject: [PATCH 1/5] prospective-parachains: respond with multiple backable candidates paves the way for elastic scaling --- .../src/fragment_tree.rs | 97 ++++- .../core/prospective-parachains/src/lib.rs | 57 +-- .../core/prospective-parachains/src/tests.rs | 359 +++++++++++++++++- polkadot/node/core/provisioner/src/lib.rs | 7 +- polkadot/node/core/provisioner/src/tests.rs | 19 +- polkadot/node/subsystem-types/src/messages.rs | 8 +- .../node/backing/prospective-parachains.md | 7 +- .../src/node/utility/provisioner.md | 8 +- 8 files changed, 483 insertions(+), 79 deletions(-) diff --git a/polkadot/node/core/prospective-parachains/src/fragment_tree.rs b/polkadot/node/core/prospective-parachains/src/fragment_tree.rs index 292e4ebe5282..d91e3a145aa7 100644 --- a/polkadot/node/core/prospective-parachains/src/fragment_tree.rs +++ b/polkadot/node/core/prospective-parachains/src/fragment_tree.rs @@ -763,43 +763,106 @@ impl FragmentTree { /// The intention of the `required_path` is to allow queries on the basis of /// one or more candidates which were previously pending availability becoming /// available and opening up more room on the core. - pub(crate) fn select_child( + pub(crate) fn select_children( &self, required_path: &[CandidateHash], + count: u32, pred: impl Fn(&CandidateHash) -> bool, - ) -> Option { + ) -> Vec { let base_node = { // traverse the required path. let mut node = NodePointer::Root; for required_step in required_path { - node = self.node_candidate_child(node, &required_step)?; + if let Some(next_node) = self.node_candidate_child(node, &required_step) { + node = next_node; + } else { + return vec![] + }; } node }; - // TODO [now]: taking the first selection might introduce bias + // TODO: taking the first best selection might introduce bias // or become gameable. // // For plausibly unique parachains, this shouldn't matter much. // figure out alternative selection criteria? - match base_node { + self.select_children_inner(base_node, count, count, &pred, &mut vec![]) + } + + // Try finding a candidate chain starting from `base_node` of length `expected_count`. + // If not possible, return the longest we could find. + // Does a depth-first search, since we're optimistic that there won't be more than one such + // chains. Assumes that the chain must be acyclic. + fn select_children_inner( + &self, + base_node: NodePointer, + expected_count: u32, + remaining_count: u32, + pred: &dyn Fn(&CandidateHash) -> bool, + accumulator: &mut Vec, + ) -> Vec { + if remaining_count == 0 { + // The best option is the chain we've accumulated so far. + return accumulator.to_vec(); + } + + let children: Vec<_> = match base_node { NodePointer::Root => self .nodes .iter() - .take_while(|n| n.parent == NodePointer::Root) - .filter(|n| self.scope.get_pending_availability(&n.candidate_hash).is_none()) - .filter(|n| pred(&n.candidate_hash)) - .map(|n| n.candidate_hash) - .next(), - NodePointer::Storage(ptr) => self.nodes[ptr] - .children - .iter() - .filter(|n| self.scope.get_pending_availability(&n.1).is_none()) - .filter(|n| pred(&n.1)) - .map(|n| n.1) - .next(), + .enumerate() + .take_while(|(_, n)| n.parent == NodePointer::Root) + .filter(|(_, n)| self.scope.get_pending_availability(&n.candidate_hash).is_none()) + .filter(|(_, n)| pred(&n.candidate_hash)) + .map(|(ptr, n)| (NodePointer::Storage(ptr), n.candidate_hash)) + .collect(), + NodePointer::Storage(base_node_ptr) => { + let base_node = &self.nodes[base_node_ptr]; + + base_node + .children + .iter() + .filter(|(_, hash)| self.scope.get_pending_availability(&hash).is_none()) + .filter(|(_, hash)| pred(&hash)) + .map(|(ptr, hash)| (*ptr, *hash)) + .collect() + }, + }; + + let mut best_result = accumulator.clone(); + for (child_ptr, child_hash) in children { + // We could use a HashSet/BTreeSet for tracking the visited nodes but realistically, + // accumulator will not hold more than 2-3 elements (the max number of cores for a + // parachain). + if accumulator.contains(&child_hash) { + // We've already visited this node. There's a cycle in the tree, ignore it. + continue + } + + let mut accumulator_copy = accumulator.clone(); + + accumulator_copy.push(child_hash); + + let result = self.select_children_inner( + child_ptr, + expected_count, + remaining_count - 1, + &pred, + &mut accumulator_copy, + ); + + // Short-circuit the search if we've found the right length. Otherwise, we'll + // search for a max. + if result.len() == expected_count as usize { + return result + } else if best_result.len() < result.len() { + best_result = result; + } } + + best_result } fn populate_from_bases(&mut self, storage: &CandidateStorage, initial_bases: Vec) { diff --git a/polkadot/node/core/prospective-parachains/src/lib.rs b/polkadot/node/core/prospective-parachains/src/lib.rs index dabcfb80e02e..731802631baf 100644 --- a/polkadot/node/core/prospective-parachains/src/lib.rs +++ b/polkadot/node/core/prospective-parachains/src/lib.rs @@ -146,12 +146,20 @@ async fn run_iteration( handle_candidate_seconded(view, para, candidate_hash), ProspectiveParachainsMessage::CandidateBacked(para, candidate_hash) => handle_candidate_backed(&mut *ctx, view, para, candidate_hash).await?, - ProspectiveParachainsMessage::GetBackableCandidate( + ProspectiveParachainsMessage::GetBackableCandidates( relay_parent, para, + n_cores, required_path, tx, - ) => answer_get_backable_candidate(&view, relay_parent, para, required_path, tx), + ) => answer_get_backable_candidate( + &view, + relay_parent, + para, + n_cores, + required_path, + tx, + ), ProspectiveParachainsMessage::GetHypotheticalFrontier(request, tx) => answer_hypothetical_frontier_request(&view, request, tx), ProspectiveParachainsMessage::GetTreeMembership(para, candidate, tx) => @@ -556,8 +564,9 @@ fn answer_get_backable_candidate( view: &View, relay_parent: Hash, para: ParaId, + n_cores: u32, required_path: Vec, - tx: oneshot::Sender>, + tx: oneshot::Sender>, ) { let data = match view.active_leaves.get(&relay_parent) { None => { @@ -568,7 +577,7 @@ fn answer_get_backable_candidate( "Requested backable candidate for inactive relay-parent." ); - let _ = tx.send(None); + let _ = tx.send(vec![]); return }, Some(d) => d, @@ -583,7 +592,7 @@ fn answer_get_backable_candidate( "Requested backable candidate for inactive para." ); - let _ = tx.send(None); + let _ = tx.send(vec![]); return }, Some(tree) => tree, @@ -598,30 +607,32 @@ fn answer_get_backable_candidate( "No candidate storage for active para", ); - let _ = tx.send(None); + let _ = tx.send(vec![]); return }, Some(s) => s, }; - let Some(child_hash) = - tree.select_child(&required_path, |candidate| storage.is_backed(candidate)) - else { - let _ = tx.send(None); - return - }; - let Some(candidate_relay_parent) = storage.relay_parent_by_candidate_hash(&child_hash) else { - gum::error!( - target: LOG_TARGET, - ?child_hash, - para_id = ?para, - "Candidate is present in fragment tree but not in candidate's storage!", - ); - let _ = tx.send(None); - return - }; + let backable_children = tree + .select_children(&required_path, n_cores, |candidate| storage.is_backed(candidate)) + .into_iter() + .filter_map(|child_hash| { + storage.relay_parent_by_candidate_hash(&child_hash).map_or_else( + || { + gum::error!( + target: LOG_TARGET, + ?child_hash, + para_id = ?para, + "Candidate is present in fragment tree but not in candidate's storage!", + ); + None + }, + |parent_hash| Some((child_hash, parent_hash)), + ) + }) + .collect(); - let _ = tx.send(Some((child_hash, candidate_relay_parent))); + let _ = tx.send(backable_children); } fn answer_hypothetical_frontier_request( diff --git a/polkadot/node/core/prospective-parachains/src/tests.rs b/polkadot/node/core/prospective-parachains/src/tests.rs index 7e369245c0e1..b192495b3892 100644 --- a/polkadot/node/core/prospective-parachains/src/tests.rs +++ b/polkadot/node/core/prospective-parachains/src/tests.rs @@ -403,25 +403,28 @@ async fn get_membership( assert_eq!(resp, expected_membership_response); } -async fn get_backable_candidate( +async fn get_backable_candidates( virtual_overseer: &mut VirtualOverseer, leaf: &TestLeaf, para_id: ParaId, required_path: Vec, - expected_result: Option<(CandidateHash, Hash)>, + count: u32, + expected_result: Vec<(CandidateHash, Hash)>, ) { let (tx, rx) = oneshot::channel(); virtual_overseer .send(overseer::FromOrchestra::Communication { - msg: ProspectiveParachainsMessage::GetBackableCandidate( + msg: ProspectiveParachainsMessage::GetBackableCandidates( leaf.hash, para_id, + count, required_path, tx, ), }) .await; let resp = rx.await.unwrap(); + assert_eq!(resp.len(), expected_result.len()); assert_eq!(resp, expected_result); } @@ -849,9 +852,9 @@ fn check_candidate_on_multiple_forks() { assert_eq!(view.candidate_storage.get(&2.into()).unwrap().len(), (0, 0)); } -// Backs some candidates and tests `GetBackableCandidate`. +// Backs some candidates and tests `GetBackableCandidates` when requesting a single candidate. #[test] -fn check_backable_query() { +fn check_backable_query_single_candidate() { let test_state = TestState::default(); let view = test_harness(|mut virtual_overseer| async move { // Leaf A @@ -896,26 +899,38 @@ fn check_backable_query() { introduce_candidate(&mut virtual_overseer, candidate_b.clone(), pvd_b).await; // Should not get any backable candidates. - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), vec![candidate_hash_a], - None, + 1, + vec![], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 0, + vec![], ) .await; + get_backable_candidates(&mut virtual_overseer, &leaf_a, 1.into(), vec![], 0, vec![]).await; // Second candidates. second_candidate(&mut virtual_overseer, candidate_a.clone()).await; second_candidate(&mut virtual_overseer, candidate_b.clone()).await; // Should not get any backable candidates. - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), vec![candidate_hash_a], - None, + 1, + vec![], ) .await; @@ -923,31 +938,46 @@ fn check_backable_query() { back_candidate(&mut virtual_overseer, &candidate_a, candidate_hash_a).await; back_candidate(&mut virtual_overseer, &candidate_b, candidate_hash_b).await; + // Should not get any backable candidates for the other para. + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 1, vec![]).await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 2.into(), + vec![candidate_hash_a], + 1, + vec![], + ) + .await; + // Get backable candidate. - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), vec![], - Some((candidate_hash_a, leaf_a.hash)), + 1, + vec![(candidate_hash_a, leaf_a.hash)], ) .await; - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), vec![candidate_hash_a], - Some((candidate_hash_b, leaf_a.hash)), + 1, + vec![(candidate_hash_b, leaf_a.hash)], ) .await; // Should not get anything at the wrong path. - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), vec![candidate_hash_b], - None, + 1, + vec![], ) .await; @@ -961,6 +991,293 @@ fn check_backable_query() { assert_eq!(view.candidate_storage.get(&2.into()).unwrap().len(), (0, 0)); } +// Backs some candidates and tests `GetBackableCandidates` when requesting a multiple candidates. +#[test] +fn check_backable_query_multiple_candidates() { + let test_state = TestState::default(); + let view = test_harness(|mut virtual_overseer| async move { + // Parachain 1 looks like this: + // +---A---+ + // | | + // | | + // +----B---+ C----+ + // | | | | + // | | | | + // | | | | + // | | | | + // D E F H + // | | + // | | + // | | + // G I + // | + // | + // J + + // Leaf A + let leaf_a = TestLeaf { + number: 100, + hash: Hash::from_low_u64_be(130), + para_data: vec![ + (1.into(), PerParaData::new(97, HeadData(vec![1, 2, 3]))), + (2.into(), PerParaData::new(100, HeadData(vec![2, 3, 4]))), + ], + }; + + // Activate leaves. + activate_leaf(&mut virtual_overseer, &leaf_a, &test_state).await; + + // Candidate A + let (candidate_a, pvd_a) = make_candidate( + leaf_a.hash, + leaf_a.number, + 1.into(), + HeadData(vec![1, 2, 3]), + HeadData(vec![1]), + test_state.validation_code_hash, + ); + let candidate_hash_a = candidate_a.hash(); + introduce_candidate(&mut virtual_overseer, candidate_a.clone(), pvd_a).await; + second_candidate(&mut virtual_overseer, candidate_a.clone()).await; + back_candidate(&mut virtual_overseer, &candidate_a, candidate_hash_a).await; + + macro_rules! make_and_back_candidates { + ($parent:expr, $index:expr) => {{ + let (mut candidate, pvd) = make_candidate( + leaf_a.hash, + leaf_a.number, + 1.into(), + $parent.commitments.head_data.clone(), + HeadData(vec![$index]), + test_state.validation_code_hash, + ); + // Set a field to make this candidate unique. + candidate.descriptor.para_head = Hash::from_low_u64_le($index); + let candidate_hash = candidate.hash(); + introduce_candidate(&mut virtual_overseer, candidate.clone(), pvd).await; + second_candidate(&mut virtual_overseer, candidate.clone()).await; + back_candidate(&mut virtual_overseer, &candidate, candidate_hash).await; + + (candidate, candidate_hash) + }}; + } + + let (candidate_b, candidate_hash_b) = make_and_back_candidates!(&candidate_a, 2); + let (candidate_c, candidate_hash_c) = make_and_back_candidates!(&candidate_a, 3); + let (_candidate_d, candidate_hash_d) = make_and_back_candidates!(&candidate_b, 4); + let (_candidate_e, candidate_hash_e) = make_and_back_candidates!(&candidate_b, 5); + let (candidate_f, candidate_hash_f) = make_and_back_candidates!(&candidate_b, 6); + let (_candidate_g, candidate_hash_g) = make_and_back_candidates!(&candidate_f, 7); + let (candidate_h, candidate_hash_h) = make_and_back_candidates!(&candidate_c, 8); + let (candidate_i, candidate_hash_i) = make_and_back_candidates!(&candidate_h, 9); + let (_candidate_j, candidate_hash_j) = make_and_back_candidates!(&candidate_i, 10); + + // Should not get any backable candidates for the other para. + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 1, vec![]).await; + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 5, vec![]).await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 2.into(), + vec![candidate_hash_a], + 1, + vec![], + ) + .await; + + // Test various scenarios with various counts. + + // empty required_path + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![], + 1, + vec![(candidate_hash_a, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![], + 4, + vec![ + (candidate_hash_a, leaf_a.hash), + (candidate_hash_b, leaf_a.hash), + (candidate_hash_f, leaf_a.hash), + (candidate_hash_g, leaf_a.hash), + ], + ) + .await; + } + + // required path of 1 + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 1, + vec![(candidate_hash_b, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 2, + vec![(candidate_hash_b, leaf_a.hash), (candidate_hash_d, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 3, + vec![ + (candidate_hash_b, leaf_a.hash), + (candidate_hash_f, leaf_a.hash), + (candidate_hash_g, leaf_a.hash), + ], + ) + .await; + + // If the requested count exceeds the largest chain, return the longest + // chain we can get. + for count in 5..10 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + count, + vec![ + (candidate_hash_c, leaf_a.hash), + (candidate_hash_h, leaf_a.hash), + (candidate_hash_i, leaf_a.hash), + (candidate_hash_j, leaf_a.hash), + ], + ) + .await; + } + } + + // required path of 2 + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_b], + 1, + vec![(candidate_hash_d, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_c], + 1, + vec![(candidate_hash_h, leaf_a.hash)], + ) + .await; + // If the requested count exceeds the largest chain, return the longest + // chain we can get. + for count in 4..10 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_c], + count, + vec![ + (candidate_hash_h, leaf_a.hash), + (candidate_hash_i, leaf_a.hash), + (candidate_hash_j, leaf_a.hash), + ], + ) + .await; + } + } + + // No more candidates in any chain. + { + let required_paths = vec![ + vec![candidate_hash_a, candidate_hash_b, candidate_hash_e], + vec![ + candidate_hash_a, + candidate_hash_c, + candidate_hash_h, + candidate_hash_i, + candidate_hash_j, + ], + ]; + for path in required_paths { + for count in 1..4 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + path.clone(), + count, + vec![], + ) + .await; + } + } + } + + // Should not get anything at the wrong path. + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_b], + 1, + vec![], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_b, candidate_hash_a], + 3, + vec![], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_b, candidate_hash_c], + 3, + vec![], + ) + .await; + + virtual_overseer + }); + + assert_eq!(view.active_leaves.len(), 1); + assert_eq!(view.candidate_storage.len(), 2); + // 10 candidates and 7 parents on para 1. + assert_eq!(view.candidate_storage.get(&1.into()).unwrap().len(), (7, 10)); + assert_eq!(view.candidate_storage.get(&2.into()).unwrap().len(), (0, 0)); +} + +#[test] +fn check_backable_query_cycle() { + // TODO +} + // Test depth query. #[test] fn check_hypothetical_frontier_query() { @@ -1448,12 +1765,13 @@ fn persists_pending_availability_candidate() { second_candidate(&mut virtual_overseer, candidate_b.clone()).await; back_candidate(&mut virtual_overseer, &candidate_b, candidate_hash_b).await; - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_b, para_id, vec![candidate_hash_a], - Some((candidate_hash_b, leaf_b_hash)), + 1, + vec![(candidate_hash_b, leaf_b_hash)], ) .await; @@ -1512,12 +1830,13 @@ fn backwards_compatible() { second_candidate(&mut virtual_overseer, candidate_a.clone()).await; back_candidate(&mut virtual_overseer, &candidate_a, candidate_hash_a).await; - get_backable_candidate( + get_backable_candidates( &mut virtual_overseer, &leaf_a, para_id, vec![], - Some((candidate_hash_a, candidate_relay_parent)), + 1, + vec![(candidate_hash_a, candidate_relay_parent)], ) .await; @@ -1537,7 +1856,7 @@ fn backwards_compatible() { ) .await; - get_backable_candidate(&mut virtual_overseer, &leaf_b, para_id, vec![], None).await; + get_backable_candidates(&mut virtual_overseer, &leaf_b, para_id, vec![], 1, vec![]).await; virtual_overseer }); diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs index 3970b8572612..51f768d782e0 100644 --- a/polkadot/node/core/provisioner/src/lib.rs +++ b/polkadot/node/core/provisioner/src/lib.rs @@ -806,15 +806,18 @@ async fn get_backable_candidate( ) -> Result, Error> { let (tx, rx) = oneshot::channel(); sender - .send_message(ProspectiveParachainsMessage::GetBackableCandidate( + .send_message(ProspectiveParachainsMessage::GetBackableCandidates( relay_parent, para_id, + 1, // core count hardcoded to 1, until elastic scaling is implemented and enabled. required_path, tx, )) .await; - rx.await.map_err(Error::CanceledBackableCandidate) + rx.await + .map_err(Error::CanceledBackableCandidate) + .map(|res| res.get(0).copied()) } /// The availability bitfield for a given core is the transpose diff --git a/polkadot/node/core/provisioner/src/tests.rs b/polkadot/node/core/provisioner/src/tests.rs index 1d7bdfcfcb89..b26df8ddb910 100644 --- a/polkadot/node/core/provisioner/src/tests.rs +++ b/polkadot/node/core/provisioner/src/tests.rs @@ -373,13 +373,18 @@ mod select_candidates { let _ = sender.send(response); }, AllMessages::ProspectiveParachains( - ProspectiveParachainsMessage::GetBackableCandidate(.., tx), - ) => match prospective_parachains_mode { - ProspectiveParachainsMode::Enabled { .. } => { - let _ = tx.send(candidates_iter.next()); - }, - ProspectiveParachainsMode::Disabled => - panic!("unexpected prospective parachains request"), + ProspectiveParachainsMessage::GetBackableCandidates(_, _, count, _, tx), + ) => { + assert_eq!(count, 1); + + match prospective_parachains_mode { + ProspectiveParachainsMode::Enabled { .. } => { + let _ = + tx.send(candidates_iter.next().map_or_else(Vec::new, |c| vec![c])); + }, + ProspectiveParachainsMode::Disabled => + panic!("unexpected prospective parachains request"), + } }, _ => panic!("Unexpected message: {:?}", from_job), } diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 1d5d82b57fdf..549e43a671d6 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -1128,14 +1128,16 @@ pub enum ProspectiveParachainsMessage { /// has been backed. This requires that the candidate was successfully introduced in /// the past. CandidateBacked(ParaId, CandidateHash), - /// Get a backable candidate hash along with its relay parent for the given parachain, + /// Get N backable candidate hashes along with their relay parents for the given parachain, /// under the given relay-parent hash, which is a descendant of the given candidate hashes. + /// N should represent the number of scheduled cores of this ParaId. /// Returns `None` on the channel if no such candidate exists. - GetBackableCandidate( + GetBackableCandidates( Hash, ParaId, + u32, Vec, - oneshot::Sender>, + oneshot::Sender>, ), /// Get the hypothetical frontier membership of candidates with the given properties /// under the specified active leaves' fragment trees. diff --git a/polkadot/roadmap/implementers-guide/src/node/backing/prospective-parachains.md b/polkadot/roadmap/implementers-guide/src/node/backing/prospective-parachains.md index 286aeddb986d..8f00ff084941 100644 --- a/polkadot/roadmap/implementers-guide/src/node/backing/prospective-parachains.md +++ b/polkadot/roadmap/implementers-guide/src/node/backing/prospective-parachains.md @@ -92,9 +92,10 @@ prospective validation data. This is unlikely to change. been backed. - Sent by the Backing Subsystem after it successfully imports a statement giving a candidate the necessary quorum of backing votes. -- `ProspectiveParachainsMessage::GetBackableCandidate` - - Get a backable candidate hash along with its relay parent for a given parachain, - under a given relay-parent (leaf) hash, which is a descendant of given candidate hashes. +- `ProspectiveParachainsMessage::GetBackableCandidates` + - Get the requested number of backable candidate hashes along with their relay parent for a given + parachain,under a given relay-parent (leaf) hash, which are descendants of given candidate + hashes. - Sent by the Provisioner when requesting backable candidates, when selecting candidates for a given relay-parent. - `ProspectiveParachainsMessage::GetHypotheticalFrontier` diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md b/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md index 0b4fe6a45873..b017259da8c0 100644 --- a/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md +++ b/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md @@ -187,16 +187,16 @@ this process is a vector of `CandidateHash`s, sorted in order of their core inde #### Required Path -Required path is a parameter for `ProspectiveParachainsMessage::GetBackableCandidate`, which the provisioner sends in +Required path is a parameter for `ProspectiveParachainsMessage::GetBackableCandidates`, which the provisioner sends in candidate selection. -An empty required path indicates that the requested candidate should be a direct child of the most recently included +An empty required path indicates that the requested candidate chain should start with the most recently included parablock for the given `para_id` as of the given relay parent. In contrast, a required path with one or more entries prompts [prospective parachains](../backing/prospective-parachains.md) to step forward through its fragment tree for the given `para_id` and -relay parent until the desired parablock is reached. We then select a direct child of that parablock to pass to the -provisioner. +relay parent until the desired parablock is reached. We then select the chain starting with the direct child of that +parablock to pass to the provisioner. The parablocks making up a required path do not need to have been previously seen as included in relay chain blocks. Thus the ability to provision backable candidates based on a required path effectively decouples backing from inclusion. From a1f208d30efe438828f36c1e00b2b2ab2846699f Mon Sep 17 00:00:00 2001 From: alindima Date: Fri, 2 Feb 2024 12:24:12 +0200 Subject: [PATCH 2/5] allow cycles, address review comments and document performance --- .../src/fragment_tree.rs | 49 ++++++++++++------- .../core/prospective-parachains/src/lib.rs | 8 +-- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/polkadot/node/core/prospective-parachains/src/fragment_tree.rs b/polkadot/node/core/prospective-parachains/src/fragment_tree.rs index d91e3a145aa7..21f0e027620d 100644 --- a/polkadot/node/core/prospective-parachains/src/fragment_tree.rs +++ b/polkadot/node/core/prospective-parachains/src/fragment_tree.rs @@ -753,12 +753,15 @@ impl FragmentTree { depths.iter_ones().collect() } - /// Select a candidate after the given `required_path` which passes - /// the predicate. + /// Select `count` candidates after the given `required_path` which pass + /// the predicate and have not already been backed on chain. /// - /// If there are multiple possibilities, this will select the first one. - /// - /// This returns `None` if there is no candidate meeting those criteria. + /// Does an exhaustive search into the tree starting after `required_path`. + /// If there are multiple possibilities of size `count`, this will select the first one. + /// If there is no chain of size `count` that matches the criteria, this will return the largest + /// chain it could find with the criteria. + /// If there are no candidates meeting those criteria, returns an empty `Vec`. + /// Cycles are accepted, see module docs for the `Cycles` section. /// /// The intention of the `required_path` is to allow queries on the basis of /// one or more candidates which were previously pending availability becoming @@ -792,9 +795,25 @@ impl FragmentTree { } // Try finding a candidate chain starting from `base_node` of length `expected_count`. - // If not possible, return the longest we could find. + // If not possible, return the longest one we could find. // Does a depth-first search, since we're optimistic that there won't be more than one such - // chains. Assumes that the chain must be acyclic. + // chains (parachains shouldn't usually have forks). So in the usual case, this will conclude + // in `O(expected_count)`. + // Cycles are accepted, but this doesn't allow for infinite execution time, because the maximum + // depth we'll reach is `expected_count`. + // + // Worst case performance is `O(num_forks ^ expected_count)`. + // Although an exponential function, this is actually a constant that can only be altered via + // sudo/governance, because: + // 1. `num_forks` at a given level is at most `max_candidate_depth * max_validators_per_core` + // (because each validator in the assigned group can second `max_candidate_depth` + // candidates). The prospective-parachains subsystem assumes that the number of para forks is + // limited by collator-protocol and backing subsystems. In practice, this is a constant which + // can only be altered by sudo or governance. + // 2. `expected_count` is equal to the number of cores a para is scheduled on (in an elastic + // scaling scenario). For non-elastic-scaling, this is just 1. In practice, this should be a + // small number (1-3), capped by the total number of available cores (a constant alterable + // only via governance/sudo). fn select_children_inner( &self, base_node: NodePointer, @@ -833,26 +852,18 @@ impl FragmentTree { let mut best_result = accumulator.clone(); for (child_ptr, child_hash) in children { - // We could use a HashSet/BTreeSet for tracking the visited nodes but realistically, - // accumulator will not hold more than 2-3 elements (the max number of cores for a - // parachain). - if accumulator.contains(&child_hash) { - // We've already visited this node. There's a cycle in the tree, ignore it. - continue - } - - let mut accumulator_copy = accumulator.clone(); - - accumulator_copy.push(child_hash); + accumulator.push(child_hash); let result = self.select_children_inner( child_ptr, expected_count, remaining_count - 1, &pred, - &mut accumulator_copy, + accumulator, ); + accumulator.pop(); + // Short-circuit the search if we've found the right length. Otherwise, we'll // search for a max. if result.len() == expected_count as usize { diff --git a/polkadot/node/core/prospective-parachains/src/lib.rs b/polkadot/node/core/prospective-parachains/src/lib.rs index 731802631baf..f513d1a43ddf 100644 --- a/polkadot/node/core/prospective-parachains/src/lib.rs +++ b/polkadot/node/core/prospective-parachains/src/lib.rs @@ -149,14 +149,14 @@ async fn run_iteration( ProspectiveParachainsMessage::GetBackableCandidates( relay_parent, para, - n_cores, + count, required_path, tx, ) => answer_get_backable_candidate( &view, relay_parent, para, - n_cores, + count, required_path, tx, ), @@ -564,7 +564,7 @@ fn answer_get_backable_candidate( view: &View, relay_parent: Hash, para: ParaId, - n_cores: u32, + count: u32, required_path: Vec, tx: oneshot::Sender>, ) { @@ -614,7 +614,7 @@ fn answer_get_backable_candidate( }; let backable_children = tree - .select_children(&required_path, n_cores, |candidate| storage.is_backed(candidate)) + .select_children(&required_path, count, |candidate| storage.is_backed(candidate)) .into_iter() .filter_map(|child_hash| { storage.relay_parent_by_candidate_hash(&child_hash).map_or_else( From d4f38bddbde588b143b58db041b61c8075b1cd70 Mon Sep 17 00:00:00 2001 From: alindima Date: Fri, 2 Feb 2024 12:29:20 +0200 Subject: [PATCH 3/5] add prdoc --- prdoc/pr_3160.prdoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 prdoc/pr_3160.prdoc diff --git a/prdoc/pr_3160.prdoc b/prdoc/pr_3160.prdoc new file mode 100644 index 000000000000..22305b6635aa --- /dev/null +++ b/prdoc/pr_3160.prdoc @@ -0,0 +1,12 @@ +title: "prospective-parachains: allow requesting a chain of backable candidates" + +topic: Node + +doc: + - audience: Node Dev + description: | + Enable requesting a chain of multiple backable candidates. Will be used by the provisioner + to build paras inherent data for elastic scaling. + +crates: + - name: polkadot-node-core-prospective-parachains From baf0fd0f8d0614a252516e72f11b881656cd7fdc Mon Sep 17 00:00:00 2001 From: alindima Date: Fri, 2 Feb 2024 15:35:32 +0200 Subject: [PATCH 4/5] more tests --- .../src/fragment_tree.rs | 45 ++ .../core/prospective-parachains/src/tests.rs | 701 ++++++++++++------ 2 files changed, 535 insertions(+), 211 deletions(-) diff --git a/polkadot/node/core/prospective-parachains/src/fragment_tree.rs b/polkadot/node/core/prospective-parachains/src/fragment_tree.rs index 21f0e027620d..555725e58255 100644 --- a/polkadot/node/core/prospective-parachains/src/fragment_tree.rs +++ b/polkadot/node/core/prospective-parachains/src/fragment_tree.rs @@ -1058,6 +1058,7 @@ mod tests { use polkadot_node_subsystem_util::inclusion_emulator::InboundHrmpLimitations; use polkadot_primitives::{BlockNumber, CandidateCommitments, CandidateDescriptor, HeadData}; use polkadot_primitives_test_helpers as test_helpers; + use std::iter; fn make_constraints( min_relay_parent_number: BlockNumber, @@ -1595,6 +1596,21 @@ mod tests { assert_eq!(tree.nodes[2].candidate_hash, candidate_a_hash); assert_eq!(tree.nodes[3].candidate_hash, candidate_a_hash); assert_eq!(tree.nodes[4].candidate_hash, candidate_a_hash); + + for count in 1..10 { + assert_eq!( + tree.select_children(&[], count, |_| true), + iter::repeat(candidate_a_hash) + .take(std::cmp::min(count as usize, max_depth + 1)) + .collect::>() + ); + assert_eq!( + tree.select_children(&[candidate_a_hash], count - 1, |_| true), + iter::repeat(candidate_a_hash) + .take(std::cmp::min(count as usize - 1, max_depth)) + .collect::>() + ); + } } #[test] @@ -1662,6 +1678,35 @@ mod tests { assert_eq!(tree.nodes[2].candidate_hash, candidate_a_hash); assert_eq!(tree.nodes[3].candidate_hash, candidate_b_hash); assert_eq!(tree.nodes[4].candidate_hash, candidate_a_hash); + + assert_eq!(tree.select_children(&[], 1, |_| true), vec![candidate_a_hash],); + assert_eq!( + tree.select_children(&[], 2, |_| true), + vec![candidate_a_hash, candidate_b_hash], + ); + assert_eq!( + tree.select_children(&[], 3, |_| true), + vec![candidate_a_hash, candidate_b_hash, candidate_a_hash], + ); + assert_eq!( + tree.select_children(&[candidate_a_hash], 2, |_| true), + vec![candidate_b_hash, candidate_a_hash], + ); + + assert_eq!( + tree.select_children(&[], 6, |_| true), + vec![ + candidate_a_hash, + candidate_b_hash, + candidate_a_hash, + candidate_b_hash, + candidate_a_hash + ], + ); + assert_eq!( + tree.select_children(&[candidate_a_hash, candidate_b_hash], 6, |_| true), + vec![candidate_a_hash, candidate_b_hash, candidate_a_hash,], + ); } #[test] diff --git a/polkadot/node/core/prospective-parachains/src/tests.rs b/polkadot/node/core/prospective-parachains/src/tests.rs index b192495b3892..732736b101de 100644 --- a/polkadot/node/core/prospective-parachains/src/tests.rs +++ b/polkadot/node/core/prospective-parachains/src/tests.rs @@ -994,288 +994,567 @@ fn check_backable_query_single_candidate() { // Backs some candidates and tests `GetBackableCandidates` when requesting a multiple candidates. #[test] fn check_backable_query_multiple_candidates() { - let test_state = TestState::default(); - let view = test_harness(|mut virtual_overseer| async move { - // Parachain 1 looks like this: - // +---A---+ - // | | - // | | - // +----B---+ C----+ - // | | | | - // | | | | - // | | | | - // | | | | - // D E F H - // | | - // | | - // | | - // G I - // | - // | - // J + macro_rules! make_and_back_candidate { + ($test_state:ident, $virtual_overseer:ident, $leaf:ident, $parent:expr, $index:expr) => {{ + let (mut candidate, pvd) = make_candidate( + $leaf.hash, + $leaf.number, + 1.into(), + $parent.commitments.head_data.clone(), + HeadData(vec![$index]), + $test_state.validation_code_hash, + ); + // Set a field to make this candidate unique. + candidate.descriptor.para_head = Hash::from_low_u64_le($index); + let candidate_hash = candidate.hash(); + introduce_candidate(&mut $virtual_overseer, candidate.clone(), pvd).await; + second_candidate(&mut $virtual_overseer, candidate.clone()).await; + back_candidate(&mut $virtual_overseer, &candidate, candidate_hash).await; + + (candidate, candidate_hash) + }}; + } - // Leaf A - let leaf_a = TestLeaf { - number: 100, - hash: Hash::from_low_u64_be(130), - para_data: vec![ - (1.into(), PerParaData::new(97, HeadData(vec![1, 2, 3]))), - (2.into(), PerParaData::new(100, HeadData(vec![2, 3, 4]))), - ], - }; + // Parachain 1 looks like this: + // +---A----+ + // | | + // +----B---+ C + // | | | | + // D E F H + // | | + // G I + // | + // J + { + let test_state = TestState::default(); + let view = test_harness(|mut virtual_overseer| async move { + // Leaf A + let leaf_a = TestLeaf { + number: 100, + hash: Hash::from_low_u64_be(130), + para_data: vec![ + (1.into(), PerParaData::new(97, HeadData(vec![1, 2, 3]))), + (2.into(), PerParaData::new(100, HeadData(vec![2, 3, 4]))), + ], + }; - // Activate leaves. - activate_leaf(&mut virtual_overseer, &leaf_a, &test_state).await; + // Activate leaves. + activate_leaf(&mut virtual_overseer, &leaf_a, &test_state).await; - // Candidate A - let (candidate_a, pvd_a) = make_candidate( - leaf_a.hash, - leaf_a.number, - 1.into(), - HeadData(vec![1, 2, 3]), - HeadData(vec![1]), - test_state.validation_code_hash, - ); - let candidate_hash_a = candidate_a.hash(); - introduce_candidate(&mut virtual_overseer, candidate_a.clone(), pvd_a).await; - second_candidate(&mut virtual_overseer, candidate_a.clone()).await; - back_candidate(&mut virtual_overseer, &candidate_a, candidate_hash_a).await; + // Candidate A + let (candidate_a, pvd_a) = make_candidate( + leaf_a.hash, + leaf_a.number, + 1.into(), + HeadData(vec![1, 2, 3]), + HeadData(vec![1]), + test_state.validation_code_hash, + ); + let candidate_hash_a = candidate_a.hash(); + introduce_candidate(&mut virtual_overseer, candidate_a.clone(), pvd_a).await; + second_candidate(&mut virtual_overseer, candidate_a.clone()).await; + back_candidate(&mut virtual_overseer, &candidate_a, candidate_hash_a).await; + + let (candidate_b, candidate_hash_b) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_a, 2); + let (candidate_c, candidate_hash_c) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_a, 3); + let (_candidate_d, candidate_hash_d) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_b, 4); + let (_candidate_e, candidate_hash_e) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_b, 5); + let (candidate_f, candidate_hash_f) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_b, 6); + let (_candidate_g, candidate_hash_g) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_f, 7); + let (candidate_h, candidate_hash_h) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_c, 8); + let (candidate_i, candidate_hash_i) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_h, 9); + let (_candidate_j, candidate_hash_j) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_i, 10); + + // Should not get any backable candidates for the other para. + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 1, vec![]) + .await; + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 5, vec![]) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 2.into(), + vec![candidate_hash_a], + 1, + vec![], + ) + .await; - macro_rules! make_and_back_candidates { - ($parent:expr, $index:expr) => {{ - let (mut candidate, pvd) = make_candidate( - leaf_a.hash, - leaf_a.number, + // Test various scenarios with various counts. + + // empty required_path + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, 1.into(), - $parent.commitments.head_data.clone(), - HeadData(vec![$index]), - test_state.validation_code_hash, - ); - // Set a field to make this candidate unique. - candidate.descriptor.para_head = Hash::from_low_u64_le($index); - let candidate_hash = candidate.hash(); - introduce_candidate(&mut virtual_overseer, candidate.clone(), pvd).await; - second_candidate(&mut virtual_overseer, candidate.clone()).await; - back_candidate(&mut virtual_overseer, &candidate, candidate_hash).await; - - (candidate, candidate_hash) - }}; - } + vec![], + 1, + vec![(candidate_hash_a, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![], + 4, + vec![ + (candidate_hash_a, leaf_a.hash), + (candidate_hash_b, leaf_a.hash), + (candidate_hash_f, leaf_a.hash), + (candidate_hash_g, leaf_a.hash), + ], + ) + .await; + } + + // required path of 1 + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 1, + vec![(candidate_hash_b, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 2, + vec![(candidate_hash_b, leaf_a.hash), (candidate_hash_d, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 3, + vec![ + (candidate_hash_b, leaf_a.hash), + (candidate_hash_f, leaf_a.hash), + (candidate_hash_g, leaf_a.hash), + ], + ) + .await; - let (candidate_b, candidate_hash_b) = make_and_back_candidates!(&candidate_a, 2); - let (candidate_c, candidate_hash_c) = make_and_back_candidates!(&candidate_a, 3); - let (_candidate_d, candidate_hash_d) = make_and_back_candidates!(&candidate_b, 4); - let (_candidate_e, candidate_hash_e) = make_and_back_candidates!(&candidate_b, 5); - let (candidate_f, candidate_hash_f) = make_and_back_candidates!(&candidate_b, 6); - let (_candidate_g, candidate_hash_g) = make_and_back_candidates!(&candidate_f, 7); - let (candidate_h, candidate_hash_h) = make_and_back_candidates!(&candidate_c, 8); - let (candidate_i, candidate_hash_i) = make_and_back_candidates!(&candidate_h, 9); - let (_candidate_j, candidate_hash_j) = make_and_back_candidates!(&candidate_i, 10); + // If the requested count exceeds the largest chain, return the longest + // chain we can get. + for count in 5..10 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + count, + vec![ + (candidate_hash_c, leaf_a.hash), + (candidate_hash_h, leaf_a.hash), + (candidate_hash_i, leaf_a.hash), + (candidate_hash_j, leaf_a.hash), + ], + ) + .await; + } + } - // Should not get any backable candidates for the other para. - get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 1, vec![]).await; - get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 5, vec![]).await; - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, - 2.into(), - vec![candidate_hash_a], - 1, - vec![], - ) - .await; + // required path of 2 + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_b], + 1, + vec![(candidate_hash_d, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_c], + 1, + vec![(candidate_hash_h, leaf_a.hash)], + ) + .await; + // If the requested count exceeds the largest chain, return the longest + // chain we can get. + for count in 4..10 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_c], + count, + vec![ + (candidate_hash_h, leaf_a.hash), + (candidate_hash_i, leaf_a.hash), + (candidate_hash_j, leaf_a.hash), + ], + ) + .await; + } + } - // Test various scenarios with various counts. + // No more candidates in any chain. + { + let required_paths = vec![ + vec![candidate_hash_a, candidate_hash_b, candidate_hash_e], + vec![ + candidate_hash_a, + candidate_hash_c, + candidate_hash_h, + candidate_hash_i, + candidate_hash_j, + ], + ]; + for path in required_paths { + for count in 1..4 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + path.clone(), + count, + vec![], + ) + .await; + } + } + } - // empty required_path - { + // Should not get anything at the wrong path. get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), - vec![], + vec![candidate_hash_b], 1, - vec![(candidate_hash_a, leaf_a.hash)], + vec![], ) .await; get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), + vec![candidate_hash_b, candidate_hash_a], + 3, vec![], - 4, - vec![ - (candidate_hash_a, leaf_a.hash), - (candidate_hash_b, leaf_a.hash), - (candidate_hash_f, leaf_a.hash), - (candidate_hash_g, leaf_a.hash), - ], ) .await; - } - - // required path of 1 - { get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), - vec![candidate_hash_a], - 1, - vec![(candidate_hash_b, leaf_a.hash)], + vec![candidate_hash_a, candidate_hash_b, candidate_hash_c], + 3, + vec![], ) .await; - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, + + virtual_overseer + }); + + assert_eq!(view.active_leaves.len(), 1); + assert_eq!(view.candidate_storage.len(), 2); + // 10 candidates and 7 parents on para 1. + assert_eq!(view.candidate_storage.get(&1.into()).unwrap().len(), (7, 10)); + assert_eq!(view.candidate_storage.get(&2.into()).unwrap().len(), (0, 0)); + } + + // A tree with multiple roots. + // Parachain 1 looks like this: + // (imaginary root) + // | | + // +----B---+ A + // | | | | + // | | | C + // D E F | + // | H + // G | + // I + // | + // J + { + let test_state = TestState::default(); + let view = test_harness(|mut virtual_overseer| async move { + // Leaf A + let leaf_a = TestLeaf { + number: 100, + hash: Hash::from_low_u64_be(130), + para_data: vec![ + (1.into(), PerParaData::new(97, HeadData(vec![1, 2, 3]))), + (2.into(), PerParaData::new(100, HeadData(vec![2, 3, 4]))), + ], + }; + + // Activate leaves. + activate_leaf(&mut virtual_overseer, &leaf_a, &test_state).await; + + // Candidate B + let (candidate_b, pvd_b) = make_candidate( + leaf_a.hash, + leaf_a.number, 1.into(), - vec![candidate_hash_a], - 2, - vec![(candidate_hash_b, leaf_a.hash), (candidate_hash_d, leaf_a.hash)], - ) - .await; + HeadData(vec![1, 2, 3]), + HeadData(vec![2]), + test_state.validation_code_hash, + ); + let candidate_hash_b = candidate_b.hash(); + introduce_candidate(&mut virtual_overseer, candidate_b.clone(), pvd_b).await; + second_candidate(&mut virtual_overseer, candidate_b.clone()).await; + back_candidate(&mut virtual_overseer, &candidate_b, candidate_hash_b).await; + + // Candidate A + let (candidate_a, pvd_a) = make_candidate( + leaf_a.hash, + leaf_a.number, + 1.into(), + HeadData(vec![1, 2, 3]), + HeadData(vec![1]), + test_state.validation_code_hash, + ); + let candidate_hash_a = candidate_a.hash(); + introduce_candidate(&mut virtual_overseer, candidate_a.clone(), pvd_a).await; + second_candidate(&mut virtual_overseer, candidate_a.clone()).await; + back_candidate(&mut virtual_overseer, &candidate_a, candidate_hash_a).await; + + let (candidate_c, candidate_hash_c) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_a, 3); + let (_candidate_d, candidate_hash_d) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_b, 4); + let (_candidate_e, candidate_hash_e) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_b, 5); + let (candidate_f, candidate_hash_f) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_b, 6); + let (_candidate_g, candidate_hash_g) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_f, 7); + let (candidate_h, candidate_hash_h) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_c, 8); + let (candidate_i, candidate_hash_i) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_h, 9); + let (_candidate_j, candidate_hash_j) = + make_and_back_candidate!(test_state, virtual_overseer, leaf_a, &candidate_i, 10); + + // Should not get any backable candidates for the other para. + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 1, vec![]) + .await; + get_backable_candidates(&mut virtual_overseer, &leaf_a, 2.into(), vec![], 5, vec![]) + .await; get_backable_candidates( &mut virtual_overseer, &leaf_a, - 1.into(), + 2.into(), vec![candidate_hash_a], - 3, - vec![ - (candidate_hash_b, leaf_a.hash), - (candidate_hash_f, leaf_a.hash), - (candidate_hash_g, leaf_a.hash), - ], + 1, + vec![], ) .await; - // If the requested count exceeds the largest chain, return the longest - // chain we can get. - for count in 5..10 { + // Test various scenarios with various counts. + + // empty required_path + { get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), - vec![candidate_hash_a], - count, + vec![], + 1, + vec![(candidate_hash_b, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![], + 2, + vec![(candidate_hash_b, leaf_a.hash), (candidate_hash_d, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![], + 4, vec![ + (candidate_hash_a, leaf_a.hash), (candidate_hash_c, leaf_a.hash), (candidate_hash_h, leaf_a.hash), (candidate_hash_i, leaf_a.hash), - (candidate_hash_j, leaf_a.hash), ], ) .await; } - } - // required path of 2 - { - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, - 1.into(), - vec![candidate_hash_a, candidate_hash_b], - 1, - vec![(candidate_hash_d, leaf_a.hash)], - ) - .await; - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, - 1.into(), - vec![candidate_hash_a, candidate_hash_c], - 1, - vec![(candidate_hash_h, leaf_a.hash)], - ) - .await; - // If the requested count exceeds the largest chain, return the longest - // chain we can get. - for count in 4..10 { + // required path of 1 + { get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), - vec![candidate_hash_a, candidate_hash_c], - count, - vec![ - (candidate_hash_h, leaf_a.hash), - (candidate_hash_i, leaf_a.hash), - (candidate_hash_j, leaf_a.hash), - ], + vec![candidate_hash_a], + 1, + vec![(candidate_hash_c, leaf_a.hash)], ) .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_b], + 1, + vec![(candidate_hash_d, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a], + 2, + vec![(candidate_hash_c, leaf_a.hash), (candidate_hash_h, leaf_a.hash)], + ) + .await; + + // If the requested count exceeds the largest chain, return the longest + // chain we can get. + for count in 2..10 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_b], + count, + vec![(candidate_hash_f, leaf_a.hash), (candidate_hash_g, leaf_a.hash)], + ) + .await; + } } - } - // No more candidates in any chain. - { - let required_paths = vec![ - vec![candidate_hash_a, candidate_hash_b, candidate_hash_e], - vec![ - candidate_hash_a, - candidate_hash_c, - candidate_hash_h, - candidate_hash_i, - candidate_hash_j, - ], - ]; - for path in required_paths { - for count in 1..4 { + // required path of 2 + { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_b, candidate_hash_f], + 1, + vec![(candidate_hash_g, leaf_a.hash)], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_c], + 1, + vec![(candidate_hash_h, leaf_a.hash)], + ) + .await; + // If the requested count exceeds the largest chain, return the longest + // chain we can get. + for count in 4..10 { get_backable_candidates( &mut virtual_overseer, &leaf_a, 1.into(), - path.clone(), + vec![candidate_hash_a, candidate_hash_c], count, - vec![], + vec![ + (candidate_hash_h, leaf_a.hash), + (candidate_hash_i, leaf_a.hash), + (candidate_hash_j, leaf_a.hash), + ], ) .await; } } - } - // Should not get anything at the wrong path. - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, - 1.into(), - vec![candidate_hash_b], - 1, - vec![], - ) - .await; - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, - 1.into(), - vec![candidate_hash_b, candidate_hash_a], - 3, - vec![], - ) - .await; - get_backable_candidates( - &mut virtual_overseer, - &leaf_a, - 1.into(), - vec![candidate_hash_a, candidate_hash_b, candidate_hash_c], - 3, - vec![], - ) - .await; + // No more candidates in any chain. + { + let required_paths = vec![ + vec![candidate_hash_b, candidate_hash_f, candidate_hash_g], + vec![candidate_hash_b, candidate_hash_e], + vec![candidate_hash_b, candidate_hash_d], + vec![ + candidate_hash_a, + candidate_hash_c, + candidate_hash_h, + candidate_hash_i, + candidate_hash_j, + ], + ]; + for path in required_paths { + for count in 1..4 { + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + path.clone(), + count, + vec![], + ) + .await; + } + } + } - virtual_overseer - }); + // Should not get anything at the wrong path. + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_d], + 1, + vec![], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_b, candidate_hash_a], + 3, + vec![], + ) + .await; + get_backable_candidates( + &mut virtual_overseer, + &leaf_a, + 1.into(), + vec![candidate_hash_a, candidate_hash_c, candidate_hash_d], + 3, + vec![], + ) + .await; - assert_eq!(view.active_leaves.len(), 1); - assert_eq!(view.candidate_storage.len(), 2); - // 10 candidates and 7 parents on para 1. - assert_eq!(view.candidate_storage.get(&1.into()).unwrap().len(), (7, 10)); - assert_eq!(view.candidate_storage.get(&2.into()).unwrap().len(), (0, 0)); -} + virtual_overseer + }); -#[test] -fn check_backable_query_cycle() { - // TODO + assert_eq!(view.active_leaves.len(), 1); + assert_eq!(view.candidate_storage.len(), 2); + // 10 candidates and 7 parents on para 1. + assert_eq!(view.candidate_storage.get(&1.into()).unwrap().len(), (7, 10)); + assert_eq!(view.candidate_storage.get(&2.into()).unwrap().len(), (0, 0)); + } } // Test depth query. From cfada9a7a608ef6a274d88fc23082908a2fa5027 Mon Sep 17 00:00:00 2001 From: alindima Date: Mon, 5 Feb 2024 11:08:16 +0200 Subject: [PATCH 5/5] fix name --- polkadot/node/core/prospective-parachains/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/node/core/prospective-parachains/src/lib.rs b/polkadot/node/core/prospective-parachains/src/lib.rs index f513d1a43ddf..b8cffd69aadf 100644 --- a/polkadot/node/core/prospective-parachains/src/lib.rs +++ b/polkadot/node/core/prospective-parachains/src/lib.rs @@ -152,7 +152,7 @@ async fn run_iteration( count, required_path, tx, - ) => answer_get_backable_candidate( + ) => answer_get_backable_candidates( &view, relay_parent, para, @@ -560,7 +560,7 @@ async fn handle_candidate_backed( Ok(()) } -fn answer_get_backable_candidate( +fn answer_get_backable_candidates( view: &View, relay_parent: Hash, para: ParaId,