diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index e1a5e0efc258..c981e8d43c3b 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -446,7 +446,7 @@ impl Pallet { // before of the block where we include a candidate (i.e. this code path). let now = >::block_number(); let relay_parent_number = now - One::one(); - let check_cx = CandidateCheckContext::::new(now, relay_parent_number); + let check_ctx = CandidateCheckContext::::new(now, relay_parent_number); // Collect candidate receipts with backers. let mut candidate_receipt_with_backing_validator_indices = @@ -481,54 +481,14 @@ impl Pallet { // // 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::::CandidateNotInParentContext, - ); - ensure!( - backed_candidate.descriptor().check_collator_signature().is_ok(), - Error::::NotCollatorSigned, - ); - - let validation_code_hash = - >::validation_code_hash_at(para_id, now, None) - // A candidate for a parachain without current validation code is not scheduled. - .ok_or_else(|| Error::::UnscheduledCandidate)?; - ensure!( - backed_candidate.descriptor().validation_code_hash == validation_code_hash, - Error::::InvalidValidationCodeHash, - ); - - ensure!( - backed_candidate.descriptor().para_head == - backed_candidate.candidate.commitments.head_data.hash(), - Error::::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::())?; - }; - for (i, assignment) in scheduled[skip..].iter().enumerate() { check_assignment_in_order(assignment)?; @@ -682,7 +642,7 @@ impl Pallet { availability_votes, relay_parent_number, backers: backers.to_bitvec(), - backed_in_number: check_cx.now, + backed_in_number: check_ctx.now, backing_group: group, }, ); @@ -704,9 +664,9 @@ impl Pallet { // `relay_parent_number` is equal to `now`. let now = >::block_number(); let relay_parent_number = now; - let check_cx = CandidateCheckContext::::new(now, relay_parent_number); + let checker_ctx = CandidateCheckContext::::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, @@ -941,17 +901,78 @@ impl AcceptanceCheckErr { } /// A collection of data required for checking a candidate. -struct CandidateCheckContext { +pub(crate) struct CandidateCheckContext { config: configuration::HostConfiguration, now: T::BlockNumber, relay_parent_number: T::BlockNumber, } impl CandidateCheckContext { - 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: >::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: ::Hash, + candidate_idx: usize, + backed_candidate: &BackedCandidate<::Hash>, + ) -> Result<(), Error> { + 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::::CandidateNotInParentContext, + ); + ensure!( + backed_candidate.descriptor().check_collator_signature().is_ok(), + Error::::NotCollatorSigned, + ); + + let validation_code_hash = >::validation_code_hash_at(para_id, now, None) + // A candidate for a parachain without current validation code is not scheduled. + .ok_or_else(|| Error::::UnscheduledCandidate)?; + ensure!( + backed_candidate.descriptor().validation_code_hash == validation_code_hash, + Error::::InvalidValidationCodeHash, + ); + + ensure!( + backed_candidate.descriptor().para_head == + backed_candidate.candidate.commitments.head_data.hash(), + Error::::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::())?; + }; + Ok(()) + } + /// Check the given outputs after candidate validation on whether it passes the acceptance /// criteria. fn check_validation_outputs( diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index a9f63df5f7c5..c72fef1811d4 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -23,7 +23,9 @@ use crate::{ disputes::DisputesHandler, - inclusion, initializer, + inclusion, + inclusion::CandidateCheckContext, + initializer, scheduler::{self, CoreAssignment, FreedReason}, shared, ump, }; @@ -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; @@ -351,6 +354,8 @@ pub mod pallet { Error::::InvalidParentHeader, ); + let now = >::block_number(); + let mut candidate_weight = backed_candidates_weight::(&backed_candidates); let mut bitfields_weight = signed_bitfields_weight::(signed_bitfields.len()); let disputes_weight = dispute_statements_weight::(&disputes); @@ -454,7 +459,6 @@ pub mod pallet { ); // Inform the disputes module of all included candidates. - let now = >::block_number(); for (_, candidate_hash) in &freed_concluded { T::DisputesHandler::note_included(current_session, *candidate_hash, now); } @@ -468,8 +472,14 @@ pub mod pallet { let backed_candidates = sanitize_backed_candidates::( parent_hash, backed_candidates, - move |candidate_hash: CandidateHash| -> bool { - ::DisputesHandler::concluded_invalid(current_session, candidate_hash) + move |_candidate_index: usize, + backed_candidate: &BackedCandidate| + -> bool { + ::DisputesHandler::concluded_invalid( + current_session, + backed_candidate.hash(), + ) + // `fn process_candidates` does the verification checks }, &scheduled[..], ); @@ -548,7 +558,7 @@ impl Pallet { 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 @@ -627,7 +637,8 @@ impl Pallet { let freed = collect_all_freed_cores::(freed_concluded.iter().cloned()); >::clear(); - >::schedule(freed, >::block_number()); + let now = >::block_number(); + >::schedule(freed, now); let scheduled = >::scheduled(); @@ -638,14 +649,24 @@ impl Pallet { bitfields, // updated schedule scheduled, + // current block + now, )) }); + let relay_parent_number = now - One::one(); + // we never check duplicate code + let checker_ctx = CandidateCheckContext::::new(now, relay_parent_number); let mut backed_candidates = sanitize_backed_candidates::( parent_hash, backed_candidates, - move |candidate_hash: CandidateHash| -> bool { - concluded_invalid_disputes.contains(&candidate_hash) + move |candidate_idx: usize, + backed_candidate: &BackedCandidate<::Hash>| + -> bool { + concluded_invalid_disputes.contains(&backed_candidate.hash()) || + checker_ctx + .verify_backed_candidate(parent_hash, candidate_idx, backed_candidate) + .is_err() }, &scheduled[..], ); @@ -954,15 +975,21 @@ pub(crate) fn sanitize_bitfields bool>( +fn sanitize_backed_candidates< + T: crate::inclusion::Config, + F: FnMut(usize, &BackedCandidate) -> bool, +>( relay_parent: T::Hash, mut backed_candidates: Vec>, - candidate_has_concluded_invalid_dispute: F, + mut candidate_has_concluded_invalid_dispute_or_is_invalid: F, scheduled: &[CoreAssignment], ) -> Vec> { // 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.