aura-ext: check slot in consensus hook and remove all `CheckInherents` logic by slumber · Pull Request #2658 · paritytech/cumulus

slumber · 2023-05-30T19:29:15Z

Resolves #2588

This PR also removes the CheckInherents functionality used during Cumulus block execution, as that is now replaced by the ConsensusHook interface of parachain_system.

slumber · 2023-05-31T13:04:36Z

pallets/aura-ext/src/consensus_hook.rs

+where
+	<T as pallet_timestamp::Config>::Moment: Into<u64>,


Aura::slot_duration returns T::Moment with a definition

type Moment: Parameter + Default + AtLeast32Bit // From<u16> + From<u32> + Scale<Self::BlockNumber, Output = Self::Moment> + Copy + MaxEncodedLen + scale_info::StaticTypeInfo;

And SlotDuration cannot be built out of it.

There are workarounds - what have you tried?

Couldn't come up with any other way. Ideally, I would expect Aura::slot_duration to return SlotDuration.
As you can see from the type definition, it's not convertible to any numeric primitive, so we have to enforce it in boundary.

It's reasonable to enforce it in the boundary as long as that aligns with the instantiations that are typically used in runtime declarations. Seems like a good solution to me.

paritytech-cicd-pr · 2023-06-06T14:30:59Z

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2951187

slumber · 2023-06-06T16:05:29Z

pallets/aura-ext/src/consensus_hook.rs

+		let (slot, authored) = pallet::Pallet::<T>::slot_info()
 			.expect("slot info is inserted on block initialization");


@rphmeier
Now that integration tests are failing due to this panic, I wonder if it's OK to optionally verify the slot and skip it if the value is not present?

I would guess that it's most likely the case that blocks built in tests don't contain the pre-runtime digest which sets the slot in pallet_aura and that they need to.

* add pallets on_initialize * tests pass * add AuraExt on_init * ".git/.scripts/commands/fmt/fmt.sh" --------- Co-authored-by: command-bot <>

BradleyOlson64 · 2023-06-22T21:45:57Z

pallets/aura-ext/src/consensus_hook.rs

+	fn on_state_proof(state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) {
 		// Ensure velocity is non-zero.
 		let velocity = V.max(1);
+		let relay_chain_slot = state_proof.read_slot().expect("failed to read relay chain slot");


What happens here if the collator isn't up to date with the latest relay block? I think a "slot number mismatch" would occur.

Right now it seems that collation is still triggered by new incoming relay blocks, so the scenario I described won't occur. But my understanding is that we will want to unteather these.

I believe the state proof comes from the relay parent the para is building on top of.

That's right - the parachain has no information about what the latest relay-parent is, other than what the collator gives it. But validators will only accept blocks with recent relay-parents.

BradleyOlson64

Nice 👍

* 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>

aura-ext: check slot in consensus hook

9751951

slumber requested a review from rphmeier May 30, 2023 19:29

convert relay chain slot

3bf8776

slumber commented May 31, 2023

View reviewed changes

slumber added 3 commits June 1, 2023 19:05

Make relay chain slot duration generic

10b9a81

use fixed velocity hook for pallets with aura

68eb7e1

purge timestamp inherent

1dcf3f0

slumber marked this pull request as ready for review June 1, 2023 18:31

slumber added 3 commits June 2, 2023 15:50

fix warning

bc8e7bb

adjust runtime tests

2a10bdf

fix slots in tests

76065bd

slumber commented Jun 6, 2023

View reviewed changes

paritytech-ci requested review from a team June 6, 2023 16:05

slumber mentioned this pull request Jun 7, 2023

runtime integration test don't initialize pallets #2703

Open

Make xcm-emulator test pass for new consensus hook (#2722)

f1cbacd

* add pallets on_initialize * tests pass * add AuraExt on_init * ".git/.scripts/commands/fmt/fmt.sh" --------- Co-authored-by: command-bot <>

rphmeier approved these changes Jun 19, 2023

View reviewed changes

paritytech-ci requested a review from a team June 19, 2023 21:33

rphmeier changed the title ~~aura-ext: check slot in consensus hook~~ aura-ext: check slot in consensus hook and remove all CheckInherents logic Jun 19, 2023

BradleyOlson64 reviewed Jun 22, 2023

View reviewed changes

BradleyOlson64 approved these changes Jun 23, 2023

View reviewed changes

BradleyOlson64 mentioned this pull request Jun 28, 2023

Asynchronous-backing compatible Aura, not plugged in #2573

Merged

6 tasks

rphmeier merged commit f09c905 into slumber-async-backing-feature Jul 3, 2023

rphmeier deleted the slumber-async-backing-check-aura-slot branch July 3, 2023 13:47

slumber mentioned this pull request Jul 13, 2023

Aura Consensus Hook should enforce the relay-parent slot is equal to the Aura slot #2588

Closed

JoshOrndorff mentioned this pull request Nov 20, 2023

Revisit soundness of timestmp inherent in parachain context Off-Narrative-Labs/Tuxedo#144

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aura-ext: check slot in consensus hook and remove all `CheckInherents` logic#2658

aura-ext: check slot in consensus hook and remove all `CheckInherents` logic#2658
rphmeier merged 9 commits intoslumber-async-backing-featurefrom
slumber-async-backing-check-aura-slot

slumber commented May 30, 2023 •

edited by rphmeier

Loading

Uh oh!

slumber May 31, 2023

Uh oh!

rphmeier May 31, 2023

Uh oh!

slumber May 31, 2023

Uh oh!

rphmeier Jun 7, 2023

Uh oh!

paritytech-cicd-pr commented Jun 6, 2023

Uh oh!

slumber Jun 6, 2023

Uh oh!

rphmeier Jun 7, 2023

Uh oh!

BradleyOlson64 Jun 22, 2023 •

edited

Loading

Uh oh!

slumber Jun 23, 2023

Uh oh!

rphmeier Jul 3, 2023

Uh oh!

BradleyOlson64 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		let (slot, authored) = pallet::Pallet::<T>::slot_info()
		.expect("slot info is inserted on block initialization");

Conversation

slumber commented May 30, 2023 • edited by rphmeier Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paritytech-cicd-pr commented Jun 6, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BradleyOlson64 Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BradleyOlson64 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

slumber commented May 30, 2023 •

edited by rphmeier

Loading

BradleyOlson64 Jun 22, 2023 •

edited

Loading