diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index a5030f1b35..368767fbdd 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/substrate/bin/node-template/node/src/service.rs @@ -9,7 +9,7 @@ use sp_inherents::InherentDataProviders; use sc_executor::native_executor_instance; pub use sc_executor::NativeExecutor; use sp_consensus_aura::sr25519::AuthorityPair as AuraPair; -use sc_consensus_aura::{ImportQueueParams, StartAuraParams}; +use sc_consensus_aura::{ImportQueueParams, StartAuraParams, SlotProportion}; use sc_finality_grandpa::SharedVoterState; use sc_keystore::LocalKeystore; use sc_telemetry::TelemetrySpan; @@ -212,6 +212,7 @@ pub fn new_full(mut config: Configuration) -> Result keystore: keystore_container.sync_keystore(), can_author_with, sync_oracle: network.clone(), + block_proposal_slot_portion: SlotProportion::new(2f32 / 3f32), }, )?; diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 80561a78a0..6fed5bf5c6 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -25,8 +25,7 @@ use sc_consensus_babe; use node_primitives::Block; use node_runtime::RuntimeApi; use sc_service::{ - config::{Configuration}, error::{Error as ServiceError}, - RpcHandlers, TaskManager, + config::Configuration, error::Error as ServiceError, RpcHandlers, TaskManager, }; use sp_inherents::InherentDataProviders; use sc_network::{Event, NetworkService}; @@ -35,6 +34,7 @@ use futures::prelude::*; use sc_client_api::{ExecutorProvider, RemoteBackend}; use node_executor::Executor; use sc_telemetry::{TelemetryConnectionNotifier, TelemetrySpan}; +use sc_consensus_babe::SlotProportion; type FullClient = sc_service::TFullClient; type FullBackend = sc_service::TFullBackend; @@ -279,6 +279,7 @@ pub fn new_full_base( backoff_authoring_blocks, babe_link, can_author_with, + block_proposal_slot_portion: SlotProportion::new(0.5), }; let babe = sc_consensus_babe::start_babe(babe_config)?; diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 1c30f136ea..12ce0e1697 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -69,6 +69,7 @@ pub use sp_consensus_aura::{ }; pub use sp_consensus::SyncOracle; pub use import_queue::{ImportQueueParams, import_queue, AuraBlockImport, CheckForEquivocation}; +pub use sc_consensus_slots::SlotProportion; type AuthorityId

=

