Merge remote-tracking branch 'origin/master' into v0.9

README.adoc

@@ -151,7 +151,7 @@ Now with this new chainspec file, you can build a "raw" chain definition for you
 
 [source, shell]
 ----
-substrate build-spec --chain ~/chainspec.json --raw > ~/mychain.json
+substrate --chain ~/chainspec.json build-spec --raw > ~/mychain.json
 ----
 
 This can be fed into Substrate:

core/network/src/sync.rs

@@ -282,8 +282,11 @@ impl<B: BlockT> ChainSync<B> {
 			self.best_queued_hash = *hash;
 		}
 		// Update common blocks
-		for (_, peer) in self.peers.iter_mut() {
-			trace!(target: "sync", "Updating peer info ours={}, theirs={}", number, peer.best_number);
+		for (n, peer) in self.peers.iter_mut() {
+			if let PeerSyncState::AncestorSearch(_) = peer.state {
+				continue;
+			}
+			trace!(target: "sync", "Updating peer {} info, ours={}, common={}, their best={}", n, number, peer.common_number, peer.best_number);
 			if peer.best_number >= number {
 				peer.common_number = number;
 				peer.common_hash = *hash;
@@ -301,22 +304,34 @@ impl<B: BlockT> ChainSync<B> {
 
 	pub(crate) fn on_block_announce(&mut self, protocol: &mut Context<B>, who: NodeIndex, hash: B::Hash, header: &B::Header) {
 		let number = *header.number();
+		if number <= As::sa(0) {
+			trace!(target: "sync", "Ignored invalid block announcement from {}: {}", who, hash);
+			return;
+		}
+		let known_parent = self.is_known(protocol, &header.parent_hash());
+		let known = self.is_known(protocol, &hash);
 		if let Some(ref mut peer) = self.peers.get_mut(&who) {
 			if number > peer.best_number {
+				// update their best block
 				peer.best_number = number;
 				peer.best_hash = hash;
 			}
-			if number <= self.best_queued_number && number > peer.common_number {
+			if let PeerSyncState::AncestorSearch(_) = peer.state {
+				return;
+			}
+			if header.parent_hash() == &self.best_queued_hash || known_parent {
+				peer.common_number = number - As::sa(1);
+			} else if known {
 				peer.common_number = number
 			}
 		} else {
 			return;
 		}
 
-		if !self.is_known_or_already_downloading(protocol, &hash) {
+		if !(known || self.is_already_downloading(&hash)) {
 			let stale = number <= self.best_queued_number;
 			if stale {
-				if !self.is_known_or_already_downloading(protocol, header.parent_hash()) {
+				if !(known_parent || self.is_already_downloading(header.parent_hash())) {
 					trace!(target: "sync", "Ignoring unknown stale block announce from {}: {} {:?}", who, hash, header);
 				} else {
 					trace!(target: "sync", "Considering new stale block announced from {}: {} {:?}", who, hash, header);
@@ -331,9 +346,12 @@ impl<B: BlockT> ChainSync<B> {
 		}
 	}
 
-	fn is_known_or_already_downloading(&self, protocol: &mut Context<B>, hash: &B::Hash) -> bool {
+	fn is_already_downloading(&self, hash: &B::Hash) -> bool {
 		self.peers.iter().any(|(_, p)| p.state == PeerSyncState::DownloadingStale(*hash))
-			|| block_status(&*protocol.client(), &*self.import_queue, *hash).ok().map_or(false, |s| s != BlockStatus::Unknown)
+	}
+
+	fn is_known(&self, protocol: &mut Context<B>, hash: &B::Hash) -> bool {
+		block_status(&*protocol.client(), &*self.import_queue, *hash).ok().map_or(false, |s| s != BlockStatus::Unknown)
 	}
 
 	pub(crate) fn peer_disconnected(&mut self, protocol: &mut Context<B>, who: NodeIndex) {

core/service/Cargo.toml

@@ -30,3 +30,6 @@ substrate-executor = { path = "../../core/executor" }
 substrate-transaction-pool = { path = "../../core/transaction-pool" }
 substrate-rpc-servers = { path = "../../core/rpc-servers" }
 substrate-telemetry = { path = "../../core/telemetry" }
+
+[dev-dependencies]
+substrate-test-client = { path = "../test-client" }

core/service/src/components.rs

@@ -184,6 +184,51 @@ pub trait MaintainTransactionPool<C: Components> {
 	) -> error::Result<()>;
 }
 
+fn on_block_imported<Api, Backend, Block, Executor, PoolApi>(
+	id: &BlockId<Block>,
+	client: &Client<Backend, Executor, Block, Api>,
+	transaction_pool: &TransactionPool<PoolApi>,
+) -> error::Result<()> where
+	Api: TaggedTransactionQueue<Block>,
+	Block: BlockT<Hash = <Blake2Hasher as ::primitives::Hasher>::Out>,
+	Backend: client::backend::Backend<Block, Blake2Hasher>,
+	Client<Backend, Executor, Block, Api>: ProvideRuntimeApi<Api = Api>,
+	Executor: client::CallExecutor<Block, Blake2Hasher>,
+	PoolApi: txpool::ChainApi<Hash = Block::Hash, Block = Block>,
+{
+	use runtime_primitives::transaction_validity::TransactionValidity;
+
+	// Avoid calling into runtime if there is nothing to prune from the pool anyway.
+	if transaction_pool.status().is_empty() {
+		return Ok(())
+	}
+
+	let block = client.block(id)?;
+	let tags = match block {
+		None => return Ok(()),
+		Some(block) => {
+			let parent_id = BlockId::hash(*block.block.header().parent_hash());
+			let mut tags = vec![];
+			for tx in block.block.extrinsics() {
+				let tx = client.runtime_api().validate_transaction(&parent_id, &tx)?;
+				match tx {
+					TransactionValidity::Valid { mut provides, .. } => {
+						tags.append(&mut provides);
+					},
+					// silently ignore invalid extrinsics,
+					// cause they might just be inherent
+					_ => {}
+				}
+
+			}
+			tags
+		}
+	};
+
+	transaction_pool.prune_tags(id, tags).map_err(|e| format!("{:?}", e))?;
+	Ok(())
+}
+
 impl<C: Components> MaintainTransactionPool<Self> for C where
 	ComponentClient<C>: ProvideRuntimeApi<Api = C::RuntimeApi>,
 	C::RuntimeApi: TaggedTransactionQueue<ComponentBlock<C>>,
@@ -194,35 +239,10 @@ impl<C: Components> MaintainTransactionPool<Self> for C where
 		client: &ComponentClient<C>,
 		transaction_pool: &TransactionPool<C::TransactionPoolApi>,
 	) -> error::Result<()> {
-		use runtime_primitives::transaction_validity::TransactionValidity;
-
-		let block = client.block(id)?;
-		let tags = match block {
-			None => return Ok(()),
-			Some(block) => {
-				let mut tags = vec![];
-				for tx in block.block.extrinsics() {
-					let tx = client.runtime_api().validate_transaction(id, &tx)?;
-					match tx {
-						TransactionValidity::Valid { mut provides, .. } => {
-							tags.append(&mut provides);
-						},
-						// silently ignore invalid extrinsics,
-						// cause they might just be inherent
-						_ => {}
-					}
-
-				}
-				tags
-			}
-		};
-
-		transaction_pool.prune_tags(id, tags).map_err(|e| format!("{:?}", e))?;
-		Ok(())
+		on_block_imported(id, client, transaction_pool)
 	}
 }
 
-
 /// The super trait that combines all required traits a `Service` needs to implement.
 pub trait ServiceTrait<C: Components>:
 	Deref<Target = Service<C>>
@@ -516,3 +536,53 @@ impl<Factory: ServiceFactory> Components for LightComponents<Factory> {
 		Factory::build_light_import_queue(config, client)
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use codec::Encode;
+	use consensus_common::BlockOrigin;
+	use substrate_test_client::{
+		self,
+		TestClient,
+		keyring::Keyring,
+		runtime::{Extrinsic, Transfer},
+	};
+
+	#[test]
+	fn should_remove_transactions_from_the_pool() {
+		let client = Arc::new(substrate_test_client::new());
+		let pool = TransactionPool::new(Default::default(), ::transaction_pool::ChainApi::new(client.clone()));
+		let transaction = {
+			let transfer = Transfer {
+				amount: 5,
+				nonce: 0,
+				from: Keyring::Alice.to_raw_public().into(),
+				to: Default::default(),
+			};
+			let signature = Keyring::from_raw_public(transfer.from.to_fixed_bytes()).unwrap().sign(&transfer.encode()).into();
+			Extrinsic { transfer, signature }
+		};
+		// store the transaction in the pool
+		pool.submit_one(&BlockId::hash(client.best_block_header().unwrap().hash()), transaction.clone()).unwrap();
+
+		// import the block
+		let mut builder = client.new_block().unwrap();
+		builder.push(transaction.clone()).unwrap();
+		let block = builder.bake().unwrap();
+		let id = BlockId::hash(block.header().hash());
+		client.import(BlockOrigin::Own, block).unwrap();
+
+		// fire notification - this should clean up the queue
+		assert_eq!(pool.status().ready, 1);
+		on_block_imported(
+			&id,
+			&client,
+			&pool,
+		).unwrap();
+
+		// then
+		assert_eq!(pool.status().ready, 0);
+		assert_eq!(pool.status().future, 0);
+	}
+}

core/service/src/lib.rs

@@ -50,6 +50,9 @@ extern crate log;
 #[macro_use]
 extern crate serde_derive;
 
+#[cfg(test)]
+extern crate substrate_test_client;
+
 mod components;
 mod error;
 mod chain_spec;

core/test-runtime/Cargo.toml

@@ -11,7 +11,7 @@ serde_derive = { version = "1.0", optional = true }
 parity-codec = { version = "2.1", default-features = false }
 parity-codec-derive = { version = "2.1", default-features = false }
 substrate-keyring = { path = "../keyring", optional = true }
-substrate-client = { path = "../client", optional = true }
+substrate-client = { path = "../client", default-features = false }
 substrate-primitives = { path = "../primitives", default-features = false }
 substrate-consensus-aura-primitives = { path = "../consensus/aura/primitives", default-features = false }
 sr-std = { path = "../sr-std", default-features = false }
@@ -27,7 +27,7 @@ std = [
 	"hex-literal",
 	"serde",
 	"serde_derive",
-	"substrate-client",
+	"substrate-client/std",
 	"substrate-keyring",
 	"parity-codec/std",
 	"sr-std/std",