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

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

@@ -51,31 +51,31 @@ All failed checks should lead to an unrecoverable error making the block invalid
   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<const EARLY_RETURN>(
-    usab: UncheckedSignedAvailabilityBitfields,
-    db: DisputedBitfield,
+* `sanitize_bitfields<T: crate::inclusion::Config, const CHECK_SIGS: bool>(
+    unchecked_bitfields: UncheckedSignedAvailabilityBitfields,
+    disputed_bitfield: DisputedBitfield,
     expected_bits: usize,
-    parent_hash: Hash,
+    parent_hash: T::Hash,
     session_index: SessionIndex,
-    validators: &[ValidatorId]
+    validators: &[ValidatorId],
   )`:
-  1. if `EARLY_RETURN` is `true`, return an error when encountering a bitfield that does not pass the checks, if `false`, drop the bitfield from the set that will be returned.
-  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 that the validator bit index is not out of bounds
-  1. check all validator signatures, iff `EARLY_RETURN=true`, since in the other case, checking is supposed to be done before the call
-  1. check that there are no bits set that reference a disputed candidate
-
-* `sanitize_backed_candidates<const EARLY_RETURN: bool>(
-	relay_parent: Hash,
-	mut backed_candidates: Vec<BackedCandidate>,
-	candidate_has_concluded_invalid_dispute: Fn(CandidateHash) -> bool,
-	scheduled: &[CoreAssignment],
-)`
-  1. if `EARLY_RETURN` is `true`, return an error when encountering a backed candidates that does not pass the checks, if `false`, drop the backed candidates from the set that will be returned.
-  1. check all backed candidates have no concluded invalid dispute by means of the provided closure `candidate_has_concluded_invalid_dispute`
-  1. check all backed candidates have the matching `relay_parent`
-  1. check all backed candidates paraid was scheduled by means of the provided `scheduled` parameter
+  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`.
@@ -104,8 +104,6 @@ All failed checks should lead to an unrecoverable error making the block invalid
   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`.
 
-* `fn sanitize_bitfields`
-
 * `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)`.
@@ -51,19 +60,29 @@ OnChainVotes: Option<ScrapedOnChainVotes>,
 
 
 * `create_inherent`: This entry-point accepts one parameter: `InherentData`.
-  1. Unpack `InherentData` into its parts, `bitfields`, `backed_candidates`, `disputes` and the `parent_header`.
-  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 [`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:
+  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 the newly concluded disputes as `current_concluded_invalid_disputes`.
-    1. If there are any concluded disputes from the current session, 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 the concluded invalid disputes in the current session as `conlcuded_invalid_disputes`.
-    1. Return `TransactionOutcome::Rollback(freed_cores, concluded_invalid_disputes)`.
-  1. Call `sanitize_bitfields<false>` and only use the sanitized set of bitfields afterwards.
-  1. Call `sanitize_backed_candidates<false>`.
-  1. Collect entropy based on `CurrentBlockRandomness::random`.
-  1. Call `apply_weight_limit` to utilize the block as well as possible, with a randomized heuristic.
-  1. Re-create a `InherentData` from the sanitized and weight bounded information.
-  1. Call `Self::enter` which now should not error anymore, but we do this call anyways for now.
-  1. Return `Call::enter { .. }`.
+    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/parachains/src/paras_inherent.rs

@@ -280,7 +280,8 @@ pub mod pallet {
 		}
 	}
 
-	/// Collect all freed cores based on storage data.
+	/// Collect all freed cores based on storage data. (i.e. append cores freed from timeouts to
+	/// the given `freed_concluded`).
 	///
 	/// The parameter `freed_concluded` contains all core indicies that became
 	/// free due to candidate that became available.
@@ -547,6 +548,8 @@ impl<T: Config> Pallet<T> {
 					});
 
 				// current concluded invalid disputes, only including the current block's votes
+				// TODO why does this say "only including the current block's votes"? This can include
+				// remote disputes, right?
 				let current_concluded_invalid_disputes = disputes
 					.iter()
 					.filter(|dss| dss.session == current_session)