::Public; @@ -142,6 +143,12 @@ pub struct StartAuraParams { pub keystore: SyncCryptoStorePtr, /// Can we author a block with this node? pub can_author_with: CAW, + /// The proportion of the slot dedicated to proposing. + /// + /// The block proposing will be limited to this proportion of the slot from the starting of the + /// slot. However, the proposing can still take longer when there is some lenience factor applied, + /// because there were no blocks produced for some slots. + pub block_proposal_slot_portion: SlotProportion, } /// Start the aura worker. The returned future should be run in a futures executor. @@ -158,6 +165,7 @@ pub fn start_aura( backoff_authoring_blocks, keystore, can_author_with, + block_proposal_slot_portion, }: StartAuraParams, ) -> Result, sp_consensus::Error> where B: BlockT, @@ -184,6 +192,7 @@ pub fn start_aura( force_authoring, backoff_authoring_blocks, _key_type: PhantomData::

, + block_proposal_slot_portion, }; register_aura_inherent_data_provider( &inherent_data_providers, @@ -208,6 +217,7 @@ struct AuraWorker { sync_oracle: SO, force_authoring: bool, backoff_authoring_blocks: Option, + block_proposal_slot_portion: SlotProportion, _key_type: PhantomData

, } @@ -365,11 +375,22 @@ where &self, head: &B::Header, slot_info: &SlotInfo, - ) -> Option { - let slot_remaining = self.slot_remaining_duration(slot_info); + ) -> std::time::Duration { + let max_proposing = slot_info.duration.mul_f32(self.block_proposal_slot_portion.get()); + + let slot_remaining = slot_info.ends_at + .checked_duration_since(std::time::Instant::now()) + .unwrap_or_default(); + + let slot_remaining = std::cmp::min(slot_remaining, max_proposing); + + // If parent is genesis block, we don't require any lenience factor. + if head.number().is_zero() { + return slot_remaining + } let parent_slot = match find_pre_digest::(head) { - Err(_) => return Some(slot_remaining), + Err(_) => return slot_remaining, Ok(d) => d, }; @@ -383,9 +404,9 @@ where slot_lenience.as_secs(), ); - Some(slot_remaining + slot_lenience) + slot_remaining + slot_lenience } else { - Some(slot_remaining) + slot_remaining } } } @@ -648,6 +669,7 @@ mod tests { backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()), keystore, can_author_with: sp_consensus::AlwaysCanAuthor, + block_proposal_slot_portion: SlotProportion::new(0.5), }).expect("Starts aura")); } @@ -708,6 +730,7 @@ mod tests { force_authoring: false, backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()), _key_type: PhantomData::, + block_proposal_slot_portion: SlotProportion::new(0.5), }; let head = Header::new( @@ -755,6 +778,7 @@ mod tests { force_authoring: false, backoff_authoring_blocks: Option::<()>::None, _key_type: PhantomData::, + block_proposal_slot_portion: SlotProportion::new(0.5), }; let head = client.header(&BlockId::Number(0)).unwrap().unwrap(); @@ -766,7 +790,7 @@ mod tests { timestamp: 0, ends_at: Instant::now() + Duration::from_secs(100), inherent_data: InherentData::new(), - duration: 1000, + duration: Duration::from_millis(1000), }, )).unwrap(); diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 5622df48db..7f2f47da5d 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -74,6 +74,7 @@ pub use sp_consensus_babe::{ }, }; pub use sp_consensus::SyncOracle; +pub use sc_consensus_slots::SlotProportion; use std::{ collections::HashMap, sync::Arc, u64, pin::Pin, time::{Instant, Duration}, any::Any, borrow::Cow, convert::TryInto, @@ -394,6 +395,13 @@ pub struct BabeParams { /// Checks if the current native implementation can author with a runtime at a given block. pub can_author_with: CAW, + + /// The proportion of the slot dedicated to proposing. + /// + /// The block proposing will be limited to this proportion of the slot from the starting of the + /// slot. However, the proposing can still take longer when there is some lenience factor applied, + /// because there were no blocks produced for some slots. + pub block_proposal_slot_portion: SlotProportion, } /// Start the babe worker. @@ -409,6 +417,7 @@ pub fn start_babe(BabeParams { backoff_authoring_blocks, babe_link, can_author_with, + block_proposal_slot_portion, }: BabeParams) -> Result< BabeWorker, sp_consensus::Error, @@ -443,6 +452,7 @@ pub fn start_babe(BabeParams { epoch_changes: babe_link.epoch_changes.clone(), slot_notification_sinks: slot_notification_sinks.clone(), config: config.clone(), + block_proposal_slot_portion, }; register_babe_inherent_data_provider(&inherent_data_providers, config.slot_duration())?; @@ -597,6 +607,7 @@ struct BabeSlotWorker { epoch_changes: SharedEpochChanges, slot_notification_sinks: SlotNotificationSinks, config: Config, + block_proposal_slot_portion: SlotProportion, } impl sc_consensus_slots::SimpleSlotWorker @@ -791,16 +802,22 @@ where &self, parent_head: &B::Header, slot_info: &SlotInfo, - ) -> Option { - let slot_remaining = self.slot_remaining_duration(slot_info); + ) -> std::time::Duration { + let max_proposing = slot_info.duration.mul_f32(self.block_proposal_slot_portion.get()); + + let slot_remaining = slot_info.ends_at + .checked_duration_since(Instant::now()) + .unwrap_or_default(); + + let slot_remaining = std::cmp::min(slot_remaining, max_proposing); // If parent is genesis block, we don't require any lenience factor. if parent_head.number().is_zero() { - return Some(slot_remaining) + return slot_remaining } let parent_slot = match find_pre_digest::(parent_head) { - Err(_) => return Some(slot_remaining), + Err(_) => return slot_remaining, Ok(d) => d.slot(), }; @@ -814,9 +831,9 @@ where slot_lenience.as_secs(), ); - Some(slot_remaining + slot_lenience) + slot_remaining + slot_lenience } else { - Some(slot_remaining) + slot_remaining } } } diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index a33a509ddc..9ffffc37fd 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -431,6 +431,7 @@ fn run_one_test( babe_link: data.link.clone(), keystore, can_author_with: sp_consensus::AlwaysCanAuthor, + block_proposal_slot_portion: SlotProportion::new(0.5), }).expect("Starts babe")); } futures::executor::block_on(future::select( diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 564d5c28c5..1b40ac102d 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -32,7 +32,7 @@ pub use slots::SlotInfo; use slots::Slots; pub use aux_schema::{check_equivocation, MAX_SLOT_CAPACITY, PRUNING_BOUND}; -use std::{fmt::Debug, ops::Deref, pin::Pin, sync::Arc, time::{Instant, Duration}}; +use std::{fmt::Debug, ops::Deref, pin::Pin, sync::Arc, time::Duration}; use codec::{Decode, Encode}; use futures::{prelude::*, future::{self, Either}}; use futures_timer::Delay; @@ -180,24 +180,12 @@ pub trait SimpleSlotWorker { /// Returns a `Proposer` to author on top of the given block. fn proposer(&mut self, block: &B::Header) -> Self::CreateProposer; - /// Remaining duration of the slot. - fn slot_remaining_duration(&self, slot_info: &SlotInfo) -> Duration { - let now = Instant::now(); - if now < slot_info.ends_at { - slot_info.ends_at.duration_since(now) - } else { - Duration::from_millis(0) - } - } - - /// Remaining duration for proposing. None means unlimited. + /// Remaining duration for proposing. fn proposing_remaining_duration( &self, - _head: &B::Header, + head: &B::Header, slot_info: &SlotInfo, - ) -> Option { - Some(self.slot_remaining_duration(slot_info)) - } + ) -> Duration; /// Implements [`SlotWorker::on_slot`]. fn on_slot( @@ -210,21 +198,19 @@ pub trait SimpleSlotWorker { { let (timestamp, slot) = (slot_info.timestamp, slot_info.slot); - let slot_remaining_duration = self.slot_remaining_duration(&slot_info); let proposing_remaining_duration = self.proposing_remaining_duration(&chain_head, &slot_info); - let proposing_remaining = match proposing_remaining_duration { - Some(r) if r.as_secs() == 0 && r.as_nanos() == 0 => { - debug!( - target: self.logging_target(), - "Skipping proposal slot {} since there's no time left to propose", - slot, - ); + let proposing_remaining = if proposing_remaining_duration == Duration::default() { + debug!( + target: self.logging_target(), + "Skipping proposal slot {} since there's no time left to propose", + slot, + ); - return Box::pin(future::ready(None)); - }, - Some(r) => Box::new(Delay::new(r)) as Box + Unpin + Send>, - None => Box::new(future::pending()) as Box<_>, + return Box::pin(future::ready(None)); + } else { + Box::new(Delay::new(proposing_remaining_duration)) + as Box + Unpin + Send> }; let epoch_data = match self.epoch_data(&chain_head, slot) { @@ -298,20 +284,25 @@ pub trait SimpleSlotWorker { let logs = self.pre_digest_data(slot, &claim); - // deadline our production to approx. the end of the slot + // 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 proposing = awaiting_proposer.and_then(move |proposer| proposer.propose( slot_info.inherent_data, sp_runtime::generic::Digest { logs, }, - slot_remaining_duration, + proposing_remaining_duration.mul_f32(0.98), ).map_err(|e| sp_consensus::Error::ClientImport(format!("{:?}", e)))); let proposal_work = futures::future::select(proposing, proposing_remaining).map(move |v| match v { Either::Left((b, _)) => b.map(|b| (b, claim)), Either::Right(_) => { - info!("⌛️ Discarding proposal for slot {}; block production took too long", slot); + info!( + "⌛️ Discarding proposal for slot {}; block production took too long", + slot, + ); // If the node was compiled with debug, tell the user to use release optimizations. #[cfg(build_type="debug")] info!("👉 Recompile your node in `--release` mode to mitigate this problem."); @@ -381,8 +372,7 @@ pub trait SimpleSlotWorker { } } -impl> SlotWorker>::Proof> for T -{ +impl> SlotWorker>::Proof> for T { fn on_slot( &mut self, chain_head: B::Header, @@ -564,6 +554,24 @@ impl SlotDuration { } } +/// A unit type wrapper to express the proportion of a slot. +pub struct SlotProportion(f32); + +impl SlotProportion { + /// Create a new proportion. + /// + /// The given value `inner` should be in the range `[0,1]`. If the value is not in the required + /// range, it is clamped into the range. + pub fn new(inner: f32) -> Self { + Self(inner.clamp(0.0, 1.0)) + } + + /// Returns the inner that is guaranted to be in the range `[0,1]`. + pub fn get(&self) -> f32 { + self.0 + } +} + /// Calculate a slot duration lenience based on the number of missed slots from current /// to parent. If the number of skipped slots is greated than 0 this method will apply /// an exponential backoff of at most `2^7 * slot_duration`, if no slots were skipped @@ -589,7 +597,7 @@ pub fn slot_lenience_exponential(parent_slot: Slot, slot_info: &SlotInfo) -> Opt let slot_lenience = skipped_slots / BACKOFF_STEP; let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP); let slot_lenience = 1 << slot_lenience; - Some(Duration::from_millis(slot_lenience * slot_info.duration)) + Some(slot_lenience * slot_info.duration) } } @@ -613,7 +621,8 @@ pub fn slot_lenience_linear(parent_slot: Slot, slot_info: &SlotInfo) -> Option super::slots::SlotInfo { super::slots::SlotInfo { slot: slot.into(), - duration: SLOT_DURATION.as_millis() as u64, + duration: SLOT_DURATION, timestamp: Default::default(), inherent_data: Default::default(), ends_at: Instant::now(), diff --git a/substrate/client/consensus/slots/src/slots.rs b/substrate/client/consensus/slots/src/slots.rs index d3bddccce0..b23d676035 100644 --- a/substrate/client/consensus/slots/src/slots.rs +++ b/substrate/client/consensus/slots/src/slots.rs @@ -40,9 +40,11 @@ pub fn duration_now() -> Duration { } /// Returns the duration until the next slot, based on current duration since -pub fn time_until_next(now: Duration, slot_duration: u64) -> Duration { - let remaining_full_millis = slot_duration - (now.as_millis() as u64 % slot_duration) - 1; - Duration::from_millis(remaining_full_millis) +pub fn time_until_next(now: Duration, slot_duration: Duration) -> Duration { + let remaining_full_millis = slot_duration.as_millis() + - (now.as_millis() % slot_duration.as_millis()) + - 1; + Duration::from_millis(remaining_full_millis as u64) } /// Information about a slot. @@ -56,13 +58,13 @@ pub struct SlotInfo { /// The inherent data. pub inherent_data: InherentData, /// Slot duration. - pub duration: u64, + pub duration: Duration, } /// A stream that returns every time there is a new slot. pub(crate) struct Slots { last_slot: Slot, - slot_duration: u64, + slot_duration: Duration, inner_delay: Option, inherent_data_providers: InherentDataProviders, timestamp_extractor: SC, @@ -77,7 +79,7 @@ impl Slots { ) -> Self { Slots { last_slot: 0.into(), - slot_duration, + slot_duration: Duration::from_millis(slot_duration), inner_delay: None, inherent_data_providers, timestamp_extractor,