grandpa: enable light clients to participate in gossip (#8796)

* network: allow gossiping to light clients

* grandpa: gossip global messages to light clients

* grandpa: don't send neighbor packets to light clients

* grandpa: fix tests

* grandpa: export run_grandpa_observer

* node: run grandpa observer on light client

* node: start network at end

* Use wasm_timer in finality-grandpa

Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>

substrate/Cargo.lock

@@ -7470,6 +7470,7 @@ dependencies = [
  "substrate-test-runtime-client",
  "tempfile",
  "tokio 0.2.25",
+ "wasm-timer",
 ]
 
 [[package]]

substrate/bin/node-template/node/src/service.rs

@@ -269,7 +269,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
 		name: Some(name),
 		observer_enabled: false,
 		keystore,
-		is_authority: role.is_authority(),
+		local_role: role,
 		telemetry: telemetry.as_ref().map(|x| x.handle()),
 	};
 
@@ -337,7 +337,7 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError>
 		on_demand.clone(),
 	));
 
-	let (grandpa_block_import, _) = sc_finality_grandpa::block_import(
+	let (grandpa_block_import, grandpa_link) = sc_finality_grandpa::block_import(
 		client.clone(),
 		&(client.clone() as Arc<_>),
 		select_chain.clone(),
@@ -387,6 +387,26 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError>
 		);
 	}
 
+	let enable_grandpa = !config.disable_grandpa;
+	if enable_grandpa {
+		let name = config.network.node_name.clone();
+
+		let config = sc_finality_grandpa::Config {
+			gossip_duration: std::time::Duration::from_millis(333),
+			justification_period: 512,
+			name: Some(name),
+			observer_enabled: false,
+			keystore: None,
+			local_role: config.role.clone(),
+			telemetry: telemetry.as_ref().map(|x| x.handle()),
+		};
+
+		task_manager.spawn_handle().spawn_blocking(
+			"grandpa-observer",
+			sc_finality_grandpa::run_grandpa_observer(config, grandpa_link, network.clone())?,
+		);
+	}
+
 	sc_service::spawn_tasks(sc_service::SpawnTasksParams {
 		remote_blockchain: Some(backend.remote_blockchain()),
 		transaction_pool,
@@ -404,6 +424,5 @@ pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError>
 	})?;
 
 	network_starter.start_network();
-
 	Ok(task_manager)
 }

substrate/bin/node/cli/src/service.rs

@@ -381,7 +381,7 @@ pub fn new_full_base(
 		name: Some(name),
 		observer_enabled: false,
 		keystore,
-		is_authority: role.is_authority(),
+		local_role: role,
 		telemetry: telemetry.as_ref().map(|x| x.handle()),
 	};
 
@@ -478,7 +478,7 @@ pub fn new_light_base(
 		on_demand.clone(),
 	));
 
-	let (grandpa_block_import, _) = grandpa::block_import(
+	let (grandpa_block_import, grandpa_link) = grandpa::block_import(
 		client.clone(),
 		&(client.clone() as Arc<_>),
 		select_chain.clone(),
@@ -529,11 +529,33 @@ pub fn new_light_base(
 			on_demand: Some(on_demand.clone()),
 			block_announce_validator_builder: None,
 		})?;
-	network_starter.start_network();
+
+	let enable_grandpa = !config.disable_grandpa;
+	if enable_grandpa {
+		let name = config.network.node_name.clone();
+
+		let config = grandpa::Config {
+			gossip_duration: std::time::Duration::from_millis(333),
+			justification_period: 512,
+			name: Some(name),
+			observer_enabled: false,
+			keystore: None,
+			local_role: config.role.clone(),
+			telemetry: telemetry.as_ref().map(|x| x.handle()),
+		};
+
+		task_manager.spawn_handle().spawn_blocking(
+			"grandpa-observer",
+			grandpa::run_grandpa_observer(config, grandpa_link, network.clone())?,
+		);
+	}
 
 	if config.offchain_worker.enabled {
 		sc_service::build_offchain_workers(
-			&config, task_manager.spawn_handle(), client.clone(), network.clone(),
+			&config,
+			task_manager.spawn_handle(),
+			client.clone(),
+			network.clone(),
 		);
 	}
 
@@ -560,6 +582,7 @@ pub fn new_light_base(
 			telemetry: telemetry.as_mut(),
 		})?;
 
