Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 0 additions & 14 deletions cumulus/client/consensus/aura/src/collators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,6 @@ pub mod basic;
pub mod lookahead;
pub mod slot_based;

// This is an arbitrary value which is guaranteed to exceed the required depth for 500ms blocks
// built with a relay parent offset of 1. It must be larger than the unincluded segment capacity.
//
// The formula we use to compute the capacity of the unincluded segment in the parachain runtime
// is:
// UNINCLUDED_SEGMENT_CAPACITY = (2 + RELAY_PARENT_OFFSET) * BLOCK_PROCESSING_VELOCITY + 1.
//
// Since we only search for parent blocks which have already been imported,
// we can guarantee that all imported blocks respect the unincluded segment
// rules specified by the parachain's runtime and thus will never be too deep. This is just an extra
// sanity check.
const PARENT_SEARCH_DEPTH: usize = 40;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth bumping this to 256 instead? It might be a good safeguard, not sure what other defaults we have in code tho

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can only go back to the included block and the list of pending blocks can not grow infinite.


// Helper to pre-connect to the backing group we got assigned to and keep the connection
// open until backing group changes or own slot ends.
struct BackingGroupConnectionHelper {
Expand Down Expand Up @@ -286,7 +273,6 @@ where
.await
.unwrap_or(DEFAULT_SCHEDULING_LOOKAHEAD)
.saturating_sub(1) as usize,
max_depth: PARENT_SEARCH_DEPTH,
ignore_alternative_branches: true,
};

Expand Down
40 changes: 11 additions & 29 deletions cumulus/client/consensus/common/src/parent_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ pub struct ParentSearchParams {
/// A limitation on the age of relay parents for parachain blocks that are being
/// considered. This is relative to the `relay_parent` number.
pub ancestry_lookback: usize,
/// How "deep" parents can be relative to the included parachain block at the relay-parent.
/// The included block has depth 0.
pub max_depth: usize,
/// Whether to only ignore "alternative" branches, i.e. branches of the chain
/// which do not contain the block pending availability.
pub ignore_alternative_branches: bool,
Expand Down Expand Up @@ -78,18 +75,17 @@ impl<B: BlockT> std::fmt::Debug for PotentialParent<B> {
/// Perform a recursive search through blocks to find potential
/// parent blocks for a new block.
///
/// This accepts a relay-chain block to be used as an anchor and a maximum search depth,
/// along with some arguments for filtering parachain blocks and performs a recursive search
/// for parachain blocks. The search begins at the last included parachain block and returns
/// a set of [`PotentialParent`]s which could be potential parents of a new block with this
/// This accepts a relay-chain block to be used as an anchor along with some arguments for
/// filtering parachain blocks and performs a recursive search for parachain blocks.
/// The search begins at the last included parachain block and returns a set of
/// [`PotentialParent`]s which could be potential parents of a new block with this
/// relay-parent according to the search parameters.
///
/// A parachain block is a potential parent if it is either the last included parachain block, the
/// pending parachain block (when `max_depth` >= 1), or all of the following hold:
/// pending parachain block, or all of the following hold:
/// * its parent is a potential parent
/// * its relay-parent is within `ancestry_lookback` of the targeted relay-parent.
/// * its relay-parent is within the same session as the targeted relay-parent.
/// * the block number is within `max_depth` blocks of the included block
pub async fn find_potential_parents<B: BlockT>(
params: ParentSearchParams,
backend: &impl Backend<B>,
Expand All @@ -111,10 +107,6 @@ pub async fn find_potential_parents<B: BlockT>(
aligned_with_pending: true,
}];

if params.max_depth == 0 {
return Ok(only_included);
};

// Pending header and hash.
let maybe_pending = {
// Fetch the most recent pending header from the relay chain. We use
Expand Down Expand Up @@ -161,7 +153,7 @@ pub async fn find_potential_parents<B: BlockT>(

// If we want to ignore alternative branches there is no reason to start
// the parent search at the included block. We can add the included block and
// the path to the pending block to the potential parents directly (limited by max_depth).
// the path to the pending block to the potential parents directly.
let (frontier, potential_parents) = match (
&maybe_pending,
params.ignore_alternative_branches,
Expand All @@ -176,10 +168,8 @@ pub async fn find_potential_parents<B: BlockT>(
return Ok(Default::default());
}

// Add all items on the path included -> pending - 1 to the potential parents, but
// not more than `max_depth`.
let num_parents_on_path =
route_to_pending.enacted().len().saturating_sub(1).min(params.max_depth);
// Add all items on the path included -> pending - 1 to the potential parents.
let num_parents_on_path = route_to_pending.enacted().len().saturating_sub(1);
for (num, block) in
route_to_pending.enacted().iter().take(num_parents_on_path).enumerate()
{
Expand Down Expand Up @@ -208,10 +198,6 @@ pub async fn find_potential_parents<B: BlockT>(
_ => (only_included, Default::default()),
};

if potential_parents.len() > params.max_depth {
return Ok(potential_parents);
}

// Build up the ancestry record of the relay chain to compare against.
let rp_ancestry =
build_relay_parent_ancestry(params.ancestry_lookback, params.relay_parent, relay_client)
Expand All @@ -223,7 +209,6 @@ pub async fn find_potential_parents<B: BlockT>(
included_header,
maybe_pending.map(|(_, hash)| hash),
backend,
params.max_depth,
params.ignore_alternative_branches,
rp_ancestry,
potential_parents,
Expand Down Expand Up @@ -323,7 +308,6 @@ pub fn search_child_branches_for_parents<Block: BlockT>(
included_header: Block::Header,
pending_hash: Option<Block::Hash>,
backend: &impl Backend<Block>,
max_depth: usize,
ignore_alternative_branches: bool,
rp_ancestry: Vec<(RelayHash, RelayHash)>,
mut potential_parents: Vec<PotentialParent<Block>>,
Expand Down Expand Up @@ -355,7 +339,7 @@ pub fn search_child_branches_for_parents<Block: BlockT>(
let is_pending = pending_hash.as_ref().map_or(false, |h| &entry.hash == h);
let is_included = included_hash == entry.hash;

// note: even if the pending block or included block have a relay parent
// Note: even if the pending block or included block have a relay parent
// outside of the expected part of the relay chain, they are always allowed
// because they have already been posted on chain.
let is_potential = is_pending || is_included || {
Expand Down Expand Up @@ -385,13 +369,11 @@ pub fn search_child_branches_for_parents<Block: BlockT>(

if is_potential {
potential_parents.push(entry);
}

if !is_potential || child_depth > max_depth {
} else {
continue;
}

// push children onto search frontier.
// Push children onto search frontier.
for child in backend.blockchain().children(hash).ok().into_iter().flatten() {
tracing::trace!(target: PARENT_SEARCH_LOG_TARGET, ?child, child_depth, ?pending_distance, "Looking at child.");

Expand Down
Loading
Loading