Refactor the runtime API to use traits. (#878)

* Add missing `As` imports.

* Adds new API traits that will be used by the client and runtime

* Switch consensus to new API's

* Switches transaction-pool to new API's

* Move runtime api stuff into its own crate

* Adds `impl_apis!` macro for implementing the new API traits

* Make `metadata` return directly a blob

* Runtime replace `impl_stubs!` with `impl_apis!`

* Switches to none feature based approach for declaring the different API traits

* Fixes compilation error

* Fixes errors

* Make the `decl_apis!` trait usable from the outside

* Make the `test-client` use the new API traits

* Remove last `impl_stubs!` bits and move some of them into wasm executor for tests

* A little bit more documentation

substrate/node/consensus/src/lib.rs

@@ -46,6 +46,7 @@ use std::sync::Arc;
 use std::time::{self, Duration, Instant};
 
 use client::{Client as SubstrateClient, CallExecutor};
+use client::runtime_api::{BlockBuilder as BlockBuilderAPI, Core, Miscellaneous, OldTxQueue};
 use codec::{Decode, Encode};
 use node_primitives::{AccountId, Timestamp, SessionKey, InherentData};
 use node_runtime::Runtime;
@@ -86,31 +87,23 @@ pub trait BlockBuilder<Block: BlockT> {
 }
 
 /// Local client abstraction for the consensus.
-pub trait AuthoringApi: Send + Sync {
+pub trait AuthoringApi:
+	Send
+	+ Sync
+	+ BlockBuilderAPI<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
+	+ Core<<Self as AuthoringApi>::Block, AuthorityId, Error=<Self as AuthoringApi>::Error>
+	+ Miscellaneous<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
+	+ OldTxQueue<<Self as AuthoringApi>::Block, Error=<Self as AuthoringApi>::Error>
+{
 	/// The block used for this API type.
 	type Block: BlockT;
 	/// The block builder for this API type.
 	type BlockBuilder: BlockBuilder<Self::Block>;
-
-	/// Get the value of the randomness beacon at a given block.
-	fn random_seed(&self, at: &BlockId<Self::Block>) -> Result<<Self::Block as BlockT>::Hash>;
-
-	/// Get validators at a given block.
-	fn validators(&self, at: &BlockId<Self::Block>) -> Result<Vec<AccountId>>;
+	/// The error used by this API type.
+	type Error;
 
 	/// Build a block on top of the given, with inherent extrinsics pre-pushed.
 	fn build_block(&self, at: &BlockId<Self::Block>, inherent_data: InherentData) -> Result<Self::BlockBuilder>;
-
-	/// Get the nonce (né index) of an account at a block.
-	fn index(&self, at: &BlockId<Self::Block>, account: AccountId) -> Result<u64>;
-
-	/// Attempt to produce the (encoded) inherent extrinsics for a block being built upon the given.
-	/// This may vary by runtime and will fail if a runtime doesn't follow the same API.
-	fn inherent_extrinsics(&self, at: &BlockId<Self::Block>, inherent_data: InherentData) -> Result<Vec<<Self::Block as BlockT>::Extrinsic>>;
-
-	/// Evaluate a block. Returns true if the block is good, false if it is known to be bad,
-	/// and an error if we can't evaluate for some reason.
-	fn evaluate_block(&self, at: &BlockId<Self::Block>, block: Self::Block) -> Result<bool>;
 }
 
 impl<B, E, Block> BlockBuilder<Block> for client::block_builder::BlockBuilder<B, E, Block, Blake2Hasher> where
@@ -134,14 +127,7 @@ impl<B, E, Block> AuthoringApi for SubstrateClient<B, E, Block> where
 {
 	type Block = Block;
 	type BlockBuilder = client::block_builder::BlockBuilder<B, E, Block, Blake2Hasher>;
-
-	fn random_seed(&self, at: &BlockId<Block>) -> Result<<Self::Block as BlockT>::Hash> {
-		self.call_api_at(at, "random_seed", &()).map_err(Into::into)
-	}
-
-	fn validators(&self, at: &BlockId<Block>) -> Result<Vec<AccountId>> {
-		self.call_api_at(at, "validators", &()).map_err(Into::into)
-	}
+	type Error = client::error::Error;
 
 	fn build_block(&self, at: &BlockId<Block>, inherent_data: InherentData) -> Result<Self::BlockBuilder> {
 		let runtime_version = self.runtime_version_at(at)?;
@@ -152,27 +138,9 @@ impl<B, E, Block> AuthoringApi for SubstrateClient<B, E, Block> where
 				block_builder.push(inherent)?;
 			}
 		}
+
 		Ok(block_builder)
 	}
-
-	fn index(&self, at: &BlockId<Block>, account: AccountId) -> Result<u64> {
-		self.call_api_at(at, "account_nonce", &account).map_err(Into::into)
-	}
-
-	fn inherent_extrinsics(&self, at: &BlockId<Self::Block>, inherent_data: InherentData) -> Result<Vec<<Block as BlockT>::Extrinsic>> {
-		self.call_api_at(at, "inherent_extrinsics", &inherent_data).map_err(Into::into)
-	}
-
-	fn evaluate_block(&self, at: &BlockId<Self::Block>, block: Self::Block) -> Result<bool> {
-		let res: client::error::Result<()> = self.call_api_at(at, "execute_block", &block);
-		match res {
-			Ok(()) => Ok(true),
-			Err(err) => match err.kind() {
-				&client::error::ErrorKind::Execution(_) => Ok(false),
-				_ => Err(err.into())
-			}
-		}
-	}
 }
 
 /// A long-lived network which can create BFT message routing processes on demand.
