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

babe: allow skipping over empty epochs #11727

Merged
merged 20 commits into from
Dec 24, 2022
Merged

babe: allow skipping over empty epochs #11727

merged 20 commits into from
Dec 24, 2022

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Jun 21, 2022 •
edited
Loading

Fixes: #11393.

Instead of assuming that slots are always produced from contiguous epochs, we allow skipping over empty epochs by re-using previously announced randomness and authorities.

Still need to add tests.

@andresilva andresilva added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 21, 2022
@github-actions github-actions bot 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 Jun 21, 2022
@andresilva andresilva requested a review from davxy June 21, 2022 23:07
davxy
davxy reviewed Jun 24, 2022
View reviewed changes
Copy link
Member

@davxy davxy left a comment

Choose a reason for hiding this comment

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

I know that this is still in Draft... but maybe some suggestions are useful

primitives/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Jul 24, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 24, 2022
@stale stale bot closed this Aug 7, 2022
@bkchr bkchr reopened this Aug 15, 2022
@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Aug 15, 2022
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/parachain-technical-summit-next-steps/51/1

@MOZGIII
Copy link
Contributor

MOZGIII commented Sep 8, 2022

Any update here?

@stale
Copy link

stale bot commented Oct 13, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Oct 13, 2022
@davxy
Copy link
Member

davxy commented Oct 14, 2022

Not stale

@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Oct 14, 2022
@andresilva andresilva marked this pull request as ready for review October 18, 2022 21:39
@bkchr bkchr added the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label Oct 28, 2022
@bkchr
Copy link
Member

bkchr commented Oct 28, 2022

I would like to have some audit here, to be safe. :P

@michalkucharczyk michalkucharczyk mentioned this pull request Nov 11, 2022
Merged
@louismerlin louismerlin added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Nov 21, 2022
@michalkucharczyk michalkucharczyk mentioned this pull request Nov 23, 2022
Closed
@stale
Copy link

stale bot commented Dec 21, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Dec 21, 2022
@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Dec 22, 2022
@bkchr
Copy link
Member

bkchr commented Dec 23, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 999b1a1 into master Dec 24, 2022
@paritytech-processbot paritytech-processbot bot deleted the andre/babe-skip-epochs branch December 24, 2022 00:03
@AurevoirXavier
Copy link
Contributor

Finally, we got this!

I still remember the day that we did the Kusama time warp.

The unexpected epoch change really annoys me in the testnet env.

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-37/1736/1

@librelois librelois mentioned this pull request Jan 23, 2023
Closed
@AurevoirXavier AurevoirXavier mentioned this pull request Feb 8, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Feb 14, 2023
Merged
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
@andresilva @bkchr @ltfschoen
babe: allow skipping over empty epochs (paritytech#11727)
65d7e69 
* babe: allow skipping epochs in pallet

* babe: detect and skip epochs on client

* babe: cleaner epoch util functions

* babe: add test for runtime handling of skipped epochs

* babe: simpler implementation of client handling of skipped epochs

* babe: test client-side handling of skipped epochs

* babe: add comments on client-side skipped epochs

* babe: remove emptyline

* babe: make it resilient to forks

* babe: typo

* babe: overflow-safe math

* babe: add test for skipping epochs across different forks

* Fix tests

* FMT

Co-authored-by: Bastian Köcher <info@kchr.de>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@andresilva @bkchr @ark0f
babe: allow skipping over empty epochs (paritytech#11727)
5d15113 
* babe: allow skipping epochs in pallet

* babe: detect and skip epochs on client

* babe: cleaner epoch util functions

* babe: add test for runtime handling of skipped epochs

* babe: simpler implementation of client handling of skipped epochs

* babe: test client-side handling of skipped epochs

* babe: add comments on client-side skipped epochs

* babe: remove emptyline

* babe: make it resilient to forks

* babe: typo

* babe: overflow-safe math

* babe: add test for skipping epochs across different forks

* Fix tests

* FMT

Co-authored-by: Bastian Köcher <info@kchr.de>
This was referenced Jul 24, 2023
Closed
Merged
@EclesioMeloJunior EclesioMeloJunior mentioned this pull request Aug 9, 2023
Closed
5 tasks
@liuchengxu liuchengxu mentioned this pull request Dec 27, 2023
Closed
@surangap surangap mentioned this pull request May 28, 2024
Merged
@MOZGIII MOZGIII mentioned this pull request Jul 26, 2024
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@davxy davxy davxy approved these changes

@bkchr bkchr bkchr approved these changes

@skunert skunert skunert approved these changes

@michalkucharczyk michalkucharczyk michalkucharczyk approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
SDK Node
Status: done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

babe: allow skipping over empty epochs
9 participants
@andresilva @Polkadot-Forum @MOZGIII @davxy @bkchr @AurevoirXavier @michalkucharczyk @skunert @louismerlin