Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Commit 59072a3

Browse files
eskimoreskimor
andauthored
Unify code paths of create_inherent and enter (#7137)
* Remove redundant enter call. * Remove optionality in dispute signature checking * Make enter_inner and create_inherent the same. * Remove redundant metric. * Unification: enter and create_inherent. * Remove `enter_inner` function. * Remove dead code. * Remove redundant import. * Remove dead code in disputes. * ".git/.scripts/commands/bench/bench.sh" runtime polkadot runtime_parachains::paras_inherent * ".git/.scripts/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent * ".git/.scripts/commands/bench/bench.sh" runtime westend runtime_parachains::paras_inherent * Merge fix. * Fix tests. * Remove obsolete comment. * ".git/.scripts/commands/bench/bench.sh" runtime polkadot runtime_parachains::paras_inherent * ".git/.scripts/commands/bench/bench.sh" runtime westend runtime_parachains::paras_inherent * ".git/.scripts/commands/bench/bench.sh" runtime kusama runtime_parachains::paras_inherent * Review remarks, fixes. * Guide updates. * Fmt. * ".git/.scripts/commands/bench/bench.sh" runtime polkadot runtime_parachains::paras_inherent --------- Co-authored-by: eskimor <eskimor@no-such-url.com> Co-authored-by: command-bot <>
1 parent b6b74fd commit 59072a3

File tree

13 files changed

+642
-1516
lines changed

13 files changed

+642
-1516
lines changed

node/metrics/src/runtime/parachain.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,12 @@ use crate::runtime::RuntimeMetricsProvider;
2121
use primitives::metric_definitions::{
2222
PARACHAIN_CREATE_INHERENT_BITFIELDS_SIGNATURE_CHECKS,
2323
PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED, PARACHAIN_INHERENT_DATA_CANDIDATES_PROCESSED,
24-
PARACHAIN_INHERENT_DATA_DISPUTE_SETS_INCLUDED, PARACHAIN_INHERENT_DATA_DISPUTE_SETS_PROCESSED,
25-
PARACHAIN_INHERENT_DATA_WEIGHT, PARACHAIN_VERIFY_DISPUTE_SIGNATURE,
24+
PARACHAIN_INHERENT_DATA_DISPUTE_SETS_PROCESSED, PARACHAIN_INHERENT_DATA_WEIGHT,
25+
PARACHAIN_VERIFY_DISPUTE_SIGNATURE,
2626
};
2727

2828
/// Register the parachain runtime metrics.
2929
pub fn register_metrics(runtime_metrics_provider: &RuntimeMetricsProvider) {
30-
runtime_metrics_provider.register_counter(PARACHAIN_INHERENT_DATA_DISPUTE_SETS_INCLUDED);
3130
runtime_metrics_provider.register_counter(PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED);
3231

3332
runtime_metrics_provider.register_countervec(PARACHAIN_INHERENT_DATA_WEIGHT);

primitives/src/v5/metrics.rs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -147,45 +147,39 @@ pub mod metric_definitions {
147147
labels: &["when"],
148148
};
149149

150-
/// Counts the number of bitfields processed in `enter_inner`.
150+
/// Counts the number of bitfields processed in `process_inherent_data`.
151151
pub const PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED: CounterDefinition = CounterDefinition {
152152
name: "polkadot_parachain_inherent_data_bitfields_processed",
153-
description: "Counts the number of bitfields processed in `enter_inner`.",
153+
description: "Counts the number of bitfields processed in `process_inherent_data`.",
154154
};
155155

156156
/// Counts the `total`, `sanitized` and `included` number of parachain block candidates
157-
/// in `enter_inner`.
157+
/// in `process_inherent_data`.
158158
pub const PARACHAIN_INHERENT_DATA_CANDIDATES_PROCESSED: CounterVecDefinition =
159159
CounterVecDefinition {
160160
name: "polkadot_parachain_inherent_data_candidates_processed",
161161
description:
162-
"Counts the number of parachain block candidates processed in `enter_inner`.",
162+
"Counts the number of parachain block candidates processed in `process_inherent_data`.",
163163
labels: &["category"],
164164
};
165165

166166
/// Counts the number of `imported`, `current` and `concluded_invalid` dispute statements sets
167-
/// processed in `enter_inner`. The `current` label refers to the disputes statement sets of
167+
/// processed in `process_inherent_data`. The `current` label refers to the disputes statement sets of
168168
/// the current session.
169169
pub const PARACHAIN_INHERENT_DATA_DISPUTE_SETS_PROCESSED: CounterVecDefinition =
170170
CounterVecDefinition {
171171
name: "polkadot_parachain_inherent_data_dispute_sets_processed",
172-
description: "Counts the number of dispute statements sets processed in `enter_inner`.",
173-
labels: &["category"],
174-
};
175-
176-
/// Counts the number of dispute statements sets included in a block in `enter_inner`.
177-
pub const PARACHAIN_INHERENT_DATA_DISPUTE_SETS_INCLUDED: CounterDefinition =
178-
CounterDefinition {
179-
name: "polkadot_parachain_inherent_data_dispute_sets_included",
180172
description:
181-
"Counts the number of dispute statements sets included in a block in `enter_inner`.",
173+
"Counts the number of dispute statements sets processed in `process_inherent_data`.",
174+
labels: &["category"],
182175
};
183176

184-
/// Counts the number of `valid` and `invalid` bitfields signature checked in `enter_inner`.
177+
/// Counts the number of `valid` and `invalid` bitfields signature checked in `process_inherent_data`.
185178
pub const PARACHAIN_CREATE_INHERENT_BITFIELDS_SIGNATURE_CHECKS: CounterVecDefinition =
186179
CounterVecDefinition {
187180
name: "polkadot_parachain_create_inherent_bitfields_signature_checks",
188-
description: "Counts the number of bitfields signature checked in `enter_inner`.",
181+
description:
182+
"Counts the number of bitfields signature checked in `process_inherent_data`.",
189183
labels: &["validity"],
190184
};
191185

Lines changed: 24 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# `ParaInherent`
22

3-
This module is responsible for providing all data given to the runtime by the block author to the various parachains modules. The entry-point is mandatory, in that it must be invoked exactly once within every block, and it is also "inherent", in that it is provided with no origin by the block author. The data within it carries its own authentication; i.e. the data takes the form of signed statements by validators. If any of the steps within fails, the entry-point is considered as having failed and the block will be invalid.
3+
This module is responsible for providing all data given to the runtime by the block author to the various parachains modules. The entry-point is mandatory, in that it must be invoked exactly once within every block, and it is also "inherent", in that it is provided with no origin by the block author. The data within it carries its own authentication; i.e. the data takes the form of signed statements by validators. Invalid data will be filtered and not applied.
44

55
This module does not have the same initialization/finalization concerns as the others, as it only requires that entry points be triggered after all modules have initialized and that finalization happens after entry points are triggered. Both of these are assumptions we have already made about the runtime's order of operations, so this module doesn't need to be initialized or finalized by the `Initializer`.
66

@@ -30,59 +30,28 @@ OnChainVotes: Option<ScrapedOnChainVotes>,
3030
## Entry Points
3131

3232
* `enter`: This entry-point accepts one parameter: [`ParaInherentData`](../types/runtime.md#ParaInherentData).
33-
1. Ensure the origin is none.
34-
1. Ensure `Included` is set as `None`.
35-
1. Set `Included` as `Some`.
36-
1. Unpack `ParachainsInherentData` into `signed_bitfields`, `backed_candidates`, `parent_header`, and `disputes`.
37-
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).
38-
1. Calculate the `candidate_weight`, `bitfields_weight`, and `disputes_weight`.
39-
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:
40-
1. clear `bitfields` and set `bitfields_weight` equal to 0.
41-
1. clear `backed_candidates` and set `candidate_weight` equal to 0.
42-
1. invoke `limit_disputes` on the `disputes` with the max block weight iff the disputes weight is greater than the max block weight.
43-
1. Invoke `Disputes::provide_multi_dispute_data`.
44-
1. If `Disputes::is_frozen`, return.
45-
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.
46-
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`.
47-
1. For each freed candidate from the `Inclusion::process_bitfields` call, invoke `Disputes::note_included(current_session, candidate)`.
48-
1. If `Scheduler::availability_timeout_predicate` is `Some`, invoke `Inclusion::collect_pending` using it and annotate each of those freed cores with `FreedReason::TimedOut`.
49-
1. Combine and sort the the bitfield-freed cores and the timed-out cores.
50-
1. Invoke `Scheduler::clear`
51-
1. Invoke `Scheduler::schedule(freed_cores, System::current_block())`
52-
1. Extract `parent_storage_root` from the parent header,
53-
1. If `Disputes::concluded_invalid(current_session, candidate)` is true for any of the `backed_candidates`, fail.
54-
1. Invoke the `Inclusion::process_candidates` routine with the parameters `(parent_storage_root, backed_candidates, Scheduler::scheduled(), Scheduler::group_validators)`.
55-
1. Deconstruct the returned `ProcessedCandidates` value into `occupied` core indices, and backing validators by candidate `backing_validators_per_candidate` represented by `Vec<(CandidateReceipt, Vec<(ValidatorIndex, ValidityAttestation)>)>`.
56-
1. Set `OnChainVotes` to `ScrapedOnChainVotes`, based on the `current_session`, concluded `disputes`, and `backing_validators_per_candidate`.
57-
1. Call `Scheduler::occupied` using the `occupied` core indices of the returned above, first sorting the list of assigned core indices.
58-
1. Call the `Ump::process_pending_upward_messages` routine to execute all messages in upward dispatch queues.
59-
1. If all of the above succeeds, set `Included` to `Some(())`.
33+
* `create_inherent`: This entry-point accepts one parameter: `InherentData`.
6034

35+
Both entry points share mostly the same code. `create_inherent` will
36+
meaningfully limit inherent data to adhere to the weight limit, in addition to
37+
sanitizing any inputs and filtering out invalid data. Conceptually it is part of
38+
the block production. The `enter` call on the other hand is part of block import
39+
and consumes/imports the data previously produced by `create_inherent`.
40+
41+
In practice both calls process inherent data and apply it to the state. Block
42+
production and block import should arrive at the same new state. Hence we re-use
43+
the same logic to ensure this is the case.
44+
45+
The only real difference between the two is, that on `create_inherent` we
46+
actually need the processed and filtered inherent data to build the block, while
47+
on `enter` the processed data should for one be identical to the incoming
48+
inherent data (assuming honest block producers) and second it is irrelevant, as
49+
we are not building a block but just processing it, so the processed inherent
50+
data is simply dropped.
51+
52+
This also means that the `enter` function keeps data around for no good reason.
53+
This seems acceptable though as the size of a block is rather limited.
54+
Nevertheless if we ever wanted to optimize this we can easily implement an
55+
inherent collector that has two implementations, where one clones and stores the
56+
data and the other just passes it on.
6157

62-
* `create_inherent`: This entry-point accepts one parameter: `InherentData`.
63-
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`.
64-
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.
65-
66-
# Routines
67-
68-
* `create_inherent_inner(data: &InherentData) -> Option<ParachainsInherentData<T::Header>>`
69-
1. Unpack `InherentData` into its parts, `bitfields`, `backed_candidates`, `disputes` and the `parent_header`. If data cannot be unpacked return `None`.
70-
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).
71-
1. Invoke `Disputes::filter_multi_dispute_data` to remove duplicates et al from `disputes`.
72-
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`.):
73-
1. Invoke `Disputes::provide_multi_dispute_data`.
74-
1. Collect `current_concluded_invalid_disputes`, the disputed candidate hashes from the current session that have concluded invalid.
75-
1. Collect `concluded_invalid_disputes`, the disputed candidate hashes from the given `backed_candidates`.
76-
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.
77-
1. Collect filtered `bitfields` by invoking [`sanitize_bitfields<false>`](inclusion.md#Routines).
78-
1. Collect `freed_concluded` by invoking `update_pending_availability_and_get_freed_cores` on the filtered bitfields.
79-
1. Collect all `freed` cores by invoking `collect_all_freed_cores` on `freed_concluding`.
80-
1. Invoke `scheduler::Pallet<T>>::clear()`.
81-
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.
82-
1. Read the new `<scheduler::Pallet<T>>::scheduled()` into `schedule`.
83-
1. From the `with_transaction` closure return `concluded_invalid_disputes`, `bitfields`, and `scheduled`.
84-
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`.
85-
1. create a `rng` from `rand_chacha::ChaChaRng::from_seed(compute_entropy::<T>(parent_hash))`.
86-
1. Invoke `limit_disputes` with the max block weight and `rng`, storing the returned weigh in `remaining_weight`.
87-
1. Fill up the remaining of the block weight with backed candidates and bitfields by invoking `apply_weight_limit` with `remaining_weigh` and `rng`.
88-
1. Return `Some(ParachainsInherentData { bitfields, backed_candidates, disputes, parent_header }`.

0 commit comments

Comments
 (0)