fix #2177: port GRANDPA node-side code to use Consensus digests (#3669)

* fix #2177: port GRANDPA node-side code to use Consensus digests

* bump runtime version

* fix service compilation

* document change precedence rules

substrate/Cargo.lock

@@ -4317,14 +4317,6 @@ dependencies = [
  "syn 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
-[[package]]
-name = "srml-staking-reward-curve-test"
-version = "2.0.0"
-dependencies = [
- "sr-primitives 2.0.0",
- "srml-staking-reward-curve 2.0.0",
-]
-
 [[package]]
 name = "srml-sudo"
 version = "2.0.0"

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

@@ -24,7 +24,7 @@ extern crate alloc;
 #[cfg(feature = "std")]
 use serde::Serialize;
 use codec::{Encode, Decode, Codec};
-use sr_primitives::{ConsensusEngineId, traits::{DigestFor, NumberFor}};
+use sr_primitives::ConsensusEngineId;
 use client::decl_runtime_apis;
 use rstd::vec::Vec;
 
@@ -74,8 +74,9 @@ pub struct ScheduledChange<N> {
 pub enum ConsensusLog<N: Codec> {
 	/// Schedule an authority set change.
 	///
-	/// Precedence towards earlier or later digest items can be given
-	/// based on the rules of the chain.
+	/// The earliest digest of this type in a single block will be respected,
+	/// provided that there is no `ForcedChange` digest. If there is, then the
+	/// `ForcedChange` will take precedence.
 	///
 	/// No change should be scheduled if one is already and the delay has not
 	/// passed completely.
@@ -90,8 +91,8 @@ pub enum ConsensusLog<N: Codec> {
 	/// Forced changes are applied after a delay of _imported_ blocks,
 	/// while pending changes are applied after a delay of _finalized_ blocks.
 	///
-	/// Precedence towards earlier or later digest items can be given
-	/// based on the rules of the chain.
+	/// The earliest digest of this type in a single block will be respected,
+	/// with others ignored.
 	///
 	/// No change should be scheduled if one is already and the delay has not
 	/// passed completely.
@@ -165,43 +166,6 @@ decl_runtime_apis! {
 	/// The consensus protocol will coordinate the handoff externally.
 	#[api_version(2)]
 	pub trait GrandpaApi {
-		/// 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.
-		///
-		/// This should be a pure function: i.e. as long as the runtime can interpret
-		/// the digest type it should return the same result regardless of the current
-		/// state.
-		fn grandpa_pending_change(digest: &DigestFor<Block>)
-			-> Option<ScheduledChange<NumberFor<Block>>>;
-
-		/// Check a digest for forced changes.
-		/// Return `None` if there are no forced changes. Otherwise, return a
-		/// tuple containing the pending change and the median last finalized
-		/// block number at the time the change was signaled.
-		///
-		/// Added in version 2.
-		///
-		/// Forced changes are applied after a delay of _imported_ blocks,
-		/// while pending changes are applied after a delay of _finalized_ blocks.
-		///
-		/// 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.
-		///
-		/// This should be a pure function: i.e. as long as the runtime can interpret
-		/// the digest type it should return the same result regardless of the current
-		/// state.
-		fn grandpa_forced_change(digest: &DigestFor<Block>)
-			-> Option<(NumberFor<Block>, ScheduledChange<NumberFor<Block>>)>;
-
 		/// Get the current GRANDPA authorities and weights. This should not change except
 		/// for when changes are scheduled and the corresponding delay has passed.
 		///

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

@@ -24,19 +24,17 @@ use parking_lot::RwLockWriteGuard;
 use client::{blockchain, CallExecutor, Client, well_known_cache_keys};
 use client::blockchain::HeaderBackend;
 use client::backend::Backend;
-use client::runtime_api::ApiExt;
 use client::utils::is_descendent_of;
 use consensus_common::{
 	BlockImport, Error as ConsensusError,
 	BlockImportParams, ImportResult, JustificationImport,
 	SelectChain,
 };
-use fg_primitives::GrandpaApi;
+use fg_primitives::{GRANDPA_ENGINE_ID, ScheduledChange, ConsensusLog};
 use sr_primitives::Justification;
-use sr_primitives::generic::BlockId;
+use sr_primitives::generic::{BlockId, OpaqueDigestItemId};
 use sr_primitives::traits::{
-	Block as BlockT, DigestFor,
-	Header as HeaderT, NumberFor, ProvideRuntimeApi,
+	Block as BlockT, DigestFor, Header as HeaderT, NumberFor,
 };
 use primitives::{H256, Blake2Hasher};
 
@@ -55,17 +53,16 @@ use crate::justification::GrandpaJustification;
 ///
 /// When using GRANDPA, the block import worker should be using this block import
 /// object.
-pub struct GrandpaBlockImport<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC> {
+pub struct GrandpaBlockImport<B, E, Block: BlockT<Hash=H256>, RA, SC> {
 	inner: Arc<Client<B, E, Block, RA>>,
 	select_chain: SC,
 	authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
 	send_voter_commands: mpsc::UnboundedSender<VoterCommand<Block::Hash, NumberFor<Block>>>,
 	consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
-	api: Arc<PRA>,
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC: Clone> Clone for
-	GrandpaBlockImport<B, E, Block, RA, PRA, SC>
+impl<B, E, Block: BlockT<Hash=H256>, RA, SC: Clone> Clone for
+	GrandpaBlockImport<B, E, Block, RA, SC>
 {
 	fn clone(&self) -> Self {
 		GrandpaBlockImport {
@@ -74,20 +71,17 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC: Clone> Clone for
 			authority_set: self.authority_set.clone(),
 			send_voter_commands: self.send_voter_commands.clone(),
 			consensus_changes: self.consensus_changes.clone(),
-			api: self.api.clone(),
 		}
 	}
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC> JustificationImport<Block>
-	for GrandpaBlockImport<B, E, Block, RA, PRA, SC> where
+impl<B, E, Block: BlockT<Hash=H256>, RA, SC> JustificationImport<Block>
+	for GrandpaBlockImport<B, E, Block, RA, SC> where
 		NumberFor<Block>: grandpa::BlockNumberOps,
 		B: Backend<Block, Blake2Hasher> + 'static,
 		E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
 		DigestFor<Block>: Encode,
 		RA: Send + Sync,
-		PRA: ProvideRuntimeApi,
-		PRA::Api: GrandpaApi<Block>,
 		SC: SelectChain<Block>,
 {
 	type Error = ConsensusError;
@@ -174,75 +168,69 @@ impl<'a, Block: 'a + BlockT> Drop for PendingSetChanges<'a, Block> {
 	}
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC>
-	GrandpaBlockImport<B, E, Block, RA, PRA, SC>
+fn find_scheduled_change<B: BlockT>(header: &B::Header)
+	-> Option<ScheduledChange<NumberFor<B>>>
+{
+	let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID);
+
+	let filter_log = |log: ConsensusLog<NumberFor<B>>| match log {
+		ConsensusLog::ScheduledChange(change) => Some(change),
+		_ => None,
+	};
+
+	// find the first consensus digest with the right ID which converts to
+	// the right kind of consensus log.
+	header.digest().convert_first(|l| l.try_to(id).and_then(filter_log))
+}
+
+fn find_forced_change<B: BlockT>(header: &B::Header)
+	-> Option<(NumberFor<B>, ScheduledChange<NumberFor<B>>)>
+{
+	let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID);
+
+	let filter_log = |log: ConsensusLog<NumberFor<B>>| match log {
+		ConsensusLog::ForcedChange(delay, change) => Some((delay, change)),
+		_ => None,
+	};
+
+	// find the first consensus digest with the right ID which converts to
+	// the right kind of consensus log.
+	header.digest().convert_first(|l| l.try_to(id).and_then(filter_log))
+}
+
+impl<B, E, Block: BlockT<Hash=H256>, RA, SC>
+	GrandpaBlockImport<B, E, Block, RA, SC>
 where
 	NumberFor<Block>: grandpa::BlockNumberOps,
 	B: Backend<Block, Blake2Hasher> + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
 	DigestFor<Block>: Encode,
 	RA: Send + Sync,
-	PRA: ProvideRuntimeApi,
-	PRA::Api: GrandpaApi<Block>,
 {
 	// check for a new authority set change.
 	fn check_new_change(&self, header: &Block::Header, hash: Block::Hash)
-		-> Result<Option<PendingChange<Block::Hash, NumberFor<Block>>>, ConsensusError>
+		-> Option<PendingChange<Block::Hash, NumberFor<Block>>>
 	{
-		let at = BlockId::hash(*header.parent_hash());
-		let digest = header.digest();
-
-		let api = self.api.runtime_api();
-
 		// check for forced change.
-		{
-			let maybe_change = api.grandpa_forced_change(
-				&at,
-				digest,
-			);
-
-			match maybe_change {
-				Err(e) => match api.has_api_with::<dyn GrandpaApi<Block>, _>(&at, |v| v >= 2) {
-					Err(e) => return Err(ConsensusError::ClientImport(e.to_string()).into()),
-					Ok(true) => {
-						// API version is high enough to support forced changes
-						// but got error, so it is legitimate.
-						return Err(ConsensusError::ClientImport(e.to_string()).into())
-					},
-					Ok(false) => {
-						// API version isn't high enough to support forced changes
-					},
-				},
-				Ok(None) => {},
-				Ok(Some((median_last_finalized, change))) => return Ok(Some(PendingChange {
-					next_authorities: change.next_authorities,
-					delay: change.delay,
-					canon_height: *header.number(),
-					canon_hash: hash,
-					delay_kind: DelayKind::Best { median_last_finalized },
-				})),
-			}
+		if let Some((median_last_finalized, change)) = find_forced_change::<Block>(header) {
+			return Some(PendingChange {
+				next_authorities: change.next_authorities,
+				delay: change.delay,
+				canon_height: *header.number(),
+				canon_hash: hash,
+				delay_kind: DelayKind::Best { median_last_finalized },
+			});
 		}
 
 		// check normal scheduled change.
-		{
-			let maybe_change = api.grandpa_pending_change(
-				&at,
-				digest,
-			);
-
-			match maybe_change {
-				Err(e) => Err(ConsensusError::ClientImport(e.to_string()).into()),
-				Ok(Some(change)) => Ok(Some(PendingChange {
-					next_authorities: change.next_authorities,
-					delay: change.delay,
-					canon_height: *header.number(),
-					canon_hash: hash,
-					delay_kind: DelayKind::Finalized,
-				})),
-				Ok(None) => Ok(None),
-			}
-		}
+		let change = find_scheduled_change::<Block>(header)?;
+		Some(PendingChange {
+			next_authorities: change.next_authorities,
+			delay: change.delay,
+			canon_height: *header.number(),
+			canon_hash: hash,
+			delay_kind: DelayKind::Finalized,
+		})
 	}
 
 	fn make_authorities_changes<'a>(&'a self, block: &mut BlockImportParams<Block>, hash: Block::Hash)
@@ -289,7 +277,7 @@ where
 		let maybe_change = self.check_new_change(
 			&block.header,
 			hash,
-		)?;
+		);
 
 		// returns a function for checking whether a block is a descendent of another
 		// consistent with querying client directly after importing the block.
@@ -388,15 +376,13 @@ where
 	}
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC> BlockImport<Block>
-	for GrandpaBlockImport<B, E, Block, RA, PRA, SC> where
+impl<B, E, Block: BlockT<Hash=H256>, RA, SC> BlockImport<Block>
+	for GrandpaBlockImport<B, E, Block, RA, SC> where
 		NumberFor<Block>: grandpa::BlockNumberOps,
 		B: Backend<Block, Blake2Hasher> + 'static,
 		E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
 		DigestFor<Block>: Encode,
 		RA: Send + Sync,
-		PRA: ProvideRuntimeApi,
-		PRA::Api: GrandpaApi<Block>,
 {
 	type Error = ConsensusError;
 
@@ -521,8 +507,8 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC> BlockImport<Block>
 	}
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC>
-	GrandpaBlockImport<B, E, Block, RA, PRA, SC>
+impl<B, E, Block: BlockT<Hash=H256>, RA, SC>
+	GrandpaBlockImport<B, E, Block, RA, SC>
 {
 	pub(crate) fn new(
 		inner: Arc<Client<B, E, Block, RA>>,
@@ -530,21 +516,19 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC>
 		authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
 		send_voter_commands: mpsc::UnboundedSender<VoterCommand<Block::Hash, NumberFor<Block>>>,
 		consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
-		api: Arc<PRA>,
-	) -> GrandpaBlockImport<B, E, Block, RA, PRA, SC> {
+	) -> GrandpaBlockImport<B, E, Block, RA, SC> {
 		GrandpaBlockImport {
 			inner,
 			select_chain,
 			authority_set,
 			send_voter_commands,
 			consensus_changes,
-			api,
 		}
 	}
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC>
-	GrandpaBlockImport<B, E, Block, RA, PRA, SC>
+impl<B, E, Block: BlockT<Hash=H256>, RA, SC>
+	GrandpaBlockImport<B, E, Block, RA, SC>
 where
 	NumberFor<Block>: grandpa::BlockNumberOps,
 	B: Backend<Block, Blake2Hasher> + 'static,

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

@@ -339,10 +339,10 @@ pub struct LinkHalf<B, E, Block: BlockT<Hash=H256>, RA, SC> {
 /// to it.
 pub fn block_import<B, E, Block: BlockT<Hash=H256>, RA, PRA, SC>(
 	client: Arc<Client<B, E, Block, RA>>,
-	api: Arc<PRA>,
+	api: &PRA,
 	select_chain: SC,
 ) -> Result<(
-		GrandpaBlockImport<B, E, Block, RA, PRA, SC>,
+		GrandpaBlockImport<B, E, Block, RA, SC>,
 		LinkHalf<B, E, Block, RA, SC>
 	), ClientError>
 where
@@ -381,7 +381,6 @@ where
 			persistent_data.authority_set.clone(),
 			voter_commands_tx,
 			persistent_data.consensus_changes.clone(),
-			api,
 		),
 		LinkHalf {
 			client,

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

@@ -37,9 +37,9 @@ use std::collections::{HashMap, HashSet};
 use std::result;
 use codec::Decode;
 use sr_primitives::traits::{ApiRef, ProvideRuntimeApi, Header as HeaderT};
-use sr_primitives::generic::BlockId;
+use sr_primitives::generic::{BlockId, DigestItem};
 use primitives::{NativeOrEncoded, ExecutionContext, crypto::Public};
-use fg_primitives::AuthorityId;
+use fg_primitives::{GRANDPA_ENGINE_ID, AuthorityId};
 
 use authorities::AuthoritySet;
 use finality_proof::{FinalityProofProvider, AuthoritySetForFinalityProver, AuthoritySetForFinalityChecker};
@@ -120,7 +120,7 @@ impl TestNetFactory for GrandpaTestNet {
 			PeersClient::Full(ref client, ref backend) => {
 				let (import, link) = block_import(
 					client.clone(),
-					Arc::new(self.test_config.clone()),
+					&self.test_config,
 					LongestChain::new(backend.clone()),
 				).expect("Could not create block import for fresh peer.");
 				let justification_import = Box::new(import.clone());
@@ -188,16 +188,12 @@ impl Future for Exit {
 #[derive(Default, Clone)]
 pub(crate) struct TestApi {
 	genesis_authorities: Vec<(AuthorityId, u64)>,
-	scheduled_changes: Arc<Mutex<HashMap<Hash, ScheduledChange<BlockNumber>>>>,
-	forced_changes: Arc<Mutex<HashMap<Hash, (BlockNumber, ScheduledChange<BlockNumber>)>>>,
 }
 
 impl TestApi {
 	pub fn new(genesis_authorities: Vec<(AuthorityId, u64)>) -> Self {
 		TestApi {
 			genesis_authorities,
-			scheduled_changes: Arc::new(Mutex::new(HashMap::new())),
-			forced_changes: Arc::new(Mutex::new(HashMap::new())),
 		}
 	}
 }
@@ -277,41 +273,6 @@ impl GrandpaApi<Block> for RuntimeApi {
 	) -> Result<NativeOrEncoded<Vec<(AuthorityId, u64)>>> {
 		Ok(self.inner.genesis_authorities.clone()).map(NativeOrEncoded::Native)
 	}
-
-	fn GrandpaApi_grandpa_pending_change_runtime_api_impl(
-		&self,
-		at: &BlockId<Block>,
-		_: ExecutionContext,
-		_: Option<(&DigestFor<Block>)>,
-		_: Vec<u8>,
-	) -> Result<NativeOrEncoded<Option<ScheduledChange<NumberFor<Block>>>>> {
-		let parent_hash = match at {
-			&BlockId::Hash(at) => at,
-			_ => panic!("not requested by block hash!!"),
-		};
-
-		// we take only scheduled changes at given block number where there are no
-		// extrinsics.
-		Ok(self.inner.scheduled_changes.lock().get(&parent_hash).map(|c| c.clone())).map(NativeOrEncoded::Native)
-	}
-
-	fn GrandpaApi_grandpa_forced_change_runtime_api_impl(
-		&self,
-		at: &BlockId<Block>,
-		_: ExecutionContext,
-		_: Option<(&DigestFor<Block>)>,
-		_: Vec<u8>,
-	)
-		-> Result<NativeOrEncoded<Option<(NumberFor<Block>, ScheduledChange<NumberFor<Block>>)>>> {
-		let parent_hash = match at {
-			&BlockId::Hash(at) => at,
-			_ => panic!("not requested by block hash!!"),
-		};
-
-		// we take only scheduled changes at given block number where there are no
-		// extrinsics.
-		Ok(self.inner.forced_changes.lock().get(&parent_hash).map(|c| c.clone())).map(NativeOrEncoded::Native)
-	}
 }
 
 impl AuthoritySetForFinalityProver<Block> for TestApi {
@@ -453,6 +414,24 @@ fn run_to_completion(
 	run_to_completion_with(runtime, blocks, net, peers, |_| None)
 }
 
+fn add_scheduled_change(block: &mut Block, change: ScheduledChange<BlockNumber>) {
+	block.header.digest_mut().push(DigestItem::Consensus(
+		GRANDPA_ENGINE_ID,
+		fg_primitives::ConsensusLog::ScheduledChange(change).encode(),
+	));
+}
+
+fn add_forced_change(
+	block: &mut Block,
+	median_last_finalized: BlockNumber,
+	change: ScheduledChange<BlockNumber>,
+) {
+	block.header.digest_mut().push(DigestItem::Consensus(
+		GRANDPA_ENGINE_ID,
+		fg_primitives::ConsensusLog::ForcedChange(median_last_finalized, change).encode(),
+	));
+}
+
 #[test]
 fn finalize_3_voters_no_observers() {
 	let _ = env_logger::try_init();
@@ -578,7 +557,6 @@ fn transition_3_voters_twice_1_full_observer() {
 	let genesis_voters = make_ids(peers_a);
 
 	let api = TestApi::new(genesis_voters);
-	let transitions = api.scheduled_changes.clone();
 	let net = Arc::new(Mutex::new(GrandpaTestNet::new(api, 8)));
 
 	let mut runtime = current_thread::Runtime::new().unwrap();
@@ -600,10 +578,6 @@ fn transition_3_voters_twice_1_full_observer() {
 	{
 		let net = net.clone();
 		let client = net.lock().peers[0].client().clone();
-		let transitions = transitions.clone();
-		let add_transition = move |parent_hash, change| {
-			transitions.lock().insert(parent_hash, change);
-		};
 		let peers_c = peers_c.clone();
 
 		// wait for blocks to be finalized before generating new ones
@@ -619,8 +593,8 @@ fn transition_3_voters_twice_1_full_observer() {
 					14 => {
 						// generate transition at block 15, applied at 20.
 						net.lock().peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
-							let block = builder.bake().unwrap();
-							add_transition(*block.header.parent_hash(), ScheduledChange {
+							let mut block = builder.bake().unwrap();
+							add_scheduled_change(&mut block, ScheduledChange {
 								next_authorities: make_ids(peers_b),
 								delay: 4,
 							});
@@ -633,8 +607,8 @@ fn transition_3_voters_twice_1_full_observer() {
 						// at block 21 we do another transition, but this time instant.
 						// add more until we have 30.
 						net.lock().peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
-							let block = builder.bake().unwrap();
-							add_transition(*block.header.parent_hash(), ScheduledChange {
+							let mut block = builder.bake().unwrap();
+							add_scheduled_change(&mut block, ScheduledChange {
 								next_authorities: make_ids(&peers_c),
 								delay: 0,
 							});
@@ -783,7 +757,6 @@ fn sync_justifications_on_change_blocks() {
 
 	// 4 peers, 3 of them are authorities and participate in grandpa
 	let api = TestApi::new(voters);
-	let transitions = api.scheduled_changes.clone();
 	let mut net = GrandpaTestNet::new(api, 4);
 
 	// add 20 blocks
@@ -791,8 +764,8 @@ fn sync_justifications_on_change_blocks() {
 
 	// at block 21 we do add a transition which is instant
 	net.peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
-		let block = builder.bake().unwrap();
-		transitions.lock().insert(*block.header.parent_hash(), ScheduledChange {
+		let mut block = builder.bake().unwrap();
+		add_scheduled_change(&mut block, ScheduledChange {
 			next_authorities: make_ids(peers_b),
 			delay: 0,
 		});
@@ -845,7 +818,6 @@ fn finalizes_multiple_pending_changes_in_order() {
 
 	// 6 peers, 3 of them are authorities and participate in grandpa from genesis
 	let api = TestApi::new(genesis_voters);
-	let transitions = api.scheduled_changes.clone();
 	let mut net = GrandpaTestNet::new(api, 6);
 
 	// add 20 blocks
@@ -853,8 +825,8 @@ fn finalizes_multiple_pending_changes_in_order() {
 
 	// at block 21 we do add a transition which is instant
 	net.peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
-		let block = builder.bake().unwrap();
-		transitions.lock().insert(*block.header.parent_hash(), ScheduledChange {
+		let mut block = builder.bake().unwrap();
+		add_scheduled_change(&mut block, ScheduledChange {
 			next_authorities: make_ids(peers_b),
 			delay: 0,
 		});
@@ -866,8 +838,8 @@ fn finalizes_multiple_pending_changes_in_order() {
 
 	// at block 26 we add another which is enacted at block 30
 	net.peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
-		let block = builder.bake().unwrap();
-		transitions.lock().insert(*block.header.parent_hash(), ScheduledChange {
+		let mut block = builder.bake().unwrap();
+		add_scheduled_change(&mut block, ScheduledChange {
 			next_authorities: make_ids(peers_c),
 			delay: 4,
 		});
@@ -929,27 +901,26 @@ fn force_change_to_new_set() {
 	let api = TestApi::new(make_ids(genesis_authorities));
 
 	let voters = make_ids(peers_a);
-	let normal_transitions = api.scheduled_changes.clone();
-	let forced_transitions = api.forced_changes.clone();
 	let net = GrandpaTestNet::new(api, 3);
 	let net = Arc::new(Mutex::new(net));
 
-	net.lock().peer(0).push_blocks(1, false);
+	net.lock().peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
+		let mut block = builder.bake().unwrap();
 
-	{
 		// add a forced transition at block 12.
-		let parent_hash = net.lock().peer(0).client().info().chain.best_hash;
-		forced_transitions.lock().insert(parent_hash, (0, ScheduledChange {
+		add_forced_change(&mut block, 0, ScheduledChange {
 			next_authorities: voters.clone(),
 			delay: 10,
-		}));
+		});
 
 		// add a normal transition too to ensure that forced changes take priority.
-		normal_transitions.lock().insert(parent_hash, ScheduledChange {
+		add_scheduled_change(&mut block, ScheduledChange {
 			next_authorities: make_ids(genesis_authorities),
 			delay: 5,
 		});
-	}
+
+		block
+	});
 
 	net.lock().peer(0).push_blocks(25, false);
 	net.lock().block_until_sync(&mut runtime);
@@ -984,8 +955,8 @@ fn allows_reimporting_change_blocks() {
 
 	let full_client = client.as_full().unwrap();
 	let builder = full_client.new_block_at(&BlockId::Number(0), Default::default()).unwrap();
-	let block = builder.bake().unwrap();
-	api.scheduled_changes.lock().insert(*block.header.parent_hash(), ScheduledChange {
+	let mut block = builder.bake().unwrap();
+	add_scheduled_change(&mut block, ScheduledChange {
 		next_authorities: make_ids(peers_b),
 		delay: 0,
 	});
@@ -1034,8 +1005,9 @@ fn test_bad_justification() {
 
 	let full_client = client.as_full().expect("only full clients are used in test");
 	let builder = full_client.new_block_at(&BlockId::Number(0), Default::default()).unwrap();
-	let block = builder.bake().unwrap();
-	api.scheduled_changes.lock().insert(*block.header.parent_hash(), ScheduledChange {
+	let mut block = builder.bake().unwrap();
+
+	add_scheduled_change(&mut block, ScheduledChange {
 		next_authorities: make_ids(peers_b),
 		delay: 0,
 	});
@@ -1413,20 +1385,21 @@ fn empty_finality_proof_is_returned_to_light_client_when_authority_set_is_differ
 	let api = TestApi::new(make_ids(&genesis_authorities));
 
 	let voters = make_ids(peers_a);
-	let forced_transitions = api.forced_changes.clone();
 	let net = GrandpaTestNet::new(api, 3);
 	let net = Arc::new(Mutex::new(net));
 
-	net.lock().peer(0).push_blocks(1, false); // best is #1
-
-	// add a forced transition at block 5.
-	if FORCE_CHANGE {
-		let parent_hash = net.lock().peer(0).client().info().chain.best_hash;
-		forced_transitions.lock().insert(parent_hash, (0, ScheduledChange {
-			next_authorities: voters.clone(),
-			delay: 3,
-		}));
-	}
+	// best is #1
+	net.lock().peer(0).generate_blocks(1, BlockOrigin::File, |builder| {
+		// add a forced transition at block 5.
+		let mut block = builder.bake().unwrap();
+		if FORCE_CHANGE {
+			add_forced_change(&mut block, 0, ScheduledChange {
+				next_authorities: voters.clone(),
+				delay: 3,
+			});
+		}
+		block
+	});
 
 	// ensure block#10 enacts authorities set change => justification is generated
 	// normally it will reach light client, but because of the forced change, it will not

substrate/node-template/runtime/src/lib.rs

@@ -14,11 +14,11 @@ use sr_primitives::{
 	ApplyResult, transaction_validity::TransactionValidity, generic, create_runtime_str,
 	impl_opaque_keys, AnySignature
 };
-use sr_primitives::traits::{NumberFor, BlakeTwo256, Block as BlockT, DigestFor, StaticLookup, Verify, ConvertInto};
+use sr_primitives::traits::{NumberFor, BlakeTwo256, Block as BlockT, StaticLookup, Verify, ConvertInto};
 use sr_primitives::weights::Weight;
 use babe::{AuthorityId as BabeId};
 use grandpa::{AuthorityId as GrandpaId, AuthorityWeight as GrandpaWeight};
-use grandpa::fg_primitives::{self, ScheduledChange};
+use grandpa::fg_primitives;
 use client::{
 	block_builder::api::{CheckInherentsResult, InherentData, self as block_builder_api},
 	runtime_api as client_api, impl_runtime_apis
@@ -356,18 +356,6 @@ impl_runtime_apis! {
 	}
 
 	impl fg_primitives::GrandpaApi<Block> for Runtime {
-		fn grandpa_pending_change(digest: &DigestFor<Block>)
-			-> Option<ScheduledChange<NumberFor<Block>>>
-		{
-			Grandpa::pending_change(digest)
-		}
-
-		fn grandpa_forced_change(digest: &DigestFor<Block>)
-			-> Option<(NumberFor<Block>, ScheduledChange<NumberFor<Block>>)>
-		{
-			Grandpa::forced_change(digest)
-		}
-
 		fn grandpa_authorities() -> Vec<(GrandpaId, GrandpaWeight)> {
 			Grandpa::grandpa_authorities()
 		}

substrate/node-template/src/service.rs

@@ -49,7 +49,7 @@ macro_rules! new_full_start {
 					.ok_or_else(|| substrate_service::Error::SelectChainRequired)?;
 				let (grandpa_block_import, grandpa_link) =
 					grandpa::block_import::<_, _, _, node_template_runtime::RuntimeApi, _, _>(
-						client.clone(), client.clone(), select_chain
+						client.clone(), &*client, select_chain
 					)?;
 				let justification_import = grandpa_block_import.clone();
 

substrate/node/cli/src/service.rs

@@ -62,7 +62,7 @@ macro_rules! new_full_start {
 					.ok_or_else(|| substrate_service::Error::SelectChainRequired)?;
 				let (grandpa_block_import, grandpa_link) =
 					grandpa::block_import::<_, _, _, node_runtime::RuntimeApi, _, _>(
-						client.clone(), client.clone(), select_chain
+						client.clone(), &*client, select_chain
 					)?;
 				let justification_import = grandpa_block_import.clone();
 

substrate/node/runtime/src/lib.rs

@@ -30,7 +30,7 @@ use node_primitives::{
 	Moment, Signature, ContractExecResult,
 };
 use babe_primitives::{AuthorityId as BabeId};
-use grandpa::fg_primitives::{self, ScheduledChange};
+use grandpa::fg_primitives;
 use client::{
 	block_builder::api::{self as block_builder_api, InherentData, CheckInherentsResult},
 	runtime_api as client_api, impl_runtime_apis
@@ -42,7 +42,7 @@ use sr_primitives::curve::PiecewiseLinear;
 use sr_primitives::transaction_validity::TransactionValidity;
 use sr_primitives::weights::Weight;
 use sr_primitives::traits::{
-	self, BlakeTwo256, Block as BlockT, DigestFor, NumberFor, StaticLookup, SaturatedConversion,
+	self, BlakeTwo256, Block as BlockT, NumberFor, StaticLookup, SaturatedConversion,
 };
 use version::RuntimeVersion;
 use elections::VoteIndex;
@@ -596,18 +596,6 @@ impl_runtime_apis! {
 	}
 
 	impl fg_primitives::GrandpaApi<Block> for Runtime {
-		fn grandpa_pending_change(digest: &DigestFor<Block>)
-			-> Option<ScheduledChange<NumberFor<Block>>>
-		{
-			Grandpa::pending_change(digest)
-		}
-
-		fn grandpa_forced_change(digest: &DigestFor<Block>)
-			-> Option<(NumberFor<Block>, ScheduledChange<NumberFor<Block>>)>
-		{
-			Grandpa::forced_change(digest)
-		}
-
 		fn grandpa_authorities() -> Vec<(GrandpaId, GrandpaWeight)> {
 			Grandpa::grandpa_authorities()
 		}

substrate/srml/grandpa/src/lib.rs

@@ -246,6 +246,8 @@ impl<T: Trait> Module<T> {
 		Authorities::get()
 	}
 
+	/// Schedule GRANDPA to pause starting in the given number of blocks.
+	/// Cannot be done when already paused.
 	pub fn schedule_pause(in_blocks: T::BlockNumber) -> Result {
 		if let StoredState::Live = <State<T>>::get() {
 			let scheduled_at = <system::Module<T>>::block_number();
@@ -261,6 +263,7 @@ impl<T: Trait> Module<T> {
 		}
 	}
 
+	/// Schedule a resume of GRANDPA after pausing.
 	pub fn schedule_resume(in_blocks: T::BlockNumber) -> Result {
 		if let StoredState::Paused = <State<T>>::get() {
 			let scheduled_at = <system::Module<T>>::block_number();