diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index 51b63e614f..c19824e9ea 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/substrate/bin/node-template/node/src/service.rs @@ -245,6 +245,7 @@ pub fn new_full(mut config: Configuration) -> Result sync_oracle: network.clone(), justification_sync_link: network.clone(), block_proposal_slot_portion: SlotProportion::new(2f32 / 3f32), + max_block_proposal_slot_portion: None, telemetry: telemetry.as_ref().map(|x| x.handle()), }, )?; diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 06e1fcc804..8fa3d2ed77 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -333,6 +333,7 @@ pub fn new_full_base( babe_link, can_author_with, block_proposal_slot_portion: SlotProportion::new(0.5), + max_block_proposal_slot_portion: None, telemetry: telemetry.as_ref().map(|x| x.handle()), }; diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index d0b0cefe8d..845e920cfc 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -140,6 +140,9 @@ pub struct StartAuraParams { /// 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, + /// The maximum proportion of the slot dedicated to proposing with any lenience factor applied + /// due to no blocks being produced. + pub max_block_proposal_slot_portion: Option, /// Telemetry instance used to report telemetry metrics. pub telemetry: Option, } @@ -160,9 +163,11 @@ pub fn start_aura( keystore, can_author_with, block_proposal_slot_portion, + max_block_proposal_slot_portion, telemetry, }: StartAuraParams, -) -> Result, sp_consensus::Error> where +) -> Result, sp_consensus::Error> +where P: Pair + Send + Sync, P::Public: AppPublic + Hash + Member + Encode + Decode, P::Signature: TryFrom> + Hash + Member + Encode + Decode, @@ -192,6 +197,7 @@ pub fn start_aura( backoff_authoring_blocks, telemetry, block_proposal_slot_portion, + max_block_proposal_slot_portion, }); Ok(sc_consensus_slots::start_slot_worker( @@ -228,6 +234,9 @@ pub struct BuildAuraWorkerParams { /// 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, + /// The maximum proportion of the slot dedicated to proposing with any lenience factor applied + /// due to no blocks being produced. + pub max_block_proposal_slot_portion: Option, /// Telemetry instance used to report telemetry metrics. pub telemetry: Option, } @@ -245,10 +254,12 @@ pub fn build_aura_worker( backoff_authoring_blocks, keystore, block_proposal_slot_portion, + max_block_proposal_slot_portion, telemetry, force_authoring, }: BuildAuraWorkerParams, -) -> impl sc_consensus_slots::SlotWorker>::Proof> where +) -> impl sc_consensus_slots::SlotWorker>::Proof> +where B: BlockT, C: ProvideRuntimeApi + BlockOf + ProvideCache + AuxStore + HeaderBackend + Send + Sync, C::Api: AuraApi>, @@ -274,6 +285,7 @@ pub fn build_aura_worker( backoff_authoring_blocks, telemetry, block_proposal_slot_portion, + max_block_proposal_slot_portion, _key_type: PhantomData::

, } } @@ -288,6 +300,7 @@ struct AuraWorker { force_authoring: bool, backoff_authoring_blocks: Option, block_proposal_slot_portion: SlotProportion, + max_block_proposal_slot_portion: Option, telemetry: Option, _key_type: PhantomData

, } @@ -452,42 +465,17 @@ where self.telemetry.clone() } - fn proposing_remaining_duration( - &self, - slot_info: &SlotInfo, - ) -> std::time::Duration { - let max_proposing = slot_info.duration.mul_f32(self.block_proposal_slot_portion.get()); + fn proposing_remaining_duration(&self, slot_info: &SlotInfo) -> std::time::Duration { + let parent_slot = find_pre_digest::(&slot_info.chain_head).ok(); - 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 slot_info.chain_head.number().is_zero() { - return slot_remaining - } - - let parent_slot = match find_pre_digest::(&slot_info.chain_head) { - Err(_) => return slot_remaining, - Ok(d) => d, - }; - - if let Some(slot_lenience) = - sc_consensus_slots::slot_lenience_exponential(parent_slot, slot_info) - { - debug!( - target: "aura", - "No block for {} slots. Applying linear lenience of {}s", - slot_info.slot.saturating_sub(parent_slot + 1), - slot_lenience.as_secs(), - ); - - slot_remaining + slot_lenience - } else { - slot_remaining - } + sc_consensus_slots::proposing_remaining_duration( + parent_slot, + slot_info, + &self.block_proposal_slot_portion, + self.max_block_proposal_slot_portion.as_ref(), + sc_consensus_slots::SlotLenienceType::Exponential, + self.logging_target(), + ) } } @@ -759,6 +747,7 @@ mod tests { keystore, can_author_with: sp_consensus::AlwaysCanAuthor, block_proposal_slot_portion: SlotProportion::new(0.5), + max_block_proposal_slot_portion: None, telemetry: None, }).expect("Starts aura")); } @@ -823,6 +812,7 @@ mod tests { telemetry: None, _key_type: PhantomData::, block_proposal_slot_portion: SlotProportion::new(0.5), + max_block_proposal_slot_portion: None, }; let head = Header::new( @@ -873,6 +863,7 @@ mod tests { telemetry: None, _key_type: PhantomData::, block_proposal_slot_portion: SlotProportion::new(0.5), + max_block_proposal_slot_portion: None, }; let head = client.header(&BlockId::Number(0)).unwrap().unwrap(); diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 8aa92f3781..8112a00416 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -412,30 +412,35 @@ pub struct BabeParams { /// because there were no blocks produced for some slots. pub block_proposal_slot_portion: SlotProportion, + /// The maximum proportion of the slot dedicated to proposing with any lenience factor applied + /// due to no blocks being produced. + pub max_block_proposal_slot_portion: Option, + /// Handle use to report telemetries. pub telemetry: Option, } /// Start the babe worker. -pub fn start_babe(BabeParams { - keystore, - client, - select_chain, - env, - block_import, - sync_oracle, - justification_sync_link, - create_inherent_data_providers, - force_authoring, - backoff_authoring_blocks, - babe_link, - can_author_with, - block_proposal_slot_portion, - telemetry, -}: BabeParams) -> Result< - BabeWorker, - sp_consensus::Error, -> where +pub fn start_babe( + BabeParams { + keystore, + client, + select_chain, + env, + block_import, + sync_oracle, + justification_sync_link, + create_inherent_data_providers, + force_authoring, + backoff_authoring_blocks, + babe_link, + can_author_with, + block_proposal_slot_portion, + max_block_proposal_slot_portion, + telemetry, + }: BabeParams, +) -> Result, sp_consensus::Error> +where B: BlockT, C: ProvideRuntimeApi + ProvideCache @@ -480,6 +485,7 @@ pub fn start_babe(BabeParams { slot_notification_sinks: slot_notification_sinks.clone(), config: config.clone(), block_proposal_slot_portion, + max_block_proposal_slot_portion, telemetry, }; @@ -630,6 +636,7 @@ struct BabeSlotWorker { slot_notification_sinks: SlotNotificationSinks, config: Config, block_proposal_slot_portion: SlotProportion, + max_block_proposal_slot_portion: Option, telemetry: Option, } @@ -637,10 +644,10 @@ impl sc_consensus_slots::SimpleSlotWorker for BabeSlotWorker where B: BlockT, - C: ProvideRuntimeApi + - ProvideCache + - HeaderBackend + - HeaderMetadata, + C: ProvideRuntimeApi + + ProvideCache + + HeaderBackend + + HeaderMetadata, C::Api: BabeApi, E: Environment, E::Proposer: Proposer>, @@ -832,42 +839,17 @@ where self.telemetry.clone() } - fn proposing_remaining_duration( - &self, - slot_info: &SlotInfo, - ) -> std::time::Duration { - let max_proposing = slot_info.duration.mul_f32(self.block_proposal_slot_portion.get()); + fn proposing_remaining_duration(&self, slot_info: &SlotInfo) -> std::time::Duration { + let parent_slot = find_pre_digest::(&slot_info.chain_head).ok().map(|d| d.slot()); - 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 slot_info.chain_head.number().is_zero() { - return slot_remaining - } - - let parent_slot = match find_pre_digest::(&slot_info.chain_head) { - Err(_) => return slot_remaining, - Ok(d) => d.slot(), - }; - - if let Some(slot_lenience) = - sc_consensus_slots::slot_lenience_exponential(parent_slot, slot_info) - { - debug!( - target: "babe", - "No block for {} slots. Applying exponential lenience of {}s", - slot_info.slot.saturating_sub(parent_slot + 1), - slot_lenience.as_secs(), - ); - - slot_remaining + slot_lenience - } else { - slot_remaining - } + sc_consensus_slots::proposing_remaining_duration( + parent_slot, + slot_info, + &self.block_proposal_slot_portion, + self.max_block_proposal_slot_portion.as_ref(), + sc_consensus_slots::SlotLenienceType::Exponential, + self.logging_target(), + ) } } diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 467de9683c..3392ffade9 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -473,6 +473,7 @@ fn run_one_test(mutator: impl Fn(&mut TestHeader, Stage) + Send + Sync + 'static can_author_with: sp_consensus::AlwaysCanAuthor, justification_sync_link: (), block_proposal_slot_portion: SlotProportion::new(0.5), + max_block_proposal_slot_portion: None, telemetry: None, }).expect("Starts babe")); } diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 188aa52881..1ec89a6f51 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -666,6 +666,96 @@ impl SlotProportion { } } +/// The strategy used to calculate the slot lenience used to increase the block proposal time when +/// slots have been skipped with no blocks authored. +pub enum SlotLenienceType { + /// Increase the lenience linearly with the number of skipped slots. + Linear, + /// Increase the lenience exponentially with the number of skipped slots. + Exponential, +} + +impl SlotLenienceType { + fn as_str(&self) -> &'static str { + match self { + SlotLenienceType::Linear => "linear", + SlotLenienceType::Exponential => "exponential", + } + } +} + +/// Calculate the remaining duration for block proposal taking into account whether any slots have +/// been skipped and applying the given lenience strategy. If `max_block_proposal_slot_portion` is +/// not none this method guarantees that the returned duration must be lower or equal to +/// `slot_info.duration * max_block_proposal_slot_portion`. +pub fn proposing_remaining_duration( + parent_slot: Option, + slot_info: &SlotInfo, + block_proposal_slot_portion: &SlotProportion, + max_block_proposal_slot_portion: Option<&SlotProportion>, + slot_lenience_type: SlotLenienceType, + log_target: &str, +) -> Duration { + use sp_runtime::traits::Zero; + + let proposing_duration = slot_info + .duration + .mul_f32(block_proposal_slot_portion.get()); + + let slot_remaining = slot_info + .ends_at + .checked_duration_since(std::time::Instant::now()) + .unwrap_or_default(); + + let proposing_duration = std::cmp::min(slot_remaining, proposing_duration); + + // If parent is genesis block, we don't require any lenience factor. + if slot_info.chain_head.number().is_zero() { + return proposing_duration; + } + + let parent_slot = match parent_slot { + Some(parent_slot) => parent_slot, + None => return proposing_duration, + }; + + let slot_lenience = match slot_lenience_type { + SlotLenienceType::Exponential => slot_lenience_exponential(parent_slot, slot_info), + SlotLenienceType::Linear => slot_lenience_linear(parent_slot, slot_info), + }; + + if let Some(slot_lenience) = slot_lenience { + let lenient_proposing_duration = + proposing_duration + slot_lenience.mul_f32(block_proposal_slot_portion.get()); + + // if we defined a maximum portion of the slot for proposal then we must make sure the + // lenience doesn't go over it + let lenient_proposing_duration = + if let Some(ref max_block_proposal_slot_portion) = max_block_proposal_slot_portion { + std::cmp::min( + lenient_proposing_duration, + slot_info + .duration + .mul_f32(max_block_proposal_slot_portion.get()), + ) + } else { + lenient_proposing_duration + }; + + debug!( + target: log_target, + "No block for {} slots. Applying {} lenience, total proposing duration: {}", + slot_info.slot.saturating_sub(parent_slot + 1), + slot_lenience_type.as_str(), + lenient_proposing_duration.as_secs(), + ); + + lenient_proposing_duration + } else { + proposing_duration + } +} + /// 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 @@ -703,7 +793,7 @@ pub fn slot_lenience_exponential( /// a linear backoff of at most `20 * slot_duration`, if no slots were skipped /// this method will return `None.` pub fn slot_lenience_linear( - parent_slot: u64, + parent_slot: Slot, slot_info: &SlotInfo, ) -> Option { // never give more than 20 times more lenience. @@ -839,7 +929,7 @@ mod test { duration: SLOT_DURATION, timestamp: Default::default(), inherent_data: Default::default(), - ends_at: Instant::now(), + ends_at: Instant::now() + SLOT_DURATION, chain_head: Header::new( 1, Default::default(), @@ -897,6 +987,36 @@ mod test { ); } + #[test] + fn proposing_remaining_duration_should_apply_lenience_based_on_proposal_slot_proportion() { + assert_eq!( + proposing_remaining_duration( + Some(0.into()), + &slot(2), + &SlotProportion(0.25), + None, + SlotLenienceType::Linear, + "test", + ), + SLOT_DURATION.mul_f32(0.25 * 2.0), + ); + } + + #[test] + fn proposing_remaining_duration_should_never_exceed_max_proposal_slot_proportion() { + assert_eq!( + proposing_remaining_duration( + Some(0.into()), + &slot(100), + &SlotProportion(0.25), + Some(SlotProportion(0.9)).as_ref(), + SlotLenienceType::Exponential, + "test", + ), + SLOT_DURATION.mul_f32(0.9), + ); + } + #[derive(PartialEq, Debug)] struct HeadState { head_number: NumberFor,