From b25d29a50247f284cde5b24e270b6db7b4a81e7e Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 21 Nov 2023 15:22:42 +0100 Subject: [PATCH] cumulus-consensus-common: block import: `delayed_best_block` flag added (#2001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR adds the `delayed_best_block` flag to `ParachainBlockImport`. If not set, the `params.fork_choice` is not updated (to `ForkChoiceStrategy::Custom`) during the block import. When `delayed_best_block` is set to `false` all parachain blocks on the [longest fork](https://github.com/paritytech/polkadot-sdk/blob/552be4800d9e4b480f79a300fc531783e04be099/substrate/client/service/src/client/client.rs#L708-L709) will be notified as the best block, allowing transaction pool to be updated with every imported block. Otherwise imported blocks will not be notified as best blocks (`fork_choice=ForkChoiceStrategy::Custom(false)`), and transaction pool would be updated only with best block received from relay-chain. Improvement for: #1202 --------- Co-authored-by: Bastian Köcher --- cumulus/client/consensus/common/src/lib.rs | 39 +++++++++++++++----- cumulus/client/consensus/common/src/tests.rs | 6 ++- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/cumulus/client/consensus/common/src/lib.rs b/cumulus/client/consensus/common/src/lib.rs index 08bceabb2b..cebe34e7ea 100644 --- a/cumulus/client/consensus/common/src/lib.rs +++ b/cumulus/client/consensus/common/src/lib.rs @@ -111,12 +111,15 @@ impl ParachainConsensus for Box + Send + /// Parachain specific block import. /// -/// This is used to set `block_import_params.fork_choice` to `false` as long as the block origin is -/// not `NetworkInitialSync`. The best block for parachains is determined by the relay chain. -/// Meaning we will update the best block, as it is included by the relay-chain. +/// Specialized block import for parachains. It supports to delay setting the best block until the +/// relay chain has included a candidate in its best block. By default the delayed best block +/// setting is disabled. The block import also monitors the imported blocks and prunes by default if +/// there are too many blocks at the same height. Too many blocks at the same height can for example +/// happen if the relay chain is rejecting the parachain blocks in the validation. pub struct ParachainBlockImport { inner: BI, monitor: Option>>, + delayed_best_block: bool, } impl> ParachainBlockImport { @@ -141,13 +144,27 @@ impl> ParachainBlockImport let monitor = level_limit.map(|level_limit| SharedData::new(LevelMonitor::new(level_limit, backend))); - Self { inner, monitor } + Self { inner, monitor, delayed_best_block: false } + } + + /// Create a new instance which delays setting the best block. + /// + /// The number of leaves per level limit is set to `LevelLimit::Default`. + pub fn new_with_delayed_best_block(inner: BI, backend: Arc) -> Self { + Self { + delayed_best_block: true, + ..Self::new_with_limit(inner, backend, LevelLimit::Default) + } } } impl Clone for ParachainBlockImport { fn clone(&self) -> Self { - ParachainBlockImport { inner: self.inner.clone(), monitor: self.monitor.clone() } + ParachainBlockImport { + inner: self.inner.clone(), + monitor: self.monitor.clone(), + delayed_best_block: self.delayed_best_block, + } } } @@ -182,11 +199,13 @@ where params.finalized = true; } - // Best block is determined by the relay chain, or if we are doing the initial sync - // we import all blocks as new best. - params.fork_choice = Some(sc_consensus::ForkChoiceStrategy::Custom( - params.origin == sp_consensus::BlockOrigin::NetworkInitialSync, - )); + if self.delayed_best_block { + // Best block is determined by the relay chain, or if we are doing the initial sync + // we import all blocks as new best. + params.fork_choice = Some(sc_consensus::ForkChoiceStrategy::Custom( + params.origin == sp_consensus::BlockOrigin::NetworkInitialSync, + )); + } let maybe_lock = self.monitor.as_ref().map(|monitor_lock| { let mut monitor = monitor_lock.shared_data_locked(); diff --git a/cumulus/client/consensus/common/src/tests.rs b/cumulus/client/consensus/common/src/tests.rs index 9658a0add7..597d1ab2ac 100644 --- a/cumulus/client/consensus/common/src/tests.rs +++ b/cumulus/client/consensus/common/src/tests.rs @@ -1124,7 +1124,8 @@ fn find_potential_parents_aligned_with_pending() { let backend = Arc::new(Backend::new_test(1000, 1)); let client = Arc::new(TestClientBuilder::with_backend(backend.clone()).build()); - let mut para_import = ParachainBlockImport::new(client.clone(), backend.clone()); + let mut para_import = + ParachainBlockImport::new_with_delayed_best_block(client.clone(), backend.clone()); let relay_parent = relay_hash_from_block_num(10); // Choose different relay parent for alternative chain to get new hashes. @@ -1279,7 +1280,8 @@ fn find_potential_parents_aligned_no_pending() { let backend = Arc::new(Backend::new_test(1000, 1)); let client = Arc::new(TestClientBuilder::with_backend(backend.clone()).build()); - let mut para_import = ParachainBlockImport::new(client.clone(), backend.clone()); + let mut para_import = + ParachainBlockImport::new_with_delayed_best_block(client.clone(), backend.clone()); let relay_parent = relay_hash_from_block_num(10); // Choose different relay parent for alternative chain to get new hashes.