Skip to content

Commit

Permalink
Refactory of next_slot method (paritytech#13155)
Browse files Browse the repository at this point in the history
Refactory of `next_slot` method

* Prevents slot worker exit if inherent data provider creation fails
* Failure is not possible anymore
* Fix potential failure after warp-sync where block headers of not already downloaded blocks are used by the inherent data provider
  • Loading branch information
davxy authored and ark0f committed Feb 27, 2023
1 parent 2c16a72 commit 5a33d7b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 33 deletions.
8 changes: 1 addition & 7 deletions client/consensus/slots/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,13 +524,7 @@ pub async fn start_slot_worker<B, C, W, SO, CIDP, Proof>(
let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client);

loop {
let slot_info = match slots.next_slot().await {
Ok(r) => r,
Err(e) => {
warn!(target: LOG_TARGET, "Error while polling for next slot: {}", e);
return
},
};
let slot_info = slots.next_slot().await;

if sync_oracle.is_major_syncing() {
debug!(target: LOG_TARGET, "Skipping proposal slot due to sync.");
Expand Down
59 changes: 33 additions & 26 deletions client/consensus/slots/src/slots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
//! This is used instead of `futures_timer::Interval` because it was unreliable.

use super::{InherentDataProviderExt, Slot, LOG_TARGET};
use sp_consensus::{Error, SelectChain};
use sp_consensus::SelectChain;
use sp_inherents::{CreateInherentDataProviders, InherentDataProvider};
use sp_runtime::traits::{Block as BlockT, Header as HeaderT};

Expand Down Expand Up @@ -90,7 +90,7 @@ impl<B: BlockT> SlotInfo<B> {
pub(crate) struct Slots<Block, SC, IDP> {
last_slot: Slot,
slot_duration: Duration,
inner_delay: Option<Delay>,
until_next_slot: Option<Delay>,
create_inherent_data_providers: IDP,
select_chain: SC,
_phantom: std::marker::PhantomData<Block>,
Expand All @@ -106,7 +106,7 @@ impl<Block, SC, IDP> Slots<Block, SC, IDP> {
Slots {
last_slot: 0.into(),
slot_duration,
inner_delay: None,
until_next_slot: None,
create_inherent_data_providers,
select_chain,
_phantom: Default::default(),
Expand All @@ -122,26 +122,22 @@ where
IDP::InherentDataProviders: crate::InherentDataProviderExt,
{
/// Returns a future that fires when the next slot starts.
pub async fn next_slot(&mut self) -> Result<SlotInfo<Block>, Error> {
pub async fn next_slot(&mut self) -> SlotInfo<Block> {
loop {
self.inner_delay = match self.inner_delay.take() {
None => {
// schedule wait.
// Wait for slot timeout
self.until_next_slot
.take()
.unwrap_or_else(|| {
// Schedule first timeout.
let wait_dur = time_until_next_slot(self.slot_duration);
Some(Delay::new(wait_dur))
},
Some(d) => Some(d),
};

if let Some(inner_delay) = self.inner_delay.take() {
inner_delay.await;
}
// timeout has fired.
Delay::new(wait_dur)
})
.await;

let ends_in = time_until_next_slot(self.slot_duration);
// Schedule delay for next slot.
let wait_dur = time_until_next_slot(self.slot_duration);
self.until_next_slot = Some(Delay::new(wait_dur));

// reschedule delay for next slot.
self.inner_delay = Some(Delay::new(ends_in));
let chain_head = match self.select_chain.best_chain().await {
Ok(x) => x,
Err(e) => {
Expand All @@ -150,30 +146,41 @@ where
"Unable to author block in slot. No best block header: {}",
e,
);
// Let's try at the next slot..
self.inner_delay.take();
// Let's retry at the next slot.
continue
},
};

let inherent_data_providers = self
let inherent_data_providers = match self
.create_inherent_data_providers
.create_inherent_data_providers(chain_head.hash(), ())
.await?;
.await
{
Ok(x) => x,
Err(e) => {
log::warn!(
target: LOG_TARGET,
"Unable to author block in slot. Failure creating inherent data provider: {}",
e,
);
// Let's retry at the next slot.
continue
},
};

let slot = inherent_data_providers.slot();

// never yield the same slot twice.
// Never yield the same slot twice.
if slot > self.last_slot {
self.last_slot = slot;

break Ok(SlotInfo::new(
break SlotInfo::new(
slot,
Box::new(inherent_data_providers),
self.slot_duration,
chain_head,
None,
))
)
}
}
}
Expand Down

0 comments on commit 5a33d7b

Please sign in to comment.