diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index e4ff69448b23255802e83d12bdf5e83fb7545b02..c3e69b4db11b29bb8f2e5c19b2e146c132b3b1b3 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -7470,6 +7470,7 @@ dependencies = [ "substrate-test-runtime-client", "tempfile", "tokio 0.2.25", + "wasm-timer", ] [[package]] diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index 86b57f689e1e6a1fda9dc250a8de5a409df35abd..ed0a0463353cd0a942e8c05a8112202072f23b46 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/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) } diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index a13f8be9af13674ebaeecd1e1b52a9ecf67f66ef..6781402c948e1088b1e19485dc7854729ecefc46 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/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, diff --git a/substrate/client/finality-grandpa/Cargo.toml b/substrate/client/finality-grandpa/Cargo.toml index 1f21f454491b3ce80efa217f6f2fb797ba983ec2..ea91460972c9c52d10491183a8a325ba84b473e9 100644 --- a/substrate/client/finality-grandpa/Cargo.toml +++ b/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" diff --git a/substrate/client/finality-grandpa/src/communication/gossip.rs b/substrate/client/finality-grandpa/src/communication/gossip.rs index a6c51f7eeee7238589eae1e9d2ecf5a39f60d1d7..878a630d0e5189ec851e7d5966ff45c0a3d78230 100644 --- a/substrate/client/finality-grandpa/src/communication/gossip.rs +++ b/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 connected_authorities(&self) -> usize { + self.inner + .iter() + .filter(|(_, info)| matches!(info.roles, ObservedRole::Authority)) + .count() } - fn non_authorities(&self) -> usize { + fn connected_full(&self) -> usize { self.inner .iter() - .filter(|(_, info)| matches!(info.roles, ObservedRole::Full | ObservedRole::Light)) + .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(), |_, _| {}); diff --git a/substrate/client/finality-grandpa/src/communication/tests.rs b/substrate/client/finality-grandpa/src/communication/tests.rs index bfc5b1d10a41379eaff642619554cd7a9798067c..ec8c97dfe3e8a8e8ddecd89361545aefeeec765b 100644 --- a/substrate/client/finality-grandpa/src/communication/tests.rs +++ b/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, } diff --git a/substrate/client/finality-grandpa/src/lib.rs b/substrate/client/finality-grandpa/src/lib.rs index 672b08d0b714281506f00b0cdaba407970826695..f249d3982cf2512cc548965f14f92203ff2efafe 100644 --- a/substrate/client/finality-grandpa/src/lib.rs +++ b/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. diff --git a/substrate/client/finality-grandpa/src/observer.rs b/substrate/client/finality-grandpa/src/observer.rs index 827a7388d6033a6d7ff27f0999dcc80006b2ca68..5434cd08a91d0973ffe9182c9d64468b6fae1f33 100644 --- a/substrate/client/finality-grandpa/src/observer.rs +++ b/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>, diff --git a/substrate/client/finality-grandpa/src/tests.rs b/substrate/client/finality-grandpa/src/tests.rs index fa4bd028bfe25e023c8f04d95b7e8af34e30d35d..475c11191b10cec7c23f891bc26bc6f0b2b931c7 100644 --- a/substrate/client/finality-grandpa/src/tests.rs +++ b/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, }; diff --git a/substrate/client/finality-grandpa/src/until_imported.rs b/substrate/client/finality-grandpa/src/until_imported.rs index bcde68d2fb338f2b96d5f262ef4236b4bfd54e3d..d2e896685658b6291a472e045cf424eac0c8e7ec 100644 --- a/substrate/client/finality-grandpa/src/until_imported.rs +++ b/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); diff --git a/substrate/client/network-gossip/src/state_machine.rs b/substrate/client/network-gossip/src/state_machine.rs index 4c006f288f011763abc0944f8530518544dcdf8c..74f716133b478b283cde86ef4543209753e146d9 100644 --- a/substrate/client/network-gossip/src/state_machine.rs +++ b/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, diff --git a/substrate/client/network/src/config.rs b/substrate/client/network/src/config.rs index 77618f27711486fa886a6059ccf2676e565605e8..a742d8c95274d83e2844cd17d9be417e4d896e1e 100644 --- a/substrate/client/network/src/config.rs +++ b/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 {