From cfa078ae8a96d82cf1c17c28ceffb885cf456952 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Mon, 2 Nov 2020 13:14:17 +0100
Subject: [PATCH] Adder collator improvements (#1896)
* Fixes bug that collator wasn't sending `Declare` message
* Set authority discovery config
* Fixes bug that collator wasn't sending `Declare` message
* Adds real overseer feature and makes the wasm_validation fail with a
proper error
* Adds README
* Remove debug stuff
* Add feature
* Make adder collator use the correct parent when building a new block
---
polkadot/Cargo.lock | 1 +
polkadot/cli/src/command.rs | 1 +
polkadot/node/service/Cargo.toml | 4 +-
polkadot/node/service/src/chain_spec.rs | 2 +-
polkadot/node/service/src/client.rs | 4 +-
polkadot/node/service/src/lib.rs | 15 +++--
polkadot/node/test/service/Cargo.toml | 2 +-
polkadot/node/test/service/src/lib.rs | 7 ++-
.../test-parachains/adder/Cargo.toml | 2 +-
.../test-parachains/adder/collator/Cargo.toml | 4 ++
.../test-parachains/adder/collator/README.md | 15 +++++
.../test-parachains/adder/collator/src/lib.rs | 62 +++++++++++--------
.../adder/collator/src/main.rs | 6 ++
.../test-parachains/adder/src/lib.rs | 2 +-
.../adder/src/wasm_validation.rs | 20 +++---
15 files changed, 98 insertions(+), 49 deletions(-)
create mode 100644 polkadot/parachain/test-parachains/adder/collator/README.md
diff --git a/polkadot/Cargo.lock b/polkadot/Cargo.lock
index 6bdc6bfc167..625e9ec23a8 100644
--- a/polkadot/Cargo.lock
+++ b/polkadot/Cargo.lock
@@ -8861,6 +8861,7 @@ dependencies = [
"polkadot-parachain",
"polkadot-primitives",
"polkadot-service",
+ "sc-authority-discovery",
"sc-cli",
"sp-core",
"structopt",
diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs
index 4de5d4c228b..2177afdd7bd 100644
--- a/polkadot/cli/src/command.rs
+++ b/polkadot/cli/src/command.rs
@@ -148,6 +148,7 @@ pub fn run() -> Result<()> {
config,
service::IsCollator::No,
grandpa_pause,
+ None,
).map(|full| full.task_manager),
}
})
diff --git a/polkadot/node/service/Cargo.toml b/polkadot/node/service/Cargo.toml
index bfafd798d30..7c662ebd8b5 100644
--- a/polkadot/node/service/Cargo.toml
+++ b/polkadot/node/service/Cargo.toml
@@ -6,7 +6,7 @@ edition = "2018"
[dependencies]
# Substrate Client
-authority-discovery = { package = "sc-authority-discovery", git = "https://github.com/paritytech/substrate", branch = "master" }
+sc-authority-discovery = { git = "https://github.com/paritytech/substrate", branch = "master" }
babe = { package = "sc-consensus-babe", git = "https://github.com/paritytech/substrate", branch = "master" }
grandpa = { package = "sc-finality-grandpa", git = "https://github.com/paritytech/substrate", branch = "master" }
sc-block-builder = { git = "https://github.com/paritytech/substrate", branch = "master" }
@@ -21,7 +21,7 @@ service = { package = "sc-service", git = "https://github.com/paritytech/substra
telemetry = { package = "sc-telemetry", git = "https://github.com/paritytech/substrate", branch = "master" }
# Substrate Primitives
-authority-discovery-primitives = { package = "sp-authority-discovery", git = "https://github.com/paritytech/substrate", branch = "master" }
+sp-authority-discovery = { git = "https://github.com/paritytech/substrate", branch = "master" }
babe-primitives = { package = "sp-consensus-babe", git = "https://github.com/paritytech/substrate", branch = "master" }
consensus_common = { package = "sp-consensus", git = "https://github.com/paritytech/substrate", branch = "master" }
grandpa_primitives = { package = "sp-finality-grandpa", git = "https://github.com/paritytech/substrate", branch = "master" }
diff --git a/polkadot/node/service/src/chain_spec.rs b/polkadot/node/service/src/chain_spec.rs
index a49f455bd44..c66eddce251 100644
--- a/polkadot/node/service/src/chain_spec.rs
+++ b/polkadot/node/service/src/chain_spec.rs
@@ -16,7 +16,7 @@
//! Polkadot chain configurations.
-use authority_discovery_primitives::AuthorityId as AuthorityDiscoveryId;
+use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId;
use babe_primitives::AuthorityId as BabeId;
use grandpa::AuthorityId as GrandpaId;
use hex_literal::hex;
diff --git a/polkadot/node/service/src/client.rs b/polkadot/node/service/src/client.rs
index 3161be303dc..04ba49fb12d 100644
--- a/polkadot/node/service/src/client.rs
+++ b/polkadot/node/service/src/client.rs
@@ -40,7 +40,7 @@ pub trait RuntimeApiCollection:
+ sp_api::Metadata<Block>
+ sp_offchain::OffchainWorkerApi<Block>
+ sp_session::SessionKeys<Block>
- + authority_discovery_primitives::AuthorityDiscoveryApi<Block>
+ + sp_authority_discovery::AuthorityDiscoveryApi<Block>
where
<Self as sp_api::ApiExt<Block>>::StateBackend: sp_api::StateBackend<BlakeTwo256>,
{}
@@ -58,7 +58,7 @@ where
+ sp_api::Metadata<Block>
+ sp_offchain::OffchainWorkerApi<Block>
+ sp_session::SessionKeys<Block>
- + authority_discovery_primitives::AuthorityDiscoveryApi<Block>,
+ + sp_authority_discovery::AuthorityDiscoveryApi<Block>,
<Self as sp_api::ApiExt<Block>>::StateBackend: sp_api::StateBackend<BlakeTwo256>,
{}
diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs
index 42c36884791..6fd2b8a4323 100644
--- a/polkadot/node/service/src/lib.rs
+++ b/polkadot/node/service/src/lib.rs
@@ -33,7 +33,7 @@ use {
polkadot_node_core_proposer::ProposerFactory,
polkadot_overseer::{AllSubsystems, BlockInfo, Overseer, OverseerHandler},
polkadot_primitives::v1::ParachainHost,
- authority_discovery::Service as AuthorityDiscoveryService,
+ sc_authority_discovery::{Service as AuthorityDiscoveryService, WorkerConfig as AuthorityWorkerConfig},
sp_blockchain::HeaderBackend,
sp_core::traits::SpawnNamed,
sp_keystore::SyncCryptoStorePtr,
@@ -475,6 +475,7 @@ pub fn new_full<RuntimeApi, Executor>(
mut config: Configuration,
is_collator: IsCollator,
grandpa_pause: Option<(u32, u32)>,
+ authority_discovery_config: Option<AuthorityWorkerConfig>,
) -> Result<NewFull<Arc<FullClient<RuntimeApi, Executor>>>, Error>
where
RuntimeApi: ConstructRuntimeApi<Block, FullClient<RuntimeApi, Executor>> + Send + Sync + 'static,
@@ -570,19 +571,20 @@ pub fn new_full<RuntimeApi, Executor>(
use futures::StreamExt;
let authority_discovery_role = if role.is_authority() {
- authority_discovery::Role::PublishAndDiscover(
+ sc_authority_discovery::Role::PublishAndDiscover(
keystore_container.keystore(),
)
} else {
// don't publish our addresses when we're only a collator
- authority_discovery::Role::Discover
+ sc_authority_discovery::Role::Discover
};
let dht_event_stream = network.event_stream("authority-discovery")
.filter_map(|e| async move { match e {
Event::Dht(e) => Some(e),
_ => None,
}});
- let (worker, service) = authority_discovery::new_worker_and_service(
+ let (worker, service) = sc_authority_discovery::new_worker_and_service_with_config(
+ authority_discovery_config.unwrap_or_default(),
client.clone(),
network.clone(),
Box::pin(dht_event_stream),
@@ -900,30 +902,35 @@ pub fn build_full(
config: Configuration,
is_collator: IsCollator,
grandpa_pause: Option<(u32, u32)>,
+ authority_discovery_config: Option<AuthorityWorkerConfig>,
) -> Result<NewFull<Client>, Error> {
if config.chain_spec.is_rococo() {
new_full::<rococo_runtime::RuntimeApi, RococoExecutor>(
config,
is_collator,
grandpa_pause,
+ authority_discovery_config,
).map(|full| full.with_client(Client::Rococo))
} else if config.chain_spec.is_kusama() {
new_full::<kusama_runtime::RuntimeApi, KusamaExecutor>(
config,
is_collator,
grandpa_pause,
+ authority_discovery_config,
).map(|full| full.with_client(Client::Kusama))
} else if config.chain_spec.is_westend() {
new_full::<westend_runtime::RuntimeApi, WestendExecutor>(
config,
is_collator,
grandpa_pause,
+ authority_discovery_config,
).map(|full| full.with_client(Client::Westend))
} else {
new_full::<polkadot_runtime::RuntimeApi, PolkadotExecutor>(
config,
is_collator,
grandpa_pause,
+ authority_discovery_config,
).map(|full| full.with_client(Client::Polkadot))
}
}
diff --git a/polkadot/node/test/service/Cargo.toml b/polkadot/node/test/service/Cargo.toml
index c7da84ece60..6e8ba29648c 100644
--- a/polkadot/node/test/service/Cargo.toml
+++ b/polkadot/node/test/service/Cargo.toml
@@ -22,7 +22,7 @@ polkadot-test-runtime = { path = "../../../runtime/test-runtime" }
polkadot-runtime-parachains = { path = "../../../runtime/parachains" }
# Substrate dependencies
-authority-discovery = { package = "sc-authority-discovery", git = "https://github.com/paritytech/substrate", branch = "master" }
+sc-authority-discovery = { git = "https://github.com/paritytech/substrate", branch = "master" }
babe = { package = "sc-consensus-babe", git = "https://github.com/paritytech/substrate", branch = "master" }
babe-primitives = { package = "sp-consensus-babe", git = "https://github.com/paritytech/substrate", branch = "master" }
consensus_common = { package = "sp-consensus", git = "https://github.com/paritytech/substrate", branch = "master" }
diff --git a/polkadot/node/test/service/src/lib.rs b/polkadot/node/test/service/src/lib.rs
index 74ae6ee5117..5fed8153b86 100644
--- a/polkadot/node/test/service/src/lib.rs
+++ b/polkadot/node/test/service/src/lib.rs
@@ -49,7 +49,7 @@ use sp_blockchain::HeaderBackend;
use sp_keyring::Sr25519Keyring;
use sp_runtime::{codec::Encode, generic, traits::IdentifyAccount, MultiSigner};
use sp_state_machine::BasicExternalities;
-use std::sync::Arc;
+use std::{sync::Arc, time::Duration};
use substrate_test_client::{BlockchainEventsExt, RpcHandlersExt, RpcTransactionOutput, RpcTransactionError};
native_executor_instance!(
@@ -76,6 +76,11 @@ pub fn polkadot_test_new_full(
config,
IsCollator::No,
None,
+ Some(sc_authority_discovery::WorkerConfig {
+ query_interval: Duration::from_secs(1),
+ query_start_delay: Duration::from_secs(0),
+ ..Default::default()
+ }),
).map_err(Into::into)
}
diff --git a/polkadot/parachain/test-parachains/adder/Cargo.toml b/polkadot/parachain/test-parachains/adder/Cargo.toml
index e18ba41872f..1364ef6b2b1 100644
--- a/polkadot/parachain/test-parachains/adder/Cargo.toml
+++ b/polkadot/parachain/test-parachains/adder/Cargo.toml
@@ -14,7 +14,7 @@ tiny-keccak = "1.5.0"
dlmalloc = { version = "0.1.3", features = [ "global" ] }
# We need to make sure the global allocator is disabled until we have support of full substrate externalities
-runtime-io = { package = "sp-io", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, features = [ "disable_allocator" ] }
+sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, features = [ "disable_allocator" ] }
[build-dependencies]
wasm-builder-runner = { package = "substrate-wasm-builder-runner", version = "2.0.0" }
diff --git a/polkadot/parachain/test-parachains/adder/collator/Cargo.toml b/polkadot/parachain/test-parachains/adder/collator/Cargo.toml
index 459017b06da..882c3d72f89 100644
--- a/polkadot/parachain/test-parachains/adder/collator/Cargo.toml
+++ b/polkadot/parachain/test-parachains/adder/collator/Cargo.toml
@@ -24,6 +24,10 @@ polkadot-node-subsystem = { path = "../../../../node/subsystem" }
sc-cli = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
+sc-authority-discovery = { git = "https://github.com/paritytech/substrate", branch = "master" }
[dev-dependencies]
polkadot-parachain = { path = "../../.." }
+
+[features]
+real-overseer = [ "polkadot-service/real-overseer" ]
diff --git a/polkadot/parachain/test-parachains/adder/collator/README.md b/polkadot/parachain/test-parachains/adder/collator/README.md
new file mode 100644
index 00000000000..c4047ed82e1
--- /dev/null
+++ b/polkadot/parachain/test-parachains/adder/collator/README.md
@@ -0,0 +1,15 @@
+# How to run this collator
+
+First start two validators that will run for the relay chain:
+```sh
+cargo run --features=real-overseer --release -- -d alice --chain rococo-local --validator --alice --port 50551
+cargo run --features=real-overseer --release -- -d bob --chain rococo-local --validator --alice --port 50552
+```
+
+Next start the collator that will collate for the adder parachain:
+```sh
+cargo run --features=real-overseer --release -p test-parachain-adder-collator -- --tmp --chain rococo-local --port 50553
+```
+
+The last step is to register the parachain using polkadot-js. The parachain id is
+100. The genesis state and the validation code are printed at startup by the collator.
diff --git a/polkadot/parachain/test-parachains/adder/collator/src/lib.rs b/polkadot/parachain/test-parachains/adder/collator/src/lib.rs
index c68434ce379..c981b786af3 100644
--- a/polkadot/parachain/test-parachains/adder/collator/src/lib.rs
+++ b/polkadot/parachain/test-parachains/adder/collator/src/lib.rs
@@ -16,12 +16,12 @@
//! Collator for the adder test parachain.
-use std::{pin::Pin, sync::{Arc, Mutex}};
+use std::{pin::Pin, sync::{Arc, Mutex}, collections::HashMap};
use test_parachain_adder::{hash_state, BlockData, HeadData, execute};
use futures::{Future, FutureExt};
use polkadot_primitives::v1::{ValidationData, PoV, Hash};
use polkadot_node_primitives::Collation;
-use codec::Encode;
+use codec::{Encode, Decode};
/// The amount we add when producing a new block.
///
@@ -30,41 +30,40 @@ const ADD: u64 = 2;
/// The state of the adder parachain.
struct State {
- genesis_state: HeadData,
- last_head: HeadData,
- state: u64,
+ head_to_state: HashMap<Arc<HeadData>, u64>,
+ number_to_head: HashMap<u64, Arc<HeadData>>,
}
impl State {
/// Init the genesis state.
fn genesis() -> Self {
- let genesis_state = HeadData {
+ let genesis_state = Arc::new(HeadData {
number: 0,
parent_hash: Default::default(),
post_state: hash_state(0),
- };
- let last_head = genesis_state.clone();
+ });
Self {
- genesis_state,
- last_head,
- state: 0,
+ head_to_state: vec![(genesis_state.clone(), 0)].into_iter().collect(),
+ number_to_head: vec![(0, genesis_state)].into_iter().collect(),
}
}
- /// Advance the state and produce a new block.
+ /// Advance the state and produce a new block based on the given `parent_head`.
///
/// Returns the new [`BlockData`] and the new [`HeadData`].
- fn advance(&mut self) -> (BlockData, HeadData) {
+ fn advance(&mut self, parent_head: HeadData) -> (BlockData, HeadData) {
let block = BlockData {
- state: self.state,
+ state: *self.head_to_state.get(&parent_head).expect("Getting state using parent head"),
add: ADD,
};
- let new_head = execute(self.last_head.hash(), self.last_head.clone(), &block)
+
+ let new_head = execute(parent_head.hash(), parent_head, &block)
.expect("Produces valid block");
- self.last_head = new_head.clone();
- self.state = self.state.wrapping_add(ADD);
+ let new_head_arc = Arc::new(new_head.clone());
+ self.head_to_state.insert(new_head_arc.clone(), block.state.wrapping_add(ADD));
+ self.number_to_head.insert(new_head.number, new_head_arc);
(block, new_head)
}
@@ -85,7 +84,7 @@ impl Collator {
/// Get the SCALE encoded genesis head of the adder parachain.
pub fn genesis_head(&self) -> Vec<u8> {
- self.state.lock().unwrap().genesis_state.encode()
+ self.state.lock().unwrap().number_to_head.get(&0).expect("Genesis header exists").encode()
}
/// Get the validation code of the adder parachain.
@@ -101,8 +100,11 @@ impl Collator {
) -> Box<dyn Fn(Hash, &ValidationData) -> Pin<Box<dyn Future<Output = Option<Collation>> + Send>> + Send + Sync> {
let state = self.state.clone();
- Box::new(move |_, _| {
- let (block_data, head_data) = state.lock().unwrap().advance();
+ Box::new(move |_, validation_data| {
+ let parent = HeadData::decode(&mut &validation_data.persisted.parent_head.0[..])
+ .expect("Decodes parent head");
+
+ let (block_data, head_data) = state.lock().unwrap().advance(parent);
let collation = Collation {
upward_messages: Vec::new(),
@@ -123,6 +125,7 @@ mod tests {
use futures::executor::block_on;
use polkadot_parachain::{primitives::ValidationParams, wasm_executor::ExecutionMode};
+ use polkadot_primitives::v1::PersistedValidationData;
use codec::Decode;
#[test]
@@ -130,10 +133,19 @@ mod tests {
let collator = Collator::new();
let collation_function = collator.create_collation_function();
- for _ in 0..5 {
- let parent_head = collator.state.lock().unwrap().last_head.clone();
- let collation = block_on(collation_function(Default::default(), &Default::default())).unwrap();
- validate_collation(&collator, parent_head, collation);
+ for i in 0..5 {
+ let parent_head = collator.state.lock().unwrap().number_to_head.get(&i).unwrap().clone();
+
+ let validation_data = ValidationData {
+ persisted: PersistedValidationData {
+ parent_head: parent_head.encode().into(),
+ ..Default::default()
+ },
+ ..Default::default()
+ };
+
+ let collation = block_on(collation_function(Default::default(), &validation_data)).unwrap();
+ validate_collation(&collator, (*parent_head).clone(), collation);
}
}
@@ -152,6 +164,6 @@ mod tests {
).unwrap();
let new_head = HeadData::decode(&mut &ret.head_data.0[..]).unwrap();
- assert_eq!(collator.state.lock().unwrap().last_head, new_head);
+ assert_eq!(**collator.state.lock().unwrap().number_to_head.get(&(parent_head.number + 1)).unwrap(), new_head);
}
}
diff --git a/polkadot/parachain/test-parachains/adder/collator/src/main.rs b/polkadot/parachain/test-parachains/adder/collator/src/main.rs
index a55f5567579..1a699df7fb2 100644
--- a/polkadot/parachain/test-parachains/adder/collator/src/main.rs
+++ b/polkadot/parachain/test-parachains/adder/collator/src/main.rs
@@ -23,6 +23,7 @@ use polkadot_node_primitives::CollationGenerationConfig;
use polkadot_primitives::v1::{CollatorPair, Id as ParaId};
use test_parachain_adder_collator::Collator;
use sp_core::{Pair, hexdisplay::HexDisplay};
+use std::time::Duration;
const PARA_ID: ParaId = ParaId::new(100);
@@ -47,6 +48,11 @@ fn main() -> Result<()> {
config,
polkadot_service::IsCollator::Yes(collator_key.public()),
None,
+ Some(sc_authority_discovery::WorkerConfig {
+ query_interval: Duration::from_secs(1),
+ query_start_delay: Duration::from_secs(0),
+ ..Default::default()
+ }),
)?;
let mut overseer_handler = full_node.overseer_handler
.expect("Overseer handler should be initialized for collators");
diff --git a/polkadot/parachain/test-parachains/adder/src/lib.rs b/polkadot/parachain/test-parachains/adder/src/lib.rs
index 2e3ab895e2b..7cec2b82372 100644
--- a/polkadot/parachain/test-parachains/adder/src/lib.rs
+++ b/polkadot/parachain/test-parachains/adder/src/lib.rs
@@ -58,7 +58,7 @@ impl HeadData {
}
/// Block data for this parachain.
-#[derive(Default, Clone, Encode, Decode)]
+#[derive(Default, Clone, Encode, Decode, Debug)]
pub struct BlockData {
/// State to begin from.
pub state: u64,
diff --git a/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs b/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs
index c0f3b56dc8e..f009b5530db 100644
--- a/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs
+++ b/polkadot/parachain/test-parachains/adder/src/wasm_validation.rs
@@ -32,15 +32,13 @@ pub extern fn validate_block(params: *const u8, len: usize) -> u64 {
let parent_hash = tiny_keccak::keccak256(¶ms.parent_head.0[..]);
- match crate::execute(parent_hash, parent_head, &block_data) {
- Ok(new_head) => parachain::write_result(
- &ValidationResult {
- head_data: GenericHeadData(new_head.encode()),
- new_validation_code: None,
- upward_messages: sp_std::vec::Vec::new(),
- processed_downward_messages: 0,
- }
- ),
- Err(_) => panic!("execution failure"),
- }
+ let new_head = crate::execute(parent_hash, parent_head, &block_data).expect("Executes block");
+ parachain::write_result(
+ &ValidationResult {
+ head_data: GenericHeadData(new_head.encode()),
+ new_validation_code: None,
+ upward_messages: sp_std::vec::Vec::new(),
+ processed_downward_messages: 0,
+ }
+ )
}
--
GitLab