Offline fallback for GRANDPA (#1619)

Co-authored-by: André Silva <andre.beat@gmail.com> * skeleton for finality tracker * dispatch events when nothing finalized for a long time * begin integrating finality tracker into grandpa * add delay field to pending change * add has_api_with function to sr_version for querying APIs * partially integrate new force changes into grandpa * implement forced changes * get srml-grandpa compiling * Update core/finality-grandpa/src/authorities.rs Co-Authored-By: rphmeier <rphmeier@gmail.com> * Update core/finality-grandpa/src/authorities.rs Co-Authored-By: rphmeier <rphmeier@gmail.com> * Update core/finality-grandpa/src/authorities.rs Co-Authored-By: rphmeier <rphmeier@gmail.com> * remove explicit dependence on CoreApi * increase node runtime version * integrate grandpa forced changes into node runtime * add some tests to finality-tracker * integrate finality tracking into node-runtime * test forced-change logic * test forced changes in the authority-set handler * kill some unneeded bounds in client * test forced-changes in finality-grandpa and fix logic * build wasm and finality-tracker is no-std * restart voter on forced change * allow returning custom error type from lock_import_and_run * extract out most DB logic to aux_schema and use atomic client ops * unify authority set writing * implement set pausing * bump runtime version * note on DB when we pause. * core: grandpa: integrate forced changes with multiple pending standard changes * core: grandpa: fix AuthoritySet tests * runtime: bump impl_version * core: clear pending justification requests after forced change import * srml: finality-tracker: use FinalizedInherentData * core: log requests for clearing justification requests * core, node: update runtimes * core: grandpa: fix tests * core: grandpa: remove todos and add comments * core: grandpa: use has_api_with from ApiExt * core: fix tests * core: grandpa: remove unnecessary mut modifier * core: replace PostImportActions bitflags with struct * core: grandpa: restrict genesis on forced authority set change * core: grandpa: add more docs * core: grandpa: prevent safety violations in Environment::finalize_block * core: grandpa: register finality tracker inherent data provider * core: grandpa: fix tests * node: update runtime blobs * core: grandpa: remove outdated todo * core: aura: fix typo in log message * core: grandpa: check re-finalization is on canonical chain * srml: finality-tracker: fix initialization * node: update runtime wasm * srml: finality-tracker: don't re-initialize config keys
2026-06-17 17:11:02 +00:00 · 2019-03-05 16:41:35 +01:00
parent 128d164f2b
commit dfb48a2405
31 changed files with 2217 additions and 516 deletions
@@ -19,9 +19,12 @@ use std::sync::Arc;
 use log::{debug, trace, info};
 use parity_codec::Encode;
 use futures::sync::mpsc;
+use parking_lot::RwLockWriteGuard;

 use client::{blockchain, CallExecutor, Client};
+use client::blockchain::HeaderBackend;
 use client::backend::Backend;
+use client::runtime_api::ApiExt;
 use consensus_common::{
 	BlockImport, Error as ConsensusError, ErrorKind as ConsensusErrorKind,
 	ImportBlock, ImportResult, JustificationImport,
@@ -35,10 +38,10 @@ use runtime_primitives::traits::{
 };
 use substrate_primitives::{H256, Ed25519AuthorityId, Blake2Hasher};

-use crate::{AUTHORITY_SET_KEY, Error};
-use crate::authorities::SharedAuthoritySet;
+use crate::{Error, CommandOrError, NewAuthoritySet, VoterCommand};
+use crate::authorities::{AuthoritySet, SharedAuthoritySet, DelayKind, PendingChange};
 use crate::consensus_changes::SharedConsensusChanges;
-use crate::environment::{finalize_block, is_descendent_of, ExitOrError, NewAuthoritySet};
+use crate::environment::{finalize_block, is_descendent_of};
 use crate::justification::GrandpaJustification;

 /// A block-import handler for GRANDPA.
@@ -53,7 +56,7 @@ use crate::justification::GrandpaJustification;
 pub struct GrandpaBlockImport<B, E, Block: BlockT<Hash=H256>, RA, PRA> {
 	inner: Arc<Client<B, E, Block, RA>>,
 	authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
-	authority_set_change: mpsc::UnboundedSender<NewAuthoritySet<Block::Hash, NumberFor<Block>>>,
+	send_voter_commands: mpsc::UnboundedSender<VoterCommand<Block::Hash, NumberFor<Block>>>,
 	consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
 	api: Arc<PRA>,
 }
@@ -80,7 +83,8 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> JustificationImport<Block>
 		// request justifications for all pending changes for which change blocks have already been imported
 		let authorities = self.authority_set.inner().read();
 		for pending_change in authorities.pending_changes() {
-			if pending_change.effective_number() > chain_info.finalized_number &&
+			if pending_change.delay_kind == DelayKind::Finalized &&
+				pending_change.effective_number() > chain_info.finalized_number &&
 				pending_change.effective_number() <= chain_info.best_number
 			{
 				let effective_block_hash = self.inner.best_containing(
@@ -109,6 +113,266 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> JustificationImport<Block>
 	}
 }

+enum AppliedChanges<H, N> {
+	Standard,
+	Forced(NewAuthoritySet<H, N>),
+	None,
+}
+
+impl<H, N> AppliedChanges<H, N> {
+	fn needs_justification(&self) -> bool {
+		match *self {
+			AppliedChanges::Standard => true,
+			AppliedChanges::Forced(_) | AppliedChanges::None => false,
+		}
+	}
+}
+
+struct PendingSetChanges<'a, Block: 'a + BlockT> {
+	just_in_case: Option<(
+		AuthoritySet<Block::Hash, NumberFor<Block>>,
+		RwLockWriteGuard<'a, AuthoritySet<Block::Hash, NumberFor<Block>>>,
+	)>,
+	applied_changes: AppliedChanges<Block::Hash, NumberFor<Block>>,
+	do_pause: bool,
+}
+
+impl<'a, Block: 'a + BlockT> PendingSetChanges<'a, Block> {
+	// revert the pending set change explicitly.
+	fn revert(self) { }
+
+	fn defuse(mut self) -> (AppliedChanges<Block::Hash, NumberFor<Block>>, bool) {
+		self.just_in_case = None;
+		let applied_changes = ::std::mem::replace(&mut self.applied_changes, AppliedChanges::None);
+		(applied_changes, self.do_pause)
+	}
+}
+
+impl<'a, Block: 'a + BlockT> Drop for PendingSetChanges<'a, Block> {
+	fn drop(&mut self) {
+		if let Some((old_set, mut authorities)) = self.just_in_case.take() {
+			*authorities = old_set;
+		}
+	}
+}
+
+impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> GrandpaBlockImport<B, E, Block, RA, PRA> where
+	NumberFor<Block>: grandpa::BlockNumberOps,
+	B: Backend<Block, Blake2Hasher> + 'static,
+	E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
+	DigestFor<Block>: Encode,
+	DigestItemFor<Block>: DigestItem<AuthorityId=Ed25519AuthorityId>,
+	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>
+	{
+		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::<GrandpaApi<Block>, _>(&at, |v| v >= 2) {
+					Err(e) => return Err(ConsensusErrorKind::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(ConsensusErrorKind::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 },
+				})),
+			}
+		}
+
+		// check normal scheduled change.
+		{
+			let maybe_change = api.grandpa_pending_change(
+				&at,
+				digest,
+			);
+
+			match maybe_change {
+				Err(e) => Err(ConsensusErrorKind::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),
+			}
+		}
+	}
+
+	fn make_authorities_changes<'a>(&'a self, block: &mut ImportBlock<Block>, hash: Block::Hash)
+		-> Result<PendingSetChanges<'a, Block>, ConsensusError>
+	{
+		// 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
+		// the old authority set on error or panic.
+		struct InnerGuard<'a, T: 'a> {
+			old: Option<T>,
+			guard: Option<RwLockWriteGuard<'a, T>>,
+		}
+
+		impl<'a, T: 'a> InnerGuard<'a, T> {
+			fn as_mut(&mut self) -> &mut T {
+				&mut **self.guard.as_mut().expect("only taken on deconstruction; qed")
+			}
+
+			fn set_old(&mut self, old: T) {
+				if self.old.is_none() {
+					// ignore "newer" old changes.
+					self.old = Some(old);
+				}
+			}
+
+			fn consume(mut self) -> Option<(T, RwLockWriteGuard<'a, T>)> {
+				if let Some(old) = self.old.take() {
+					Some((old, self.guard.take().expect("only taken on deconstruction; qed")))
+				} else {
+					None
+				}
+			}
+		}
+
+		impl<'a, T: 'a> Drop for InnerGuard<'a, T> {
+			fn drop(&mut self) {
+				if let (Some(mut guard), Some(old)) = (self.guard.take(), self.old.take()) {
+					*guard = old;
+				}
+			}
+		}
+
+		let number = block.header.number().clone();
+		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.
+		let parent_hash = *block.header.parent_hash();
+		let is_descendent_of = is_descendent_of(&self.inner, Some((&hash, &parent_hash)));
+
+		let mut guard = InnerGuard {
+			guard: Some(self.authority_set.inner().write()),
+			old: None,
+		};
+
+		// whether to pause the old authority set -- happens after import
+		// of a forced change block.
+		let mut do_pause = false;
+
+		// add any pending changes.
+		if let Some(change) = maybe_change {
+			let old = guard.as_mut().clone();
+			guard.set_old(old);
+
+			if let DelayKind::Best { .. } = change.delay_kind {
+				do_pause = true;
+			}
+
+			guard.as_mut().add_pending_change(
+				change,
+				&is_descendent_of,
+			).map_err(|e| ConsensusError::from(ConsensusErrorKind::ClientImport(e.to_string())))?;
+		}
+
+		let applied_changes = {
+			let forced_change_set = guard.as_mut().apply_forced_changes(hash, number, &is_descendent_of)
+				.map_err(|e| ConsensusErrorKind::ClientImport(e.to_string()))
+				.map_err(ConsensusError::from)?;
+
+			if let Some((median_last_finalized_number, new_set)) = forced_change_set {
+				let new_authorities = {
+					let (set_id, new_authorities) = new_set.current();
+
+					// we will use the median last finalized number as a hint
+					// for the canon block the new authority set should start
+					// with. we use the minimum between the median and the local
+					// best finalized block.
+					let best_finalized_number = self.inner.backend().blockchain().info()
+						.map_err(|e| ConsensusErrorKind::ClientImport(e.to_string()))?
+						.finalized_number;
+
+					let canon_number = best_finalized_number.min(median_last_finalized_number);
+
+					let canon_hash =
+						self.inner.backend().blockchain().header(BlockId::Number(canon_number))
+							.map_err(|e| ConsensusErrorKind::ClientImport(e.to_string()))?
+							.expect("the given block number is less or equal than the current best finalized number; \
+									 current best finalized number must exist in chain; qed.")
+							.hash();
+
+					NewAuthoritySet {
+						canon_number,
+						canon_hash,
+						set_id,
+						authorities: new_authorities.to_vec(),
+					}
+				};
+				let old = ::std::mem::replace(guard.as_mut(), new_set);
+				guard.set_old(old);
+
+				AppliedChanges::Forced(new_authorities)
+			} else {
+				let did_standard = guard.as_mut().enacts_standard_change(hash, number, &is_descendent_of)
+					.map_err(|e| ConsensusErrorKind::ClientImport(e.to_string()))
+					.map_err(ConsensusError::from)?;
+
+				if did_standard {
+					AppliedChanges::Standard
+				} else {
+					AppliedChanges::None
+				}
+			}
+		};
+
+		// consume the guard safely and write necessary changes.
+		let just_in_case = guard.consume();
+		if let Some((_, ref authorities)) = just_in_case {
+			let authorities_change = match applied_changes {
+				AppliedChanges::Forced(ref new) => Some(new),
+				AppliedChanges::Standard => None, // the change isn't actually applied yet.
+				AppliedChanges::None => None,
+			};
+
+			crate::aux_schema::update_authority_set(
+				authorities,
+				authorities_change,
+				|insert| block.auxiliary.extend(
+					insert.iter().map(|(k, v)| (k.to_vec(), Some(v.to_vec())))
+				)
+			);
+		}
+
+		Ok(PendingSetChanges { just_in_case, applied_changes, do_pause })
+	}
+}
+
 impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> BlockImport<Block>
 	for GrandpaBlockImport<B, E, Block, RA, PRA> where
 		NumberFor<Block>: grandpa::BlockNumberOps,