+	network_starter.start_network();
 	Ok((
 		task_manager,
 		rpc_handlers,

substrate/client/finality-grandpa/Cargo.toml

@@ -48,6 +48,7 @@ finality-grandpa = { version = "0.14.0", features = ["derive-codec"] }
 pin-project = "1.0.4"
 linked-hash-map = "0.5.2"
 async-trait = "0.1.42"
+wasm-timer = "0.2"
 
 [dev-dependencies]
 assert_matches = "1.3.0"

substrate/client/finality-grandpa/src/communication/gossip.rs

@@ -100,7 +100,8 @@ use crate::{environment, CatchUp, CompactCommit, SignedMessage};
 use super::{cost, benefit, Round, SetId};
 
 use std::collections::{HashMap, VecDeque, HashSet};
-use std::time::{Duration, Instant};
+use std::time::Duration;
+use wasm_timer::Instant;
 
 const REBROADCAST_AFTER: Duration = Duration::from_secs(60 * 5);
 const CATCH_UP_REQUEST_TIMEOUT: Duration = Duration::from_secs(45);
@@ -494,10 +495,10 @@ impl<N: Ord> Peers<N> {
 		match role {
 			ObservedRole::Authority if self.lucky_authorities.len() < MIN_LUCKY => {
 				self.lucky_authorities.insert(who.clone());
-			},
-			ObservedRole::Full | ObservedRole::Light if self.lucky_peers.len() < MIN_LUCKY => {
+			}
+			ObservedRole::Full if self.lucky_peers.len() < MIN_LUCKY => {
 				self.lucky_peers.insert(who.clone());
-			},
+			}
 			_ => {}
 		}
 		self.inner.insert(who, PeerInfo::new(role));
@@ -562,27 +563,43 @@ impl<N: Ord> Peers<N> {
 		self.inner.get(who)
 	}
 
-	fn authorities(&self) -> usize {
-		self.inner.iter().filter(|(_, info)| matches!(info.roles, ObservedRole::Authority)).count()
-	}
-
-	fn non_authorities(&self) -> usize {
+	fn connected_authorities(&self) -> usize {
 		self.inner
 			.iter()
-			.filter(|(_, info)| matches!(info.roles, ObservedRole::Full | ObservedRole::Light))
+			.filter(|(_, info)| matches!(info.roles, ObservedRole::Authority))
+			.count()
+	}
+
+	fn connected_full(&self) -> usize {
+		self.inner
+			.iter()
+			.filter(|(_, info)| matches!(info.roles, ObservedRole::Full))
 			.count()
 	}
 
 	fn reshuffle(&mut self) {
-		let mut lucky_peers: Vec<_> = self.inner
+		let mut lucky_peers: Vec<_> = self
+			.inner
 			.iter()
-			.filter_map(|(id, info)|
-				if matches!(info.roles, ObservedRole::Full | ObservedRole::Light) { Some(id.clone()) } else { None })
+			.filter_map(|(id, info)| {
+				if matches!(info.roles, ObservedRole::Full) {
+					Some(id.clone())
+				} else {
+					None
+				}
+			})
 			.collect();
-		let mut lucky_authorities: Vec<_> = self.inner
+
+		let mut lucky_authorities: Vec<_> = self
+			.inner
 			.iter()
-			.filter_map(|(id, info)|
-				if matches!(info.roles, ObservedRole::Authority) { Some(id.clone()) } else { None })
+			.filter_map(|(id, info)| {
+				if matches!(info.roles, ObservedRole::Authority) {
+					Some(id.clone())
+				} else {
+					None
+				}
+			})
 			.collect();
 
 		let num_non_authorities = ((lucky_peers.len() as f32).sqrt() as usize)
@@ -662,10 +679,14 @@ impl CatchUpConfig {
 	fn request_allowed<N>(&self, peer: &PeerInfo<N>) -> bool {
 		match self {
 			CatchUpConfig::Disabled => false,
-			CatchUpConfig::Enabled { only_from_authorities, .. } => match peer.roles {
+			CatchUpConfig::Enabled {
+				only_from_authorities,
+				..
+			} => match peer.roles {
 				ObservedRole::Authority => true,
-				_ => !only_from_authorities
-			}
+				ObservedRole::Light => false,
+				ObservedRole::Full => !only_from_authorities,
+			},
 		}
 	}
 }
@@ -685,8 +706,12 @@ type MaybeMessage<Block> = Option<(Vec<PeerId>, NeighborPacket<NumberFor<Block>>
 
 impl<Block: BlockT> Inner<Block> {
 	fn new(config: crate::Config) -> Self {
-		let catch_up_config = if config.observer_enabled {
-			if config.is_authority {
+		let catch_up_config = if config.local_role.is_light() {
+			// if we are a light client we shouldn't be issuing any catch-up requests
+			// as we don't participate in the full GRANDPA protocol
+			CatchUpConfig::disabled()
+		} else if config.observer_enabled {
+			if config.local_role.is_authority() {
 				// since the observer protocol is enabled, we will only issue
 				// catch-up requests if we are an authority (and only to other
 				// authorities).
@@ -697,8 +722,8 @@ impl<Block: BlockT> Inner<Block> {
 				CatchUpConfig::disabled()
 			}
 		} else {
-			// if the observer protocol isn't enabled, then any full node should
-			// be able to answer catch-up requests.
+			// if the observer protocol isn't enabled and we're not a light client, then any full
+			// node should be able to answer catch-up requests.
 			CatchUpConfig::enabled(false)
 		};
 
@@ -1103,7 +1128,22 @@ impl<Block: BlockT> Inner<Block> {
 				commit_finalized_height: *local_view.last_commit_height().unwrap_or(&Zero::zero()),
 			};
 
-			let peers = self.peers.inner.keys().cloned().collect();
+			let peers = self
+				.peers
+				.inner
+				.iter()
+				.filter_map(|(id, info)| {
+					// light clients don't participate in the full GRANDPA voter protocol
+					// and therefore don't need to be informed about view updates
+					if info.roles.is_light() {
+						None
+					} else {
+						Some(id)
+					}
+				})
+				.cloned()
+				.collect();
+
 			(peers, packet)
 		})
 	}
@@ -1157,7 +1197,7 @@ impl<Block: BlockT> Inner<Block> {
 			None => return false,
 		};
 
-		if !self.config.is_authority
+		if !self.config.local_role.is_authority()
 			&& round_elapsed < round_duration * PROPAGATION_ALL
 		{
 			// non-authority nodes don't gossip any messages right away. we
@@ -1169,7 +1209,7 @@ impl<Block: BlockT> Inner<Block> {
 
 		match peer.roles {
 			ObservedRole::Authority => {
-				let authorities = self.peers.authorities();
+				let authorities = self.peers.connected_authorities();
 
 				// the target node is an authority, on the first round duration we start by
 				// sending the message to only `sqrt(authorities)` (if we're
@@ -1184,8 +1224,8 @@ impl<Block: BlockT> Inner<Block> {
 					// authorities for whom it is polite to do so
 					true
 				}
-			},
-			ObservedRole::Full | ObservedRole::Light => {
+			}
+			ObservedRole::Full => {
 				// the node is not an authority so we apply stricter filters
 				if round_elapsed >= round_duration * PROPAGATION_ALL {
 					// if we waited for 3 (or more) rounds
@@ -1197,7 +1237,12 @@ impl<Block: BlockT> Inner<Block> {
 				} else {
 					false
 				}
-			},
+			}
+			ObservedRole::Light => {
+				// we never gossip round messages to light clients as they don't
+				// participate in the full grandpa protocol
+				false
+			}
 		}
 	}
 
@@ -1224,7 +1269,7 @@ impl<Block: BlockT> Inner<Block> {
 
 		match peer.roles {
 			ObservedRole::Authority => {
-				let authorities = self.peers.authorities();
+				let authorities = self.peers.connected_authorities();
 
 				// the target node is an authority, on the first round duration we start by
 				// sending the message to only `sqrt(authorities)` (if we're
@@ -1239,9 +1284,9 @@ impl<Block: BlockT> Inner<Block> {
 					// authorities for whom it is polite to do so
 					true
 				}
-			},
+			}
 			ObservedRole::Full | ObservedRole::Light => {
-				let non_authorities = self.peers.non_authorities();
+				let non_authorities = self.peers.connected_full();
 
 				// the target node is not an authority, on the first and second
 				// round duration we start by sending the message to only
@@ -1638,6 +1683,7 @@ pub(super) struct PeerReport {
 mod tests {
 	use super::*;
 	use super::environment::SharedVoterSetState;
+	use sc_network::config::Role;
 	use sc_network_gossip::Validator as GossipValidatorT;
 	use sc_network_test::Block;
 	use sp_core::{crypto::Public, H256};
@@ -1649,7 +1695,7 @@ mod tests {
 			justification_period: 256,
 			keystore: None,
 			name: None,
-			is_authority: true,
+			local_role: Role::Authority,
 			observer_enabled: true,
 			telemetry: None,
 		}
@@ -2174,7 +2220,7 @@ mod tests {
 
 			// if the observer protocol is enabled and we are not an authority,
 			// then we don't issue any catch-up requests.
-			c.is_authority = false;
+			c.local_role = Role::Full;
 			c.observer_enabled = true;
 
 			c
@@ -2468,15 +2514,10 @@ mod tests {
 	fn non_authorities_never_gossip_messages_on_first_round_duration() {
 		let mut config = config();
 		config.gossip_duration = Duration::from_secs(300); // Set to high value to prevent test race
-		config.is_authority = false;
+		config.local_role = Role::Full;
 		let round_duration = config.gossip_duration * ROUND_DURATION;
 
-		let (val, _) = GossipValidator::<Block>::new(
-			config,
-			voter_set_state(),
-			None,
-			None,
-		);
+		let (val, _) = GossipValidator::<Block>::new(config, voter_set_state(), None, None);
 
 		// the validator start at set id 0
 		val.note_set(SetId(0), Vec::new(), |_, _| {});

substrate/client/finality-grandpa/src/communication/tests.rs

@@ -20,7 +20,7 @@
 
 use sp_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender};
 use futures::prelude::*;
-use sc_network::{Event as NetworkEvent, ObservedRole, PeerId};
+use sc_network::{config::Role, Event as NetworkEvent, ObservedRole, PeerId};
 use sc_network_test::{Block, Hash};
 use sc_network_gossip::Validator;
 use std::sync::Arc;
@@ -137,7 +137,7 @@ fn config() -> crate::Config {
 		justification_period: 256,
 		keystore: None,
 		name: None,
-		is_authority: true,
+		local_role: Role::Authority,
 		observer_enabled: true,
 		telemetry: None,
 	}

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

@@ -123,10 +123,11 @@ mod voting_rule;
 
 pub use authorities::{AuthoritySet, AuthoritySetChanges, SharedAuthoritySet};
 pub use aux_schema::best_justification;
-pub use finality_proof::{FinalityProof, FinalityProofProvider, FinalityProofError};
-pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream};
-pub use import::{find_scheduled_change, find_forced_change, GrandpaBlockImport};
+pub use finality_proof::{FinalityProof, FinalityProofError, FinalityProofProvider};
+pub use import::{find_forced_change, find_scheduled_change, GrandpaBlockImport};
 pub use justification::GrandpaJustification;
+pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream};
+pub use observer::run_grandpa_observer;
 pub use voting_rule::{
 	BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRuleResult,
 	VotingRulesBuilder,
@@ -134,9 +135,9 @@ pub use voting_rule::{
 pub use finality_grandpa::voter::report;
 
 use aux_schema::PersistentData;
+use communication::{Network as NetworkT, NetworkBridge};
 use environment::{Environment, VoterSetState};
 use until_imported::UntilGlobalMessageBlocksImported;
-use communication::{NetworkBridge, Network as NetworkT};
 use sp_finality_grandpa::{AuthorityList, AuthoritySignature, SetId};
 
 // Re-export these two because it's just so damn convenient.
@@ -265,8 +266,8 @@ pub struct Config {
 	/// protocol (we will only issue catch-up requests to authorities when the
 	/// observer protocol is enabled).
 	pub observer_enabled: bool,
-	/// Whether the node is running as an authority (i.e. running the full GRANDPA protocol).
-	pub is_authority: bool,
+	/// The role of the local node (i.e. authority, full-node or light).
+	pub local_role: sc_network::config::Role,
 	/// Some local identifier of the voter.
 	pub name: Option<String>,
 	/// The keystore that manages the keys of this node.

substrate/client/finality-grandpa/src/observer.rs

@@ -156,7 +156,6 @@ where
 /// already been instantiated with `block_import`.
 /// NOTE: this is currently not part of the crate's public API since we don't consider
 /// it stable enough to use on a live network.
-#[allow(unused)]
 pub fn run_grandpa_observer<BE, Block: BlockT, Client, N, SC>(
 	config: Config,
 	link: LinkHalf<Block, Client, SC>,

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

@@ -25,7 +25,7 @@ use sc_network_test::{
 	Block, BlockImportAdapter, Hash, PassThroughVerifier, Peer, PeersClient, PeersFullClient,
 	TestClient, TestNetFactory, FullPeerConfig,
 };
-use sc_network::config::ProtocolConfig;
+use sc_network::config::{ProtocolConfig, Role};
 use parking_lot::{RwLock, Mutex};
 use futures_timer::Delay;
 use futures::executor::block_on;
@@ -277,7 +277,7 @@ fn initialize_grandpa(
 				justification_period: 32,
 				keystore: Some(keystore),
 				name: Some(format!("peer#{}", peer_id)),
-				is_authority: true,
+				local_role: Role::Authority,
 				observer_enabled: true,
 				telemetry: None,
 			},
@@ -421,7 +421,7 @@ fn finalize_3_voters_1_full_observer() {
 				justification_period: 32,
 				keystore: None,
 				name: Some(format!("peer#{}", peer_id)),
-				is_authority: true,
+				local_role: Role::Authority,
 				observer_enabled: true,
 				telemetry: None,
 			},
@@ -524,7 +524,7 @@ fn transition_3_voters_twice_1_full_observer() {
 				justification_period: 32,
 				keystore: Some(keystore),
 				name: Some(format!("peer#{}", peer_id)),
-				is_authority: true,
+				local_role: Role::Authority,
 				observer_enabled: true,
 				telemetry: None,
 			},
@@ -952,7 +952,7 @@ fn voter_persists_its_votes() {
 			justification_period: 32,
 			keystore: Some(bob_keystore.clone()),
 			name: Some(format!("peer#{}", 1)),
-			is_authority: true,
+			local_role: Role::Authority,
 			observer_enabled: true,
 			telemetry: None,
 		};
@@ -995,7 +995,7 @@ fn voter_persists_its_votes() {
 				justification_period: 32,
 				keystore: Some(keystore),
 				name: Some(format!("peer#{}", 0)),
-				is_authority: true,
+				local_role: Role::Authority,
 				observer_enabled: true,
 				telemetry: None,
 			},
@@ -1036,7 +1036,7 @@ fn voter_persists_its_votes() {
 				justification_period: 32,
 				keystore: Some(keystore),
 				name: Some(format!("peer#{}", 0)),
-				is_authority: true,
+				local_role: Role::Authority,
 				observer_enabled: true,
 				telemetry: None,
 			},
@@ -1196,7 +1196,7 @@ fn finalize_3_voters_1_light_observer() {
 			justification_period: 32,
 			keystore: None,
 			name: Some("observer".to_string()),
-			is_authority: false,
+			local_role: Role::Full,
 			observer_enabled: true,
 			telemetry: None,
 		},
@@ -1238,7 +1238,7 @@ fn voter_catches_up_to_latest_round_when_behind() {
 				justification_period: 32,
 				keystore,
 				name: Some(format!("peer#{}", peer_id)),
-				is_authority: true,
+				local_role: Role::Authority,
 				observer_enabled: true,
 				telemetry: None,
 			},
@@ -1361,7 +1361,7 @@ where
 		justification_period: 32,
 		keystore,
 		name: None,
-		is_authority: true,
+		local_role: Role::Authority,
 		observer_enabled: true,
 		telemetry: None,
 	};

substrate/client/finality-grandpa/src/until_imported.rs

@@ -48,7 +48,8 @@ use std::collections::{HashMap, VecDeque};
 use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
-use std::time::{Duration, Instant};
+use std::time::Duration;
+use wasm_timer::Instant;
 
 const LOG_PENDING_INTERVAL: Duration = Duration::from_secs(15);
 

substrate/client/network-gossip/src/state_machine.rs

@@ -197,11 +197,6 @@ impl<B: BlockT> ConsensusGossip<B> {
 
 	/// Handle new connected peer.
 	pub fn new_peer(&mut self, network: &mut dyn Network<B>, who: PeerId, role: ObservedRole) {
-		// light nodes are not valid targets for consensus gossip messages
-		if role.is_light() {
-			return;
-		}
-
 		tracing::trace!(
 			target:"gossip",
 			%who,

substrate/client/network/src/config.rs

@@ -141,6 +141,11 @@ impl Role {
 	pub fn is_authority(&self) -> bool {
 		matches!(self, Role::Authority { .. })
 	}
+
+	/// True for `Role::Light`
+	pub fn is_light(&self) -> bool {
+		matches!(self, Role::Light { .. })
+	}
 }
 
 impl fmt::Display for Role {