From 60efa2042bf52ac344a2db003eb305345a8bdcde Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Silva?= <andre.beat@gmail.com>
Date: Fri, 9 Aug 2019 14:25:28 +0100
Subject: [PATCH] node: setup disabled grandpa properly

---
 core/finality-grandpa/src/lib.rs              | 29 ++++++++++++++++++-
 core/network/src/protocol/consensus_gossip.rs | 22 ++++++++++++++
 node/cli/src/service.rs                       | 13 ++-------
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs
index 73368a52d8330..f5010debdcf8d 100644
--- a/core/finality-grandpa/src/lib.rs
+++ b/core/finality-grandpa/src/lib.rs
@@ -444,7 +444,7 @@ fn global_communication<Block: BlockT<Hash=H256>, B, E, N, RA>(
 
 /// Register the finality tracker inherent data provider (which is used by
 /// GRANDPA), if not registered already.
-pub fn register_finality_tracker_inherent_data_provider<B, E, Block: BlockT<Hash=H256>, RA>(
+fn register_finality_tracker_inherent_data_provider<B, E, Block: BlockT<Hash=H256>, RA>(
 	client: Arc<Client<B, E, Block, RA>>,
 	inherent_data_providers: &InherentDataProviders,
 ) -> Result<(), consensus_common::Error> where
@@ -740,6 +740,33 @@ pub fn run_grandpa<B, E, Block: BlockT<Hash=H256>, N, RA, SC, X>(
 	run_grandpa_voter(grandpa_params)
 }
 
+/// When GRANDPA is not initialized we still need to register the finality
+/// tracker inherent provider which might be expected by the runtime for block
+/// authoring. Additionally, we register a gossip message validator that
+/// discards all GRANDPA messages (otherwise, we end up banning nodes that send
+/// us a `Neighbor` message, since there is no registered gossip validator for
+/// the engine id defined in the message.)
+pub fn setup_disabled_grandpa<B, E, Block: BlockT<Hash=H256>, RA, N>(
+	client: Arc<Client<B, E, Block, RA>>,
+	inherent_data_providers: &InherentDataProviders,
+	network: N,
+) -> Result<(), consensus_common::Error> where
+	B: Backend<Block, Blake2Hasher> + 'static,
+	E: CallExecutor<Block, Blake2Hasher> + Send + Sync + 'static,
+	RA: Send + Sync + 'static,
+	N: Network<Block> + Send + Sync + 'static,
+	N::In: Send + 'static,
+{
+	register_finality_tracker_inherent_data_provider(
+		client,
+		inherent_data_providers,
+	)?;
+
+	network.register_validator(Arc::new(network::consensus_gossip::DiscardAll));
+
+	Ok(())
+}
+
 /// Checks if this node is a voter in the given voter set.
 ///
 /// Returns the key pair of the node that is being used in the current voter set or `None`.
diff --git a/core/network/src/protocol/consensus_gossip.rs b/core/network/src/protocol/consensus_gossip.rs
index c2a2debaea24a..328fe197ea540 100644
--- a/core/network/src/protocol/consensus_gossip.rs
+++ b/core/network/src/protocol/consensus_gossip.rs
@@ -554,6 +554,28 @@ impl<B: BlockT> ConsensusGossip<B> {
 	}
 }
 
+/// A gossip message validator that discards all messages.
+pub struct DiscardAll;
+
+impl<B: BlockT> Validator<B> for DiscardAll {
+	fn validate(
+		&self,
+		_context: &mut dyn ValidatorContext<B>,
+		_sender: &PeerId,
+		_data: &[u8],
+	) -> ValidationResult<B::Hash> {
+		ValidationResult::Discard
+	}
+
+	fn message_expired<'a>(&'a self) -> Box<dyn FnMut(B::Hash, &[u8]) -> bool + 'a> {
+		Box::new(move |_topic, _data| true)
+	}
+
+	fn message_allowed<'a>(&'a self) -> Box<dyn FnMut(&PeerId, MessageIntent, &B::Hash, &[u8]) -> bool + 'a> {
+		Box::new(move |_who, _intent, _topic, _data| false)
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use sr_primitives::testing::{H256, Block as RawBlock, ExtrinsicWrapper};
diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs
index f1fba54829f0d..57a360f3ca0dd 100644
--- a/node/cli/src/service.rs
+++ b/node/cli/src/service.rs
@@ -158,9 +158,6 @@ construct_service_factory! {
 							service.on_exit(),
 						)?));
 					},
-					(false, true) => {
-						// nothing to do here
-					},
 					(true, false) => {
 						// start the full GRANDPA voter
 						let telemetry_on_connect = TelemetryOnConnect {
@@ -176,15 +173,11 @@ construct_service_factory! {
 						};
 						service.spawn_task(Box::new(grandpa::run_grandpa_voter(grandpa_config)?));
 					},
-					(true, true) => {
-						// since we are an authority, when authoring blocks we
-						// expect inherent data regarding what our last
-						// finalized block is, to be available. since we don't
-						// start the grandpa voter, we need to register the
-						// inherent data provider ourselves.
-						grandpa::register_finality_tracker_inherent_data_provider(
+					(_, true) => {
+						grandpa::setup_disabled_grandpa(
 							service.client(),
 							&service.config().custom.inherent_data_providers,
+							service.network(),
 						)?;
 					},
 				}