From e12bc3134d5ad891ac21ec6538cffdae9d1fd512 Mon Sep 17 00:00:00 2001 From: Zeke Mostov Date: Tue, 16 Nov 2021 16:15:40 +0100 Subject: [PATCH] Update implementors guide with `sanitize_*` & `enter` (#4294) --- .../src/runtime/inclusion.md | 46 ++++++++-------- .../src/runtime/parainherent.md | 53 +++++++++++++------ .../implementers-guide/src/types/runtime.md | 4 +- runtime/parachains/src/paras_inherent.rs | 5 +- 4 files changed, 65 insertions(+), 43 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/inclusion.md b/roadmap/implementers-guide/src/runtime/inclusion.md index 52e09fd60f9f..84b513cb985c 100644 --- a/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/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( - usab: UncheckedSignedAvailabilityBitfields, - db: DisputedBitfield, +* `sanitize_bitfields( + 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( - relay_parent: Hash, - mut backed_candidates: Vec, - 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 bool>( + relay_parent: T::Hash, + mut backed_candidates: Vec>, + 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, group_validators: Fn(GroupIndex) -> Option>)`: 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 diff --git a/roadmap/implementers-guide/src/runtime/parainherent.md b/roadmap/implementers-guide/src/runtime/parainherent.md index 82804cf1cd95..dd67f9f108f8 100644 --- a/roadmap/implementers-guide/src/runtime/parainherent.md +++ b/roadmap/implementers-guide/src/runtime/parainherent.md @@ -29,10 +29,19 @@ OnChainVotes: Option, ## 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, * `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>` + 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` and only use the sanitized set of bitfields afterwards. - 1. Call `sanitize_backed_candidates`. - 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 { .. }`. \ No newline at end of file + 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`](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>::clear()`. + 1. Invoke `scheduler::Pallet>::schedule` with `freed` and the current block number to create the same schedule of the cores that `enter` will create. + 1. Read the new `>::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::(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 }`. diff --git a/roadmap/implementers-guide/src/types/runtime.md b/roadmap/implementers-guide/src/types/runtime.md index 5749aeca866a..b8eef8f11204 100644 --- a/roadmap/implementers-guide/src/types/runtime.md +++ b/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 } ``` diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index d5655f8203cd..4c683d037cf0 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/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 Pallet { }); // 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)