Extrinsics root is calculated as part of block-building (#120)

* extrinsics root is calculated as part of block-building.

* Fix build.

* Fix xt root.

* Couple of fixes.

* Logging and more useful APIs.

* Fix test.

* Update log version.

* Switch back to correct version of log.

substrate/Cargo.lock

@@ -1171,6 +1171,7 @@ name = "polkadot-api"
 version = "0.1.0"
 dependencies = [
  "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "polkadot-executor 0.1.0",
  "polkadot-primitives 0.1.0",
  "polkadot-runtime 0.1.0",
@@ -1229,7 +1230,7 @@ dependencies = [
  "ed25519 0.1.0",
  "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
- "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "polkadot-api 0.1.0",
  "polkadot-collator 0.1.0",
@@ -1331,7 +1332,7 @@ dependencies = [
  "ed25519 0.1.0",
  "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
- "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "polkadot-api 0.1.0",
  "polkadot-consensus 0.1.0",
@@ -1365,6 +1366,7 @@ dependencies = [
  "ed25519 0.1.0",
  "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethereum-types 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "polkadot-api 0.1.0",
  "polkadot-primitives 0.1.0",
  "polkadot-runtime 0.1.0",
@@ -1732,7 +1734,7 @@ dependencies = [
  "ed25519 0.1.0",
  "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
- "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "substrate-codec 0.1.0",
  "substrate-executor 0.1.0",
@@ -1871,6 +1873,7 @@ dependencies = [
  "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "jsonrpc-core 8.0.1 (git+https://github.com/paritytech/jsonrpc.git)",
  "jsonrpc-macros 8.0.0 (git+https://github.com/paritytech/jsonrpc.git)",
+ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "substrate-client 0.1.0",
  "substrate-executor 0.1.0",

substrate/polkadot/api/Cargo.toml

@@ -5,6 +5,7 @@ authors = ["Parity Technologies <admin@parity.io>"]
 
 [dependencies]
 error-chain = "0.11"
+log = "0.3"
 polkadot-executor = { path = "../executor" }
 polkadot-runtime = { path = "../runtime" }
 polkadot-primitives = { path = "../primitives" }

substrate/polkadot/api/src/lib.rs

@@ -31,6 +31,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;
 
@@ -174,7 +177,7 @@ impl<B: Backend> PolkadotApi for Client<B, NativeExecutor<LocalDispatch>>
 	fn check_id(&self, id: BlockId) -> Result<CheckedId> {
 		// bail if the code is not the same as the natively linked.
 		if self.code_at(&id)? != LocalDispatch::native_equivalent() {
-			bail!(ErrorKind::UnknownRuntime);
+			warn!("This node is out of date. Block authoring may not work correctly.")
 		}
 
 		Ok(CheckedId(id))
@@ -324,19 +327,15 @@ impl<S: state_machine::Backend> BlockBuilder for ClientBlockBuilder<S>
 	}
 
 	fn bake(mut self) -> Block {
-		use substrate_runtime_executive::extrinsics_root;
-
 		let mut ext = state_machine::Ext {
 			overlay: &mut self.changes,
 			backend: &self.state,
 		};
 
-		let mut final_header = ::substrate_executor::with_native_environment(
+		let final_header = ::substrate_executor::with_native_environment(
 			&mut ext,
 			move || runtime::Executive::finalise_block()
 		).expect("all inherent extrinsics pushed; all other extrinsics executed correctly; qed");
-
-		final_header.extrinsics_root = extrinsics_root::<runtime_io::BlakeTwo256, _>(&self.extrinsics);
 		Block {
 			header: final_header,
 			extrinsics: self.extrinsics,

substrate/polkadot/consensus/Cargo.toml

@@ -9,7 +9,7 @@ parking_lot = "0.4"
 tokio-core = "0.1.12"
 ed25519 = { path = "../../substrate/ed25519" }
 error-chain = "0.11"
-log = "0.4"
+log = "0.3"
 polkadot-api = { path = "../api" }
 polkadot-collator = { path = "../collator" }
 polkadot-primitives = { path = "../primitives" }

substrate/polkadot/service/Cargo.toml

@@ -8,7 +8,7 @@ futures = "0.1.17"
 parking_lot = "0.4"
 tokio-timer = "0.1.2"
 error-chain = "0.11"
-log = "0.4"
+log = "0.3"
 tokio-core = "0.1.12"
 ed25519 = { path = "../../substrate/ed25519" }
 polkadot-primitives = { path = "../primitives" }

substrate/polkadot/transaction-pool/Cargo.toml

@@ -4,6 +4,7 @@ version = "0.1.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 
 [dependencies]
+log = "0.4.0"
 transaction-pool = "1.9.0"
 error-chain = "0.11"
 polkadot-api = { path = "../api" }

substrate/polkadot/transaction-pool/src/lib.rs

@@ -28,6 +28,9 @@ extern crate transaction_pool;
 #[macro_use]
 extern crate error_chain;
 
+#[macro_use]
+extern crate log;
+
 use std::collections::HashMap;
 use std::cmp::Ordering;
 use std::sync::Arc;
@@ -296,12 +299,10 @@ impl<'a, T: 'a + PolkadotApi> transaction_pool::Ready<VerifiedTransaction> for R
 		let next_index = self.known_indices.entry(sender)
 			.or_insert_with(|| api_handle.index(at_block, sender).ok().unwrap_or_else(u64::max_value));
 
-		*next_index += 1;
-
 		match xt.inner.index.cmp(&next_index) {
 			Ordering::Greater => Readiness::Future,
 			Ordering::Equal => Readiness::Ready,
-			Ordering::Less => Readiness::Stalled,
+			Ordering::Less => Readiness::Stalled,	// TODO: this is not "stalled" but rather stale and can be discarded.
 		}
 	}
 }
@@ -330,12 +331,14 @@ impl TransactionPool {
 
 		let verified = VerifiedTransaction::create(xt, insertion_index)?;
 
+		info!("Extrinsic verified {:?}. Importing...", verified);
+
 		// TODO: just use a foreign link when the error type is made public.
 		let hash = verified.hash.clone();
 		self.inner.import(verified)
 			.map_err(|e|
 				match e {
-					// TODO: make error types public in transaction_pool. For now just treat all errors as AlradyImported
+					// TODO: make error types public in transaction_pool. For now just treat all errors as AlreadyImported
 					_ => ErrorKind::AlreadyImported(hash),
 					// transaction_pool::error::AlreadyImported(h) => ErrorKind::AlreadyImported(h),
 					// e => ErrorKind::Import(Box::new(e)),
@@ -376,9 +379,12 @@ impl TransactionPool {
 
 impl substrate_rpc::author::AsyncAuthorApi for TransactionPool {
 	fn submit_extrinsic(&mut self, xt: Extrinsic) -> substrate_rpc::author::error::Result<()> {
-		self.import(xt
-			.using_encoded(|ref mut s| UncheckedExtrinsic::decode(s))
-			.ok_or(substrate_rpc::author::error::ErrorKind::InvalidFormat)?)
+		use substrate_primitives::hexdisplay::HexDisplay;
+		info!("Extrinsic submitted: {}", HexDisplay::from(&xt.0));
+		let xt = xt.using_encoded(|ref mut s| UncheckedExtrinsic::decode(s))
+			.ok_or(substrate_rpc::author::error::ErrorKind::InvalidFormat)?;
+		info!("Correctly formatted: {:?}", xt);
+		self.import(xt)
 			.map(|_| ())
 			.map_err(|_| substrate_rpc::author::error::ErrorKind::PoolError.into())
 	}

substrate/substrate/bft/Cargo.toml

@@ -12,7 +12,7 @@ ed25519 = { path = "../ed25519" }
 tokio-timer = "0.1.2"
 parking_lot = "0.4"
 error-chain = "0.11"
-log = "0.4"
+log = "0.3"
 
 [dev-dependencies]
 substrate-keyring = { path = "../keyring" }

substrate/substrate/executor/src/wasm_executor.rs

@@ -179,9 +179,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		let key = this.memory.get(key_data, key_len as usize).map_err(|_| DummyUserError)?;
 		let value = this.memory.get(value_data, value_len as usize).map_err(|_| DummyUserError)?;
 		if let Some(preimage) = this.hash_lookup.get(&key) {
-			info!(target: "wasm-trace", "*** Setting storage: %{} -> {}   [k={}]", ascii_format(&preimage), HexDisplay::from(&value), HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "*** Setting storage: %{} -> {}   [k={}]", ascii_format(&preimage), HexDisplay::from(&value), HexDisplay::from(&key));
 		} else {
-			info!(target: "wasm-trace", "*** Setting storage:  {} -> {}   [k={}]", ascii_format(&key), HexDisplay::from(&value), HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "*** Setting storage:  {} -> {}   [k={}]", ascii_format(&key), HexDisplay::from(&value), HexDisplay::from(&key));
 		}
 		this.ext.set_storage(key, value);
 		Ok(())
@@ -189,9 +189,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 	ext_clear_storage(key_data: *const u8, key_len: u32) => {
 		let key = this.memory.get(key_data, key_len as usize).map_err(|_| DummyUserError)?;
 		if let Some(preimage) = this.hash_lookup.get(&key) {
-			info!(target: "wasm-trace", "*** Clearing storage: %{}   [k={}]", ascii_format(&preimage), HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "*** Clearing storage: %{}   [k={}]", ascii_format(&preimage), HexDisplay::from(&key));
 		} else {
-			info!(target: "wasm-trace", "*** Clearing storage:  {}   [k={}]", ascii_format(&key), HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "*** Clearing storage:  {}   [k={}]", ascii_format(&key), HexDisplay::from(&key));
 		}
 		this.ext.clear_storage(&key);
 		Ok(())
@@ -202,9 +202,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		let maybe_value = this.ext.storage(&key);
 
 		if let Some(preimage) = this.hash_lookup.get(&key) {
-			info!(target: "wasm-trace", "    Getting storage: %{} == {}   [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "    Getting storage: %{} == {}   [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
 		} else {
-			info!(target: "wasm-trace", "    Getting storage:  {} == {}   [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "    Getting storage:  {} == {}   [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
 		}
 
 		if let Some(value) = maybe_value {
@@ -222,9 +222,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		let key = this.memory.get(key_data, key_len as usize).map_err(|_| DummyUserError)?;
 		let maybe_value = this.ext.storage(&key);
 		if let Some(preimage) = this.hash_lookup.get(&key) {
-			info!(target: "wasm-trace", "    Getting storage: %{} == {}   [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "    Getting storage: %{} == {}   [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
 		} else {
-			info!(target: "wasm-trace", "    Getting storage:  {} == {}   [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
+			trace!(target: "wasm-trace", "    Getting storage:  {} == {}   [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
 		}
 		if let Some(value) = maybe_value {
 			let value = &value[value_offset as usize..];

substrate/substrate/rpc/Cargo.toml

@@ -5,6 +5,7 @@ authors = ["Parity Technologies <admin@parity.io>"]
 
 [dependencies]
 parking_lot = "0.4"
+log = "0.3"
 error-chain = "0.11"
 jsonrpc-core = { git="https://github.com/paritytech/jsonrpc.git" }
 jsonrpc-macros = { git="https://github.com/paritytech/jsonrpc.git" }

substrate/substrate/rpc/src/lib.rs

@@ -28,6 +28,8 @@ extern crate substrate_state_machine as state_machine;
 extern crate error_chain;
 #[macro_use]
 extern crate jsonrpc_macros;
+#[macro_use]
+extern crate log;
 
 #[cfg(test)]
 extern crate substrate_executor;

substrate/substrate/rpc/src/state/mod.rs

@@ -32,13 +32,21 @@ use self::error::Result;
 build_rpc_trait! {
 	/// Polkadot state API
 	pub trait StateApi {
+		/// Returns a storage entry.
+		#[rpc(name = "state_getStorageAt")]
+		fn storage_at(&self, StorageKey, block::HeaderHash) -> Result<StorageData>;
+
+		/// Call a contract.
+		#[rpc(name = "state_callAt")]
+		fn call_at(&self, String, Vec<u8>, block::HeaderHash) -> Result<Vec<u8>>;
+
 		/// Returns a storage entry.
 		#[rpc(name = "state_getStorage")]
-		fn storage(&self, StorageKey, block::HeaderHash) -> Result<StorageData>;
+		fn storage(&self, StorageKey) -> Result<StorageData>;
 
 		/// Call a contract.
 		#[rpc(name = "state_call")]
-		fn call(&self, String, Vec<u8>, block::HeaderHash) -> Result<Vec<u8>>;
+		fn call(&self, String, Vec<u8>) -> Result<Vec<u8>>;
 	}
 }
 
@@ -47,11 +55,22 @@ impl<B, E> StateApi for Arc<Client<B, E>> where
 	E: state_machine::CodeExecutor + Send + Sync + 'static,
 	client::error::Error: From<<<B as client::backend::Backend>::State as state_machine::backend::Backend>::Error>,
 {
-	fn storage(&self, key: StorageKey, block: block::HeaderHash) -> Result<StorageData> {
+	fn storage_at(&self, key: StorageKey, block: block::HeaderHash) -> Result<StorageData> {
 		Ok(self.as_ref().storage(&block::Id::Hash(block), &key)?)
 	}
 
-	fn call(&self, method: String, data: Vec<u8>, block: block::HeaderHash) -> Result<Vec<u8>> {
+	fn call_at(&self, method: String, data: Vec<u8>, block: block::HeaderHash) -> Result<Vec<u8>> {
 		Ok(self.as_ref().call(&block::Id::Hash(block), &method, &data)?.return_data)
 	}
+
+	fn storage(&self, key: StorageKey) -> Result<StorageData> {
+		let at = block::Id::Hash(self.as_ref().info()?.chain.best_hash);
+		use primitives::hexdisplay::HexDisplay;
+		info!("Querying storage at {:?} for key {}", at, HexDisplay::from(&key.0));
+		Ok(self.as_ref().storage(&at, &key)?)
+	}
+
+	fn call(&self, method: String, data: Vec<u8>) -> Result<Vec<u8>> {
+		Ok(self.as_ref().call(&block::Id::Hash(self.as_ref().info()?.chain.best_hash), &method, &data)?.return_data)
+	}
 }

substrate/substrate/rpc/src/state/tests.rs

@@ -34,7 +34,7 @@ fn should_return_storage() {
 	let genesis_hash = test_genesis_block.blake2_256().into();
 
 	assert_matches!(
-		StateApi::storage(&client, StorageKey(vec![10]), genesis_hash),
+		StateApi::storage_at(&client, StorageKey(vec![10]), genesis_hash),
 		Err(Error(ErrorKind::Client(client::error::ErrorKind::NoValueForKey(ref k)), _)) if *k == vec![10]
 	)
 }
@@ -55,7 +55,7 @@ fn should_call_contract() {
 	let genesis_hash = test_genesis_block.blake2_256().into();
 
 	assert_matches!(
-		StateApi::call(&client, "balanceOf".into(), vec![1,2,3], genesis_hash),
+		StateApi::call_at(&client, "balanceOf".into(), vec![1,2,3], genesis_hash),
 		Err(Error(ErrorKind::Client(client::error::ErrorKind::Execution(_)), _))
 	)
 }

substrate/substrate/runtime/executive/src/lib.rs

@@ -49,13 +49,7 @@ use runtime_io::Hashing;
 use runtime_support::StorageValue;
 use primitives::traits::{self, Header, Zero, One, Checkable, Applyable, CheckEqual, Executable, MakePayment};
 use codec::Slicable;
-
-/// Compute the extrinsics root of a list of extrinsics.
-pub fn extrinsics_root<H: Hashing, E: Slicable>(extrinsics: &[E]) -> H::Output {
-	let xts = extrinsics.iter().map(Slicable::encode).collect::<Vec<_>>();
-	let xts = xts.iter().map(Vec::as_slice).collect::<Vec<_>>();
-	H::enumerated_trie_root(&xts)
-}
+use system::extrinsics_root;
 
 pub struct Executive<
 	System,
@@ -103,7 +97,7 @@ impl<
 
 		// execute transactions
 		let (header, extrinsics) = block.deconstruct();
-		extrinsics.into_iter().for_each(Self::apply_extrinsic);
+		extrinsics.into_iter().for_each(Self::apply_extrinsic_inner);
 
 		// post-transactional book-keeping.
 		Finalisation::execute();
@@ -120,6 +114,9 @@ impl<
 	pub fn finalise_block() -> System::Header {
 		Finalisation::execute();
 
+		// setup extrinsics
+		<system::Module<System>>::derive_extrinsics();
+
 		let header = <system::Module<System>>::finalise();
 		Self::post_finalise(&header);
 
@@ -127,8 +124,16 @@ impl<
 	}
 
 	/// Apply outside of the block execution function.
-	/// This doesn't attempt to validate anything regarding the block.
+	/// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt
+	/// hashes.
 	pub fn apply_extrinsic(uxt: Block::Extrinsic) {
+		<system::Module<System>>::note_extrinsic(uxt.encode());
+		Self::apply_extrinsic_inner(uxt);
+	}
+
+	/// Apply outside of the block execution function.
+	/// This doesn't attempt to validate anything regarding the block.
+	fn apply_extrinsic_inner(uxt: Block::Extrinsic) {
 		// Verify the signature is good.
 		let xt = match uxt.check() {
 			Ok(xt) => xt,

substrate/substrate/runtime/system/src/lib.rs

@@ -47,6 +47,17 @@ use codec::Slicable;
 #[cfg(any(feature = "std", test))]
 use runtime_io::{twox_128, TestExternalities};
 
+/// Compute the extrinsics root of a list of extrinsics.
+pub fn extrinsics_root<H: Hashing, E: codec::Slicable>(extrinsics: &[E]) -> H::Output {
+	extrinsics_data_root::<H>(extrinsics.iter().map(codec::Slicable::encode).collect())
+}
+
+/// Compute the extrinsics root of a list of extrinsics.
+pub fn extrinsics_data_root<H: Hashing>(xts: Vec<Vec<u8>>) -> H::Output {
+	let xts = xts.iter().map(Vec::as_slice).collect::<Vec<_>>();
+	H::enumerated_trie_root(&xts)
+}
+
 pub trait Trait {
 	type Index: Parameter + Default + SimpleArithmetic + Copy;
 	type BlockNumber: Parameter + SimpleArithmetic + Default + Bounded + Copy;
@@ -68,6 +79,7 @@ decl_storage! {
 	pub BlockHash get(block_hash): b"sys:old" => required map [ T::BlockNumber => T::Hash ];
 
 	pub ExtrinsicIndex get(extrinsic_index): b"sys:xti" => required u32;
+	pub ExtrinsicData get(extrinsic_data): b"sys:xtd" => required map [ u32 => Vec<u8> ];
 	RandomSeed get(random_seed): b"sys:rnd" => required T::Hash;
 	// The current block number being processed. Set by `execute_block`.
 	Number get(block_number): b"sys:num" => required T::BlockNumber;
@@ -170,6 +182,19 @@ impl<T: Trait> Module<T> {
 	pub fn inc_account_index(who: &T::AccountId) {
 		<AccountIndex<T>>::insert(who, Self::account_index(who) + T::Index::one());
 	}
+
+	/// Note what the extrinsic data of the current extrinsic index is. If this is called, then
+	/// ensure `derive_extrinsics` is also called before block-building is completed.
+	pub fn note_extrinsic(encoded_xt: Vec<u8>) {
+		<ExtrinsicData<T>>::insert(Self::extrinsic_index(), encoded_xt);
+	}
+
+	/// Remove all extrinsics data and save the extrinsics trie root.
+	pub fn derive_extrinsics() {
+		let extrinsics = (0..Self::extrinsic_index()).map(<ExtrinsicData<T>>::take).collect();
+		let xts_root = extrinsics_data_root::<T::Hashing>(extrinsics);
+		<ExtrinsicsRoot<T>>::put(xts_root);
+	}
 }
 
 #[cfg(any(feature = "std", test))]