@@ -125,12 +389,8 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> BlockImport<Block>
 	fn import_block(&self, mut block: ImportBlock<Block>, new_authorities: Option<Vec<Ed25519AuthorityId>>)
 		-> Result<ImportResult, Self::Error>
 	{
-		use crate::authorities::PendingChange;
-		use client::blockchain::HeaderBackend;
-
 		let hash = block.post_header().hash();
-		let parent_hash = *block.header.parent_hash();
-		let number = *block.header.number();
+		let number = block.header.number().clone();

 		// early exit if block already in chain, otherwise the check for
 		// authority changes will error when trying to re-import a change block
@@ -140,82 +400,68 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> BlockImport<Block>
 			Err(e) => return Err(ConsensusErrorKind::ClientImport(e.to_string()).into()),
 		}

-		let maybe_change = self.api.runtime_api().grandpa_pending_change(
-			&BlockId::hash(parent_hash),
-			&block.header.digest().clone(),
-		);
-
-		let maybe_change = match maybe_change {
-			Err(e) => return Err(ConsensusErrorKind::ClientImport(e.to_string()).into()),
-			Ok(maybe) => maybe,
-		};
-
-		// 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
-		// the old authority set on error.
-		let is_descendent_of = is_descendent_of(&self.inner, Some((&hash, &parent_hash)));
-		let just_in_case = if let Some(change) = maybe_change {
-			let mut authorities = self.authority_set.inner().write();
-			let old_set = authorities.clone();
-
-			authorities.add_pending_change(
-				PendingChange {
-					next_authorities: change.next_authorities,
-					finalization_depth: change.delay,
-					canon_height: number,
-					canon_hash: hash,
-				},
-				&is_descendent_of,
-			).map_err(|e| ConsensusError::from(ConsensusErrorKind::ClientImport(e.to_string())))?;
-
-			block.auxiliary.push((AUTHORITY_SET_KEY.to_vec(), Some(authorities.encode())));
-			Some((old_set, authorities))
-		} else {
-			None
-		};
+		let pending_changes = self.make_authorities_changes(&mut block, hash)?;

 		// we don't want to finalize on `inner.import_block`
 		let justification = block.justification.take();
 		let enacts_consensus_change = new_authorities.is_some();
 		let import_result = self.inner.import_block(block, new_authorities);

