Skip to content

Commit 1bde9ac

Browse files
octolbkchr
authored andcommitted
slots: incrementally backoff claiming slots if finality lags behind (paritytech#7186)
* babe: backoff authoring blocks when finality lags * babe: move backoff authoring params to default constructor * babe: deduplicate the test a bit * babe: set backoff constants in service * babe: use better names for backoff authoring block parameters * babe: remove last unwrap * babe: slight style tweak * babe: fix comment * slots: move backoff block authorship logic to SimpleSlotWorker * aura: append SlotInfo in on_slot * slots: use the correct types for parameters * slots: fix review comments * aura: add missing backoff authoring blocks parameters * slots: add comments for default values * slots: add additional checks in test * slots: update implementation for new master * slots: revert the change to SlotInfo * Fix review comments * slots: rework unit tests for backing off claiming slots * slots: add test for asymptotic behaviour for slot claims * slots: address review comments * slots: add test for max_interval * slots: add assertion for intervals between between claimed slots * slots: remove rustfmt directive * slots: another attempt at explaining authoring_rate * slots: up unfinalized_slack to 50 by default * slots: add tests for time to reach max_interval * slots: fix typo in comments * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * slots: additional tweaks to comments and info calls * slots: rename to BackoffAuthoringOnFinalizedHeadLagging * slots: make the backing off strategy generic * Apply suggestions from code review Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * slots: implement backoff trait for () for simplicity * slots: move logging inside backing off function to make it more specific * aura: add missing function parameter Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
1 parent 84af538 commit 1bde9ac

File tree

9 files changed

+540
-20
lines changed

9 files changed

+540
-20
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/node-template/node/src/service.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
111111

112112
let role = config.role.clone();
113113
let force_authoring = config.force_authoring;
114+
let backoff_authoring_blocks: Option<()> = None;
114115
let name = config.network.node_name.clone();
115116
let enable_grandpa = !config.disable_grandpa;
116117
let prometheus_registry = config.prometheus_registry().cloned();
@@ -155,7 +156,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
155156
let can_author_with =
156157
sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone());
157158

