diff --git a/substrate/core/network/src/config.rs b/substrate/core/network/src/config.rs index 7e67d1cda8..965aeba8c9 100644 --- a/substrate/core/network/src/config.rs +++ b/substrate/core/network/src/config.rs @@ -73,6 +73,18 @@ bitflags! { } } +impl Roles { + /// Does this role represents a client that holds full chain data locally? + pub fn is_full(&self) -> bool { + self.intersects(Roles::FULL | Roles::AUTHORITY) + } + + /// Does this role represents a client that does not hold full chain data locally? + pub fn is_light(&self) -> bool { + !self.is_full() + } +} + impl parity_codec::Encode for Roles { fn encode_to(&self, dest: &mut T) { dest.push_byte(self.bits()) diff --git a/substrate/core/network/src/consensus_gossip.rs b/substrate/core/network/src/consensus_gossip.rs index 95753c54a7..c7747a9101 100644 --- a/substrate/core/network/src/consensus_gossip.rs +++ b/substrate/core/network/src/consensus_gossip.rs @@ -266,7 +266,7 @@ impl ConsensusGossip { /// Handle new connected peer. pub fn new_peer(&mut self, protocol: &mut Context, who: PeerId, roles: Roles) { // light nodes are not valid targets for consensus gossip messages - if !roles.intersects(Roles::FULL | Roles::AUTHORITY) { + if !roles.is_full() { return; } diff --git a/substrate/core/network/src/on_demand.rs b/substrate/core/network/src/on_demand.rs index 0d7538936d..aea762eefc 100644 --- a/substrate/core/network/src/on_demand.rs +++ b/substrate/core/network/src/on_demand.rs @@ -284,7 +284,7 @@ impl OnDemandService for OnDemand where B::Header: HeaderT, { fn on_connect(&self, peer: PeerId, role: Roles, best_number: NumberFor) { - if !role.intersects(Roles::FULL | Roles::AUTHORITY) { + if !role.is_full() { return; } diff --git a/substrate/core/network/src/protocol.rs b/substrate/core/network/src/protocol.rs index 2189a33928..355900d970 100644 --- a/substrate/core/network/src/protocol.rs +++ b/substrate/core/network/src/protocol.rs @@ -341,7 +341,7 @@ impl, H: ExHashT> Protocol { } /// Returns an object representing the status of the protocol. - pub fn status(&mut self) -> ProtocolStatus { + pub fn status(&self) -> ProtocolStatus { ProtocolStatus { sync: self.sync.status(), num_peers: self.context_data.peers.values().count(), @@ -611,6 +611,15 @@ impl, H: ExHashT> Protocol { request.from, request.to, request.max); + + // sending block requests to the node that is unable to serve it is considered a bad behavior + if !self.config.roles.is_full() { + trace!(target: "sync", "Peer {} is trying to sync from the light node", peer); + self.network_chan.send(NetworkMsg::DisconnectPeer(peer.clone())); + self.network_chan.send(NetworkMsg::ReportPeer(peer, i32::min_value())); + return; + } + let mut blocks = Vec::new(); let mut id = match request.from { message::FromBlock::Hash(h) => BlockId::Hash(h), @@ -772,7 +781,7 @@ impl, H: ExHashT> Protocol { self.network_chan.send(NetworkMsg::DisconnectPeer(who)); return; } - if self.config.roles & Roles::LIGHT == Roles::LIGHT { + if self.config.roles.is_light() { let self_best_block = self .context_data .chain @@ -961,7 +970,7 @@ impl, H: ExHashT> Protocol { ); // blocks are not announced by light clients - if self.config.roles & Roles::LIGHT == Roles::LIGHT { + if self.config.roles.is_light() { return; } diff --git a/substrate/core/network/src/sync.rs b/substrate/core/network/src/sync.rs index d5fc490a05..7358672231 100644 --- a/substrate/core/network/src/sync.rs +++ b/substrate/core/network/src/sync.rs @@ -181,7 +181,7 @@ impl ChainSync { info: &ClientInfo, ) -> Self { let mut required_block_attributes = message::BlockAttributes::HEADER | message::BlockAttributes::JUSTIFICATION; - if role.intersects(Roles::FULL | Roles::AUTHORITY) { + if role.is_full() { required_block_attributes |= message::BlockAttributes::BODY; } @@ -233,6 +233,12 @@ impl ChainSync { /// Handle new connected peer. Call this method whenever we connect to a new peer. pub(crate) fn new_peer(&mut self, protocol: &mut Context, who: PeerId) { if let Some(info) = protocol.peer_info(&who) { + // there's nothing sync can get from the node that has no blockchain data + // (the opposite is not true, but all requests are served at protocol level) + if !info.roles.is_full() { + return; + } + let status = block_status(&*protocol.client(), &self.queue_blocks, info.best_hash); match (status, info.best_number) { (Err(e), _) => { diff --git a/substrate/core/network/src/test/mod.rs b/substrate/core/network/src/test/mod.rs index c2deab5a32..a75cb4d24c 100644 --- a/substrate/core/network/src/test/mod.rs +++ b/substrate/core/network/src/test/mod.rs @@ -23,7 +23,6 @@ mod sync; use std::collections::{HashMap, HashSet, VecDeque}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; use log::trace; use crate::chain::FinalityProofProvider; @@ -45,7 +44,9 @@ use crate::message::Message; use network_libp2p::PeerId; use parking_lot::{Mutex, RwLock}; use primitives::{H256, sr25519::Public as AuthorityId, Blake2Hasher}; -use crate::protocol::{ConnectedPeer, Context, Protocol, ProtocolMsg, CustomMessageOutcome}; +use crate::protocol::{ + ConnectedPeer, Context, Protocol, ProtocolStatus, ProtocolMsg, CustomMessageOutcome, +}; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{AuthorityIdFor, Block as BlockT, Digest, DigestItem, Header, NumberFor}; use runtime_primitives::{Justification, ConsensusEngineId}; @@ -268,12 +269,11 @@ impl> Link for TestLink { } pub struct Peer> { - pub is_offline: Arc, - pub is_major_syncing: Arc, pub peers: Arc>>>, pub peer_id: PeerId, client: PeersClient, net_proto_channel: ProtocolChannel, + protocol_status: Arc>>, pub import_queue: Box>, pub data: D, best_hash: Mutex>, @@ -385,8 +385,7 @@ impl> ProtocolChannel { impl> Peer { fn new( - is_offline: Arc, - is_major_syncing: Arc, + protocol_status: Arc>>, peers: Arc>>>, client: PeersClient, import_queue: Box>, @@ -408,8 +407,7 @@ impl> Peer { ); import_queue.start(Box::new(network_link)).expect("Test ImportQueue always starts"); Peer { - is_offline, - is_major_syncing, + protocol_status, peers, peer_id: PeerId::random(), client, @@ -443,13 +441,18 @@ impl> Peer { /// SyncOracle: are we connected to any peer? #[cfg(test)] pub fn is_offline(&self) -> bool { - self.is_offline.load(std::sync::atomic::Ordering::Relaxed) + self.protocol_status.read().sync.is_offline() } /// SyncOracle: are we in the process of catching-up with the chain? #[cfg(test)] pub fn is_major_syncing(&self) -> bool { - self.is_major_syncing.load(std::sync::atomic::Ordering::Relaxed) + self.protocol_status.read().sync.is_major_syncing() + } + + /// Get protocol status. + pub fn protocol_status(&self) -> ProtocolStatus { + self.protocol_status.read().clone() } /// Called on connection to other indicated peer. @@ -741,8 +744,7 @@ pub trait TestNetFactory: Sized { /// Add created peer. fn add_peer( &mut self, - is_offline: Arc, - is_major_syncing: Arc, + protocol_status: Arc>>, import_queue: Box>, mut protocol: Protocol, mut network_to_protocol_rx: mpsc::UnboundedReceiver>, @@ -784,9 +786,7 @@ pub trait TestNetFactory: Sized { return Ok(Async::Ready(())) } - - is_offline.store(protocol.is_offline(), Ordering::Relaxed); - is_major_syncing.store(protocol.is_major_syncing(), Ordering::Relaxed); + *protocol_status.write() = protocol.status(); Ok(Async::NotReady) })); @@ -821,8 +821,6 @@ pub trait TestNetFactory: Sized { finality_proof_import, finality_proof_request_builder, )); - let is_offline = Arc::new(AtomicBool::new(true)); - let is_major_syncing = Arc::new(AtomicBool::new(false)); let specialization = self::SpecializationFactory::create(); let peers: Arc>>> = Arc::new(Default::default()); @@ -839,15 +837,14 @@ pub trait TestNetFactory: Sized { specialization, ).unwrap(); + let protocol_status = Arc::new(RwLock::new(protocol.status())); self.add_peer( - is_offline.clone(), - is_major_syncing.clone(), + protocol_status.clone(), import_queue.clone(), protocol, network_to_protocol_rx, Arc::new(Peer::new( - is_offline, - is_major_syncing, + protocol_status, peers, PeersClient::Full(client), import_queue, @@ -879,8 +876,6 @@ pub trait TestNetFactory: Sized { finality_proof_import, finality_proof_request_builder, )); - let is_offline = Arc::new(AtomicBool::new(true)); - let is_major_syncing = Arc::new(AtomicBool::new(false)); let specialization = self::SpecializationFactory::create(); let peers: Arc>>> = Arc::new(Default::default()); @@ -897,15 +892,14 @@ pub trait TestNetFactory: Sized { specialization, ).unwrap(); + let protocol_status = Arc::new(RwLock::new(protocol.status())); self.add_peer( - is_offline.clone(), - is_major_syncing.clone(), + protocol_status.clone(), import_queue.clone(), protocol, network_to_protocol_rx, Arc::new(Peer::new( - is_offline, - is_major_syncing, + protocol_status, peers, PeersClient::Light(client), import_queue, diff --git a/substrate/core/network/src/test/sync.rs b/substrate/core/network/src/test/sync.rs index 990d80e3f1..da42ae4758 100644 --- a/substrate/core/network/src/test/sync.rs +++ b/substrate/core/network/src/test/sync.rs @@ -16,6 +16,7 @@ use client::{backend::Backend, blockchain::HeaderBackend}; use crate::config::Roles; +use crate::message; use consensus::BlockOrigin; use std::collections::HashSet; use super::*; @@ -398,3 +399,56 @@ fn can_sync_small_non_best_forks() { assert!(net.peer(0).client().header(&BlockId::Hash(small_hash)).unwrap().is_some()); assert!(net.peer(1).client().header(&BlockId::Hash(small_hash)).unwrap().is_some()); } + +#[test] +fn can_not_sync_from_light_peer() { + let _ = ::env_logger::try_init(); + + // given the network with 1 full nodes (#0) and 1 light node (#1) + let mut net = TestNet::new(1); + net.add_light_peer(&Default::default()); + + // generate some blocks on #0 + net.peer(0).push_blocks(1, false); + + // and let the light client sync from this node + // (mind the #1 is disconnected && not syncing) + net.sync(); + + // ensure #0 && #1 have the same best block + let full0_info = net.peer(0).client.info().unwrap().chain; + let light_info = net.peer(1).client.info().unwrap().chain; + assert_eq!(full0_info.best_number, 1); + assert_eq!(light_info.best_number, 1); + assert_eq!(light_info.best_hash, full0_info.best_hash); + + // add new full client (#2) && sync without #0 + net.add_full_peer(&Default::default()); + net.peer(1).on_connect(net.peer(2)); + net.peer(2).on_connect(net.peer(1)); + net.peer(1).announce_block(light_info.best_hash); + net.sync_with(true, Some(vec![0].into_iter().collect())); + + // ensure that the #2 has failed to sync block #1 + assert_eq!(net.peer(2).client.info().unwrap().chain.best_number, 0); + // and that the #1 is still connected to #2 + // (because #2 has not tried to fetch block data from the #1 light node) + assert_eq!(net.peer(1).protocol_status().num_peers, 2); + + // and now try to fetch block data from light peer #1 + // (this should result in disconnect) + net.peer(1).receive_message( + &net.peer(2).peer_id, + message::generic::Message::BlockRequest(message::generic::BlockRequest { + id: 0, + fields: message::BlockAttributes::HEADER, + from: message::FromBlock::Hash(light_info.best_hash), + to: None, + direction: message::Direction::Ascending, + max: Some(1), + }), + ); + net.sync(); + // check that light #1 has disconnected from #2 + assert_eq!(net.peer(1).protocol_status().num_peers, 1); +} diff --git a/substrate/core/service/src/lib.rs b/substrate/core/service/src/lib.rs index 70b2a435a8..30a34f7fe2 100644 --- a/substrate/core/service/src/lib.rs +++ b/substrate/core/service/src/lib.rs @@ -169,7 +169,7 @@ impl Service { Components::build_transaction_pool(config.transaction_pool.clone(), client.clone())? ); let transaction_pool_adapter = Arc::new(TransactionPoolAdapter:: { - imports_external_transactions: !(config.roles == Roles::LIGHT), + imports_external_transactions: !config.roles.is_light(), pool: transaction_pool.clone(), client: client.clone(), });