From ea642dab64c11f56f0dc00550ccd51926feec3f6 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 11:18:13 +0100 Subject: [PATCH 1/7] type annotations needed error --- Cargo.lock | 2 + client/consensus/aura/src/lib.rs | 13 +++-- client/consensus/babe/src/lib.rs | 4 +- client/consensus/slots/Cargo.toml | 2 + client/consensus/slots/src/lib.rs | 75 ++++++++++++++++++++++----- client/consensus/slots/src/slots.rs | 43 +++++++-------- primitives/consensus/slots/src/lib.rs | 5 ++ 7 files changed, 104 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 962d5f3adbe32..ae0f809cd4ae3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7953,11 +7953,13 @@ dependencies = [ "sp-arithmetic", "sp-blockchain", "sp-consensus", + "sp-consensus-babe", "sp-consensus-slots", "sp-core", "sp-inherents", "sp-runtime", "sp-state-machine", + "sp-timestamp", "substrate-test-runtime-client", "thiserror", ] diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 50a02726cf56a..544861f567522 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -216,12 +216,12 @@ where PF::Proposer: Proposer>, SO: SyncOracle + Send + Sync + Clone, L: sc_consensus::JustificationSyncLink, - CIDP: CreateInherentDataProviders + Send, + CIDP: CreateInherentDataProviders + Send + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send, BS: BackoffAuthoringBlocksStrategy> + Send + Sync + 'static, Error: std::error::Error + Send + From + 'static, { - let worker = build_aura_worker::(BuildAuraWorkerParams { + let worker = build_aura_worker::(BuildAuraWorkerParams { client, block_import, proposer_factory, @@ -283,7 +283,7 @@ pub struct BuildAuraWorkerParams { /// Build the aura worker. /// /// The caller is responsible for running this worker, otherwise it will do nothing. -pub fn build_aura_worker( +pub fn build_aura_worker( BuildAuraWorkerParams { client, block_import, @@ -300,6 +300,7 @@ pub fn build_aura_worker( }: BuildAuraWorkerParams>, ) -> impl sc_consensus_slots::SimpleSlotWorker< B, + CIDP, Proposer = PF::Proposer, BlockImport = I, SyncOracle = SO, @@ -321,6 +322,8 @@ where SO: SyncOracle + Send + Sync + Clone, L: sc_consensus::JustificationSyncLink, BS: BackoffAuthoringBlocksStrategy> + Send + Sync + 'static, + for<'async_trait> CIDP: CreateInherentDataProviders + Send + 'async_trait, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, { AuraWorker { client, @@ -356,7 +359,7 @@ struct AuraWorker { } #[async_trait::async_trait] -impl sc_consensus_slots::SimpleSlotWorker +impl sc_consensus_slots::SimpleSlotWorker for AuraWorker> where B: BlockT, @@ -372,6 +375,8 @@ where L: sc_consensus::JustificationSyncLink, BS: BackoffAuthoringBlocksStrategy> + Send + Sync + 'static, Error: std::error::Error + Send + From + 'static, + for<'async_trait> CIDP: CreateInherentDataProviders + Send + 'async_trait, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, { type BlockImport = I; type SyncOracle = SO; diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 109e5aade02a7..9467eb6dbde7f 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -715,7 +715,7 @@ struct BabeSlotWorker { } #[async_trait::async_trait] -impl sc_consensus_slots::SimpleSlotWorker +impl sc_consensus_slots::SimpleSlotWorker for BabeSlotWorker where B: BlockT, @@ -728,6 +728,8 @@ where L: sc_consensus::JustificationSyncLink, BS: BackoffAuthoringBlocksStrategy> + Sync, Error: std::error::Error + Send + From + From + 'static, + for<'async_trait> CIDP: CreateInherentDataProviders + Send + 'async_trait, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, { type Claim = (PreDigest, AuthorityId); type SyncOracle = SO; diff --git a/client/consensus/slots/Cargo.toml b/client/consensus/slots/Cargo.toml index 4bb9387cf5596..5e3bffa808874 100644 --- a/client/consensus/slots/Cargo.toml +++ b/client/consensus/slots/Cargo.toml @@ -22,6 +22,7 @@ log = "0.4.17" thiserror = "1.0.30" sc-client-api = { version = "4.0.0-dev", path = "../../api" } sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } +sp-consensus-babe = { version = "0.10.0-dev", path = "../../../primitives/consensus/babe" } sc-telemetry = { version = "4.0.0-dev", path = "../../telemetry" } sp-arithmetic = { version = "6.0.0", path = "../../../primitives/arithmetic" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } @@ -30,6 +31,7 @@ sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/conse sp-core = { version = "7.0.0", path = "../../../primitives/core" } sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } sp-runtime = { version = "7.0.0", path = "../../../primitives/runtime" } +sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" } sp-state-machine = { version = "0.13.0", path = "../../../primitives/state-machine" } [dev-dependencies] diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 90bfef6c1609c..724f851587b69 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -42,7 +42,12 @@ 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 std::{fmt::Debug, ops::Deref, time::Duration}; +use sp_inherents::InherentDataProvider; +use std::{ + fmt::Debug, + ops::Deref, + time::{Duration, Instant}, +}; /// The changes that need to applied to the storage to create the state for a block. /// @@ -64,19 +69,33 @@ pub struct SlotResult { /// The implementation should not make any assumptions of the slot being bound to the time or /// similar. The only valid assumption is that the slot number is always increasing. #[async_trait::async_trait] -pub trait SlotWorker { +pub trait SlotWorker +where + B: BlockT, + CIDP: CreateInherentDataProviders + Send, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, +{ /// Called when a new slot is triggered. /// /// Returns a future that resolves to a [`SlotResult`] iff a block was successfully built in /// the slot. Otherwise `None` is returned. - async fn on_slot(&mut self, slot_info: SlotInfo) -> Option>; + async fn on_slot( + &mut self, + slot_info: SlotInfo, + create_inherent_data_providers: &CIDP, + ) -> Option>; } /// A skeleton implementation for `SlotWorker` which tries to claim a slot at /// its beginning and tries to produce a block if successfully claimed, timing /// out if block production takes too long. #[async_trait::async_trait] -pub trait SimpleSlotWorker { +pub trait SimpleSlotWorker +where + B: BlockT, + for<'async_trait> CIDP: CreateInherentDataProviders + Send + 'async_trait, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, +{ /// A handle to a `BlockImport`. type BlockImport: BlockImport>::Transaction> + Send @@ -184,6 +203,7 @@ pub trait SimpleSlotWorker { proposer: Self::Proposer, claim: &Self::Claim, slot_info: SlotInfo, + inherent_data: sp_inherents::InherentData, proposing_remaining: Delay, ) -> Option< Proposal< @@ -203,7 +223,7 @@ pub trait SimpleSlotWorker { // the result to be returned. let proposing = proposer .propose( - slot_info.inherent_data, + inherent_data, sp_runtime::generic::Digest { logs }, proposing_remaining_duration.mul_f32(0.98), None, @@ -246,6 +266,7 @@ pub trait SimpleSlotWorker { async fn on_slot( &mut self, slot_info: SlotInfo, + create_inherent_data_providers: &CIDP, ) -> Option>::Proof>> where Self: Sync, @@ -310,6 +331,28 @@ pub trait SimpleSlotWorker { let claim = self.claim_slot(&slot_info.chain_head, slot, &aux_data).await?; + let inherent_data_providers = create_inherent_data_providers + .create_inherent_data_providers(slot_info.chain_head.hash(), ()) + .await.ok()?; + + if Instant::now() > slot_info.ends_at { + log::warn!( + target: "slots", + "Creating inherent data providers took more time than we had left for the slot.", + ); + } + + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + + let slot = + sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + SlotDuration::from_duration(slot_info.duration), + ) + .slot(); + + let inherent_data = inherent_data_providers.create_inherent_data().ok()?; + if self.should_backoff(slot, &slot_info.chain_head) { return None } @@ -335,7 +378,10 @@ pub trait SimpleSlotWorker { }, }; - let proposal = self.propose(proposer, &claim, slot_info, proposing_remaining).await?; + // use inherent_data here + let proposal = self + .propose(proposer, &claim, slot_info, inherent_data, proposing_remaining) + .await?; let (block, storage_proof) = (proposal.block, proposal.proof); let (header, body) = block.deconstruct(); @@ -416,14 +462,19 @@ pub trait SimpleSlotWorker { pub struct SimpleSlotWorkerToSlotWorker(pub T); #[async_trait::async_trait] -impl + Send + Sync, B: BlockT> - SlotWorker>::Proof> for SimpleSlotWorkerToSlotWorker +impl SlotWorker>::Proof> for SimpleSlotWorkerToSlotWorker +where + T: SimpleSlotWorker + Send + Sync, + B: BlockT, + for<'async_trait> CIDP: CreateInherentDataProviders + Send + 'async_trait, + CIDP::InherentDataProviders: InherentDataProviderExt + Send, { async fn on_slot( &mut self, slot_info: SlotInfo, + create_inherent_data_providers: &CIDP, ) -> Option>::Proof>> { - self.0.on_slot(slot_info).await + self.0.on_slot(slot_info, create_inherent_data_providers).await } } @@ -472,12 +523,12 @@ pub async fn start_slot_worker( ) where B: BlockT, C: SelectChain, - W: SlotWorker, + W: SlotWorker, SO: SyncOracle + Send, CIDP: CreateInherentDataProviders + Send, CIDP::InherentDataProviders: InherentDataProviderExt + Send, { - let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client); + let mut slots = Slots::new(slot_duration.as_duration(), client); loop { let slot_info = match slots.next_slot().await { @@ -493,7 +544,7 @@ pub async fn start_slot_worker( continue } - let _ = worker.on_slot(slot_info).await; + let _ = worker.on_slot(slot_info, &create_inherent_data_providers).await; } } diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index f3dc485a8e819..0e00ee6325fe9 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -22,6 +22,7 @@ use super::{InherentDataProviderExt, Slot}; use sp_consensus::{Error, SelectChain}; +use sp_consensus_slots::SlotDuration; use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; @@ -52,8 +53,6 @@ pub struct SlotInfo { pub slot: Slot, /// The instant at which the slot ends. pub ends_at: Instant, - /// The inherent data. - pub inherent_data: InherentData, /// Slot duration. pub duration: Duration, /// The chain header this slot is based on. @@ -70,14 +69,12 @@ impl SlotInfo { /// `ends_at` is calculated using `timestamp` and `duration`. pub fn new( slot: Slot, - inherent_data: InherentData, duration: Duration, chain_head: B::Header, block_size_limit: Option, ) -> Self { Self { slot, - inherent_data, duration, chain_head, block_size_limit, @@ -87,39 +84,31 @@ impl SlotInfo { } /// A stream that returns every time there is a new slot. -pub(crate) struct Slots { +pub(crate) struct Slots { last_slot: Slot, slot_duration: Duration, inner_delay: Option, - create_inherent_data_providers: IDP, select_chain: SC, _phantom: std::marker::PhantomData, } -impl Slots { +impl Slots { /// Create a new `Slots` stream. - pub fn new( - slot_duration: Duration, - create_inherent_data_providers: IDP, - select_chain: SC, - ) -> Self { + pub fn new(slot_duration: Duration, select_chain: SC) -> Self { Slots { last_slot: 0.into(), slot_duration, inner_delay: None, - create_inherent_data_providers, select_chain, _phantom: Default::default(), } } } -impl Slots +impl Slots where Block: BlockT, SC: SelectChain, - IDP: CreateInherentDataProviders, - IDP::InherentDataProviders: crate::InherentDataProviderExt, { /// Returns a future that fires when the next slot starts. pub async fn next_slot(&mut self) -> Result, Error> { @@ -159,10 +148,10 @@ where }, }; - let inherent_data_providers = self - .create_inherent_data_providers - .create_inherent_data_providers(chain_head.hash(), ()) - .await?; + //let inherent_data_providers = self + // .create_inherent_data_providers + // .create_inherent_data_providers(chain_head.hash(), ()) + // .await?; if Instant::now() > ends_at { log::warn!( @@ -171,14 +160,22 @@ where ); } - let slot = inherent_data_providers.slot(); - let inherent_data = inherent_data_providers.create_inherent_data()?; + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + + let slot = + sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + SlotDuration::from_duration(self.slot_duration), + ).slot(); + + //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, inherent_data, self.slot_duration, chain_head, None)) + break Ok(SlotInfo::new(slot, self.slot_duration, chain_head, None)) } } } diff --git a/primitives/consensus/slots/src/lib.rs b/primitives/consensus/slots/src/lib.rs index 21b3cad1e7167..c9dac7520c52e 100644 --- a/primitives/consensus/slots/src/lib.rs +++ b/primitives/consensus/slots/src/lib.rs @@ -124,6 +124,11 @@ impl SlotDuration { pub const fn as_duration(&self) -> sp_std::time::Duration { sp_std::time::Duration::from_millis(self.0) } + + /// Initiliase from given [`sp_std::time::Duration`]. + pub const fn from_duration(duration: sp_std::time::Duration) -> Self { + Self::from_millis(duration.as_millis() as u64) // OH GOD NO + } } /// Represents an equivocation proof. An equivocation happens when a validator From ab34534da77c3fa0e1ec785fd935b54515680455 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 12:02:33 +0100 Subject: [PATCH 2/7] hard coded logging targets --- client/consensus/aura/src/lib.rs | 6 ++++-- client/consensus/babe/src/lib.rs | 7 +++++-- client/consensus/slots/src/lib.rs | 9 +++++---- client/consensus/slots/src/slots.rs | 5 ++--- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 544861f567522..c06f4ad187b13 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -494,7 +494,8 @@ where chain_head_slot, self.client.info().finalized_number, slot, - self.logging_target(), + "aura", + //::Header as HeaderT>::Number> as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), ) } } @@ -529,7 +530,8 @@ where &self.block_proposal_slot_portion, self.max_block_proposal_slot_portion.as_ref(), sc_consensus_slots::SlotLenienceType::Exponential, - self.logging_target(), + "aura", + //::Header as HeaderT>::Number> as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), ) } } diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 9467eb6dbde7f..c3c6e58c88cdc 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -874,7 +874,7 @@ where chain_head_slot, self.client.info().finalized_number, slot, - self.logging_target(), + as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), ) } } @@ -910,7 +910,10 @@ where &self.block_proposal_slot_portion, self.max_block_proposal_slot_portion.as_ref(), sc_consensus_slots::SlotLenienceType::Exponential, - self.logging_target(), + as sc_consensus_slots::SimpleSlotWorker< + B, + CIDP, + >>::logging_target(self), ) } } diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 724f851587b69..6b71bdc2fab5b 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -40,9 +40,8 @@ use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO, use sp_arithmetic::traits::BaseArithmetic; use sp_consensus::{Proposal, Proposer, SelectChain, SyncOracle}; use sp_consensus_slots::{Slot, SlotDuration}; -use sp_inherents::CreateInherentDataProviders; +use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_runtime::traits::{Block as BlockT, HashFor, Header as HeaderT}; -use sp_inherents::InherentDataProvider; use std::{ fmt::Debug, ops::Deref, @@ -333,7 +332,8 @@ where let inherent_data_providers = create_inherent_data_providers .create_inherent_data_providers(slot_info.chain_head.hash(), ()) - .await.ok()?; + .await + .ok()?; if Instant::now() > slot_info.ends_at { log::warn!( @@ -462,7 +462,8 @@ where pub struct SimpleSlotWorkerToSlotWorker(pub T); #[async_trait::async_trait] -impl SlotWorker>::Proof> for SimpleSlotWorkerToSlotWorker +impl SlotWorker>::Proof> + for SimpleSlotWorkerToSlotWorker where T: SimpleSlotWorker + Send + Sync, B: BlockT, diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index 0e00ee6325fe9..2e42453726595 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -20,11 +20,10 @@ //! //! This is used instead of `futures_timer::Interval` because it was unreliable. -use super::{InherentDataProviderExt, Slot}; +use super::Slot; use sp_consensus::{Error, SelectChain}; use sp_consensus_slots::SlotDuration; -use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider}; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_runtime::traits::Block as BlockT; use futures_timer::Delay; use std::time::{Duration, Instant}; From 1c8bd81ce7204aee2d425adbce285f7558705b2f Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 14:00:50 +0100 Subject: [PATCH 3/7] Remove dead code --- client/consensus/slots/src/slots.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index 2e42453726595..e51b416499fe4 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -131,8 +131,6 @@ where // reschedule delay for next slot. self.inner_delay = Some(Delay::new(ends_in)); - let ends_at = Instant::now() + ends_in; - let chain_head = match self.select_chain.best_chain().await { Ok(x) => x, Err(e) => { @@ -147,29 +145,13 @@ where }, }; - //let inherent_data_providers = self - // .create_inherent_data_providers - // .create_inherent_data_providers(chain_head.hash(), ()) - // .await?; - - if Instant::now() > ends_at { - log::warn!( - target: "slots", - "Creating inherent data providers took more time than we had left for the slot.", - ); - } - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( *timestamp, SlotDuration::from_duration(self.slot_duration), ).slot(); - //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; From 3b5b476e2ec017958169fb6d480070d67db10404 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 15:39:49 +0100 Subject: [PATCH 4/7] refactor --- client/consensus/aura/src/lib.rs | 2 +- client/consensus/slots/src/lib.rs | 51 ++++++++++++++------------- client/consensus/slots/src/slots.rs | 32 +++++++++++++---- primitives/consensus/slots/src/lib.rs | 7 ++-- 4 files changed, 58 insertions(+), 34 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index c06f4ad187b13..733fd950ed34d 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -494,7 +494,7 @@ where chain_head_slot, self.client.info().finalized_number, slot, - "aura", + self.logging_target(), //::Header as HeaderT>::Number> as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), ) } diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 6b71bdc2fab5b..699125f8488b3 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -39,8 +39,9 @@ use sc_consensus::{BlockImport, JustificationSyncLink}; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO, CONSENSUS_WARN}; use sp_arithmetic::traits::BaseArithmetic; use sp_consensus::{Proposal, Proposer, SelectChain, SyncOracle}; +use sp_consensus_babe::inherents::InherentType; use sp_consensus_slots::{Slot, SlotDuration}; -use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; +use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider}; use sp_runtime::traits::{Block as BlockT, HashFor, Header as HeaderT}; use std::{ fmt::Debug, @@ -330,28 +331,9 @@ where let claim = self.claim_slot(&slot_info.chain_head, slot, &aux_data).await?; - let inherent_data_providers = create_inherent_data_providers - .create_inherent_data_providers(slot_info.chain_head.hash(), ()) - .await - .ok()?; - - if Instant::now() > slot_info.ends_at { - log::warn!( - target: "slots", - "Creating inherent data providers took more time than we had left for the slot.", - ); - } - - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - - let slot = - sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - SlotDuration::from_duration(slot_info.duration), - ) - .slot(); - - let inherent_data = inherent_data_providers.create_inherent_data().ok()?; + let (inherent_data, slot) = + Self::extract_inherent_data_and_slot(&slot_info, create_inherent_data_providers) + .await?; if self.should_backoff(slot, &slot_info.chain_head) { return None @@ -378,7 +360,6 @@ where }, }; - // use inherent_data here let proposal = self .propose(proposer, &claim, slot_info, inherent_data, proposing_remaining) .await?; @@ -452,6 +433,28 @@ where Some(SlotResult { block: B::new(header, body), storage_proof }) } + + /// Creates inherent data and returns it and its slot. + async fn extract_inherent_data_and_slot( + slot_info: &SlotInfo, + create_inherent_data_providers: &CIDP, + ) -> Option<(InherentData, InherentType)> { + let inherent_data_providers = create_inherent_data_providers + .create_inherent_data_providers(slot_info.chain_head.hash(), ()) + .await + .ok()?; + + if Instant::now() > slot_info.ends_at { + log::warn!( + target: "slots", + "Creating inherent data providers took more time than we had left for the slot.", + ); + } + + let inherent_data = inherent_data_providers.create_inherent_data().ok()?; + + Some((inherent_data, inherent_data_providers.slot())) + } } /// A type that implements [`SlotWorker`] for a type that implements [`SimpleSlotWorker`]. diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index e51b416499fe4..d1a2f27284322 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -26,6 +26,7 @@ use sp_consensus_slots::SlotDuration; use sp_runtime::traits::Block as BlockT; use futures_timer::Delay; +use sp_consensus_babe::inherents::InherentType; use std::time::{Duration, Instant}; /// Returns current duration since unix epoch. @@ -145,12 +146,18 @@ where }, }; - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); - let slot = - sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( - *timestamp, - SlotDuration::from_duration(self.slot_duration), - ).slot(); + let slot = match self.get_slot() { + Some(x) => x, + None => { + log::warn!( + target: "slots", + "Unable to get_block in next_slot. Duration conversion error ", + ); + // Let's try at the next slot.. + self.inner_delay.take(); + continue + }, + }; // never yield the same slot twice. if slot > self.last_slot { @@ -160,4 +167,17 @@ where } } } + + fn get_slot(&mut self) -> Option { + let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); + let slot_duration = SlotDuration::from_duration(self.slot_duration)?; + let slot = + sp_consensus_babe::inherents::InherentDataProvider::from_timestamp_and_slot_duration( + *timestamp, + slot_duration, + ) + .slot(); + + Some(slot) + } } diff --git a/primitives/consensus/slots/src/lib.rs b/primitives/consensus/slots/src/lib.rs index c9dac7520c52e..2db7a2552ffad 100644 --- a/primitives/consensus/slots/src/lib.rs +++ b/primitives/consensus/slots/src/lib.rs @@ -125,9 +125,10 @@ impl SlotDuration { sp_std::time::Duration::from_millis(self.0) } - /// Initiliase from given [`sp_std::time::Duration`]. - pub const fn from_duration(duration: sp_std::time::Duration) -> Self { - Self::from_millis(duration.as_millis() as u64) // OH GOD NO + /// Initialise from given [`sp_std::time::Duration`]. + pub fn from_duration(duration: sp_std::time::Duration) -> Option { + let millis: u64 = duration.as_millis().try_into().ok()?; + Some(Self::from_millis(millis)) } } From 3050406d2e67efd761cc1d7de7cad2b0e2b89364 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 15:41:15 +0100 Subject: [PATCH 5/7] minor --- client/consensus/slots/src/slots.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index d1a2f27284322..19d6987cc7d88 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -151,7 +151,7 @@ where None => { log::warn!( target: "slots", - "Unable to get_block in next_slot. Duration conversion error ", + "Unable to get_slot in next_slot. Duration conversion error ", ); // Let's try at the next slot.. self.inner_delay.take(); From bbb6525163e0c13e4319fdaad7639cc195794e04 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 17:22:56 +0100 Subject: [PATCH 6/7] Fix last type error --- client/consensus/aura/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 733fd950ed34d..deb6bd4eb6753 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -494,8 +494,7 @@ where chain_head_slot, self.client.info().finalized_number, slot, - self.logging_target(), - //::Header as HeaderT>::Number> as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), + ::Header as sp_api::HeaderT>::Number> as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), ) } } From 7d91b13d8f5a027aefd2ce846959a9c10c8d263b Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 16 Nov 2022 17:23:29 +0100 Subject: [PATCH 7/7] cargo fmt --- client/consensus/aura/src/lib.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index deb6bd4eb6753..8a038e3cf2a9e 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -494,7 +494,16 @@ where chain_head_slot, self.client.info().finalized_number, slot, - ::Header as sp_api::HeaderT>::Number> as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), + ::Header as sp_api::HeaderT>::Number, + > as sc_consensus_slots::SimpleSlotWorker>::logging_target(self), ) } }