Fix and cleanups (#314)

* Cleanups (remove genesis.wasm & nicer errors)

- Pretty errors for version mismatch
- Remove the need for genesis wasm

* Remove unneeded wasm files

* Improve code of conduct

* Leaner code

* Test fixes

* fix tests

* Fix consensus checking

substrate/CODE_OF_CONDUCT.md

@@ -22,6 +22,13 @@ Examples of unacceptable behavior by participants include:
 * Publishing others' private information, such as a physical or electronic address, without explicit permission
 * Other conduct which could reasonably be considered inappropriate in a professional setting
 
+### Facilitation, Not Strongarming
+
+We recognise that this software is merely a tool for users to create and maintain their blockchain of preference. We see that blockchains are naturally community platforms with users being the ultimate decision makers. We assert that good software will maximise user agency by facilitate user-expression on the network. As such:
+
+* This project will strive to give users as much choice as is both reasonable and possible over what protocol they adhere to; but
+* use of the project's technical forums, commenting systems, pull requests and issue trackers as a means to express individual protocol preferences is forbidden.
+
 ## Our Responsibilities
 
 Project maintainers are responsible for clarifying the standards of acceptable behavior and are expected to take appropriate and fair corrective action in response to any instances of unacceptable behavior.

substrate/Cargo.lock

@@ -2125,6 +2125,7 @@ dependencies = [
  "substrate-primitives 0.1.0",
  "substrate-runtime-primitives 0.1.0",
  "substrate-runtime-support 0.1.0",
+ "substrate-runtime-version 0.1.0",
  "tokio 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

substrate/demo/cli/src/lib.rs

@@ -107,7 +107,7 @@ pub fn run<I, T>(args: I) -> error::Result<()> where
 	init_logger(log_pattern);
 
 	// Create client
-	let executor = demo_executor::Executor::new();
+	let executor = demo_executor::Executor::with_heap_pages(8);
 
 	let god_key = hex!["3d866ec8a9190c8343c2fc593d21d8a6d0c5c4763aaab2349de3a6111d64d124"];
 	let genesis_config = GenesisConfig {

substrate/demo/executor/src/lib.rs

@@ -93,6 +93,10 @@ mod tests {
 		Header::new(n, Default::default(), Default::default(), [69; 32].into(), Default::default())
 	}
 
+	fn executor() -> ::substrate_executor::NativeExecutor<Executor> {
+		Executor::with_heap_pages(8)
+	}
+
 	#[test]
 	fn panic_execution_with_foreign_code_gives_error() {
 		let mut t: TestExternalities = map![
@@ -105,9 +109,9 @@ mod tests {
 			twox_128(&<system::BlockHash<Concrete>>::key_for(0)).to_vec() => vec![0u8; 32]
 		];
 
-		let r = Executor::new().call(&mut t, BLOATY_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
+		let r = executor().call(&mut t, BLOATY_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
 		assert!(r.is_ok());
-		let v = Executor::new().call(&mut t, BLOATY_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0.unwrap();
+		let v = executor().call(&mut t, BLOATY_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0.unwrap();
 		let r = ApplyResult::decode(&mut &v[..]).unwrap();
 		assert_eq!(r, Err(ApplyError::CantPay));
 	}
@@ -124,9 +128,9 @@ mod tests {
 			twox_128(&<system::BlockHash<Concrete>>::key_for(0)).to_vec() => vec![0u8; 32]
 		];
 
-		let r = Executor::new().call(&mut t, COMPACT_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
+		let r = executor().call(&mut t, COMPACT_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
 		assert!(r.is_ok());
-		let v = Executor::new().call(&mut t, COMPACT_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0.unwrap();
+		let v = executor().call(&mut t, COMPACT_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0.unwrap();
 		let r = ApplyResult::decode(&mut &v[..]).unwrap();
 		assert_eq!(r, Err(ApplyError::CantPay));
 	}
@@ -143,9 +147,9 @@ mod tests {
 			twox_128(&<system::BlockHash<Concrete>>::key_for(0)).to_vec() => vec![0u8; 32]
 		];
 
-		let r = Executor::new().call(&mut t, COMPACT_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
+		let r = executor().call(&mut t, COMPACT_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
 		assert!(r.is_ok());
-		let r = Executor::new().call(&mut t, COMPACT_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0;
+		let r = executor().call(&mut t, COMPACT_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0;
 		assert!(r.is_ok());
 
 		runtime_io::with_externalities(&mut t, || {
@@ -166,9 +170,9 @@ mod tests {
 			twox_128(&<system::BlockHash<Concrete>>::key_for(0)).to_vec() => vec![0u8; 32]
 		];
 
-		let r = Executor::new().call(&mut t, BLOATY_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
+		let r = executor().call(&mut t, BLOATY_CODE, "initialise_block", &vec![].and(&from_block_number(1u64)), true).0;
 		assert!(r.is_ok());
-		let r = Executor::new().call(&mut t, BLOATY_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0;
+		let r = executor().call(&mut t, BLOATY_CODE, "apply_extrinsic", &vec![].and(&xt()), true).0;
 		assert!(r.is_ok());
 
 		runtime_io::with_externalities(&mut t, || {
@@ -289,14 +293,14 @@ mod tests {
 	fn full_native_block_import_works() {
 		let mut t = new_test_ext();
 
-		Executor::new().call(&mut t, COMPACT_CODE, "execute_block", &block1().0, true).0.unwrap();
+		executor().call(&mut t, COMPACT_CODE, "execute_block", &block1().0, true).0.unwrap();
 
 		runtime_io::with_externalities(&mut t, || {
 			assert_eq!(Staking::voting_balance(&alice()), 41);
 			assert_eq!(Staking::voting_balance(&bob()), 69);
 		});
 
-		Executor::new().call(&mut t, COMPACT_CODE, "execute_block", &block2().0, true).0.unwrap();
+		executor().call(&mut t, COMPACT_CODE, "execute_block", &block2().0, true).0.unwrap();
 
 		runtime_io::with_externalities(&mut t, || {
 			assert_eq!(Staking::voting_balance(&alice()), 30);

substrate/polkadot/api/src/full.rs

@@ -200,7 +200,7 @@ mod tests {
 			timestamp: Some(Default::default()),
 		};
 
-		::client::new_in_mem(LocalDispatch::new(), genesis_config).unwrap()
+		::client::new_in_mem(LocalDispatch::with_heap_pages(8), genesis_config).unwrap()
 	}
 
 	#[test]

substrate/polkadot/consensus/src/service.rs

@@ -56,6 +56,7 @@ fn start_bft<F, C>(
 	C: bft::BlockImport<Block> + bft::Authorities<Block> + 'static,
 	F::Error: ::std::fmt::Debug,
 	<F::Proposer as bft::Proposer<Block>>::Error: ::std::fmt::Display + Into<error::Error>,
+	<F as bft::Environment<Block>>::Error: ::std::fmt::Display
 {
 	let mut handle = LocalThreadHandle::current();
 	match bft_service.build_upon(&header) {
@@ -63,7 +64,7 @@ fn start_bft<F, C>(
 			debug!(target: "bft", "Couldn't initialize BFT agreement: {:?}", e);
 		},
 		Ok(None) => {},
-		Err(e) => debug!(target: "bft", "BFT agreement error: {:?}", e),
+		Err(e) => warn!(target: "bft", "BFT agreement error: {}", e),
 	}
 }
 

substrate/polkadot/service/src/chain_spec.rs

@@ -38,7 +38,7 @@ fn staging_testnet_config_genesis() -> GenesisConfig {
 	];
 	GenesisConfig {
 		consensus: Some(ConsensusConfig {
-			code: include_bytes!("../../runtime/wasm/genesis.wasm").to_vec(),	// TODO change
+			code: include_bytes!("../../runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.compact.wasm").to_vec(),	// TODO change
 			authorities: initial_authorities.clone(),
 		}),
 		system: None,

substrate/substrate/bft/Cargo.toml

@@ -9,6 +9,7 @@ substrate-codec = { path = "../codec" }
 substrate-primitives = { path = "../primitives" }
 substrate-runtime-support = { path = "../runtime-support" }
 substrate-runtime-primitives = { path = "../runtime/primitives" }
+substrate-runtime-version = { path = "../runtime/version" }
 ed25519 = { path = "../ed25519" }
 tokio = "0.1.7"
 parking_lot = "0.4"

substrate/substrate/bft/src/error.rs

@@ -15,6 +15,7 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Error types in the BFT service.
+use runtime_version::RuntimeVersion;
 
 error_chain! {
 	errors {
@@ -55,9 +56,21 @@ error_chain! {
 		}
 
 		/// Authoring interface does not match the runtime.
-		InvalidRuntime {
+		IncompatibleAuthoringRuntime(native: RuntimeVersion, on_chain: RuntimeVersion) {
 			description("Authoring for current runtime is not supported"),
-			display("Authoring for current runtime is not supported."),
+			display("Authoring for current runtime is not supported. Native ({}) cannot author for on-chain ({}).", native, on_chain),
+		}
+
+		/// Authoring interface does not match the runtime.
+		RuntimeVersionMissing {
+			description("Current runtime has no version"),
+			display("Authoring for current runtime is not supported since it has no version."),
+		}
+
+		/// Authoring interface does not match the runtime.
+		NativeRuntimeMissing {
+			description("This build has no native runtime"),
+			display("Authoring in current build is not supported since it has no runtime."),
 		}
 
 		/// Justification requirements not met.

substrate/substrate/bft/src/lib.rs

@@ -30,12 +30,15 @@
 //! In general, this future should be pre-empted by the import of a justification
 //! set for this block height.
 
+#![recursion_limit="128"]
+
 pub mod error;
 
 extern crate substrate_codec as codec;
 extern crate substrate_primitives as primitives;
 extern crate substrate_runtime_support as runtime_support;
 extern crate substrate_runtime_primitives as runtime_primitives;
+extern crate substrate_runtime_version as runtime_version;
 extern crate ed25519;
 extern crate tokio;
 extern crate parking_lot;

substrate/substrate/client/src/client.rs

@@ -479,10 +479,14 @@ impl<B, E, Block> bft::Authorities<Block> for Client<B, E, Block>
 		Block: BlockT,
 {
 	fn authorities(&self, at: &BlockId<Block>) -> Result<Vec<AuthorityId>, bft::Error> {
-		let version: Result<_, bft::Error> = self.runtime_version_at(at).map_err(|_| bft::ErrorKind::InvalidRuntime.into());
-		let version = version?;
-		if !self.executor.native_runtime_version().map_or(true, |v| v.can_author_with(&version)) {
-			return Err(bft::ErrorKind::InvalidRuntime.into())
+		let on_chain_version: Result<_, bft::Error> = self.runtime_version_at(at)
+			.map_err(|e| { trace!("Error getting runtime version {:?}", e); bft::ErrorKind::RuntimeVersionMissing.into() });
+		let on_chain_version = on_chain_version?;
+		let native_version: Result<_, bft::Error> = self.executor.native_runtime_version()
+			.ok_or_else(|| bft::ErrorKind::NativeRuntimeMissing.into());
+		let native_version = native_version?;
+		if !on_chain_version.can_author_with(&native_version) {
+			return Err(bft::ErrorKind::IncompatibleAuthoringRuntime(on_chain_version, native_version).into())
 		}
 		self.authorities_at(at).map_err(|_| {
 			let descriptor = format!("{:?}", at);

substrate/substrate/client/src/genesis.rs

@@ -54,6 +54,10 @@ mod tests {
 
 	native_executor_instance!(Executor, test_client::runtime::api::dispatch, test_client::runtime::VERSION, include_bytes!("../../test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm"));
 
+	fn executor() -> ::executor::NativeExecutor<Executor> {
+		Executor::with_heap_pages(8)
+	}
+
 	fn construct_block(backend: &InMemory, number: BlockNumber, parent_hash: Hash, state_root: Hash, txs: Vec<Transfer>) -> (Vec<u8>, Hash) {
 		use triehash::ordered_trie_root;
 
@@ -80,7 +84,7 @@ mod tests {
 		execute(
 			backend,
 			&mut overlay,
-			&Executor::new(),
+			&executor(),
 			"initialise_block",
 			&header.encode(),
 			ExecutionStrategy::NativeWhenPossible,
@@ -90,7 +94,7 @@ mod tests {
 			execute(
 				backend,
 				&mut overlay,
-				&Executor::new(),
+				&executor(),
 				"apply_extrinsic",
 				&tx.encode(),
 				ExecutionStrategy::NativeWhenPossible,
@@ -100,7 +104,7 @@ mod tests {
 		let (ret_data, _) = execute(
 			backend,
 			&mut overlay,
-			&Executor::new(),
+			&executor(),
 			"finalise_block",
 			&[],
 			ExecutionStrategy::NativeWhenPossible,
@@ -142,7 +146,7 @@ mod tests {
 		let _ = execute(
 			&backend,
 			&mut overlay,
-			&Executor::new(),
+			&executor(),
 			"execute_block",
 			&b1data,
 			ExecutionStrategy::NativeWhenPossible,

substrate/substrate/client/src/light/call_executor.rs

@@ -154,7 +154,7 @@ mod tests {
 		let remote_execution_proof = remote_client.execution_proof(&remote_block_id, "authorities", &[]).unwrap().1;
 
 		// check remote execution proof locally
-		let local_executor = test_client::NativeExecutor::new();
+		let local_executor = test_client::NativeExecutor::with_heap_pages(8);
 		do_check_execution_proof(remote_block_storage_root.into(), &local_executor, &RemoteCallRequest {
 			block: test_client::runtime::Hash::default(),
 			method: "authorities".into(),

substrate/substrate/executor/src/native_executor.rs

@@ -215,9 +215,6 @@ macro_rules! native_executor_instance {
 		}
 
 		impl $name {
-			pub fn new() -> $crate::NativeExecutor<$name> {
-				$crate::NativeExecutor::new()
-			}
 			pub fn with_heap_pages(heap_pages: usize) -> $crate::NativeExecutor<$name> {
 				$crate::NativeExecutor::with_heap_pages(heap_pages)
 			}

substrate/substrate/rpc/src/chain/tests.rs

@@ -35,7 +35,7 @@ fn should_return_header() {
 		Ok(Some(ref x)) if x == &Header {
 			parent_hash: 0.into(),
 			number: 0,
-			state_root: "17dccc74bd9200b7ce5a2f6a1bf379f1cdcf91bca3d19c3d17e1478b8d404703".into(),
+			state_root: "f8e419c265702a3eb72114255a7d9bcd2e8c1de4c66aadafd67b85ce3493c309".into(),
 			extrinsics_root: "56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421".into(),
 			digest: Default::default(),
 		}
@@ -72,7 +72,7 @@ fn should_notify_about_latest_block() {
 	// assert notification send to transport
 	let (notification, next) = core.block_on(transport.into_future()).unwrap();
 	assert_eq!(notification, Some(
-		r#"{"jsonrpc":"2.0","method":"test","params":{"result":{"digest":{"logs":[]},"extrinsicsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","number":1,"parentHash":"0x50fb1e7f32e8ad17f553846f4338861e17eb95132e7c3b433e0429ffab2f8f13","stateRoot":"0x17dccc74bd9200b7ce5a2f6a1bf379f1cdcf91bca3d19c3d17e1478b8d404703"},"subscription":0}}"#.to_owned()
+		r#"{"jsonrpc":"2.0","method":"test","params":{"result":{"digest":{"logs":[]},"extrinsicsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","number":1,"parentHash":"0x7393d002cbd1dc754bf03fa24ebf38624af50b74a20d9846ca7111fa1dd8e62e","stateRoot":"0xf8e419c265702a3eb72114255a7d9bcd2e8c1de4c66aadafd67b85ce3493c309"},"subscription":0}}"#.to_owned()
 	));
 	// no more notifications on this channel
 	assert_eq!(core.block_on(next.into_future()).unwrap().0, None);

substrate/substrate/runtime/version/src/lib.rs

@@ -39,6 +39,9 @@ use codec::Slicable;
 #[cfg(feature = "std")]
 use std::borrow::Cow;
 
+#[cfg(feature = "std")]
+use std::fmt;
+
 #[cfg(feature = "std")]
 pub type VersionString = ::std::borrow::Cow<'static, str>;
 #[cfg(not(feature = "std"))]
@@ -108,6 +111,13 @@ impl Default for RuntimeVersion {
 	}
 }
 
+#[cfg(feature = "std")]
+impl fmt::Display for RuntimeVersion {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		write!(f, "{}-{}:{}({}-{})", self.spec_name, self.spec_version, self.authoring_version, self.impl_name, self.impl_version)
+	}
+}
+
 #[cfg(feature = "std")]
 impl RuntimeVersion {
 	/// Check if this version matches other version for calling into runtime.

substrate/substrate/state-machine/src/lib.rs

@@ -258,22 +258,37 @@ pub fn execute_using_consensus_failure_handler<
 ) -> Result<(Vec<u8>, B::Transaction), Box<Error>> {
 	let strategy: ExecutionStrategy = (&manager).into();
 	let result = {
-		let mut externalities = ext::Ext::new(overlay, backend);
-		// make a copy.
-		let code = externalities.storage(b":code")
-			.ok_or(Box::new(ExecutionError::CodeEntryDoesNotExist) as Box<Error>)?
-			.to_vec();
+		let maybe_orig_prospective =
+			if let &ExecutionManager::Both(_) = &manager { Some(overlay.prospective.clone()) } else { None };
 
-		let (result, was_native) = exec.call(
-			&mut externalities,
-			&code,
-			method,
-			call_data,
-			// attempt to run native first, if we're not directed to run wasm only
-			strategy != ExecutionStrategy::AlwaysWasm,
-		);
+		let ((result, was_native), code, delta) = {
+			let mut externalities = ext::Ext::new(overlay, backend);
+			// make a copy.
+			let code = externalities.storage(b":code")
+				.ok_or(Box::new(ExecutionError::CodeEntryDoesNotExist) as Box<Error>)?
+				.to_vec();
+
+			(
+				exec.call(
+					&mut externalities,
+					&code,
+					method,
+					call_data,
+					// attempt to run native first, if we're not directed to run wasm only
+					strategy != ExecutionStrategy::AlwaysWasm,
+				),
+				code,
+				externalities.transaction()
+			)
+		};
+		
 		// run wasm separately if we did run native the first time and we're meant to run both
-		let result = if let (true, ExecutionManager::Both(on_consensus_failure)) = (was_native, manager) {
+		let (result, delta) = if let (true, ExecutionManager::Both(on_consensus_failure), Some(mut orig_prospective)) =
+			(was_native, manager, maybe_orig_prospective)
+		{
+			::std::mem::swap(&mut orig_prospective, &mut overlay.prospective);
+			let mut externalities = ext::Ext::new(overlay, backend);
+
 			let (wasm_result, _) = exec.call(
 				&mut externalities,
 				&code,
@@ -281,18 +296,20 @@ pub fn execute_using_consensus_failure_handler<
 				call_data,
 				false
 			);
-			if (result.is_ok() && wasm_result.is_ok() && result.as_ref().unwrap() == wasm_result.as_ref().unwrap())
+			let wasm_delta = externalities.transaction();
+
+			if (result.is_ok() && wasm_result.is_ok() && result.as_ref().unwrap() == wasm_result.as_ref().unwrap()/* && delta == wasm_delta*/)
 				|| (result.is_err() && wasm_result.is_err() && format!("{}", result.as_ref().unwrap_err()) == format!("{}", wasm_result.as_ref().unwrap_err()))
 			{
-				result
+				(result, delta)
 			} else {
 				// Consensus error.
-				on_consensus_failure(wasm_result, result)
+				(on_consensus_failure(wasm_result, result), wasm_delta)
 			}
 		} else {
-			result
+			(result, delta)
 		};
-		result.map(move |out| (out, externalities.transaction()))
+		result.map(move |out| (out, delta))
 	};
 
 	match result {

substrate/substrate/test-client/src/client_ext.rs

@@ -38,7 +38,7 @@ pub trait TestClient {
 
 impl TestClient for Client<Backend, Executor, runtime::Block> {
 	fn new_for_tests() -> Self {
-		client::new_in_mem(NativeExecutor::new(), genesis_storage()).unwrap()
+		client::new_in_mem(NativeExecutor::with_heap_pages(8), genesis_storage()).unwrap()
 	}
 
 	fn justify_and_import(&self, origin: client::BlockOrigin, block: runtime::Block) -> client::error::Result<()> {

substrate/substrate/test-runtime/src/genesismap.rs

@@ -37,7 +37,7 @@ impl GenesisConfig {
 	}
 
 	pub fn genesis_map(&self) -> HashMap<Vec<u8>, Vec<u8>> {
-		let wasm_runtime = include_bytes!("../wasm/genesis.wasm").to_vec();
+		let wasm_runtime = include_bytes!("../wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm").to_vec();
 		self.balances.iter()
 			.map(|&(account, balance)| (account.to_keyed_vec(b"balance:"), vec![].and(&balance)))
 			.map(|(k, v)| (twox_128(&k[..])[..].to_vec(), v.to_vec()))

substrate/update-genesis.sh

@@ -1,4 +0,0 @@
-#!/usr/bin/env bash
-
-cp polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.compact.wasm polkadot/runtime/wasm/genesis.wasm
-cp substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm substrate/test-runtime/wasm/genesis.wasm