Runtime API clean up (#892)

* Rename `NewTxQueue` to `TaggedTransactionQueue`

* Remove `BlockBuilder` API and support adding documentation to the API functions

* Adds new `BlockBuilder` API

* Fixes compilation with new `BlockBuilder` API

* Some more cleanup

* Cargo.lock update

* Try to fix on stable

substrate/Cargo.lock

@@ -608,7 +608,7 @@ dependencies = [
  "futures 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-codec 2.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "parity-codec-derive 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parity-codec-derive 2.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

substrate/core/client/src/block_builder.rs

@@ -17,20 +17,22 @@
 //! Utility struct to build a block.
 
 use std::vec::Vec;
-use codec::{Decode, Encode};
-use state_machine::{self, native_when_possible};
+use std::marker::PhantomData;
+use codec::Encode;
+use state_machine;
 use runtime_primitives::traits::{Header as HeaderT, Hash, Block as BlockT, One, HashFor};
 use runtime_primitives::generic::BlockId;
+use runtime_api::BlockBuilder as BlockBuilderAPI;
 use {backend, error, Client, CallExecutor};
-use runtime_primitives::{ApplyResult, ApplyOutcome};
+use runtime_primitives::ApplyOutcome;
 use primitives::{Blake2Hasher};
 use hash_db::Hasher;
 
 /// Utility for building new (valid) blocks from a stream of extrinsics.
-pub struct BlockBuilder<B, E, Block, H>
+pub struct BlockBuilder<'a, B, E, Block, H>
 where
-	B: backend::Backend<Block, H>,
-	E: CallExecutor<Block, H> + Clone,
+	B: backend::Backend<Block, H> + 'a,
+	E: CallExecutor<Block, H> + Clone + 'a,
 	Block: BlockT,
 	H: Hasher,
 	H::Out: Ord,
@@ -38,25 +40,26 @@ where
 {
 	header: <Block as BlockT>::Header,
 	extrinsics: Vec<<Block as BlockT>::Extrinsic>,
-	executor: E,
-	state: B::State,
+	client: &'a Client<B, E, Block>,
+	block_id: BlockId<Block>,
 	changes: state_machine::OverlayedChanges,
+	_marker: PhantomData<H>,
 }
 
-impl<B, E, Block> BlockBuilder<B, E, Block, Blake2Hasher>
+impl<'a, B, E, Block> BlockBuilder<'a, B, E, Block, Blake2Hasher>
 where
-	B: backend::Backend<Block, Blake2Hasher>,
-	E: CallExecutor<Block, Blake2Hasher> + Clone,
+	B: backend::Backend<Block, Blake2Hasher> + 'a,
+	E: CallExecutor<Block, Blake2Hasher> + Clone + 'a,
 	Block: BlockT,
 {
 	/// Create a new instance of builder from the given client, building on the latest block.
-	pub fn new(client: &Client<B, E, Block>) -> error::Result<Self> {
+	pub fn new(client: &'a Client<B, E, Block>) -> error::Result<Self> {
 		client.info().and_then(|i| Self::at_block(&BlockId::Hash(i.chain.best_hash), client))
 	}
 
 	/// Create a new instance of builder from the given client using a particular block's ID to
 	/// build upon.
-	pub fn at_block(block_id: &BlockId<Block>, client: &Client<B, E, Block>) -> error::Result<Self> {
+	pub fn at_block(block_id: &BlockId<Block>, client: &'a Client<B, E, Block>) -> error::Result<Self> {
 		let number = client.block_number_from_id(block_id)?
 			.ok_or_else(|| error::ErrorKind::UnknownBlock(format!("{}", block_id)))?
 			+ One::one();
@@ -64,8 +67,6 @@ where
 		let parent_hash = client.block_hash_from_id(block_id)?
 			.ok_or_else(|| error::ErrorKind::UnknownBlock(format!("{}", block_id)))?;
 
-		let executor = client.executor().clone();
-		let state = client.state_at(block_id)?;
 		let mut changes = Default::default();
 		let header = <<Block as BlockT>::Header as HeaderT>::new(
 			number,
@@ -75,15 +76,16 @@ where
 			Default::default()
 		);
 
-		executor.call_at_state(&state, &mut changes, "initialise_block", &header.encode(), native_when_possible())?;
+		client.initialise_block(block_id, &mut changes, &header)?;
 		changes.commit_prospective();
 
 		Ok(BlockBuilder {
 			header,
 			extrinsics: Vec::new(),
-			executor,
-			state,
+			client,
+			block_id: *block_id,
 			changes,
+			_marker: Default::default(),
 		})
 	}
 
@@ -91,22 +93,18 @@ where
 	/// can be validly executed (by executing it); if it is invalid, it'll be returned along with
 	/// the error. Otherwise, it will return a mutable reference to self (in order to chain).
 	pub fn push(&mut self, xt: <Block as BlockT>::Extrinsic) -> error::Result<()> {
-		match self.executor.call_at_state(&self.state, &mut self.changes, "apply_extrinsic", &xt.encode(), native_when_possible()) {
-			Ok((result, _, _)) => {
-				match ApplyResult::decode(&mut result.as_slice()) {
-					Some(Ok(ApplyOutcome::Success)) | Some(Ok(ApplyOutcome::Fail)) => {
+		match self.client.apply_extrinsic(&self.block_id, &mut self.changes, &xt) {
+			Ok(result) => {
+				match result {
+					Ok(ApplyOutcome::Success) | Ok(ApplyOutcome::Fail) => {
 						self.extrinsics.push(xt);
 						self.changes.commit_prospective();
 						Ok(())
 					}
-					Some(Err(e)) => {
+					Err(e) => {
 						self.changes.discard_prospective();
 						Err(error::ErrorKind::ApplyExtinsicFailed(e).into())
 					}
-					None => {
-						self.changes.discard_prospective();
-						Err(error::ErrorKind::CallResultDecode("apply_extrinsic").into())
-					}
 				}
 			}
 			Err(e) => {
@@ -118,15 +116,7 @@ where
 
 	/// Consume the builder to return a valid `Block` containing all pushed extrinsics.
 	pub fn bake(mut self) -> error::Result<Block> {
-		let (output, _, _) = self.executor.call_at_state(
-			&self.state,
-			&mut self.changes,
-			"finalise_block",
-			&[],
-			native_when_possible(),
-		)?;
-		self.header = <<Block as BlockT>::Header as Decode>::decode(&mut &output[..])
-			.expect("Header came straight out of runtime so must be valid");
+		self.header = self.client.finalise_block(&self.block_id, &mut self.changes)?;
 
 		debug_assert_eq!(
 			self.header.extrinsics_root().clone(),

substrate/core/client/src/client.rs

@@ -17,7 +17,7 @@
 //! Substrate Client
 
 use std::sync::Arc;
-use error::Error;
+use error::{Error, ErrorKind};
 use futures::sync::mpsc;
 use parking_lot::{Mutex, RwLock};
 use primitives::AuthorityId;
@@ -32,7 +32,7 @@ use codec::{Encode, Decode};
 use state_machine::{
 	Backend as StateBackend, CodeExecutor,
 	ExecutionStrategy, ExecutionManager, prove_read,
-	key_changes, key_changes_proof,
+	key_changes, key_changes_proof, OverlayedChanges
 };
 
 use backend::{self, BlockImportOperation};
@@ -441,37 +441,43 @@ impl<B, E, Block> Client<B, E, Block> where
 				.ok_or_else(|| error::ErrorKind::UnknownBlock(format!("{:?}", parent)))?,
 			Default::default()
 		);
-		self.state_at(&parent).and_then(|state| {
-			let mut overlay = Default::default();
-			let execution_manager = || match self.api_execution_strategy {
-				ExecutionStrategy::NativeWhenPossible => ExecutionManager::NativeWhenPossible,
-				ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm,
-				ExecutionStrategy::Both => ExecutionManager::Both(|wasm_result, native_result| {
-					warn!("Consensus error between wasm and native runtime execution at block {:?}", at);
-					warn!("   Function {:?}", function);
-					warn!("   Native result {:?}", native_result);
-					warn!("   Wasm result {:?}", wasm_result);
-					wasm_result
-				}),
-			};
-			self.executor().call_at_state(
-				&state,
-				&mut overlay,
-				"initialise_block",
-				&header.encode(),
-				execution_manager()
-			)?;
-			let (r, _, _) = args.using_encoded(|input|
-				self.executor().call_at_state(
-				&state,
-				&mut overlay,
-				function,
-				input,
-				execution_manager()
-			))?;
-			Ok(R::decode(&mut &r[..])
-			   .ok_or_else(|| error::Error::from(error::ErrorKind::CallResultDecode(function)))?)
-		})
+		let mut overlay = Default::default();
+
+		self.call_at_state(at, "initialise_block", &header, &mut overlay)?;
+		self.call_at_state(at, function, args, &mut overlay)
+	}
+
+	fn call_at_state<A: Encode, R: Decode>(
+		&self,
+		at: &BlockId<Block>,
+		function: &'static str,
+		args: &A,
+		changes: &mut OverlayedChanges
+	) -> error::Result<R> {
+		let state = self.state_at(at)?;
+
+		let execution_manager = || match self.api_execution_strategy {
+			ExecutionStrategy::NativeWhenPossible => ExecutionManager::NativeWhenPossible,
+			ExecutionStrategy::AlwaysWasm => ExecutionManager::AlwaysWasm,
+			ExecutionStrategy::Both => ExecutionManager::Both(|wasm_result, native_result| {
+				warn!("Consensus error between wasm and native runtime execution at block {:?}", at);
+				warn!("   Function {:?}", function);
+				warn!("   Native result {:?}", native_result);
+				warn!("   Wasm result {:?}", wasm_result);
+				wasm_result
+			}),
+		};
+
+		self.executor.call_at_state(
+			&state,
+			changes,
+			function,
+			&args.encode(),
+			execution_manager()
+		).and_then(|res|
+			R::decode(&mut &res.0[..])
+				.ok_or_else(|| Error::from(ErrorKind::CallResultDecode(function)))
+		)
 	}
 
 	/// Check a header's justification.
@@ -1060,7 +1066,7 @@ impl<B, E, Block> api::Core<Block, AuthorityId> for Client<B, E, Block> where
 		bft::Authorities::authorities(self, at).map_err(Into::into)
 	}
 
-	fn execute_block(&self, at: &BlockId<Block>, block: Block) -> Result<(), Self::Error> {
+	fn execute_block(&self, at: &BlockId<Block>, block: &Block) -> Result<(), Self::Error> {
 		self.call_api_at(at, "execute_block", &(block))
 	}
 }
@@ -1083,21 +1089,36 @@ impl<B, E, Block> api::BlockBuilder<Block> for Client<B, E, Block> where
 	Block: BlockT,
 {
 	type Error = Error;
+	type OverlayedChanges = OverlayedChanges;
 
-	fn initialise_block(&self, at: &BlockId<Block>, header: <Block as BlockT>::Header) -> Result<(), Self::Error> {
-		self.call_api_at(at, "initialise_block", &(header))
+	fn initialise_block(
+		&self,
+		at: &BlockId<Block>,
+		changes: &mut OverlayedChanges,
+		header: &<Block as BlockT>::Header
+	) -> Result<(), Self::Error> {
+		self.call_at_state(at, "initialise_block", header, changes)
 	}
 
-	fn apply_extrinsic(&self, at: &BlockId<Block>, extrinsic: <Block as BlockT>::Extrinsic) -> Result<ApplyResult, Self::Error> {
-		self.call_api_at(at, "apply_extrinsic", &(extrinsic))
+	fn apply_extrinsic(
+		&self,
+		at: &BlockId<Block>,
+		changes: &mut OverlayedChanges,
+		extrinsic: &<Block as BlockT>::Extrinsic
+	) -> Result<ApplyResult, Self::Error> {
+		self.call_at_state(at, "apply_extrinsic", extrinsic, changes)
 	}
 
-	fn finalise_block(&self, at: &BlockId<Block>) -> Result<<Block as BlockT>::Header, Self::Error> {
-		self.call_api_at(at, "finalise_block", &())
+	fn finalise_block(
+		&self,
+		at: &BlockId<Block>,
+		changes: &mut OverlayedChanges
+	) -> Result<<Block as BlockT>::Header, Self::Error> {
+		self.call_at_state(at, "finalise_block", &(), changes)
 	}
 
 	fn inherent_extrinsics<InherentExtrinsic: Encode + Decode, UncheckedExtrinsic: Encode + Decode>(
-		&self, at: &BlockId<Block>, inherent: InherentExtrinsic
+		&self, at: &BlockId<Block>, inherent: &InherentExtrinsic
 	) -> Result<Vec<UncheckedExtrinsic>, Self::Error> {
 		self.call_api_at(at, "inherent_extrinsics", &(inherent))
 	}
@@ -1115,19 +1136,19 @@ impl<B, E, Block> api::OldTxQueue<Block> for Client<B, E, Block> where
 	type Error = Error;
 
 	fn account_nonce<AccountId: Encode + Decode, Index: Encode + Decode>(
-		&self, at: &BlockId<Block>, account: AccountId
+		&self, at: &BlockId<Block>, account: &AccountId
 	) -> Result<Index, Self::Error> {
 		self.call_api_at(at, "account_nonce", &(account))
 	}
 
 	fn lookup_address<Address: Encode + Decode, AccountId: Encode + Decode>(
-		&self, at: &BlockId<Block>, address: Address
+		&self, at: &BlockId<Block>, address: &Address
 	) -> Result<Option<AccountId>, Self::Error> {
 		self.call_api_at(at, "lookup_address", &(address))
 	}
 }
 
-impl<B, E, Block> api::NewTxQueue<Block> for Client<B, E, Block> where
+impl<B, E, Block> api::TaggedTransactionQueue<Block> for Client<B, E, Block> where
 	B: backend::Backend<Block, Blake2Hasher>,
 	E: CallExecutor<Block, Blake2Hasher>,
 	Block: BlockT,
@@ -1135,7 +1156,7 @@ impl<B, E, Block> api::NewTxQueue<Block> for Client<B, E, Block> where
 	type Error = Error;
 
 	fn validate_transaction<TransactionValidity: Encode + Decode>(
-		&self, at: &BlockId<Block>, tx: <Block as BlockT>::Extrinsic
+		&self, at: &BlockId<Block>, tx: &<Block as BlockT>::Extrinsic
 	) -> Result<TransactionValidity, Self::Error> {
 		self.call_api_at(at, "validate_transaction", &(tx))
 	}

substrate/core/sr-api/src/lib.rs

@@ -25,7 +25,7 @@ pub extern crate parity_codec as codec;
 extern crate sr_version as runtime_version;
 
 #[doc(hidden)]
-pub use primitives::{ApplyResult, traits::Block as BlockT, generic::BlockId};
+pub use primitives::{traits::Block as BlockT, generic::BlockId, ApplyResult};
 use runtime_version::RuntimeVersion;
 use rstd::vec::Vec;
 #[doc(hidden)]
@@ -39,8 +39,12 @@ pub use codec::{Encode, Decode};
 ///
 /// ```nocompile
 /// decl_apis!{
-///     pub trait Test<Event> {
+///     pub trait Test<Event> ExtraClientSide<ClientArg> {
 ///         fn test<AccountId>(event: Event) -> AccountId;
+///
+///         /// A function that will have the extra parameter `param` on the client side,
+///         /// the runtime does not have any parameter.
+///         fn testWithExtraParams() ExtraClientSide(param: &Self::ClientArg);
 ///     }
 /// }
 /// ```
@@ -56,7 +60,9 @@ pub use codec::{Encode, Decode};
 ///
 /// pub trait Test<Block: BlockT, Event> {
 ///     type Error;
+///     type ClientArg;
 ///     fn test<AccountId: Encode + Decode>(&self, at: &BlockId<Block>, event: Event) -> Result<Event, Self::Error>;
+///     fn testWithExtraParams(&self, at: &BlockId<Block>, param: &Client) -> Result<Event, Self::Error>;
 /// }
 /// ```
 ///
@@ -68,11 +74,16 @@ macro_rules! decl_apis {
 	(
 		$(
 			$( #[$attr:meta] )*
-			pub trait $name:ident $(< $( $generic_param:ident $( : $generic_bound:ident )* ),* >)* {
+			pub trait $name:ident $(< $( $generic_param:ident $( : $generic_bound:ident )* ),* >)*
+				$( ExtraClientSide < $( $client_generic_param:ident $( : $client_generic_bound:ident )* ),+ > )*
+			{
 				$(
+					$( #[$fn_attr:meta] )*
 					fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-						$( $param_name:ident : $param_type:ty )*
-					) $( -> $return_ty:ty)*;
+						$( $param_name:ident : $param_type:ty ),*
+					)
+					$( ExtraClientSide ( $( $client_param_name:ident : $client_param_type:ty ),+ ) )*
+					$( -> $return_ty:ty)*;
 				)*
 			}
 		)*
@@ -82,8 +93,13 @@ macro_rules! decl_apis {
 				@ADD_BLOCK_GENERIC
 				$( #[$attr] )*
 				pub trait $name $(< $( $generic_param $( : $generic_bound )* ),* >)* {
+					$( $( type $client_generic_param $( : $client_generic_bound )*; )* )*
 					$(
-						fn $fn_name $( < $( $fn_generic ),* > )* ($( $param_name : $param_type )* ) $( -> $return_ty )*;
+						$( #[$fn_attr] )*
+						fn $fn_name $( < $( $fn_generic ),* > )* (
+							$( $( $client_param_name: $client_param_type, )* )*
+							$( $param_name : &$param_type, )*
+						) $( -> $return_ty )*;
 					)*
 				};
 				;
@@ -97,6 +113,7 @@ macro_rules! decl_apis {
 				$( #[$attr] )*
 				pub trait $name $(< $( $generic_param $( : $generic_bound )* ),* >)* {
 					$(
+						$( #[$fn_attr] )*
 						fn $fn_name $( < $( $fn_generic ),* > )* ($( $param_name : $param_type )* ) $( -> $return_ty )*;
 					)*
 				};
@@ -106,9 +123,11 @@ macro_rules! decl_apis {
 	(@ADD_BLOCK_GENERIC
 		$( #[$attr:meta] )*
 		pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty)*;
 			)*
 		};
@@ -121,9 +140,11 @@ macro_rules! decl_apis {
 			@ADD_BLOCK_GENERIC
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
@@ -135,9 +156,11 @@ macro_rules! decl_apis {
 	(@ADD_BLOCK_GENERIC
 		$( #[$attr:meta] )*
 		pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 	 		$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty )*;
 			)*
 		};
@@ -150,9 +173,11 @@ macro_rules! decl_apis {
 			@ADD_BLOCK_GENERIC
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
@@ -164,9 +189,11 @@ macro_rules! decl_apis {
 	(@ADD_BLOCK_GENERIC
 		$( #[$attr:meta] )*
 		pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty )*;
 			)*
 		};
@@ -174,12 +201,14 @@ macro_rules! decl_apis {
 	 	$( $generic_param_parsed:ident $( : $generic_bound_parsed:path )* ),*;
 	) => {
 		decl_apis!(
-			@GENERATE_FNS
+			@GENERATE_RETURN_TYPES
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
@@ -191,9 +220,11 @@ macro_rules! decl_apis {
 	(@ADD_BLOCK_GENERIC
 		$( #[$attr:meta] )*
 		pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty )*;
 			)*
 		};
@@ -201,12 +232,14 @@ macro_rules! decl_apis {
 		$( $generic_param_parsed:ident $( : $generic_bound_parsed:ident )* ),*;
 	) => {
 		decl_apis!(
-			@GENERATE_FNS
+			@GENERATE_RETURN_TYPES
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
@@ -216,12 +249,14 @@ macro_rules! decl_apis {
 			$( $( $return_ty )*; )*
 		);
 	};
-	(@GENERATE_FNS
+	(@GENERATE_RETURN_TYPES
         $( #[$attr:meta] )*
         pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty)*;
 			)*
         };
@@ -231,12 +266,14 @@ macro_rules! decl_apis {
 		$( $( $return_ty_rest:ty )*; )*
 	) => {
 		decl_apis!(
-			@GENERATE_FNS
+			@GENERATE_RETURN_TYPES
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
@@ -245,12 +282,14 @@ macro_rules! decl_apis {
 			$( $( $return_ty_rest )*; )*
 		);
 	};
-	(@GENERATE_FNS
+	(@GENERATE_RETURN_TYPES
         $( #[$attr:meta] )*
         pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty)*;
 			)*
         };
@@ -260,12 +299,14 @@ macro_rules! decl_apis {
 		$( $( $return_ty_rest:ty )*; )*
 	) => {
 		decl_apis!(
-			@GENERATE_FNS
+			@GENERATE_RETURN_TYPES
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
@@ -274,12 +315,14 @@ macro_rules! decl_apis {
 			$( $( $return_ty_rest )*; )*
 		);
 	};
-	(@GENERATE_FNS
+	(@GENERATE_RETURN_TYPES
         $( #[$attr:meta] )*
         pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty)*;
 			)*
         };
@@ -290,35 +333,40 @@ macro_rules! decl_apis {
 			@GENERATE_CLIENT_TRAITS
 			$( #[$attr] )*
 			pub trait $name $(< $( $generic_param_orig $( : $generic_bound_orig )* ),* >)* {
+				$( type $client_generic_param $( : $client_generic_bound )*; )*
 				$(
+					$( #[$fn_attr] )*
 					fn $fn_name $( < $( $fn_generic ),* > )* (
-						$( $param_name : $param_type )*
+						$( $param_name : $param_type, )*
 					) $( -> $return_ty )*;
 				)*
 			};
 			$( $generic_param_parsed $( : $generic_bound_parsed )* ),*;
 			{ $( $result_return_ty; )* };
-			$( $( $generic_param_orig $( : $generic_bound_orig )*, )* )* $( $( $( $fn_generic, )* )* )*;
 		);
 	};
 	(@GENERATE_CLIENT_TRAITS
 		$( #[$attr:meta] )*
 		pub trait $name:ident $(< $( $generic_param_orig:ident $( : $generic_bound_orig:ident )* ),* >)* {
+			$( type $client_generic_param:ident $( : $client_generic_bound:ident )*; )*
 			$(
+				$( #[$fn_attr:meta] )*
 				fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
-					$( $param_name:ident : $param_type:ty )*
+					$( $param_name:ident : $param_type:ty, )*
 				) $( -> $return_ty:ty)*;
 			)*
 		};
 		$( $generic_param_parsed:ident $( : $generic_bound_parsed:path )* ),*;
 		{ $( $result_return_ty:ty; )* };
-		$( $generic_param_joined:ident $( : $generic_bound_joined:ident )*, )*;
 	) => {
 		$( #[$attr] )*
 		pub trait $name < $( $generic_param_parsed $( : $generic_bound_parsed )* ),* > {
+			/// The Error type returned by this API.
 			type Error;
+			$( type $client_generic_param $( : $client_generic_bound )*; )*
 
 			$(
+				$( #[$fn_attr] )*
 				fn $fn_name $( < $( $fn_generic: $crate::Encode + $crate::Decode ),* > )* (
 					&self, at: &$crate::BlockId<Block> $(, $param_name: $param_type )*
 				) -> $result_return_ty;
@@ -330,6 +378,7 @@ macro_rules! decl_apis {
 			$( #[$attr:meta] )*
 			pub trait $name:ident $(< $( $generic_param:ident $( : $generic_bound:ident )* ),* >)* {
 				$(
+					$( #[$fn_attr:meta] )*
 					fn $fn_name:ident $( < $( $fn_generic:ident ),* > )* (
 						$( $param_name:ident : $param_type:ty )*
 					) $( -> $return_ty:ty)*;
@@ -343,6 +392,7 @@ macro_rules! decl_apis {
 				$( #[$attr] )*
 				pub trait $name < $( $( $generic_param $( : $generic_bound )*, )* )* $( $( $( $fn_generic, )* )* )* > {
 					$(
+						$( #[$fn_attr] )*
 						fn $fn_name ($( $param_name: $param_type ),*) $( -> $return_ty )*;
 					)*
 				}
@@ -354,11 +404,13 @@ macro_rules! decl_apis {
 			$( #[$attr:meta] )*
 			pub trait $name:ident < $( $generic_param:ident $( : $generic_bound:ident )*, )* > {
 				$(
+					$( #[$fn_attr:meta] )*
 					fn $fn_name:ident($( $param_name:ident : $param_type:ty )*) $( -> $return_ty:ty)*;
 				)*
 			}
 		)*
 	) => {
+		/// The API traits to implement on the runtime side.
 		pub mod runtime {
 			use super::*;
 
@@ -366,6 +418,7 @@ macro_rules! decl_apis {
 				$( #[$attr] )*
 				pub trait $name < $( $generic_param $( : $generic_bound )* ),* > {
 					$(
+						$( #[$fn_attr] )*
 						fn $fn_name ($( $param_name: $param_type ),*) $( -> $return_ty )*;
 					)*
 				}
@@ -387,23 +440,14 @@ decl_apis! {
 		fn metadata() -> Vec<u8>;
 	}
 
-	/// The `BlockBuilder` api trait that provides required functions for building a block for a runtime.
-	pub trait BlockBuilder<Block: BlockT> {
-		fn initialise_block(header: <Block as BlockT>::Header);
-		fn apply_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyResult;
-		fn finalise_block() -> <Block as BlockT>::Header;
-		fn inherent_extrinsics<InherentExtrinsic, UncheckedExtrinsic>(inherent: InherentExtrinsic) -> Vec<UncheckedExtrinsic>;
-		fn random_seed() -> <Block as BlockT>::Hash;
-	}
-
 	/// The `OldTxQueue` api trait for interfering with the old transaction queue.
 	pub trait OldTxQueue {
 		fn account_nonce<AccountId, Index>(account: AccountId) -> Index;
 		fn lookup_address<Address, LookupId>(address: Address) -> Option<LookupId>;
 	}
 
-	/// The `NewTxQueue` api trait for interfering with the new transaction queue.
-	pub trait NewTxQueue<Block: BlockT> {
+	/// The `TaggedTransactionQueue` api trait for interfering with the new transaction queue.
+	pub trait TaggedTransactionQueue<Block: BlockT> {
 		fn validate_transaction<TransactionValidity>(tx: <Block as BlockT>::Extrinsic) -> TransactionValidity;
 	}
 
@@ -413,6 +457,20 @@ decl_apis! {
 		fn validators<AccountId>() -> Vec<AccountId>;
 		fn timestamp<Moment>() -> Moment;
 	}
+
+	/// The `BlockBuilder` api trait that provides required functions for building a block for a runtime.
+	pub trait BlockBuilder<Block: BlockT> ExtraClientSide <OverlayedChanges> {
+		/// Initialise a block with the given header.
+		fn initialise_block(header: <Block as BlockT>::Header) ExtraClientSide(changes: &mut Self::OverlayedChanges);
+		/// Apply the given extrinsics.
+		fn apply_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) ExtraClientSide(changes: &mut Self::OverlayedChanges) -> ApplyResult;
+		/// Finish the current block.
+		fn finalise_block() ExtraClientSide(changes: &mut Self::OverlayedChanges) -> <Block as BlockT>::Header;
+		/// Generate inherent extrinsics.
+		fn inherent_extrinsics<InherentExtrinsic, UncheckedExtrinsic>(inherent: InherentExtrinsic) -> Vec<UncheckedExtrinsic>;
+		/// Generate a random seed.
+		fn random_seed() -> <Block as BlockT>::Hash;
+	}
 }
 
 /// Implement the given API's for the given runtime.

substrate/core/test-client/src/block_builder_ext.rs

@@ -29,7 +29,7 @@ pub trait BlockBuilderExt {
 	fn push_transfer(&mut self, transfer: runtime::Transfer) -> Result<(), client::error::Error>;
 }
 
-impl<B, E> BlockBuilderExt for client::block_builder::BlockBuilder<B, E, runtime::Block, Blake2Hasher>
+impl<'a, B, E> BlockBuilderExt for client::block_builder::BlockBuilder<'a, B, E, runtime::Block, Blake2Hasher>
     where
         B: client::backend::Backend<runtime::Block, Blake2Hasher>,
         E: client::CallExecutor<runtime::Block, Blake2Hasher> + Clone,

substrate/core/test-runtime/Cargo.toml

@@ -34,5 +34,5 @@ std = [
 	"srml-support/std",
 	"substrate-primitives/std",
 	"sr-primitives/std",
-	"sr-version/std"
+	"sr-version/std",
 ]

substrate/node/consensus/src/lib.rs

@@ -46,7 +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 client::runtime_api::{Core, BlockBuilder as BlockBuilderAPI, Miscellaneous, OldTxQueue};
 use codec::{Decode, Encode};
 use node_primitives::{AccountId, Timestamp, SessionKey, InherentData};
 use node_runtime::Runtime;
@@ -81,9 +81,6 @@ const MAX_TRANSACTIONS_SIZE: usize = 4 * 1024 * 1024;
 pub trait BlockBuilder<Block: BlockT> {
 	/// Push an extrinsic onto the block. Fails if the extrinsic is invalid.
 	fn push_extrinsic(&mut self, extrinsic: <Block as BlockT>::Extrinsic) -> Result<()>;
-
-	/// Bake the block with provided extrinsics.
-	fn bake(self) -> Result<Block>;
 }
 
 /// Local client abstraction for the consensus.
@@ -97,16 +94,19 @@ pub trait AuthoringApi:
 {
 	/// The block used for this API type.
 	type Block: BlockT;
-	/// The block builder for this API type.
-	type BlockBuilder: BlockBuilder<Self::Block>;
 	/// 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>;
+	fn build_block<F: FnMut(&mut BlockBuilder<Self::Block>) -> ()>(
+		&self,
+		at: &BlockId<Self::Block>,
+		inherent_data: InherentData,
+		build_ctx: F,
+	) -> Result<Self::Block>;
 }
 
-impl<B, E, Block> BlockBuilder<Block> for client::block_builder::BlockBuilder<B, E, Block, Blake2Hasher> where
+impl<'a, B, E, Block> BlockBuilder<Block> for client::block_builder::BlockBuilder<'a, B, E, Block, Blake2Hasher> where
 	B: client::backend::Backend<Block, Blake2Hasher> + Send + Sync + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
 	Block: BlockT
@@ -114,32 +114,34 @@ impl<B, E, Block> BlockBuilder<Block> for client::block_builder::BlockBuilder<B,
 	fn push_extrinsic(&mut self, extrinsic: <Block as BlockT>::Extrinsic) -> Result<()> {
 		client::block_builder::BlockBuilder::push(self, extrinsic).map_err(Into::into)
 	}
-
-	fn bake(self) -> Result<Block> {
-		client::block_builder::BlockBuilder::bake(self).map_err(Into::into)
-	}
 }
 
-impl<B, E, Block> AuthoringApi for SubstrateClient<B, E, Block> where
+impl<'a, B, E, Block> AuthoringApi for SubstrateClient<B, E, Block> where
 	B: client::backend::Backend<Block, Blake2Hasher> + Send + Sync + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
 	Block: BlockT,
 {
 	type Block = Block;
-	type BlockBuilder = client::block_builder::BlockBuilder<B, E, Block, Blake2Hasher>;
 	type Error = client::error::Error;
 
-	fn build_block(&self, at: &BlockId<Block>, inherent_data: InherentData) -> Result<Self::BlockBuilder> {
+	fn build_block<F: FnMut(&mut BlockBuilder<Self::Block>) -> ()>(
+		&self,
+		at: &BlockId<Self::Block>,
+		inherent_data: InherentData,
+		mut build_ctx: F,
+	) -> Result<Self::Block> {
 		let runtime_version = self.runtime_version_at(at)?;
 
 		let mut block_builder = self.new_block_at(at)?;
 		if runtime_version.has_api(*b"inherent", 1) {
-			for inherent in self.inherent_extrinsics(at, inherent_data)? {
+			for inherent in self.inherent_extrinsics(at, &inherent_data)? {
 				block_builder.push(inherent)?;
 			}
 		}
 
-		Ok(block_builder)
+		build_ctx(&mut block_builder);
+
+		block_builder.bake().map_err(Into::into)
 	}
 }
 
@@ -300,34 +302,33 @@ impl<C> bft::Proposer<<C as AuthoringApi>::Block> for Proposer<C> where
 			offline_indices,
 		};
 
-		let mut block_builder = self.client.build_block(&self.parent_id, inherent_data)?;
+		let block = self.client.build_block(
+			&self.parent_id,
+			inherent_data,
+			|block_builder| {
+				let mut unqueue_invalid = Vec::new();
+				let result = self.transaction_pool.cull_and_get_pending(&BlockId::hash(self.parent_hash), |pending_iterator| {
+					let mut pending_size = 0;
+					for pending in pending_iterator {
+						if pending_size + pending.verified.encoded_size() >= MAX_TRANSACTIONS_SIZE { break }
 
-		{
-			let mut unqueue_invalid = Vec::new();
-			let result = self.transaction_pool.cull_and_get_pending(&BlockId::hash(self.parent_hash), |pending_iterator| {
-				let mut pending_size = 0;
-				for pending in pending_iterator {
-					if pending_size + pending.verified.encoded_size() >= MAX_TRANSACTIONS_SIZE { break }
-
-					match block_builder.push_extrinsic(pending.original.clone()) {
-						Ok(()) => {
-							pending_size += pending.verified.encoded_size();
-						}
-						Err(e) => {
-							trace!(target: "transaction-pool", "Invalid transaction: {}", e);
-							unqueue_invalid.push(pending.verified.hash().clone());
+						match block_builder.push_extrinsic(pending.original.clone()) {
+							Ok(()) => {
+								pending_size += pending.verified.encoded_size();
+							}
+							Err(e) => {
+								trace!(target: "transaction-pool", "Invalid transaction: {}", e);
+								unqueue_invalid.push(pending.verified.hash().clone());
+							}
 						}
 					}
+				});
+				if let Err(e) = result {
+					warn!("Unable to get the pending set: {:?}", e);
 				}
-			});
-			if let Err(e) = result {
-				warn!("Unable to get the pending set: {:?}", e);
-			}
 
-			self.transaction_pool.remove(&unqueue_invalid, false);
-		}
-
-		let block = block_builder.bake()?;
+				self.transaction_pool.remove(&unqueue_invalid, false);
+			})?;
 
 		info!("Proposing block [number: {}; hash: {}; parent_hash: {}; extrinsics: [{}]]",
 			  block.header().number(),
@@ -404,7 +405,7 @@ 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 = match self.client.execute_block(&self.parent_id, unchecked_proposal.clone()).map_err(Error::from) {
+		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),
@@ -445,7 +446,7 @@ 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(|| self.client.account_nonce(&self.parent_id, local_id))
+				.unwrap_or_else(|| self.client.account_nonce(&self.parent_id, &local_id))
 				.map_err(Error::from)
 			);
 

substrate/node/runtime/src/lib.rs

@@ -311,7 +311,7 @@ impl_apis! {
 		}
 	}
 
-	impl NewTxQueue<Block, TransactionValidity> for Runtime {
+	impl TaggedTransactionQueue<Block, TransactionValidity> for Runtime {
 		fn validate_transaction(tx: <Block as BlockT>::Extrinsic) -> TransactionValidity {
 			Executive::validate_transaction(tx)
 		}

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

@@ -162,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_address(&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")
 	}
 }
 
@@ -223,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.account_nonce(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);