Rollup of various testnet-related fixes.

fix a deadlock when spawning agreement as non-authority

fix test compilation for BFT

more accurate consensus superseding logic

mild revision to `can_build_on` logic

block evaluation without redundant initialisation

refactor BFT delay: update rhododendron and poll after delaying. (#589)

dropping BFT future before poll doesn't lead to service deadlock

api/Cargo.toml

@@ -16,6 +16,7 @@ substrate-client = { path = "../../substrate/client" }
 substrate-primitives = { path = "../../substrate/primitives" }
 substrate-executor = { path = "../../substrate/executor" }
 substrate-state-machine = { path = "../../substrate/state-machine" }
+log = "0.3"
 
 [dev-dependencies]
 substrate-keyring = { path = "../../substrate/keyring" }

api/src/full.rs

@@ -96,18 +96,33 @@ impl<B: LocalBackend<Block>> PolkadotApi for Client<B, LocalCallExecutor<B, Nati
 
 	fn evaluate_block(&self, at: &BlockId, block: Block) -> Result<bool> {
 		use substrate_executor::error::ErrorKind as ExecErrorKind;
-		use codec::{Decode, Encode};
-		use runtime::Block as RuntimeBlock;
+		use codec::Encode;
+		use state_machine::ExecutionManager;
+		use client::CallExecutor;
+
+		let parent = at;
+		let res = self.state_at(&parent).map_err(Error::from).and_then(|state| {
+			let mut overlay = Default::default();
+			let execution_manager = || ExecutionManager::Both(|wasm_result, native_result| {
+				warn!("Consensus error between wasm and native runtime execution at block {:?}", at);
+				warn!("   While executing block {:?}", (block.header.number, block.header.hash()));
+				warn!("   Native result {:?}", native_result);
+				warn!("   Wasm result {:?}", wasm_result);
+				wasm_result
+			});
+			let (r, _) = self.executor().call_at_state(
+				&state,
+				&mut overlay,
+				"execute_block",
+				&block.encode(),
+				execution_manager()
+			)?;
+
+			Ok(r)
+		});
 
-		let encoded = block.encode();
-		let runtime_block = match RuntimeBlock::decode(&mut &encoded[..]) {
-			Some(x) => x,
-			None => return Ok(false),
-		};
-
-		let res = with_runtime!(self, at, || ::runtime::Executive::execute_block(runtime_block));
 		match res {
-			Ok(()) => Ok(true),
+			Ok(_) => Ok(true),
 			Err(err) => match err.kind() {
 				&ErrorKind::Executor(ExecErrorKind::Runtime) => Ok(false),
 				_ => Err(err)
@@ -230,6 +245,7 @@ mod tests {
 
 		assert_eq!(block.header.number, 1);
 		assert!(block.header.extrinsics_root != Default::default());
+		assert!(client.evaluate_block(&id, block).unwrap());
 	}
 
 	#[test]
@@ -252,6 +268,7 @@ mod tests {
 
 		assert_eq!(block.header.number, 1);
 		assert!(block.header.extrinsics_root != Default::default());
+		assert!(client.evaluate_block(&id, block).unwrap());
 	}
 
 	#[test]

api/src/lib.rs

@@ -32,6 +32,9 @@ extern crate substrate_state_machine as state_machine;
 #[macro_use]
 extern crate error_chain;
 
+#[macro_use]
+extern crate log;
+
 #[cfg(test)]
 extern crate substrate_keyring as keyring;
 

consensus/src/service.rs

@@ -61,32 +61,26 @@ fn start_bft<F, C>(
 	const DELAY_UNTIL: Duration = Duration::from_millis(5000);
 
 	let mut handle = LocalThreadHandle::current();
-	let work = Delay::new(Instant::now() + DELAY_UNTIL)
-		.then(move |res| {
-			if let Err(e) = res {
-				warn!(target: "bft", "Failed to force delay of consensus: {:?}", e);
-			}
+	match bft_service.build_upon(&header) {
+		Ok(Some(bft_work)) => {
+			// do not poll work for some amount of time.
+			let work = Delay::new(Instant::now() + DELAY_UNTIL).then(move |res| {
+				if let Err(e) = res {
+					warn!(target: "bft", "Failed to force delay of consensus: {:?}", e);
+				}
 
-			match bft_service.build_upon(&header) {
-				Ok(maybe_bft_work) => {
-					if maybe_bft_work.is_some() {
-						debug!(target: "bft", "Starting agreement. After forced delay for {:?}",
-							DELAY_UNTIL);
-					}
+				debug!(target: "bft", "Starting agreement. After forced delay for {:?}",
+					DELAY_UNTIL);
 
-					maybe_bft_work
-				}
-				Err(e) => {
-					warn!(target: "bft", "BFT agreement error: {}", e);
-					None
-				}
+				bft_work
+			});
+			if let Err(e) = handle.spawn_local(Box::new(work)) {
+		    	warn!(target: "bft", "Couldn't initialize BFT agreement: {:?}", e);
 			}
-		})
-		.map(|_| ());
-
-	if let Err(e) = handle.spawn_local(Box::new(work)) {
-    	debug!(target: "bft", "Couldn't initialize BFT agreement: {:?}", e);
-    }
+		}
+		Ok(None) => trace!(target: "bft", "Could not start agreement on top of {}", header.hash()),
+		Err(e) => warn!(target: "bft", "BFT agreement error: {}", e),
+	}
 }
 
 /// Consensus service. Starts working when created.
@@ -138,6 +132,7 @@ impl Service {
 
 				client.import_notification_stream().for_each(move |notification| {
 					if notification.is_new_best {
+						trace!(target: "bft", "Attempting to start new consensus round after import notification of {:?}", notification.hash);
 						start_bft(notification.header, bft_service.clone());
 					}
 					Ok(())
@@ -161,15 +156,12 @@ impl Service {
 				let c = client.clone();
 				let s = bft_service.clone();
 
-				interval.map_err(|e| debug!("Timer error: {:?}", e)).for_each(move |_| {
+				interval.map_err(|e| debug!(target: "bft", "Timer error: {:?}", e)).for_each(move |_| {
 					if let Ok(best_block) = c.best_block_header() {
 						let hash = best_block.hash();
-						let last_agreement = s.last_agreement();
-						let can_build_upon = last_agreement
-							.map_or(true, |x| !x.live || x.parent_hash != hash);
 
-						if hash == prev_best && can_build_upon {
-							debug!("Starting consensus round after a timeout");
+						if hash == prev_best {
+							debug!(target: "bft", "Starting consensus round after a timeout");
 							start_bft(best_block, s.clone());
 						}
 						prev_best = hash;