Refactor: extract most aura logic out to standalone module, make use of these #13764

rphmeier · 2023-03-29T22:12:15Z

The changeset here is simply to create a standalone module in sc_consensus_aura and extract out useful behaviors such as slot claiming, seal generation, header verification to it. Then the parts of sc_consensus_aura that use these behaviors have been changed to invoke these functions.

This is also an experiment in refactoring Substrate consensus code - as @bkchr and I have discussed in the past, we've introduced abstractions at many of the wrong layers. By breaking up the consensus code into batches of standalone functions (@tomaka may cry tears of joy at this), it makes integration, modification, and customization easier.

The motivation here is to do the same in Cumulus. I am currently breaking out many primitives in Cumulus to make consensus drive the collator, and as per paritytech/cumulus#2301 we may need modifications to the meaning of Aura slots to prepare for asynchronous backing.

This is intended to be fully backwards-compatible in both API and behavior.

paritytech-cicd-pr · 2023-03-29T22:27:53Z

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/substrate/-/jobs/2614187

client/consensus/aura/src/standalone.rs

bkchr · 2023-03-30T07:46:04Z

client/consensus/aura/src/import_queue.rs

+		Err(SealVerificationError::Deferred(header, slot)) =>
+			Ok(CheckedHeader::Deferred(header, slot)),
+		Err(SealVerificationError::Unsealed) => Err(Error::HeaderUnsealed(hash)),
+		Err(SealVerificationError::BadSeal) => Err(Error::HeaderBadSeal(hash)),
+		Err(SealVerificationError::BadSignature) => Err(Error::BadSignature(hash)),
+		Err(SealVerificationError::SlotAuthorNotFound) => Err(Error::SlotAuthorNotFound),
+		Err(SealVerificationError::InvalidPreDigest(e)) => Err(Error::from(e)),


Please add SealVerificationError as variant to Error.

This would break the API, so I don't think it's a good idea.

SealVerificationError is basically just a copy of the Error variants that could be produced within seal verification.

bkchr · 2023-03-30T07:47:29Z

client/consensus/aura/src/standalone.rs

