remove Default from CandidateDescriptor (#4484)

* remove Default from CandidateHash * Apply suggestions from code review Co-authored-by: Andronik Ordian <write@reusable.software> * chore: fmt * remove backed candidate default * Partial migration away from CandidateReceipt::default * Remove more CandidateReceipt defaults * fmt * Mostly remove CommittedCandidateReceipt default usage * Remove CommittedCandidateReceipt * Remove more Defaults from polakdot primitives v1 + fmt * Remove more Default from polkadot primites v1 * WIP trying to get overseer example + tests to compile * feat: add primitives test helpers * reduce deps of helper * update primitive helpers * make candidate validation compile * fixup cargo lock * make av-store compile * fixup disputes coordinator tests * test: fixup backing * test: fixup approval voting * fixup bitfield signing * test: fixup runtime-api * test: fixup availability dist * foxi[ pverseer test] * remove some Defaults, remove bounds from `dummy` All `fn dummy` in primitives need to be removed anyways. This aids in the transition. * it's a test helper, so always use std * test: fixup parachains runtime tests Excluding benches. * fix keyring * fix paras runtime properly, no more default * Remove fn dummy() usage from approval voting * Move TestCandidateBuilder out of av store to test helpers * Make candidate validation tests pass * Make most dispute coirdinator tests pass * Make provisioner tests work * Make availability recovery tests work with test helpers * Update polkadot-collator-protocol tests * Update statement distribution tests * Update polkadot overseer examples and tests * Derive default for validation code so we don't break unrelated things * Make para runtime test pass (no bench) * Some more work * chore: cargo fmt * cargo fix * avoid some Default::default * fixup dispute coordinator test * remove unused crate deps * remove Default::default wherever possible, replace by dummy_* for the most part * chore: cargo fmt * Remove some warnings * Remove CommittedCandidateReceipt dummy * Remove CandidateReceipt dummy * Remove CandidateDescriptor dummy * Remove commented out code * Fix para runtime tests * chore: nightly * Some updates to the builder * Dynamically adjust mock head data size * Make dispute cooridinator tests work * Fix test candidate_backing_reorders_votes work * +nightly-2021-10-29 fmt * Spelling and remove a default use in builder * Various clean up * More small updates * fmt * More small updates * Doc comments for test helpers * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_parachains::paras_inherent --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_parachains_paras_inherent.rs * cargo run --quiet --release --features=runtime-benchmarks -- benchmark --chain=polkadot-dev --steps=50 --repeat=20 --pallet=runtime_parachains::paras_inherent --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/polkadot/src/weights/runtime_parachains_paras_inherent.rs * Update lib.rs * review comments * fix warnings * fix test by using correct candidate receipt relay parent Co-authored-by: Andronik Ordian <write@reusable.software> Co-authored-by: emostov <32168567+emostov@users.noreply.github.com> Co-authored-by: Parity Bot <admin@parity.io> Co-authored-by: Gavin Wood <gavin@parity.io>
2026-05-30 07:01:03 +00:00 · 2021-12-10 13:12:07 +01:00
parent 916497e5db
commit 0f1a9fb1eb
68 changed files with 993 additions and 500 deletions
@@ -49,7 +49,7 @@ hex-literal = "0.3.4"
 keyring = { package = "sp-keyring", git = "https://github.com/paritytech/substrate", branch = "master" }
 frame-support-test = { git = "https://github.com/paritytech/substrate", branch = "master" }
 sc-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" }
-
+test-helpers = { package = "polkadot-primitives-test-helpers", path = "../../primitives/test-helpers"}

 [features]
 default = ["std"]
@@ -23,13 +23,13 @@ use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec};
 use frame_support::pallet_prelude::*;
 use primitives::v1::{
 	collator_signature_payload, AvailabilityBitfield, BackedCandidate, CandidateCommitments,
-	CandidateDescriptor, CandidateHash, CollatorId, CommittedCandidateReceipt, CompactStatement,
-	CoreIndex, CoreOccupied, DisputeStatement, DisputeStatementSet, GroupIndex, HeadData,
-	Id as ParaId, InherentData as ParachainsInherentData, InvalidDisputeStatementKind,
+	CandidateDescriptor, CandidateHash, CollatorId, CollatorSignature, CommittedCandidateReceipt,
+	CompactStatement, CoreIndex, CoreOccupied, DisputeStatement, DisputeStatementSet, GroupIndex,
+	HeadData, Id as ParaId, InherentData as ParachainsInherentData, InvalidDisputeStatementKind,
 	PersistedValidationData, SessionIndex, SigningContext, UncheckedSigned,
 	ValidDisputeStatementKind, ValidationCode, ValidatorId, ValidatorIndex, ValidityAttestation,
 };
