start addressing basti comments

substrate/core/client/src/client.rs

@@ -916,7 +916,8 @@ impl<B, E, Block, RA> CallApiAt<Block> for Client<B, E, Block, RA> where
 	B: backend::Backend<Block, Blake2Hasher>,
 	E: CallExecutor<Block, Blake2Hasher> + Clone + Send + Sync,
 	Block: BlockT<Hash=H256>,
-	RA: Send + Sync,
+	RA: CoreAPI<Block>, // not strictly necessary at the moment
+						// but we want to bound to make sure the API is actually available.
 {
 	fn call_api_at(
 		&self,

substrate/core/finality-grandpa/primitives/src/lib.rs

@@ -54,27 +54,82 @@ pub mod id {
 	pub const GRANDPA_API: ApiId = *b"fgrandpa";
 }
 
-/// APIs for integrating the GRANDPA finality gadget into runtimes.
+decl_runtime_apis! {
-/// This should be implemented on the runtime side.
+	/// APIs for integrating the GRANDPA finality gadget into runtimes.
-///
+	/// This should be implemented on the runtime side.
-/// This is primarily used for negotiating authority-set changes for the
-/// gadget. GRANDPA uses a signalling model of changing authority sets:
-/// changes should be signalled with a delay of N blocks, and then automatically
-/// applied in the runtime after those N blocks have passed.
-///
-/// The consensus protocol will coordinate the handoff externally.
-pub trait GrandpaApi<B: BlockT> {
-	/// Check a digest for pending changes.
-	/// Return `None` if there are no pending changes.
 	///
-	/// Precedence towards earlier or later digest items can be given
+	/// This is primarily used for negotiating authority-set changes for the
-	/// based on the rules of the chain.
+	/// gadget. GRANDPA uses a signalling model of changing authority sets:
+	/// changes should be signalled with a delay of N blocks, and then automatically
+	/// applied in the runtime after those N blocks have passed.
 	///
-	/// No change should be scheduled if one is already and the delay has not
+	/// The consensus protocol will coordinate the handoff externally.
-	/// passed completely.
+	pub trait GrandpaApi<Block: BlockT> {
-	fn grandpa_pending_change(digest: DigestFor<B>) -> Option<ScheduledChange<NumberFor<B>>>;
+		/// Check a digest for pending changes.
+		/// Return `None` if there are no pending changes.
+		///
+		/// Precedence towards earlier or later digest items can be given
+		/// based on the rules of the chain.
+		///
+		/// No change should be scheduled if one is already and the delay has not
+		/// passed completely.
+		fn grandpa_pending_change(digest: DigestFor<Block>)
+			-> Option<ScheduledChange<NumberFor<Block>>>;
 
-	/// Get the current GRANDPA authorities and weights. This should not change except
+		/// Get the current GRANDPA authorities and weights. This should not change except
-	/// for when changes are scheduled and the corresponding delay has passed.
+		/// for when changes are scheduled and the corresponding delay has passed.
-	fn grandpa_authorities() -> Vec<(AuthorityId, u64)>;
+		fn grandpa_authorities() -> Vec<(AuthorityId, u64)>;
+	}
+}
+
+#[cfg(feature = "std")]
+mod implementation {
+	use super::{GrandpaApi, ScheduledChange};
+	use sr_primitives::traits::{Block as BlockT, DigestFor, NumberFor};
+	use sr_primitives::generic::BlockId;
+	use parity_codec::{Encode, Decode};
+	use client::{Client, error::Error as ClientError, backend::Backend, CallExecutor};
+	use client::runtime_api::{CallApiAt, Core as CoreAPI};
+	use substrate_primitives::{AuthorityId, H256, Blake2Hasher};
+
+	// TODO [basti]: do this implementation in runtime.
+	impl<B, E, Block: BlockT<Hash=H256>, RA> GrandpaApi<Block> for Client<B, E, Block, RA> where
+		B: Backend<Block, Blake2Hasher> + 'static,
+		E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
+		DigestFor<Block>: Encode,
+		RA: CoreAPI<Block>,
+	{
+		fn grandpa_authorities(&self, at: &BlockId<Block>) -> Result<Vec<(AuthorityId, u64)>, ClientError> {
+			let raw = self.call_api_at(
+				&at,
+				::AUTHORITIES_CALL,
+				Encode::encode(&()),
+				&mut Default::default(),
+				&mut None,
+			);
+
+			// TODO [basti]: implement this in runtime with macro.
+			match Decode::decode(&mut &raw[..]) {
+				Some(x) => Ok(x),
+				None => Err(::client::error::ErrorKind::CallResultDecode(::AUTHORITIES_CALL).into()),
+			}
+		}
+
+		fn grandpa_pending_change(&self, at: &BlockId<Block>, digest: DigestFor<Block>)
+			-> Result<Option<ScheduledChange<NumberFor<Block>>>, ClientError>
+		{
+			let raw = self.call_api_at(
+				at,
+				::PENDING_CHANGE_CALL,
+				digest.encode(),
+				&mut Default::default(),
+				&mut None,
+			);
+
+			match Decode::decode(&mut &raw[..]) {
+				Some(x) => Ok(x),
+				None => Err(::client::error::ErrorKind::CallResultDecode(::PENDING_CHANGE_CALL).into()),
+			}
+		}
+	}
 }

substrate/core/finality-grandpa/src/lib.rs

@@ -84,12 +84,13 @@ use futures::stream::Fuse;
 use futures::sync::mpsc;
 use client::{Client, error::Error as ClientError, ImportNotifications, backend::Backend, CallExecutor};
 use client::blockchain::HeaderBackend;
-use client::runtime_api::{CallApiAt, TaggedTransactionQueue};
+use client::runtime_api::{CallApiAt, TaggedTransactionQueue, Core as CoreAPI};
 use codec::{Encode, Decode};
 use consensus_common::{BlockImport, ImportBlock, ImportResult};
 use runtime_primitives::traits::{
 	NumberFor, Block as BlockT, Header as HeaderT, DigestFor,
 };
+use fg_primitives::GrandpaApi;
 use runtime_primitives::generic::BlockId;
 use substrate_primitives::{ed25519, H256, AuthorityId, Blake2Hasher};
 use tokio::timer::Interval;
@@ -765,46 +766,7 @@ impl<B, E, Block: BlockT<Hash=H256>, N, RA> voter::Environment<Block::Hash, Numb
 	}
 }
 
-/// Client side of the GRANDPA APIs declared in fg-primitives.
-pub trait ApiClient<Block: BlockT<Hash=H256>> {
-	/// Get the genesis authorities for GRANDPA.
-	fn genesis_authorities(&self) -> Result<Vec<(AuthorityId, u64)>, ClientError>;
 
-	/// Check a header's digest for a scheduled change.
-	fn scheduled_change(&self, header: &Block::Header)
-		-> Result<Option<ScheduledChange<NumberFor<Block>>>, ClientError>;
-}
-
-impl<B, E, Block: BlockT<Hash=H256>, RA> ApiClient<Block> for Arc<Client<B, E, Block, RA>> where
-	B: Backend<Block, Blake2Hasher> + 'static,
-	E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
-	DigestFor<Block>: Encode,
-	RA: Send + Sync,
-{
-	fn genesis_authorities(&self) -> Result<Vec<(AuthorityId, u64)>, ClientError> {
-		use runtime_primitives::traits::Zero;
-
-		self.call_api_at_strong(
-			&BlockId::Number(NumberFor::<Block>::zero()),
-			fg_primitives::AUTHORITIES_CALL,
-			&(),
-			&mut Default::default(),
-			&mut None,
-		)
-	}
-
-	fn scheduled_change(&self, header: &Block::Header)
-		-> Result<Option<ScheduledChange<NumberFor<Block>>>, ClientError>
-	{
-		self.call_api_at_strong(
-			&BlockId::hash(header.parent_hash().clone()),
-			fg_primitives::PENDING_CHANGE_CALL,
-			header.digest(),
-			&mut Default::default(),
-			&mut None,
-		)
-	}
-}
 
 /// A block-import handler for GRANDPA.
 ///
@@ -821,7 +783,7 @@ impl<B, E, Block: BlockT<Hash=H256>, Api, RA> BlockImport<Block> for GrandpaBloc
 	B: Backend<Block, Blake2Hasher> + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
 	DigestFor<Block>: Encode,
-	Api: ApiClient<Block>,
+	Api: GrandpaApi<Block>,
 	RA: TaggedTransactionQueue<Block> + Send + Sync, // necessary for client to import `BlockImport`.
 {
 	type Error = ClientError;
@@ -831,7 +793,10 @@ impl<B, E, Block: BlockT<Hash=H256>, Api, RA> BlockImport<Block> for GrandpaBloc
 	{
 		use authorities::PendingChange;
 
-		let maybe_change = self.api_client.scheduled_change(&block.header)?;
+		let maybe_change = self.api_client.grandpa_pending_change(
+			&BlockId::hash(*block.header.parent_hash()),
+			&block.header.digest().clone(),
+		)?;
 
 		// when we update the authorities, we need to hold the lock
 		// until the block is written to prevent a race if we need to restore
@@ -879,9 +844,10 @@ pub fn block_import<B, E, Block: BlockT<Hash=H256>, Api, RA>(client: Arc<Client<
 	where
 	B: Backend<Block, Blake2Hasher> + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + 'static,
-	Api: ApiClient<Block>,
+	Api: GrandpaApi<Block>,
 	RA: Send + Sync,
 {
+	use runtime_primitives::traits::Zero;
 	let authority_set = match client.backend().get_aux(AUTHORITY_SET_KEY)? {
 		None => {
 			info!(target: "afg", "Loading GRANDPA authorities \
@@ -890,7 +856,8 @@ pub fn block_import<B, E, Block: BlockT<Hash=H256>, Api, RA>(client: Arc<Client<
 			// no authority set on disk: fetch authorities from genesis state.
 			// if genesis state is not available, we may be a light client, but these
 			// are unsupported for following GRANDPA directly.
-			let genesis_authorities: Vec<(AuthorityId, u64)> = api_client.genesis_authorities()?;
+			let genesis_authorities = api_client
+				.grandpa_authorities(&BlockId::number(Zero::zero()))?;
 
 			let authority_set = SharedAuthoritySet::genesis(genesis_authorities);
 			let encoded = authority_set.inner().read().encode();