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

Use BABE instead of AuRa in node #3171

Merged
merged 37 commits into from
Jul 24, 2019
Merged

Use BABE instead of AuRa in node #3171

merged 37 commits into from
Jul 24, 2019

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Jul 22, 2019 •
edited
Loading

Depends on #3028.

This PR updates the node to use BABE instead of AuRa for block authoring. Additionally, I made some changes to the srml-babe module, notably it now tracks the current slot and the epoch start slot. This is used to implement ShouldEndSession and also for the node to track the expected last epoch slot accurately.

@andresilva andresilva added A3-in_progress Pull request is in progress. No review needed at this stage. M4-core labels Jul 22, 2019
@andresilva
Copy link
Contributor Author

@jacogr Renamed the weight type to BabeWeight to avoid the issue you mentioned.

dvc94ch
dvc94ch reviewed Jul 22, 2019
View reviewed changes
node/primitives/src/lib.rs Outdated Show resolved Hide resolved
Demi-Marie
Demi-Marie approved these changes Jul 22, 2019
View reviewed changes
Copy link
Contributor

@Demi-Marie Demi-Marie left a comment

Choose a reason for hiding this comment

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

This looks good!

@andresilva
Copy link
Contributor Author

I believe I have ironed out all the kinks, seems to be working fine. @rphmeier mentioned that some clarification is needed regarding the threshold calculation from the research team.

jacogr
jacogr reviewed Jul 23, 2019
View reviewed changes
srml/babe/src/lib.rs Outdated Show resolved Hide resolved
@andresilva andresilva force-pushed the andre/node-babe-runtime branch 3 times, most recently from d594b06 to 6c4681b Compare July 23, 2019 11:11
Demi-Marie
Demi-Marie suggested changes Jul 23, 2019
View reviewed changes
Copy link
Contributor

@Demi-Marie Demi-Marie left a comment

Choose a reason for hiding this comment

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

BABE needs to buffer authority sets and randomness by 1 epoch. I will fix this.

core/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
core/consensus/babe/src/tests.rs
@@ -232,7 +237,10 @@ fn run_one_test() {
// wait for all finalized on each.
let wait_for = futures::future::join_all(import_notifications);

let drive_to_completion = futures::future::poll_fn(|| { net.lock().poll(); Ok(Async::NotReady) });
let drive_to_completion = futures::future::poll_fn(|| {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this hunk should be reverted, since it is just a style fix and conflicts with #3146.

srml/babe/src/lib.rs Outdated Show resolved Hide resolved
rphmeier
rphmeier reviewed Jul 23, 2019
View reviewed changes
node/runtime/src/lib.rs Outdated Show resolved Hide resolved
@andresilva andresilva 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 Jul 23, 2019
@andresilva
Copy link
Contributor Author

I think all the issues we have discussed offline have been fixed. For the threshold calculation, the exponentation is done in f64 which is then converted into a num::BigRational. Then we use num::BigUint to multiply by the previous ratio (multiply by 1 << 128, where 128 is the number of bits from the VRF we'll use for comparing). For the constant c I set it to 0.3 but I have no basis for this (W3F research team should get back to us on this).

The light client is currently not working, I will file some issues for follow-up PRs to fix it. I updated the sync integration test to generate valid BABE predigests but disabled the light clients for now.

@Demi-Marie
Merge remote-tracking branch 'origin/master' into andre/node-babe-run…
ae5094e 
…time
@Demi-Marie
Fix CI
8489fa2 
* Bump `spec_version`
* Fix type error in test helpers
@Demi-Marie Demi-Marie added this to the 2.0-k milestone Jul 24, 2019
@Demi-Marie Demi-Marie merged commit 8b7d0fb into master Jul 24, 2019
@Demi-Marie Demi-Marie deleted the andre/node-babe-runtime branch July 24, 2019 19:53
@jacogr jacogr mentioned this pull request Jul 24, 2019
Merged
@Demi-Marie Demi-Marie mentioned this pull request Jul 23, 2019
Closed
7 tasks
@gnunicorn gnunicorn modified the milestones: 2.1, Polkadot Mar 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rphmeier rphmeier rphmeier left review comments

@jacogr jacogr jacogr left review comments

@tomaka tomaka tomaka left review comments

@dvc94ch dvc94ch dvc94ch left review comments

@Demi-Marie Demi-Marie Demi-Marie approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Milestone
Polkadot
Development

Successfully merging this pull request may close these issues.
8 participants
@andresilva @dvc94ch @tomaka @jacogr @rphmeier @Demi-Marie @gnunicorn @devops-parity