158-
let aura = sc_consensus_aura::start_aura::<_, _, _, _, _, AuraPair, _, _, _>(
159+
let aura = sc_consensus_aura::start_aura::<_, _, _, _, _, AuraPair, _, _, _,_>(
159160
sc_consensus_aura::slot_duration(&*client)?,
160161
client.clone(),
161162
select_chain,
@@ -164,6 +165,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
164165
network.clone(),
165166
inherent_data_providers.clone(),
166167
force_authoring,
168+
backoff_authoring_blocks,
167169
keystore_container.sync_keystore(),
168170
can_author_with,
169171
)?;

bin/node/cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ sc-chain-spec = { version = "2.0.0", path = "../../../client/chain-spec" }
6464
sc-consensus = { version = "0.8.0", path = "../../../client/consensus/common" }
6565
sc-transaction-pool = { version = "2.0.0", path = "../../../client/transaction-pool" }
6666
sc-network = { version = "0.8.0", path = "../../../client/network" }
67+
sc-consensus-slots = { version = "0.8.0", path = "../../../client/consensus/slots" }
6768
sc-consensus-babe = { version = "0.8.0", path = "../../../client/consensus/babe" }
6869
grandpa = { version = "0.8.0", package = "sc-finality-grandpa", path = "../../../client/finality-grandpa" }
6970
sc-client-db = { version = "0.8.0", default-features = false, path = "../../../client/db" }

bin/node/cli/src/service.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ pub fn new_full_base(
204204

205205
let role = config.role.clone();
206206
let force_authoring = config.force_authoring;
207+
let backoff_authoring_blocks =
208+
Some(sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging::default());
207209
let name = config.network.node_name.clone();
208210
let enable_grandpa = !config.disable_grandpa;
209211
let prometheus_registry = config.prometheus_registry().cloned();
@@ -249,6 +251,7 @@ pub fn new_full_base(
249251
sync_oracle: network.clone(),
250252
inherent_data_providers: inherent_data_providers.clone(),
251253
force_authoring,
254+
backoff_authoring_blocks,
252255
babe_link,
253256
can_author_with,
254257
};

client/consensus/aura/src/lib.rs

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ use sp_core::crypto::Public;
5959
use sp_application_crypto::{AppKey, AppPublic};
6060
use sp_runtime::{
6161
generic::{BlockId, OpaqueDigestItemId},
62-
Justification,
62+
traits::NumberFor, Justification,
6363
};
6464
use sp_runtime::traits::{Block as BlockT, Header, DigestItemFor, Zero, Member};
6565
use sp_api::ProvideRuntimeApi;
@@ -73,6 +73,7 @@ use sc_telemetry::{telemetry, CONSENSUS_TRACE, CONSENSUS_DEBUG, CONSENSUS_INFO};
7373

7474
use sc_consensus_slots::{
7575
CheckedHeader, SlotInfo, SlotCompatible, StorageChanges, check_equivocation,
76+
BackoffAuthoringBlocksStrategy,
7677
};
7778

7879
use sp_api::ApiExt;
@@ -138,7 +139,7 @@ impl SlotCompatible for AuraSlotCompatible {
138139
}
139140

140141
/// Start the aura worker. The returned future should be run in a futures executor.
141-
pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
142+
pub fn start_aura<B, C, SC, E, I, P, SO, CAW, BS, Error>(
142143
slot_duration: SlotDuration,
143144
client: Arc<C>,
144145
select_chain: SC,
@@ -147,11 +148,12 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
147148
sync_oracle: SO,
148149
inherent_data_providers: InherentDataProviders,
149150
force_authoring: bool,
151+
backoff_authoring_blocks: Option<BS>,
150152
keystore: SyncCryptoStorePtr,
151153
can_author_with: CAW,
152154
) -> Result<impl Future<Output = ()>, sp_consensus::Error> where
153155
B: BlockT,
154-
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + AuxStore + Send + Sync,
156+
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + AuxStore + HeaderBackend<B> + Send + Sync,
155157
C::Api: AuraApi<B, AuthorityId<P>>,
156158
SC: SelectChain<B>,
157159
E: Environment<B, Error = Error> + Send + Sync + 'static,
@@ -163,6 +165,7 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
163165
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
164166
SO: SyncOracle + Send + Sync + Clone,
165167
CAW: CanAuthorWith<B> + Send,
168+
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + 'static,
166169
{
167170
let worker = AuraWorker {
168171
client,
@@ -171,6 +174,7 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
171174
keystore,
172175
sync_oracle: sync_oracle.clone(),
173176
force_authoring,
177+
backoff_authoring_blocks,
174178
_key_type: PhantomData::<P>,
175179
};
176180
register_aura_inherent_data_provider(
@@ -188,20 +192,22 @@ pub fn start_aura<B, C, SC, E, I, P, SO, CAW, Error>(
188192
))
189193
}
190194

191-
struct AuraWorker<C, E, I, P, SO> {
195+
struct AuraWorker<C, E, I, P, SO, BS> {
192196
client: Arc<C>,
193197
block_import: Arc<Mutex<I>>,
194198
env: E,
195199
keystore: SyncCryptoStorePtr,
196200
sync_oracle: SO,
197201
force_authoring: bool,
202+
backoff_authoring_blocks: Option<BS>,
198203
_key_type: PhantomData<P>,
199204
}
200205

201-
impl<B, C, E, I, P, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for AuraWorker<C, E, I, P, SO>
206+
impl<B, C, E, I, P, Error, SO, BS> sc_consensus_slots::SimpleSlotWorker<B>
207+
for AuraWorker<C, E, I, P, SO, BS>
202208
where
203209
B: BlockT,
204-
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + Sync,
210+
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B> + HeaderBackend<B> + Sync,
205211
C::Api: AuraApi<B, AuthorityId<P>>,
206212
E: Environment<B, Error = Error>,
207213
E::Proposer: Proposer<B, Error = Error, Transaction = sp_api::TransactionFor<C, B>>,
@@ -210,6 +216,7 @@ where
210216
P::Public: AppPublic + Public + Member + Encode + Decode + Hash,
211217
P::Signature: TryFrom<Vec<u8>> + Member + Encode + Decode + Hash + Debug,
212218
SO: SyncOracle + Send + Clone,
219+
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + 'static,
213220
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
214221
{
215222
type BlockImport = I;
@@ -316,6 +323,21 @@ where
316323
self.force_authoring
317324
}
318325

326+
fn should_backoff(&self, slot_number: u64, chain_head: &B::Header) -> bool {
327+
if let Some(ref strategy) = self.backoff_authoring_blocks {
328+
if let Ok(chain_head_slot) = find_pre_digest::<B, P>(chain_head) {
329+
return strategy.should_backoff(
330+
*chain_head.number(),
331+
chain_head_slot,
332+
self.client.info().finalized_number,
333+
slot_number,
334+
self.logging_target(),
335+
);
336+
}
337+
}
338+
false
339+
}
340+
319341
fn sync_oracle(&mut self) -> &mut Self::SyncOracle {
320342
&mut self.sync_oracle
321343
}
@@ -863,7 +885,7 @@ mod tests {
863885
use sp_keyring::sr25519::Keyring;
864886
use sc_client_api::BlockchainEvents;
865887
use sp_consensus_aura::sr25519::AuthorityPair;
866-
use sc_consensus_slots::SimpleSlotWorker;
888+
use sc_consensus_slots::{SimpleSlotWorker, BackoffAuthoringOnFinalizedHeadLagging};
867889
use std::task::Poll;
868890
use sc_block_builder::BlockBuilderProvider;
869891
use sp_runtime::traits::Header as _;
@@ -1012,7 +1034,7 @@ mod tests {
10121034
&inherent_data_providers, slot_duration.get()
10131035
).expect("Registers aura inherent data provider");
10141036

1015-
aura_futures.push(start_aura::<_, _, _, _, _, AuthorityPair, _, _, _>(
1037+
aura_futures.push(start_aura::<_, _, _, _, _, AuthorityPair, _, _, _, _>(
10161038
slot_duration,
10171039
client.clone(),
10181040
select_chain,
@@ -1021,6 +1043,7 @@ mod tests {
10211043
DummyOracle,
10221044
inherent_data_providers,
10231045
false,
1046+
Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
10241047
keystore,
10251048
sp_consensus::AlwaysCanAuthor,
10261049
).expect("Starts aura"));
@@ -1081,6 +1104,7 @@ mod tests {
10811104
keystore: keystore.into(),
10821105
sync_oracle: DummyOracle.clone(),
10831106
force_authoring: false,
1107+
backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
10841108
_key_type: PhantomData::<AuthorityPair>,
10851109
};
10861110

client/consensus/babe/src/lib.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ use log::{debug, info, log, trace, warn};
114114
use prometheus_endpoint::Registry;
115115
use sc_consensus_slots::{
116116
SlotInfo, SlotCompatible, StorageChanges, CheckedHeader, check_equivocation,
117+
BackoffAuthoringBlocksStrategy,
117118
};
118119
use sc_consensus_epochs::{
119120
descendent_query, SharedEpochChanges, EpochChangesFor, Epoch as EpochT, ViableEpochDescriptor,
@@ -354,7 +355,7 @@ impl std::ops::Deref for Config {
354355
}
355356

356357
/// Parameters for BABE.
357-
pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW> {
358+
pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW, BS> {
358359
/// The keystore that manages the keys of the node.
359360
pub keystore: SyncCryptoStorePtr,
360361

@@ -381,6 +382,9 @@ pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW> {
381382
/// Force authoring of blocks even if we are offline
382383
pub force_authoring: bool,
383384

385+
/// Strategy and parameters for backing off block production.
386+
pub backoff_authoring_blocks: Option<BS>,
387+
384388
/// The source of timestamps for relative slots
385389
pub babe_link: BabeLink<B>,
386390

@@ -389,7 +393,7 @@ pub struct BabeParams<B: BlockT, C, E, I, SO, SC, CAW> {
389393
}
390394

391395
/// Start the babe worker.
392-
pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
396+
pub fn start_babe<B, C, SC, E, I, SO, CAW, BS, Error>(BabeParams {
393397
keystore,
394398
client,
395399
select_chain,
@@ -398,9 +402,10 @@ pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
398402
sync_oracle,
399403
inherent_data_providers,
400404
force_authoring,
405+
backoff_authoring_blocks,
401406
babe_link,
402407
can_author_with,
403-
}: BabeParams<B, C, E, I, SO, SC, CAW>) -> Result<
408+
}: BabeParams<B, C, E, I, SO, SC, CAW, BS>) -> Result<
404409
BabeWorker<B>,
405410
sp_consensus::Error,
406411
> where
@@ -416,6 +421,7 @@ pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
416421
Error: std::error::Error + Send + From<ConsensusError> + From<I::Error> + 'static,
417422
SO: SyncOracle + Send + Sync + Clone + 'static,
418423
CAW: CanAuthorWith<B> + Send + 'static,
424+
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + 'static,
419425
{
420426
let config = babe_link.config;
421427
let slot_notification_sinks = Arc::new(Mutex::new(Vec::new()));
@@ -426,6 +432,7 @@ pub fn start_babe<B, C, SC, E, I, SO, CAW, Error>(BabeParams {
426432
env,
427433
sync_oracle: sync_oracle.clone(),
428434
force_authoring,
435+
backoff_authoring_blocks,
429436
keystore,
430437
epoch_changes: babe_link.epoch_changes.clone(),
431438
slot_notification_sinks: slot_notification_sinks.clone(),
@@ -490,19 +497,22 @@ impl<B: BlockT> futures::Future for BabeWorker<B> {
490497
/// Slot notification sinks.
491498
type SlotNotificationSinks<B> = Arc<Mutex<Vec<Sender<(u64, ViableEpochDescriptor<<B as BlockT>::Hash, NumberFor<B>, Epoch>)>>>>;
492499

493-
struct BabeSlotWorker<B: BlockT, C, E, I, SO> {
500+
struct BabeSlotWorker<B: BlockT, C, E, I, SO, BS> {
494501
client: Arc<C>,
495502
block_import: Arc<Mutex<I>>,
496503
env: E,
497504
sync_oracle: SO,
498505
force_authoring: bool,
506+
backoff_authoring_blocks: Option<BS>,
499507
keystore: SyncCryptoStorePtr,
500508
epoch_changes: SharedEpochChanges<B, Epoch>,
501509
slot_notification_sinks: SlotNotificationSinks<B>,
502510
config: Config,
503511
}
504512

505-
impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeSlotWorker<B, C, E, I, SO> where
513+
impl<B, C, E, I, Error, SO, BS> sc_consensus_slots::SimpleSlotWorker<B>
514+
for BabeSlotWorker<B, C, E, I, SO, BS>
515+
where
506516
B: BlockT,
507517
C: ProvideRuntimeApi<B> +
508518
ProvideCache<B> +
@@ -513,6 +523,7 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeSlot
513523
E::Proposer: Proposer<B, Error = Error, Transaction = sp_api::TransactionFor<C, B>>,
514524
I: BlockImport<B, Transaction = sp_api::TransactionFor<C, B>> + Send + Sync + 'static,
515525
SO: SyncOracle + Send + Clone,
526+
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>>,
516527
Error: std::error::Error + Send + From<ConsensusError> + From<I::Error> + 'static,
517528
{
518529
type EpochData = ViableEpochDescriptor<B::Hash, NumberFor<B>, Epoch>;
@@ -657,6 +668,23 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeSlot
657668
self.force_authoring
658669
}
659670

671+
fn should_backoff(&self, slot_number: u64, chain_head: &B::Header) -> bool {
672+
if let Some(ref strategy) = self.backoff_authoring_blocks {
673+
if let Ok(chain_head_slot) = find_pre_digest::<B>(chain_head)
674+
.map(|digest| digest.slot_number())
675+
{
676+
return strategy.should_backoff(
677+
*chain_head.number(),
678+
chain_head_slot,
679+
self.client.info().finalized_number,
680+
slot_number,
681+
self.logging_target(),
682+
);
683+
}
684+
}
685+
false
686+
}
687+
660688
fn sync_oracle(&mut self) -> &mut Self::SyncOracle {
661689
&mut self.sync_oracle
662690
}

client/consensus/babe/src/tests.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use sp_consensus_babe::{
3333
make_transcript,
3434
make_transcript_data,
3535
};
36+
use sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging;
3637
use sc_block_builder::{BlockBuilder, BlockBuilderProvider};
3738
use sp_consensus::{
3839
NoNetwork as DummyOracle, Proposal, RecordProof, AlwaysCanAuthor,
@@ -434,6 +435,7 @@ fn run_one_test(
434435
sync_oracle: DummyOracle,
435436
inherent_data_providers: data.inherent_data_providers.clone(),
436437
force_authoring: false,
438+
backoff_authoring_blocks: Some(BackoffAuthoringOnFinalizedHeadLagging::default()),
437439
babe_link: data.link.clone(),
438440
keystore,
439441
can_author_with: sp_consensus::AlwaysCanAuthor,

client/consensus/slots/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ sc-client-api = { version = "2.0.0", path = "../../api" }
1919
sp-core = { version = "2.0.0", path = "../../../primitives/core" }
2020
sp-trie = { version = "2.0.0", path = "../../../primitives/trie" }
2121
sp-application-crypto = { version = "2.0.0", path = "../../../primitives/application-crypto" }
22+
sp-arithmetic = { version = "2.0.0", path = "../../../primitives/arithmetic" }
2223
sp-blockchain = { version = "2.0.0", path = "../../../primitives/blockchain" }
2324
sp-consensus-slots = { version = "0.8.0", path = "../../../primitives/consensus/slots" }
2425
sp-runtime = { version = "2.0.0", path = "../../../primitives/runtime" }

0 commit comments

Comments
 (0)