Skip to content

Commit

Permalink
Wait for block import in parachain consensus (paritytech#271)
Browse files Browse the repository at this point in the history
* Wait for block import in parachain consensus

There was a bug in the parachain consensus that when importing a relay
chain block that sets a new best parachain block, but the required
parachain block was not yet imported. This pr fixes this by waiting for
the block to be imported.

* Finish docs
  • Loading branch information
bkchr authored Jan 5, 2021
1 parent 518c6fa commit 9f0085c
Show file tree
Hide file tree
Showing 14 changed files with 809 additions and 313 deletions.
10 changes: 6 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions collator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ edition = "2018"

[dependencies]
# Substrate dependencies
sp-blockchain = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand All @@ -16,7 +15,6 @@ sp-state-machine = { git = "https://github.com/paritytech/substrate", branch = "
sp-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-client-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-cli = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-block-builder = { git = "https://github.com/paritytech/substrate", branch = "master" }

# Polkadot dependencies
polkadot-service = { git = "https://github.com/paritytech/polkadot", features = [ "real-overseer" ] , branch = "master" }
Expand All @@ -28,7 +26,6 @@ polkadot-overseer = { git = "https://github.com/paritytech/polkadot", branch = "
polkadot-node-subsystem = { git = "https://github.com/paritytech/polkadot", branch = "master" }

# Cumulus dependencies
cumulus-consensus = { path = "../consensus" }
cumulus-network = { path = "../network" }
cumulus-primitives = { path = "../primitives" }
cumulus-runtime = { path = "../runtime" }
Expand Down
49 changes: 8 additions & 41 deletions collator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ use cumulus_primitives::{
};
use cumulus_runtime::ParachainBlockData;

use sc_client_api::{BlockBackend, Finalizer, StateBackend, UsageProvider};
use sp_blockchain::HeaderBackend;
use sc_client_api::{BlockBackend, StateBackend};
use sp_consensus::{
BlockImport, BlockImportParams, BlockOrigin, BlockStatus, Environment, Error as ConsensusError,
ForkChoiceStrategy, Proposal, Proposer, RecordProof,
Expand Down Expand Up @@ -522,13 +521,12 @@ where
}

/// Parameters for [`start_collator`].
pub struct StartCollatorParams<Block: BlockT, PF, BI, Backend, Client, BS, Spawner, PClient, PBackend> {
pub struct StartCollatorParams<Block: BlockT, PF, BI, Backend, BS, Spawner, PClient, PBackend> {
pub proposer_factory: PF,
pub inherent_data_providers: InherentDataProviders,
pub backend: Arc<Backend>,
pub block_import: BI,
pub block_status: Arc<BS>,
pub client: Arc<Client>,
pub announce_block: Arc<dyn Fn(Block::Hash, Vec<u8>) + Send + Sync>,
pub overseer_handler: OverseerHandler,
pub spawner: Spawner,
Expand All @@ -543,7 +541,6 @@ pub async fn start_collator<
PF,
BI,
Backend,
Client,
BS,
Spawner,
PClient,
Expand All @@ -557,15 +554,14 @@ pub async fn start_collator<
backend,
block_import,
block_status,
client,
announce_block,
mut overseer_handler,
spawner,
para_id,
key,
polkadot_client,
polkadot_backend,
}: StartCollatorParams<Block, PF, BI, Backend, Client, BS, Spawner, PClient, PBackend2>,
}: StartCollatorParams<Block, PF, BI, Backend, BS, Spawner, PClient, PBackend2>,
) -> Result<(), String>
where
PF: Environment<Block> + Send + 'static,
Expand All @@ -574,14 +570,6 @@ where
+ Sync
+ 'static,
Backend: sc_client_api::Backend<Block> + 'static,
Client: Finalizer<Block, Backend>
+ UsageProvider<Block>
+ HeaderBackend<Block>
+ Send
+ Sync
+ BlockBackend<Block>
+ 'static,
for<'a> &'a Client: BlockImport<Block>,
BS: BlockBackend<Block> + Send + Sync + 'static,
Spawner: SpawnNamed + Clone + Send + Sync + 'static,
PBackend: sc_client_api::Backend<PBlock> + 'static,
Expand All @@ -591,18 +579,6 @@ where
PBackend2: sc_client_api::Backend<PBlock> + 'static,
PBackend2::State: StateBackend<BlakeTwo256>,
{
let follow = match cumulus_consensus::follow_polkadot(
para_id,
client,
polkadot_client.clone(),
announce_block.clone(),
) {
Ok(follow) => follow,
Err(e) => return Err(format!("Could not start following polkadot: {:?}", e)),
};

spawner.spawn("cumulus-follow-polkadot", follow.map(|_| ()).boxed());

let collator = Collator::new(
para_id,
proposer_factory,
Expand Down Expand Up @@ -644,13 +620,12 @@ mod tests {
use super::*;
use std::{pin::Pin, time::Duration};

use sc_block_builder::BlockBuilderProvider;
use sp_core::{testing::TaskExecutor, Pair};
use sp_inherents::InherentData;
use sp_runtime::traits::DigestFor;

use cumulus_test_client::{
generate_block_inherents, Client, DefaultTestClientBuilderExt, TestClientBuilder,
Client, DefaultTestClientBuilderExt, InitBlockBuilder, TestClientBuilder,
TestClientBuilderExt,
};
use cumulus_test_runtime::{Block, Header};
Expand Down Expand Up @@ -700,19 +675,12 @@ mod tests {
fn propose(
self,
_: InherentData,
digest: DigestFor<Block>,
_: DigestFor<Block>,
_: Duration,
record_proof: RecordProof,
_: RecordProof,
) -> Self::Proposal {
let block_id = BlockId::Hash(self.header.hash());
let mut builder = self
.client
.new_block_at(&block_id, digest, record_proof.yes())
.expect("Initializes new block");

generate_block_inherents(&*self.client, None)
.into_iter()
.for_each(|e| builder.push(e).expect("Pushes an inherent"));
let builder = self.client.init_block_builder_at(&block_id, None);

let (block, storage_changes, proof) =
builder.build().expect("Creates block").into_inner();
Expand Down Expand Up @@ -766,14 +734,13 @@ mod tests {
};

let collator_start =
start_collator::<_, _, _, _, _, _, _, _, polkadot_service::FullBackend, _, _>(
start_collator::<_, _, _, _, _, _, _, polkadot_service::FullBackend, _, _>(
StartCollatorParams {
proposer_factory: DummyFactory(client.clone()),
inherent_data_providers: Default::default(),
backend,
block_import: client.clone(),
block_status: client.clone(),
client: client.clone(),
announce_block: Arc::new(announce_block),
overseer_handler: handler,
spawner,
Expand Down
21 changes: 16 additions & 5 deletions consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ authors = ["Parity Technologies <[email protected]>"]
edition = "2018"

[dependencies]
# substrate deps
# Substrate deps
sc-client-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-consensus = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand All @@ -17,12 +17,23 @@ sp-blockchain = { git = "https://github.com/paritytech/substrate", branch = "mas
sp-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
substrate-prometheus-endpoint = { git = "https://github.com/paritytech/substrate", branch = "master" }

# polkadot deps
# Polkadot deps
polkadot-primitives = { git = "https://github.com/paritytech/polkadot", branch = "master" }
polkadot-runtime = { git = "https://github.com/paritytech/polkadot", branch = "master" }

# other deps
futures = { version = "0.3.1", features = ["compat"] }
# Other deps
futures = { version = "0.3.8", features = ["compat"] }
tokio = "0.1.22"
codec = { package = "parity-scale-codec", version = "1.3.0", features = [ "derive" ] }
log = "0.4"
tracing = "0.1.22"

[dev-dependencies]
# Substrate deps
sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }

# Cumulus dependencies
cumulus-test-runtime = { path = "../test/runtime" }
cumulus-test-client = { path = "../test/client" }

# Other deps
futures-timer = "3.0.2"
Loading

0 comments on commit 9f0085c

Please sign in to comment.