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

@@ -8861,6 +8861,7 @@ dependencies = [
  "polkadot-parachain",
  "polkadot-primitives",
  "polkadot-service",
+ "sc-authority-discovery",
  "sc-cli",
  "sp-core",
  "structopt",

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),
 				}
 			})

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" }

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;

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>,
 {}
 

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))
 	}
 }

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" }

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)
 }
 

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" }

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" ]

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.

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,
+	head_to_state: HashMap<Arc<HeadData>, u64>,
-	last_head: HeadData,
+	number_to_head: HashMap<u64, Arc<HeadData>>,
-	state: u64,
 }
 
 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,
+			head_to_state: vec![(genesis_state.clone(), 0)].into_iter().collect(),
-			last_head,
+			number_to_head: vec![(0, genesis_state)].into_iter().collect(),
-			state: 0,
 		}
 	}
 
-	/// 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();
+		let new_head_arc = Arc::new(new_head.clone());
-		self.state = self.state.wrapping_add(ADD);
+		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 |_, _| {
+		Box::new(move |_, validation_data| {
-			let (block_data, head_data) = state.lock().unwrap().advance();
+			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 {
+		for i in 0..5 {
-			let parent_head = collator.state.lock().unwrap().last_head.clone();
+			let parent_head = collator.state.lock().unwrap().number_to_head.get(&i).unwrap().clone();
-			let collation = block_on(collation_function(Default::default(), &Default::default())).unwrap();
+
-			validate_collation(&collator, parent_head, collation);
+			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);
 	}
 }

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");

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,

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(&params.parent_head.0[..]);
 
-	match crate::execute(parent_hash, parent_head, &block_data) {
+	let new_head = crate::execute(parent_hash, parent_head, &block_data).expect("Executes block");
-		Ok(new_head) => parachain::write_result(
+	parachain::write_result(
-			&ValidationResult {
+		&ValidationResult {
-				head_data: GenericHeadData(new_head.encode()),
+			head_data: GenericHeadData(new_head.encode()),
-				new_validation_code: None,
+			new_validation_code: None,
-				upward_messages: sp_std::vec::Vec::new(),
+			upward_messages: sp_std::vec::Vec::new(),
-				processed_downward_messages: 0,
+			processed_downward_messages: 0,
-			}
+		}
-		),
+	)
-		Err(_) => panic!("execution failure"),
-	}
 }