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

Independence of Slot-based algorithms from system Timestamp #12224

Merged
merged 18 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Cargo.lock

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

4 changes: 2 additions & 2 deletions bin/node-template/node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub fn new_partial(
slot_duration,
);

Ok((timestamp, slot))
Ok((slot, timestamp))
davxy marked this conversation as resolved.
Show resolved Hide resolved
},
spawner: &task_manager.spawn_essential_handle(),
registry: config.prometheus_registry(),
Expand Down Expand Up @@ -269,7 +269,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
slot_duration,
);

Ok((timestamp, slot))
Ok((slot, timestamp))
},
force_authoring,
backoff_authoring_blocks,
Expand Down
4 changes: 2 additions & 2 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ pub fn new_partial(
let uncles =
sp_authorship::InherentDataProvider::<<Block as BlockT>::Header>::check_inherents();

Ok((timestamp, slot, uncles))
Ok((slot, timestamp, uncles))
},
&task_manager.spawn_essential_handle(),
config.prometheus_registry(),
Expand Down Expand Up @@ -453,7 +453,7 @@ pub fn new_full_base(
&parent,
)?;

Ok((timestamp, slot, uncles, storage_proof))
Ok((slot, timestamp, uncles, storage_proof))
}
},
force_authoring,
Expand Down
26 changes: 11 additions & 15 deletions client/consensus/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ mod tests {
traits::{Block as BlockT, Header as _},
Digest,
};
use sp_timestamp::InherentDataProvider as TimestampInherentDataProvider;
davxy marked this conversation as resolved.
Show resolved Hide resolved
use sp_timestamp::Timestamp;
use std::{
task::Poll,
time::{Duration, Instant},
Expand All @@ -579,6 +579,8 @@ mod tests {
TestClient,
};

const SLOT_DURATION_MS: u64 = 1000;

type Error = sp_blockchain::Error;

struct DummyFactory(Arc<TestClient>);
Expand Down Expand Up @@ -619,16 +621,14 @@ mod tests {
}
}

const SLOT_DURATION: u64 = 1000;

