remove ApprovalCheckingVotingRule

Author: ordian

node/service/src/grandpa_support.rs

@@ -23,156 +23,7 @@ use sp_runtime::generic::BlockId;
 use sp_runtime::traits::Header as _;
 
 #[cfg(feature = "full-node")]
-use {
-	polkadot_primitives::v1::{Hash, Block as PolkadotBlock, Header as PolkadotHeader},
-	polkadot_subsystem::messages::ApprovalVotingMessage,
-	polkadot_overseer::Handle,
-	futures::channel::oneshot,
-};
-
-/// A custom GRANDPA voting rule that acts as a diagnostic for the approval
-/// voting subsystem's desired votes.
-///
-/// The practical effect of this voting rule is to implement a fixed delay of
-/// blocks.
-#[cfg(feature = "full-node")]
-#[derive(Clone)]
-pub(crate) struct ApprovalCheckingVotingRule {
-	overseer: Handle,
-}
-
-#[cfg(feature = "full-node")]
-impl ApprovalCheckingVotingRule {
-	/// Create a new approval checking diagnostic voting rule.
-	pub fn new(overseer: Handle) -> Self {
-		Self {
-			overseer,
-		}
-	}
-}
-
-#[cfg(feature = "full-node")]
-#[derive(Debug, PartialEq)]
-/// Vote explicitly on the given hash.
-enum ParachainVotingRuleTarget<H, N> {
-	Explicit((H, N)),
-	/// Vote on the current target.
-	Current,
-	/// Vote on the base target - the minimal possible vote.
-	Base,
-}
-
-#[cfg(feature = "full-node")]
-fn approval_checking_vote_to_grandpa_vote<H, N: PartialOrd>(
-	approval_checking_vote: Option<(H, N)>,
-	current_number: N,
-) -> ParachainVotingRuleTarget<H, N> {
-	match approval_checking_vote {
-		Some((hash, number)) => if number > current_number {
-			// respect other voting rule.
-			ParachainVotingRuleTarget::Current
-		} else {
-			ParachainVotingRuleTarget::Explicit((hash, number))
-		},
-		// If approval-voting chooses 'None', that means we should vote on the base (last round estimate).
-		None => ParachainVotingRuleTarget::Base,
-	}
-}
-
-/// The maximum amount of unfinalized blocks we are willing to allow due to approval checking lag.
-/// This is a safety net that should be removed at some point in the future.
-#[cfg(feature = "full-node")]
-const MAX_APPROVAL_CHECKING_FINALITY_LAG: polkadot_primitives::v1::BlockNumber = 50;
-
-#[cfg(feature = "full-node")]
-impl<B> grandpa::VotingRule<PolkadotBlock, B> for ApprovalCheckingVotingRule
-	where B: sp_blockchain::HeaderBackend<PolkadotBlock> + 'static
-{
-	fn restrict_vote(
-		&self,
-		backend: Arc<B>,
-		base: &PolkadotHeader,
-		best_target: &PolkadotHeader,
-		current_target: &PolkadotHeader,
-	) -> grandpa::VotingRuleResult<PolkadotBlock> {
-		// Query approval checking and issue metrics.
-		let mut overseer = self.overseer.clone();
-
-		let best_hash = best_target.hash();
-		let best_number = best_target.number.clone();
-		let best_header = best_target.clone();
-
-		let current_hash = current_target.hash();
-		let current_number = current_target.number.clone();
-
-		let base_hash = base.hash();
-		let base_number = base.number;
-
-		Box::pin(async move {
-			let (tx, rx) = oneshot::channel();
-			let approval_checking_subsystem_vote = {
-				overseer.send_msg(
-					ApprovalVotingMessage::ApprovedAncestor(
-						best_hash,
-						base_number,
-						tx,
-					),
-					std::any::type_name::<Self>(),
-				).await;
-
-				rx.await.ok().and_then(|v| v)
-			};
-
-			let approval_checking_subsystem_lag = approval_checking_subsystem_vote.map_or(
-				best_number - base_number,
-				|(_h, n)| best_number - n,
-			);
-
-			let min_vote = {
-				let diff = best_number.saturating_sub(base_number);
-				if diff >= MAX_APPROVAL_CHECKING_FINALITY_LAG {
-					// Catch up to the best, with some extra lag.
-					let target_number = best_number - MAX_APPROVAL_CHECKING_FINALITY_LAG;
-					if target_number >= current_number {
-						Some((current_hash, current_number))
-					} else {
-						walk_backwards_to_target_block(&*backend, target_number, &best_header).ok()
-					}
-				} else {
-					Some((base_hash, base_number))
-				}
-			};
-
-			let vote = match approval_checking_vote_to_grandpa_vote(
-				approval_checking_subsystem_vote,
-				current_number,
-			) {
-				ParachainVotingRuleTarget::Explicit(vote) => {
-					if min_vote.as_ref().map_or(false, |min| min.1 > vote.1) {
-						min_vote
-					} else {
-						Some(vote)
-					}
-				}
-				ParachainVotingRuleTarget::Current => Some((current_hash, current_number)),
-				ParachainVotingRuleTarget::Base => min_vote.or(Some((base_hash, base_number))),
-			};
-
-			tracing::trace!(
-				target: "parachain::approval-voting",
-				?vote,
-				?approval_checking_subsystem_vote,
-				approval_checking_subsystem_lag,
-				current_number,
-				best_number,
-				base_number,
-				"GRANDPA: voting based on approved ancestor.",
-			);
-
-			vote
-		})
-	}
-}
+use polkadot_primitives::v1::Hash;
 
 /// Returns the block hash of the block at the given `target_number` by walking
 /// backwards from the given `current_header`.
