From e4121041837f482f9262f395a15ac7e93dfd01d6 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Fri, 8 Oct 2021 12:21:41 +0200 Subject: [PATCH] compiling --- primitives/src/v1/mod.rs | 2 +- runtime/parachains/src/inclusion.rs | 111 ++++++----- runtime/parachains/src/paras_inherent.rs | 187 +++++++++++------- runtime/parachains/src/runtime_api_impl/v1.rs | 2 +- 4 files changed, 178 insertions(+), 124 deletions(-) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 0b75c7f64833..3fd2d9dcff82 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -538,7 +538,7 @@ impl CandidateCommitments { #[derive(Default, PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] pub struct DisputedBitfield(pub BitVec); -impl From> for AvailabilityBitfield { +impl From> for DisputedBitfield { fn from(inner: BitVec) -> Self { Self(inner) } diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index 388401ff85d7..d5e534ecaf52 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -23,13 +23,18 @@ use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; use frame_support::pallet_prelude::*; use parity_scale_codec::{Decode, Encode}; -use primitives::v1::{AvailabilityBitfield, BackedCandidate, CandidateCommitments, CandidateDescriptor, CandidateHash, CandidateReceipt, CommittedCandidateReceipt, CoreIndex, DisputedBitfield, GroupIndex, Hash, HeadData, Id as ParaId, SignedAvailabilityBitfields, SigningContext, UncheckedSignedAvailabilityBitfields, ValidatorIndex, ValidityAttestation}; +use primitives::v1::{ + AvailabilityBitfield, BackedCandidate, CandidateCommitments, CandidateDescriptor, + CandidateHash, CandidateReceipt, CommittedCandidateReceipt, CoreIndex, GroupIndex, Hash, + HeadData, Id as ParaId, SignedAvailabilityBitfields, SigningContext, ValidatorId, + ValidatorIndex, ValidityAttestation, +}; use scale_info::TypeInfo; use sp_runtime::{ traits::{One, Saturating}, DispatchError, }; -use sp_std::prelude::*; +use sp_std::{collections::btree_set::BTreeSet, prelude::*}; use crate::{configuration, disputes, dmp, hrmp, paras, scheduler::CoreAssignment, shared, ump}; @@ -258,10 +263,8 @@ impl Pallet { expected_bits: usize, checked_bitfields: SignedAvailabilityBitfields, core_lookup: impl Fn(CoreIndex) -> Option, + validators: &[ValidatorId], ) -> Vec<(CoreIndex, CandidateHash)> { - let validators = shared::Pallet::::active_validator_keys(); - let session_index = shared::Pallet::::session_index(); - let mut assigned_paras_record = (0..expected_bits) .map(|bit_index| core_lookup(CoreIndex::from(bit_index as u32))) .map(|opt_para_id| { @@ -269,12 +272,10 @@ impl Pallet { }) .collect::>(); - let now = >::block_number(); for checked_bitfield in checked_bitfields { for (bit_idx, _) in - checked_bitfield.0.iter().enumerate() - .filter(|(_, is_av)| **is_av) + checked_bitfield.payload().0.iter().enumerate().filter(|(_, is_av)| **is_av) { let pending_availability = if let Some((_, pending_availability)) = assigned_paras_record[bit_idx].as_mut() @@ -299,9 +300,9 @@ impl Pallet { } } - let validator_index = signed_bitfield.validator_index(); + let validator_index = checked_bitfield.validator_index(); let record = AvailabilityBitfieldRecord { - bitfield: signed_bitfield.into_payload(), + bitfield: checked_bitfield.into_payload(), submitted_at: now, }; @@ -773,7 +774,7 @@ impl Pallet { /// Cleans up all paras pending availability that are in the given list of disputed candidates. /// /// Returns a vector of cleaned-up core IDs. - pub(crate) fn collect_disputed(disputed: Vec) -> Vec { + pub(crate) fn collect_disputed(disputed: &BTreeSet) -> Vec { let mut cleaned_up_ids = Vec::new(); let mut cleaned_up_cores = Vec::new(); @@ -1310,12 +1311,14 @@ mod tests { &signing_context, )); - assert!(ParaInclusion::process_bitfields( - expected_bits(), - vec![signed.into()], - &core_lookup, - ) - .is_err()); + assert_eq!( + ParaInclusion::process_bitfields( + expected_bits(), + vec![signed.into()], + &core_lookup, + ), + vec![] + ); } // wrong number of bits: other way around. @@ -1329,12 +1332,14 @@ mod tests { &signing_context, )); - assert!(ParaInclusion::process_bitfields( - expected_bits() + 1, - vec![signed.into()], - &core_lookup, - ) - .is_err()); + assert_eq!( + ParaInclusion::process_bitfields( + expected_bits() + 1, + vec![signed.into()], + &core_lookup, + ), + vec![] + ); } // duplicate. @@ -1349,12 +1354,14 @@ mod tests { )) .into(); - assert!(ParaInclusion::process_bitfields( - expected_bits(), - vec![signed.clone(), signed], - &core_lookup, - ) - .is_err()); + assert_eq!( + ParaInclusion::process_bitfields( + expected_bits(), + vec![signed.clone(), signed], + &core_lookup, + ), + vec![] + ); } // out of order. @@ -1378,12 +1385,14 @@ mod tests { )) .into(); - assert!(ParaInclusion::process_bitfields( - expected_bits(), - vec![signed_1, signed_0], - &core_lookup, - ) - .is_err()); + assert_eq!( + ParaInclusion::process_bitfields( + expected_bits(), + vec![signed_1, signed_0], + &core_lookup, + ), + vec![] + ); } // non-pending bit set. @@ -1403,7 +1412,7 @@ mod tests { vec![signed.into()], &core_lookup, ), - Ok(vec![]) + vec![] ); } @@ -1418,12 +1427,14 @@ mod tests { &signing_context, )); - assert!(ParaInclusion::process_bitfields( - expected_bits(), - vec![signed.into()], - &core_lookup, - ) - .is_ok()); + assert!( + ParaInclusion::process_bitfields( + expected_bits(), + vec![signed.into()], + &core_lookup, + ) + .len() == 1 + ); } // bitfield signed with pending bit signed. @@ -1460,12 +1471,14 @@ mod tests { &signing_context, )); - assert!(ParaInclusion::process_bitfields( - expected_bits(), - vec![signed.into()], - &core_lookup, - ) - .is_ok()); + assert!( + ParaInclusion::process_bitfields( + expected_bits(), + vec![signed.into()], + &core_lookup, + ) + .len() == 1 + ); >::remove(chain_a); PendingAvailabilityCommitments::::remove(chain_a); @@ -1508,7 +1521,7 @@ mod tests { vec![signed.into()], &core_lookup, ), - Ok(vec![]), + vec![], ); } }); diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index 9eed296bc910..94a3e8fc4f58 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -22,6 +22,7 @@ //! this module. use crate::{ + configuration::Config, disputes::DisputesHandler, inclusion, scheduler::{self, FreedReason}, @@ -33,9 +34,17 @@ use frame_support::{ pallet_prelude::*, }; use frame_system::pallet_prelude::*; -use primitives::v1::{BackedCandidate, DisputedBitfield, InherentData as ParachainsInherentData, PARACHAINS_INHERENT_IDENTIFIER, ScrapedOnChainVotes, SignedAvailabilityBitfields, UncheckedSignedAvailabilityBitfields}; +use primitives::v1::{ + BackedCandidate, CandidateHash, CoreIndex, DisputedBitfield, + InherentData as ParachainsInherentData, ScrapedOnChainVotes, SessionIndex, + SignedAvailabilityBitfields, SigningContext, UncheckedSignedAvailabilityBitfields, ValidatorId, + PARACHAINS_INHERENT_IDENTIFIER, +}; use sp_runtime::traits::Header as HeaderT; -use sp_std::prelude::*; +use sp_std::{ + collections::{btree_map::BTreeMap, btree_set::BTreeSet}, + prelude::*, +}; pub use pallet::*; @@ -179,7 +188,7 @@ pub mod pallet { // Handle disputes logic. let current_session = >::session_index(); - let disputed_bits = { + let (disputed_bits, concluded_invalid_disputed_candidates) = { let new_current_dispute_sets: Vec<_> = disputes .iter() .filter(|s| s.session == current_session) @@ -193,20 +202,38 @@ pub mod pallet { return Ok(Some(MINIMAL_INCLUSION_INHERENT_WEIGHT).into()) } - let mut freed_disputed = if !new_current_dispute_sets.is_empty() { - let concluded_invalid_disputes: Vec = new_current_dispute_sets - .iter() - .filter(|(session, candidate)| T::DisputesHandler::concluded_invalid(*session, *candidate)) - .map(|(_, candidate)| *candidate) - .collect(); - - >::collect_disputed(concluded_invalid_disputes) - .into_iter() - .map(|core| (core, FreedReason::Concluded)) - .collect() - } else { - Vec::new() - }; + let (mut freed_disputed, concluded_invalid_disputed_candidates) = + if !new_current_dispute_sets.is_empty() { + let concluded_invalid_disputes = new_current_dispute_sets + .iter() + .filter(|(session, candidate)| { + T::DisputesHandler::concluded_invalid(*session, *candidate) + }) + .map(|(_, candidate)| *candidate) + .collect::>(); + + let freed_disputed = + >::collect_disputed(&concluded_invalid_disputes) + .into_iter() + .map(|core| (core, FreedReason::Concluded)) + .collect(); + (freed_disputed, concluded_invalid_disputes) + } else { + (Vec::new(), BTreeSet::new()) + }; + + // create a bit index from the set of core indicies. + let mut bitvec = BitVec::with_capacity(expected_bits); + if expected_bits > 0 { + bitvec.set(expected_bits.saturating_sub(1), false); + for (core_idx, _) in &freed_disputed { + let core_idx = core_idx.0 as usize; + if core_idx < expected_bits { + bitvec.set(core_idx, true); + } + } + } + let disputed_bitfield = DisputedBitfield::from(bitvec); if !freed_disputed.is_empty() { // unstable sort is fine, because core indices are unique @@ -215,23 +242,18 @@ pub mod pallet { >::free_cores(freed_disputed); } - // create a bit index from the set of core indicies. - let mut bitvec = BitVec::with_capacity(expected_bits); - bitvec.unset(expected_bits); - for core_idx in &freed_disputed { - if core_idx < expected_bits { - bitvec.set(core_idx); - } - } - DisputedBitfield::from(bitvec) + (disputed_bitfield, concluded_invalid_disputed_candidates) }; - let checked_bitfields = filter_bitfields( + let validators = shared::Pallet::::active_validator_keys(); + + let checked_bitfields = filter_bitfields::( signed_bitfields, disputed_bits, expected_bits, parent_hash, current_session, + &validators, ); // Process new availability bitfields, yielding any availability cores whose @@ -240,7 +262,8 @@ pub mod pallet { expected_bits, checked_bitfields, >::core_para, - )?; + &validators, + ); // Inform the disputes module of all included candidates. let now = >::block_number(); @@ -257,19 +280,22 @@ pub mod pallet { }; // Schedule paras again, given freed cores, and reasons for freeing. - let mut freed = freed_concluded + let freed = freed_concluded .into_iter() .map(|(c, _hash)| (c, FreedReason::Concluded)) .chain(freed_timeout.into_iter().map(|c| (c, FreedReason::TimedOut))) .collect::>(); - let backed_candidates = filter_backed_candidates(current_session, backed_candidates, &freed)?; + let backed_candidates = filter_backed_candidates::( + parent_hash, + backed_candidates, + concluded_invalid_disputed_candidates, + ); let backed_candidates_len = backed_candidates.len() as Weight; >::clear(); >::schedule(freed, >::block_number()); - // Process backed candidates according to scheduled cores. let parent_storage_root = parent_header.state_root().clone(); let inclusion::ProcessedCandidates::<::Hash> { @@ -306,79 +332,83 @@ pub mod pallet { .into()) } } - } /// Filter bitfields based on freed core indices. /// /// Do sanity checks on the bitfields: -/// 1. no more than one bitfield per validator -/// 2. bitfields are ascending by validator index. -/// 3. each bitfield has exactly `expected_bits` -/// 4. signature is valid -/// 5. remove any disputed core indices +/// +/// 1. no more than one bitfield per validator +/// 2. bitfields are ascending by validator index. +/// 3. each bitfield has exactly `expected_bits` +/// 4. signature is valid +/// 5. remove any disputed core indices /// /// If any of those is not passed, the bitfield is dropped. fn filter_bitfields( - mut unchecked_bitfields: UncheckedSignedAvailabilityBitfields, + unchecked_bitfields: UncheckedSignedAvailabilityBitfields, disputed_bits: DisputedBitfield, expected_bits: usize, parent_hash: T::Hash, session_index: SessionIndex, + validators: &[ValidatorId], ) -> SignedAvailabilityBitfields { let mut bitfields = Vec::with_capacity(unchecked_bitfields.len()); let mut last_index = None; - if expected_bits != disputed_bits.len() { + if disputed_bits.0.len() != expected_bits { return Default::default() } - for (validator_idx, unchecked_bitfield) in unchecked_bitfields { + if validators.len() != expected_bits { + return Default::default() + } - let signing_context = SigningContext { - parent_hash, - session_index, - }; + for unchecked_bitfield in unchecked_bitfields { + let signing_context = SigningContext { parent_hash, session_index }; if unchecked_bitfield.unchecked_payload().0.len() != expected_bits { continue } - if last_index - .map_or(false, |last| last >= unchecked_bitfield.unchecked_validator_index()) { + if last_index.map_or(false, |last| last >= unchecked_bitfield.unchecked_validator_index()) { continue } if (unchecked_bitfield.unchecked_validator_index().0 as usize) >= validators.len() { continue } - let validator_public = - &validators[unchecked_bitfield.unchecked_validator_index().0 as usize]; + let validator_index = unchecked_bitfield.unchecked_validator_index(); - let signed_bitfield = if let Ok(signed_bitfield) = unchecked_bitfield - .try_into_checked(&signing_context, validator_public) { + let validator_public = &validators[validator_index.0 as usize]; + + let signed_bitfield = if let Ok(signed_bitfield) = + unchecked_bitfield.try_into_checked(&signing_context, validator_public) + { signed_bitfield } else { continue }; - last_index = Some(unchecked_bitfield.unchecked_validator_index()); + last_index = Some(validator_index); + + // TODO FIXME do we want to adjust the bitfield or drop it entirely? // remove all 1 bits which map to concluded disputes - signed_bitfield.0.iter_mut() - .enumerate() - .for_each(|(core_idx, bit)| { - *bit = if **bit { - // ignore those that have matching invalid dispute bits - // which always works, since we checked for uniformat bit length - // before - !disputed_bits[core_idx] - } else { - // bit wasn't set in the first place - false - }; - }); + // signed_bitfield.payload.0.iter_mut() + // .enumerate() + // .any(|(core_idx, mut bit)| { + // *bit = if *bit { + // // ignore those that have matching invalid dispute bits + // // which always works, since we checked for uniformat bit length + // // before + // !disputed_bits.0[core_idx] + // } else { + // // bit wasn't set in the first place + // false + // }; + // }); // TODO at this point the signature won't match, since we modified the payload // TODO double check that the signature is not used at a later point bitfields.push(signed_bitfield) @@ -387,18 +417,19 @@ fn filter_bitfields( } /// Filter candidates based on the concluded disputes. -fn filter_backed_candidates(current_session: SessionIndex, relay_parent: Hash, mut backed_candidates: Vec) -> Vec { - /// Remove any candidates that were concluded invalid. +fn filter_backed_candidates( + relay_parent: T::Hash, + mut backed_candidates: Vec>, + disputed_candidates: BTreeSet, +) -> Vec> { + // Remove any candidates that were concluded invalid. backed_candidates.retain(|backed_candidate| { - !T::DisputesHandler::concluded_invalid( - current_session, - backed_candidate.candidate.hash(), - ) + let candidate_hash = backed_candidate.candidate.hash(); + !disputed_candidates.contains(&candidate_hash) }); // Remove candidates referencing a different relay parent. - backed_candidates.retain(|backed_candidate| { - backed_candidate.descriptor().relay_parent == relay_parent - }); + backed_candidates + .retain(|backed_candidate| backed_candidate.descriptor().relay_parent == relay_parent); // // Limit weight, to avoid overweight block. limit_backed_candidates::(backed_candidates) @@ -513,6 +544,16 @@ mod tests { } } + mod filter_bitfields { + use super::*; + // TODO + } + + mod filter_candidates { + use super::*; + // TODO + } + mod paras_inherent_weight { use super::*; diff --git a/runtime/parachains/src/runtime_api_impl/v1.rs b/runtime/parachains/src/runtime_api_impl/v1.rs index 5909ea55290b..1607dcd22528 100644 --- a/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/runtime/parachains/src/runtime_api_impl/v1.rs @@ -332,6 +332,6 @@ pub fn validation_code_by_hash( } /// Disputes imported via means of on-chain imports. -pub fn on_chain_votes() -> Option> { +pub fn on_chain_votes() -> Option> { >::on_chain_votes() }