From 430e3920e5c045eaeccc0a789a160458a499f028 Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Wed, 22 Aug 2018 10:57:12 +0200
Subject: [PATCH] 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
---
polkadot/api/Cargo.toml | 1 +
polkadot/api/src/full.rs | 37 ++++++++++++++++-------
polkadot/api/src/lib.rs | 3 ++
polkadot/consensus/src/service.rs | 50 +++++++++++++------------------
4 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/polkadot/api/Cargo.toml b/polkadot/api/Cargo.toml
index 7b5015b9913..435e27c146b 100644
--- a/polkadot/api/Cargo.toml
+++ b/polkadot/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" }
diff --git a/polkadot/api/src/full.rs b/polkadot/api/src/full.rs
index ca17188eacf..59b7304e81a 100644
--- a/polkadot/api/src/full.rs
+++ b/polkadot/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]
diff --git a/polkadot/api/src/lib.rs b/polkadot/api/src/lib.rs
index 8c1e4b1de83..1265faf961f 100644
--- a/polkadot/api/src/lib.rs
+++ b/polkadot/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;
diff --git a/polkadot/consensus/src/service.rs b/polkadot/consensus/src/service.rs
index d7360bb45a3..7dc252e5c8c 100644
--- a/polkadot/consensus/src/service.rs
+++ b/polkadot/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;
--
GitLab