New sessions, kill consensus module #2802

gavofyork · 2019-06-05T15:12:20Z

Prototype of new sessions module. This does all the stuff discussed, and adds more stuff needed:

No duplicate keys are allowed (all active keys are maintained in a sorted vec).
Trait handlers/signals to allow control over when sessions are rotated (for BABE @demimarie-parity), when new keys are switched in (era) and when slots are disabled.
Tuple-keys to allow for different keys to be stored and switch synchronously.
Interface for the tuple keys to allow comparison for duplicate checking.
Interface for checking proof-of-ownership over a key set.
Some reformatting of staking to stop line-length checks complaining.
Removes the consensus module and any notion of a general "authority set" or authority ID type. All consensus modules are responsible to define or figure out their own authority types and set.

TODO:

Addresses paritytech/polkadot#47
Supersedes #2713 #2768 #2628

gavofyork · 2019-06-05T15:13:14Z

gavofyork · 2019-06-06T13:42:30Z

@demimarie-parity could you take a look at the Babe-relevant parts of this? Is Babe affected by the lack of a consensus module? Can you integrate ok through the ShouldSessionEnd trait?

Demi-Marie

@gavofyork Thank you for pinging me on this one.

BABE does not currently implement sessions, slashing, or validator set changes. The current srml-babe is a stub module that is sufficient for testing the consensus engine, but not for an actual blockchain. Implementing a proper srml-babe was blocked on inherent digests, so I was not able to implement it until recently. I plan on including it in my relative digests PR (#2820), which is barely started. That said, I do not believe that BABE needs any other SRML interfaces that AuRa does not, so ShouldEndSession should be sufficient.

node/cli/src/chain_spec.rs

Demi-Marie · 2019-06-07T00:51:34Z

node/runtime/src/lib.rs

+
+// NOTE: `SessionHandler` and `SessionKeys` are co-dependent: One key will be used for each handler.
+// The number and order of items in `SessionHandler` *MUST* be the same number and order of keys in
+// `SessionKeys`.


What happens if they do not match? Also, we could use a macro for this.

basically, you'll end up with default keys rather than the actual keys. a macro would be easiest to do. it may be possible to do something cleverer with traits to avoid exposing a macro to the user, but i haven't though about it too deeply.

srml/session/src/lib.rs

srml/staking/src/lib.rs

Co-Authored-By: DemiMarie-parity <[email protected]>

…essions

The docs don't mention Log and nor should they - it's an outdated, unstable API.

Demi-Marie

The parts I looked at (AuRa and BABE, mostly) look good. I did not review parts of the codebase I have never worked on, though.

Demi-Marie · 2019-06-13T15:06:19Z

core/sr-api-macros/benches/bench.rs

 	});

 	c.bench_function("calling function in wasm", |b| {
-		let client = test_client::new_with_execution_strategy(ExecutionStrategy::AlwaysWasm);


Is this correct? CI didn’t pass without it.

gavofyork · 2019-06-14T08:59:57Z

@svyatonik this touches some of your code - could you take another look?

svyatonik

I was mostly checking that authorities-change signals are still emitted - that seems true. Can't say much about other (SRML) changes.

node/executor/src/lib.rs

srml/aura/src/lib.rs

@dvc94ch

* Draft of new sessions * Reintroduce tuple impls * Move staking module to new session API * More work on staking and grandpa. * Use iterator to avoid cloning and tuple macro * Make runtime build again * Polish the OpaqueKeys devex * Move consensus logic into system & aura. * Fix up system module * Get build mostly going. Stuck at service.rs * Building again * Update srml/staking/src/lib.rs Co-Authored-By: DemiMarie-parity <[email protected]> * Refactoring out Consensus module, AuthorityIdOf, &c. * Refactored out DigestItem::AuthoritiesChanged. Building. * Remove tentative code * Remove invalid comment * Make Seal opaque and introduce nice methods for handling opaque items. * Start to use proper digest for Aura authorities tracking. * Fix up grandpa, remove system::Raw/Log * Refactor Grandpa to use new logging infrastructure. Also make authorityid/sessionkey static. Switch over to storing authorities in a straight Vec. * Building again * Tidy up some AuthorityIds * Expunge most of the rest of the AuthorityKey confusion. Also, de-generify Babe and re-generify Aura. * Remove cruft * Untangle last of the `AuthorityId`s. * Sort out finality_tracker * Refactor median getting * Apply suggestions from code review Co-Authored-By: Robert Habermeier <[email protected]> * Session tests works * Update core/sr-primitives/src/generic/digest.rs Co-Authored-By: DemiMarie-parity <[email protected]> * Session tests works * Fix for staking from @dvc94ch * log an error * fix test runtime build * Some test fixes * Staking mock update to new session api. * Fix build. * Move OpaqueKeys to primitives. * Use on_initialize instead of check_rotate_session. * Update tests to new staking api. * fixup mock * Fix bond_extra_and_withdraw_unbonded_works. * Fix bond_with_little_staked_value_bounded_by_slot_stake. * Fix bond_with_no_staked_value. * Fix change_controller_works. * Fix less_than_needed_candidates_works. * Fix multi_era_reward_should_work. * Fix nominating_and_rewards_should_work. * Fix nominators_also_get_slashed. * Fix phragmen_large_scale_test. * Fix phragmen_poc_works. * Fix phragmen_score_should_be_accurate_on_large_stakes. * Fix phragmen_should_not_overflow. * Fix reward_destination_works. * Fix rewards_should_work. * Fix sessions_and_eras_should_work. * Fix slot_stake_is_least_staked_validator. * Fix too_many_unbond_calls_should_not_work. * Fix wrong_vote_is_null. * Fix runtime. * Fix wasm runtime build. * Update Cargo.lock * Fix warnings. * Fix grandpa tests. * Fix test-runtime build. * Fix template node build. * Fix stuff. * Update Cargo.lock to fix CI * Re-add missing AuRa logs Runtimes are required to know about every digest they receive ― they panic otherwise. This re-adds support for AuRa pre-runtime digests. * Update core/consensus/babe/src/digest.rs Co-Authored-By: DemiMarie-parity <[email protected]> * Kill log trait and all that jazz. * Refactor staking tests. * Fix ci runtime wasm check. * Line length 120. * Make tests build again * Remove trailing commas in function declarations The `extern_functions!` macro doesn’t like them, perhaps due to a bug in rustc. * Fix type error * Fix compilation errors * Fix a test * Another couple of fixes * Fix another test * More test fixes * Another test fix * Bump runtime. * Wrap long line * Fix build, remove redundant code. * Issue to track TODO * Leave the benchmark code alone. * Fix missing `std::time::{Instant, Duration}` * Indentation * Aura ConsensusLog as enum

@dvc94ch

* Draft of new sessions * Reintroduce tuple impls * Move staking module to new session API * More work on staking and grandpa. * Use iterator to avoid cloning and tuple macro * Make runtime build again * Polish the OpaqueKeys devex * Move consensus logic into system & aura. * Fix up system module * Get build mostly going. Stuck at service.rs * Building again * Update srml/staking/src/lib.rs Co-Authored-By: DemiMarie-parity <[email protected]> * Refactoring out Consensus module, AuthorityIdOf, &c. * Refactored out DigestItem::AuthoritiesChanged. Building. * Remove tentative code * Remove invalid comment * Make Seal opaque and introduce nice methods for handling opaque items. * Start to use proper digest for Aura authorities tracking. * Fix up grandpa, remove system::Raw/Log * Refactor Grandpa to use new logging infrastructure. Also make authorityid/sessionkey static. Switch over to storing authorities in a straight Vec. * Building again * Tidy up some AuthorityIds * Expunge most of the rest of the AuthorityKey confusion. Also, de-generify Babe and re-generify Aura. * Remove cruft * Untangle last of the `AuthorityId`s. * Sort out finality_tracker * Refactor median getting * Apply suggestions from code review Co-Authored-By: Robert Habermeier <[email protected]> * Session tests works * Update core/sr-primitives/src/generic/digest.rs Co-Authored-By: DemiMarie-parity <[email protected]> * Session tests works * Fix for staking from @dvc94ch * log an error * fix test runtime build * Some test fixes * Staking mock update to new session api. * Fix build. * Move OpaqueKeys to primitives. * Use on_initialize instead of check_rotate_session. * Update tests to new staking api. * fixup mock * Fix bond_extra_and_withdraw_unbonded_works. * Fix bond_with_little_staked_value_bounded_by_slot_stake. * Fix bond_with_no_staked_value. * Fix change_controller_works. * Fix less_than_needed_candidates_works. * Fix multi_era_reward_should_work. * Fix nominating_and_rewards_should_work. * Fix nominators_also_get_slashed. * Fix phragmen_large_scale_test. * Fix phragmen_poc_works. * Fix phragmen_score_should_be_accurate_on_large_stakes. * Fix phragmen_should_not_overflow. * Fix reward_destination_works. * Fix rewards_should_work. * Fix sessions_and_eras_should_work. * Fix slot_stake_is_least_staked_validator. * Fix too_many_unbond_calls_should_not_work. * Fix wrong_vote_is_null. * Fix runtime. * Fix wasm runtime build. * Update Cargo.lock * Fix warnings. * Fix grandpa tests. * Fix test-runtime build. * Fix template node build. * Fix stuff. * Update Cargo.lock to fix CI * Re-add missing AuRa logs Runtimes are required to know about every digest they receive ― they panic otherwise. This re-adds support for AuRa pre-runtime digests. * Update core/consensus/babe/src/digest.rs Co-Authored-By: DemiMarie-parity <[email protected]> * Kill log trait and all that jazz. * Refactor staking tests. * Fix ci runtime wasm check. * Line length 120. * Make tests build again * Remove trailing commas in function declarations The `extern_functions!` macro doesn’t like them, perhaps due to a bug in rustc. * Fix type error * Fix compilation errors * Fix a test * Another couple of fixes * Fix another test * More test fixes * Another test fix * Bump runtime. * Wrap long line * Fix build, remove redundant code. * Issue to track TODO * Leave the benchmark code alone. * Fix missing `std::time::{Instant, Duration}` * Indentation * Aura ConsensusLog as enum

Draft of new sessions

81d2237

gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jun 5, 2019

devops-parity added the A4-gotissues label Jun 5, 2019

gavofyork added this to the 2.0-kusama milestone Jun 5, 2019

gavofyork added 7 commits June 5, 2019 17:27

Reintroduce tuple impls

4fc86da

Merge remote-tracking branch 'origin/master' into gav-new-sessions

d24568a

Move staking module to new session API

84f1f1c

More work on staking and grandpa.

5c67469

Use iterator to avoid cloning and tuple macro

5acfa04

Make runtime build again

fde708d

Polish the OpaqueKeys devex

b9b7d42

gavofyork added 3 commits June 6, 2019 16:06

Move consensus logic into system & aura.

445df61

Fix up system module

4db0062

Get build mostly going. Stuck at service.rs

6696c55

gavofyork changed the title ~~Draft of new sessions~~ New sessions, kill consensus module Jun 6, 2019

Building again

dc9e8c6

Demi-Marie self-requested a review June 7, 2019 00:31

Demi-Marie suggested changes Jun 7, 2019

View reviewed changes

gavofyork and others added 5 commits June 7, 2019 08:44

Update srml/staking/src/lib.rs

7da4b04

Co-Authored-By: DemiMarie-parity <[email protected]>

Refactoring out Consensus module, AuthorityIdOf, &c.

e2a0f2e

Merge remote-tracking branch 'origin/gav-new-sessions' into gav-new-s…

6440ca8

…essions

Refactored out DigestItem::AuthoritiesChanged. Building.

cfea92a

Remove tentative code

d3de475

rphmeier mentioned this pull request Jun 7, 2019

Pluggable Consensus Refactoring #2821

Open

6 tasks

gavofyork added 4 commits June 7, 2019 15:17

Remove invalid comment

a061fd7

Make Seal opaque and introduce nice methods for handling opaque items.

7f303f9

Merge remote-tracking branch 'origin/master' into gav-new-sessions

a38084d

Start to use proper digest for Aura authorities tracking.

f8b24e0

gavofyork added 2 commits June 13, 2019 16:12

Merge branch 'master' into gav-new-sessions

bb19f0b

Bump runtime.

3fcb3a7

devops-parity added the B2-breaksapi label Jun 13, 2019

Wrap long line

03249cd

gavofyork removed the A4-gotissues label Jun 13, 2019

Fix build, remove redundant code.

56f5ac1

gavofyork requested a review from Demi-Marie June 13, 2019 14:41

Issue to track TODO

db47886

Demi-Marie approved these changes Jun 13, 2019

View reviewed changes

gavofyork and others added 3 commits June 13, 2019 17:31

Leave the benchmark code alone.

6b44e5f

Merge remote-tracking branch 'origin/master' into gav-new-sessions

2e8c85f

Fix missing std::time::{Instant, Duration}

5beee4b

gavofyork requested review from rphmeier and svyatonik June 14, 2019 08:59

svyatonik approved these changes Jun 14, 2019

View reviewed changes

node/executor/src/lib.rs Outdated Show resolved Hide resolved

srml/aura/src/lib.rs Outdated Show resolved Hide resolved

gavofyork added 2 commits June 14, 2019 13:49

Indentation

3ee9625

Aura ConsensusLog as enum

0ad33b9

gavofyork merged commit dbf3226 into master Jun 14, 2019

gavofyork deleted the gav-new-sessions branch June 14, 2019 14:34

xlc mentioned this pull request Jun 17, 2019

Session length and sessions per era is no longer configurable #2878

Closed

kigawas mentioned this pull request Jun 26, 2019

Log type seems to be removed polkadot-developers/substrate-developer-hub.github.io#149

Closed

JoshOrndorff mentioned this pull request Jun 26, 2019

Staking: Minor typo fix: of -> or #2963

Merged

JoshOrndorff mentioned this pull request Jan 10, 2020

Remove references to frame_consensus from docs #4582

Merged

gnunicorn modified the milestones: 2.1, Polkadot Mar 4, 2020

pmikolajczyk41 mentioned this pull request Aug 11, 2022

Change consensus engine ID Cardinal-Cryptography/aleph-node#584

Closed

New sessions, kill consensus module #2802

New sessions, kill consensus module #2802

Uh oh!

Conversation

gavofyork commented Jun 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gavofyork commented Jun 5, 2019

Uh oh!

gavofyork commented Jun 6, 2019

Uh oh!

Demi-Marie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Demi-Marie Jun 7, 2019

Choose a reason for hiding this comment

Uh oh!

gavofyork Jun 7, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Demi-Marie left a comment

Choose a reason for hiding this comment

Uh oh!

Demi-Marie Jun 13, 2019

Choose a reason for hiding this comment

Uh oh!

gavofyork Jun 13, 2019

Choose a reason for hiding this comment

Uh oh!

gavofyork commented Jun 14, 2019

Uh oh!

svyatonik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gavofyork commented Jun 5, 2019 •

edited

Loading