move paras inherent filtering to runtime (#4028)

* move things around, add filter methods��

* validator keys, modify availability bitfields according to disputes

* simplify, keep the filter -> sanitize generic for both usecases

* minor

* assure tests still work, reduce changeset

* integration

* start entropy passing

* fixins

* compile, 1 failing test

* filter with coverage

* fixins

* Update runtime/parachains/src/paras_inherent.rs

Co-authored-by: Robert Habermeier <rphmeier@gmail.com>

* slip of the pen

* improve test cases

* misc

* fix

* fixins

* test avoid extra into() calls in assert_noop!

* chores

* ff

* test fixup superfluous into call

* chore: pfmt

* improve apply_block_weight_limit to try to maximize the number of sufficiently backed

blocks and add extra bitfields in a round-robin fashion

* new code treats the lack of backed candidates as ok

* Use vrf based entropy

* fixup vrf random

* add warn

* slip of the pen

* fixup

* assure ordering

* rethink apply_weights

* mock

* use a closure as predicate check

* extract and use DisputedBitfield

* chore: simplify

* remove stray dbg

* chore: fmt

* address feedback

* fix test, halfway there

* stage1

* dbg stuff

* make group selection align

* fix session index

* fix wrongly returned candidates

* cleanup

* chore fmt

* fix ensure check

* make good case test work

* more tests for bitfields

* create sanitize_backed_candidates

* fixup tests

* update guide

* add check referenced in the guide

* improve weights code

* fmt

* fixins

* Update roadmap/implementers-guide/src/runtime/inclusion.md

Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>

* compiling + address review

* add comments

* fix weight calc

* address review comments and test failure

* fix

* fix: condition

* Fix random_sel function

* Fix overlength block check

* Zeke + Ladi commit for disputes filtering + integration test builder + runtime benchmarks + integration tests

* Add benchmarks for code upgrades

* Code upgrade bench; Feature gate TestWeightInfo

* Try and make CI happier

* Feature gate enter test to not(benchmarks)

* Make sure no unused imports/fn

* refactor, re-use, the beginning

* Fix issue with frame benchmarking dep compilation

* More precise feature gating for some derives

* integrate piece-wise

* foo

* fixins

* chore fmt

* fixins

* rename const generic

* Update runtime/parachains/src/paras_inherent.rs

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

* Fix compilation

* limit to test

* remove unused spam slots

* spellcheck

* remove a tick, fix a typo

* Add Code upgrade weights

* comment improvements + >=

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

* remove another tick

* Update runtime/parachains/src/paras_inherent/benchmarking.rs

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

* saturating fixins + some spaces

* fix

* benchmarking - preliminary results

* Add training wheels

* Refactor some early exit logic for enter

* Gracefully handle filtering bitfields & candidates (#4280)

This updates the logic for sanitize_bitfields and sanitize_backed_candidates to never error when there is an issue, but instead to simply skip the problematic items.

* Refactor inherent data weight limiting logic (#4287)

* Apply suggestions from code review

* Update runtime/parachains/src/builder.rs

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

* Update runtime/parachains/src/builder.rs

* Update runtime/parachains/src/paras_inherent.rs

* final pass

* Run cargo +nightly-2021-10-29 fmt

* Update implementors guide with `sanitize_*` & `enter` (#4294)

* Make spell check happier

* Make wasm runtimes compile with benchmarks enabled (#4303)

* comment stuff out, use old toml

* Seems to be working?

* Remove feature gating from builder

* Remove commented out stuff

* Remove generic from digest

* Update weight files for runtime

Co-authored-by: Robert Habermeier <rphmeier@gmail.com>
Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com>
Co-authored-by: Zeke Mostov <z.mostov@gmail.com>
Co-authored-by: Bastian Köcher <info@kchr.de>

parachain/Cargo.toml

@@ -36,3 +36,4 @@ std = [
 	"polkadot-core-primitives/std",
 	"frame-support/std",
 ]
+runtime-benchmarks = []

parachain/src/primitives.rs

@@ -41,9 +41,19 @@ pub use polkadot_core_primitives::BlockNumber as RelayChainBlockNumber;
 
 /// Parachain head data included in the chain.
 #[derive(
-	PartialEq, Eq, Clone, PartialOrd, Ord, Encode, Decode, RuntimeDebug, derive_more::From, TypeInfo,
+	PartialEq,
+	Eq,
+	Clone,
+	PartialOrd,
+	Ord,
+	Encode,
+	Decode,
+	RuntimeDebug,
+	derive_more::From,
+	TypeInfo,
+	Default,
 )]
-#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Default, Hash, MallocSizeOf))]
+#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Hash, MallocSizeOf))]
 pub struct HeadData(#[cfg_attr(feature = "std", serde(with = "bytes"))] pub Vec<u8>);
 
 impl HeadData {

primitives/Cargo.toml

@@ -55,4 +55,4 @@ std = [
 	"polkadot-core-primitives/std",
 	"bitvec/std",
 	"frame-system/std",
-]
+]

primitives/src/v0.rs

@@ -107,8 +107,8 @@ impl MallocSizeOf for ValidatorId {
 }
 
 /// Index of the validator is used as a lightweight replacement of the `ValidatorId` when appropriate.
-#[derive(Eq, Ord, PartialEq, PartialOrd, Copy, Clone, Encode, Decode, TypeInfo)]
-#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug, Hash, MallocSizeOf))]
+#[derive(Eq, Ord, PartialEq, PartialOrd, Copy, Clone, Encode, Decode, TypeInfo, Debug)]
+#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Hash, MallocSizeOf))]
 pub struct ValidatorIndex(pub u32);
 
 // We should really get https://github.com/paritytech/polkadot/issues/2403 going ..

primitives/src/v1/mod.rs

@@ -327,8 +327,8 @@ fn check_collator_signature<H: AsRef<[u8]>>(
 }
 
 /// A unique descriptor of the candidate receipt.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
-#[cfg_attr(feature = "std", derive(Debug, Default, Hash, MallocSizeOf))]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, Default)]
+#[cfg_attr(feature = "std", derive(Debug, Hash, MallocSizeOf))]
 pub struct CandidateDescriptor<H = Hash> {
 	/// The ID of the para this is a candidate for.
 	pub para_id: Id,
@@ -407,8 +407,8 @@ pub struct FullCandidateReceipt<H = Hash, N = BlockNumber> {
 }
 
 /// A candidate-receipt with commitments directly included.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
-#[cfg_attr(feature = "std", derive(Debug, Default, Hash, MallocSizeOf))]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, Default)]
+#[cfg_attr(feature = "std", derive(Debug, Hash, MallocSizeOf))]
 pub struct CommittedCandidateReceipt<H = Hash> {
 	/// The descriptor of the candidate.
 	pub descriptor: CandidateDescriptor<H>,
@@ -509,8 +509,8 @@ impl<H: Encode, N: Encode> PersistedValidationData<H, N> {
 }
 
 /// Commitments made in a `CandidateReceipt`. Many of these are outputs of validation.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
-#[cfg_attr(feature = "std", derive(Debug, Default, Hash, MallocSizeOf))]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, Default)]
+#[cfg_attr(feature = "std", derive(Debug, Hash, MallocSizeOf))]
 pub struct CandidateCommitments<N = BlockNumber> {
 	/// Messages destined to be interpreted by the Relay chain itself.
 	pub upward_messages: Vec<UpwardMessage>,
@@ -534,6 +534,8 @@ impl CandidateCommitments {
 }
 
 /// A bitfield concerning availability of backed candidates.
+///
+/// Every bit refers to an availability core index.
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
 pub struct AvailabilityBitfield(pub BitVec<bitvec::order::Lsb0, u8>);
 

primitives/src/v1/signed.rs

@@ -41,6 +41,13 @@ use crate::v0::{SigningContext, ValidatorId, ValidatorIndex, ValidatorSignature}
 #[derive(Clone, PartialEq, Eq, RuntimeDebug)]
 pub struct Signed<Payload, RealPayload = Payload>(UncheckedSigned<Payload, RealPayload>);
 
+impl<Payload, RealPayload> Signed<Payload, RealPayload> {
+	/// Convert back to an unchecked type.
+	pub fn into_unchecked(self) -> UncheckedSigned<Payload, RealPayload> {
+		self.0
+	}
+}
+
 /// Unchecked signed data, can be converted to `Signed` by checking the signature.
 #[derive(Clone, PartialEq, Eq, RuntimeDebug, Encode, Decode, TypeInfo)]
 pub struct UncheckedSigned<Payload, RealPayload = Payload> {
@@ -253,6 +260,37 @@ impl<Payload: EncodeAs<RealPayload>, RealPayload: Encode> UncheckedSigned<Payloa
 			Err(())
 		}
 	}
+
+	/// Sign this payload with the given context and pair.
+	///
+	/// # WARNING
+	/// Only meant for usage in tests and and benchmarks.
+	pub fn benchmark_sign<H: Encode>(
+		public: &crate::v0::ValidatorId,
+		payload: Payload,
+		context: &SigningContext<H>,
+		validator_index: ValidatorIndex,
+	) -> Self {
+		use application_crypto::RuntimeAppPublic;
+		let data = Self::payload_data(&payload, context);
+		let signature = public.sign(&data).unwrap();
+
+		Self { payload, validator_index, signature, real_payload: sp_std::marker::PhantomData }
+	}
+
+	/// Immutably access the signature.
+	///
+	/// # WARNING
+	/// Only meant for usage in tests and and benchmarks.
+	pub fn benchmark_signature(&self) -> ValidatorSignature {
+		self.signature.clone()
+	}
+
+	/// Set the signature. Only should be used for creating testing mocks.
+	#[cfg(feature = "std")]
+	pub fn set_signature(&mut self, signature: ValidatorSignature) {
+		self.signature = signature
+	}
 }
 
 impl<Payload, RealPayload> From<Signed<Payload, RealPayload>>

roadmap/implementers-guide/src/runtime/inclusion.md

@@ -45,13 +45,38 @@ PendingAvailabilityCommitments: map ParaId => CandidateCommitments;
 All failed checks should lead to an unrecoverable error making the block invalid.
 
 * `process_bitfields(expected_bits, Bitfields, core_lookup: Fn(CoreIndex) -> Option<ParaId>)`:
-  1. check that there is at most 1 bitfield per validator and that the number of bits in each bitfield is equal to `expected_bits`.
-  1. check that there are no duplicates
-  1. check all validator signatures.
+  1. call `sanitize_bitfields<true>` and use the sanitized `signed_bitfields` from now on.
+  1. call `sanitize_backed_candidates<true>` and use the sanitized `backed_candidates` from now on.
   1. apply each bit of bitfield to the corresponding pending candidate. looking up parathread cores using the `core_lookup`. Disregard bitfields that have a `1` bit for any free cores.
   1. For each applied bit of each availability-bitfield, set the bit for the validator in the `CandidatePendingAvailability`'s `availability_votes` bitfield. Track all candidates that now have >2/3 of bits set in their `availability_votes`. These candidates are now available and can be enacted.
   1. For all now-available candidates, invoke the `enact_candidate` routine with the candidate and relay-parent number.
   1. Return a list of `(CoreIndex, CandidateHash)` from freed cores consisting of the cores where candidates have become available.
+* `sanitize_bitfields<T: crate::inclusion::Config, const CHECK_SIGS: bool>(
+    unchecked_bitfields: UncheckedSignedAvailabilityBitfields,
+    disputed_bitfield: DisputedBitfield,
+    expected_bits: usize,
+    parent_hash: T::Hash,
+    session_index: SessionIndex,
+    validators: &[ValidatorId],
+  )`:
+  1. check that `disputed_bitfield` has the same number of bits as the `expected_bits`, iff not return early with an empty vec.
+  1. each of the below checks is for each bitfield. If a check does not pass the bitfield will be skipped.
+  1. check that there are no bits set that reference a disputed candidate.
+  1. check that the number of bits is equal to `expected_bits`.
+  1. check that the validator index is strictly increasing (and thus also unique).
+  1. check that the validator bit index is not out of bounds.
+  1. check the validators signature, iff `CHECK_SIGS=true`.
+
+* `sanitize_backed_candidates<T: crate::inclusion::Config, F: Fn(CandidateHash) -> bool>(
+    relay_parent: T::Hash,
+    mut backed_candidates: Vec<BackedCandidate<T::Hash>>,
+    candidate_has_concluded_invalid_dispute: F,
+    scheduled: &[CoreAssignment],
+  ) `
+  1. filter out any backed candidates that have concluded invalid.
+  1. filter out backed candidates that don't have a matching `relay_parent`.
+  1. filters backed candidates whom's paraid was scheduled by means of the provided `scheduled` parameter.
+
 * `process_candidates(parent_storage_root, BackedCandidates, scheduled: Vec<CoreAssignment>, group_validators: Fn(GroupIndex) -> Option<Vec<ValidatorIndex>>)`:
   1. check that each candidate corresponds to a scheduled core and that they are ordered in the same order the cores appear in assignments in `scheduled`.
   1. check that `scheduled` is sorted ascending by `CoreIndex`, without duplicates.
@@ -78,6 +103,7 @@ All failed checks should lead to an unrecoverable error making the block invalid
   1. call `Hrmp::prune_hrmp` with the para id of the candiate and the candidate's `hrmp_watermark`.
   1. call `Hrmp::queue_outbound_hrmp` with the para id of the candidate and the list of horizontal messages taken from the commitment,
   1. Call `Paras::note_new_head` using the `HeadData` from the receipt and `relay_parent_number`.
+
 * `collect_pending`:
 
   ```rust

roadmap/implementers-guide/src/runtime/parainherent.md

@@ -29,10 +29,19 @@ OnChainVotes: Option<ScrapedOnChainVotes>,
 
 ## Entry Points
 
-* `enter`: This entry-point accepts three parameters: The relay-chain parent block header, [`Bitfields`](../types/availability.md#signed-availability-bitfield) and [`BackedCandidates`](../types/backing.md#backed-candidate).
-    1. Hash the parent header and make sure that it corresponds to the block hash of the parent (tracked by the `frame_system` FRAME module),
+* `enter`: This entry-point accepts one parameter: [`ParaInherentData`](../types/runtime.md#ParaInherentData).
+    1. Ensure the origin is none.
+    1. Ensure `Included` is set as `None`.
+    1. Set `Included` as `Some`.
+    1. Unpack `ParachainsInherentData` into `signed_bitfields`, `backed_candidates`, `parent_header`, and `disputes`.
+    1. Hash the parent header and make sure that it corresponds to the block hash of the parent (tracked by the `frame_system` FRAME module).
+    1. Calculate the `candidate_weight`, `bitfields_weight`, and `disputes_weight`.
+    1. If the sum of `candidate_weight`, `bitfields_weight`, and `disputes_weight` is greater than the max block weight we do the following with the goal of prioritizing the inclusion of disputes without making it game-able by block authors:
+      1. clear `bitfields` and set `bitfields_weight` equal to 0.
+      1. clear `backed_candidates` and set `candidate_weight` equal to 0.
+      1. invoke `limit_disputes` on the `disputes` with the max block weight iff the disputes weight is greater than the max block weight.
     1. Invoke `Disputes::provide_multi_dispute_data`.
-    1. If `Disputes::is_frozen`, return and set `Included` to `Some(())`.
+    1. If `Disputes::is_frozen`, return.
     1. If there are any concluded disputes from the current session, invoke `Inclusion::collect_disputed` with the disputed candidates. Annotate each returned core with `FreedReason::Concluded`, sort them, and invoke `Scheduler::free_cores` with them.
     1. The `Bitfields` are first forwarded to the `Inclusion::process_bitfields` routine, returning a set included candidates and the respective freed cores. Provide the number of availability cores (`Scheduler::availability_cores().len()`) as the expected number of bits and a `Scheduler::core_para` as a core-lookup to the `process_bitfields` routine. Annotate each of these freed cores with `FreedReason::Concluded`.
     1. For each freed candidate from the `Inclusion::process_bitfields` call, invoke `Disputes::note_included(current_session, candidate)`.
@@ -48,3 +57,32 @@ OnChainVotes: Option<ScrapedOnChainVotes>,
     1. Call `Scheduler::occupied` using the `occupied` core indices of the returned  above, first sorting the list of assigned core indices.
     1. Call the `Ump::process_pending_upward_messages` routine to execute all messages in upward dispatch queues.
     1. If all of the above succeeds, set `Included` to `Some(())`.
+
+
+* `create_inherent`: This entry-point accepts one parameter: `InherentData`.
+  1. Invoke [`create_inherent_inner(InherentData)`](#routines), the unit testable logic for filtering and sanitzing the inherent data used when invoking `enter`. Save the result as `inherent_data`.
+  1. If the `inherent_data` is an `Err` variant, return the `enter` call signature with all inherent data cleared else return the `enter` call signature with `inherent_data` passed in as the `data` param.
+
+# Routines
+
+* `create_inherent_inner(data: &InherentData) -> Option<ParachainsInherentData<T::Header>>`
+  1. Unpack `InherentData` into its parts, `bitfields`, `backed_candidates`, `disputes` and the `parent_header`. If data cannot be unpacked return `None`.
+  1. Hash the `parent_header` and make sure that it corresponds to the block hash of the parent (tracked by the `frame_system` FRAME module).
+  1. Invoke `Disputes::filter_multi_dispute_data` to remove duplicates et al from `disputes`.
+  1. Run the following within a  `with_transaction` closure to avoid side effects (we are essentially replicating the logic that would otherwise happen within `enter` so we can get the filtered bitfields and the `concluded_invalid_disputes` + `scheduled` to use in filtering the `backed_candidates`.):
+    1. Invoke `Disputes::provide_multi_dispute_data`.
+    1. Collect `current_concluded_invalid_disputes`, the disputed candidate hashes from the current session that have concluded invalid.
+    1. Collect `concluded_invalid_disputes`, the disputed candidate hashes from the given `backed_candidates`.
+    1. Invoke `Inclusion::collect_disputed` with the newly disputed candidates. Annotate each returned core with `FreedReason::Concluded`, sort them, and invoke `Scheduler::free_cores` with them.
+    1. Collect filtered `bitfields` by invoking [`sanitize_bitfields<false>`](inclusion.md#Routines).
+    1. Collect `freed_concluded` by invoking `update_pending_availability_and_get_freed_cores` on the filtered bitfields.
+    1. Collect all `freed` cores by invoking `collect_all_freed_cores` on `freed_concluding`.
+    1. Invoke `scheduler::Pallet<T>>::clear()`.
+    1. Invoke `scheduler::Pallet<T>>::schedule` with `freed` and the current block number to create the same schedule of the cores that `enter` will create.
+    1. Read the new `<scheduler::Pallet<T>>::scheduled()` into `schedule`.
+    1. From the `with_transaction` closure return `concluded_invalid_disputes`, `bitfields`, and `scheduled`.
+  1. Invoke `sanitize_backed_candidates` using the `scheduled` return from the `with_transaction` and pass the closure `|candidate_hash: CandidateHash| -> bool { DisputesHandler::concluded_invalid(current_session, candidate_hash) }` for the param `candidate_has_concluded_invalid_dispute`.
+  1. create a `rng` from `rand_chacha::ChaChaRng::from_seed(compute_entropy::<T>(parent_hash))`.
+  1. Invoke `limit_disputes` with the max block weight and `rng`, storing the returned weigh in `remaining_weight`.
+  1. Fill up the remaining of the block weight with backed candidates and bitfields by invoking `apply_weight_limit` with `remaining_weigh` and `rng`.
+  1. Return `Some(ParachainsInherentData { bitfields, backed_candidates, disputes, parent_header }`.

roadmap/implementers-guide/src/types/runtime.md

@@ -116,15 +116,17 @@ struct HostConfiguration {
 
 Inherent data passed to a runtime entry-point for the advancement of parachain consensus.
 
-This contains 3 pieces of data:
+This contains 4 pieces of data:
 1. [`Bitfields`](availability.md#signed-availability-bitfield)
 2. [`BackedCandidates`](backing.md#backed-candidate)
 3. [`MultiDisputeStatementSet`](disputes.md#multidisputestatementset)
+4. `Header`
 
 ```rust
 struct ParaInherentData {
 	bitfields: Bitfields,
 	backed_candidates: BackedCandidates,
 	dispute_statements: MultiDisputeStatementSet,
+  parent_header: Header
 }
 ```

runtime/kusama/Cargo.toml

@@ -214,6 +214,7 @@ runtime-benchmarks = [
 	"xcm-builder/runtime-benchmarks",
 	"frame-election-provider-support/runtime-benchmarks",
 	"pallet-bags-list/runtime-benchmarks",
+	"runtime-parachains/runtime-benchmarks",
 ]
 try-runtime = [
 	"frame-executive/try-runtime",

runtime/kusama/src/lib.rs

@@ -1201,7 +1201,9 @@ impl parachains_hrmp::Config for Runtime {
 	type Currency = Balances;
 }
 
-impl parachains_paras_inherent::Config for Runtime {}
+impl parachains_paras_inherent::Config for Runtime {
+	type WeightInfo = weights::runtime_parachains_paras_inherent::WeightInfo<Runtime>;
+}
 
 impl parachains_scheduler::Config for Runtime {}
 
@@ -1933,6 +1935,7 @@ sp_api::impl_runtime_apis! {
 			list_benchmark!(list, extra, runtime_common::paras_registrar, Registrar);
 			list_benchmark!(list, extra, runtime_parachains::configuration, Configuration);
 			list_benchmark!(list, extra, runtime_parachains::initializer, Initializer);
+			list_benchmark!(list, extra, runtime_parachains::paras_inherent, ParaInherent);
 			list_benchmark!(list, extra, runtime_parachains::paras, Paras);
 			// Substrate
 			list_benchmark!(list, extra, pallet_bags_list, BagsList);
@@ -2010,6 +2013,7 @@ sp_api::impl_runtime_apis! {
 			add_benchmark!(params, batches, runtime_common::paras_registrar, Registrar);
 			add_benchmark!(params, batches, runtime_parachains::configuration, Configuration);
 			add_benchmark!(params, batches, runtime_parachains::initializer, Initializer);
+			add_benchmark!(params, batches, runtime_parachains::paras_inherent, ParaInherent);
 			add_benchmark!(params, batches, runtime_parachains::paras, Paras);
 			// Substrate
 			add_benchmark!(params, batches, pallet_balances, Balances);

runtime/kusama/src/weights/mod.rs

@@ -47,3 +47,4 @@ pub mod runtime_common_slots;
 pub mod runtime_parachains_configuration;
 pub mod runtime_parachains_initializer;
 pub mod runtime_parachains_paras;
+pub mod runtime_parachains_paras_inherent;