Skip to content

Commit

Permalink
collation-generation: resolve mismatch between descriptor and commitm…
Browse files Browse the repository at this point in the history
…ents core index (#7104)

## Issue
[[#7107] Core Index Mismatch in Commitments and
Descriptor](#7107)

## Description
This PR resolves a bug where normal (non-malus) undying collators failed
to generate and submit collations, resulting in the following error:

`ERROR tokio-runtime-worker parachain::collation-generation: Failed to
construct and distribute collation: V2 core index check failed: The core
index in commitments doesn't match the one in descriptor.`

More details about the issue and reproduction steps are described in the
[related issue](#7107).

## Summary of Fix
- When core selectors are provided in the UMP signals, core indexes will
be chosen using them;
- The fix ensures that functionality remains unchanged for parachains
not using UMP signals;
- Added checks to stop processing if the same core is selected
repeatedly.

## TODO
- [X] Implement the fix;
- [x] Add tests;
- [x] Add PRdoc.
  • Loading branch information
sw10pa authored Jan 22, 2025
1 parent 1bdb817 commit 4eb9228
Show file tree
Hide file tree
Showing 3 changed files with 299 additions and 28 deletions.
88 changes: 78 additions & 10 deletions polkadot/node/collation-generation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ use polkadot_primitives::{
node_features::FeatureIndex,
vstaging::{
transpose_claim_queue, CandidateDescriptorV2, CandidateReceiptV2 as CandidateReceipt,
CommittedCandidateReceiptV2, TransposedClaimQueue,
ClaimQueueOffset, CommittedCandidateReceiptV2, TransposedClaimQueue,
},
CandidateCommitments, CandidateDescriptor, CollatorPair, CoreIndex, Hash, Id as ParaId,
NodeFeatures, OccupiedCoreAssumption, PersistedValidationData, SessionIndex,
ValidationCodeHash,
};
use schnellru::{ByLength, LruMap};
use sp_core::crypto::Pair;
use std::sync::Arc;
use std::{collections::HashSet, sync::Arc};

mod error;

Expand Down Expand Up @@ -276,13 +276,15 @@ impl CollationGenerationSubsystem {
let claim_queue =
ClaimQueueSnapshot::from(request_claim_queue(relay_parent, ctx.sender()).await.await??);

let cores_to_build_on = claim_queue
.iter_claims_at_depth(0)
.filter_map(|(core_idx, para_id)| (para_id == config.para_id).then_some(core_idx))
let assigned_cores = claim_queue
.iter_all_claims()
.filter_map(|(core_idx, para_ids)| {
para_ids.iter().any(|&para_id| para_id == config.para_id).then_some(*core_idx)
})
.collect::<Vec<_>>();

// Nothing to do if no core assigned to us.
if cores_to_build_on.is_empty() {
// Nothing to do if no core is assigned to us at any depth.
if assigned_cores.is_empty() {
return Ok(())
}

Expand Down Expand Up @@ -342,9 +344,13 @@ impl CollationGenerationSubsystem {
ctx.spawn(
"chained-collation-builder",
Box::pin(async move {
let transposed_claim_queue = transpose_claim_queue(claim_queue.0);
let transposed_claim_queue = transpose_claim_queue(claim_queue.0.clone());

for core_index in cores_to_build_on {
// Track used core indexes not to submit collations on the same core.
let mut used_cores = HashSet::new();

for i in 0..assigned_cores.len() {
// Get the collation.
let collator_fn = match task_config.collator.as_ref() {
Some(x) => x,
None => return,
Expand All @@ -363,6 +369,68 @@ impl CollationGenerationSubsystem {
},
};

// Use the core_selector method from CandidateCommitments to extract
// CoreSelector and ClaimQueueOffset.
let mut commitments = CandidateCommitments::default();
commitments.upward_messages = collation.upward_messages.clone();

let (cs_index, cq_offset) = match commitments.core_selector() {
// Use the CoreSelector's index if provided.
Ok(Some((sel, off))) => (sel.0 as usize, off),
// Fallback to the sequential index if no CoreSelector is provided.
Ok(None) => (i, ClaimQueueOffset(0)),
Err(err) => {
gum::debug!(
target: LOG_TARGET,
?para_id,
"error processing UMP signals: {}",
err
);
return
},
};

// Identify the cores to build collations on using the given claim queue offset.
let cores_to_build_on = claim_queue
.iter_claims_at_depth(cq_offset.0 as usize)
.filter_map(|(core_idx, para_id)| {
(para_id == task_config.para_id).then_some(core_idx)
})
.collect::<Vec<_>>();

if cores_to_build_on.is_empty() {
gum::debug!(
target: LOG_TARGET,
?para_id,
"no core is assigned to para at depth {}",
cq_offset.0,
);
return
}

let descriptor_core_index =
cores_to_build_on[cs_index % cores_to_build_on.len()];

// Ensure the core index has not been used before.
if used_cores.contains(&descriptor_core_index.0) {
gum::warn!(
target: LOG_TARGET,
?para_id,
"parachain repeatedly selected the same core index: {}",
descriptor_core_index.0,
);
return
}

used_cores.insert(descriptor_core_index.0);
gum::trace!(
target: LOG_TARGET,
?para_id,
"selected core index: {}",
descriptor_core_index.0,
);

// Distribute the collation.
let parent_head = collation.head_data.clone();
if let Err(err) = construct_and_distribute_receipt(
PreparedCollation {
Expand All @@ -372,7 +440,7 @@ impl CollationGenerationSubsystem {
validation_data: validation_data.clone(),
validation_code_hash,
n_validators,
core_index,
core_index: descriptor_core_index,
session_index,
},
task_config.key.clone(),
Expand Down
Loading

0 comments on commit 4eb9228

Please sign in to comment.