From c1b7c3025aa4423d4cf3e57309b60fb7602c2db6 Mon Sep 17 00:00:00 2001
From: Sebastian Kunert <skunert49@gmail.com>
Date: Thu, 16 Jan 2025 16:57:02 +0100
Subject: [PATCH] Improve comments
---
Cargo.lock | 4 ---
cumulus/client/consensus/aura/Cargo.toml | 6 +----
.../consensus/aura/src/collators/mod.rs | 27 +++++++++++--------
3 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 24292f5a52f..75ff73c59f0 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4642,9 +4642,7 @@ dependencies = [
"cumulus-relay-chain-interface",
"cumulus-test-client",
"cumulus-test-relay-sproof-builder 0.7.0",
- "cumulus-test-runtime",
"futures",
- "futures-timer",
"parity-scale-codec",
"parking_lot 0.12.3",
"polkadot-node-primitives",
@@ -4673,9 +4671,7 @@ dependencies = [
"sp-runtime 31.0.1",
"sp-state-machine 0.35.0",
"sp-timestamp 26.0.0",
- "sp-tracing 16.0.0",
"sp-trie 29.0.0",
- "sp-version 29.0.0",
"substrate-prometheus-endpoint",
"tokio",
"tracing",
diff --git a/cumulus/client/consensus/aura/Cargo.toml b/cumulus/client/consensus/aura/Cargo.toml
index b390c867591..84c9de863a3 100644
--- a/cumulus/client/consensus/aura/Cargo.toml
+++ b/cumulus/client/consensus/aura/Cargo.toml
@@ -60,13 +60,9 @@ polkadot-overseer = { workspace = true, default-features = true }
polkadot-primitives = { workspace = true, default-features = true }
[dev-dependencies]
-sp-tracing = { workspace = true, default-features = true }
-cumulus-test-client = { workspace = true }
-sp-version.workspace = true
+cumulus-test-client.workspace = true
sp-keyring.workspace = true
-futures-timer.workspace = true
cumulus-test-relay-sproof-builder.workspace = true
-cumulus-test-runtime.workspace = true
[features]
# Allows collator to use full PoV size for block building
diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs
index f7eadbe6ce0..c8668da739e 100644
--- a/cumulus/client/consensus/aura/src/collators/mod.rs
+++ b/cumulus/client/consensus/aura/src/collators/mod.rs
@@ -179,9 +179,11 @@ where
let authorities = runtime_api.authorities(parent_hash).ok()?;
let author_pub = aura_internal::claim_slot::<P>(para_slot, &authorities, keystore).await?;
- // This check is necessary because we can encounter situations where the unincluded segment
- // in the runtime is full, but the included block hash we pass is not known to it. We need to
- // make sure that building on the included block is always allowed.
+ // This function is typically called when we want to build block N. At that point, the
+ // unincluded segment in the runtime is unaware of the hash of block N-1. If the unincluded
+ // segment in the runtime is full, but block N-1 is the included block, the unincluded segment
+ // should have length 0 and we can build. Since the hash is not available to the runtime
+ // however, we need this extra check here.
if parent_hash == included_block {
return Some(SlotClaim::unchecked::<P>(author_pub, para_slot, timestamp));
}
@@ -255,15 +257,13 @@ mod tests {
use codec::Encode;
use cumulus_primitives_aura::Slot;
use cumulus_primitives_core::BlockT;
- use cumulus_relay_chain_interface::{PHash, RelayChainInterface};
+ use cumulus_relay_chain_interface::PHash;
use cumulus_test_client::{
- runtime::{Block, Hash, Header},
- Backend, Client, DefaultTestClientBuilderExt, InitBlockBuilder, TestClientBuilder,
+ runtime::{Block, Hash},
+ Client, DefaultTestClientBuilderExt, InitBlockBuilder, TestClientBuilder,
TestClientBuilderExt,
};
use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder;
- use futures::{executor::block_on, FutureExt, StreamExt};
- use polkadot_node_subsystem::ChainApiBackend;
use polkadot_primitives::HeadData;
use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy};
use sp_consensus::BlockOrigin;
@@ -299,7 +299,7 @@ mod tests {
let block_builder = client.init_block_builder(None, sproof).block_builder;
- let mut block = block_builder.build().unwrap().block;
+ let block = block_builder.build().unwrap().block;
let origin = BlockOrigin::NetworkInitialSync;
import_block(client, block.clone(), origin, true).await;
@@ -326,19 +326,24 @@ mod tests {
/// we are ensuring on the node side that we are are always able to build on the included block.
#[tokio::test]
async fn test_can_build_upon() {
- sp_tracing::try_init_simple();
let (client, keystore) = set_up_components();
let genesis_hash = client.chain_info().genesis_hash;
let mut last_hash = None;
- for _ in 0..cumulus_test_runtime::UNINCLUDED_SEGMENT_CAPACITY {
+
+ // Fill up the unincluded segment tracker in the runtime.
+ for _ in 0..cumulus_test_client::runtime::UNINCLUDED_SEGMENT_CAPACITY {
let block = build_and_import_block(&client, genesis_hash).await;
last_hash = Some(block.header().hash());
}
let last_hash = last_hash.expect("must exist");
+ // We don't care much about the slots.
let para_slot = Slot::from(u64::MAX);
let relay_slot = Slot::from(u64::MAX);
+
+ // Blocks were built with the genesis hash set as included block.
+ // We call `can_build_upon` with the last built block as the included block.
let result = can_build_upon::<_, _, sp_consensus_aura::sr25519::AuthorityPair>(
para_slot,
relay_slot,
--
GitLab