Feature Flagging Consensus Hook Type Parameter #2911

BradleyOlson64 · 2023-07-19T22:25:14Z

This is a first try for me using feature flags. Hopefully somewhat correct :)

rphmeier · 2023-07-19T22:47:08Z

parachain-template/runtime/src/lib.rs

@@ -225,11 +225,14 @@ const MAXIMUM_BLOCK_WEIGHT: Weight = Weight::from_parts(

 /// Maximum number of blocks simultaneously accepted by the Runtime, not yet included
 /// into the relay chain.
+#[cfg(feature = "parameterized-consensus-hook")]


we don't actually need feature-flags in the parachain-template, as it's meant to be example code. So it should use the most recently available logic.

rphmeier · 2023-07-19T22:47:44Z

parachains/runtimes/assets/asset-hub-kusama/src/lib.rs

@@ -47,7 +47,9 @@ use sp_version::NativeVersion;
 use sp_version::RuntimeVersion;

 use codec::{Decode, Encode, MaxEncodedLen};
-use constants::{consensus::*, currency::*, fee::WeightToFee};
+#[cfg(feature = "parameterized-consensus-hook")]


we don't need the feature flag in system-parachain runtimes either, as they're binaries, not libraries.

rphmeier · 2023-07-19T22:48:48Z

test/runtime/src/lib.rs

@@ -286,6 +286,7 @@ impl cumulus_pallet_parachain_system::Config for Runtime {
 	type XcmpMessageHandler = ();
 	type ReservedXcmpWeight = ();
 	type CheckAssociatedRelayNumber = cumulus_pallet_parachain_system::AnyRelayNumber;
+	#[cfg(feature = "parameterized-consensus-hook")]


likewise here - it seems fine for the test runtime (which is effectively a binary crate used internally, not a library) to explicitly use the RequireParentIncluded

Which way is best to get this to compile without the feature flag here?

I expect some change is needed around line 215 of pallets/parachain-system/src/lib.rs such that it can still accept a ConsensusHook type even without the feature flag enabled. Just haven't seen an example of what that would look like.

Can we just have it unconditionally enable the crate feature in its Cargo.toml?

That does make sense 👍 Will be back on to try it this evening.

Robert probably meant doing

cumulus-pallet-parachain-system = { path = "../../../../pallets/parachain-system", default-features = false, features = [ "parameterized-consensus-hook", ] }

instead of introducing a feature to test-crates.

pallets/parachain-system/src/lib.rs

rphmeier

Looks good! This looks properly backwards-compatible.

Co-authored-by: asynchronous rob <rphmeier@gmail.com>

slumber · 2023-07-21T08:16:16Z

pallets/parachain-system/Cargo.toml

@@ -77,3 +77,5 @@ runtime-benchmarks = [
 ]

 try-runtime = ["frame-support/try-runtime"]
+
+parameterized-consensus-hook = []


missing endlines though

* Update substrate & polkadot * min changes to make async backing compile * (async backing) parachain-system: track limitations for unincluded blocks (#2438) * unincluded segment draft * read para head from storage proof * read_para_head -> read_included_para_head * Provide pub interface * add errors * fix unincluded segment update * BlockTracker -> Ancestor * add a dmp limit * Read para head depending on the storage switch * doc comments * storage items docs * add a sanity check on block initialize * Check watermark * append to the segment on block finalize * Move segment update into set_validation_data * Resolve para head todo * option watermark * fix comment * Drop dmq check * fix weight * doc-comments on inherent invariant * Remove TODO * add todo * primitives tests * pallet tests * doc comments * refactor unincluded segment length into a ConsensusHook (#2501) * refactor unincluded segment length into a ConsensusHook * add docs * refactor bandwidth_out calculation Co-authored-by: Chris Sosnin <48099298+slumber@users.noreply.github.com> * test for limits from impl * fmt * make tests compile * update comment * uncomment test * fix collator test by adding parent to state proof * patch HRMP watermark rules for unincluded segment * get consensus-common tests to pass, using unincluded segment * fix unincluded segment tests * get all tests passing * fmt * rustdoc CI * aura-ext: limit the number of authored blocks per slot (#2551) * aura_ext consensus hook * reverse dependency * include weight into hook * fix tests * remove stray println Co-authored-by: Chris Sosnin <48099298+slumber@users.noreply.github.com> * fix test warning * fix doc link --------- Co-authored-by: Chris Sosnin <48099298+slumber@users.noreply.github.com> Co-authored-by: Chris Sosnin <chris125_@live.com> * parachain-system: ignore go ahead signal once upgrade is processed (#2594) * handle goahead signal for unincluded segment * doc comment * add test * parachain-system: drop processed messages from inherent data (#2590) * implement `drop_processed_messages` * drop messages based on relay parent number * adjust tests * drop changes to mqc * fix comment * drop test * drop more dead code * clippy * aura-ext: check slot in consensus hook and remove all `CheckInherents` logic (#2658) * aura-ext: check slot in consensus hook * convert relay chain slot * Make relay chain slot duration generic * use fixed velocity hook for pallets with aura * purge timestamp inherent * fix warning * adjust runtime tests * fix slots in tests * Make `xcm-emulator` test pass for new consensus hook (#2722) * add pallets on_initialize * tests pass * add AuraExt on_init * ".git/.scripts/commands/fmt/fmt.sh" --------- Co-authored-by: command-bot <> --------- Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com> * update polkadot git refs * CollationGenerationConfig closure is now optional (#2772) * CollationGenerationConfig closure is now optional * fix test * propagate network-protocol-staging feature (#2899) * Feature Flagging Consensus Hook Type Parameter (#2911) * First pass * fmt * Added as default feature in tomls * Changed to direct dependency feature * Dealing with clippy error * Update pallets/parachain-system/src/lib.rs Co-authored-by: asynchronous rob <rphmeier@gmail.com> --------- Co-authored-by: asynchronous rob <rphmeier@gmail.com> * fmt * bump deps and remove warning * parachain-system: update RelevantMessagingState according to the unincluded segment (#2948) * mostly address 2471 with a bug introduced * adjust relevant messaging state after computing total * fmt * max -> min * fix test implementation of xcmp source * add test * fix test message sending logic * fix + test * add more to unincluded segment test * fmt --------- Co-authored-by: Chris Sosnin <chris125_@live.com> * Integrate new Aura / Parachain Consensus Logic in Parachain-Template / Polkadot-Parachain (#2864) * add a comment * refactor client/service utilities * deprecate start_collator * update parachain-template * update test-service in the same way * update polkadot-parachain crate * fmt * wire up new SubmitCollation message * some runtime utilities for implementing unincluded segment runtime APIs * allow parachains to configure their level of sybil-resistance when starting the network * make aura-ext compile * update to specify sybil resistance levels * fmt * specify relay chain slot duration in milliseconds * update Aura to explicitly produce Send futures also, make relay_chain_slot_duration a Duration * add authoring duration to basic collator and document params * integrate new basic collator into parachain-template * remove assert_send used for testing * basic-aura: only author when parent included * update polkadot-parachain-bin * fmt * some fixes * fixes * add a RelayNumberMonotonicallyIncreases * add a utility function for initializing subsystems * some logging for timestamp adjustment * fmt * some fixes for lookahead collator * add a log * update `find_potential_parents` to account for sessions * bound the loop * restore & deprecate old start_collator and start_full_node functions. * remove unnecessary await calls * fix warning * clippy * more clippy * remove unneeded logic * ci * update comment Co-authored-by: Marcin S. <marcin@bytedude.com> * (async backing) restore `CheckInherents` for backwards-compatibility (#2977) * bring back timestamp * Restore CheckInherents * revert to empty CheckInherents * make CheckInherents optional * attempt * properly end system blocks * add some more comments * ignore failing system parachain tests * update refs after main feature branch merge * comment out the offending tests because CI runs ignored tests * fix warnings * fmt * revert to polkadot master * cargo update -p polkadot-primitives -p sp-io --------- Co-authored-by: asynchronous rob <rphmeier@gmail.com> Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com> Co-authored-by: Bradley Olson <34992650+BradleyOlson64@users.noreply.github.com> Co-authored-by: Marcin S. <marcin@bytedude.com> Co-authored-by: eskimor <eskimor@users.noreply.github.com> Co-authored-by: Andronik <write@reusable.software>

First pass

5f0980a

BradleyOlson64 requested review from rphmeier and slumber July 19, 2023 22:25

BradleyOlson64 self-assigned this Jul 19, 2023

fmt

e155997

rphmeier reviewed Jul 19, 2023

View reviewed changes

paritytech-ci requested review from a team July 19, 2023 22:47

rphmeier reviewed Jul 19, 2023

View reviewed changes

paritytech-ci requested review from a team July 19, 2023 22:48

rphmeier reviewed Jul 19, 2023

View reviewed changes

paritytech-ci requested review from a team July 19, 2023 22:49

BradleyOlson64 added 3 commits July 20, 2023 00:36

Added as default feature in tomls

7afe68b

Changed to direct dependency feature

d1238b0

Dealing with clippy error

6ff8923

rphmeier reviewed Jul 20, 2023

View reviewed changes

pallets/parachain-system/src/lib.rs Show resolved Hide resolved

rphmeier approved these changes Jul 20, 2023

View reviewed changes

Update pallets/parachain-system/src/lib.rs

eec072d

Co-authored-by: asynchronous rob <rphmeier@gmail.com>

rphmeier merged commit 51d9e9d into slumber-async-backing-feature Jul 20, 2023
27 of 29 checks passed

rphmeier deleted the brad-consensus-hook-feature branch July 20, 2023 22:57

BradleyOlson64 mentioned this pull request Jul 20, 2023

Consensus hook should be feature-flagged #2905

Closed

slumber reviewed Jul 21, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature Flagging Consensus Hook Type Parameter #2911

Feature Flagging Consensus Hook Type Parameter #2911

BradleyOlson64 commented Jul 19, 2023

rphmeier Jul 19, 2023

rphmeier Jul 19, 2023

rphmeier Jul 19, 2023

BradleyOlson64 Jul 20, 2023

rphmeier Jul 20, 2023

BradleyOlson64 Jul 20, 2023

slumber Jul 20, 2023 •

edited

rphmeier left a comment

slumber Jul 21, 2023

Feature Flagging Consensus Hook Type Parameter #2911

Feature Flagging Consensus Hook Type Parameter #2911

Conversation

BradleyOlson64 commented Jul 19, 2023

rphmeier Jul 19, 2023

Choose a reason for hiding this comment

rphmeier Jul 19, 2023

Choose a reason for hiding this comment

rphmeier Jul 19, 2023

Choose a reason for hiding this comment

BradleyOlson64 Jul 20, 2023

Choose a reason for hiding this comment

rphmeier Jul 20, 2023

Choose a reason for hiding this comment

BradleyOlson64 Jul 20, 2023

Choose a reason for hiding this comment

slumber Jul 20, 2023 • edited

Choose a reason for hiding this comment

rphmeier left a comment

Choose a reason for hiding this comment

slumber Jul 21, 2023

Choose a reason for hiding this comment

slumber Jul 20, 2023 •

edited