+{
+	client
+		.runtime_api()
+		.slot_duration(client.usage_info().chain.best_hash)


Maybe we should pass the bloc hash to the slot_duration function.

I agree, but unfortunately, there was already a pub fn slot_duration with this exact interface exported from the lib.rs. To maintain backwards compatibility the interface needs to stay the same.

I will add another function which takes a block hash explicitly.

client/consensus/aura/src/import_queue.rs

client/consensus/aura/src/lib.rs

improve docs Co-authored-by: Bastian Köcher <[email protected]>

rphmeier · 2023-03-30T23:55:34Z

bot merge

paritytech-processbot · 2023-03-30T23:55:42Z

This PR cannot be merged at the moment due to: 11 of 14 required status checks are expected.

processbot expects that the problem will be solved automatically later and so the auto-merge process will be started. You can simply wait for now.

rphmeier · 2023-03-30T23:56:31Z

Ci looks stuck, either that or it somehow hasn't run in several hours. mildly blocking

bkchr · 2023-03-31T09:41:24Z

bot rebase

…functions

paritytech-processbot · 2023-03-31T09:41:29Z

Rebased

bkchr · 2023-03-31T09:42:43Z

bot merge

paritytech-processbot · 2023-03-31T09:42:50Z

Waiting for commit status.

paritytech-processbot · 2023-03-31T09:44:00Z

Merge cancelled due to error. Error: Statuses failed for a18f21e

bkchr · 2023-03-31T11:03:19Z

bot fmt

command-bot · 2023-03-31T11:03:24Z

@bkchr https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2625236 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 30-8c7a1214-8ed4-43c3-aed9-cc85cd5eabbd to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2023-03-31T11:04:37Z

@bkchr Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2625236 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2625236/artifacts/download.

bkchr · 2023-03-31T11:04:41Z

bot merge

paritytech-processbot · 2023-03-31T11:04:48Z

Waiting for commit status.

paritytech-processbot · 2023-03-31T11:21:07Z

Merge cancelled due to error. Error: Statuses failed for b5e4271

…of these (#13764) * Extract most aura logic out to standalone module, make use of these * Update client/consensus/aura/src/standalone.rs improve docs Co-authored-by: Bastian Köcher <[email protected]> * add slot_duration_at * ".git/.scripts/commands/fmt/fmt.sh" --------- Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: parity-processbot <>

…of these (paritytech#13764) * Extract most aura logic out to standalone module, make use of these * Update client/consensus/aura/src/standalone.rs improve docs Co-authored-by: Bastian Köcher <[email protected]> * add slot_duration_at * ".git/.scripts/commands/fmt/fmt.sh" --------- Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: parity-processbot <>

rphmeier requested review from andresilva and davxy March 29, 2023 22:12

rphmeier added the B0-silent Changes should not be mentioned in any release notes label Mar 29, 2023

Extract most aura logic out to standalone module, make use of these

2686d95

rphmeier force-pushed the rh-aura-standalone-functions branch from 288f0e8 to 2686d95 Compare March 30, 2023 04:15

bkchr approved these changes Mar 30, 2023

View reviewed changes

mrcnski approved these changes Mar 30, 2023

View reviewed changes

davxy approved these changes Mar 30, 2023

View reviewed changes

client/consensus/aura/src/import_queue.rs Show resolved Hide resolved

client/consensus/aura/src/lib.rs Show resolved Hide resolved

rphmeier and others added 2 commits March 30, 2023 08:53

Update client/consensus/aura/src/standalone.rs

af4182b

improve docs Co-authored-by: Bastian Köcher <[email protected]>

add slot_duration_at

e1a46dc

Merge remote-tracking branch 'origin/master' into rh-aura-standalone-…

a18f21e

…functions

".git/.scripts/commands/fmt/fmt.sh"

b5e4271

bkchr merged commit a7bc9c2 into master Mar 31, 2023

bkchr deleted the rh-aura-standalone-functions branch March 31, 2023 12:00

rphmeier mentioned this pull request Apr 19, 2023

Consensus utilities and rearchitecture for more dynamic collators paritytech/cumulus#2382

Merged

7 tasks

github-actions bot mentioned this pull request Jun 15, 2023

Update substrate/polkadot/cumulus from v0.9.40 to v0.9.43 moonbeam-foundation/moonbeam#2354

Closed

kacperzuk-neti mentioned this pull request Jun 23, 2023

Polkadot v0.9.43 liberland/liberland_substrate#295

Merged

15 tasks

Refactor: extract most aura logic out to standalone module, make use of these #13764

Refactor: extract most aura logic out to standalone module, make use of these #13764

Uh oh!

Conversation

rphmeier commented Mar 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paritytech-cicd-pr commented Mar 29, 2023

Uh oh!

Uh oh!

bkchr Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

rphmeier Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

bkchr Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

rphmeier Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rphmeier commented Mar 30, 2023

Uh oh!

paritytech-processbot bot commented Mar 30, 2023

Uh oh!

rphmeier commented Mar 30, 2023

Uh oh!

bkchr commented Mar 31, 2023

Uh oh!

paritytech-processbot bot commented Mar 31, 2023

Uh oh!

bkchr commented Mar 31, 2023

Uh oh!

paritytech-processbot bot commented Mar 31, 2023

Uh oh!

paritytech-processbot bot commented Mar 31, 2023

Uh oh!

bkchr commented Mar 31, 2023

Uh oh!

command-bot bot commented Mar 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

command-bot bot commented Mar 31, 2023

Uh oh!

bkchr commented Mar 31, 2023

Uh oh!

paritytech-processbot bot commented Mar 31, 2023

Uh oh!

paritytech-processbot bot commented Mar 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rphmeier commented Mar 29, 2023 •

edited

Loading

command-bot bot commented Mar 31, 2023 •

edited

Loading