-use sp_core::H256;
+use sp_core::{sr25519, H256};
 use sp_runtime::{
 	generic::Digest,
 	traits::{Header as HeaderT, One, Zero},
@@ -37,7 +37,7 @@ use sp_runtime::{
 };
 use sp_std::{collections::btree_map::BTreeMap, convert::TryInto, prelude::Vec, vec};

-fn dummy_validation_code() -> ValidationCode {
+fn mock_validation_code() -> ValidationCode {
 	ValidationCode(vec![1, 2, 3])
 }

@@ -47,7 +47,7 @@ fn dummy_validation_code() -> ValidationCode {
 /// "features = runtime-benchmarks".
 fn account<AccountId: Decode + Default>(name: &'static str, index: u32, seed: u32) -> AccountId {
 	let entropy = (name, index, seed).using_encoded(sp_io::hashing::blake2_256);
-	AccountId::decode(&mut &entropy[..]).unwrap_or_default()
+	AccountId::decode(&mut &entropy[..]).expect("256 bit input is valid. qed.")
 }

 /// Create a 32 byte slice based on the given number.
@@ -232,6 +232,25 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 		(para_id, core_idx, group_idx)
 	}

+	fn mock_head_data() -> HeadData {
+		let max_head_size = configuration::Pallet::<T>::config().max_head_data_size;
+		HeadData(vec![0xFF; max_head_size as usize])
+	}
+
+	fn candidate_descriptor_mock() -> CandidateDescriptor<T::Hash> {
+		CandidateDescriptor::<T::Hash> {
+			para_id: 0.into(),
+			relay_parent: Default::default(),
+			collator: CollatorId::from(sr25519::Public::from_raw([42u8; 32])),
+			persisted_validation_data_hash: Default::default(),
+			pov_hash: Default::default(),
+			erasure_root: Default::default(),
+			signature: CollatorSignature::from(sr25519::Signature([42u8; 64])),
+			para_head: Default::default(),
+			validation_code_hash: mock_validation_code().hash(),
+		}
+	}
+
 	/// Create a mock of `CandidatePendingAvailability`.
 	fn candidate_availability_mock(
 		group_idx: GroupIndex,
@@ -240,14 +259,14 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 		availability_votes: BitVec<BitOrderLsb0, u8>,
 	) -> inclusion::CandidatePendingAvailability<T::Hash, T::BlockNumber> {
 		inclusion::CandidatePendingAvailability::<T::Hash, T::BlockNumber>::new(
-			core_idx,           // core
-			candidate_hash,     // hash
-			Default::default(), // candidate descriptor
-			availability_votes, // availability votes
-			Default::default(), // backers
-			Zero::zero(),       // relay parent
-			One::one(),         // relay chain block this was backed in
-			group_idx,          // backing group
+			core_idx,                          // core
+			candidate_hash,                    // hash
+			Self::candidate_descriptor_mock(), // candidate descriptor
+			availability_votes,                // availability votes
+			Default::default(),                // backers
+			Zero::zero(),                      // relay parent
+			One::one(),                        // relay chain block this was backed in
+			group_idx,                         // backing group
 		)
 	}

@@ -269,7 +288,14 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 			candidate_hash,
 			availability_votes,
 		);
-		let commitments = CandidateCommitments::<u32>::default();
+		let commitments = CandidateCommitments::<u32> {
+			upward_messages: vec![],
+			horizontal_messages: vec![],
+			new_validation_code: None,
+			head_data: Self::mock_head_data(),
+			processed_downward_messages: 0,
+			hrmp_watermark: 0u32.into(),
+		};
 		inclusion::PendingAvailability::<T>::insert(para_id, candidate_availability);
 		inclusion::PendingAvailabilityCommitments::<T>::insert(&para_id, commitments);
 	}
@@ -315,8 +341,8 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 			paras::Pallet::<T>::schedule_para_initialize(
 				para_id,
 				paras::ParaGenesisArgs {
-					genesis_head: Default::default(),
-					validation_code: dummy_validation_code(),
+					genesis_head: Self::mock_head_data(),
+					validation_code: mock_validation_code(),
 					parachain: true,
 				},
 			)
@@ -464,7 +490,7 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 				let collator_public = CollatorId::generate_pair(None);
 				let header = Self::header(self.block_number.clone());
 				let relay_parent = header.hash();
