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

Conversation

@andresilva
Copy link
Contributor

@andresilva andresilva commented Aug 13, 2019

Depends on #3377.

This PR extends the BABE consensus engine into Aurababeous. There are now two kinds of slot assignments: Primary (VRF-based) and Secondary (Slot-number based). Primary slots take precedence over secondary slots, when authoring the node starts by trying to claim a primary slot and falls back to a secondary slot claim attempt. The fork choice rule is weight-based, where weight equals the number of primary blocks in the chain. We will pick the heaviest chain and will go with the longest one in case of a tie.

The primary block author is chosen using the BABE VRF function threshold calculation. The secondary block author is chosen using: blake2_256(epoch_randomness ++ slot_number) % authorities_len.

I decided to keep the same name for the engine instead of renaming it or duplicating it. There are currently open PRs for BABE (#3301) that we want to include for Kusama and the rename would create unnecessary conflicts. I also decided not to create a new standalone engine because re-using the runtime module code is not trivial, and if we disable secondary slot assignment the engine should behave exactly like straight BABE. Additionally, I think we should keep the name BABE and just extend the spec (the slot assignment rule for secondary proposals isn't related to Aura at this point anyway).

@bkchr Suggested naming it backup slot instead. What do you think? I actually prefer primary/secondary.

TODO

  • - Fix tests
  • - Documentation
  • - Allow disabling secondary slot assignment from runtime
  • - Change secondary slot assignment rule

@andresilva andresilva added A0-please_review Pull request needs code review. M4-core labels Aug 13, 2019
@andresilva andresilva requested a review from rphmeier August 13, 2019 11:57
@andresilva andresilva force-pushed the andre/aurababeous branch 2 times, most recently from cabb01a to 6b8c40b Compare August 13, 2019 18:36
@andresilva andresilva added the A3-in_progress Pull request is in progress. No review needed at this stage. label Aug 13, 2019
@burdges
Copy link

burdges commented Aug 14, 2019

I agree with keeping the same name, but..

Any way we can easily support the aura-like blocks running on a much slower bock time than the babe slots? We should keep a testnet running with babe's erratic block times because anything that breaks indicates deeper problems.

@rphmeier
Copy link
Contributor

I actually prefer primary/secondary.

👍 - the term "primary" has some pedigree for this purpose in the literature as well. extending to "secondary" makes sense.

@andresilva andresilva removed the A3-in_progress Pull request is in progress. No review needed at this stage. label Aug 15, 2019
}

let rand = {
let mut data = [0u8; 40];
Copy link
Member

@gavofyork gavofyork Aug 15, 2019

Choose a reason for hiding this comment

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

and remove this block :)

return None;
}

let rand = {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let rand = {
let rand = U256::from((randomness, slot_number).using_encoded(blake2_256));

Copy link
Member

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

Minor remark but looks good otherwise

@gavofyork gavofyork added A9-buythatmanabeer and removed A0-please_review Pull request needs code review. labels Aug 15, 2019
@gavofyork
Copy link
Member

@andresilva just needs resolving then good to merge.

@andresilva andresilva merged commit 7eb937e into master Aug 16, 2019
@andresilva andresilva deleted the andre/aurababeous branch August 16, 2019 13:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants