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

add additional checks for create_inherent filtering #4348

Closed
sirmergealot wants to merge 2 commits into from
Closed

add additional checks for create_inherent filtering #4348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9540903
minor: move checks into separate fn
drahnr Nov 22, 2021
7877ad1
add additional validity checks
drahnr Nov 22, 2021
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
123 changes: 72 additions & 51 deletions runtime/parachains/src/inclusion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ impl<T: Config> Pallet<T> {
// before of the block where we include a candidate (i.e. this code path).
let now = <frame_system::Pallet<T>>::block_number();
let relay_parent_number = now - One::one();
let check_cx = CandidateCheckContext::<T>::new(now, relay_parent_number);
let check_ctx = CandidateCheckContext::<T>::new(now, relay_parent_number);

// Collect candidate receipts with backers.
let mut candidate_receipt_with_backing_validator_indices =
Expand Down Expand Up @@ -481,54 +481,14 @@ impl<T: Config> Pallet<T> {
//
// In the meantime, we do certain sanity checks on the candidates and on the scheduled
// list.
'a: for (candidate_idx, backed_candidate) in candidates.iter().enumerate() {
'next_backed_candidate: for (candidate_idx, backed_candidate) in
candidates.iter().enumerate()
{
check_ctx.verify_backed_candidate(parent_hash, candidate_idx, backed_candidate)?;

let para_id = backed_candidate.descriptor().para_id;
let mut backers = bitvec::bitvec![BitOrderLsb0, u8; 0; validators.len()];

// we require that the candidate is in the context of the parent block.
ensure!(
backed_candidate.descriptor().relay_parent == parent_hash,
Error::<T>::CandidateNotInParentContext,
);
ensure!(
backed_candidate.descriptor().check_collator_signature().is_ok(),
Error::<T>::NotCollatorSigned,
);

let validation_code_hash =
<paras::Pallet<T>>::validation_code_hash_at(para_id, now, None)
// A candidate for a parachain without current validation code is not scheduled.
.ok_or_else(|| Error::<T>::UnscheduledCandidate)?;
ensure!(
backed_candidate.descriptor().validation_code_hash == validation_code_hash,
Error::<T>::InvalidValidationCodeHash,
);

ensure!(
backed_candidate.descriptor().para_head ==
backed_candidate.candidate.commitments.head_data.hash(),
Error::<T>::ParaHeadMismatch,
);

if let Err(err) = check_cx.check_validation_outputs(
para_id,
&backed_candidate.candidate.commitments.head_data,
&backed_candidate.candidate.commitments.new_validation_code,
backed_candidate.candidate.commitments.processed_downward_messages,
&backed_candidate.candidate.commitments.upward_messages,
T::BlockNumber::from(backed_candidate.candidate.commitments.hrmp_watermark),
&backed_candidate.candidate.commitments.horizontal_messages,
) {
log::debug!(
target: LOG_TARGET,
"Validation outputs checking during inclusion of a candidate {} for parachain `{}` failed: {:?}",
candidate_idx,
u32::from(para_id),
err,
);
Err(err.strip_into_dispatch_err::<T>())?;
};

for (i, assignment) in scheduled[skip..].iter().enumerate() {
check_assignment_in_order(assignment)?;

Expand Down Expand Up @@ -682,7 +642,7 @@ impl<T: Config> Pallet<T> {
availability_votes,
relay_parent_number,
backers: backers.to_bitvec(),
backed_in_number: check_cx.now,
backed_in_number: check_ctx.now,
backing_group: group,
},
);
Expand All @@ -704,9 +664,9 @@ impl<T: Config> Pallet<T> {
// `relay_parent_number` is equal to `now`.
let now = <frame_system::Pallet<T>>::block_number();
let relay_parent_number = now;
let check_cx = CandidateCheckContext::<T>::new(now, relay_parent_number);
let checker_ctx = CandidateCheckContext::<T>::new(now, relay_parent_number);

if let Err(err) = check_cx.check_validation_outputs(
if let Err(err) = checker_ctx.check_validation_outputs(
para_id,
&validation_outputs.head_data,
&validation_outputs.new_validation_code,
Expand Down Expand Up @@ -941,17 +901,78 @@ impl<BlockNumber> AcceptanceCheckErr<BlockNumber> {
}

/// A collection of data required for checking a candidate.
struct CandidateCheckContext<T: Config> {
pub(crate) struct CandidateCheckContext<T: Config> {
config: configuration::HostConfiguration<T::BlockNumber>,
now: T::BlockNumber,
relay_parent_number: T::BlockNumber,
}

impl<T: Config> CandidateCheckContext<T> {
fn new(now: T::BlockNumber, relay_parent_number: T::BlockNumber) -> Self {
pub(crate) fn new(now: T::BlockNumber, relay_parent_number: T::BlockNumber) -> Self {
Self { config: <configuration::Pallet<T>>::config(), now, relay_parent_number }
}

/// Execute verification of the candidate.
///
/// Assures:
/// * correct expecte relay parent reference
/// * collator signature check passes
/// * code hash of commitments matches current code hash
/// * para head in the descriptor and commitments match
pub(crate) fn verify_backed_candidate(
&self,
parent_hash: <T as frame_system::Config>::Hash,
candidate_idx: usize,
backed_candidate: &BackedCandidate<<T as frame_system::Config>::Hash>,
) -> Result<(), Error<T>> {
let para_id = backed_candidate.descriptor().para_id;
let now = self.now;

// we require that the candidate is in the context of the parent block.
ensure!(
backed_candidate.descriptor().relay_parent == parent_hash,
Error::<T>::CandidateNotInParentContext,
);
ensure!(
backed_candidate.descriptor().check_collator_signature().is_ok(),
Error::<T>::NotCollatorSigned,
);

let validation_code_hash = <paras::Pallet<T>>::validation_code_hash_at(para_id, now, None)
// A candidate for a parachain without current validation code is not scheduled.
.ok_or_else(|| Error::<T>::UnscheduledCandidate)?;
ensure!(
backed_candidate.descriptor().validation_code_hash == validation_code_hash,
Error::<T>::InvalidValidationCodeHash,
);

ensure!(
backed_candidate.descriptor().para_head ==
backed_candidate.candidate.commitments.head_data.hash(),
Error::<T>::ParaHeadMismatch,
);

if let Err(err) = self.check_validation_outputs(
para_id,
&backed_candidate.candidate.commitments.head_data,
&backed_candidate.candidate.commitments.new_validation_code,
backed_candidate.candidate.commitments.processed_downward_messages,
&backed_candidate.candidate.commitments.upward_messages,
T::BlockNumber::from(backed_candidate.candidate.commitments.hrmp_watermark),
&backed_candidate.candidate.commitments.horizontal_messages,
) {
log::debug!(
target: LOG_TARGET,
"Validation outputs checking during inclusion of a candidate {} for parachain `{}` failed: {:?}",
candidate_idx,
u32::from(para_id),
err,
);
Err(err.strip_into_dispatch_err::<T>())?;
};
Ok(())
}

/// Check the given outputs after candidate validation on whether it passes the acceptance
/// criteria.
fn check_validation_outputs(
Expand Down
53 changes: 40 additions & 13 deletions runtime/parachains/src/paras_inherent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@

use crate::{
disputes::DisputesHandler,
inclusion, initializer,
inclusion,
inclusion::CandidateCheckContext,
initializer,
scheduler::{self, CoreAssignment, FreedReason},
shared, ump,
};
Expand All @@ -44,13 +46,14 @@ use primitives::v1::{
};
use rand::{Rng, SeedableRng};
use scale_info::TypeInfo;
use sp_runtime::traits::Header as HeaderT;
use sp_runtime::traits::{Header as HeaderT, One};
use sp_std::{
cmp::Ordering,
collections::{btree_map::BTreeMap, btree_set::BTreeSet},
prelude::*,
vec::Vec,
};

#[cfg(feature = "runtime-benchmarks")]
mod benchmarking;

Expand Down Expand Up @@ -351,6 +354,8 @@ pub mod pallet {
Error::<T>::InvalidParentHeader,
);

let now = <frame_system::Pallet<T>>::block_number();

let mut candidate_weight = backed_candidates_weight::<T>(&backed_candidates);
let mut bitfields_weight = signed_bitfields_weight::<T>(signed_bitfields.len());
let disputes_weight = dispute_statements_weight::<T>(&disputes);
Expand Down Expand Up @@ -454,7 +459,6 @@ pub mod pallet {
);

// Inform the disputes module of all included candidates.
let now = <frame_system::Pallet<T>>::block_number();
for (_, candidate_hash) in &freed_concluded {
T::DisputesHandler::note_included(current_session, *candidate_hash, now);
}
Expand All @@ -468,8 +472,14 @@ pub mod pallet {
let backed_candidates = sanitize_backed_candidates::<T, _>(
parent_hash,
backed_candidates,
move |candidate_hash: CandidateHash| -> bool {
<T>::DisputesHandler::concluded_invalid(current_session, candidate_hash)
move |_candidate_index: usize,
backed_candidate: &BackedCandidate<T::Hash>|
-> bool {
<T>::DisputesHandler::concluded_invalid(
current_session,
backed_candidate.hash(),
)
// `fn process_candidates` does the verification checks
},
&scheduled[..],
);
Expand Down Expand Up @@ -548,7 +558,7 @@ impl<T: Config> Pallet<T> {

T::DisputesHandler::filter_multi_dispute_data(&mut disputes);

let (concluded_invalid_disputes, mut bitfields, scheduled) =
let (concluded_invalid_disputes, mut bitfields, scheduled, now) =
frame_support::storage::with_transaction(|| {
// we don't care about fresh or not disputes
// this writes them to storage, so let's query it via those means
Expand Down Expand Up @@ -627,7 +637,8 @@ impl<T: Config> Pallet<T> {
let freed = collect_all_freed_cores::<T, _>(freed_concluded.iter().cloned());

<scheduler::Pallet<T>>::clear();
<scheduler::Pallet<T>>::schedule(freed, <frame_system::Pallet<T>>::block_number());
let now = <frame_system::Pallet<T>>::block_number();
<scheduler::Pallet<T>>::schedule(freed, now);

let scheduled = <scheduler::Pallet<T>>::scheduled();

Expand All @@ -638,14 +649,24 @@ impl<T: Config> Pallet<T> {
bitfields,
// updated schedule
scheduled,
// current block
now,
))
});

let relay_parent_number = now - One::one();
// we never check duplicate code
let checker_ctx = CandidateCheckContext::<T>::new(now, relay_parent_number);
let mut backed_candidates = sanitize_backed_candidates::<T, _>(
parent_hash,
backed_candidates,
move |candidate_hash: CandidateHash| -> bool {
concluded_invalid_disputes.contains(&candidate_hash)
move |candidate_idx: usize,
backed_candidate: &BackedCandidate<<T as frame_system::Config>::Hash>|
-> bool {
concluded_invalid_disputes.contains(&backed_candidate.hash()) ||
checker_ctx
.verify_backed_candidate(parent_hash, candidate_idx, backed_candidate)
.is_err()
},
&scheduled[..],
);
Expand Down Expand Up @@ -954,15 +975,21 @@ pub(crate) fn sanitize_bitfields<T: crate::inclusion::Config, const CHECK_SIGS:
///
/// `candidate_has_concluded_invalid_dispute` must return `true` if the candidate
/// is disputed, false otherwise
fn sanitize_backed_candidates<T: crate::inclusion::Config, F: Fn(CandidateHash) -> bool>(
fn sanitize_backed_candidates<
T: crate::inclusion::Config,
F: FnMut(usize, &BackedCandidate<T::Hash>) -> bool,
>(
relay_parent: T::Hash,
mut backed_candidates: Vec<BackedCandidate<T::Hash>>,
candidate_has_concluded_invalid_dispute: F,
mut candidate_has_concluded_invalid_dispute_or_is_invalid: F,
scheduled: &[CoreAssignment],
) -> Vec<BackedCandidate<T::Hash>> {
// Remove any candidates that were concluded invalid.
backed_candidates.retain(|backed_candidate| {
!candidate_has_concluded_invalid_dispute(backed_candidate.candidate.hash())
let mut index = 0;
backed_candidates.retain(move |backed_candidate| {
let ret = !candidate_has_concluded_invalid_dispute_or_is_invalid(index, backed_candidate);
index += 1;
ret
});

// Assure the backed candidate's `ParaId`'s core is free.
Expand Down