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

refactor unincluded segment length into a ConsensusHook #2501

Merged
merged 19 commits into from
May 17, 2023

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented May 2, 2023

This introduces the notion of a ConsensusHook for managing the unincluded segment capacity and performing consensus-specific checks. We'll provide some basic hooks, including an ExpectParentIncluded hook which is used to smooth over any transition periods where the collators aren't yet posting the included block's hash.

The idea is that we can write a custom ConsensusHook for the logic described in #2476 (probably fixed velocity=1, capacity=2/4, enforces max velocity + 1 blocks per slot).

I also fixed a bug in UMP queue size calculation and fixed some tests accordingly. Also relaxed the watermark rules to make them make sense, related to paritytech/polkadot#7188

The vision with the consensus hook is something like this: it will be able to handle the relay-chain state proof and extract information like the number of cores currently assigned or upcoming on-demand claims and adjust the velocity and capacity of the unincluded segment. It will allow us to do stuff like dynamically switch between on-demand and Aura/SASSAFRAS. and so on.

@rphmeier rphmeier added A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes F3-breaks_API This PR changes public API; next release should be major. T1-runtime This PR/Issue is related to the topic “runtime”. labels May 2, 2023
@rphmeier rphmeier requested a review from slumber May 2, 2023 00:21
}
}

pub(crate) fn is_expecting_included_parent(&self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capacity expecting included parent seems confusing, or perhaps tangential?

Why isn't it is_set or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's more of a "is demanding" than "is expecting", true.

pallets/parachain-system/src/tests.rs Outdated Show resolved Hide resolved
pallets/parachain-system/src/tests.rs Outdated Show resolved Hide resolved
@paritytech-ci paritytech-ci requested review from a team May 5, 2023 16:11
@rphmeier rphmeier added the C1-low PR touches the given topic and has a low impact on builders. label May 5, 2023
@rphmeier rphmeier added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label May 6, 2023
slumber and others added 2 commits May 11, 2023 12:24
* aura_ext consensus hook

* reverse dependency

* include weight into hook

* fix tests
Co-authored-by: Chris Sosnin <[email protected]>
@paritytech-cicd-pr
Copy link

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/2830823

@rphmeier rphmeier merged commit dcf62e5 into slumber-async-backing-feature May 17, 2023
@rphmeier rphmeier deleted the rh-unincluded-consensus-hooks branch May 17, 2023 00:30
Copy link
Contributor

@BradleyOlson64 BradleyOlson64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proved to be a great primer on the Cumulus side of Async Backing 👍

paritytech-processbot bot pushed a commit that referenced this pull request Aug 18, 2023
* 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 <[email protected]>

* 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 <[email protected]>

* fix test warning

* fix doc link

---------

Co-authored-by: Chris Sosnin <[email protected]>
Co-authored-by: Chris Sosnin <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: asynchronous rob <[email protected]>

* 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 <[email protected]>

* 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. <[email protected]>

* (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 <[email protected]>
Co-authored-by: Ignacio Palacios <[email protected]>
Co-authored-by: Bradley Olson <[email protected]>
Co-authored-by: Marcin S. <[email protected]>
Co-authored-by: eskimor <[email protected]>
Co-authored-by: Andronik <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. F3-breaks_API This PR changes public API; next release should be major. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants