From 7b82edf6ce75a71eb03a983e22f72f7f808ab173 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Fri, 18 Nov 2022 16:40:08 +0100 Subject: [PATCH 01/28] Move create_inherent_data call to use side --- client/consensus/aura/src/lib.rs | 4 ++-- client/consensus/slots/src/lib.rs | 7 ++++--- client/consensus/slots/src/slots.rs | 19 ++++++++++++------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 50a02726cf56a..d3a0988a0ae3d 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -216,7 +216,7 @@ 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, @@ -960,7 +960,7 @@ mod tests { let res = executor::block_on(worker.on_slot(SlotInfo { slot: 0.into(), ends_at: Instant::now() + Duration::from_secs(100), - inherent_data: InherentData::new(), + create_inherent_data: InherentData::new(), duration: Duration::from_millis(1000), chain_head: head, block_size_limit: None, diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 90bfef6c1609c..24e5afbd9cb98 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -201,9 +201,10 @@ pub trait SimpleSlotWorker { // deadline our production to 98% of the total time left for proposing. As we deadline // the proposing below to the same total time left, the 2% margin should be enough for // the result to be returned. + let inherent_data = slot_info.create_inherent_data.create_inherent_data().ok()?; let proposing = proposer .propose( - slot_info.inherent_data, + inherent_data, sp_runtime::generic::Digest { logs }, proposing_remaining_duration.mul_f32(0.98), None, @@ -474,7 +475,7 @@ pub async fn start_slot_worker( C: SelectChain, W: SlotWorker, SO: SyncOracle + Send, - CIDP: CreateInherentDataProviders + Send, + CIDP: CreateInherentDataProviders + Send + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send, { let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client); @@ -786,7 +787,7 @@ mod test { super::slots::SlotInfo { slot: slot.into(), duration: SLOT_DURATION, - inherent_data: Default::default(), + create_inherent_data: Default::default(), ends_at: Instant::now() + SLOT_DURATION, chain_head: Header::new( 1, diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index f3dc485a8e819..a372c5b622141 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -52,8 +52,8 @@ pub struct SlotInfo { pub slot: Slot, /// The instant at which the slot ends. pub ends_at: Instant, - /// The inherent data. - pub inherent_data: InherentData, + /// The inherent data provider. + pub create_inherent_data: Box, /// Slot duration. pub duration: Duration, /// The chain header this slot is based on. @@ -70,14 +70,14 @@ impl SlotInfo { /// `ends_at` is calculated using `timestamp` and `duration`. pub fn new( slot: Slot, - inherent_data: InherentData, + inherent_data: Box, duration: Duration, chain_head: B::Header, block_size_limit: Option, ) -> Self { Self { slot, - inherent_data, + create_inherent_data: inherent_data, duration, chain_head, block_size_limit, @@ -118,7 +118,7 @@ impl Slots where Block: BlockT, SC: SelectChain, - IDP: CreateInherentDataProviders, + IDP: CreateInherentDataProviders + 'static, IDP::InherentDataProviders: crate::InherentDataProviderExt, { /// Returns a future that fires when the next slot starts. @@ -172,13 +172,18 @@ where } 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, + Box::new(inherent_data_providers), + self.slot_duration, + chain_head, + None, + )) } } } From 4512834089d37eb2213adfce1be8d75d4b457a48 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sun, 20 Nov 2022 19:26:09 +0100 Subject: [PATCH 02/28] Make provide_inherent_data async --- Cargo.lock | 1 + bin/node-template/node/Cargo.toml | 1 + bin/node-template/node/src/benchmarking.rs | 9 ++++++--- bin/node/bench/src/construct.rs | 4 +++- bin/node/cli/src/benchmarking.rs | 9 ++++++--- client/consensus/aura/src/import_queue.rs | 1 + client/consensus/babe/src/lib.rs | 1 + .../consensus/manual-seal/src/consensus/timestamp.rs | 2 +- client/consensus/manual-seal/src/seal_block.rs | 2 +- client/consensus/pow/src/lib.rs | 3 ++- client/consensus/slots/src/lib.rs | 2 +- client/consensus/slots/src/slots.rs | 2 +- primitives/authorship/src/lib.rs | 2 +- primitives/consensus/aura/src/inherents.rs | 2 +- primitives/consensus/babe/src/inherents.rs | 2 +- primitives/inherents/src/client_side.rs | 10 +++++----- primitives/timestamp/src/lib.rs | 2 +- primitives/transaction-storage-proof/src/lib.rs | 2 +- 18 files changed, 35 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a22cfa8ba8dd6..a964200adca11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4699,6 +4699,7 @@ dependencies = [ "frame-benchmarking", "frame-benchmarking-cli", "frame-system", + "futures", "jsonrpsee", "node-template-runtime", "pallet-transaction-payment", diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 16f87470dbc17..9791df3a18545 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -18,6 +18,7 @@ name = "node-template" [dependencies] clap = { version = "4.0.9", features = ["derive"] } +futures = { version = "0.3.21", features = ["thread-pool"] } sc-cli = { version = "0.10.0-dev", path = "../../../client/cli", features = ["wasmtime"] } sp-core = { version = "7.0.0", path = "../../../primitives/core" } diff --git a/bin/node-template/node/src/benchmarking.rs b/bin/node-template/node/src/benchmarking.rs index 90fe06edf04b8..739336bc1176e 100644 --- a/bin/node-template/node/src/benchmarking.rs +++ b/bin/node-template/node/src/benchmarking.rs @@ -176,8 +176,11 @@ pub fn inherent_benchmark_data() -> Result { let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - timestamp - .provide_inherent_data(&mut inherent_data) - .map_err(|e| format!("creating inherent data: {:?}", e))?; + use futures::TryFutureExt; + futures::executor::block_on( + timestamp + .provide_inherent_data(&mut inherent_data) + .map_err(|e| format!("creating inherent data: {:?}", e)), + )?; Ok(inherent_data) } diff --git a/bin/node/bench/src/construct.rs b/bin/node/bench/src/construct.rs index 50b9db9f019d1..2e20a7acb1e38 100644 --- a/bin/node/bench/src/construct.rs +++ b/bin/node/bench/src/construct.rs @@ -150,8 +150,10 @@ impl core::Benchmark for ConstructionBenchmark { ) .expect("Proposer initialization failed"); + let inherent_data = futures::executor::block_on(timestamp_provider.create_inherent_data()) + .expect("Create inherent data failed"); let _block = futures::executor::block_on(proposer.propose( - timestamp_provider.create_inherent_data().expect("Create inherent data failed"), + inherent_data, Default::default(), std::time::Duration::from_secs(20), None, diff --git a/bin/node/cli/src/benchmarking.rs b/bin/node/cli/src/benchmarking.rs index 19bd1660a4dd9..8a237706b5074 100644 --- a/bin/node/cli/src/benchmarking.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -116,8 +116,11 @@ pub fn inherent_benchmark_data() -> Result { let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - timestamp - .provide_inherent_data(&mut inherent_data) - .map_err(|e| format!("creating inherent data: {:?}", e))?; + use futures::TryFutureExt; + futures::executor::block_on( + timestamp + .provide_inherent_data(&mut inherent_data) + .map_err(|e| format!("creating inherent data: {:?}", e)), + )?; Ok(inherent_data) } diff --git a/client/consensus/aura/src/import_queue.rs b/client/consensus/aura/src/import_queue.rs index b17feae45897e..07f982542c95b 100644 --- a/client/consensus/aura/src/import_queue.rs +++ b/client/consensus/aura/src/import_queue.rs @@ -203,6 +203,7 @@ where let mut inherent_data = create_inherent_data_providers .create_inherent_data() + .await .map_err(Error::::Inherent)?; let slot_now = create_inherent_data_providers.slot(); diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 109e5aade02a7..ac5bce9ee6083 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -1238,6 +1238,7 @@ where // timestamp in the inherents actually matches the slot set in the seal. let mut inherent_data = create_inherent_data_providers .create_inherent_data() + .await .map_err(Error::::CreateInherents)?; inherent_data.babe_replace_inherent_data(slot); diff --git a/client/consensus/manual-seal/src/consensus/timestamp.rs b/client/consensus/manual-seal/src/consensus/timestamp.rs index f899b80d6c9af..2cb9887a81f40 100644 --- a/client/consensus/manual-seal/src/consensus/timestamp.rs +++ b/client/consensus/manual-seal/src/consensus/timestamp.rs @@ -141,7 +141,7 @@ impl SlotTimestampProvider { #[async_trait::async_trait] impl InherentDataProvider for SlotTimestampProvider { - fn provide_inherent_data( + async fn provide_inherent_data( &self, inherent_data: &mut InherentData, ) -> Result<(), sp_inherents::Error> { diff --git a/client/consensus/manual-seal/src/seal_block.rs b/client/consensus/manual-seal/src/seal_block.rs index 32e3acf68506e..25c091bf460ec 100644 --- a/client/consensus/manual-seal/src/seal_block.rs +++ b/client/consensus/manual-seal/src/seal_block.rs @@ -113,7 +113,7 @@ pub async fn seal_block( .await .map_err(|e| Error::Other(e))?; - let inherent_data = inherent_data_providers.create_inherent_data()?; + let inherent_data = inherent_data_providers.create_inherent_data().await?; let proposer = env.init(&parent).map_err(|err| Error::StringError(err.to_string())).await?; let inherents_len = inherent_data.len(); diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index dcf069d617bab..ac7ce3b411333 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -275,6 +275,7 @@ where let inherent_data = inherent_data_providers .create_inherent_data() + .await .map_err(|e| Error::CreateInherents(e))?; let inherent_res = self @@ -585,7 +586,7 @@ where }, }; - let inherent_data = match inherent_data_providers.create_inherent_data() { + let inherent_data = match inherent_data_providers.create_inherent_data().await { Ok(r) => r, Err(e) => { warn!( diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 24e5afbd9cb98..6caaf174a8d75 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -201,7 +201,7 @@ pub trait SimpleSlotWorker { // deadline our production to 98% of the total time left for proposing. As we deadline // the proposing below to the same total time left, the 2% margin should be enough for // the result to be returned. - let inherent_data = slot_info.create_inherent_data.create_inherent_data().ok()?; + let inherent_data = slot_info.create_inherent_data.create_inherent_data().await.ok()?; let proposing = proposer .propose( inherent_data, diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index a372c5b622141..df4ce937d9581 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -22,7 +22,7 @@ use super::{InherentDataProviderExt, Slot}; use sp_consensus::{Error, SelectChain}; -use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider}; +use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use futures_timer::Delay; diff --git a/primitives/authorship/src/lib.rs b/primitives/authorship/src/lib.rs index 7ea19d9ea5ff5..6ff6607a896cc 100644 --- a/primitives/authorship/src/lib.rs +++ b/primitives/authorship/src/lib.rs @@ -81,7 +81,7 @@ impl InherentDataProvider { #[cfg(feature = "std")] #[async_trait::async_trait] impl sp_inherents::InherentDataProvider for InherentDataProvider { - fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { + async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { inherent_data.put_data(INHERENT_IDENTIFIER, &self.uncles) } diff --git a/primitives/consensus/aura/src/inherents.rs b/primitives/consensus/aura/src/inherents.rs index ce3d832c78ee9..135ae2660b32f 100644 --- a/primitives/consensus/aura/src/inherents.rs +++ b/primitives/consensus/aura/src/inherents.rs @@ -80,7 +80,7 @@ impl sp_std::ops::Deref for InherentDataProvider { #[cfg(feature = "std")] #[async_trait::async_trait] impl sp_inherents::InherentDataProvider for InherentDataProvider { - fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { + async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { inherent_data.put_data(INHERENT_IDENTIFIER, &self.slot) } diff --git a/primitives/consensus/babe/src/inherents.rs b/primitives/consensus/babe/src/inherents.rs index c26dc514ae158..2634507968f8e 100644 --- a/primitives/consensus/babe/src/inherents.rs +++ b/primitives/consensus/babe/src/inherents.rs @@ -86,7 +86,7 @@ impl sp_std::ops::Deref for InherentDataProvider { #[cfg(feature = "std")] #[async_trait::async_trait] impl sp_inherents::InherentDataProvider for InherentDataProvider { - fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { + async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { inherent_data.put_data(INHERENT_IDENTIFIER, &self.slot) } diff --git a/primitives/inherents/src/client_side.rs b/primitives/inherents/src/client_side.rs index 42068e24e029c..1ece7e1e4dea3 100644 --- a/primitives/inherents/src/client_side.rs +++ b/primitives/inherents/src/client_side.rs @@ -83,16 +83,16 @@ pub trait InherentDataProvider: Send + Sync { /// Convenience function for creating [`InherentData`]. /// /// Basically maps around [`Self::provide_inherent_data`]. - fn create_inherent_data(&self) -> Result { + async fn create_inherent_data(&self) -> Result { let mut inherent_data = InherentData::new(); - self.provide_inherent_data(&mut inherent_data)?; + self.provide_inherent_data(&mut inherent_data).await?; Ok(inherent_data) } /// Provide inherent data that should be included in a block. /// /// The data should be stored in the given `InherentData` structure. - fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error>; + async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error>; /// Convert the given encoded error to a string. /// @@ -108,8 +108,8 @@ pub trait InherentDataProvider: Send + Sync { #[async_trait::async_trait] impl InherentDataProvider for Tuple { for_tuples!( where #( Tuple: Send + Sync )* ); - fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { - for_tuples!( #( Tuple.provide_inherent_data(inherent_data)?; )* ); + async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { + for_tuples!( #( Tuple.provide_inherent_data(inherent_data).await?; )* ); Ok(()) } diff --git a/primitives/timestamp/src/lib.rs b/primitives/timestamp/src/lib.rs index d88b1839babe6..14b06779340f2 100644 --- a/primitives/timestamp/src/lib.rs +++ b/primitives/timestamp/src/lib.rs @@ -228,7 +228,7 @@ impl sp_std::ops::Deref for InherentDataProvider { #[cfg(feature = "std")] #[async_trait::async_trait] impl sp_inherents::InherentDataProvider for InherentDataProvider { - fn provide_inherent_data( + async fn provide_inherent_data( &self, inherent_data: &mut InherentData, ) -> Result<(), sp_inherents::Error> { diff --git a/primitives/transaction-storage-proof/src/lib.rs b/primitives/transaction-storage-proof/src/lib.rs index fde84c1c58b1a..43928c83f21ed 100644 --- a/primitives/transaction-storage-proof/src/lib.rs +++ b/primitives/transaction-storage-proof/src/lib.rs @@ -87,7 +87,7 @@ impl InherentDataProvider { #[cfg(feature = "std")] #[async_trait::async_trait] impl sp_inherents::InherentDataProvider for InherentDataProvider { - fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { + async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { if let Some(proof) = &self.proof { inherent_data.put_data(INHERENT_IDENTIFIER, proof) } else { From 207d38fb7b45f2e016e34a5e767749a8dfcda524 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sun, 20 Nov 2022 20:08:39 +0100 Subject: [PATCH 03/28] Fix tests --- primitives/inherents/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/primitives/inherents/src/lib.rs b/primitives/inherents/src/lib.rs index a3ef963c47b39..59db5ef3e8802 100644 --- a/primitives/inherents/src/lib.rs +++ b/primitives/inherents/src/lib.rs @@ -56,7 +56,7 @@ //! //! #[async_trait::async_trait] //! impl sp_inherents::InherentDataProvider for InherentDataProvider { -//! fn provide_inherent_data( +//! async fn provide_inherent_data( //! &self, //! inherent_data: &mut InherentData, //! ) -> Result<(), sp_inherents::Error> { @@ -106,7 +106,7 @@ //! # struct InherentDataProvider; //! # #[async_trait::async_trait] //! # impl sp_inherents::InherentDataProvider for InherentDataProvider { -//! # fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), sp_inherents::Error> { +//! # async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), sp_inherents::Error> { //! # inherent_data.put_data(INHERENT_IDENTIFIER, &"hello") //! # } //! # async fn try_handle_error( @@ -443,7 +443,7 @@ mod tests { #[async_trait::async_trait] impl InherentDataProvider for TestInherentDataProvider { - fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { + async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { data.put_data(TEST_INHERENT_0, &42) } @@ -460,7 +460,7 @@ mod tests { fn create_inherent_data() { let provider = TestInherentDataProvider; - let inherent_data = provider.create_inherent_data().unwrap(); + let inherent_data = futures::executor::block_on(provider.create_inherent_data()).unwrap(); assert_eq!(inherent_data.get_data::(&TEST_INHERENT_0).unwrap().unwrap(), 42u32); } From db7da78a99eb6f1e2b6caeb9e93bfe50f54d8bff Mon Sep 17 00:00:00 2001 From: alexgparity <115470171+alexgparity@users.noreply.github.com> Date: Sun, 20 Nov 2022 23:15:18 +0100 Subject: [PATCH 04/28] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- bin/node-template/node/src/benchmarking.rs | 6 ++---- bin/node/cli/src/benchmarking.rs | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/bin/node-template/node/src/benchmarking.rs b/bin/node-template/node/src/benchmarking.rs index 739336bc1176e..519f10e6a2c32 100644 --- a/bin/node-template/node/src/benchmarking.rs +++ b/bin/node-template/node/src/benchmarking.rs @@ -176,11 +176,9 @@ pub fn inherent_benchmark_data() -> Result { let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - use futures::TryFutureExt; futures::executor::block_on( timestamp - .provide_inherent_data(&mut inherent_data) - .map_err(|e| format!("creating inherent data: {:?}", e)), - )?; + .provide_inherent_data(&mut inherent_data), + ).map_err(|e| format!("creating inherent data: {:?}", e))?; Ok(inherent_data) } diff --git a/bin/node/cli/src/benchmarking.rs b/bin/node/cli/src/benchmarking.rs index 8a237706b5074..c97e1809e81f2 100644 --- a/bin/node/cli/src/benchmarking.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -116,11 +116,9 @@ pub fn inherent_benchmark_data() -> Result { let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - use futures::TryFutureExt; futures::executor::block_on( timestamp - .provide_inherent_data(&mut inherent_data) - .map_err(|e| format!("creating inherent data: {:?}", e)), - )?; + .provide_inherent_data(&mut inherent_data), + ).map_err(|e| format!("creating inherent data: {:?}", e))?; Ok(inherent_data) } From b547ac3df3024998ef0043d0e120f74a27c8923e Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sun, 20 Nov 2022 23:15:37 +0100 Subject: [PATCH 05/28] Log errors --- client/consensus/slots/src/lib.rs | 35 ++++++++++++++++++++++++++++- client/consensus/slots/src/slots.rs | 10 --------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 6caaf174a8d75..45aec6f2c1e5a 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -31,6 +31,7 @@ mod slots; pub use aux_schema::{check_equivocation, MAX_SLOT_CAPACITY, PRUNING_BOUND}; pub use slots::SlotInfo; use slots::Slots; +use std::time::Instant; use futures::{future::Either, Future, TryFutureExt}; use futures_timer::Delay; @@ -195,13 +196,15 @@ pub trait SimpleSlotWorker { let slot = slot_info.slot; let telemetry = self.telemetry(); let logging_target = self.logging_target(); + + let inherent_data = Self::create_inherent_data(&slot_info, &logging_target).await?; + let proposing_remaining_duration = self.proposing_remaining_duration(&slot_info); let logs = self.pre_digest_data(slot, claim); // deadline our production to 98% of the total time left for proposing. As we deadline // the proposing below to the same total time left, the 2% margin should be enough for // the result to be returned. - let inherent_data = slot_info.create_inherent_data.create_inherent_data().await.ok()?; let proposing = proposer .propose( inherent_data, @@ -243,6 +246,36 @@ pub trait SimpleSlotWorker { Some(proposal) } + /// Calls `create_inherent_data` and handles errors. + async fn create_inherent_data( + slot_info: &SlotInfo, + logging_target: &str, + ) -> Option { + let inherent_data = match slot_info.create_inherent_data.create_inherent_data().await { + Ok(data) => data, + Err(err) => { + warn!( + target: logging_target, + "Unable to create inherent data for for block {:?}: {}", + slot_info.chain_head.hash(), + err, + ); + return None + }, + }; + + if Instant::now() > slot_info.ends_at { + warn!( + target: "slots", + "Creating inherent data took more time than we had left for the slot.", + ); + + return None + } + + Some(inherent_data) + } + /// Implements [`SlotWorker::on_slot`]. async fn on_slot( &mut self, diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index df4ce937d9581..a8de7f0345081 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -142,9 +142,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) => { @@ -164,13 +161,6 @@ where .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 slot = inherent_data_providers.slot(); // never yield the same slot twice. From 9ec8b852e99c1871882bfce1adc9f9dec14d565b Mon Sep 17 00:00:00 2001 From: alexgparity Date: Mon, 21 Nov 2022 11:42:47 +0100 Subject: [PATCH 06/28] Fix test --- Cargo.lock | 1 + bin/node-template/node/src/benchmarking.rs | 6 ++---- bin/node/cli/src/benchmarking.rs | 6 ++---- client/consensus/slots/src/lib.rs | 24 +++++++++++++++++++++- test-utils/runtime/Cargo.toml | 1 + test-utils/runtime/src/lib.rs | 21 +++++++++++++++++++ 6 files changed, 50 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a964200adca11..1ae0331fb85c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10385,6 +10385,7 @@ dependencies = [ name = "substrate-test-runtime" version = "2.0.0" dependencies = [ + "async-trait", "beefy-merkle-tree", "beefy-primitives", "cfg-if", diff --git a/bin/node-template/node/src/benchmarking.rs b/bin/node-template/node/src/benchmarking.rs index 519f10e6a2c32..480e547c9c73c 100644 --- a/bin/node-template/node/src/benchmarking.rs +++ b/bin/node-template/node/src/benchmarking.rs @@ -176,9 +176,7 @@ pub fn inherent_benchmark_data() -> Result { let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - futures::executor::block_on( - timestamp - .provide_inherent_data(&mut inherent_data), - ).map_err(|e| format!("creating inherent data: {:?}", e))?; + futures::executor::block_on(timestamp.provide_inherent_data(&mut inherent_data)) + .map_err(|e| format!("creating inherent data: {:?}", e))?; Ok(inherent_data) } diff --git a/bin/node/cli/src/benchmarking.rs b/bin/node/cli/src/benchmarking.rs index c97e1809e81f2..16ea9109d0c1f 100644 --- a/bin/node/cli/src/benchmarking.rs +++ b/bin/node/cli/src/benchmarking.rs @@ -116,9 +116,7 @@ pub fn inherent_benchmark_data() -> Result { let d = Duration::from_millis(0); let timestamp = sp_timestamp::InherentDataProvider::new(d.into()); - futures::executor::block_on( - timestamp - .provide_inherent_data(&mut inherent_data), - ).map_err(|e| format!("creating inherent data: {:?}", e))?; + futures::executor::block_on(timestamp.provide_inherent_data(&mut inherent_data)) + .map_err(|e| format!("creating inherent data: {:?}", e))?; Ok(inherent_data) } diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 45aec6f2c1e5a..7396ba5f5baca 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -810,17 +810,39 @@ impl BackoffAuthoringBlocksStrategy for () { #[cfg(test)] mod test { use super::*; + use sp_inherents::{Error, InherentData, InherentDataProvider, InherentIdentifier}; use sp_runtime::traits::NumberFor; use std::time::{Duration, Instant}; use substrate_test_runtime_client::runtime::{Block, Header}; const SLOT_DURATION: Duration = Duration::from_millis(6000); + #[derive(Clone)] + struct TestInherentDataProvider; + + const ERROR_TO_STRING: &str = "Found error!"; + const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; + + #[async_trait::async_trait] + impl sp_inherents::InherentDataProvider for TestInherentDataProvider { + async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { + data.put_data(TEST_INHERENT_0, &42) + } + + async fn try_handle_error( + &self, + _: &InherentIdentifier, + _: &[u8], + ) -> Option> { + Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) + } + } + fn slot(slot: u64) -> super::slots::SlotInfo { super::slots::SlotInfo { slot: slot.into(), duration: SLOT_DURATION, - create_inherent_data: Default::default(), + create_inherent_data: Box::new(TestInherentDataProvider), ends_at: Instant::now() + SLOT_DURATION, chain_head: Header::new( 1, diff --git a/test-utils/runtime/Cargo.toml b/test-utils/runtime/Cargo.toml index b64a847b15943..eb173f7d37bf8 100644 --- a/test-utils/runtime/Cargo.toml +++ b/test-utils/runtime/Cargo.toml @@ -48,6 +48,7 @@ sp-state-machine = { version = "0.13.0", default-features = false, path = "../.. sp-externalities = { version = "0.13.0", default-features = false, path = "../../primitives/externalities" } # 3rd party +async-trait = "0.1.57" cfg-if = "1.0" log = { version = "0.4.17", default-features = false } serde = { version = "1.0.136", optional = true, features = ["derive"] } diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 8bda4ea602428..21820e100ba18 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -1401,3 +1401,24 @@ mod tests { runtime_api.test_witness(&block_id, proof, root).unwrap(); } } + +//#[derive(Clone)] +//struct TestInherentDataProvider; +// +//const ERROR_TO_STRING: &str = "Found error!"; +//const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; +// +//#[async_trait::async_trait] +//impl sp_inherents::InherentDataProvider for TestInherentDataProvider { +// async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { +// data.put_data(TEST_INHERENT_0, &42) +// } +// +// async fn try_handle_error( +// &self, +// _: &InherentIdentifier, +// _: &[u8], +// ) -> Option> { +// Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) +// } +//} From be841365b5b66c0044e08f81ec2cb67aa8f5df48 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Mon, 21 Nov 2022 12:10:41 +0100 Subject: [PATCH 07/28] Fix test --- client/consensus/aura/src/lib.rs | 26 +++++++++++++++++++++++++- client/consensus/slots/src/lib.rs | 2 +- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index d3a0988a0ae3d..50cfad7c2004b 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -660,6 +660,30 @@ mod tests { TestClient, }; + #[derive(Clone)] + struct TestInherentDataProvider; + + const ERROR_TO_STRING: &str = "Found error!"; + const TEST_INHERENT_0: sp_inherents::InherentIdentifier = *b"testinh0"; + + #[async_trait::async_trait] + impl sp_inherents::InherentDataProvider for TestInherentDataProvider { + async fn provide_inherent_data( + &self, + data: &mut sp_inherents::InherentData, + ) -> Result<(), sp_inherents::Error> { + data.put_data(TEST_INHERENT_0, &42) + } + + async fn try_handle_error( + &self, + _: &sp_inherents::InherentIdentifier, + _: &[u8], + ) -> Option> { + Some(Err(sp_inherents::Error::Application(Box::from(ERROR_TO_STRING)))) + } + } + const SLOT_DURATION_MS: u64 = 1000; type Error = sp_blockchain::Error; @@ -960,7 +984,7 @@ mod tests { let res = executor::block_on(worker.on_slot(SlotInfo { slot: 0.into(), ends_at: Instant::now() + Duration::from_secs(100), - create_inherent_data: InherentData::new(), + create_inherent_data: Box::new(TestInherentDataProvider), duration: Duration::from_millis(1000), chain_head: head, block_size_limit: None, diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 7396ba5f5baca..8e9189cb4e682 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -824,7 +824,7 @@ mod test { const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; #[async_trait::async_trait] - impl sp_inherents::InherentDataProvider for TestInherentDataProvider { + impl InherentDataProvider for TestInherentDataProvider { async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { data.put_data(TEST_INHERENT_0, &42) } From 7afe2db43436504f1b3ebf69dfe1f4936c07ce06 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Mon, 21 Nov 2022 12:39:10 +0100 Subject: [PATCH 08/28] fix --- bin/node/cli/src/service.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 6c29f0c08ee13..e7b825a8e5ef1 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -692,14 +692,16 @@ mod tests { slot += 1; }; - let inherent_data = ( - sp_timestamp::InherentDataProvider::new( - std::time::Duration::from_millis(SLOT_DURATION * slot).into(), - ), - sp_consensus_babe::inherents::InherentDataProvider::new(slot.into()), + let inherent_data = futures::executor::block_on( + ( + sp_timestamp::InherentDataProvider::new( + std::time::Duration::from_millis(SLOT_DURATION * slot).into(), + ), + sp_consensus_babe::inherents::InherentDataProvider::new(slot.into()), + ) + .create_inherent_data(), ) - .create_inherent_data() - .expect("Creates inherent data"); + .expect("Creates inherent data"); digest.push(::babe_pre_digest(babe_pre_digest)); From ba46adbe089329c78cd69ccdb08e27ed67bd77cf Mon Sep 17 00:00:00 2001 From: alexgparity Date: Mon, 21 Nov 2022 17:12:40 +0100 Subject: [PATCH 09/28] Deduplicate test code --- Cargo.lock | 1 + client/consensus/aura/src/lib.rs | 26 +---------------- client/consensus/slots/src/lib.rs | 24 +--------------- primitives/inherents/Cargo.toml | 1 + primitives/inherents/src/lib.rs | 23 +--------------- test-utils/runtime/src/lib.rs | 46 +++++++++++++++++-------------- 6 files changed, 30 insertions(+), 91 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ae0331fb85c0..bf09a5e37404d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9646,6 +9646,7 @@ dependencies = [ "sp-core", "sp-runtime", "sp-std", + "substrate-test-runtime-client", "thiserror", ] diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 50cfad7c2004b..b8b8f485b01b3 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -656,34 +656,10 @@ mod tests { time::{Duration, Instant}, }; use substrate_test_runtime_client::{ - runtime::{Header, H256}, + runtime::{Header, TestInherentDataProvider, H256}, TestClient, }; - #[derive(Clone)] - struct TestInherentDataProvider; - - const ERROR_TO_STRING: &str = "Found error!"; - const TEST_INHERENT_0: sp_inherents::InherentIdentifier = *b"testinh0"; - - #[async_trait::async_trait] - impl sp_inherents::InherentDataProvider for TestInherentDataProvider { - async fn provide_inherent_data( - &self, - data: &mut sp_inherents::InherentData, - ) -> Result<(), sp_inherents::Error> { - data.put_data(TEST_INHERENT_0, &42) - } - - async fn try_handle_error( - &self, - _: &sp_inherents::InherentIdentifier, - _: &[u8], - ) -> Option> { - Some(Err(sp_inherents::Error::Application(Box::from(ERROR_TO_STRING)))) - } - } - const SLOT_DURATION_MS: u64 = 1000; type Error = sp_blockchain::Error; diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 8e9189cb4e682..506b982188d47 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -810,34 +810,12 @@ impl BackoffAuthoringBlocksStrategy for () { #[cfg(test)] mod test { use super::*; - use sp_inherents::{Error, InherentData, InherentDataProvider, InherentIdentifier}; use sp_runtime::traits::NumberFor; use std::time::{Duration, Instant}; - use substrate_test_runtime_client::runtime::{Block, Header}; + use substrate_test_runtime_client::runtime::{Block, Header, TestInherentDataProvider}; const SLOT_DURATION: Duration = Duration::from_millis(6000); - #[derive(Clone)] - struct TestInherentDataProvider; - - const ERROR_TO_STRING: &str = "Found error!"; - const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; - - #[async_trait::async_trait] - impl InherentDataProvider for TestInherentDataProvider { - async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { - data.put_data(TEST_INHERENT_0, &42) - } - - async fn try_handle_error( - &self, - _: &InherentIdentifier, - _: &[u8], - ) -> Option> { - Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) - } - } - fn slot(slot: u64) -> super::slots::SlotInfo { super::slots::SlotInfo { slot: slot.into(), diff --git a/primitives/inherents/Cargo.toml b/primitives/inherents/Cargo.toml index 8f6d8aef155ac..59a5e2061dd37 100644 --- a/primitives/inherents/Cargo.toml +++ b/primitives/inherents/Cargo.toml @@ -24,6 +24,7 @@ sp-std = { version = "5.0.0", default-features = false, path = "../std" } [dev-dependencies] futures = "0.3.21" +substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } [features] default = [ "std" ] diff --git a/primitives/inherents/src/lib.rs b/primitives/inherents/src/lib.rs index 59db5ef3e8802..ee113b3ef6357 100644 --- a/primitives/inherents/src/lib.rs +++ b/primitives/inherents/src/lib.rs @@ -436,30 +436,9 @@ mod tests { assert!(data.put_data(TEST_INHERENT_0, &10).is_err()); } - #[derive(Clone)] - struct TestInherentDataProvider; - - const ERROR_TO_STRING: &str = "Found error!"; - - #[async_trait::async_trait] - impl InherentDataProvider for TestInherentDataProvider { - async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { - data.put_data(TEST_INHERENT_0, &42) - } - - async fn try_handle_error( - &self, - _: &InherentIdentifier, - _: &[u8], - ) -> Option> { - Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) - } - } - #[test] fn create_inherent_data() { - let provider = TestInherentDataProvider; - + let provider = substrate_test_runtime_client::runtime::TestInherentDataProvider; let inherent_data = futures::executor::block_on(provider.create_inherent_data()).unwrap(); assert_eq!(inherent_data.get_data::(&TEST_INHERENT_0).unwrap().unwrap(), 42u32); diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 21820e100ba18..69de1ded6bac5 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -45,7 +45,7 @@ use frame_support::{ use frame_system::limits::{BlockLength, BlockWeights}; use sp_api::{decl_runtime_apis, impl_runtime_apis}; pub use sp_core::hash::H256; -use sp_inherents::{CheckInherentsResult, InherentData}; +use sp_inherents::{CheckInherentsResult, InherentData, InherentIdentifier}; #[cfg(feature = "std")] use sp_runtime::traits::NumberFor; use sp_runtime::{ @@ -1402,23 +1402,27 @@ mod tests { } } -//#[derive(Clone)] -//struct TestInherentDataProvider; -// -//const ERROR_TO_STRING: &str = "Found error!"; -//const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; -// -//#[async_trait::async_trait] -//impl sp_inherents::InherentDataProvider for TestInherentDataProvider { -// async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { -// data.put_data(TEST_INHERENT_0, &42) -// } -// -// async fn try_handle_error( -// &self, -// _: &InherentIdentifier, -// _: &[u8], -// ) -> Option> { -// Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) -// } -//} +#[derive(Clone)] +pub struct TestInherentDataProvider; + +const ERROR_TO_STRING: &str = "Found error!"; +const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; + +#[cfg(feature = "std")] +#[async_trait::async_trait] +impl sp_inherents::InherentDataProvider for TestInherentDataProvider { + async fn provide_inherent_data( + &self, + data: &mut InherentData, + ) -> Result<(), sp_inherents::Error> { + data.put_data(TEST_INHERENT_0, &42) + } + + async fn try_handle_error( + &self, + _: &InherentIdentifier, + _: &[u8], + ) -> Option> { + Some(Err(sp_inherents::Error::Application(Box::from(ERROR_TO_STRING)))) + } +} From 7f02482ebb19bc14dbe474e5b46366c3075fbba8 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Mon, 21 Nov 2022 17:45:13 +0100 Subject: [PATCH 10/28] fix --- test-utils/runtime/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 69de1ded6bac5..52a750556c6da 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -1402,10 +1402,13 @@ mod tests { } } +#[cfg(feature = "std")] #[derive(Clone)] pub struct TestInherentDataProvider; +#[cfg(feature = "std")] const ERROR_TO_STRING: &str = "Found error!"; +#[cfg(feature = "std")] const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; #[cfg(feature = "std")] From f7d687b79bb44e4cb42a4902572c21f95583068b Mon Sep 17 00:00:00 2001 From: alexgparity Date: Mon, 21 Nov 2022 18:43:59 +0100 Subject: [PATCH 11/28] flag --- test-utils/runtime/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 52a750556c6da..a08c657bf5759 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -45,7 +45,9 @@ use frame_support::{ use frame_system::limits::{BlockLength, BlockWeights}; use sp_api::{decl_runtime_apis, impl_runtime_apis}; pub use sp_core::hash::H256; -use sp_inherents::{CheckInherentsResult, InherentData, InherentIdentifier}; +#[cfg(feature = "std")] +use sp_inherents::InherentIdentifier; +use sp_inherents::{CheckInherentsResult, InherentData}; #[cfg(feature = "std")] use sp_runtime::traits::NumberFor; use sp_runtime::{ From f374a2fcb06b01e7691c892222de4b666156f2af Mon Sep 17 00:00:00 2001 From: alexgparity <115470171+alexgparity@users.noreply.github.com> Date: Tue, 22 Nov 2022 09:39:46 +0100 Subject: [PATCH 12/28] Update client/consensus/slots/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/consensus/slots/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 506b982188d47..6cad88a1c9ff8 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -820,7 +820,7 @@ mod test { super::slots::SlotInfo { slot: slot.into(), duration: SLOT_DURATION, - create_inherent_data: Box::new(TestInherentDataProvider), + create_inherent_data: Box::new(|_, ()| Ok(sp_timestamp::InherentDataProvider::from_system_time())), ends_at: Instant::now() + SLOT_DURATION, chain_head: Header::new( 1, From 7267c540794c6d2cf5368afcd4afaf8052a653c2 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Tue, 22 Nov 2022 09:55:03 +0100 Subject: [PATCH 13/28] Revert "Deduplicate test code" This reverts commit ba46adbe089329c78cd69ccdb08e27ed67bd77cf. --- Cargo.lock | 1 - client/consensus/aura/src/lib.rs | 26 +++++++++++++++++++++++++- client/consensus/slots/src/lib.rs | 24 +++++++++++++++++++++++- primitives/inherents/Cargo.toml | 1 - primitives/inherents/src/lib.rs | 23 ++++++++++++++++++++++- test-utils/runtime/src/lib.rs | 30 ------------------------------ 6 files changed, 70 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf09a5e37404d..1ae0331fb85c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9646,7 +9646,6 @@ dependencies = [ "sp-core", "sp-runtime", "sp-std", - "substrate-test-runtime-client", "thiserror", ] diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index b8b8f485b01b3..50cfad7c2004b 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -656,10 +656,34 @@ mod tests { time::{Duration, Instant}, }; use substrate_test_runtime_client::{ - runtime::{Header, TestInherentDataProvider, H256}, + runtime::{Header, H256}, TestClient, }; + #[derive(Clone)] + struct TestInherentDataProvider; + + const ERROR_TO_STRING: &str = "Found error!"; + const TEST_INHERENT_0: sp_inherents::InherentIdentifier = *b"testinh0"; + + #[async_trait::async_trait] + impl sp_inherents::InherentDataProvider for TestInherentDataProvider { + async fn provide_inherent_data( + &self, + data: &mut sp_inherents::InherentData, + ) -> Result<(), sp_inherents::Error> { + data.put_data(TEST_INHERENT_0, &42) + } + + async fn try_handle_error( + &self, + _: &sp_inherents::InherentIdentifier, + _: &[u8], + ) -> Option> { + Some(Err(sp_inherents::Error::Application(Box::from(ERROR_TO_STRING)))) + } + } + const SLOT_DURATION_MS: u64 = 1000; type Error = sp_blockchain::Error; diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 6cad88a1c9ff8..16e568478b280 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -810,12 +810,34 @@ impl BackoffAuthoringBlocksStrategy for () { #[cfg(test)] mod test { use super::*; + use sp_inherents::{Error, InherentData, InherentDataProvider, InherentIdentifier}; use sp_runtime::traits::NumberFor; use std::time::{Duration, Instant}; - use substrate_test_runtime_client::runtime::{Block, Header, TestInherentDataProvider}; + use substrate_test_runtime_client::runtime::{Block, Header}; const SLOT_DURATION: Duration = Duration::from_millis(6000); + #[derive(Clone)] + struct TestInherentDataProvider; + + const ERROR_TO_STRING: &str = "Found error!"; + const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; + + #[async_trait::async_trait] + impl InherentDataProvider for TestInherentDataProvider { + async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { + data.put_data(TEST_INHERENT_0, &42) + } + + async fn try_handle_error( + &self, + _: &InherentIdentifier, + _: &[u8], + ) -> Option> { + Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) + } + } + fn slot(slot: u64) -> super::slots::SlotInfo { super::slots::SlotInfo { slot: slot.into(), diff --git a/primitives/inherents/Cargo.toml b/primitives/inherents/Cargo.toml index 59a5e2061dd37..8f6d8aef155ac 100644 --- a/primitives/inherents/Cargo.toml +++ b/primitives/inherents/Cargo.toml @@ -24,7 +24,6 @@ sp-std = { version = "5.0.0", default-features = false, path = "../std" } [dev-dependencies] futures = "0.3.21" -substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } [features] default = [ "std" ] diff --git a/primitives/inherents/src/lib.rs b/primitives/inherents/src/lib.rs index ee113b3ef6357..59db5ef3e8802 100644 --- a/primitives/inherents/src/lib.rs +++ b/primitives/inherents/src/lib.rs @@ -436,9 +436,30 @@ mod tests { assert!(data.put_data(TEST_INHERENT_0, &10).is_err()); } + #[derive(Clone)] + struct TestInherentDataProvider; + + const ERROR_TO_STRING: &str = "Found error!"; + + #[async_trait::async_trait] + impl InherentDataProvider for TestInherentDataProvider { + async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { + data.put_data(TEST_INHERENT_0, &42) + } + + async fn try_handle_error( + &self, + _: &InherentIdentifier, + _: &[u8], + ) -> Option> { + Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) + } + } + #[test] fn create_inherent_data() { - let provider = substrate_test_runtime_client::runtime::TestInherentDataProvider; + let provider = TestInherentDataProvider; + let inherent_data = futures::executor::block_on(provider.create_inherent_data()).unwrap(); assert_eq!(inherent_data.get_data::(&TEST_INHERENT_0).unwrap().unwrap(), 42u32); diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index a08c657bf5759..8bda4ea602428 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -45,8 +45,6 @@ use frame_support::{ use frame_system::limits::{BlockLength, BlockWeights}; use sp_api::{decl_runtime_apis, impl_runtime_apis}; pub use sp_core::hash::H256; -#[cfg(feature = "std")] -use sp_inherents::InherentIdentifier; use sp_inherents::{CheckInherentsResult, InherentData}; #[cfg(feature = "std")] use sp_runtime::traits::NumberFor; @@ -1403,31 +1401,3 @@ mod tests { runtime_api.test_witness(&block_id, proof, root).unwrap(); } } - -#[cfg(feature = "std")] -#[derive(Clone)] -pub struct TestInherentDataProvider; - -#[cfg(feature = "std")] -const ERROR_TO_STRING: &str = "Found error!"; -#[cfg(feature = "std")] -const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; - -#[cfg(feature = "std")] -#[async_trait::async_trait] -impl sp_inherents::InherentDataProvider for TestInherentDataProvider { - async fn provide_inherent_data( - &self, - data: &mut InherentData, - ) -> Result<(), sp_inherents::Error> { - data.put_data(TEST_INHERENT_0, &42) - } - - async fn try_handle_error( - &self, - _: &InherentIdentifier, - _: &[u8], - ) -> Option> { - Some(Err(sp_inherents::Error::Application(Box::from(ERROR_TO_STRING)))) - } -} From 38e8c3008327f8f53941ae4d91a163db0b23c2d2 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Tue, 22 Nov 2022 10:34:44 +0100 Subject: [PATCH 14/28] Fix test --- Cargo.lock | 1 + client/consensus/slots/Cargo.toml | 1 + client/consensus/slots/src/lib.rs | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 1ae0331fb85c0..233ca01a199c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7959,6 +7959,7 @@ dependencies = [ "sp-inherents", "sp-runtime", "sp-state-machine", + "sp-timestamp", "substrate-test-runtime-client", "thiserror", ] diff --git a/client/consensus/slots/Cargo.toml b/client/consensus/slots/Cargo.toml index 4bb9387cf5596..8e15f70121bad 100644 --- a/client/consensus/slots/Cargo.toml +++ b/client/consensus/slots/Cargo.toml @@ -31,6 +31,7 @@ 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-state-machine = { version = "0.13.0", path = "../../../primitives/state-machine" } +sp-timestamp = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/timestamp" } [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0", path = "../../../test-utils/runtime/client" } diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 16e568478b280..a6c0269f44d55 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -842,7 +842,9 @@ mod test { super::slots::SlotInfo { slot: slot.into(), duration: SLOT_DURATION, - create_inherent_data: Box::new(|_, ()| Ok(sp_timestamp::InherentDataProvider::from_system_time())), + create_inherent_data: Box::new(()), + //create_inherent_data: Box::new(|_, ()| + // Ok(sp_timestamp::InherentDataProvider::from_system_time())), ends_at: Instant::now() + SLOT_DURATION, chain_head: Header::new( 1, From 23af20761cd77e1578fc3b211f377aa09ed16a9d Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 23 Nov 2022 10:45:18 +0100 Subject: [PATCH 15/28] remove commented out code --- client/consensus/slots/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index a6c0269f44d55..b801813dc3003 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -843,8 +843,6 @@ mod test { slot: slot.into(), duration: SLOT_DURATION, create_inherent_data: Box::new(()), - //create_inherent_data: Box::new(|_, ()| - // Ok(sp_timestamp::InherentDataProvider::from_system_time())), ends_at: Instant::now() + SLOT_DURATION, chain_head: Header::new( 1, From c09f968cded6d4259b396c6ba1024a9492f20f9d Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 23 Nov 2022 11:21:59 +0100 Subject: [PATCH 16/28] minor to start CI run --- bin/node-template/node/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 51234e25279b6..d609edc88401d 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -18,7 +18,7 @@ name = "node-template" [dependencies] clap = { version = "4.0.9", features = ["derive"] } -futures = { version = "0.3.21", features = ["thread-pool"] } +futures = { version = "0.3.21", features = ["thread-pool"]} sc-cli = { version = "0.10.0-dev", path = "../../../client/cli" } sp-core = { version = "7.0.0", path = "../../../primitives/core" } From c8b5abd78935d78bcf8d9516f286d271639808d1 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Wed, 23 Nov 2022 11:46:14 +0100 Subject: [PATCH 17/28] start CI --- bin/node-template/node/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index d609edc88401d..51234e25279b6 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -18,7 +18,7 @@ name = "node-template" [dependencies] clap = { version = "4.0.9", features = ["derive"] } -futures = { version = "0.3.21", features = ["thread-pool"]} +futures = { version = "0.3.21", features = ["thread-pool"] } sc-cli = { version = "0.10.0-dev", path = "../../../client/cli" } sp-core = { version = "7.0.0", path = "../../../primitives/core" } From 0810c4cbe8d9f204233a869e2340bd503969b943 Mon Sep 17 00:00:00 2001 From: alexgparity <115470171+alexgparity@users.noreply.github.com> Date: Wed, 23 Nov 2022 14:28:12 +0100 Subject: [PATCH 18/28] Update client/consensus/slots/src/lib.rs Co-authored-by: Marcin S. --- client/consensus/slots/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index b801813dc3003..69bf52096b398 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -256,7 +256,7 @@ pub trait SimpleSlotWorker { Err(err) => { warn!( target: logging_target, - "Unable to create inherent data for for block {:?}: {}", + "Unable to create inherent data for block {:?}: {}", slot_info.chain_head.hash(), err, ); From f615d602121a69f3bf0ddb1a4a3cb4947e60992d Mon Sep 17 00:00:00 2001 From: alexgparity Date: Fri, 25 Nov 2022 12:48:46 +0100 Subject: [PATCH 19/28] Apply PR suggestions --- Cargo.lock | 2 -- client/consensus/aura/src/lib.rs | 26 +------------------------- client/consensus/slots/Cargo.toml | 1 - client/consensus/slots/src/lib.rs | 4 +++- test-utils/runtime/Cargo.toml | 1 - 5 files changed, 4 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 77658192e3bab..698ce270233bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7959,7 +7959,6 @@ dependencies = [ "sp-inherents", "sp-runtime", "sp-state-machine", - "sp-timestamp", "substrate-test-runtime-client", "thiserror", ] @@ -10387,7 +10386,6 @@ dependencies = [ name = "substrate-test-runtime" version = "2.0.0" dependencies = [ - "async-trait", "beefy-merkle-tree", "beefy-primitives", "cfg-if", diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index 50cfad7c2004b..839965a556e04 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -660,30 +660,6 @@ mod tests { TestClient, }; - #[derive(Clone)] - struct TestInherentDataProvider; - - const ERROR_TO_STRING: &str = "Found error!"; - const TEST_INHERENT_0: sp_inherents::InherentIdentifier = *b"testinh0"; - - #[async_trait::async_trait] - impl sp_inherents::InherentDataProvider for TestInherentDataProvider { - async fn provide_inherent_data( - &self, - data: &mut sp_inherents::InherentData, - ) -> Result<(), sp_inherents::Error> { - data.put_data(TEST_INHERENT_0, &42) - } - - async fn try_handle_error( - &self, - _: &sp_inherents::InherentIdentifier, - _: &[u8], - ) -> Option> { - Some(Err(sp_inherents::Error::Application(Box::from(ERROR_TO_STRING)))) - } - } - const SLOT_DURATION_MS: u64 = 1000; type Error = sp_blockchain::Error; @@ -984,7 +960,7 @@ mod tests { let res = executor::block_on(worker.on_slot(SlotInfo { slot: 0.into(), ends_at: Instant::now() + Duration::from_secs(100), - create_inherent_data: Box::new(TestInherentDataProvider), + create_inherent_data: Box::new(()), duration: Duration::from_millis(1000), chain_head: head, block_size_limit: None, diff --git a/client/consensus/slots/Cargo.toml b/client/consensus/slots/Cargo.toml index 8e15f70121bad..4bb9387cf5596 100644 --- a/client/consensus/slots/Cargo.toml +++ b/client/consensus/slots/Cargo.toml @@ -31,7 +31,6 @@ 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-state-machine = { version = "0.13.0", path = "../../../primitives/state-machine" } -sp-timestamp = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/timestamp" } [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0", path = "../../../test-utils/runtime/client" } diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index b801813dc3003..c1882bf8fac92 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -267,7 +267,9 @@ pub trait SimpleSlotWorker { if Instant::now() > slot_info.ends_at { warn!( target: "slots", - "Creating inherent data took more time than we had left for the slot.", + "Creating inherent data took more time than we had left for slot {} for block {:?}.", + slot_info.slot, + slot_info.chain_head.hash(), ); return None diff --git a/test-utils/runtime/Cargo.toml b/test-utils/runtime/Cargo.toml index eb173f7d37bf8..b64a847b15943 100644 --- a/test-utils/runtime/Cargo.toml +++ b/test-utils/runtime/Cargo.toml @@ -48,7 +48,6 @@ sp-state-machine = { version = "0.13.0", default-features = false, path = "../.. sp-externalities = { version = "0.13.0", default-features = false, path = "../../primitives/externalities" } # 3rd party -async-trait = "0.1.57" cfg-if = "1.0" log = { version = "0.4.17", default-features = false } serde = { version = "1.0.136", optional = true, features = ["derive"] } From 439945de47f986473d9649f803d103660f558c46 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Fri, 25 Nov 2022 12:51:47 +0100 Subject: [PATCH 20/28] Apply PR suggestions --- client/consensus/slots/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 10723f6f4682d..3a3e4f4505c24 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -819,7 +819,6 @@ mod test { const SLOT_DURATION: Duration = Duration::from_millis(6000); - #[derive(Clone)] struct TestInherentDataProvider; const ERROR_TO_STRING: &str = "Found error!"; From 8a69c39db4ca0c361e1bcbfca567f513d9413bac Mon Sep 17 00:00:00 2001 From: alexgparity <115470171+alexgparity@users.noreply.github.com> Date: Fri, 25 Nov 2022 14:57:02 +0100 Subject: [PATCH 21/28] Update client/consensus/slots/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/consensus/slots/src/lib.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 3a3e4f4505c24..21dbe7142f1f3 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -819,26 +819,6 @@ mod test { const SLOT_DURATION: Duration = Duration::from_millis(6000); - struct TestInherentDataProvider; - - const ERROR_TO_STRING: &str = "Found error!"; - const TEST_INHERENT_0: InherentIdentifier = *b"testinh0"; - - #[async_trait::async_trait] - impl InherentDataProvider for TestInherentDataProvider { - async fn provide_inherent_data(&self, data: &mut InherentData) -> Result<(), Error> { - data.put_data(TEST_INHERENT_0, &42) - } - - async fn try_handle_error( - &self, - _: &InherentIdentifier, - _: &[u8], - ) -> Option> { - Some(Err(Error::Application(Box::from(ERROR_TO_STRING)))) - } - } - fn slot(slot: u64) -> super::slots::SlotInfo { super::slots::SlotInfo { slot: slot.into(), From ba700bc6ac2f8f680960ff1c2884e77e5cede893 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Fri, 25 Nov 2022 15:12:14 +0100 Subject: [PATCH 22/28] minor --- client/consensus/slots/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 21dbe7142f1f3..b23a0932653c2 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -812,7 +812,6 @@ impl BackoffAuthoringBlocksStrategy for () { #[cfg(test)] mod test { use super::*; - use sp_inherents::{Error, InherentData, InherentDataProvider, InherentIdentifier}; use sp_runtime::traits::NumberFor; use std::time::{Duration, Instant}; use substrate_test_runtime_client::runtime::{Block, Header}; From d8a45718b7fbeacf4529d0240b40081495aed7a9 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sat, 26 Nov 2022 18:01:11 +0100 Subject: [PATCH 23/28] kickoff CI --- bin/node-template/node/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node-template/node/Cargo.toml b/bin/node-template/node/Cargo.toml index 51234e25279b6..d609edc88401d 100644 --- a/bin/node-template/node/Cargo.toml +++ b/bin/node-template/node/Cargo.toml @@ -18,7 +18,7 @@ name = "node-template" [dependencies] clap = { version = "4.0.9", features = ["derive"] } -futures = { version = "0.3.21", features = ["thread-pool"] } +futures = { version = "0.3.21", features = ["thread-pool"]} sc-cli = { version = "0.10.0-dev", path = "../../../client/cli" } sp-core = { version = "7.0.0", path = "../../../primitives/core" } From c3c168edefe31516bd1736179ec85cd1ea0b7d8a Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sat, 26 Nov 2022 20:37:16 +0100 Subject: [PATCH 24/28] PR suggestions --- Cargo.lock | 1 + client/consensus/slots/Cargo.toml | 1 + client/consensus/slots/src/lib.rs | 35 +++++++++++++++++------------ client/consensus/slots/src/slots.rs | 4 ++-- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 698ce270233bf..fdd7fa9ca1c6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7943,6 +7943,7 @@ dependencies = [ name = "sc-consensus-slots" version = "0.10.0-dev" dependencies = [ + "async-std", "async-trait", "futures", "futures-timer", diff --git a/client/consensus/slots/Cargo.toml b/client/consensus/slots/Cargo.toml index 4bb9387cf5596..a3a8fff771866 100644 --- a/client/consensus/slots/Cargo.toml +++ b/client/consensus/slots/Cargo.toml @@ -14,6 +14,7 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] +async-std = "1.11.0" async-trait = "0.1.57" codec = { package = "parity-scale-codec", version = "3.0.0" } futures = "0.3.21" diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index b23a0932653c2..3bc8c205ddfb6 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -31,8 +31,8 @@ mod slots; pub use aux_schema::{check_equivocation, MAX_SLOT_CAPACITY, PRUNING_BOUND}; pub use slots::SlotInfo; use slots::Slots; -use std::time::Instant; +use async_std::prelude::FutureExt; use futures::{future::Either, Future, TryFutureExt}; use futures_timer::Delay; use log::{debug, info, warn}; @@ -251,29 +251,36 @@ pub trait SimpleSlotWorker { slot_info: &SlotInfo, logging_target: &str, ) -> Option { - let inherent_data = match slot_info.create_inherent_data.create_inherent_data().await { - Ok(data) => data, - Err(err) => { + let inherent_data = match slot_info + .create_inherent_data + .create_inherent_data() + .timeout(slot_info.duration) + .await + { + Ok(Ok(data)) => data, + Ok(Err(err)) => { warn!( target: logging_target, "Unable to create inherent data for block {:?}: {}", slot_info.chain_head.hash(), err, ); + return None }, - }; - if Instant::now() > slot_info.ends_at { - warn!( - target: "slots", - "Creating inherent data took more time than we had left for slot {} for block {:?}.", - slot_info.slot, - slot_info.chain_head.hash(), - ); + Err(err) => { + warn!( + target: logging_target, + "Creating inherent data for slot {} for block {:?} timed out: {}.", + slot_info.slot, + slot_info.chain_head.hash(), + err, + ); - return None - } + return None + }, + }; Some(inherent_data) } diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index a8de7f0345081..f8f366d89c82c 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -70,14 +70,14 @@ impl SlotInfo { /// `ends_at` is calculated using `timestamp` and `duration`. pub fn new( slot: Slot, - inherent_data: Box, + create_inherent_data: Box, duration: Duration, chain_head: B::Header, block_size_limit: Option, ) -> Self { Self { slot, - create_inherent_data: inherent_data, + create_inherent_data, duration, chain_head, block_size_limit, From a4059fb4c2e35770f979e829fca8f3152e3ac0f6 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sat, 26 Nov 2022 21:09:34 +0100 Subject: [PATCH 25/28] Compute remaining duration instead of using slot_info.duration --- client/consensus/slots/src/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 3bc8c205ddfb6..9e56f85460bc9 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -43,7 +43,11 @@ 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 std::{ + fmt::Debug, + ops::Deref, + time::{Duration, Instant}, +}; /// The changes that need to applied to the storage to create the state for a block. /// @@ -251,10 +255,11 @@ pub trait SimpleSlotWorker { slot_info: &SlotInfo, logging_target: &str, ) -> Option { + let remaining_duration = slot_info.ends_at - Instant::now(); let inherent_data = match slot_info .create_inherent_data .create_inherent_data() - .timeout(slot_info.duration) + .timeout(remaining_duration) .await { Ok(Ok(data)) => data, From bc720d80b9203b5a2edff579b956cad91a39035f Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sat, 26 Nov 2022 21:14:29 +0100 Subject: [PATCH 26/28] Don't rely on sub implementation for Instant --- client/consensus/slots/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 9e56f85460bc9..5eb7a9edc3de4 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -255,7 +255,12 @@ pub trait SimpleSlotWorker { slot_info: &SlotInfo, logging_target: &str, ) -> Option { - let remaining_duration = slot_info.ends_at - Instant::now(); + let now = Instant::now(); + let remaining_duration = if now > slot_info.ends_at { + Duration::from_millis(0) + } else { + slot_info.ends_at - now + }; let inherent_data = match slot_info .create_inherent_data .create_inherent_data() From dfe3d29e31b1bb339d9953a3bebd7ff329257c72 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sun, 27 Nov 2022 10:16:18 +0100 Subject: [PATCH 27/28] Apply PR suggestions --- Cargo.lock | 1 - client/consensus/slots/Cargo.toml | 1 - client/consensus/slots/src/lib.rs | 21 ++++++++------------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fdd7fa9ca1c6c..698ce270233bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7943,7 +7943,6 @@ dependencies = [ name = "sc-consensus-slots" version = "0.10.0-dev" dependencies = [ - "async-std", "async-trait", "futures", "futures-timer", diff --git a/client/consensus/slots/Cargo.toml b/client/consensus/slots/Cargo.toml index a3a8fff771866..4bb9387cf5596 100644 --- a/client/consensus/slots/Cargo.toml +++ b/client/consensus/slots/Cargo.toml @@ -14,7 +14,6 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -async-std = "1.11.0" async-trait = "0.1.57" codec = { package = "parity-scale-codec", version = "3.0.0" } futures = "0.3.21" diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 5eb7a9edc3de4..b875b194257ec 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -32,7 +32,6 @@ pub use aux_schema::{check_equivocation, MAX_SLOT_CAPACITY, PRUNING_BOUND}; pub use slots::SlotInfo; use slots::Slots; -use async_std::prelude::FutureExt; use futures::{future::Either, Future, TryFutureExt}; use futures_timer::Delay; use log::{debug, info, warn}; @@ -261,14 +260,12 @@ pub trait SimpleSlotWorker { } else { slot_info.ends_at - now }; - let inherent_data = match slot_info - .create_inherent_data - .create_inherent_data() - .timeout(remaining_duration) - .await - { - Ok(Ok(data)) => data, - Ok(Err(err)) => { + + let delay = Delay::new(remaining_duration); + let cid = slot_info.create_inherent_data.create_inherent_data(); + let inherent_data = match futures::future::select(delay, cid).await { + Either::Right((Ok(data), _)) => data, + Either::Right((Err(err), _)) => { warn!( target: logging_target, "Unable to create inherent data for block {:?}: {}", @@ -278,14 +275,12 @@ pub trait SimpleSlotWorker { return None }, - - Err(err) => { + Either::Left(_) => { warn!( target: logging_target, - "Creating inherent data for slot {} for block {:?} timed out: {}.", + "Creating inherent data took more time than we had left for slot {} for block {:?}.", slot_info.slot, slot_info.chain_head.hash(), - err, ); return None From e4b1de110cc6d8f8317b49e327f7c7210e689d84 Mon Sep 17 00:00:00 2001 From: alexgparity Date: Sun, 27 Nov 2022 12:10:32 +0100 Subject: [PATCH 28/28] Use saturating_duration_since --- client/consensus/slots/src/lib.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index b875b194257ec..bc68797dc734e 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -254,13 +254,7 @@ pub trait SimpleSlotWorker { slot_info: &SlotInfo, logging_target: &str, ) -> Option { - let now = Instant::now(); - let remaining_duration = if now > slot_info.ends_at { - Duration::from_millis(0) - } else { - slot_info.ends_at - now - }; - + let remaining_duration = slot_info.ends_at.saturating_duration_since(Instant::now()); let delay = Delay::new(remaining_duration); let cid = slot_info.create_inherent_data.create_inherent_data(); let inherent_data = match futures::future::select(delay, cid).await {