@@ -410,7 +261,6 @@ mod tests {
 	use sp_runtime::{generic::BlockId, traits::Header};
 	use consensus_common::BlockOrigin;
 	use std::sync::Arc;
-	use super::{approval_checking_vote_to_grandpa_vote, ParachainVotingRuleTarget};
 
 	#[test]
 	fn grandpa_pause_voting_rule_works() {
@@ -522,27 +372,4 @@ mod tests {
 			None,
 		);
 	}
-
-	#[test]
-	fn approval_checking_to_grandpa_rules() {
-		assert_eq!(
-			approval_checking_vote_to_grandpa_vote::<(), _>(None, 5),
-			ParachainVotingRuleTarget::Base,
-		);
-
-		assert_eq!(
-			approval_checking_vote_to_grandpa_vote(Some(("2", 2)), 3),
-			ParachainVotingRuleTarget::Explicit(("2", 2)),
-		);
-
-		assert_eq!(
-			approval_checking_vote_to_grandpa_vote(Some(("2", 2)), 2),
-			ParachainVotingRuleTarget::Explicit(("2", 2)),
-		);
-
-		assert_eq!(
-			approval_checking_vote_to_grandpa_vote(Some(("2", 2)), 1),
-			ParachainVotingRuleTarget::Current,
-		);
-	}
 }

node/service/src/lib.rs

@@ -907,23 +907,6 @@ pub fn new_full<RuntimeApi, Executor, OverseerGenerator>(
 		// after the given pause block is finalized and restarting after the
 		// given delay.
 		let builder = grandpa::VotingRulesBuilder::default();
-		// we should enable approval checking voting rule before we deploy parachains on polkadot
-		let enable_approval_checking_voting_rule = chain_spec.is_kusama()
-			|| chain_spec.is_westend()
-			|| chain_spec.is_rococo()
-			|| chain_spec.is_wococo();
-
-		let builder = if let Some(ref overseer) = overseer_handler {
-			if enable_approval_checking_voting_rule {
-				builder.add(grandpa_support::ApprovalCheckingVotingRule::new(
-					overseer.clone(),
-				))
-			} else {
-				builder
-			}
-		} else {
-			builder
-		};
 
 		let voting_rule = match grandpa_pause {
 			Some((block, delay)) => {