-				let head_data: HeadData = Default::default();
+				let head_data = Self::mock_head_data();
 				let persisted_validation_data_hash = PersistedValidationData::<H256> {
 					parent_head: head_data.clone(),
 					relay_parent_number: self.relay_parent_number(),
@@ -474,7 +500,7 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 				.hash();

 				let pov_hash = Default::default();
-				let validation_code_hash = dummy_validation_code().hash();
+				let validation_code_hash = mock_validation_code().hash();
 				let payload = collator_signature_payload(
 					&relay_parent,
 					&para_id,
@@ -509,7 +535,7 @@ impl<T: paras_inherent::Config> BenchBuilder<T> {
 						upward_messages: Vec::new(),
 						horizontal_messages: Vec::new(),
 						new_validation_code: includes_code_upgrade
-							.map(|v| ValidationCode(vec![0u8; v as usize])),
+							.map(|v| ValidationCode(vec![42u8; v as usize])),
 						head_data,
 						processed_downward_messages: 0,
 						hrmp_watermark: self.relay_parent_number(),
@@ -71,7 +71,7 @@ pub(crate) enum FullCheck {

 /// A backed candidate pending availability.
 #[derive(Encode, Decode, PartialEq, TypeInfo)]
-#[cfg_attr(test, derive(Debug, Default))]
+#[cfg_attr(test, derive(Debug))]
 pub struct CandidatePendingAvailability<H, N> {
 	/// The availability core this is assigned to.
 	core: CoreIndex,
@@ -40,6 +40,10 @@ use primitives::{
 use sc_keystore::LocalKeystore;
 use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr};
 use std::sync::Arc;
+use test_helpers::{
+	dummy_candidate_descriptor, dummy_collator, dummy_collator_signature, dummy_hash,
+	dummy_validation_code,
+};

 fn default_config() -> HostConfiguration<BlockNumber> {
 	let mut config = HostConfiguration::default();
@@ -58,7 +62,7 @@ pub(crate) fn genesis_config(paras: Vec<(ParaId, bool)>) -> MockGenesisConfig {
 						id,
 						ParaGenesisArgs {
 							genesis_head: Vec::new().into(),
-							validation_code: Vec::new().into(),
+							validation_code: dummy_validation_code(),
 							parachain: is_chain,
 						},
 					)
@@ -238,7 +242,6 @@ pub(crate) async fn sign_bitfield(
 	.unwrap()
 }

-#[derive(Default)]
 pub(crate) struct TestCandidateBuilder {
 	pub(crate) para_id: ParaId,
 	pub(crate) head_data: HeadData,
@@ -251,6 +254,23 @@ pub(crate) struct TestCandidateBuilder {
 	pub(crate) hrmp_watermark: BlockNumber,
 }

+impl std::default::Default for TestCandidateBuilder {
+	fn default() -> Self {
+		let zeros = Hash::zero();
+		Self {
+			para_id: 0.into(),
+			head_data: Default::default(),
+			para_head_hash: None,
+			pov_hash: zeros,
+			relay_parent: zeros,
+			persisted_validation_data_hash: zeros,
+			new_validation_code: None,
+			validation_code: dummy_validation_code(),
+			hrmp_watermark: 0u32.into(),
+		}
+	}
+}
+
 impl TestCandidateBuilder {
 	pub(crate) fn build(self) -> CommittedCandidateReceipt {
 		CommittedCandidateReceipt {
@@ -261,7 +281,9 @@ impl TestCandidateBuilder {
 				persisted_validation_data_hash: self.persisted_validation_data_hash,
 				validation_code_hash: self.validation_code.hash(),
 				para_head: self.para_head_hash.unwrap_or_else(|| self.head_data.hash()),
-				..Default::default()
+				erasure_root: Default::default(),
+				signature: dummy_collator_signature(),
+				collator: dummy_collator(),
 			},
 			commitments: CandidateCommitments {
 				head_data: self.head_data,
@@ -388,7 +410,13 @@ fn bitfield_checks() {
 					p_id,
 					CandidatePendingAvailability {
 						availability_votes: default_availability_votes(),
-						..Default::default()
+						core: Default::default(),
+						hash: Default::default(),
+						descriptor: dummy_candidate_descriptor(dummy_hash()),
+						backers: Default::default(),
+						relay_parent_number: Default::default(),
+						backed_in_number: Default::default(),
+						backing_group: Default::default(),
 					},
 				)
 			}
@@ -339,7 +339,8 @@ mod tests {
 		new_test_ext, Configuration, Dmp, Initializer, MockGenesisConfig, Paras, SessionInfo,
 		System,
 	};
-	use primitives::v1::Id as ParaId;
+	use primitives::v1::{HeadData, Id as ParaId};
+	use test_helpers::dummy_validation_code;

 	use frame_support::{
 		assert_ok,
@@ -426,8 +427,8 @@ mod tests {

 		let mock_genesis = crate::paras::ParaGenesisArgs {
 			parachain: true,
-			genesis_head: Default::default(),
-			validation_code: Default::default(),
+			genesis_head: HeadData(vec![4, 5, 6]),
+			validation_code: dummy_validation_code(),
 		};

 		new_test_ext(MockGenesisConfig {
@@ -456,13 +456,20 @@ pub mod pallet {
 			new_code: ValidationCode,
 		) -> DispatchResult {
 			ensure_root(origin)?;
-			let prior_code_hash = <Self as Store>::CurrentCodeHash::get(&para).unwrap_or_default();
+			let maybe_prior_code_hash = <Self as Store>::CurrentCodeHash::get(&para);
 			let new_code_hash = new_code.hash();
 			Self::increase_code_ref(&new_code_hash, &new_code);
 			<Self as Store>::CurrentCodeHash::insert(&para, new_code_hash);

 			let now = frame_system::Pallet::<T>::block_number();
-			Self::note_past_code(para, now, now, prior_code_hash);
+			if let Some(prior_code_hash) = maybe_prior_code_hash {
+				Self::note_past_code(para, now, now, prior_code_hash);
+			} else {
+				log::error!(
+					"Pallet paras storage is inconsistent, prior code not found {:?}",
+					&para
+				);
+			}
 			Self::deposit_event(Event::CurrentCodeUpdated(para));
 			Ok(())
 		}
@@ -959,8 +966,13 @@ impl<T: Config> Pallet<T> {
 				<Self as Store>::UpgradeGoAheadSignal::remove(&id);

 				// Both should always be `Some` in this case, since a code upgrade is scheduled.
-				let new_code_hash = FutureCodeHash::<T>::take(&id).unwrap_or_default();
-				let prior_code_hash = CurrentCodeHash::<T>::get(&id).unwrap_or_default();
+				let new_code_hash = if let Some(new_code_hash) = FutureCodeHash::<T>::take(&id) {
+					new_code_hash
+				} else {
+					log::error!("Missing future code hash for {:?}", &id);
+					return T::DbWeight::get().reads_writes(3, 1 + 3)
+				};
+				let prior_code_hash = CurrentCodeHash::<T>::get(&id);
 				CurrentCodeHash::<T>::insert(&id, &new_code_hash);

 				let log = ConsensusLog::ParaUpgradeCode(id, new_code_hash);
@@ -969,7 +981,12 @@ impl<T: Config> Pallet<T> {
 				// `now` is only used for registering pruning as part of `fn note_past_code`
 				let now = <frame_system::Pallet<T>>::block_number();

-				let weight = Self::note_past_code(id, expected_at, now, prior_code_hash);
+				let weight = if let Some(prior_code_hash) = prior_code_hash {
+					Self::note_past_code(id, expected_at, now, prior_code_hash)
+				} else {
+					log::error!("Missing prior code hash for {:?}", &id);
+					0 as Weight
+				};

 				// add 1 to writes due to heads update.
 				weight + T::DbWeight::get().reads_writes(3, 1 + 3)
@@ -1078,6 +1095,7 @@ mod tests {
 	use super::*;
 	use frame_support::{assert_err, assert_ok};
 	use primitives::v1::BlockNumber;
+	use test_helpers::{dummy_head_data, dummy_validation_code};

 	use crate::{
 		configuration::HostConfiguration,
@@ -1195,7 +1213,7 @@ mod tests {
 					1000.into(),
 					ParaGenesisArgs {
 						parachain: false,
-						genesis_head: Default::default(),
+						genesis_head: dummy_head_data(),
 						validation_code: ValidationCode(vec![]),
 					}
 				),
@@ -1206,7 +1224,7 @@ mod tests {
 				1000.into(),
 				ParaGenesisArgs {
 					parachain: false,
-					genesis_head: Default::default(),
+					genesis_head: dummy_head_data(),
 					validation_code: ValidationCode(vec![1]),
 				}
 			));
@@ -1221,16 +1239,16 @@ mod tests {
 				0u32.into(),
 				ParaGenesisArgs {
 					parachain: true,
-					genesis_head: Default::default(),
-					validation_code: Default::default(),
+					genesis_head: dummy_head_data(),
+					validation_code: dummy_validation_code(),
 				},
 			),
 			(
 				1u32.into(),
 				ParaGenesisArgs {
 					parachain: false,
-					genesis_head: Default::default(),
-					validation_code: Default::default(),
+					genesis_head: dummy_head_data(),
+					validation_code: dummy_validation_code(),
 				},
 			),
 		];
@@ -1248,7 +1266,7 @@ mod tests {
 			let id = ParaId::from(0u32);
 			let at_block: BlockNumber = 10;
 			let included_block: BlockNumber = 12;
-			let validation_code = ValidationCode(vec![1, 2, 3]);
+			let validation_code = ValidationCode(vec![4, 5, 6]);

 			Paras::increase_code_ref(&validation_code.hash(), &validation_code);
 			<Paras as Store>::PastCodeHash::insert(&(id, at_block), &validation_code.hash());
@@ -1289,8 +1307,8 @@ mod tests {
 			0u32.into(),
 			ParaGenesisArgs {
 				parachain: true,
-				genesis_head: Default::default(),
-				validation_code: Default::default(),
+				genesis_head: dummy_head_data(),
+				validation_code: dummy_validation_code(),
 			},
 		)];

@@ -1306,7 +1324,7 @@ mod tests {
 		new_test_ext(genesis_config).execute_with(|| {
 			let id_a = ParaId::from(0u32);

-			assert_eq!(Paras::para_head(&id_a), Some(Default::default()));
+			assert_eq!(Paras::para_head(&id_a), Some(dummy_head_data()));

 			Paras::note_new_head(id_a, vec![1, 2, 3].into(), 0);

@@ -1322,16 +1340,16 @@ mod tests {
 				0u32.into(),
 				ParaGenesisArgs {
 					parachain: true,
-					genesis_head: Default::default(),
-					validation_code: Default::default(),
+					genesis_head: dummy_head_data(),
+					validation_code: dummy_validation_code(),
 				},
 			),
 			(
 				1u32.into(),
 				ParaGenesisArgs {
 					parachain: false,
-					genesis_head: Default::default(),
-					validation_code: Default::default(),
+					genesis_head: dummy_head_data(),
+					validation_code: dummy_validation_code(),
 				},
 			),
 		];
@@ -1375,7 +1393,7 @@ mod tests {
 			0u32.into(),
 			ParaGenesisArgs {
 				parachain: true,
-				genesis_head: Default::default(),
+				genesis_head: dummy_head_data(),
 				validation_code: original_code.clone(),
 			},
 		)];
@@ -1482,7 +1500,7 @@ mod tests {
 			0u32.into(),
 			ParaGenesisArgs {
 				parachain: true,
-				genesis_head: Default::default(),
+				genesis_head: dummy_head_data(),
 				validation_code: original_code.clone(),
 			},
 		)];
@@ -1570,7 +1588,7 @@ mod tests {
 			0u32.into(),
 			ParaGenesisArgs {
 				parachain: true,
-				genesis_head: Default::default(),
+				genesis_head: dummy_head_data(),
 				validation_code: vec![1, 2, 3].into(),
 			},
 		)];
@@ -1625,7 +1643,7 @@ mod tests {
 			0u32.into(),
 			ParaGenesisArgs {
 				parachain: true,
-				genesis_head: Default::default(),
+				genesis_head: dummy_head_data(),
 				validation_code: original_code.clone(),
 			},
 		)];
@@ -1808,7 +1826,7 @@ mod tests {
 			0u32.into(),
 			ParaGenesisArgs {
 				parachain: true,
-				genesis_head: Default::default(),
+				genesis_head: dummy_head_data(),
 				validation_code: vec![1, 2, 3].into(),
 			},
 		)];
@@ -49,7 +49,7 @@ use crate::{configuration, initializer::SessionChangeNotification, paras};
 pub use pallet::*;

 /// A queued parathread entry, pre-assigned to a core.
-#[derive(Encode, Decode, Default, TypeInfo)]
+#[derive(Encode, Decode, TypeInfo)]
 #[cfg_attr(test, derive(PartialEq, Debug))]
 pub struct QueuedParathread {
 	claim: ParathreadEntry,
@@ -57,7 +57,7 @@ pub struct QueuedParathread {
 }

 /// The queue of all parathread claims.
-#[derive(Encode, Decode, Default, TypeInfo)]
+#[derive(Encode, Decode, TypeInfo)]
 #[cfg_attr(test, derive(PartialEq, Debug))]
 pub struct ParathreadClaimQueue {
 	queue: Vec<QueuedParathread>,
@@ -89,6 +89,12 @@ impl ParathreadClaimQueue {
 	}
 }

+impl Default for ParathreadClaimQueue {
+	fn default() -> Self {
+		Self { queue: vec![], next_core_offset: 0 }
+	}
+}
+
 /// Reasons a core might be freed
 #[derive(Clone, Copy)]
 pub enum FreedReason {