Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Babe epoch newtype #1596

Merged
merged 2 commits into from
Sep 17, 2023
Merged

Babe epoch newtype #1596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f962bf2
Babe client epoch newtype wrapping the primitive one
davxy Sep 16, 2023
206a556
Cargo.toml cleanup
davxy Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions substrate/client/consensus/babe/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
async-trait = "0.1.57"
scale-info = { version = "2.5.0", features = ["derive"] }
codec = { package = "parity-scale-codec", version = "3.6.1", features = ["derive"] }
futures = "0.3.21"
log = "0.4.17"
Expand Down Expand Up @@ -45,10 +44,8 @@ sp-keystore = { path = "../../../primitives/keystore" }
sp-runtime = { path = "../../../primitives/runtime" }

[dev-dependencies]
rand_chacha = "0.2.2"
sc-block-builder = { path = "../../block-builder" }
sp-keyring = { path = "../../../primitives/keyring" }
sc-network = { path = "../../network" }
sc-network-test = { path = "../../network/test" }
sp-timestamp = { path = "../../../primitives/timestamp" }
sp-tracing = { path = "../../../primitives/tracing" }
Expand Down
21 changes: 10 additions & 11 deletions substrate/client/consensus/babe/src/authorship.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,22 @@ fn claim_secondary_slot(
keystore: &KeystorePtr,
author_secondary_vrf: bool,
) -> Option<(PreDigest, AuthorityId)> {
let Epoch { authorities, randomness, mut epoch_index, .. } = epoch;

if authorities.is_empty() {
if epoch.authorities.is_empty() {
return None
}

let mut epoch_index = epoch.epoch_index;
if epoch.end_slot() <= slot {
// Slot doesn't strictly belong to the epoch, create a clone with fixed values.
epoch_index = epoch.clone_for_slot(slot).epoch_index;
}

let expected_author = secondary_slot_author(slot, authorities, *randomness)?;
let expected_author = secondary_slot_author(slot, &epoch.authorities, epoch.randomness)?;

for (authority_id, authority_index) in keys {
if authority_id == expected_author {
let pre_digest = if author_secondary_vrf {
let data = make_vrf_sign_data(randomness, slot, epoch_index);
let data = make_vrf_sign_data(&epoch.randomness, slot, epoch_index);
let result =
keystore.sr25519_vrf_sign(AuthorityId::ID, authority_id.as_ref(), &data);
if let Ok(Some(vrf_signature)) = result {
Expand Down Expand Up @@ -232,19 +231,18 @@ fn claim_primary_slot(
keystore: &KeystorePtr,
keys: &[(AuthorityId, usize)],
) -> Option<(PreDigest, AuthorityId)> {
let Epoch { authorities, randomness, mut epoch_index, .. } = epoch;

let mut epoch_index = epoch.epoch_index;
if epoch.end_slot() <= slot {
// Slot doesn't strictly belong to the epoch, create a clone with fixed values.
epoch_index = epoch.clone_for_slot(slot).epoch_index;
}

let data = make_vrf_sign_data(randomness, slot, epoch_index);
let data = make_vrf_sign_data(&epoch.randomness, slot, epoch_index);

for (authority_id, authority_index) in keys {
let result = keystore.sr25519_vrf_sign(AuthorityId::ID, authority_id.as_ref(), &data);
if let Ok(Some(vrf_signature)) = result {
let threshold = calculate_primary_threshold(c, authorities, *authority_index);
let threshold = calculate_primary_threshold(c, &epoch.authorities, *authority_index);

let can_claim = authority_id
.as_inner_ref()
Expand Down Expand Up @@ -274,7 +272,7 @@ fn claim_primary_slot(
#[cfg(test)]
mod tests {
use super::*;
use sp_consensus_babe::{AllowedSlots, AuthorityId, BabeEpochConfiguration};
use sp_consensus_babe::{AllowedSlots, AuthorityId, BabeEpochConfiguration, Epoch};
use sp_core::{crypto::Pair as _, sr25519::Pair};
use sp_keystore::testing::MemoryKeystore;

Expand All @@ -300,7 +298,8 @@ mod tests {
c: (3, 10),
allowed_slots: AllowedSlots::PrimaryAndSecondaryPlainSlots,
},
};
}
.into();

assert!(claim_slot(10.into(), &epoch, &keystore).is_none());

Expand Down
55 changes: 26 additions & 29 deletions substrate/client/consensus/babe/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
use std::{
collections::HashSet,
future::Future,
ops::{Deref, DerefMut},
pin::Pin,
sync::Arc,
task::{Context, Poll},
Expand Down Expand Up @@ -156,20 +157,27 @@ const AUTHORING_SCORE_VRF_CONTEXT: &[u8] = b"substrate-babe-vrf";
const AUTHORING_SCORE_LENGTH: usize = 16;

/// BABE epoch information
#[derive(Decode, Encode, PartialEq, Eq, Clone, Debug, scale_info::TypeInfo)]
pub struct Epoch {
/// The epoch index.
pub epoch_index: u64,
/// The starting slot of the epoch.
pub start_slot: Slot,
/// The duration of this epoch.
pub duration: u64,
/// The authorities and their weights.
pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>,
/// Randomness for this epoch.
pub randomness: Randomness,
/// Configuration of the epoch.
pub config: BabeEpochConfiguration,
#[derive(Clone, Debug, PartialEq, Eq, Encode, Decode)]
pub struct Epoch(sp_consensus_babe::Epoch);

impl Deref for Epoch {
type Target = sp_consensus_babe::Epoch;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl DerefMut for Epoch {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl From<sp_consensus_babe::Epoch> for Epoch {
fn from(epoch: sp_consensus_babe::Epoch) -> Self {
Epoch(epoch)
}
}

impl EpochT for Epoch {
Expand All @@ -180,14 +188,15 @@ impl EpochT for Epoch {
&self,
(descriptor, config): (NextEpochDescriptor, BabeEpochConfiguration),
) -> Epoch {
Epoch {
sp_consensus_babe::Epoch {
epoch_index: self.epoch_index + 1,
start_slot: self.start_slot + self.duration,
duration: self.duration,
authorities: descriptor.authorities,
randomness: descriptor.randomness,
config,
}
.into()
}

fn start_slot(&self) -> Slot {
Expand All @@ -199,25 +208,12 @@ impl EpochT for Epoch {
}
}

impl From<sp_consensus_babe::Epoch> for Epoch {
fn from(epoch: sp_consensus_babe::Epoch) -> Self {
Epoch {
epoch_index: epoch.epoch_index,
start_slot: epoch.start_slot,
duration: epoch.duration,
authorities: epoch.authorities,
randomness: epoch.randomness,
config: epoch.config,
}
}
}

impl Epoch {
/// Create the genesis epoch (epoch #0).
///
/// This is defined to start at the slot of the first block, so that has to be provided.
pub fn genesis(genesis_config: &BabeConfiguration, slot: Slot) -> Epoch {
Epoch {
sp_consensus_babe::Epoch {
epoch_index: 0,
start_slot: slot,
duration: genesis_config.epoch_length,
Expand All @@ -228,6 +224,7 @@ impl Epoch {
allowed_slots: genesis_config.allowed_slots,
},
}
.into()
}

/// Clone and tweak epoch information to refer to the specified slot.
Expand Down
4 changes: 3 additions & 1 deletion substrate/client/consensus/babe/src/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,18 @@ impl EpochT for EpochV0 {
}
}

// Implement From<EpochV0> for Epoch
impl EpochV0 {
/// Migrate the sturct to current epoch version.
pub fn migrate(self, config: &BabeConfiguration) -> Epoch {
Epoch {
sp_consensus_babe::Epoch {
epoch_index: self.epoch_index,
start_slot: self.start_slot,
duration: self.duration,
authorities: self.authorities,
randomness: self.randomness,
config: BabeEpochConfiguration { c: config.c, allowed_slots: config.allowed_slots },
}
.into()
}
}
10 changes: 6 additions & 4 deletions substrate/client/consensus/babe/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ fn claim_epoch_slots() {
let authority = Sr25519Keyring::Alice;
let keystore = create_keystore(authority);

let mut epoch = Epoch {
let mut epoch: Epoch = sp_consensus_babe::Epoch {
start_slot: 0.into(),
authorities: vec![(authority.public().into(), 1)],
randomness: [0; 32],
Expand All @@ -511,7 +511,8 @@ fn claim_epoch_slots() {
c: (3, 10),
allowed_slots: AllowedSlots::PrimaryAndSecondaryPlainSlots,
},
};
}
.into();

let claim_slot_wrap = |s, e| match claim_slot(Slot::from(s as u64), &e, &keystore) {
None => 0,
Expand Down Expand Up @@ -551,7 +552,7 @@ fn claim_vrf_check() {

let public = authority.public();

let epoch = Epoch {
let epoch: Epoch = sp_consensus_babe::Epoch {
start_slot: 0.into(),
authorities: vec![(public.into(), 1)],
randomness: [0; 32],
Expand All @@ -561,7 +562,8 @@ fn claim_vrf_check() {
c: (3, 10),
allowed_slots: AllowedSlots::PrimaryAndSecondaryVRFSlots,
},
};
}
.into();

// We leverage the predictability of claim types given a constant randomness.

Expand Down