-		let import_result = {
-			// we scope this so that `just_in_case` is dropped eagerly and releases the authorities lock
-			let revert_authorities = || if let Some((old_set, mut authorities)) = just_in_case {
-				*authorities = old_set;
-			};
-
+		let mut imported_aux = {
 			match import_result {
-				Ok(ImportResult::Queued) => ImportResult::Queued,
+				Ok(ImportResult::Imported(aux)) => aux,
 				Ok(r) => {
 					debug!(target: "afg", "Restoring old authority set after block import result: {:?}", r);
-					revert_authorities();
+					pending_changes.revert();
 					return Ok(r);
 				},
 				Err(e) => {
 					debug!(target: "afg", "Restoring old authority set after block import error: {:?}", e);
-					revert_authorities();
+					pending_changes.revert();
 					return Err(ConsensusErrorKind::ClientImport(e.to_string()).into());
 				},
 			}
 		};

-		let enacts_change = self.authority_set.inner().read().enacts_change(
-			hash,
-			number,
-			&is_descendent_of,
-		).map_err(|e| ConsensusError::from(ConsensusErrorKind::ClientImport(e.to_string())))?;
+		let (applied_changes, do_pause) = pending_changes.defuse();

-		if !enacts_change && !enacts_consensus_change {
-			return Ok(import_result);
+		// Send the pause signal after import but BEFORE sending a `ChangeAuthorities` message.
+		if do_pause {
+			let _ = self.send_voter_commands.unbounded_send(
+				VoterCommand::Pause(format!("Forced change scheduled after inactivity"))
+			);
+		}
+
+		let needs_justification = applied_changes.needs_justification();
+		if let AppliedChanges::Forced(new) = applied_changes {
+			// NOTE: when we do a force change we are "discrediting" the old set so we
+			// ignore any justifications from them. this block may contain a justification
+			// which should be checked and imported below against the new authority
+			// triggered by this forced change. the new grandpa voter will start at the
+			// last median finalized block (which is before the block that enacts the
+			// change), full nodes syncing the chain will not be able to successfully
+			// import justifications for those blocks since their local authority set view
+			// is still of the set before the forced change was enacted, still after #1867
+			// they should import the block and discard the justification, and they will
+			// then request a justification from sync if it's necessary (which they should
+			// then be able to successfully validate).
+			let _ = self.send_voter_commands.unbounded_send(VoterCommand::ChangeAuthorities(new));
+
+			// we must clear all pending justifications requests, presumably they won't be
+			// finalized hence why this forced changes was triggered
+			imported_aux.clear_justification_requests = true;
+		}
+
+		if !needs_justification && !enacts_consensus_change {
+			return Ok(ImportResult::Imported(imported_aux));
 		}

 		match justification {
 			Some(justification) => {
-				self.import_justification(hash, number, justification, enacts_change)?;
+				self.import_justification(hash, number, justification, needs_justification)?;
 			},
 			None => {
-				if enacts_change {
+				if needs_justification {
 					trace!(
 						target: "finality",
 						"Imported unjustified block #{} that enacts authority set change, waiting for finality for enactment.",
@@ -229,11 +475,11 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> BlockImport<Block>
 					self.consensus_changes.lock().note_change((number, hash));
 				}

-				return Ok(ImportResult::NeedsJustification);
+				imported_aux.needs_justification = true;
 			}
 		}

-		Ok(import_result)
+		Ok(ImportResult::Imported(imported_aux))
 	}

 	fn check_block(
@@ -249,22 +495,22 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> GrandpaBlockImport<B, E, Block, RA
 	pub(crate) fn new(
 		inner: Arc<Client<B, E, Block, RA>>,
 		authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
-		authority_set_change: mpsc::UnboundedSender<NewAuthoritySet<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> {
 		GrandpaBlockImport {
 			inner,
 			authority_set,
-			authority_set_change,
+			send_voter_commands,
 			consensus_changes,
 			api,
 		}
 	}
 }

-impl<B, E, Block: BlockT<Hash=H256>, RA, PRA>
-	GrandpaBlockImport<B, E, Block, RA, PRA> where
+impl<B, E, Block: BlockT<Hash=H256>, RA, PRA> GrandpaBlockImport<B, E, Block, RA, PRA>
+	where
 		NumberFor<Block>: grandpa::BlockNumberOps,
 		B: Backend<Block, Blake2Hasher> + 'static,
 		E: CallExecutor<Block, Blake2Hasher> + 'static + Clone + Send + Sync,
@@ -304,13 +550,15 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, PRA>
 		);

 		match result {
-			Err(ExitOrError::AuthoritiesChanged(new)) => {
-				info!(target: "finality", "Imported justification for block #{} that enacts authority set change, signalling voter.", number);
-				if let Err(e) = self.authority_set_change.unbounded_send(new) {
+			Err(CommandOrError::VoterCommand(command)) => {
+				info!(target: "finality", "Imported justification for block #{} that triggers \
+					command {}, signalling voter.", number, command);
+
+				if let Err(e) = self.send_voter_commands.unbounded_send(command) {
 					return Err(ConsensusErrorKind::ClientImport(e.to_string()).into());
 				}
 			},
-			Err(ExitOrError::Error(e)) => {
+			Err(CommandOrError::Error(e)) => {
 				return Err(match e {
 					Error::Grandpa(error) => ConsensusErrorKind::ClientImport(error.to_string()),
 					Error::Network(error) => ConsensusErrorKind::ClientImport(error),