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 9 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
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(),
can_author_with: sp_consensus::CanAuthorWithNativeVersion::new(
Expand Down Expand Up @@ -275,7 +275,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 @@ -457,7 +457,7 @@ pub fn new_full_base(
&parent,
)?;

Ok((timestamp, slot, uncles, storage_proof))
Ok((slot, timestamp, uncles, storage_proof))
}
},
force_authoring,
Expand Down
1 change: 1 addition & 0 deletions breaks
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"Breakpoint":{"BKPTOptions":{"AutoContinue":false,"ConditionText":"","EnabledState":true,"IgnoreCount":0,"OneShotState":false},"BKPTResolver":{"Options":{"Column":0,"Exact":false,"FileName":"/mnt/ssd/users/develop/parity/substrate/client/consensus/babe/src/lib.rs","Inlines":true,"LineNumber":1076,"Offset":0,"SkipPrologue":true},"Type":"FileAndLine"},"Hardware":false,"SearchFilter":{"Options":{},"Type":"Unconstrained"}}}]
7 changes: 3 additions & 4 deletions client/consensus/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ mod tests {
dyn CreateInherentDataProviders<
TestBlock,
(),
InherentDataProviders = (TimestampInherentDataProvider, InherentDataProvider),
InherentDataProviders = (InherentDataProvider,),
davxy marked this conversation as resolved.
Show resolved Hide resolved
>,
>,
>;
Expand Down Expand Up @@ -668,7 +668,7 @@ mod tests {
SlotDuration::from_millis(6000),
);

Ok((timestamp, slot))
Ok((slot,))
}),
AlwaysCanAuthor,
CheckForEquivocation::Yes,
Expand Down Expand Up @@ -753,7 +753,7 @@ mod tests {
SlotDuration::from_millis(6000),
);

Ok((timestamp, slot))
Ok((slot, timestamp))
},
force_authoring: false,
backoff_authoring_blocks: Some(
Expand Down Expand Up @@ -887,7 +887,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
6 changes: 3 additions & 3 deletions client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub struct TestVerifier {
dyn CreateInherentDataProviders<
TestBlock,
(),
InherentDataProviders = (TimestampInherentDataProvider, InherentDataProvider),
InherentDataProviders = (InherentDataProvider,),
>,
>,
>,
Expand Down Expand Up @@ -328,7 +328,7 @@ impl TestNetFactory for BabeTestNet {
SlotDuration::from_millis(6000),
);

Ok((timestamp, slot))
Ok((slot,))
}),
config: data.link.config.clone(),
epoch_changes: data.link.epoch_changes.clone(),
Expand Down Expand Up @@ -441,7 +441,7 @@ fn run_one_test(mutator: impl Fn(&mut TestHeader, Stage) + Send + Sync + 'static
SlotDuration::from_millis(6000),
);

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
56 changes: 18 additions & 38 deletions client/consensus/slots/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ use sp_runtime::{
generic::BlockId,
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 @@ -255,7 +254,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 @@ -319,23 +318,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 @@ -443,44 +433,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 @@ -823,7 +804,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
5 changes: 5 additions & 0 deletions lldb.init
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
file target/debug/node-sassafras
#file target/debug/deps/sp_keystore-a8d1167f7eadae2e)
#file target/debug/substrate
#file target/debug/deps/sc_client_db-4f42f24599f45c19
breakpoint read -f breaks
5 changes: 3 additions & 2 deletions primitives/timestamp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,13 @@ impl TimestampInherentData for InherentData {
///
/// This timestamp is the time since the UNIX epoch.
#[cfg(feature = "std")]
fn current_timestamp() -> std::time::Duration {
pub fn current_timestamp() -> Timestamp {
davxy marked this conversation as resolved.
Show resolved Hide resolved
use std::time::SystemTime;

let now = SystemTime::now();
now.duration_since(SystemTime::UNIX_EPOCH)
.expect("Current time is always after unix epoch; qed")
.into()
}

/// Provide duration since unix epoch in millisecond for timestamp inherent.
Expand All @@ -190,7 +191,7 @@ impl InherentDataProvider {
pub fn from_system_time() -> Self {
Self {
max_drift: std::time::Duration::from_secs(60).into(),
timestamp: current_timestamp().into(),
timestamp: current_timestamp(),
}
}

Expand Down
24 changes: 24 additions & 0 deletions run-benches.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash

#BINARY=substrate
BINARY=node-sassafras

TARGET=debug
#TARGET=release

PALLET=pallet_sassafras
EXTRINSIC=submit_tickets
STEPS=50
REPEAT=20

OUTFILE=output.rs

./target/$TARGET/$BINARY benchmark pallet \
--chain dev \
--execution wasm \
--wasm-execution compiled \
--pallet $PALLET \
--extrinsic $EXTRINSIC \
--steps $STEPS \
--repeat $REPEAT \
--output $OUTFILE