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

Commit

Permalink
validator assignment fixes for backing and collator protocol (#6158)
Browse files Browse the repository at this point in the history 
* Rename depth->ancestry len in tests

* Refactor group assignments

* Remove implicit assignments

* backing: consider occupied core assignments

* Track a single para on validator side
  • Loading branch information
@slumber
slumber authored Oct 19, 2022
1 parent 6138103 commit 604cfc4
Show file tree
Hide file tree
Showing 6 changed files with 226 additions and 165 deletions.
40 changes: 21 additions & 19 deletions node/core/backing/src/lib.rs
View file
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
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

0 comments on commit 604cfc4

Please sign in to comment.