fix the test again

polkadot/runtime/parachains/src/builder.rs

@@ -53,7 +53,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<AccountId: Decode>(name: &'static str, index: u32, seed: u32) -> AccountId {
+pub fn account<AccountId: Decode>(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")

polkadot/runtime/parachains/src/disputes.rs

@@ -229,6 +229,9 @@ pub trait DisputesHandler<BlockNumber: Ord> {
 		included_in: BlockNumber,
 	);
 
+	/// Get a list of disputes in a session that concluded invalid.
+	fn disputes_concluded_invalid(session: SessionIndex) -> BTreeSet<CandidateHash>;
+
 	/// Retrieve the included state of a given candidate in a particular session. If it
 	/// returns `Some`, then we have a local dispute for the given `candidate_hash`.
 	fn included_state(session: SessionIndex, candidate_hash: CandidateHash) -> Option<BlockNumber>;
@@ -271,6 +274,10 @@ impl<BlockNumber: Ord> DisputesHandler<BlockNumber> for () {
 		Ok(Vec::new())
 	}
 
+	fn disputes_concluded_invalid(_session: SessionIndex) -> BTreeSet<CandidateHash> {
+		BTreeSet::new()
+	}
+
 	fn note_included(
 		_session: SessionIndex,
 		_candidate_hash: CandidateHash,
@@ -320,6 +327,10 @@ where
 		pallet::Pallet::<T>::process_checked_multi_dispute_data(statement_sets)
 	}
 
+	fn disputes_concluded_invalid(session: SessionIndex) -> BTreeSet<CandidateHash> {
+		pallet::Pallet::<T>::disputes_concluded_invalid(session)
+	}
+
 	fn note_included(
 		session: SessionIndex,
 		candidate_hash: CandidateHash,
@@ -555,7 +566,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,
@@ -691,6 +702,7 @@ impl<BlockNumber: Clone> DisputeStateImporter<BlockNumber> {
 		let mut undo =
 			ImportUndo { validator_index: validator, vote_kind: kind, new_participant: false };
 
+		log::debug!(target: LOG_TARGET, "Setting dispute bit: {}", validator.0);
 		bits.set(validator.0 as usize, true);
 		if kind.is_backing() {
 			let is_new = self.backers.insert(validator);
@@ -948,7 +960,10 @@ impl<T: Config> Pallet<T> {
 		// Load session info to access validators
 		let session_info = match session_info::Sessions::<T>::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::<T>::get();
@@ -959,6 +974,7 @@ impl<T: Config> Pallet<T> {
 		let dispute_state = {
 			if let Some(dispute_state) = Disputes::<T>::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
 				}
 
@@ -976,6 +992,7 @@ impl<T: Config> Pallet<T> {
 
 		let backers =
 			BackersOnDisputes::<T>::get(&set.session, &set.candidate_hash).unwrap_or_default();
+		log::debug!(target: LOG_TARGET, "Dispute import with {} backers", backers.len());
 
 		// Check and import all votes.
 		let summary = {
@@ -986,17 +1003,20 @@ impl<T: Config> Pallet<T> {
 				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,
 				};
 
 				let kind = VoteKind::from(statement);
+				log::debug!(target: LOG_TARGET, "Importing {:?} vote from {}", kind, validator_index.0);
 
 				let undo = match importer.import(*validator_index, kind) {
 					Ok(u) => u,
 					Err(_) => {
 						filter.remove_index(i);
+						log::debug!(target: LOG_TARGET, "Import of a dispute failed for validator: {}", validator_index.0);
 						continue
 					},
 				};
@@ -1022,7 +1042,7 @@ impl<T: Config> Pallet<T> {
 					// 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);
@@ -1037,13 +1057,15 @@ impl<T: Config> Pallet<T> {
 		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
 		}
 
 		// Reject disputes containing less votes than needed for confirmation.
 		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
 		}
 
@@ -1215,12 +1237,13 @@ impl<T: Config> Pallet<T> {
 		let revert_to = included_in - One::one();
 
 		Included::<T>::insert(&session, &candidate_hash, revert_to);
+	}
 
-		if let Some(state) = Disputes::<T>::get(&session, candidate_hash) {
-			if has_supermajority_against(&state) {
-				Self::revert_and_freeze(revert_to);
-			}
-		}
+	pub(crate) fn disputes_concluded_invalid(session: SessionIndex) -> BTreeSet<CandidateHash> {
+		Disputes::<T>::iter_prefix(session)
+			.filter(|(_hash, state)| has_supermajority_against(state))
+			.map(|(hash, _state)| hash)
+			.collect()
 	}
 
 	pub(crate) fn included_state(

polkadot/runtime/parachains/src/disputes/tests.rs

@@ -93,10 +93,10 @@ pub(crate) fn make_dispute_concluding_against(
 	validators: &[(ValidatorIndex, Sr25519Keyring)],
 ) -> DisputeStatementSet {
 	let mut statements = Vec::new();
-	for (i, key) in validators.iter() {
+	for (j, (i, key)) in validators.iter().enumerate() {
 		// make the first statement backing
 		// and the rest against it
-		let statement = if i.0 == 0 {
+		let statement = if j == 0 {
 			DisputeStatement::Valid(ValidDisputeStatementKind::BackingSeconded(candidate_hash.0))
 		} else {
 			DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit)

polkadot/runtime/parachains/src/paras_inherent/mod.rs

@@ -402,12 +402,14 @@ impl<T: Config> Pallet<T> {
 
 		// Limit the disputes first, since the following statements depend on the votes include
 		// here.
+		log::debug!(target: LOG_TARGET, "Disputes before sanitization: {}", disputes.len());
 		let (checked_disputes_sets, checked_disputes_sets_consumed_weight) =
 			limit_and_sanitize_disputes::<T, _>(
 				disputes,
 				dispute_statement_set_valid,
 				max_block_weight,
 			);
+		log::debug!(target: LOG_TARGET, "Disputes after sanitization: {}", checked_disputes_sets.len());
 
 		let all_weight_after = if context == ProcessInherentDataContext::ProvideInherent {
 			// Assure the maximum block weight is adhered, by limiting bitfields and backed
@@ -456,13 +458,11 @@ impl<T: Config> Pallet<T> {
 		// Note that `process_checked_multi_dispute_data` will iterate and import each
 		// dispute; so the input here must be reasonably bounded,
 		// which is guaranteed by the checks and weight limitation above.
-		// We don't care about fresh or not disputes
-		// this writes them to storage, so let's query it via those means
-		// if this fails for whatever reason, that's ok.
 		if let Err(e) =
 			T::DisputesHandler::process_checked_multi_dispute_data(&checked_disputes_sets)
 		{
 			log::warn!(target: LOG_TARGET, "MultiDisputesData failed to update: {:?}", e);
+			// return Err(e.into())
 		};
 		METRICS.on_disputes_imported(checked_disputes_sets.len() as u64);
 
@@ -490,26 +490,15 @@ impl<T: Config> Pallet<T> {
 		// Contains the disputes that are concluded in the current session only,
 		// since these are the only ones that are relevant for the occupied cores
 		// and lightens the load on `free_disputed` significantly.
-		// Cores can't be occupied with candidates of the previous sessions, and only
-		// things with new votes can have just concluded. We only need to collect
-		// cores with disputes that conclude just now, because disputes that
-		// concluded longer ago have already had any corresponding cores cleaned up.
-		let current_concluded_invalid_disputes = checked_disputes_sets
-			.iter()
-			.map(AsRef::as_ref)
-			.filter(|dss| dss.session == current_session)
-			.map(|dss| (dss.session, dss.candidate_hash))
-			.filter(|(session, candidate)| {
-				<T>::DisputesHandler::concluded_invalid(*session, *candidate)
-			})
-			.map(|(_session, candidate)| candidate)
-			.collect::<BTreeSet<CandidateHash>>();
+		let mut current_concluded_invalid_disputes =
+			<T>::DisputesHandler::disputes_concluded_invalid(current_session);
 
 		// Get the cores freed as a result of concluded invalid candidates.
 		let (freed_disputed, concluded_invalid_hashes): (Vec<CoreIndex>, BTreeSet<CandidateHash>) =
 			inclusion::Pallet::<T>::free_disputed(&current_concluded_invalid_disputes)
 				.into_iter()
 				.unzip();
+		current_concluded_invalid_disputes.extend(concluded_invalid_hashes);
 
 		// 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.
@@ -582,7 +571,7 @@ impl<T: Config> Pallet<T> {
 		let backed_candidates_with_core = sanitize_backed_candidates::<T>(
 			backed_candidates,
 			&allowed_relay_parents,
-			concluded_invalid_hashes,
+			current_concluded_invalid_disputes,
 			scheduled,
 			core_index_enabled,
 		);
@@ -1079,8 +1068,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);
 				}
 			}
 		});