@@ -217,7 +185,8 @@ impl<N, C> bft::Environment<<C as AuthoringApi>::Block> for ProposerFactory<N, C
 	N: Network<Block=<C as AuthoringApi>::Block>,
 	C: AuthoringApi + TPClient<Block=<C as AuthoringApi>::Block>,
 	<<C as AuthoringApi>::Block as BlockT>::Hash:
-		Into<<Runtime as SystemT>::Hash> + PartialEq<primitives::H256> + Into<primitives::H256>
+		Into<<Runtime as SystemT>::Hash> + PartialEq<primitives::H256> + Into<primitives::H256>,
+	Error: From<<C as AuthoringApi>::Error>
 {
 	type Proposer = Proposer<C>;
 	type Input = N::Input;
@@ -297,7 +266,8 @@ impl<C: AuthoringApi + TPClient> Proposer<C> {
 impl<C> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C> where
 	C: AuthoringApi + TPClient<Block=<C as AuthoringApi>::Block>,
 	<<C as AuthoringApi>::Block as BlockT>::Hash:
-		Into<<Runtime as SystemT>::Hash> + PartialEq<primitives::H256> + Into<primitives::H256>
+		Into<<Runtime as SystemT>::Hash> + PartialEq<primitives::H256> + Into<primitives::H256>,
+	error::Error: From<<C as AuthoringApi>::Error>
 {
 	type Create = Result<<C as AuthoringApi>::Block>;
 	type Error = Error;
@@ -419,7 +389,7 @@ impl<C> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C> where
 
 			match timestamp_delay {
 				Some(duration) => future::Either::A(
-					Delay::new(duration).map_err(|e| Error::from(ErrorKind::Timer(e)))
+					Delay::new(duration).map_err(|e| ErrorKind::Timer(e).into())
 				),
 				None => future::Either::B(future::ok(())),
 			}
@@ -434,9 +404,13 @@ impl<C> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C> where
 
 		// evaluate whether the block is actually valid.
 		// TODO: is it better to delay this until the delays are finished?
-		let evaluated = self.client
-			.evaluate_block(&self.parent_id, unchecked_proposal.clone())
-			.map_err(Into::into);
+		let evaluated = match self.client.execute_block(&self.parent_id, unchecked_proposal.clone()).map_err(Error::from) {
+			Ok(()) => Ok(true),
+			Err(err) => match err.kind() {
+				error::ErrorKind::Client(client::error::ErrorKind::Execution(_)) => Ok(false),
+				_ => Err(err)
+			}
+		};
 
 		let future = future::result(evaluated).and_then(move |good| {
 			let end_result = future::ok(good);
@@ -471,7 +445,8 @@ impl<C> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C> where
 				.filter(|tx| tx.verified.sender == local_id)
 				.last()
 				.map(|tx| Ok(tx.verified.index()))
-				.unwrap_or_else(|| AuthoringApi::index(self.client.as_ref(), &self.parent_id, local_id))
+				.unwrap_or_else(|| self.client.account_nonce(&self.parent_id, local_id))
+				.map_err(Error::from)
 			);
 
 			match cur_index {

substrate/node/consensus/src/service.rs

@@ -83,6 +83,7 @@ impl Service {
 	) -> Service
 		where
 			A: AuthoringApi + TPClient<Block = <A as AuthoringApi>::Block> + 'static,
+			error::Error: From<<A as AuthoringApi>::Error>,
 			C: BlockchainEvents<<A as AuthoringApi>::Block>
 				+ ChainHead<<A as AuthoringApi>::Block>
 				+ BlockBody<<A as AuthoringApi>::Block>,

substrate/node/runtime/Cargo.toml

@@ -8,9 +8,10 @@ rustc-hex = "1.0"
 hex-literal = "0.1.0"
 serde = { version = "1.0", default-features = false }
 serde_derive = { version = "1.0", optional = true }
-safe-mix = { version = "1.0", default-features = false}
+safe-mix = { version = "1.0", default-features = false }
 parity-codec = "2.0"
 parity-codec-derive = "2.0"
+sr-api = { path = "../../core/sr-api", default-features = false }
 sr-std = { path = "../../core/sr-std" }
 sr-io = { path = "../../core/sr-io" }
 srml-support = { path = "../../srml/support" }
@@ -36,6 +37,7 @@ default = ["std"]
 std = [
 	"parity-codec/std",
 	"substrate-primitives/std",
+	"sr-api/std",
 	"sr-std/std",
 	"sr-io/std",
 	"srml-support/std",

substrate/node/runtime/src/lib.rs

@@ -20,9 +20,11 @@
 // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256.
 #![recursion_limit="256"]
 
-#[macro_use]
 extern crate sr_io as runtime_io;
 
+#[macro_use]
+extern crate sr_api as runtime_api;
+
 #[macro_use]
 extern crate srml_support;
 
@@ -61,8 +63,11 @@ mod checked_block;
 use rstd::prelude::*;
 use substrate_primitives::u32_trait::{_2, _4};
 use node_primitives::{AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, SessionKey, Signature, InherentData};
+use runtime_api::runtime::*;
+use runtime_primitives::ApplyResult;
+use runtime_primitives::transaction_validity::TransactionValidity;
 use runtime_primitives::generic;
-use runtime_primitives::traits::{Convert, BlakeTwo256, DigestItem};
+use runtime_primitives::traits::{Convert, BlakeTwo256, DigestItem, Block as BlockT};
 use version::{RuntimeVersion, ApiId};
 use council::{motions as council_motions, voting as council_voting};
 #[cfg(feature = "std")]
@@ -243,37 +248,86 @@ pub type CheckedExtrinsic = generic::CheckedExtrinsic<AccountId, Index, Call>;
 /// Executive: handles dispatch to the various modules.
 pub type Executive = executive::Executive<Runtime, Block, balances::ChainContext<Runtime>, Balances, AllModules>;
 
-pub mod api {
-	impl_stubs!(
-		version => |()| super::VERSION,
-		metadata => |()| super::Runtime::metadata(),
-		authorities => |()| super::Consensus::authorities(),
-		initialise_block => |header| super::Executive::initialise_block(&header),
-		apply_extrinsic => |extrinsic| super::Executive::apply_extrinsic(extrinsic),
-		execute_block => |block| super::Executive::execute_block(block),
-		finalise_block => |()| super::Executive::finalise_block(),
-		inherent_extrinsics => |inherent| super::inherent_extrinsics(inherent),
-		validator_count => |()| super::Session::validator_count(),
-		validators => |()| super::Session::validators(),
-		timestamp => |()| super::Timestamp::get(),
-		random_seed => |()| super::System::random_seed(),
-		account_nonce => |account| super::System::account_nonce(&account),
-		lookup_address => |address| super::Balances::lookup_address(address),
-		validate_transaction => |tx| super::Executive::validate_transaction(tx)
-	);
-}
+impl_apis! {
+	impl Core<Block, SessionKey> for Runtime {
+		fn version() -> RuntimeVersion {
+			VERSION
+		}
 
-/// Produces the list of inherent extrinsics.
-fn inherent_extrinsics(data: InherentData) -> Vec<UncheckedExtrinsic> {
-	let mut inherent = vec![generic::UncheckedMortalExtrinsic::new_unsigned(
-		Call::Timestamp(TimestampCall::set(data.timestamp))
-	)];
+		fn authorities() -> Vec<SessionKey> {
+			Consensus::authorities()
+		}
 
-	if !data.offline_indices.is_empty() {
-		inherent.push(generic::UncheckedMortalExtrinsic::new_unsigned(
-			Call::Consensus(ConsensusCall::note_offline(data.offline_indices))
-		));
+		fn execute_block(block: Block) {
+			Executive::execute_block(block)
+		}
 	}
 
-	inherent
+	impl Metadata for Runtime {
+		fn metadata() -> Vec<u8> {
+			Runtime::metadata()
+		}
+	}
+
+	impl BlockBuilder<Block, InherentData, UncheckedExtrinsic> for Runtime {
+		fn initialise_block(header: <Block as BlockT>::Header) {
+			Executive::initialise_block(&header)
+		}
+
+		fn apply_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyResult {
+			Executive::apply_extrinsic(extrinsic)
+		}
+
+		fn finalise_block() -> <Block as BlockT>::Header {
+			Executive::finalise_block()
+		}
+
+		fn inherent_extrinsics(data: InherentData) -> Vec<UncheckedExtrinsic> {
+			let mut inherent = vec![generic::UncheckedMortalExtrinsic::new_unsigned(
+				Call::Timestamp(TimestampCall::set(data.timestamp))
+			)];
+
+			if !data.offline_indices.is_empty() {
+				inherent.push(generic::UncheckedMortalExtrinsic::new_unsigned(
+					Call::Consensus(ConsensusCall::note_offline(data.offline_indices))
+				));
+			}
+
+			inherent
+		}
+
+		fn random_seed() -> <Block as BlockT>::Hash {
+			System::random_seed()
+		}
+	}
+
+	impl OldTxQueue<AccountId, Index, Address, AccountId> for Runtime {
+		fn account_nonce(account: AccountId) -> Index {
+			System::account_nonce(&account)
+		}
+
+		fn lookup_address(address: Address) -> Option<AccountId> {
+			Balances::lookup_address(address)
+		}
+	}
+
+	impl NewTxQueue<Block, TransactionValidity> for Runtime {
+		fn validate_transaction(tx: <Block as BlockT>::Extrinsic) -> TransactionValidity {
+			Executive::validate_transaction(tx)
+		}
+	}
+
+	impl Miscellaneous<AccountId, u64> for Runtime {
+		fn validator_count() -> u32 {
+			Session::validator_count()
+		}
+
+		fn validators() -> Vec<AccountId> {
+			Session::validators()
+		}
+
+		fn timestamp() -> u64 {
+			Timestamp::get()
+		}
+	}
 }

substrate/node/runtime/wasm/Cargo.lock

@@ -268,6 +268,7 @@ dependencies = [
  "parity-codec 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-codec-derive 2.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "safe-mix 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "sr-api 0.1.0",
  "sr-io 0.1.0",
  "sr-primitives 0.1.0",
  "sr-std 0.1.0",
@@ -523,6 +524,16 @@ dependencies = [
  "unreachable 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "sr-api"
+version = "0.1.0"
+dependencies = [
+ "parity-codec 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "sr-primitives 0.1.0",
+ "sr-std 0.1.0",
+ "sr-version 0.1.0",
+]
+
 [[package]]
 name = "sr-io"
 version = "0.1.0"
@@ -550,6 +561,7 @@ dependencies = [
  "serde_derive 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)",
  "sr-io 0.1.0",
  "sr-std 0.1.0",
+ "sr-version 0.1.0",
  "substrate-primitives 0.1.0",
 ]
 
@@ -561,7 +573,7 @@ dependencies = [
  "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "sr-std 0.1.0",
  "substrate-primitives 0.1.0",
- "wasmi 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "wasmi 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -851,7 +863,7 @@ dependencies = [
  "twox-hash 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "uint 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "untrusted 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "wasmi 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "wasmi 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -968,7 +980,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "wasmi"
-version = "0.4.0"
+version = "0.4.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "byteorder 1.2.6 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1074,7 +1086,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum untrusted 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f392d7819dbe58833e26872f5f6f0d68b7bbbe90fc3667e98731c4a15ad9a7ae"
 "checksum version_check 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "914b1a6776c4c929a602fafd8bc742e06365d4bcbe48c30f9cca5824f70dc9dd"
 "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
-"checksum wasmi 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "522fe3fdd44a56f25cd5ddcd8ccdb1cf2e982ceb28fcb00f41d8a018ae5245a8"
+"checksum wasmi 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "d184c4b7081f30316f74f8d73c197314dcb56ea7af9323522b42a2fa9cb19453"
 "checksum winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "92c1eb33641e276cfa214a0522acad57be5c56b10cb348b3c5117db75f3ac4b0"
 "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
 "checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"

substrate/node/runtime/wasm/Cargo.toml

@@ -12,6 +12,7 @@ safe-mix = { version = "1.0", default-features = false}
 parity-codec-derive = { version = "2.0" }
 parity-codec = { version = "2.0", default-features = false }
 substrate-primitives = { path = "../../../core/primitives", default-features = false }
+sr-api = { path = "../../../core/sr-api", default-features = false }
 sr-std = { path = "../../../core/sr-std", default-features = false }
 sr-io = { path = "../../../core/sr-io", default-features = false }
 srml-support = { path = "../../../srml/support", default-features = false }
@@ -36,6 +37,7 @@ std = [
 	"safe-mix/std",
 	"parity-codec/std",
 	"substrate-primitives/std",
+	"sr-api/std",
 	"sr-std/std",
 	"sr-io/std",
 	"srml-support/std",

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

@@ -41,6 +41,7 @@ use std::{
 
 use codec::{Decode, Encode};
 use client::{Client as SubstrateClient, CallExecutor};
+use client::runtime_api::OldTxQueue;
 use transaction_pool::{Readiness, scoring::{Change, Choice}, VerifiedFor, ExtrinsicFor};
 use primitives::{AccountId, Hash, Index};
 use runtime::{Address, UncheckedExtrinsic};
@@ -62,13 +63,11 @@ pub trait Client:
 	Send
 	+ Sync
 	+ CurrentHeight<BlockNumber=<<<Self as Client>::Block as BlockT>::Header as HeaderT>::Number>
-	+ BlockNumberToHash<BlockNumber=<<<Self as Client>::Block as BlockT>::Header as HeaderT>::Number, Hash=<<Self as Client>::Block as BlockT>::Hash> {
+	+ BlockNumberToHash<BlockNumber=<<<Self as Client>::Block as BlockT>::Header as HeaderT>::Number, Hash=<<Self as Client>::Block as BlockT>::Hash>
+	+ OldTxQueue<<Self as Client>::Block>
+{
 	/// The block used for this API type.
 	type Block: BlockT;
-	/// Get the nonce (né index) of an account at a block.
-	fn index(&self, at: &BlockId<Self::Block>, account: AccountId) -> Result<u64>;
-	/// Get the account id of an address at a block.
-	fn lookup(&self, at: &BlockId<Self::Block>, address: Address) -> Result<Option<AccountId>>;
 }
 
 impl<B, E, Block> Client for SubstrateClient<B, E, Block> where
@@ -77,14 +76,6 @@ impl<B, E, Block> Client for SubstrateClient<B, E, Block> where
 	Block: BlockT,
 {
 	type Block = Block;
-
-	fn index(&self, at: &BlockId<Block>, account: AccountId) -> Result<u64> {
-		self.call_api_at(at, "account_nonce", &account).map_err(Into::into)
-	}
-
-	fn lookup(&self, at: &BlockId<Block>, address: Address) -> Result<Option<AccountId>> {
-		self.call_api_at(at, "lookup_address", &address).map_err(Into::into)
-	}
 }
 
 /// Type alias for the transaction pool.
@@ -171,7 +162,7 @@ impl<'a, C: 'a + Client> Lookup for LocalContext<'a, C> {
 	type Source = Address;
 	type Target = AccountId;
 	fn lookup(&self, a: Address) -> ::std::result::Result<AccountId, &'static str> {
-		self.0.lookup(&BlockId::number(self.current_height()), a).unwrap_or(None).ok_or("error with lookup")
+		self.0.lookup_address(&BlockId::number(self.current_height()), a).unwrap_or(None).ok_or("error with lookup")
 	}
 }
 
@@ -232,7 +223,7 @@ impl<C: Client> transaction_pool::ChainApi for ChainApi<C> {
 		// transaction-pool trait.
 		let api = &self.api;
 		let next_index = known_nonces.entry(sender)
-			.or_insert_with(|| api.index(at, sender).ok().unwrap_or_else(Bounded::max_value));
+			.or_insert_with(|| api.account_nonce(at, sender).ok().unwrap_or_else(Bounded::max_value));
 
 		trace!(target: "transaction-pool", "Next index for sender is {}; xt index is {}", next_index, xt.verified.index);