Skip to content

Commit

Permalink
runtime: make the candidate relay parent progression check more strict (
Browse files Browse the repository at this point in the history
#5113)

Previously, we were checking if the relay parent of a new candidate does
not move backwards from the latest included on-chain candidate. This was
fine prior to elastic scaling. We now need to also check that the relay
parent progresses from the latest pending availability candidate, as
well as check the progression within the candidate chain in the inherent
data.

Prospective-parachains is already doing this check but we should also
add it in the runtime
  • Loading branch information
alindima committed Jul 26, 2024
1 parent bc2680e commit 73015f6
Show file tree
Hide file tree
Showing 4 changed files with 664 additions and 52 deletions.
16 changes: 14 additions & 2 deletions polkadot/runtime/parachains/src/inclusion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,8 @@ impl<T: Config> Pallet<T> {
for (candidate, core) in para_candidates.iter() {
let candidate_hash = candidate.candidate().hash();

// The previous context is None, as it's already checked during candidate
// sanitization.
let check_ctx = CandidateCheckContext::<T>::new(None);
let relay_parent_number = check_ctx.verify_backed_candidate(
&allowed_relay_parents,
Expand Down Expand Up @@ -719,7 +721,7 @@ impl<T: Config> Pallet<T> {
})
}

// Get the latest backed output head data of this para.
// Get the latest backed output head data of this para (including pending availability).
pub(crate) fn para_latest_head_data(para_id: &ParaId) -> Option<HeadData> {
match PendingAvailability::<T>::get(para_id).and_then(|pending_candidates| {
pending_candidates.back().map(|x| x.commitments.head_data.clone())
Expand All @@ -729,6 +731,16 @@ impl<T: Config> Pallet<T> {
}
}

// Get the relay parent number of the most recent candidate (including pending availability).
pub(crate) fn para_most_recent_context(para_id: &ParaId) -> Option<BlockNumberFor<T>> {
match PendingAvailability::<T>::get(para_id)
.and_then(|pending_candidates| pending_candidates.back().map(|x| x.relay_parent_number))
{
Some(relay_parent_number) => Some(relay_parent_number),
None => paras::MostRecentContext::<T>::get(para_id),
}
}

fn check_backing_votes(
backed_candidate: &BackedCandidate<T::Hash>,
validators: &[ValidatorId],
Expand Down Expand Up @@ -798,7 +810,7 @@ impl<T: Config> Pallet<T> {
relay_parent_number: BlockNumberFor<T>,
validation_outputs: polkadot_primitives::CandidateCommitments,
) -> bool {
let prev_context = paras::MostRecentContext::<T>::get(para_id);
let prev_context = Self::para_most_recent_context(&para_id);
let check_ctx = CandidateCheckContext::<T>::new(prev_context);

if check_ctx
Expand Down
46 changes: 26 additions & 20 deletions polkadot/runtime/parachains/src/paras_inherent/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1243,22 +1243,27 @@ fn filter_unchained_candidates<T: inclusion::Config + paras::Config + inclusion:
candidates: &mut BTreeMap<ParaId, Vec<BackedCandidate<T::Hash>>>,
allowed_relay_parents: &AllowedRelayParentsTracker<T::Hash, BlockNumberFor<T>>,
) {
let mut para_latest_head_data: BTreeMap<ParaId, HeadData> = BTreeMap::new();
let mut para_latest_context: BTreeMap<ParaId, (HeadData, BlockNumberFor<T>)> = BTreeMap::new();
for para_id in candidates.keys() {
let latest_head_data = match inclusion::Pallet::<T>::para_latest_head_data(&para_id) {
None => {
defensive!("Latest included head data for paraid {:?} is None", para_id);
continue
},
Some(latest_head_data) => latest_head_data,
let Some(latest_head_data) = inclusion::Pallet::<T>::para_latest_head_data(&para_id) else {
defensive!("Latest included head data for paraid {:?} is None", para_id);
continue
};
para_latest_head_data.insert(*para_id, latest_head_data);
let Some(latest_relay_parent) = inclusion::Pallet::<T>::para_most_recent_context(&para_id)
else {
defensive!("Latest relay parent for paraid {:?} is None", para_id);
continue
};
para_latest_context.insert(*para_id, (latest_head_data, latest_relay_parent));
}

let mut para_visited_candidates: BTreeMap<ParaId, BTreeSet<CandidateHash>> = BTreeMap::new();

retain_candidates::<T, _, _>(candidates, |para_id, candidate| {
let Some(latest_head_data) = para_latest_head_data.get(&para_id) else { return false };
let Some((latest_head_data, latest_relay_parent)) = para_latest_context.get(&para_id)
else {
return false
};
let candidate_hash = candidate.candidate().hash();

let visited_candidates =
Expand All @@ -1277,15 +1282,23 @@ fn filter_unchained_candidates<T: inclusion::Config + paras::Config + inclusion:
visited_candidates.insert(candidate_hash);
}

let prev_context = paras::MostRecentContext::<T>::get(para_id);
let check_ctx = CandidateCheckContext::<T>::new(prev_context);
let check_ctx = CandidateCheckContext::<T>::new(Some(*latest_relay_parent));

let res = match check_ctx.verify_backed_candidate(
match check_ctx.verify_backed_candidate(
&allowed_relay_parents,
candidate.candidate(),
latest_head_data.clone(),
) {
Ok(_) => true,
Ok(relay_parent_block_number) => {
para_latest_context.insert(
para_id,
(
candidate.candidate().commitments.head_data.clone(),
relay_parent_block_number,
),
);
true
},
Err(err) => {
log::debug!(
target: LOG_TARGET,
Expand All @@ -1296,14 +1309,7 @@ fn filter_unchained_candidates<T: inclusion::Config + paras::Config + inclusion:
);
false
},
};

if res {
para_latest_head_data
.insert(para_id, candidate.candidate().commitments.head_data.clone());
}

res
});
}

Expand Down
Loading

0 comments on commit 73015f6

Please sign in to comment.