Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

validator assignment fixes for backing and collator protocol #6158

Merged
merged 5 commits into from
Oct 19, 2022
Merged

validator assignment fixes for backing and collator protocol #6158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
032f3ab
Rename depth->ancestry len in tests
slumber Oct 14, 2022
59419e0
Refactor group assignments
slumber Oct 14, 2022
c21514b
Remove implicit assignments
slumber Oct 14, 2022
9a08be5
backing: consider occupied core assignments
slumber Oct 14, 2022
2023826
Track a single para on validator side
slumber Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions node/core/backing/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ async fn validate_and_make_available(

let pov = match pov {
PoVData::Ready(pov) => pov,
PoVData::FetchFromValidator { from_validator, candidate_hash, pov_hash } => {
PoVData::FetchFromValidator { from_validator, candidate_hash, pov_hash } =>
match request_pov(
&mut sender,
relay_parent,
Expand All @@ -674,8 +674,7 @@ async fn validate_and_make_available(
},
Err(err) => return Err(err),
Ok(pov) => pov,
}
}
},
};

let v = {
Expand Down Expand Up @@ -1077,16 +1076,26 @@ async fn construct_per_relay_parent_state<Context>(
let mut assignment = None;

for (idx, core) in cores.into_iter().enumerate() {
// Ignore prospective assignments on occupied cores for the time being.
if let CoreState::Scheduled(scheduled) = core {
let core_index = CoreIndex(idx as _);
let group_index = group_rotation_info.group_for_core(core_index, n_cores);
if let Some(g) = validator_groups.get(group_index.0 as usize) {
if validator.as_ref().map_or(false, |v| g.contains(&v.index())) {
assignment = Some((scheduled.para_id, scheduled.collator));
}
groups.insert(scheduled.para_id, g.clone());
let core_para_id = match core {
CoreState::Scheduled(scheduled) => scheduled.para_id,
CoreState::Occupied(occupied) =>
if mode.is_enabled() {
// Async backing makes it legal to build on top of
// occupied core.
occupied.candidate_descriptor.para_id
} else {
continue
},
CoreState::Free => continue,
};

let core_index = CoreIndex(idx as _);
let group_index = group_rotation_info.group_for_core(core_index, n_cores);
if let Some(g) = validator_groups.get(group_index.0 as usize) {
if validator.as_ref().map_or(false, |v| g.contains(&v.index())) {
assignment = Some(core_para_id);
}
groups.insert(core_para_id, g.clone());
}
}

Expand All @@ -1098,13 +1107,6 @@ async fn construct_per_relay_parent_state<Context>(
},
};

// TODO [now]: I've removed the `required_collator` more broadly,
// because it's not used in practice and was intended for parathreads.
//
// We should attempt parathreads another way, I think, so it makes sense
// to remove.
let assignment = assignment.map(|(a, _required_collator)| a);

Ok(Some(PerRelayParentState {
prospective_parachains_mode: mode,
parent,
Expand Down
166 changes: 147 additions & 19 deletions node/core/backing/src/tests/prospective_parachains.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Tests for the backing subsystem with enabled prospective parachains.

use polkadot_node_subsystem::{messages::ChainApiMessage, TimeoutExt};
use polkadot_primitives::v2::{BlockNumber, Header};
use polkadot_primitives::v2::{BlockNumber, Header, OccupiedCore};

use super::*;

Expand Down Expand Up @@ -299,7 +299,7 @@ fn seconding_sanity_check_allowed() {
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate is seconded in a parent of the activated `leaf_a`.
const LEAF_A_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_A_DEPTH: BlockNumber = 3;
const LEAF_A_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

let leaf_b_hash = Hash::from_low_u64_be(128);
Expand All @@ -312,19 +312,19 @@ fn seconding_sanity_check_allowed() {
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

const LEAF_B_BLOCK_NUMBER: BlockNumber = LEAF_A_BLOCK_NUMBER + 2;
const LEAF_B_DEPTH: BlockNumber = 4;
const LEAF_B_ANCESTRY_LEN: BlockNumber = 4;

let activated = ActivatedLeaf {
hash: leaf_b_hash,
number: LEAF_B_BLOCK_NUMBER,
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_B_BLOCK_NUMBER - LEAF_B_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_B_BLOCK_NUMBER - LEAF_B_ANCESTRY_LEN)];
let test_leaf_b = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;
Expand Down Expand Up @@ -436,7 +436,7 @@ fn seconding_sanity_check_disallowed() {
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate is seconded in a parent of the activated `leaf_a`.
const LEAF_A_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_A_DEPTH: BlockNumber = 3;
const LEAF_A_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

let leaf_b_hash = Hash::from_low_u64_be(128);
Expand All @@ -449,19 +449,19 @@ fn seconding_sanity_check_disallowed() {
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

const LEAF_B_BLOCK_NUMBER: BlockNumber = LEAF_A_BLOCK_NUMBER + 2;
const LEAF_B_DEPTH: BlockNumber = 4;
const LEAF_B_ANCESTRY_LEN: BlockNumber = 4;

let activated = ActivatedLeaf {
hash: leaf_b_hash,
number: LEAF_B_BLOCK_NUMBER,
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_B_BLOCK_NUMBER - LEAF_B_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_B_BLOCK_NUMBER - LEAF_B_ANCESTRY_LEN)];
let test_leaf_b = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;
Expand Down Expand Up @@ -633,7 +633,7 @@ fn prospective_parachains_reject_candidate() {
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate is seconded in a parent of the activated `leaf_a`.
const LEAF_A_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_A_DEPTH: BlockNumber = 3;
const LEAF_A_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

let leaf_a_hash = Hash::from_low_u64_be(130);
Expand All @@ -644,7 +644,7 @@ fn prospective_parachains_reject_candidate() {
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;
Expand Down Expand Up @@ -798,7 +798,7 @@ fn second_multiple_candidates_per_relay_parent() {
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate `a` is seconded in a parent of the activated `leaf`.
const LEAF_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_DEPTH: BlockNumber = 3;
const LEAF_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

let leaf_hash = Hash::from_low_u64_be(130);
Expand All @@ -810,7 +810,7 @@ fn second_multiple_candidates_per_relay_parent() {
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_BLOCK_NUMBER - LEAF_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_BLOCK_NUMBER - LEAF_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;
Expand Down Expand Up @@ -922,7 +922,7 @@ fn backing_works() {
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate `a` is seconded in a parent of the activated `leaf`.
const LEAF_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_DEPTH: BlockNumber = 3;
const LEAF_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

let leaf_hash = Hash::from_low_u64_be(130);
Expand All @@ -933,7 +933,7 @@ fn backing_works() {
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_BLOCK_NUMBER - LEAF_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_BLOCK_NUMBER - LEAF_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;
Expand Down Expand Up @@ -1081,7 +1081,7 @@ fn concurrent_dependent_candidates() {
// Candidate `a` is seconded in a grandparent of the activated `leaf`,
// candidate `b` -- in parent.
const LEAF_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_DEPTH: BlockNumber = 3;
const LEAF_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

let leaf_hash = Hash::from_low_u64_be(130);
Expand All @@ -1093,7 +1093,7 @@ fn concurrent_dependent_candidates() {
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_BLOCK_NUMBER - LEAF_DEPTH)];
let min_relay_parents = vec![(para_id, LEAF_BLOCK_NUMBER - LEAF_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;
Expand Down Expand Up @@ -1308,7 +1308,7 @@ fn seconding_sanity_check_occupy_same_depth() {
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate `a` is seconded in a parent of the activated `leaf`.
const LEAF_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_DEPTH: BlockNumber = 3;
const LEAF_ANCESTRY_LEN: BlockNumber = 3;

let para_id_a = test_state.chain_ids[0];
let para_id_b = test_state.chain_ids[1];
Expand All @@ -1323,7 +1323,7 @@ fn seconding_sanity_check_occupy_same_depth() {
span: Arc::new(jaeger::Span::Disabled),
};

let min_block_number = LEAF_BLOCK_NUMBER - LEAF_DEPTH;
let min_block_number = LEAF_BLOCK_NUMBER - LEAF_ANCESTRY_LEN;
let min_relay_parents = vec![(para_id_a, min_block_number), (para_id_b, min_block_number)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

Expand Down Expand Up @@ -1433,3 +1433,131 @@ fn seconding_sanity_check_occupy_same_depth() {
virtual_overseer
});
}

// Test that the subsystem doesn't skip occupied cores assignments.
#[test]
fn occupied_core_assignment() {
let mut test_state = TestState::default();
test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
// Candidate is seconded in a parent of the activated `leaf_a`.
const LEAF_A_BLOCK_NUMBER: BlockNumber = 100;
const LEAF_A_ANCESTRY_LEN: BlockNumber = 3;
let para_id = test_state.chain_ids[0];

// Set the core state to occupied.
let mut candidate_descriptor = ::test_helpers::dummy_candidate_descriptor(Hash::zero());
candidate_descriptor.para_id = para_id;
test_state.availability_cores[0] = CoreState::Occupied(OccupiedCore {
group_responsible: Default::default(),
next_up_on_available: None,
occupied_since: 100_u32,
time_out_at: 200_u32,
next_up_on_time_out: None,
availability: Default::default(),
candidate_descriptor,
candidate_hash: Default::default(),
});

let leaf_a_hash = Hash::from_low_u64_be(130);
let leaf_a_parent = get_parent_hash(leaf_a_hash);
let activated = ActivatedLeaf {
hash: leaf_a_hash,
number: LEAF_A_BLOCK_NUMBER,
status: LeafStatus::Fresh,
span: Arc::new(jaeger::Span::Disabled),
};
let min_relay_parents = vec![(para_id, LEAF_A_BLOCK_NUMBER - LEAF_A_ANCESTRY_LEN)];
let test_leaf_a = TestLeaf { activated, min_relay_parents };

activate_leaf(&mut virtual_overseer, test_leaf_a, &test_state, 0).await;

let pov = PoV { block_data: BlockData(vec![42, 43, 44]) };
let pvd = dummy_pvd();
let validation_code = ValidationCode(vec![1, 2, 3]);

let expected_head_data = test_state.head_data.get(&para_id).unwrap();

let pov_hash = pov.hash();
let candidate = TestCandidateBuilder {
para_id,
relay_parent: leaf_a_parent,
pov_hash,
head_data: expected_head_data.clone(),
erasure_root: make_erasure_root(&test_state, pov.clone(), pvd.clone()),
persisted_validation_data_hash: pvd.hash(),
validation_code: validation_code.0.clone(),
..Default::default()
}
.build();

let second = CandidateBackingMessage::Second(
leaf_a_hash,
candidate.to_plain(),
pvd.clone(),
pov.clone(),
);

virtual_overseer.send(FromOrchestra::Communication { msg: second }).await;

assert_validate_seconded_candidate(
&mut virtual_overseer,
leaf_a_parent,
&candidate,
&pov,
&pvd,
&validation_code,
expected_head_data,
false,
)
.await;

// `seconding_sanity_check`
let expected_request = HypotheticalDepthRequest {
candidate_hash: candidate.hash(),
candidate_para: para_id,
parent_head_data_hash: pvd.parent_head.hash(),
candidate_relay_parent: leaf_a_parent,
fragment_tree_relay_parent: leaf_a_hash,
};
assert_hypothetical_depth_requests(
&mut virtual_overseer,
vec![(expected_request, vec![0, 1, 2, 3])],
)
.await;
// Prospective parachains are notified.
assert_matches!(
virtual_overseer.recv().await,
AllMessages::ProspectiveParachains(
ProspectiveParachainsMessage::CandidateSeconded(
candidate_para,
candidate_receipt,
_pvd,
tx,
),
) if candidate_receipt == candidate && candidate_para == para_id && pvd == _pvd => {
// Any non-empty response will do.
tx.send(vec![(leaf_a_hash, vec![0, 1, 2, 3])]).unwrap();
}
);

assert_matches!(
virtual_overseer.recv().await,
AllMessages::StatementDistribution(
StatementDistributionMessage::Share(
parent_hash,
_signed_statement,
)
) if parent_hash == leaf_a_parent => {}
);

assert_matches!(
virtual_overseer.recv().await,
AllMessages::CollatorProtocol(CollatorProtocolMessage::Seconded(hash, statement)) => {
assert_eq!(leaf_a_parent, hash);
assert_matches!(statement.payload(), Statement::Seconded(_));
}
);

virtual_overseer
});
}
Loading