diff --git a/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index 65e56881c315..cd4141328f0b 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -62,7 +62,7 @@ fn mock_validation_code() -> ValidationCode { /// /// This is directly from frame-benchmarking. Copy/pasted so we can use it when not compiling with /// "features = runtime-benchmarks". -fn account(name: &'static str, index: u32, seed: u32) -> AccountId { +pub fn account(name: &'static str, index: u32, seed: u32) -> AccountId { let entropy = (name, index, seed).using_encoded(sp_io::hashing::blake2_256); AccountId::decode(&mut TrailingZeroInput::new(&entropy[..])) .expect("infinite input; no invalid input; qed") diff --git a/polkadot/runtime/parachains/src/disputes.rs b/polkadot/runtime/parachains/src/disputes.rs index f86573dadf56..00c9dba24f20 100644 --- a/polkadot/runtime/parachains/src/disputes.rs +++ b/polkadot/runtime/parachains/src/disputes.rs @@ -41,7 +41,7 @@ use sp_runtime::{ #[cfg(test)] #[allow(unused_imports)] -pub(crate) use self::tests::run_to_block; +pub(crate) use self::tests::{make_dispute_concluding_against, run_to_block}; pub mod slashing; #[cfg(test)] @@ -556,7 +556,7 @@ enum VoteImportError { MaliciousBacker, } -#[derive(RuntimeDebug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] enum VoteKind { /// A backing vote that is counted as "for" vote in dispute resolution. Backing, @@ -949,7 +949,10 @@ impl Pallet { // Load session info to access validators let session_info = match session_info::Sessions::::get(set.session) { Some(s) => s, - None => return StatementSetFilter::RemoveAll, + None => { + log::debug!(target: LOG_TARGET, "SessionInfo not found: {}", set.session); + return StatementSetFilter::RemoveAll + }, }; let config = configuration::ActiveConfig::::get(); @@ -960,6 +963,7 @@ impl Pallet { let dispute_state = { if let Some(dispute_state) = Disputes::::get(&set.session, &set.candidate_hash) { if dispute_state.concluded_at.as_ref().map_or(false, |c| c < &oldest_accepted) { + log::debug!(target: LOG_TARGET, "Dispute is ancient: {:?}", set.candidate_hash); return StatementSetFilter::RemoveAll } @@ -987,6 +991,7 @@ impl Pallet { let validator_public = match session_info.validators.get(*validator_index) { None => { filter.remove_index(i); + log::debug!(target: LOG_TARGET, "Invalid validator index in a dispute: {}", validator_index.0); continue }, Some(v) => v, @@ -998,6 +1003,7 @@ impl Pallet { Ok(u) => u, Err(_) => { filter.remove_index(i); + log::debug!(target: LOG_TARGET, "Import of a dispute failed for validator: {}", validator_index.0); continue }, }; @@ -1023,7 +1029,7 @@ impl Pallet { // config. config.approval_voting_params.max_approval_coalesce_count > 1, ) { - log::warn!("Failed to check dispute signature"); + log::warn!(target: LOG_TARGET, "Failed to check dispute signature"); importer.undo(undo); filter.remove_index(i); @@ -1038,6 +1044,7 @@ impl Pallet { if summary.state.validators_for.count_ones() == 0 || summary.state.validators_against.count_ones() == 0 { + log::debug!(target: LOG_TARGET, "Rejected one-sided dispute"); return StatementSetFilter::RemoveAll } @@ -1045,6 +1052,7 @@ impl Pallet { if (summary.state.validators_for.clone() | &summary.state.validators_against).count_ones() <= byzantine_threshold(summary.state.validators_for.len()) { + log::debug!(target: LOG_TARGET, "Rejected unconfirmed dispute"); return StatementSetFilter::RemoveAll } @@ -1216,12 +1224,6 @@ impl Pallet { let revert_to = included_in - One::one(); Included::::insert(&session, &candidate_hash, revert_to); - - if let Some(state) = Disputes::::get(&session, candidate_hash) { - if has_supermajority_against(&state) { - Self::revert_and_freeze(revert_to); - } - } } pub(crate) fn included_state( diff --git a/polkadot/runtime/parachains/src/disputes/tests.rs b/polkadot/runtime/parachains/src/disputes/tests.rs index f505bf4625a6..a4810b1b5f8e 100644 --- a/polkadot/runtime/parachains/src/disputes/tests.rs +++ b/polkadot/runtime/parachains/src/disputes/tests.rs @@ -31,6 +31,7 @@ use frame_support::{ use frame_system::pallet_prelude::BlockNumberFor; use polkadot_primitives::BlockNumber; use sp_core::{crypto::CryptoType, Pair}; +use sp_keyring::Sr25519Keyring; const VOTE_FOR: VoteKind = VoteKind::ExplicitValid; const VOTE_AGAINST: VoteKind = VoteKind::Invalid; @@ -86,6 +87,27 @@ type NewSession<'a> = ( Option>, ); +pub(crate) fn make_dispute_concluding_against( + candidate_hash: CandidateHash, + session: SessionIndex, + validators: &[(ValidatorIndex, Sr25519Keyring)], +) -> DisputeStatementSet { + let mut statements = Vec::new(); + for (j, (i, key)) in validators.iter().enumerate() { + // make the first statement backing + // and the rest against it + let statement = if j == 0 { + DisputeStatement::Valid(ValidDisputeStatementKind::BackingSeconded(candidate_hash.0)) + } else { + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit) + }; + let payload_data = statement.payload_data(candidate_hash, session).unwrap(); + let signature = key.sign(&payload_data); + statements.push((statement, *i, signature.into())); + } + DisputeStatementSet { candidate_hash, session, statements } +} + // Run to specific block, while calling disputes pallet hooks manually, because disputes is not // integrated in initializer yet. pub(crate) fn run_to_block<'a>( @@ -555,71 +577,6 @@ fn test_disputes_with_missing_backing_votes_are_rejected() { }) } -#[test] -fn test_freeze_on_note_included() { - new_test_ext(Default::default()).execute_with(|| { - let v0 = ::Pair::generate().0; - let v1 = ::Pair::generate().0; - - run_to_block(6, |b| { - // a new session at each block - Some(( - true, - b, - vec![(&0, v0.public()), (&1, v1.public())], - Some(vec![(&0, v0.public()), (&1, v1.public())]), - )) - }); - - let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); - let inclusion_parent = sp_core::H256::repeat_byte(0xff); - let session = 3; - - // v0 votes for 3 - let stmts = vec![DisputeStatementSet { - candidate_hash, - session: 3, - statements: vec![ - ( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(0), - v0.sign( - &ExplicitDisputeStatement { valid: false, candidate_hash, session: 3 } - .signing_payload(), - ), - ), - ( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(1), - v1.sign( - &ExplicitDisputeStatement { valid: false, candidate_hash, session: 3 } - .signing_payload(), - ), - ), - ( - DisputeStatement::Valid(ValidDisputeStatementKind::BackingValid( - inclusion_parent, - )), - ValidatorIndex(1), - v0.sign(&CompactStatement::Valid(candidate_hash).signing_payload( - &SigningContext { session_index: session, parent_hash: inclusion_parent }, - )), - ), - ], - }]; - assert!(Pallet::::process_checked_multi_dispute_data( - &stmts - .into_iter() - .map(CheckedDisputeStatementSet::unchecked_from_unchecked) - .collect() - ) - .is_ok()); - - Pallet::::note_included(3, candidate_hash, 3); - assert_eq!(Frozen::::get(), Some(2)); - }); -} - #[test] fn test_freeze_provided_against_supermajority_for_included() { new_test_ext(Default::default()).execute_with(|| { diff --git a/polkadot/runtime/parachains/src/inclusion/mod.rs b/polkadot/runtime/parachains/src/inclusion/mod.rs index b1d22996fd12..09b28ad09735 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -1050,14 +1050,11 @@ impl Pallet { /// /// Returns a vector of cleaned-up core IDs, along with the evicted candidate hashes. pub(crate) fn free_disputed( - disputed: &BTreeSet, + disputed: impl Fn(CandidateHash) -> bool, ) -> Vec<(CoreIndex, CandidateHash)> { - Self::free_failed_cores( - |candidate| disputed.contains(&candidate.hash), - Some(disputed.len()), - ) - .map(|candidate| (candidate.core, candidate.hash)) - .collect() + Self::free_failed_cores(|candidate| disputed(candidate.hash), None) + .map(|candidate| (candidate.core, candidate.hash)) + .collect() } // Clean up cores whose candidates are deemed as failed by the predicate. `pred` returns true if diff --git a/polkadot/runtime/parachains/src/inclusion/tests.rs b/polkadot/runtime/parachains/src/inclusion/tests.rs index 95fd66bf8e4f..d21c4a38a3c4 100644 --- a/polkadot/runtime/parachains/src/inclusion/tests.rs +++ b/polkadot/runtime/parachains/src/inclusion/tests.rs @@ -526,12 +526,10 @@ fn free_disputed() { let mut config = genesis_config(paras); config.configuration.config.scheduler_params.group_rotation_frequency = 3; new_test_ext(config).execute_with(|| { - let disputed_cores = ParaInclusion::free_disputed(&BTreeSet::new()); + let disputed_cores = ParaInclusion::free_disputed(|_| false); assert!(disputed_cores.is_empty()); - let disputed_cores = ParaInclusion::free_disputed( - &[CandidateHash::default()].into_iter().collect::>(), - ); + let disputed_cores = ParaInclusion::free_disputed(|h| h == CandidateHash::default()); assert!(disputed_cores.is_empty()); let make_candidate = |core_index: u32| { @@ -610,7 +608,7 @@ fn free_disputed() { ] .into_iter() .collect::>(); - let disputed_cores = ParaInclusion::free_disputed(&disputed_candidates); + let disputed_cores = ParaInclusion::free_disputed(|h| disputed_candidates.contains(&h)); assert_eq!( disputed_cores.into_iter().map(|(core, _)| core).collect::>(), diff --git a/polkadot/runtime/parachains/src/paras_inherent/mod.rs b/polkadot/runtime/parachains/src/paras_inherent/mod.rs index bd8d08a842c3..b1f9cd24e675 100644 --- a/polkadot/runtime/parachains/src/paras_inherent/mod.rs +++ b/polkadot/runtime/parachains/src/paras_inherent/mod.rs @@ -510,9 +510,16 @@ impl Pallet { // Get the cores freed as a result of concluded invalid candidates. let (freed_disputed, concluded_invalid_hashes): (Vec, BTreeSet) = - inclusion::Pallet::::free_disputed(¤t_concluded_invalid_disputes) - .into_iter() - .unzip(); + inclusion::Pallet::::free_disputed(|h| { + current_concluded_invalid_disputes.contains(&h) + }) + .into_iter() + .unzip(); + // Also include cores freed as a result of concluded invalid candidates. + let current_concluded_invalid_disputes = |hash: &CandidateHash| { + concluded_invalid_hashes.contains(hash) || + ::DisputesHandler::concluded_invalid(current_session, *hash) + }; // Create a bit index from the set of core indices where each index corresponds to // a core index that was freed due to a dispute. @@ -607,7 +614,7 @@ impl Pallet { let backed_candidates_with_core = sanitize_backed_candidates::( backed_candidates, &allowed_relay_parents, - concluded_invalid_hashes, + current_concluded_invalid_disputes, eligible, core_index_enabled, ); @@ -980,7 +987,7 @@ pub(crate) fn sanitize_bitfields( fn sanitize_backed_candidates( backed_candidates: Vec>, allowed_relay_parents: &AllowedRelayParentsTracker>, - concluded_invalid_with_descendants: BTreeSet, + concluded_invalid_with_descendants: impl Fn(&CandidateHash) -> bool, scheduled: BTreeMap>, core_index_enabled: bool, ) -> BTreeMap, CoreIndex)>> { @@ -1001,13 +1008,14 @@ fn sanitize_backed_candidates( // Remove any candidates that were concluded invalid or who are descendants of concluded invalid // candidates (along with their descendants). retain_candidates::(&mut candidates_per_para, |_, candidate| { - let keep = !concluded_invalid_with_descendants.contains(&candidate.candidate().hash()); + let hash = candidate.candidate().hash(); + let keep = !concluded_invalid_with_descendants(&hash); if !keep { log::debug!( target: LOG_TARGET, "Found backed candidate {:?} which was concluded invalid or is a descendant of a concluded invalid candidate, for paraid {:?}.", - candidate.candidate().hash(), + hash, candidate.descriptor().para_id ); } @@ -1104,8 +1112,11 @@ fn limit_and_sanitize_disputes< if max_consumable_weight.all_gte(updated) { // Always apply the weight. Invalid data cost processing time too: weight_acc = updated; + let candidate_hash = dss.candidate_hash; if let Some(checked) = dispute_statement_set_valid(dss) { checked_acc.push(checked); + } else { + log::debug!(target: LOG_TARGET, "Filtered dispute: {:?}", candidate_hash); } } }); diff --git a/polkadot/runtime/parachains/src/paras_inherent/tests.rs b/polkadot/runtime/parachains/src/paras_inherent/tests.rs index e34055bfa9f2..620f4c2fccb4 100644 --- a/polkadot/runtime/parachains/src/paras_inherent/tests.rs +++ b/polkadot/runtime/parachains/src/paras_inherent/tests.rs @@ -121,10 +121,10 @@ mod enter { } } - #[test] // Validate that if we create 2 backed candidates which are assigned to 2 cores that will be // freed via becoming fully available, the backed candidates will not be filtered out in // `create_inherent` and will not cause `enter` to early. + #[test] fn include_backed_candidates() { let config = MockGenesisConfig::default(); assert!(config.configuration.config.scheduler_params.lookahead > 0); @@ -582,8 +582,8 @@ mod enter { }); } - #[test] // Ensure that disputes are filtered out if the session is in the future. + #[test] fn filter_multi_dispute_data() { new_test_ext(MockGenesisConfig::default()).execute_with(|| { // Create the inherent data for this block @@ -653,9 +653,9 @@ mod enter { }); } - #[test] // Ensure that when dispute data establishes an over weight block that we adequately // filter out disputes according to our prioritization rule + #[test] fn limit_dispute_data() { sp_tracing::try_init_simple(); new_test_ext(MockGenesisConfig::default()).execute_with(|| { @@ -721,10 +721,10 @@ mod enter { }); } - #[test] // Ensure that when a block is over weight due to disputes, but there is still sufficient // block weight to include a number of signed bitfields, the inherent data is filtered // as expected + #[test] fn limit_dispute_data_ignore_backed_candidates() { new_test_ext(MockGenesisConfig::default()).execute_with(|| { // Create the inherent data for this block @@ -804,9 +804,9 @@ mod enter { }); } - #[test] // Ensure an overweight block with an excess amount of disputes and bitfields, the bitfields are // filtered to accommodate the block size and no backed candidates are included. + #[test] fn limit_bitfields_some() { new_test_ext(MockGenesisConfig::default()).execute_with(|| { // Create the inherent data for this block @@ -892,8 +892,8 @@ mod enter { }); } - #[test] // Ensure that when a block is over weight due to disputes and bitfields, we filter. + #[test] fn limit_bitfields_overweight() { new_test_ext(MockGenesisConfig::default()).execute_with(|| { // Create the inherent data for this block @@ -1798,6 +1798,8 @@ mod sanitizers { } mod candidates { + use std::collections::HashMap; + use crate::{ mock::{set_disabled_validators, RuntimeOrigin}, scheduler::{common::Assignment, ParasEntry}, @@ -1805,6 +1807,7 @@ mod sanitizers { }; use alloc::collections::vec_deque::VecDeque; use polkadot_primitives::ValidationCode; + use sp_runtime::{Digest, ModuleError}; use super::*; @@ -1823,15 +1826,15 @@ mod sanitizers { // Add the relay parent to `shared` pallet. Otherwise some code (e.g. filtering backing // votes) won't behave correctly + let header = default_header(); + let relay_parent = header.hash(); shared::Pallet::::add_allowed_relay_parent( - default_header().hash(), - Default::default(), + relay_parent, + *header.state_root(), RELAY_PARENT_NUM, - 1, + 10, ); - let header = default_header(); - let relay_parent = header.hash(); let session_index = SessionIndex::from(0_u32); let keystore = LocalKeystore::in_memory(); @@ -1882,14 +1885,14 @@ mod sanitizers { CoreIndex::from(0), VecDeque::from([ParasEntry::new( Assignment::Pool { para_id: 1.into(), core_index: CoreIndex(0) }, - RELAY_PARENT_NUM, + RELAY_PARENT_NUM + 10, )]), ), ( CoreIndex::from(1), VecDeque::from([ParasEntry::new( Assignment::Pool { para_id: 2.into(), core_index: CoreIndex(1) }, - RELAY_PARENT_NUM, + RELAY_PARENT_NUM + 10, )]), ), ])); @@ -3469,6 +3472,10 @@ mod sanitizers { } } + fn no_disputes(_h: &CandidateHash) -> bool { + false + } + #[rstest] #[case(false)] #[case(true)] @@ -3478,13 +3485,14 @@ mod sanitizers { backed_candidates, expected_backed_candidates_with_core, scheduled_paras: scheduled, + .. } = get_test_data_one_core_per_para(core_index_enabled); assert_eq!( sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled ), @@ -3502,13 +3510,14 @@ mod sanitizers { backed_candidates, expected_backed_candidates_with_core, scheduled_paras: scheduled, + .. } = get_test_data_multiple_cores_per_para(core_index_enabled); assert_eq!( sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled ), @@ -3526,13 +3535,14 @@ mod sanitizers { backed_candidates, scheduled_paras: scheduled, expected_backed_candidates_with_core, + .. } = get_test_data_for_order_checks(core_index_enabled); assert_eq!( sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled, ), @@ -3564,7 +3574,7 @@ mod sanitizers { sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled, ), @@ -3604,7 +3614,7 @@ mod sanitizers { let res = sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled, ); @@ -3674,7 +3684,7 @@ mod sanitizers { let res = sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled, ); @@ -3712,7 +3722,7 @@ mod sanitizers { let sanitized_backed_candidates = sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - BTreeSet::new(), + no_disputes, scheduled, core_index_enabled, ); @@ -3748,7 +3758,7 @@ mod sanitizers { > = sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - set, + |h| set.contains(&h), scheduled, core_index_enabled, ); @@ -3785,7 +3795,7 @@ mod sanitizers { > = sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - invalid_set, + |h| invalid_set.contains(&h), scheduled, true, ); @@ -3821,7 +3831,7 @@ mod sanitizers { > = sanitize_backed_candidates::( backed_candidates.clone(), &shared::AllowedRelayParents::::get(), - invalid_set, + |h| invalid_set.contains(&h), scheduled, true, ); @@ -3837,6 +3847,262 @@ mod sanitizers { }); } + #[test] + fn concluded_invalid_in_the_past_are_filtered_out() { + use crate::{ + builder::account, + disputes::{make_dispute_concluding_against, run_to_block}, + mock::AccountId, + }; + use assert_matches::assert_matches; + + new_test_ext(default_config()).execute_with(|| { + const RELAY_PARENT_NUMBER: u32 = 1; + + let mut hc = configuration::ActiveConfig::::get(); + hc.async_backing_params.allowed_ancestry_len = 10; + configuration::Pallet::::force_set_active_config(hc); + + let header = default_header(); + let relay_parent = header.hash(); + + let session = SessionIndex::from(0_u32); + let keystore = LocalKeystore::in_memory(); + let keystore = Arc::new(keystore) as KeystorePtr; + + let validators_keyrings = vec![ + sp_keyring::Sr25519Keyring::Alice, + sp_keyring::Sr25519Keyring::Bob, + sp_keyring::Sr25519Keyring::Charlie, + sp_keyring::Sr25519Keyring::Dave, + sp_keyring::Sr25519Keyring::Eve, + ]; + for keyring in validators_keyrings.iter() { + Keystore::sr25519_generate_new( + &*keystore, + PARACHAIN_KEY_TYPE_ID, + Some(&keyring.to_seed()), + ) + .unwrap(); + } + let validators_to_keyrings: HashMap = + validators_keyrings.iter().map(|v| (v.public().into(), *v)).collect(); + let validators_keys: Vec<(_, ValidatorId)> = validators_keyrings + .iter() + .enumerate() + .map(|(i, v)| { + let acc: AccountId = account("validator", i as u32, i as u32); + (acc, v.public().into()) + }) + .collect(); + initializer::Pallet::::test_trigger_on_new_session( + true, + session, + validators_keys.iter().map(|(a, v)| (a, v.clone())), + None, + ); + let validators_keys = shared::ActiveValidatorKeys::::get(); + let mut validators: Vec<_> = validators_keys + .iter() + .enumerate() + .map(|(i, v)| (ValidatorIndex::from(i as u32), v)) + .collect(); + let groups = scheduler::ValidatorGroups::::get(); + let para_id_to_group: BTreeMap> = groups + .into_iter() + .enumerate() + .map(|(i, g)| (ParaId::from(i as u32 + 1), g)) + .collect(); + let shuffled_indices = shared::ActiveValidatorIndices::::get(); + // shuffle the keyrings in the same way as the validators + let validators_keyrings = + crate::util::take_active_subset(&shuffled_indices, &validators_keyrings); + + // Set the on-chain included head data for paras. + paras::Pallet::::set_current_head(ParaId::from(1), HeadData(vec![1])); + paras::Pallet::::set_current_head(ParaId::from(2), HeadData(vec![2])); + + // Set the current_code_hash + paras::Pallet::::force_set_current_code( + RuntimeOrigin::root(), + ParaId::from(1), + ValidationCode(vec![1]), + ) + .unwrap(); + paras::Pallet::::force_set_current_code( + RuntimeOrigin::root(), + ParaId::from(2), + ValidationCode(vec![2]), + ) + .unwrap(); + // Set the most recent relay parent. + paras::Pallet::::force_set_most_recent_context( + RuntimeOrigin::root(), + ParaId::from(1), + BlockNumberFor::::from(0u32), + ) + .unwrap(); + paras::Pallet::::force_set_most_recent_context( + RuntimeOrigin::root(), + ParaId::from(2), + BlockNumberFor::::from(0u32), + ) + .unwrap(); + + let backed_candidates = (0_usize..2) + .into_iter() + .map(|idx0| { + let idx1 = idx0 + 1; + let para_id = ParaId::from(idx1); + let mut candidate = TestCandidateBuilder { + para_id, + relay_parent, + pov_hash: Hash::repeat_byte(idx1 as u8), + persisted_validation_data_hash: make_persisted_validation_data::( + para_id, + RELAY_PARENT_NUMBER, + Default::default(), + ) + .unwrap() + .hash(), + hrmp_watermark: RELAY_PARENT_NUMBER, + validation_code: ValidationCode(vec![idx1 as u8]), + ..Default::default() + } + .build(); + + collator_sign_candidate(Sr25519Keyring::One, &mut candidate); + + let signing_context = + SigningContext { parent_hash: relay_parent, session_index: session }; + let group = para_id_to_group[¶_id].as_ref(); + let backed = back_candidate( + candidate, + &validators_keyrings, + group, + &keystore, + &signing_context, + BackingKind::Threshold, + None, + ); + backed + }) + .collect::>(); + + let disputes: MultiDisputeStatementSet = backed_candidates + .iter() + .map(|c| { + // make sure the backer is the first in the list + let para_id = c.descriptor().para_id; + let group = para_id_to_group.get(¶_id).unwrap(); + let backer = validators.iter().find(|v| group.contains(&v.0)).unwrap().0; + validators.swap(0, backer.0 as _); + let keyrings: Vec<_> = validators + .iter() + .map(|(i, v)| (*i, validators_to_keyrings[v])) + .collect(); + + make_dispute_concluding_against(c.hash(), session, &keyrings) + }) + .collect(); + + let parent_block_number = RELAY_PARENT_NUMBER; + let parent_header = HeaderFor::::new( + parent_block_number, // `block_number`, + Default::default(), // `extrinsics_root`, + Default::default(), // `storage_root`, + default_header().hash(), // `parent_hash`, + Default::default(), // digest, + ); + + run_to_block(parent_block_number + 1, |_| None); + frame_system::Pallet::::reset_events(); + frame_system::Pallet::::initialize( + &(parent_header.number() + 1), + &parent_header.hash(), + &Digest { logs: Vec::new() }, + ); + + let n_disputes = disputes.len(); + let data = ParachainsInherentData { + disputes, + parent_header: parent_header.clone(), + bitfields: Vec::new(), + backed_candidates: Vec::new(), + }; + let mut inherent_data = InherentData::new(); + inherent_data.put_data(PARACHAINS_INHERENT_IDENTIFIER, &data).unwrap(); + + let header = default_header(); + shared::Pallet::::add_allowed_relay_parent( + header.hash(), + *header.state_root(), + RELAY_PARENT_NUMBER, + 10, + ); + // Update scheduler's claimqueue with the parachains + let ttl = RELAY_PARENT_NUMBER + 10; + scheduler::Pallet::::set_claim_queue(BTreeMap::from([ + ( + CoreIndex::from(0), + VecDeque::from([ParasEntry::new( + Assignment::Pool { para_id: 1.into(), core_index: CoreIndex(0) }, + ttl, + )]), + ), + ( + CoreIndex::from(1), + VecDeque::from([ParasEntry::new( + Assignment::Pool { para_id: 2.into(), core_index: CoreIndex(1) }, + ttl, + )]), + ), + ])); + let _ = Pallet::::enter(frame_system::RawOrigin::None.into(), data).unwrap(); + + assert_matches!(pallet::OnChainVotes::::get(), Some(ScrapedOnChainVotes { + disputes, + .. + } ) => { + assert_eq!(disputes.len(), n_disputes); + }); + + let parent_block_number = RELAY_PARENT_NUMBER + 2; + run_to_block(parent_block_number, |_| None); + let parent_header = HeaderFor::::new( + parent_block_number, // `block_number`, + Default::default(), // `extrinsics_root`, + Default::default(), // `storage_root`, + parent_header.hash(), // `parent_hash`, + Default::default(), // digest, + ); + + frame_system::Pallet::::reset_events(); + frame_system::Pallet::::initialize( + &(parent_header.number() + 1), + &parent_header.hash(), + &Digest { logs: Vec::new() }, + ); + + let data = ParachainsInherentData { + disputes: Vec::new(), + parent_header, + bitfields: Vec::new(), + backed_candidates, + }; + + let err = + Pallet::::enter(frame_system::RawOrigin::None.into(), data).unwrap_err(); + assert_matches!( + err.error, + DispatchError::Module(ModuleError { + message: Some("CandidatesFilteredDuringExecution"), + .. + }) + ); + }); + } + #[rstest] #[case(false)] #[case(true)] diff --git a/prdoc/pr_4690.prdoc b/prdoc/pr_4690.prdoc new file mode 100644 index 000000000000..154f513847bf --- /dev/null +++ b/prdoc/pr_4690.prdoc @@ -0,0 +1,13 @@ +title: Relay chain runtime filtering changes for backing votes + +doc: + - audience: Runtime Dev + description: | + This change is only relevant for the relay chain runtime. + This change is necessary to ensure that the backing votes are correctly + filtered in the runtime for an extra layer of protection against bugs in + the code. + +crates: + - name: polkadot-runtime-parachains + bump: major