type AuraVerifier = import_queue::AuraVerifier<
PeersFullClient,
AuthorityPair,
Box<
dyn CreateInherentDataProviders<
TestBlock,
(),
InherentDataProviders = (TimestampInherentDataProvider, InherentDataProvider),
InherentDataProviders = (InherentDataProvider,),
davxy marked this conversation as resolved.
Show resolved Hide resolved
>,
>,
>;
Expand All @@ -648,17 +648,15 @@ mod tests {
let client = client.as_client();
let slot_duration = slot_duration(&*client).expect("slot duration available");

assert_eq!(slot_duration.as_millis() as u64, SLOT_DURATION);
assert_eq!(slot_duration.as_millis() as u64, SLOT_DURATION_MS);
import_queue::AuraVerifier::new(
client,
Box::new(|_, _| async {
let timestamp = TimestampInherentDataProvider::from_system_time();
let slot = InherentDataProvider::from_timestamp_and_slot_duration(
*timestamp,
SlotDuration::from_millis(6000),
Timestamp::current(),
SlotDuration::from_millis(SLOT_DURATION_MS),
);

Ok((timestamp, slot))
Ok((slot,))
}),
CheckForEquivocation::Yes,
None,
Expand Down Expand Up @@ -736,13 +734,12 @@ mod tests {
sync_oracle: DummyOracle,
justification_sync_link: (),
create_inherent_data_providers: |_, _| async {
let timestamp = TimestampInherentDataProvider::from_system_time();
let slot = InherentDataProvider::from_timestamp_and_slot_duration(
*timestamp,
SlotDuration::from_millis(6000),
Timestamp::current(),
SlotDuration::from_millis(SLOT_DURATION_MS),
);

Ok((timestamp, slot))
Ok((slot,))
},
force_authoring: false,
backoff_authoring_blocks: Some(
Expand Down Expand Up @@ -875,7 +872,6 @@ mod tests {

let res = executor::block_on(worker.on_slot(SlotInfo {
slot: 0.into(),
timestamp: 0.into(),
ends_at: Instant::now() + Duration::from_secs(100),
inherent_data: InherentData::new(),
duration: Duration::from_millis(1000),
Expand Down
22 changes: 10 additions & 12 deletions client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use sp_runtime::{
generic::{Digest, DigestItem},
traits::Block as BlockT,
};
use sp_timestamp::InherentDataProvider as TimestampInherentDataProvider;
davxy marked this conversation as resolved.
Show resolved Hide resolved
use sp_timestamp::Timestamp;
use std::{cell::RefCell, task::Poll, time::Duration};

type Item = DigestItem;
Expand All @@ -68,6 +68,8 @@ type Mutator = Arc<dyn Fn(&mut TestHeader, Stage) + Send + Sync>;
type BabeBlockImport =
PanickingBlockImport<crate::BabeBlockImport<TestBlock, TestClient, Arc<TestClient>>>;

const SLOT_DURATION_MS: u64 = 1000;

#[derive(Clone)]
struct DummyFactory {
client: Arc<TestClient>,
Expand Down Expand Up @@ -239,7 +241,7 @@ pub struct TestVerifier {
dyn CreateInherentDataProviders<
TestBlock,
(),
InherentDataProviders = (TimestampInherentDataProvider, InherentDataProvider),
InherentDataProviders = (InherentDataProvider,),
>,
>,
>,
Expand Down Expand Up @@ -321,13 +323,11 @@ impl TestNetFactory for BabeTestNet {
client: client.clone(),
select_chain: longest_chain,
create_inherent_data_providers: Box::new(|_, _| async {
let timestamp = TimestampInherentDataProvider::from_system_time();
let slot = InherentDataProvider::from_timestamp_and_slot_duration(
*timestamp,
SlotDuration::from_millis(6000),
Timestamp::current(),
SlotDuration::from_millis(SLOT_DURATION_MS),
);

Ok((timestamp, slot))
Ok((slot,))
}),
config: data.link.config.clone(),
epoch_changes: data.link.epoch_changes.clone(),
Expand Down Expand Up @@ -433,13 +433,11 @@ fn run_one_test(mutator: impl Fn(&mut TestHeader, Stage) + Send + Sync + 'static
env: environ,
sync_oracle: DummyOracle,
create_inherent_data_providers: Box::new(|_, _| async {
let timestamp = TimestampInherentDataProvider::from_system_time();
let slot = InherentDataProvider::from_timestamp_and_slot_duration(
*timestamp,
SlotDuration::from_millis(6000),
Timestamp::current(),
SlotDuration::from_millis(SLOT_DURATION_MS),
);

Ok((timestamp, slot))
Ok((slot,))
davxy marked this conversation as resolved.
Show resolved Hide resolved
}),
force_authoring: false,
backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
Expand Down
1 change: 0 additions & 1 deletion client/consensus/slots/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ sp-core = { version = "6.0.0", path = "../../../primitives/core" }
sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" }
sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" }
sp-state-machine = { version = "0.12.0", path = "../../../primitives/state-machine" }
sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" }

[dev-dependencies]
substrate-test-runtime-client = { version = "2.0.0", path = "../../../test-utils/runtime/client" }
56 changes: 18 additions & 38 deletions client/consensus/slots/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ use sp_consensus::{Proposal, Proposer, SelectChain, SyncOracle};
use sp_consensus_slots::{Slot, SlotDuration};
use sp_inherents::CreateInherentDataProviders;
use sp_runtime::traits::{Block as BlockT, HashFor, Header as HeaderT};
use sp_timestamp::Timestamp;
davxy marked this conversation as resolved.
Show resolved Hide resolved
use std::{fmt::Debug, ops::Deref, time::Duration};

/// The changes that need to applied to the storage to create the state for a block.
Expand Down Expand Up @@ -252,7 +251,7 @@ pub trait SimpleSlotWorker<B: BlockT> {
where
Self: Sync,
{
let (timestamp, slot) = (slot_info.timestamp, slot_info.slot);
let slot = slot_info.slot;
let telemetry = self.telemetry();
let logging_target = self.logging_target();

Expand Down Expand Up @@ -316,23 +315,14 @@ pub trait SimpleSlotWorker<B: BlockT> {
return None
}

debug!(
target: logging_target,
"Starting authorship at slot {}; timestamp = {}", slot, *timestamp,
);
debug!(target: logging_target, "Starting authorship at slot: {slot}");

telemetry!(
telemetry;
CONSENSUS_DEBUG;
"slots.starting_authorship";
"slot_num" => *slot,
"timestamp" => *timestamp,
);
telemetry!(telemetry; CONSENSUS_DEBUG; "slots.starting_authorship"; "slot_num" => slot);

let proposer = match self.proposer(&slot_info.chain_head).await {
Ok(p) => p,
Err(err) => {
warn!(target: logging_target, "Unable to author block in slot {:?}: {}", slot, err,);
warn!(target: logging_target, "Unable to author block in slot {slot:?}: {err}");

telemetry!(
telemetry;
Expand Down Expand Up @@ -440,44 +430,35 @@ impl<T: SimpleSlotWorker<B> + Send + Sync, B: BlockT>

/// Slot specific extension that the inherent data provider needs to implement.
pub trait InherentDataProviderExt {
/// The current timestamp that will be found in the
/// [`InherentData`](`sp_inherents::InherentData`).
fn timestamp(&self) -> Timestamp;

/// The current slot that will be found in the [`InherentData`](`sp_inherents::InherentData`).
fn slot(&self) -> Slot;
}

/// Small macro for implementing `InherentDataProviderExt` for inherent data provider tuple.
macro_rules! impl_inherent_data_provider_ext_tuple {
( T, S $(, $TN:ident)* $( , )?) => {
impl<T, S, $( $TN ),*> InherentDataProviderExt for (T, S, $($TN),*)
( S $(, $TN:ident)* $( , )?) => {
impl<S, $( $TN ),*> InherentDataProviderExt for (S, $($TN),*)
where
T: Deref<Target = Timestamp>,
S: Deref<Target = Slot>,
{
fn timestamp(&self) -> Timestamp {
*self.0.deref()
}

fn slot(&self) -> Slot {
*self.1.deref()
*self.0.deref()
}
}
}
}

impl_inherent_data_provider_ext_tuple!(T, S);
impl_inherent_data_provider_ext_tuple!(T, S, A);
impl_inherent_data_provider_ext_tuple!(T, S, A, B);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E, F);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E, F, G);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E, F, G, H);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E, F, G, H, I);
impl_inherent_data_provider_ext_tuple!(T, S, A, B, C, D, E, F, G, H, I, J);
impl_inherent_data_provider_ext_tuple!(S);
impl_inherent_data_provider_ext_tuple!(S, A);
impl_inherent_data_provider_ext_tuple!(S, A, B);
impl_inherent_data_provider_ext_tuple!(S, A, B, C);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D, E);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D, E, F);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D, E, F, G);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D, E, F, G, H);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D, E, F, G, H, I);
impl_inherent_data_provider_ext_tuple!(S, A, B, C, D, E, F, G, H, I, J);

/// Start a new slot worker.
///
Expand Down Expand Up @@ -806,7 +787,6 @@ mod test {
super::slots::SlotInfo {
slot: slot.into(),
duration: SLOT_DURATION,
timestamp: Default::default(),
inherent_data: Default::default(),
ends_at: Instant::now() + SLOT_DURATION,
chain_head: Header::new(
Expand Down
14 changes: 1 addition & 13 deletions client/consensus/slots/src/slots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ pub fn time_until_next_slot(slot_duration: Duration) -> Duration {
pub struct SlotInfo<B: BlockT> {
/// The slot number as found in the inherent data.
pub slot: Slot,
/// Current timestamp as found in the inherent data.
pub timestamp: sp_timestamp::Timestamp,
/// The instant at which the slot ends.
pub ends_at: Instant,
/// The inherent data.
Expand All @@ -72,15 +70,13 @@ impl<B: BlockT> SlotInfo<B> {
/// `ends_at` is calculated using `timestamp` and `duration`.
pub fn new(
slot: Slot,
timestamp: sp_timestamp::Timestamp,
inherent_data: InherentData,
duration: Duration,
chain_head: B::Header,
block_size_limit: Option<usize>,
) -> Self {
Self {
slot,
timestamp,
inherent_data,
duration,
chain_head,
Expand Down Expand Up @@ -175,22 +171,14 @@ where
);
}

let timestamp = inherent_data_providers.timestamp();
let slot = inherent_data_providers.slot();
let inherent_data = inherent_data_providers.create_inherent_data()?;

// never yield the same slot twice.
if slot > self.last_slot {
self.last_slot = slot;

break Ok(SlotInfo::new(
slot,
timestamp,
inherent_data,
self.slot_duration,
chain_head,
None,
))
break Ok(SlotInfo::new(slot, inherent_data, self.slot_duration, chain_head, None))
}
}
}
Expand Down
Loading