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

babe: secondary blocks with VRF #5501

Merged
merged 37 commits into from Apr 24, 2020
Merged

babe: secondary blocks with VRF #5501

merged 37 commits into from Apr 24, 2020

Conversation

sorpaas
Copy link
Member

@sorpaas sorpaas commented Apr 2, 2020

This enables secondary blocks (aura-block) with VRF in BABE. cc @burdges
Currently, secondary blocks in BABE only has slot number and authority index. This adds an additional secondary block type, which also contains an VRF. The VRF is not used in beacon randomness, but is available and may be useful for parachains.

It does not yet support live upgrade. This is due to BabeConfiguration currently only fetched from genesis block. Should be possible to lift the restriction in the future.

polkadot companion: paritytech/polkadot#1012

@sorpaas sorpaas added the A0-please_review Pull request needs code review. label Apr 2, 2020
@rphmeier
Copy link
Contributor

rphmeier commented Apr 2, 2020

It does not yet support live upgrade. This is due to BabeConfiguration currently only fetched from genesis block. Should be possible to lift the restriction in the future.

Live upgrade from primary-only should not be possible but live upgrade from secondary to secondary-VRF should, right? This is the purpose of the check has_api_at(2) as I understand the code.

@rphmeier
Copy link
Contributor

rphmeier commented Apr 2, 2020

Also, @burdges should we be including these VRF outputs in the epoch randomness?

@burdges
Copy link

burdges commented Apr 2, 2020

No. We'll only provide "fast" randomness to A&V and to parachains using this.

Primary blocks can freely choose their ancestors from among the recent secondary blocks, so including the secondary randomness would only increase the primary block producers influence of over the randomness beacon. Fewer choices is better. :)

@sorpaas
Copy link
Member Author

sorpaas commented Apr 2, 2020

Live upgrade from primary-only should not be possible but live upgrade from secondary to secondary-VRF should, right?

It does not yet in this PR. The consensus engine checks BabeConfiguration returned by runtime to determine if it should use a) primary only, b) primary and plain secondary, or c) primary and vrf secondary. So live upgrade from secondary to secondary-VRF still require live-reading of BabeConfiguration.

@sorpaas sorpaas added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Apr 2, 2020
@sorpaas sorpaas added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Apr 3, 2020
@sorpaas
Copy link
Member Author

sorpaas commented Apr 3, 2020

Attempted support for live upgrades of BABE engine #5514

@sorpaas sorpaas requested a review from tomusdrw as a code owner April 3, 2020 12:05
@sorpaas
Copy link
Member Author

sorpaas commented Apr 20, 2020

Would love if I can get some reviews on #5514 first, and then I can rebase this PR based on that.

@sorpaas
Copy link
Member Author

sorpaas commented Apr 20, 2020

Rebased on #5514, so when merging, need to merge that first (merge #5514 first, and then merge this PR).

@sorpaas
Copy link
Member Author

sorpaas commented Apr 20, 2020

Tested syncing Kusama for the EpochChanges migration by syncing without this PR and then with the PR for a few hundred blocks, not detecting issues.

Copy link
Member

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

lgtm, great work as usual :) could you update the polkadot companion PR? once it's green we can merge this and #5514.

@andresilva andresilva merged commit cddb42c into master Apr 24, 2020
@andresilva andresilva deleted the sp-aura-vrf branch April 24, 2020 15:03
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
* Companion PR for paritytech#5501

* Bump runtime versions

* add MaxRegistrars config

* Pull substrate master in PR-5501

* Attempt to fix pin commit again

* update to substrate master

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
Co-authored-by: André Silva <andre.beat@gmail.com>
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.
Projects
No open projects
Consensus
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

6 participants