grandpa: pluggable voting rules (#3673)

* grandpa: support pluggable custom voting rules

* grandpa: add docs to grandpa voting rule

* grandpa: make voting rule mandatory

* grandpa: add test for voting rule

* node: add GRANDPA voting rule

* grandpa: pass backend to VotingRule

* core: fix docs in SelectChain::finality_target

* grandpa: implement 3/4 of unfinalized chain restriction as voting rule

* grandpa: rename AlwaysBehindBestBlock voting rule

* grandpa: fix tests

* grandpa: remove useless test

* grandpa: extend environemnt voting rule test

* grandpa: add proofs to unreachable statements

* grandpa: fix typo

* grandpa: fix docs

Author: andresilva

core/consensus/common/src/select_chain.rs

@@ -42,8 +42,9 @@ pub trait SelectChain<Block: BlockT>: Sync + Send + Clone {
 	/// best chain to author new blocks upon and probably finalize.
 	fn best_chain(&self) -> Result<<Block as BlockT>::Header, Error>;
 
-	/// Get the best ancestor of `target_hash` that we should attempt
-	/// to finalize next.
+	/// Get the best descendent of `target_hash` that we should attempt to
+	/// finalize next, if any. It is valid to return the given `target_hash`
+	/// itself if no better descendent exists.
 	fn finality_target(
 		&self,
 		target_hash: <Block as BlockT>::Hash,

core/finality-grandpa/src/environment.rs

@@ -52,6 +52,7 @@ use crate::authorities::{AuthoritySet, SharedAuthoritySet};
 use crate::consensus_changes::SharedConsensusChanges;
 use crate::justification::GrandpaJustification;
 use crate::until_imported::UntilVoteTargetImported;
+use crate::voting_rule::VotingRule;
 use fg_primitives::{AuthorityId, AuthoritySignature, SetId, RoundNumber};
 
 type HistoricalVotes<Block> = grandpa::HistoricalVotes<
@@ -368,7 +369,7 @@ impl<Block: BlockT> SharedVoterSetState<Block> {
 }
 
 /// The environment we run GRANDPA in.
-pub(crate) struct Environment<B, E, Block: BlockT, N: Network<Block>, RA, SC> {
+pub(crate) struct Environment<B, E, Block: BlockT, N: Network<Block>, RA, SC, VR> {
 	pub(crate) inner: Arc<Client<B, E, Block, RA>>,
 	pub(crate) select_chain: SC,
 	pub(crate) voters: Arc<VoterSet<AuthorityId>>,
@@ -378,9 +379,10 @@ pub(crate) struct Environment<B, E, Block: BlockT, N: Network<Block>, RA, SC> {
 	pub(crate) network: crate::communication::NetworkBridge<Block, N>,
 	pub(crate) set_id: SetId,
 	pub(crate) voter_set_state: SharedVoterSetState<Block>,
+	pub(crate) voting_rule: VR,
 }
 
-impl<B, E, Block: BlockT, N: Network<Block>, RA, SC> Environment<B, E, Block, N, RA, SC> {
+impl<B, E, Block: BlockT, N: Network<Block>, RA, SC, VR> Environment<B, E, Block, N, RA, SC, VR> {
 	/// Updates the voter set state using the given closure. The write lock is
 	/// held during evaluation of the closure and the environment's voter set
 	/// state is set to its result if successful.
@@ -396,16 +398,18 @@ impl<B, E, Block: BlockT, N: Network<Block>, RA, SC> Environment<B, E, Block, N,
 	}
 }
 
-impl<Block: BlockT<Hash=H256>, B, E, N, RA, SC>
+impl<Block: BlockT<Hash=H256>, B, E, N, RA, SC, VR>
 	grandpa::Chain<Block::Hash, NumberFor<Block>>
-for Environment<B, E, Block, N, RA, SC>
+for Environment<B, E, Block, N, RA, SC, VR>
 where
 	Block: 'static,
 	B: Backend<Block, Blake2Hasher> + 'static,
-	E: CallExecutor<Block, Blake2Hasher> + 'static,
+	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + 'static,
 	N: Network<Block> + 'static,
 	N::In: 'static,
 	SC: SelectChain<Block> + 'static,
+	VR: VotingRule<Block, Client<B, E, Block, RA>>,
+	RA: Send + Sync,
 	NumberFor<Block>: BlockNumberOps,
 {
 	fn ancestry(&self, base: Block::Hash, block: Block::Hash) -> Result<Vec<Block::Hash>, GrandpaError> {
@@ -429,7 +433,7 @@ where
 		debug!(target: "afg", "Finding best chain containing block {:?} with number limit {:?}", block, limit);
 
 		match self.select_chain.finality_target(block, None) {
-			Ok(Some(mut best_hash)) => {
+			Ok(Some(best_hash)) => {
 				let base_header = self.inner.header(&BlockId::Hash(block)).ok()?
 					.expect("Header known to exist after `best_containing` call; qed");
 
@@ -445,35 +449,51 @@ where
 					}
 				}
 
-				let mut best_header = self.inner.header(&BlockId::Hash(best_hash)).ok()?
+				let best_header = self.inner.header(&BlockId::Hash(best_hash)).ok()?
 					.expect("Header known to exist after `best_containing` call; qed");
 
-				// we target a vote towards 3/4 of the unfinalized chain (rounding up)
-				let target = {
-					let two = NumberFor::<Block>::one() + One::one();
-					let three = two + One::one();
-					let four = three + One::one();
-
-					let diff = *best_header.number() - *base_header.number();
-					let diff = ((diff * three) + two) / four;
+				// check if our vote is currently being limited due to a pending change
+				let limit = limit.filter(|limit| limit < best_header.number());
+				let target;
+
+				let target_header = if let Some(target_number) = limit {
+					let mut target_header = best_header.clone();
+
+					// walk backwards until we find the target block
+					loop {
+						if *target_header.number() < target_number {
+							unreachable!(
+								"we are traversing backwards from a known block; \
+								 blocks are stored contiguously; \
+								 qed"
+							);
+						}
+
+						if *target_header.number() == target_number {
+							break;
+						}
+
+						target_header = self.inner.header(&BlockId::Hash(*target_header.parent_hash())).ok()?
+							.expect("Header known to exist after `best_containing` call; qed");
+					}
 
-					*base_header.number() + diff
+					target = target_header;
+					&target
+				} else {
+					// otherwise just use the given best as the target
+					&best_header
 				};
 
-				// unless our vote is currently being limited due to a pending change
-				let target = limit.map(|limit| limit.min(target)).unwrap_or(target);
-
-				// walk backwards until we find the target block
-				loop {
-					if *best_header.number() < target { unreachable!(); }
-					if *best_header.number() == target {
-						return Some((best_hash, *best_header.number()));
-					}
-
-					best_hash = *best_header.parent_hash();
-					best_header = self.inner.header(&BlockId::Hash(best_hash)).ok()?
-						.expect("Header known to exist after `best_containing` call; qed");
-				}
+				// restrict vote according to the given voting rule, if the
+				// voting rule doesn't restrict the vote then we keep the
+				// previous target.
+				//
+				// note that we pass the original `best_header`, i.e. before the
+				// authority set limit filter, which can be considered a
+				// mandatory/implicit voting rule.
+				self.voting_rule
+					.restrict_vote(&*self.inner, &base_header, &best_header, target_header)
+					.or(Some((target_header.hash(), *target_header.number())))
 			},
 			Ok(None) => {
 				debug!(target: "afg", "Encountered error finding best chain containing {:?}: couldn't find target block", block);
@@ -519,9 +539,9 @@ pub(crate) fn ancestry<B, Block: BlockT<Hash=H256>, E, RA>(
 	Ok(tree_route.retracted().iter().skip(1).map(|e| e.hash).collect())
 }
 
-impl<B, E, Block: BlockT<Hash=H256>, N, RA, SC>
+impl<B, E, Block: BlockT<Hash=H256>, N, RA, SC, VR>
 	voter::Environment<Block::Hash, NumberFor<Block>>
-for Environment<B, E, Block, N, RA, SC>
+for Environment<B, E, Block, N, RA, SC, VR>
 where
 	Block: 'static,
 	B: Backend<Block, Blake2Hasher> + 'static,
@@ -530,6 +550,7 @@ where
 	N::In: 'static + Send,
 	RA: 'static + Send + Sync,
 	SC: SelectChain<Block> + 'static,
+	VR: VotingRule<Block, Client<B, E, Block, RA>>,
 	NumberFor<Block>: BlockNumberOps,
 {
 	type Timer = Box<dyn Future<Item = (), Error = Self::Error> + Send>;

core/finality-grandpa/src/lib.rs

@@ -92,11 +92,15 @@ mod justification;
 mod light_import;
 mod observer;
 mod until_imported;
+mod voting_rule;
 
 pub use communication::Network;
 pub use finality_proof::FinalityProofProvider;
 pub use light_import::light_block_import;
 pub use observer::run_grandpa_observer;
+pub use voting_rule::{
+	BeforeBestBlock, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder
+};
 
 use aux_schema::PersistentData;
 use environment::{Environment, VoterSetState};
@@ -466,7 +470,7 @@ fn register_finality_tracker_inherent_data_provider<B, E, Block: BlockT<Hash=H25
 }
 
 /// Parameters used to run Grandpa.
-pub struct GrandpaParams<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X> {
+pub struct GrandpaParams<B, E, Block: BlockT<Hash=H256>, N, RA, SC, VR, X> {
 	/// Configuration for the GRANDPA service.
 	pub config: Config,
 	/// A link to the block import worker.
@@ -479,19 +483,22 @@ pub struct GrandpaParams<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X> {
 	pub on_exit: X,
 	/// If supplied, can be used to hook on telemetry connection established events.
 	pub telemetry_on_connect: Option<mpsc::UnboundedReceiver<()>>,
+	/// A voting rule used to potentially restrict target votes.
+	pub voting_rule: VR,
 }
 
 /// Run a GRANDPA voter as a task. Provide configuration and a link to a
 /// block import worker that has already been instantiated with `block_import`.
-pub fn run_grandpa_voter<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
-	grandpa_params: GrandpaParams<B, E, Block, N, RA, SC, X>,
+pub fn run_grandpa_voter<B, E, Block: BlockT<Hash=H256>, N, RA, SC, VR, X>(
+	grandpa_params: GrandpaParams<B, E, Block, N, RA, SC, VR, X>,
 ) -> client::error::Result<impl Future<Item=(),Error=()> + Send + 'static> where
 	Block::Hash: Ord,
 	B: Backend<Block, Blake2Hasher> + 'static,
 	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + 'static,
 	N: Network<Block> + Send + Sync + 'static,
 	N::In: Send + 'static,
 	SC: SelectChain<Block> + 'static,
+	VR: VotingRule<Block, Client<B, E, Block, RA>> + Clone + 'static,
 	NumberFor<Block>: BlockNumberOps,
 	DigestFor<Block>: Encode,
 	RA: Send + Sync + 'static,
@@ -504,6 +511,7 @@ pub fn run_grandpa_voter<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
 		inherent_data_providers,
 		on_exit,
 		telemetry_on_connect,
+		voting_rule,
 	} = grandpa_params;
 
 	let LinkHalf {
@@ -556,8 +564,9 @@ pub fn run_grandpa_voter<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
 		config,
 		network,
 		select_chain,
+		voting_rule,
 		persistent_data,
-		voter_commands_rx
+		voter_commands_rx,
 	);
 
 	let voter_work = voter_work
@@ -578,13 +587,13 @@ pub fn run_grandpa_voter<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
 
 /// Future that powers the voter.
 #[must_use]
-struct VoterWork<B, E, Block: BlockT, N: Network<Block>, RA, SC> {
+struct VoterWork<B, E, Block: BlockT, N: Network<Block>, RA, SC, VR> {
 	voter: Box<dyn Future<Item = (), Error = CommandOrError<Block::Hash, NumberFor<Block>>> + Send>,
-	env: Arc<Environment<B, E, Block, N, RA, SC>>,
+	env: Arc<Environment<B, E, Block, N, RA, SC, VR>>,
 	voter_commands_rx: mpsc::UnboundedReceiver<VoterCommand<Block::Hash, NumberFor<Block>>>,
 }
 
-impl<B, E, Block, N, RA, SC> VoterWork<B, E, Block, N, RA, SC>
+impl<B, E, Block, N, RA, SC, VR> VoterWork<B, E, Block, N, RA, SC, VR>
 where
 	Block: BlockT<Hash=H256>,
 	N: Network<Block> + Sync,
@@ -594,12 +603,14 @@ where
 	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + 'static,
 	B: Backend<Block, Blake2Hasher> + 'static,
 	SC: SelectChain<Block> + 'static,
+	VR: VotingRule<Block, Client<B, E, Block, RA>> + Clone + 'static,
 {
 	fn new(
 		client: Arc<Client<B, E, Block, RA>>,
 		config: Config,
 		network: NetworkBridge<Block, N>,
 		select_chain: SC,
+		voting_rule: VR,
 		persistent_data: PersistentData<Block>,
 		voter_commands_rx: mpsc::UnboundedReceiver<VoterCommand<Block::Hash, NumberFor<Block>>>,
 	) -> Self {
@@ -608,6 +619,7 @@ where
 		let env = Arc::new(Environment {
 			inner: client,
 			select_chain,
+			voting_rule,
 			voters: Arc::new(voters),
 			config,
 			network,
@@ -731,6 +743,7 @@ where
 					authority_set: self.env.authority_set.clone(),
 					consensus_changes: self.env.consensus_changes.clone(),
 					network: self.env.network.clone(),
+					voting_rule: self.env.voting_rule.clone(),
 				});
 
 				self.rebuild_voter();
@@ -755,7 +768,7 @@ where
 	}
 }
 
-impl<B, E, Block, N, RA, SC> Future for VoterWork<B, E, Block, N, RA, SC>
+impl<B, E, Block, N, RA, SC, VR> Future for VoterWork<B, E, Block, N, RA, SC, VR>
 where
 	Block: BlockT<Hash=H256>,
 	N: Network<Block> + Sync,
@@ -765,6 +778,7 @@ where
 	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + 'static,
 	B: Backend<Block, Blake2Hasher> + 'static,
 	SC: SelectChain<Block> + 'static,
+	VR: VotingRule<Block, Client<B, E, Block, RA>> + Clone + 'static,
 {
 	type Item = ();
 	type Error = Error;
@@ -809,8 +823,8 @@ where
 }
 
 #[deprecated(since = "1.1", note = "Please switch to run_grandpa_voter.")]
-pub fn run_grandpa<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
-	grandpa_params: GrandpaParams<B, E, Block, N, RA, SC, X>,
+pub fn run_grandpa<B, E, Block: BlockT<Hash=H256>, N, RA, SC, VR, X>(
+	grandpa_params: GrandpaParams<B, E, Block, N, RA, SC, VR, X>,
 ) -> ::client::error::Result<impl Future<Item=(),Error=()> + Send + 'static> where
 	Block::Hash: Ord,
 	B: Backend<Block, Blake2Hasher> + 'static,
@@ -821,6 +835,7 @@ pub fn run_grandpa<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
 	NumberFor<Block>: BlockNumberOps,
 	DigestFor<Block>: Encode,
 	RA: Send + Sync + 'static,
+	VR: VotingRule<Block, Client<B, E, Block, RA>> + Clone + 'static,
 	X: Future<Item=(),Error=()> + Clone + Send + 'static,
 {
 	run_grandpa_voter(grandpa_params)