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

Refactor epoch changes to a separate crate #4785

Merged
merged 15 commits into from
Feb 6, 2020
Merged

Conversation

sorpaas
Copy link
Member

@sorpaas sorpaas commented Jan 31, 2020

(Split off from #4600)

This refactors EpochChanges to be in a separate crate, so that it can be reused in other engines, such as Sassafras.

@sorpaas sorpaas added the A0-please_review Pull request needs code review. label Jan 31, 2020
@@ -41,7 +41,7 @@ use sp_std::vec::Vec;
/// (VRF based) and to a secondary (slot number based).
#[cfg(feature = "std")]
#[derive(Clone, Debug)]
pub enum BabePreDigest {
pub enum PreDigest {
Copy link
Member Author

Choose a reason for hiding this comment

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

There are some naming changes. This is opinioned, but the rationale is that:

  • digest mod is renamed to digests. This is to keep it consistent with inherents mod.
  • BabePreDigest is renamed to PreDigest. We usually only use one engine at a time, and thus it is not common to have naming collisions of multiple PreDigest. Plus, it keeps it consistent with NextEpochDescriptor.

Copy link
Contributor

@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.

changes lgtm, but babe is failing to compile due to diverging parking_lot types.

client/consensus/epochs/src/lib.rs Show resolved Hide resolved
@sorpaas
Copy link
Member Author

sorpaas commented Feb 6, 2020

@andresilva Can you take another look?

@rphmeier rphmeier dismissed andresilva’s stale review February 6, 2020 15:48

stale - does compile now

@rphmeier rphmeier merged commit 8d7bf66 into master Feb 6, 2020
@rphmeier rphmeier deleted the sp-epoch-changes branch February 6, 2020 15:48
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants