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

Companion PR for substrate PR 8072 - Add a config field to babe epochs #2467

Merged
25 commits merged into from
Mar 10, 2021

Conversation

expenses
Copy link
Contributor

Companion PR for paritytech/substrate#8072.

@expenses expenses added B0-silent Changes should not be mentioned in any release notes A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. C1-low PR touches the given topic and has a low impact on builders. labels Feb 17, 2021
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Feb 17, 2021
@expenses expenses added the A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). label Feb 18, 2021
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.

Where is the initial EpochConfig being passed to BabeGenesisConfig? We can't just use the default value since this is currently configured with non-default values.

babe_primitives::BabeGenesisConfiguration {
slot_duration: Babe::slot_duration(),
epoch_length: EpochDuration::get(),
c: PRIMARY_PROBABILITY,
genesis_authorities: Babe::authorities(),
randomness: Babe::randomness(),
allowed_slots: babe_primitives::AllowedSlots::PrimaryAndSecondaryVRFSlots,
}

runtime/rococo/src/lib.rs Outdated Show resolved Hide resolved
runtime/westend/src/lib.rs Outdated Show resolved Hide resolved
runtime/rococo/src/lib.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
bkchr
bkchr previously requested changes Feb 24, 2021
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

This requires a runtime migration for the BABE pallet.

@expenses expenses added B2-breaksapi and removed B0-silent Changes should not be mentioned in any release notes A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). labels Mar 3, 2021
runtime/rococo/src/lib.rs Show resolved Hide resolved
genesis_authorities: Babe::authorities(),
randomness: Babe::randomness(),
allowed_slots: babe_primitives::AllowedSlots::PrimaryAndSecondaryVRFSlots,
allowed_slots: BABE_GENESIS_EPOCH_CONFIG.allowed_slots,
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is only called at genesis?

@andresilva ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's fine. The migration has the correct value PrimaryAndSecondaryPlainSlots.

runtime/test-runtime/src/lib.rs Outdated Show resolved Hide resolved
@expenses expenses requested a review from bkchr March 5, 2021 17:21
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Mar 10, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented Mar 10, 2021

Checks failed; merge aborted.

@expenses
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Mar 10, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented Mar 10, 2021

Checks failed; merge aborted.

@expenses
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Mar 10, 2021

Waiting for commit status.

@ghost ghost merged commit e6b065f into master Mar 10, 2021
@ghost ghost deleted the ashley-change-babe-epoch branch March 10, 2021 09:39
This pull request was closed.
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. A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants