From ae1ceb9bed8ddb0767122d727705803289bfb0ae Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 11 May 2022 11:00:05 +0300 Subject: [PATCH 01/15] Remove direct dependency of `sc-network` on `sc-network-light` --- Cargo.lock | 2 ++ client/network/Cargo.toml | 2 +- client/network/src/lib.rs | 1 - client/network/test/Cargo.toml | 1 + client/network/test/src/lib.rs | 2 +- client/service/Cargo.toml | 1 + client/service/src/builder.rs | 2 +- 7 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1683cc2b06b69..024a95da08a8f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8634,6 +8634,7 @@ dependencies = [ "sc-consensus", "sc-network", "sc-network-common", + "sc-network-light", "sc-service", "sp-blockchain", "sp-consensus", @@ -8816,6 +8817,7 @@ dependencies = [ "sc-keystore", "sc-network", "sc-network-common", + "sc-network-light", "sc-offchain", "sc-rpc", "sc-rpc-server", diff --git a/client/network/Cargo.toml b/client/network/Cargo.toml index 7e12d9862f8ef..ba250475dfc92 100644 --- a/client/network/Cargo.toml +++ b/client/network/Cargo.toml @@ -51,7 +51,6 @@ sc-block-builder = { version = "0.10.0-dev", path = "../block-builder" } sc-client-api = { version = "4.0.0-dev", path = "../api" } sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } sc-network-common = { version = "0.10.0-dev", path = "./common" } -sc-network-light = { version = "0.10.0-dev", path = "./light" } sc-network-sync = { version = "0.10.0-dev", path = "./sync" } sc-peerset = { version = "4.0.0-dev", path = "../peerset" } sc-utils = { version = "4.0.0-dev", path = "../utils" } @@ -68,6 +67,7 @@ async-std = "1.11.0" quickcheck = "1.0.3" rand = "0.7.2" tempfile = "3.1.0" +sc-network-light = { version = "0.10.0-dev", path = "./light" } sp-test-primitives = { version = "2.0.0", path = "../../primitives/test-primitives" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } substrate-test-runtime = { version = "2.0.0", path = "../../test-utils/runtime" } diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index fff30550eb8c9..77a4afaa3af5e 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -266,7 +266,6 @@ pub use protocol::{ event::{DhtEvent, Event, ObservedRole}, PeerInfo, }; -pub use sc_network_light::light_client_requests; pub use sc_network_sync::{ block_request_handler, state::StateDownloadProgress, diff --git a/client/network/test/Cargo.toml b/client/network/test/Cargo.toml index 025658afcad22..78ea2454b90e0 100644 --- a/client/network/test/Cargo.toml +++ b/client/network/test/Cargo.toml @@ -26,6 +26,7 @@ sc-client-api = { version = "4.0.0-dev", path = "../../api" } sc-consensus = { version = "0.10.0-dev", path = "../../consensus/common" } sc-network = { version = "0.10.0-dev", path = "../" } sc-network-common = { version = "0.10.0-dev", path = "../common" } +sc-network-light = { version = "0.10.0-dev", path = "../light" } sc-service = { version = "0.10.0-dev", default-features = false, features = ["test-helpers"], path = "../../service" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 9e23a4cc678e9..c933ce36b922d 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -53,11 +53,11 @@ use sc_network::{ MultiaddrWithPeerId, NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, ProtocolConfig, Role, SyncMode, TransportConfig, }, - light_client_requests::handler::LightClientRequestHandler, state_request_handler::StateRequestHandler, warp_request_handler, Multiaddr, NetworkService, NetworkWorker, }; pub use sc_network_common::config::ProtocolId; +use sc_network_light::light_client_requests::handler::LightClientRequestHandler; use sc_service::client::Client; use sp_blockchain::{ well_known_cache_keys::{self, Id as CacheKeyId}, diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index d59e8e8d19c4c..fa578485816ab 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -51,6 +51,7 @@ sc-consensus = { version = "0.10.0-dev", path = "../../client/consensus/common" sp-inherents = { version = "4.0.0-dev", path = "../../primitives/inherents" } sp-storage = { version = "6.0.0", path = "../../primitives/storage" } sc-network-common = { version = "0.10.0-dev", path = "../network/common" } +sc-network-light = { version = "0.10.0-dev", path = "../network/light" } sc-network = { version = "0.10.0-dev", path = "../network" } sc-chain-spec = { version = "4.0.0-dev", path = "../chain-spec" } sc-client-api = { version = "4.0.0-dev", path = "../api" } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 5319bf24d5e72..a8ecae938db6d 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -40,11 +40,11 @@ use sc_keystore::LocalKeystore; use sc_network::{ block_request_handler::{self, BlockRequestHandler}, config::{Role, SyncMode}, - light_client_requests::{self, handler::LightClientRequestHandler}, state_request_handler::{self, StateRequestHandler}, warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler, WarpSyncProvider}, NetworkService, }; +use sc_network_light::light_client_requests::{self, handler::LightClientRequestHandler}; use sc_rpc::{ author::AuthorApiServer, chain::ChainApiServer, From 427d3163dc26bac62c6592fb93a79f9d5f7dae7b Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 11 May 2022 11:31:32 +0300 Subject: [PATCH 02/15] Move `WarpSyncProvider` trait and surrounding data structures into `sc-network-common` --- Cargo.lock | 3 + client/finality-grandpa/Cargo.toml | 1 + client/finality-grandpa/src/warp_proof.rs | 2 +- client/network/common/Cargo.toml | 2 + client/network/common/src/lib.rs | 1 + .../network/common/src/warp_sync_provider.rs | 59 +++++++++++++++++++ client/network/src/config.rs | 2 +- client/network/src/protocol.rs | 14 +++-- client/network/sync/src/lib.rs | 6 +- client/network/sync/src/warp.rs | 6 +- .../network/sync/src/warp_request_handler.rs | 45 +------------- client/network/test/src/lib.rs | 21 +++---- client/service/src/builder.rs | 3 +- 13 files changed, 97 insertions(+), 68 deletions(-) create mode 100644 client/network/common/src/warp_sync_provider.rs diff --git a/Cargo.lock b/Cargo.lock index 024a95da08a8f..1ac5d8778d81f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8395,6 +8395,7 @@ dependencies = [ "sc-consensus", "sc-keystore", "sc-network", + "sc-network-common", "sc-network-gossip", "sc-network-test", "sc-telemetry", @@ -8544,6 +8545,8 @@ dependencies = [ "prost-build", "sc-peerset", "smallvec", + "sp-finality-grandpa", + "sp-runtime", ] [[package]] diff --git a/client/finality-grandpa/Cargo.toml b/client/finality-grandpa/Cargo.toml index a0e53da23fc54..bbd93c1a552af 100644 --- a/client/finality-grandpa/Cargo.toml +++ b/client/finality-grandpa/Cargo.toml @@ -36,6 +36,7 @@ sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } sc-keystore = { version = "4.0.0-dev", path = "../keystore" } sc-network = { version = "0.10.0-dev", path = "../network" } sc-network-gossip = { version = "0.10.0-dev", path = "../network-gossip" } +sc-network-common = { version = "0.10.0-dev", path = "../network/common" } sc-telemetry = { version = "4.0.0-dev", path = "../telemetry" } sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-api = { version = "4.0.0-dev", path = "../../primitives/api" } diff --git a/client/finality-grandpa/src/warp_proof.rs b/client/finality-grandpa/src/warp_proof.rs index 90f6828a1105d..b67eccede845f 100644 --- a/client/finality-grandpa/src/warp_proof.rs +++ b/client/finality-grandpa/src/warp_proof.rs @@ -23,7 +23,7 @@ use crate::{ BlockNumberOps, GrandpaJustification, SharedAuthoritySet, }; use sc_client_api::Backend as ClientBackend; -use sc_network::warp_request_handler::{EncodedProof, VerificationResult, WarpSyncProvider}; +use sc_network_common::warp_sync_provider::{EncodedProof, VerificationResult, WarpSyncProvider}; use sp_blockchain::{Backend as BlockchainBackend, HeaderBackend}; use sp_finality_grandpa::{AuthorityList, SetId, GRANDPA_ENGINE_ID}; use sp_runtime::{ diff --git a/client/network/common/Cargo.toml b/client/network/common/Cargo.toml index c41a7895888ae..2b594ec4091fb 100644 --- a/client/network/common/Cargo.toml +++ b/client/network/common/Cargo.toml @@ -24,3 +24,5 @@ futures = "0.3.21" libp2p = "0.44.0" smallvec = "1.8.0" sc-peerset = { version = "4.0.0-dev", path = "../../peerset" } +sp-finality-grandpa = { version = "4.0.0-dev", path = "../../../primitives/finality-grandpa" } +sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } diff --git a/client/network/common/src/lib.rs b/client/network/common/src/lib.rs index 81769e23debbb..387251b3e5f16 100644 --- a/client/network/common/src/lib.rs +++ b/client/network/common/src/lib.rs @@ -21,3 +21,4 @@ pub mod config; pub mod message; pub mod request_responses; +pub mod warp_sync_provider; diff --git a/client/network/common/src/warp_sync_provider.rs b/client/network/common/src/warp_sync_provider.rs new file mode 100644 index 0000000000000..db62283110718 --- /dev/null +++ b/client/network/common/src/warp_sync_provider.rs @@ -0,0 +1,59 @@ +// Copyright 2021 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Generic data structures for handling (i.e. answering) grandpa warp sync provider. + +use codec::{Decode, Encode}; +pub use sp_finality_grandpa::{AuthorityList, SetId}; +use sp_runtime::traits::Block as BlockT; + +/// Scale-encoded warp sync proof response. +pub struct EncodedProof(pub Vec); + +/// Warp sync request +#[derive(Encode, Decode, Debug)] +pub struct WarpProofRequest { + /// Start collecting proofs from this block. + pub begin: B::Hash, +} + +/// Proof verification result. +pub enum VerificationResult { + /// Proof is valid, but the target was not reached. + Partial(SetId, AuthorityList, Block::Hash), + /// Target finality is proved. + Complete(SetId, AuthorityList, Block::Header), +} + +/// Warp sync backend. Handles retrieveing and verifying warp sync proofs. +pub trait WarpSyncProvider: Send + Sync { + /// Generate proof starting at given block hash. The proof is accumulated until maximum proof + /// size is reached. + fn generate( + &self, + start: Block::Hash, + ) -> Result>; + /// Verify warp proof against current set of authorities. + fn verify( + &self, + proof: &EncodedProof, + set_id: SetId, + authorities: AuthorityList, + ) -> Result, Box>; + /// Get current list of authorities. This is supposed to be genesis authorities when starting + /// sync. + fn current_authorities(&self) -> AuthorityList; +} diff --git a/client/network/src/config.rs b/client/network/src/config.rs index e44977e5be6b3..202cd4ce938c8 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -26,8 +26,8 @@ pub use sc_network_common::{ request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, + warp_sync_provider::WarpSyncProvider, }; -pub use sc_network_sync::warp_request_handler::WarpSyncProvider; pub use libp2p::{build_multiaddr, core::PublicKey, identity}; diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 387a7b3fdde90..d0005853bd536 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -20,7 +20,6 @@ use crate::{ config, error, request_responses::RequestFailure, utils::{interval, LruHashSet}, - warp_request_handler::{EncodedProof, WarpSyncProvider}, }; use bytes::Bytes; @@ -48,7 +47,10 @@ use prometheus_endpoint::{register, Gauge, GaugeVec, Opts, PrometheusError, Regi use prost::Message as _; use sc_client_api::{BlockBackend, HeaderBackend, ProofProvider}; use sc_consensus::import_queue::{BlockImportError, BlockImportStatus, IncomingBlock, Origin}; -use sc_network_common::config::ProtocolId; +use sc_network_common::{ + config::ProtocolId, + warp_sync_provider::{EncodedProof, WarpProofRequest, WarpSyncProvider}, +}; use sc_network_sync::{ message::{ BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, BlockState, @@ -56,7 +58,7 @@ use sc_network_sync::{ }, schema::v1::StateResponse, BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, - PollBlockAnnounceValidation, Status as SyncStatus, + PollBlockAnnounceValidation, SyncStatus, }; use sp_arithmetic::traits::SaturatedConversion; use sp_consensus::{block_validation::BlockAnnounceValidator, BlockOrigin}; @@ -741,7 +743,7 @@ where pub fn on_warp_sync_response( &mut self, peer_id: PeerId, - response: crate::warp_request_handler::EncodedProof, + response: EncodedProof, ) -> CustomMessageOutcome { match self.sync.on_warp_sync_data(&peer_id, response) { Ok(()) => CustomMessageOutcome::None, @@ -1305,7 +1307,7 @@ fn prepare_state_request( fn prepare_warp_sync_request( peers: &mut HashMap>, who: PeerId, - request: crate::warp_request_handler::Request, + request: WarpProofRequest, ) -> CustomMessageOutcome { let (tx, rx) = oneshot::channel(); @@ -1361,7 +1363,7 @@ pub enum CustomMessageOutcome { /// A new warp sync request must be emitted. WarpSyncRequest { target: PeerId, - request: crate::warp_request_handler::Request, + request: WarpProofRequest, pending_response: oneshot::Sender, RequestFailure>>, }, /// Peer has a reported a new head of chain. diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index bc0ed46c3f068..c92010c7f8f78 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -42,10 +42,7 @@ use crate::{ message::{BlockAnnounce, BlockAttributes, BlockRequest, BlockResponse}, schema::v1::{StateRequest, StateResponse}, state::{StateDownloadProgress, StateSync}, - warp::{ - EncodedProof, WarpProofImportResult, WarpProofRequest, WarpSync, WarpSyncPhase, - WarpSyncProgress, WarpSyncProvider, - }, + warp::{WarpProofImportResult, WarpSync, WarpSyncPhase, WarpSyncProgress}, }; use codec::Encode; use either::Either; @@ -55,6 +52,7 @@ use libp2p::PeerId; use log::{debug, error, info, trace, warn}; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; +use sc_network_common::warp_sync_provider::{EncodedProof, WarpProofRequest, WarpSyncProvider}; use sp_arithmetic::traits::Saturating; use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata}; use sp_consensus::{ diff --git a/client/network/sync/src/warp.rs b/client/network/sync/src/warp.rs index d3d9d7d244153..9ea4d70d9e9d0 100644 --- a/client/network/sync/src/warp.rs +++ b/client/network/sync/src/warp.rs @@ -18,14 +18,14 @@ //! Warp sync support. -pub use crate::warp_request_handler::{ - EncodedProof, Request as WarpProofRequest, VerificationResult, WarpSyncProvider, -}; use crate::{ schema::v1::{StateRequest, StateResponse}, state::{ImportResult, StateSync}, }; use sc_client_api::ProofProvider; +use sc_network_common::warp_sync_provider::{ + EncodedProof, VerificationResult, WarpProofRequest, WarpSyncProvider, +}; use sp_blockchain::HeaderBackend; use sp_finality_grandpa::{AuthorityList, SetId}; use sp_runtime::traits::{Block as BlockT, NumberFor, Zero}; diff --git a/client/network/sync/src/warp_request_handler.rs b/client/network/sync/src/warp_request_handler.rs index 4f66e0a6daf17..b6fffe6ee20d5 100644 --- a/client/network/sync/src/warp_request_handler.rs +++ b/client/network/sync/src/warp_request_handler.rs @@ -16,7 +16,7 @@ //! Helper for handling (i.e. answering) grandpa warp sync requests from a remote peer. -use codec::{Decode, Encode}; +use codec::Decode; use futures::{ channel::{mpsc, oneshot}, stream::StreamExt, @@ -27,52 +27,13 @@ use sc_network_common::{ request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, + warp_sync_provider::{EncodedProof, WarpProofRequest, WarpSyncProvider}, }; use sp_runtime::traits::Block as BlockT; use std::{sync::Arc, time::Duration}; -pub use sp_finality_grandpa::{AuthorityList, SetId}; - -/// Scale-encoded warp sync proof response. -pub struct EncodedProof(pub Vec); - -/// Warp sync request -#[derive(Encode, Decode, Debug)] -pub struct Request { - /// Start collecting proofs from this block. - pub begin: B::Hash, -} - const MAX_RESPONSE_SIZE: u64 = 16 * 1024 * 1024; -/// Proof verification result. -pub enum VerificationResult { - /// Proof is valid, but the target was not reached. - Partial(SetId, AuthorityList, Block::Hash), - /// Target finality is proved. - Complete(SetId, AuthorityList, Block::Header), -} - -/// Warp sync backend. Handles retrieveing and verifying warp sync proofs. -pub trait WarpSyncProvider: Send + Sync { - /// Generate proof starting at given block hash. The proof is accumulated until maximum proof - /// size is reached. - fn generate( - &self, - start: B::Hash, - ) -> Result>; - /// Verify warp proof against current set of authorities. - fn verify( - &self, - proof: &EncodedProof, - set_id: SetId, - authorities: AuthorityList, - ) -> Result, Box>; - /// Get current list of authorities. This is supposed to be genesis authorities when starting - /// sync. - fn current_authorities(&self) -> AuthorityList; -} - /// Generates a [`RequestResponseConfig`] for the grandpa warp sync request protocol, refusing /// incoming requests. pub fn generate_request_response_config(protocol_id: ProtocolId) -> RequestResponseConfig { @@ -115,7 +76,7 @@ impl RequestHandler { payload: Vec, pending_response: oneshot::Sender, ) -> Result<(), HandleRequestError> { - let request = Request::::decode(&mut &payload[..])?; + let request = WarpProofRequest::::decode(&mut &payload[..])?; let EncodedProof(proof) = self .backend diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index c933ce36b922d..57ec91bcf8b38 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -57,6 +57,7 @@ use sc_network::{ warp_request_handler, Multiaddr, NetworkService, NetworkWorker, }; pub use sc_network_common::config::ProtocolId; +use sc_network_common::warp_sync_provider; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; use sc_service::client::Client; use sp_blockchain::{ @@ -638,27 +639,27 @@ impl VerifierAdapter { struct TestWarpSyncProvider(Arc>); -impl warp_request_handler::WarpSyncProvider for TestWarpSyncProvider { +impl warp_sync_provider::WarpSyncProvider for TestWarpSyncProvider { fn generate( &self, _start: B::Hash, - ) -> Result> { + ) -> Result> { let info = self.0.info(); let best_header = self.0.header(BlockId::hash(info.best_hash)).unwrap().unwrap(); - Ok(warp_request_handler::EncodedProof(best_header.encode())) + Ok(warp_sync_provider::EncodedProof(best_header.encode())) } fn verify( &self, - proof: &warp_request_handler::EncodedProof, - _set_id: warp_request_handler::SetId, - _authorities: warp_request_handler::AuthorityList, - ) -> Result, Box> + proof: &warp_sync_provider::EncodedProof, + _set_id: warp_sync_provider::SetId, + _authorities: warp_sync_provider::AuthorityList, + ) -> Result, Box> { - let warp_request_handler::EncodedProof(encoded) = proof; + let warp_sync_provider::EncodedProof(encoded) = proof; let header = B::Header::decode(&mut encoded.as_slice()).unwrap(); - Ok(warp_request_handler::VerificationResult::Complete(0, Default::default(), header)) + Ok(warp_sync_provider::VerificationResult::Complete(0, Default::default(), header)) } - fn current_authorities(&self) -> warp_request_handler::AuthorityList { + fn current_authorities(&self) -> warp_sync_provider::AuthorityList { Default::default() } } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index a8ecae938db6d..ff7bcdfd99e7e 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -41,9 +41,10 @@ use sc_network::{ block_request_handler::{self, BlockRequestHandler}, config::{Role, SyncMode}, state_request_handler::{self, StateRequestHandler}, - warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler, WarpSyncProvider}, + warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler}, NetworkService, }; +use sc_network_common::warp_sync_provider::WarpSyncProvider; use sc_network_light::light_client_requests::{self, handler::LightClientRequestHandler}; use sc_rpc::{ author::AuthorApiServer, From 84ae2822a0f25c58a625317eeeadf4f3c00e2455 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 11 May 2022 11:50:05 +0300 Subject: [PATCH 03/15] Move `WarpSyncProvider` trait and surrounding data structures into `sc-network-common` --- client/finality-grandpa/src/warp_proof.rs | 2 +- client/network/common/src/lib.rs | 2 +- .../{warp_sync_provider.rs => warp_sync.rs} | 0 client/network/src/config.rs | 2 +- client/network/src/protocol.rs | 2 +- client/network/sync/src/lib.rs | 2 +- client/network/sync/src/warp.rs | 2 +- .../network/sync/src/warp_request_handler.rs | 2 +- client/network/test/src/lib.rs | 23 +++++++++---------- client/service/src/builder.rs | 2 +- 10 files changed, 19 insertions(+), 20 deletions(-) rename client/network/common/src/{warp_sync_provider.rs => warp_sync.rs} (100%) diff --git a/client/finality-grandpa/src/warp_proof.rs b/client/finality-grandpa/src/warp_proof.rs index b67eccede845f..b2088710284ab 100644 --- a/client/finality-grandpa/src/warp_proof.rs +++ b/client/finality-grandpa/src/warp_proof.rs @@ -23,7 +23,7 @@ use crate::{ BlockNumberOps, GrandpaJustification, SharedAuthoritySet, }; use sc_client_api::Backend as ClientBackend; -use sc_network_common::warp_sync_provider::{EncodedProof, VerificationResult, WarpSyncProvider}; +use sc_network_common::warp_sync::{EncodedProof, VerificationResult, WarpSyncProvider}; use sp_blockchain::{Backend as BlockchainBackend, HeaderBackend}; use sp_finality_grandpa::{AuthorityList, SetId, GRANDPA_ENGINE_ID}; use sp_runtime::{ diff --git a/client/network/common/src/lib.rs b/client/network/common/src/lib.rs index 387251b3e5f16..99e9af4c0b605 100644 --- a/client/network/common/src/lib.rs +++ b/client/network/common/src/lib.rs @@ -21,4 +21,4 @@ pub mod config; pub mod message; pub mod request_responses; -pub mod warp_sync_provider; +pub mod warp_sync; diff --git a/client/network/common/src/warp_sync_provider.rs b/client/network/common/src/warp_sync.rs similarity index 100% rename from client/network/common/src/warp_sync_provider.rs rename to client/network/common/src/warp_sync.rs diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 202cd4ce938c8..cb42550683699 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -26,7 +26,7 @@ pub use sc_network_common::{ request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, - warp_sync_provider::WarpSyncProvider, + warp_sync::WarpSyncProvider, }; pub use libp2p::{build_multiaddr, core::PublicKey, identity}; diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index d0005853bd536..1374c81efd748 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -49,7 +49,7 @@ use sc_client_api::{BlockBackend, HeaderBackend, ProofProvider}; use sc_consensus::import_queue::{BlockImportError, BlockImportStatus, IncomingBlock, Origin}; use sc_network_common::{ config::ProtocolId, - warp_sync_provider::{EncodedProof, WarpProofRequest, WarpSyncProvider}, + warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}, }; use sc_network_sync::{ message::{ diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index c92010c7f8f78..1c3cf56f7c8b8 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -52,7 +52,7 @@ use libp2p::PeerId; use log::{debug, error, info, trace, warn}; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; -use sc_network_common::warp_sync_provider::{EncodedProof, WarpProofRequest, WarpSyncProvider}; +use sc_network_common::warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}; use sp_arithmetic::traits::Saturating; use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata}; use sp_consensus::{ diff --git a/client/network/sync/src/warp.rs b/client/network/sync/src/warp.rs index 9ea4d70d9e9d0..d09f0f089ee59 100644 --- a/client/network/sync/src/warp.rs +++ b/client/network/sync/src/warp.rs @@ -23,7 +23,7 @@ use crate::{ state::{ImportResult, StateSync}, }; use sc_client_api::ProofProvider; -use sc_network_common::warp_sync_provider::{ +use sc_network_common::warp_sync::{ EncodedProof, VerificationResult, WarpProofRequest, WarpSyncProvider, }; use sp_blockchain::HeaderBackend; diff --git a/client/network/sync/src/warp_request_handler.rs b/client/network/sync/src/warp_request_handler.rs index b6fffe6ee20d5..5390c0b6cb944 100644 --- a/client/network/sync/src/warp_request_handler.rs +++ b/client/network/sync/src/warp_request_handler.rs @@ -27,7 +27,7 @@ use sc_network_common::{ request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, - warp_sync_provider::{EncodedProof, WarpProofRequest, WarpSyncProvider}, + warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}, }; use sp_runtime::traits::Block as BlockT; use std::{sync::Arc, time::Duration}; diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 57ec91bcf8b38..d833238f1342c 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -57,7 +57,7 @@ use sc_network::{ warp_request_handler, Multiaddr, NetworkService, NetworkWorker, }; pub use sc_network_common::config::ProtocolId; -use sc_network_common::warp_sync_provider; +use sc_network_common::warp_sync; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; use sc_service::client::Client; use sp_blockchain::{ @@ -639,27 +639,26 @@ impl VerifierAdapter { struct TestWarpSyncProvider(Arc>); -impl warp_sync_provider::WarpSyncProvider for TestWarpSyncProvider { +impl warp_sync::WarpSyncProvider for TestWarpSyncProvider { fn generate( &self, _start: B::Hash, - ) -> Result> { + ) -> Result> { let info = self.0.info(); let best_header = self.0.header(BlockId::hash(info.best_hash)).unwrap().unwrap(); - Ok(warp_sync_provider::EncodedProof(best_header.encode())) + Ok(warp_sync::EncodedProof(best_header.encode())) } fn verify( &self, - proof: &warp_sync_provider::EncodedProof, - _set_id: warp_sync_provider::SetId, - _authorities: warp_sync_provider::AuthorityList, - ) -> Result, Box> - { - let warp_sync_provider::EncodedProof(encoded) = proof; + proof: &warp_sync::EncodedProof, + _set_id: warp_sync::SetId, + _authorities: warp_sync::AuthorityList, + ) -> Result, Box> { + let warp_sync::EncodedProof(encoded) = proof; let header = B::Header::decode(&mut encoded.as_slice()).unwrap(); - Ok(warp_sync_provider::VerificationResult::Complete(0, Default::default(), header)) + Ok(warp_sync::VerificationResult::Complete(0, Default::default(), header)) } - fn current_authorities(&self) -> warp_sync_provider::AuthorityList { + fn current_authorities(&self) -> warp_sync::AuthorityList { Default::default() } } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index ff7bcdfd99e7e..f79fd1b846657 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -44,7 +44,7 @@ use sc_network::{ warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler}, NetworkService, }; -use sc_network_common::warp_sync_provider::WarpSyncProvider; +use sc_network_common::warp_sync::WarpSyncProvider; use sc_network_light::light_client_requests::{self, handler::LightClientRequestHandler}; use sc_rpc::{ author::AuthorApiServer, From 5dd28b0ef556cd256150b60459b5fcc9d278c8ad Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 12 May 2022 02:14:40 +0300 Subject: [PATCH 04/15] Create `sync` module in `sc-network-common`, create `ChainSync` trait there (not used yet), move a bunch of associated data structures from `sc-network-sync` --- Cargo.lock | 4 +- client/network/common/Cargo.toml | 3 + client/network/common/src/lib.rs | 5 +- client/network/common/src/sync.rs | 327 ++++++++++++++++++ .../network/common/src/sync/extra_requests.rs | 25 ++ .../{sync/src => common/src/sync}/message.rs | 2 +- .../common/src/{warp_sync.rs => sync/warp.rs} | 41 ++- client/network/src/lib.rs | 8 +- client/network/src/protocol.rs | 18 +- client/network/src/protocol/message.rs | 10 +- client/network/src/service.rs | 2 +- client/network/sync/Cargo.toml | 1 - .../network/sync/src/block_request_handler.rs | 6 +- client/network/sync/src/blocks.rs | 4 +- client/network/sync/src/extra_requests.rs | 11 +- client/network/sync/src/lib.rs | 229 +++--------- client/network/sync/src/state.rs | 10 +- client/network/sync/src/warp.rs | 45 +-- 18 files changed, 480 insertions(+), 271 deletions(-) create mode 100644 client/network/common/src/sync.rs create mode 100644 client/network/common/src/sync/extra_requests.rs rename client/network/{sync/src => common/src/sync}/message.rs (99%) rename client/network/common/src/{warp_sync.rs => sync/warp.rs} (63%) diff --git a/Cargo.lock b/Cargo.lock index 1ac5d8778d81f..d7383fa6bce0f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8539,12 +8539,15 @@ dependencies = [ name = "sc-network-common" version = "0.10.0-dev" dependencies = [ + "bitflags", "futures", "libp2p", "parity-scale-codec", "prost-build", + "sc-consensus", "sc-peerset", "smallvec", + "sp-consensus", "sp-finality-grandpa", "sp-runtime", ] @@ -8591,7 +8594,6 @@ dependencies = [ name = "sc-network-sync" version = "0.10.0-dev" dependencies = [ - "bitflags", "either", "fork-tree", "futures", diff --git a/client/network/common/Cargo.toml b/client/network/common/Cargo.toml index 2b594ec4091fb..f579283b21a6d 100644 --- a/client/network/common/Cargo.toml +++ b/client/network/common/Cargo.toml @@ -17,12 +17,15 @@ targets = ["x86_64-unknown-linux-gnu"] prost-build = "0.9" [dependencies] +bitflags = "1.3.2" codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive", ] } futures = "0.3.21" libp2p = "0.44.0" smallvec = "1.8.0" +sc-consensus = { version = "0.10.0-dev", path = "../../consensus/common" } sc-peerset = { version = "4.0.0-dev", path = "../../peerset" } +sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } sp-finality-grandpa = { version = "4.0.0-dev", path = "../../../primitives/finality-grandpa" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } diff --git a/client/network/common/src/lib.rs b/client/network/common/src/lib.rs index 99e9af4c0b605..70260584a6c3b 100644 --- a/client/network/common/src/lib.rs +++ b/client/network/common/src/lib.rs @@ -21,4 +21,7 @@ pub mod config; pub mod message; pub mod request_responses; -pub mod warp_sync; +pub mod sync; + +// TODO: Remove re-export +pub use sync::warp as warp_sync; diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs new file mode 100644 index 0000000000000..a8044d2fe9973 --- /dev/null +++ b/client/network/common/src/sync.rs @@ -0,0 +1,327 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! Abstract interfaces and data structures related to network sync. + +pub mod extra_requests; +pub mod message; +pub mod warp; + +use libp2p::PeerId; +use message::{BlockAnnounce, BlockRequest, BlockResponse}; +use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; +use sp_consensus::BlockOrigin; +use sp_runtime::{ + traits::{Block as BlockT, NumberFor}, + Justifications, +}; +use std::{fmt, task::Poll}; +use warp::{EncodedProof, WarpProofRequest, WarpSyncProgress}; + +/// The sync status of a peer we are trying to sync with +#[derive(Debug)] +pub struct PeerInfo { + /// Their best block hash. + pub best_hash: Block::Hash, + /// Their best block number. + pub best_number: NumberFor, +} + +/// Reported sync state. +#[derive(Clone, Eq, PartialEq, Debug)] +pub enum SyncState { + /// Initial sync is complete, keep-up sync is active. + Idle, + /// Actively catching up with the chain. + Downloading, +} + +/// Reported state download progress. +#[derive(Clone, Eq, PartialEq, Debug)] +pub struct StateDownloadProgress { + /// Estimated download percentage. + pub percentage: u32, + /// Total state size in bytes downloaded so far. + pub size: u64, +} + +/// Syncing status and statistics. +#[derive(Clone)] +pub struct SyncStatus { + /// Current global sync state. + pub state: SyncState, + /// Target sync block number. + pub best_seen_block: Option>, + /// Number of peers participating in syncing. + pub num_peers: u32, + /// Number of blocks queued for import + pub queued_blocks: u32, + /// State sync status in progress, if any. + pub state_sync: Option, + /// Warp sync in progress, if any. + pub warp_sync: Option>, +} + +/// A peer did not behave as expected and should be reported. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BadPeer(pub PeerId, pub sc_peerset::ReputationChange); + +impl fmt::Display for BadPeer { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Bad peer {}; Reputation change: {:?}", self.0, self.1) + } +} + +impl std::error::Error for BadPeer {} + +/// Result of [`ChainSync::on_block_data`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OnBlockData { + /// The block should be imported. + Import(BlockOrigin, Vec>), + /// A new block request needs to be made to the given peer. + Request(PeerId, BlockRequest), +} + +/// Result of [`ChainSync::on_block_justification`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OnBlockJustification { + /// The justification needs no further handling. + Nothing, + /// The justification should be imported. + Import { + peer: PeerId, + hash: Block::Hash, + number: NumberFor, + justifications: Justifications, + }, +} + +/// Result of [`ChainSync::on_state_data`]. +#[derive(Debug)] +pub enum OnStateData { + /// The block and state that should be imported. + Import(BlockOrigin, IncomingBlock), + /// A new state request needs to be made to the given peer. + Continue, +} + +/// Result of [`ChainSync::poll_block_announce_validation`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PollBlockAnnounceValidation { + /// The announcement failed at validation. + /// + /// The peer reputation should be decreased. + Failure { + /// Who sent the processed block announcement? + who: PeerId, + /// Should the peer be disconnected? + disconnect: bool, + }, + /// The announcement does not require further handling. + Nothing { + /// Who sent the processed block announcement? + who: PeerId, + /// Was this their new best block? + is_best: bool, + /// The announcement. + announce: BlockAnnounce, + }, + /// The announcement header should be imported. + ImportHeader { + /// Who sent the processed block announcement? + who: PeerId, + /// Was this their new best block? + is_best: bool, + /// The announcement. + announce: BlockAnnounce, + }, + /// The block announcement should be skipped. + Skip, +} + +#[derive(Debug)] +pub struct Metrics { + pub queued_blocks: u32, + pub fork_targets: u32, + pub justifications: extra_requests::Metrics, +} + +// TODO: Make concrete types for `StateRequest` and `StateResponse` that can be converted to network +// messages from scheme later +pub trait ChainSync { + /// Returns the state of the sync of the given peer. + /// + /// Returns `None` if the peer is unknown. + fn peer_info(&self, who: &PeerId) -> Option>; + + /// Returns the current sync status. + fn status(&self) -> SyncStatus; + + /// Number of active forks requests. This includes + /// requests that are pending or could be issued right away. + fn num_sync_requests(&self) -> usize; + + /// Number of downloaded blocks. + fn num_downloaded_blocks(&self) -> usize; + + /// Returns the current number of peers stored within this state machine. + fn num_peers(&self) -> usize; + + /// Handle a new connected peer. + /// + /// Call this method whenever we connect to a new peer. + fn new_peer( + &mut self, + who: PeerId, + best_hash: Block::Hash, + best_number: NumberFor, + ) -> Result>, BadPeer>; + + /// Signal that a new best block has been imported. + /// `ChainSync` state with that information. + fn update_chain_info(&mut self, best_hash: &Block::Hash, best_number: NumberFor); + + /// Schedule a justification request for the given block. + fn request_justification(&mut self, hash: &Block::Hash, number: NumberFor); + + /// Clear all pending justification requests. + fn clear_justification_requests(&mut self); + + /// Request syncing for the given block from given set of peers. + // The implementation is similar to on_block_announce with unknown parent hash. + fn set_sync_fork_request( + &mut self, + peers: Vec, + hash: &Block::Hash, + number: NumberFor, + ); + + /// Get an iterator over all scheduled justification requests. + fn justification_requests( + &mut self, + ) -> Box)> + '_>; + + /// Get an iterator over all block requests of all peers. + fn block_requests(&mut self) -> Box)> + '_>; + + /// Get a state request, if any. + fn state_request(&mut self) -> Option<(PeerId, StateRequest)>; + + /// Get a warp sync request, if any. + fn warp_sync_request(&mut self) -> Option<(PeerId, WarpProofRequest)>; + + /// Handle a response from the remote to a block request that we made. + /// + /// `request` must be the original request that triggered `response`. + /// or `None` if data comes from the block announcement. + /// + /// If this corresponds to a valid block, this outputs the block that + /// must be imported in the import queue. + fn on_block_data( + &mut self, + who: &PeerId, + request: Option>, + response: BlockResponse, + ) -> Result, BadPeer>; + + /// Handle a response from the remote to a state request that we made. + /// + /// Returns next request if any. + fn on_state_data( + &mut self, + who: &PeerId, + response: StateResponse, + ) -> Result, BadPeer>; + + /// Handle a response from the remote to a warp proof request that we made. + /// + /// Returns next request. + fn on_warp_sync_data(&mut self, who: &PeerId, response: EncodedProof) -> Result<(), BadPeer>; + + /// Handle a response from the remote to a justification request that we made. + /// + /// `request` must be the original request that triggered `response`. + /// + /// Returns `Some` if this produces a justification that must be imported + /// into the import queue. + fn on_block_justification( + &mut self, + who: PeerId, + response: BlockResponse, + ) -> Result, BadPeer>; + + /// A batch of blocks have been processed, with or without errors. + /// + /// Call this when a batch of blocks have been processed by the import + /// queue, with or without errors. + /// + /// `peer_info` is passed in case of a restart. + fn on_blocks_processed( + &mut self, + imported: usize, + count: usize, + results: Vec<(Result>, BlockImportError>, Block::Hash)>, + ) -> Box), BadPeer>>>; + + /// Call this when a justification has been processed by the import queue, + /// with or without errors. + fn on_justification_import( + &mut self, + hash: Block::Hash, + number: NumberFor, + success: bool, + ); + + /// Notify about finalization of the given block. + fn on_block_finalized(&mut self, hash: &Block::Hash, number: NumberFor); + + /// Push a block announce validation. + /// + /// It is required that [`ChainSync::poll_block_announce_validation`] is called + /// to check for finished block announce validations. + fn push_block_announce_validation( + &mut self, + who: PeerId, + hash: Block::Hash, + announce: BlockAnnounce, + is_best: bool, + ); + + /// Poll block announce validation. + /// + /// Block announce validations can be pushed by using + /// [`ChainSync::push_block_announce_validation`]. + /// + /// This should be polled until it returns [`Poll::Pending`]. + /// + /// If [`PollBlockAnnounceValidation::ImportHeader`] is returned, then the caller MUST try to + /// import passed header (call `on_block_data`). The network request isn't sent in this case. + fn poll_block_announce_validation( + &mut self, + cx: &mut std::task::Context, + ) -> Poll>; + + /// Call when a peer has disconnected. + /// Canceled obsolete block request may result in some blocks being ready for + /// import, so this functions checks for such blocks and returns them. + fn peer_disconnected(&mut self, who: &PeerId) -> Option>; + + /// Return some key metrics. + fn metrics(&self) -> Metrics; +} diff --git a/client/network/common/src/sync/extra_requests.rs b/client/network/common/src/sync/extra_requests.rs new file mode 100644 index 0000000000000..15ff090a8ccac --- /dev/null +++ b/client/network/common/src/sync/extra_requests.rs @@ -0,0 +1,25 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#[derive(Debug)] +pub struct Metrics { + pub pending_requests: u32, + pub active_requests: u32, + pub importing_requests: u32, + pub failed_requests: u32, +} diff --git a/client/network/sync/src/message.rs b/client/network/common/src/sync/message.rs similarity index 99% rename from client/network/sync/src/message.rs rename to client/network/common/src/sync/message.rs index 996ee5231cf2e..27ab2704e6471 100644 --- a/client/network/sync/src/message.rs +++ b/client/network/common/src/sync/message.rs @@ -124,8 +124,8 @@ impl BlockAnnounce { /// Generic types. pub mod generic { use super::{BlockAttributes, BlockState, Direction}; + use crate::message::RequestId; use codec::{Decode, Encode, Input, Output}; - use sc_network_common::message::RequestId; use sp_runtime::{EncodedJustification, Justifications}; /// Block data sent in the response. diff --git a/client/network/common/src/warp_sync.rs b/client/network/common/src/sync/warp.rs similarity index 63% rename from client/network/common/src/warp_sync.rs rename to client/network/common/src/sync/warp.rs index db62283110718..335a9a978e351 100644 --- a/client/network/common/src/warp_sync.rs +++ b/client/network/common/src/sync/warp.rs @@ -14,11 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Generic data structures for handling (i.e. answering) grandpa warp sync provider. - use codec::{Decode, Encode}; pub use sp_finality_grandpa::{AuthorityList, SetId}; -use sp_runtime::traits::Block as BlockT; +use sp_runtime::traits::{Block as BlockT, NumberFor}; +use std::fmt; /// Scale-encoded warp sync proof response. pub struct EncodedProof(pub Vec); @@ -57,3 +56,39 @@ pub trait WarpSyncProvider: Send + Sync { /// sync. fn current_authorities(&self) -> AuthorityList; } + +/// Reported warp sync phase. +#[derive(Clone, Eq, PartialEq, Debug)] +pub enum WarpSyncPhase { + /// Waiting for peers to connect. + AwaitingPeers, + /// Downloading and verifying grandpa warp proofs. + DownloadingWarpProofs, + /// Downloading state data. + DownloadingState, + /// Importing state. + ImportingState, + /// Downloading block history. + DownloadingBlocks(NumberFor), +} + +impl fmt::Display for WarpSyncPhase { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::AwaitingPeers => write!(f, "Waiting for peers"), + Self::DownloadingWarpProofs => write!(f, "Downloading finality proofs"), + Self::DownloadingState => write!(f, "Downloading state"), + Self::ImportingState => write!(f, "Importing state"), + Self::DownloadingBlocks(n) => write!(f, "Downloading block history (#{})", n), + } + } +} + +/// Reported warp sync progress. +#[derive(Clone, Eq, PartialEq, Debug)] +pub struct WarpSyncProgress { + /// Estimated download percentage. + pub phase: WarpSyncPhase, + /// Total bytes downloaded so far. + pub total_bytes: u64, +} diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 77a4afaa3af5e..331f4790e9a6f 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -266,13 +266,11 @@ pub use protocol::{ event::{DhtEvent, Event, ObservedRole}, PeerInfo, }; -pub use sc_network_sync::{ - block_request_handler, - state::StateDownloadProgress, - state_request_handler, +pub use sc_network_common::sync::{ warp::{WarpSyncPhase, WarpSyncProgress}, - warp_request_handler, SyncState, + StateDownloadProgress, SyncState, }; +pub use sc_network_sync::{block_request_handler, state_request_handler, warp_request_handler}; pub use service::{ DecodingError, IfDisconnected, KademliaKey, Keypair, NetworkService, NetworkWorker, NotificationSender, NotificationSenderReady, OutboundFailure, PublicKey, RequestFailure, diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 1374c81efd748..51603a5999c2c 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -49,17 +49,17 @@ use sc_client_api::{BlockBackend, HeaderBackend, ProofProvider}; use sc_consensus::import_queue::{BlockImportError, BlockImportStatus, IncomingBlock, Origin}; use sc_network_common::{ config::ProtocolId, - warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}, -}; -use sc_network_sync::{ - message::{ - BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, BlockState, - FromBlock, + sync::{ + message::{ + BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, BlockState, + FromBlock, + }, + BadPeer, OnBlockData, OnBlockJustification, OnStateData, PollBlockAnnounceValidation, + SyncStatus, }, - schema::v1::StateResponse, - BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, - PollBlockAnnounceValidation, SyncStatus, + warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}, }; +use sc_network_sync::{schema::v1::StateResponse, ChainSync}; use sp_arithmetic::traits::SaturatedConversion; use sp_consensus::{block_validation::BlockAnnounceValidator, BlockOrigin}; use sp_runtime::{ diff --git a/client/network/src/protocol/message.rs b/client/network/src/protocol/message.rs index a57740ec2746b..c9512f82e23bb 100644 --- a/client/network/src/protocol/message.rs +++ b/client/network/src/protocol/message.rs @@ -63,10 +63,12 @@ pub mod generic { use bitflags::bitflags; use codec::{Decode, Encode, Input, Output}; use sc_client_api::StorageProof; - use sc_network_common::message::RequestId; - use sc_network_sync::message::{ - generic::{BlockRequest, BlockResponse}, - BlockAnnounce, + use sc_network_common::{ + message::RequestId, + sync::message::{ + generic::{BlockRequest, BlockResponse}, + BlockAnnounce, + }, }; use sp_runtime::ConsensusEngineId; diff --git a/client/network/src/service.rs b/client/network/src/service.rs index edd30e9c9dee4..c5249c9177ce7 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -60,7 +60,7 @@ use metrics::{Histogram, HistogramVec, MetricSources, Metrics}; use parking_lot::Mutex; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; -use sc_network_sync::{Status as SyncStatus, SyncState}; +use sc_network_common::sync::{SyncState, SyncStatus}; use sc_peerset::PeersetHandle; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use sp_blockchain::{HeaderBackend, HeaderMetadata}; diff --git a/client/network/sync/Cargo.toml b/client/network/sync/Cargo.toml index 9cf8afa58649c..c82895812a18c 100644 --- a/client/network/sync/Cargo.toml +++ b/client/network/sync/Cargo.toml @@ -17,7 +17,6 @@ targets = ["x86_64-unknown-linux-gnu"] prost-build = "0.9" [dependencies] -bitflags = "1.3.2" codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive", ] } diff --git a/client/network/sync/src/block_request_handler.rs b/client/network/sync/src/block_request_handler.rs index 78d6a1a7d1680..73ec5c2201d00 100644 --- a/client/network/sync/src/block_request_handler.rs +++ b/client/network/sync/src/block_request_handler.rs @@ -17,10 +17,7 @@ //! Helper for handling (i.e. answering) block requests from a remote peer via the //! `crate::request_responses::RequestResponsesBehaviour`. -use crate::{ - message::BlockAttributes, - schema::v1::{block_request::FromBlock, BlockResponse, Direction}, -}; +use crate::schema::v1::{block_request::FromBlock, BlockResponse, Direction}; use codec::{Decode, Encode}; use futures::{ channel::{mpsc, oneshot}, @@ -34,6 +31,7 @@ use sc_client_api::BlockBackend; use sc_network_common::{ config::ProtocolId, request_responses::{IncomingRequest, OutgoingResponse, ProtocolConfig}, + sync::message::BlockAttributes, }; use sp_blockchain::HeaderBackend; use sp_runtime::{ diff --git a/client/network/sync/src/blocks.rs b/client/network/sync/src/blocks.rs index b897184e7a44c..d1d086409c7fd 100644 --- a/client/network/sync/src/blocks.rs +++ b/client/network/sync/src/blocks.rs @@ -16,9 +16,9 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::message; use libp2p::PeerId; use log::trace; +use sc_network_common::sync::message; use sp_runtime::traits::{Block as BlockT, NumberFor, One}; use std::{ cmp, @@ -217,8 +217,8 @@ impl BlockCollection { #[cfg(test)] mod test { use super::{BlockCollection, BlockData, BlockRangeState}; - use crate::message; use libp2p::PeerId; + use sc_network_common::sync::message; use sp_core::H256; use sp_runtime::testing::{Block as RawBlock, ExtrinsicWrapper}; diff --git a/client/network/sync/src/extra_requests.rs b/client/network/sync/src/extra_requests.rs index c684d8e72783e..230e6fb067ff5 100644 --- a/client/network/sync/src/extra_requests.rs +++ b/client/network/sync/src/extra_requests.rs @@ -20,6 +20,7 @@ use crate::{PeerSync, PeerSyncState}; use fork_tree::ForkTree; use libp2p::PeerId; use log::{debug, trace, warn}; +use sc_network_common::sync::extra_requests::Metrics; use sp_blockchain::Error as ClientError; use sp_runtime::traits::{Block as BlockT, NumberFor, Zero}; use std::{ @@ -56,15 +57,6 @@ pub(crate) struct ExtraRequests { request_type_name: &'static str, } -#[derive(Debug)] -pub struct Metrics { - pub pending_requests: u32, - pub active_requests: u32, - pub importing_requests: u32, - pub failed_requests: u32, - _priv: (), -} - impl ExtraRequests { pub(crate) fn new(request_type_name: &'static str) -> Self { Self { @@ -258,7 +250,6 @@ impl ExtraRequests { active_requests: self.active_requests.len().try_into().unwrap_or(std::u32::MAX), failed_requests: self.failed_requests.len().try_into().unwrap_or(std::u32::MAX), importing_requests: self.importing_requests.len().try_into().unwrap_or(std::u32::MAX), - _priv: (), } } } diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 1c3cf56f7c8b8..44a102f4fcb03 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -30,7 +30,6 @@ pub mod block_request_handler; pub mod blocks; -pub mod message; pub mod schema; pub mod state; pub mod state_request_handler; @@ -39,10 +38,9 @@ pub mod warp_request_handler; use crate::{ blocks::BlockCollection, - message::{BlockAnnounce, BlockAttributes, BlockRequest, BlockResponse}, schema::v1::{StateRequest, StateResponse}, - state::{StateDownloadProgress, StateSync}, - warp::{WarpProofImportResult, WarpSync, WarpSyncPhase, WarpSyncProgress}, + state::StateSync, + warp::{WarpProofImportResult, WarpSync}, }; use codec::Encode; use either::Either; @@ -52,7 +50,15 @@ use libp2p::PeerId; use log::{debug, error, info, trace, warn}; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; -use sc_network_common::warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}; +use sc_network_common::sync::{ + message::{ + BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, Direction, + FromBlock, + }, + warp::{EncodedProof, WarpProofRequest, WarpSyncPhase, WarpSyncProgress, WarpSyncProvider}, + BadPeer, Metrics, OnBlockData, OnBlockJustification, OnStateData, PeerInfo, + PollBlockAnnounceValidation, SyncState, SyncStatus, +}; use sp_arithmetic::traits::Saturating; use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata}; use sp_consensus::{ @@ -69,7 +75,6 @@ use sp_runtime::{ }; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, - fmt, ops::Range, pin::Pin, sync::Arc, @@ -281,15 +286,6 @@ impl PeerSync { } } -/// The sync status of a peer we are trying to sync with -#[derive(Debug)] -pub struct PeerInfo { - /// Their best block hash. - pub best_hash: B::Hash, - /// Their best block number. - pub best_number: NumberFor, -} - struct ForkTarget { number: NumberFor, parent_hash: Option, @@ -328,108 +324,6 @@ impl PeerSyncState { } } -/// Reported sync state. -#[derive(Clone, Eq, PartialEq, Debug)] -pub enum SyncState { - /// Initial sync is complete, keep-up sync is active. - Idle, - /// Actively catching up with the chain. - Downloading, -} - -/// Syncing status and statistics. -#[derive(Clone)] -pub struct Status { - /// Current global sync state. - pub state: SyncState, - /// Target sync block number. - pub best_seen_block: Option>, - /// Number of peers participating in syncing. - pub num_peers: u32, - /// Number of blocks queued for import - pub queued_blocks: u32, - /// State sync status in progress, if any. - pub state_sync: Option, - /// Warp sync in progress, if any. - pub warp_sync: Option>, -} - -/// A peer did not behave as expected and should be reported. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct BadPeer(pub PeerId, pub sc_peerset::ReputationChange); - -impl fmt::Display for BadPeer { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Bad peer {}; Reputation change: {:?}", self.0, self.1) - } -} - -impl std::error::Error for BadPeer {} - -/// Result of [`ChainSync::on_block_data`]. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum OnBlockData { - /// The block should be imported. - Import(BlockOrigin, Vec>), - /// A new block request needs to be made to the given peer. - Request(PeerId, BlockRequest), -} - -impl OnBlockData { - /// Returns `self` as request. - #[cfg(test)] - fn into_request(self) -> Option<(PeerId, BlockRequest)> { - if let Self::Request(peer, req) = self { - Some((peer, req)) - } else { - None - } - } -} - -/// Result of [`ChainSync::on_state_data`]. -#[derive(Debug)] -pub enum OnStateData { - /// The block and state that should be imported. - Import(BlockOrigin, IncomingBlock), - /// A new state request needs to be made to the given peer. - Continue, -} - -/// Result of [`ChainSync::poll_block_announce_validation`]. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum PollBlockAnnounceValidation { - /// The announcement failed at validation. - /// - /// The peer reputation should be decreased. - Failure { - /// Who sent the processed block announcement? - who: PeerId, - /// Should the peer be disconnected? - disconnect: bool, - }, - /// The announcement does not require further handling. - Nothing { - /// Who sent the processed block announcement? - who: PeerId, - /// Was this their new best block? - is_best: bool, - /// The announcement. - announce: BlockAnnounce, - }, - /// The announcement header should be imported. - ImportHeader { - /// Who sent the processed block announcement? - who: PeerId, - /// Was this their new best block? - is_best: bool, - /// The announcement. - announce: BlockAnnounce, - }, - /// The block announcement should be skipped. - Skip, -} - /// Result of [`ChainSync::block_announce_validation`]. #[derive(Debug, Clone, PartialEq, Eq)] enum PreValidateBlockAnnounce { @@ -465,15 +359,6 @@ enum PreValidateBlockAnnounce { Skip, } -/// Result of [`ChainSync::on_block_justification`]. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum OnBlockJustification { - /// The justification needs no further handling. - Nothing, - /// The justification should be imported. - Import { peer: PeerId, hash: B::Hash, number: NumberFor, justifications: Justifications }, -} - /// Operation mode. #[derive(Debug, PartialEq, Eq)] pub enum SyncMode { @@ -575,7 +460,7 @@ where } /// Returns the current sync status. - pub fn status(&self) -> Status { + pub fn status(&self) -> SyncStatus { let best_seen = self.peers.values().map(|p| p.best_number).max(); let sync_state = if let Some(n) = best_seen { // A chain is classified as downloading if the provided best block is @@ -601,7 +486,7 @@ where _ => None, }; - Status { + SyncStatus { state: sync_state, best_seen_block: best_seen, num_peers: self.peers.len() as u32, @@ -843,12 +728,12 @@ where "`Matcher::next` guarantees the `PeerId` comes from the given peers; qed", ) .state = PeerSyncState::DownloadingJustification(request.0); - let req = message::generic::BlockRequest { + let req = BlockRequest:: { id: 0, fields: BlockAttributes::JUSTIFICATION, - from: message::FromBlock::Hash(request.0), + from: FromBlock::Hash(request.0), to: None, - direction: message::Direction::Ascending, + direction: Direction::Ascending, max: Some(1), }; Some((peer, req)) @@ -1064,10 +949,7 @@ where let mut gap = false; let new_blocks: Vec> = if let Some(peer) = self.peers.get_mut(who) { let mut blocks = response.blocks; - if request - .as_ref() - .map_or(false, |r| r.direction == message::Direction::Descending) - { + if request.as_ref().map_or(false, |r| r.direction == Direction::Descending) { trace!(target: "sync", "Reversing incoming block list"); blocks.reverse() } @@ -2171,7 +2053,6 @@ where queued_blocks: self.queue_blocks.len().try_into().unwrap_or(std::u32::MAX), fork_targets: self.fork_targets.len().try_into().unwrap_or(std::u32::MAX), justifications: self.extra_justifications.metrics(), - _priv: (), } } @@ -2212,23 +2093,15 @@ fn legacy_justification_mapping( justification.map(|just| (*b"FRNK", just).into()) } -#[derive(Debug)] -pub struct Metrics { - pub queued_blocks: u32, - pub fork_targets: u32, - pub justifications: extra_requests::Metrics, - _priv: (), -} - /// Request the ancestry for a block. Sends a request for header and justification for the given /// block number. Used during ancestry search. fn ancestry_request(block: NumberFor) -> BlockRequest { - message::generic::BlockRequest { + BlockRequest:: { id: 0, fields: BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION, - from: message::FromBlock::Number(block), + from: FromBlock::Number(block), to: None, - direction: message::Direction::Ascending, + direction: Direction::Ascending, max: Some(1), } } @@ -2306,7 +2179,7 @@ fn peer_block_request( id: &PeerId, peer: &PeerSync, blocks: &mut BlockCollection, - attrs: message::BlockAttributes, + attrs: BlockAttributes, max_parallel_downloads: u32, finalized: NumberFor, best_num: NumberFor, @@ -2334,17 +2207,17 @@ fn peer_block_request( let last = range.end.saturating_sub(One::one()); let from = if peer.best_number == last { - message::FromBlock::Hash(peer.best_hash) + FromBlock::Hash(peer.best_hash) } else { - message::FromBlock::Number(last) + FromBlock::Number(last) }; - let request = message::generic::BlockRequest { + let request = BlockRequest:: { id: 0, fields: attrs, from, to: None, - direction: message::Direction::Descending, + direction: Direction::Descending, max: Some((range.end - range.start).saturated_into::()), }; @@ -2356,7 +2229,7 @@ fn peer_gap_block_request( id: &PeerId, peer: &PeerSync, blocks: &mut BlockCollection, - attrs: message::BlockAttributes, + attrs: BlockAttributes, target: NumberFor, common_number: NumberFor, ) -> Option<(Range>, BlockRequest)> { @@ -2371,14 +2244,14 @@ fn peer_gap_block_request( // The end is not part of the range. let last = range.end.saturating_sub(One::one()); - let from = message::FromBlock::Number(last); + let from = FromBlock::Number(last); - let request = message::generic::BlockRequest { + let request = BlockRequest:: { id: 0, fields: attrs, from, to: None, - direction: message::Direction::Descending, + direction: Direction::Descending, max: Some((range.end - range.start).saturated_into::()), }; Some((range, request)) @@ -2390,7 +2263,7 @@ fn fork_sync_request( targets: &mut HashMap>, best_num: NumberFor, finalized: NumberFor, - attributes: message::BlockAttributes, + attributes: BlockAttributes, check_block: impl Fn(&B::Hash) -> BlockStatus, ) -> Option<(B::Hash, BlockRequest)> { targets.retain(|hash, r| { @@ -2423,12 +2296,12 @@ fn fork_sync_request( trace!(target: "sync", "Downloading requested fork {:?} from {}, {} blocks", hash, id, count); return Some(( *hash, - message::generic::BlockRequest { + BlockRequest:: { id: 0, fields: attributes, - from: message::FromBlock::Hash(*hash), + from: FromBlock::Hash(*hash), to: None, - direction: message::Direction::Descending, + direction: Direction::Descending, max: Some(count), }, )) @@ -2463,7 +2336,7 @@ where /// /// It is expected that `blocks` are in ascending order. fn validate_blocks( - blocks: &Vec>, + blocks: &Vec>, who: &PeerId, request: Option>, ) -> Result>, BadPeer> { @@ -2480,16 +2353,13 @@ fn validate_blocks( return Err(BadPeer(*who, rep::NOT_REQUESTED)) } - let block_header = if request.direction == message::Direction::Descending { - blocks.last() - } else { - blocks.first() - } - .and_then(|b| b.header.as_ref()); + let block_header = + if request.direction == Direction::Descending { blocks.last() } else { blocks.first() } + .and_then(|b| b.header.as_ref()); let expected_block = block_header.as_ref().map_or(false, |h| match request.from { - message::FromBlock::Hash(hash) => h.hash() == hash, - message::FromBlock::Number(n) => h.number() == &n, + FromBlock::Hash(hash) => h.hash() == hash, + FromBlock::Number(n) => h.number() == &n, }); if !expected_block { @@ -2503,7 +2373,7 @@ fn validate_blocks( return Err(BadPeer(*who, rep::NOT_REQUESTED)) } - if request.fields.contains(message::BlockAttributes::HEADER) && + if request.fields.contains(BlockAttributes::HEADER) && blocks.iter().any(|b| b.header.is_none()) { trace!( @@ -2515,8 +2385,7 @@ fn validate_blocks( return Err(BadPeer(*who, rep::BAD_RESPONSE)) } - if request.fields.contains(message::BlockAttributes::BODY) && - blocks.iter().any(|b| b.body.is_none()) + if request.fields.contains(BlockAttributes::BODY) && blocks.iter().any(|b| b.body.is_none()) { trace!( target: "sync", @@ -2567,13 +2436,10 @@ fn validate_blocks( #[cfg(test)] mod test { - use super::{ - message::{BlockState, FromBlock}, - *, - }; - use crate::message::BlockData; + use super::*; use futures::{executor::block_on, future::poll_fn}; use sc_block_builder::BlockBuilderProvider; + use sc_network_common::sync::message::{BlockData, BlockState, FromBlock}; use sp_blockchain::HeaderBackend; use sp_consensus::block_validation::DefaultBlockAnnounceValidator; use substrate_test_runtime_client::{ @@ -2684,7 +2550,7 @@ mod test { assert!(sync.justification_requests().any(|(p, r)| { p == peer_id3 && r.fields == BlockAttributes::JUSTIFICATION && - r.from == message::FromBlock::Hash(b1_hash) && + r.from == FromBlock::Hash(b1_hash) && r.to == None })); @@ -3081,10 +2947,11 @@ mod test { let response = create_block_response(vec![block.clone()]); let on_block_data = sync.on_block_data(&peer_id1, Some(request), response).unwrap(); - request = match on_block_data.into_request() { - Some(req) => req.1, + request = if let OnBlockData::Request(_peer, request) = on_block_data { + request + } else { // We found the ancenstor - None => break, + break }; log::trace!(target: "sync", "Request: {:?}", request); diff --git a/client/network/sync/src/state.rs b/client/network/sync/src/state.rs index 4041c28af0eba..e70d3b6b33a28 100644 --- a/client/network/sync/src/state.rs +++ b/client/network/sync/src/state.rs @@ -23,6 +23,7 @@ use codec::{Decode, Encode}; use log::debug; use sc_client_api::{CompactProof, ProofProvider}; use sc_consensus::ImportedState; +use sc_network_common::sync::StateDownloadProgress; use smallvec::SmallVec; use sp_core::storage::well_known_keys; use sp_runtime::traits::{Block as BlockT, Header, NumberFor}; @@ -42,15 +43,6 @@ pub struct StateSync { skip_proof: bool, } -/// Reported state download progress. -#[derive(Clone, Eq, PartialEq, Debug)] -pub struct StateDownloadProgress { - /// Estimated download percentage. - pub percentage: u32, - /// Total state size in bytes downloaded so far. - pub size: u64, -} - /// Import state chunk result. pub enum ImportResult { /// State is complete and ready for import. diff --git a/client/network/sync/src/warp.rs b/client/network/sync/src/warp.rs index d09f0f089ee59..dc650eb422cb2 100644 --- a/client/network/sync/src/warp.rs +++ b/client/network/sync/src/warp.rs @@ -23,55 +23,22 @@ use crate::{ state::{ImportResult, StateSync}, }; use sc_client_api::ProofProvider; -use sc_network_common::warp_sync::{ - EncodedProof, VerificationResult, WarpProofRequest, WarpSyncProvider, +use sc_network_common::{ + sync::warp::WarpSyncPhase, + warp_sync::{ + EncodedProof, VerificationResult, WarpProofRequest, WarpSyncProgress, WarpSyncProvider, + }, }; use sp_blockchain::HeaderBackend; use sp_finality_grandpa::{AuthorityList, SetId}; use sp_runtime::traits::{Block as BlockT, NumberFor, Zero}; -use std::{fmt, sync::Arc}; +use std::sync::Arc; enum Phase { WarpProof { set_id: SetId, authorities: AuthorityList, last_hash: B::Hash }, State(StateSync), } -/// Reported warp sync phase. -#[derive(Clone, Eq, PartialEq, Debug)] -pub enum WarpSyncPhase { - /// Waiting for peers to connect. - AwaitingPeers, - /// Downloading and verifying grandpa warp proofs. - DownloadingWarpProofs, - /// Downloading state data. - DownloadingState, - /// Importing state. - ImportingState, - /// Downloading block history. - DownloadingBlocks(NumberFor), -} - -impl fmt::Display for WarpSyncPhase { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Self::AwaitingPeers => write!(f, "Waiting for peers"), - Self::DownloadingWarpProofs => write!(f, "Downloading finality proofs"), - Self::DownloadingState => write!(f, "Downloading state"), - Self::ImportingState => write!(f, "Importing state"), - Self::DownloadingBlocks(n) => write!(f, "Downloading block history (#{})", n), - } - } -} - -/// Reported warp sync progress. -#[derive(Clone, Eq, PartialEq, Debug)] -pub struct WarpSyncProgress { - /// Estimated download percentage. - pub phase: WarpSyncPhase, - /// Total bytes downloaded so far. - pub total_bytes: u64, -} - /// Import warp proof result. pub enum WarpProofImportResult { /// Import was successful. From 922f1ae800a2ecd385b998fc48ee84c9e63d5865 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 12 May 2022 03:01:10 +0300 Subject: [PATCH 05/15] Switch from concrete implementation to `ChainSync` trait from `sc-network-common` --- Cargo.lock | 1 - client/beefy/src/tests.rs | 14 +- client/consensus/aura/src/lib.rs | 14 +- client/consensus/babe/src/tests.rs | 18 +- client/finality-grandpa/src/tests.rs | 20 +- client/network/common/src/sync.rs | 15 +- client/network/src/config.rs | 5 - client/network/src/protocol.rs | 153 +++----- client/network/src/service.rs | 31 +- client/network/sync/Cargo.toml | 1 - client/network/sync/src/lib.rs | 559 ++++++++++++--------------- client/network/test/src/lib.rs | 57 +-- 12 files changed, 366 insertions(+), 522 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d7383fa6bce0f..cc5122e139513 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8594,7 +8594,6 @@ dependencies = [ name = "sc-network-sync" version = "0.10.0-dev" dependencies = [ - "either", "fork-tree", "futures", "libp2p", diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 1d035a6a447c2..ebde62b9197f8 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -28,7 +28,6 @@ use sc_chain_spec::{ChainSpec, GenericChainSpec}; use sc_client_api::HeaderBackend; use sc_consensus::BoxJustificationImport; use sc_keystore::LocalKeystore; -use sc_network::config::ProtocolConfig; use sc_network_test::{ Block, BlockImportAdapter, FullPeerConfig, PassThroughVerifier, Peer, PeersClient, TestNetFactory, @@ -111,6 +110,7 @@ pub(crate) struct PeerData { pub(crate) beefy_link_half: Mutex>, } +#[derive(Default)] pub(crate) struct BeefyTestNet { peers: Vec, } @@ -166,17 +166,7 @@ impl TestNetFactory for BeefyTestNet { type BlockImport = PeersClient; type PeerData = PeerData; - /// Create new test network with peers and given config. - fn from_config(_config: &ProtocolConfig) -> Self { - BeefyTestNet { peers: Vec::new() } - } - - fn make_verifier( - &self, - _client: PeersClient, - _cfg: &ProtocolConfig, - _: &PeerData, - ) -> Self::Verifier { + fn make_verifier(&self, _client: PeersClient, _: &PeerData) -> Self::Verifier { PassThroughVerifier::new(false) // use non-instant finality. } diff --git a/client/consensus/aura/src/lib.rs b/client/consensus/aura/src/lib.rs index ac3b89f2ff9a2..ee8be727dcdac 100644 --- a/client/consensus/aura/src/lib.rs +++ b/client/consensus/aura/src/lib.rs @@ -566,7 +566,6 @@ mod tests { use sc_consensus::BoxJustificationImport; use sc_consensus_slots::{BackoffAuthoringOnFinalizedHeadLagging, SimpleSlotWorker}; use sc_keystore::LocalKeystore; - use sc_network::config::ProtocolConfig; use sc_network_test::{Block as TestBlock, *}; use sp_application_crypto::key_types::AURA; use sp_consensus::{ @@ -645,6 +644,7 @@ mod tests { >; type AuraPeer = Peer<(), PeersClient>; + #[derive(Default)] pub struct AuraTestNet { peers: Vec, } @@ -654,17 +654,7 @@ mod tests { type PeerData = (); type BlockImport = PeersClient; - /// Create new test network with peers and given config. - fn from_config(_config: &ProtocolConfig) -> Self { - AuraTestNet { peers: Vec::new() } - } - - fn make_verifier( - &self, - client: PeersClient, - _cfg: &ProtocolConfig, - _peer_data: &(), - ) -> Self::Verifier { + fn make_verifier(&self, client: PeersClient, _peer_data: &()) -> Self::Verifier { let client = client.as_client(); let slot_duration = slot_duration(&*client).expect("slot duration available"); diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index db19deda06fd8..9d6c522c6f886 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -18,9 +18,6 @@ //! BABE testsuite -// FIXME #2532: need to allow deprecated until refactor is done -// https://github.com/paritytech/substrate/issues/2532 -#![allow(deprecated)] use super::*; use authorship::claim_slot; use futures::executor::block_on; @@ -32,7 +29,6 @@ use sc_client_api::{backend::TransactionFor, BlockchainEvents, Finalizer}; use sc_consensus::{BoxBlockImport, BoxJustificationImport}; use sc_consensus_slots::BackoffAuthoringOnFinalizedHeadLagging; use sc_keystore::LocalKeystore; -use sc_network::config::ProtocolConfig; use sc_network_test::{Block as TestBlock, *}; use sp_application_crypto::key_types::BABE; use sp_consensus::{AlwaysCanAuthor, DisableProofRecording, NoNetwork as DummyOracle, Proposal}; @@ -223,6 +219,7 @@ where type BabePeer = Peer, BabeBlockImport>; +#[derive(Default)] pub struct BabeTestNet { peers: Vec, } @@ -281,12 +278,6 @@ impl TestNetFactory for BabeTestNet { type PeerData = Option; type BlockImport = BabeBlockImport; - /// Create new test network with peers and given config. - fn from_config(_config: &ProtocolConfig) -> Self { - debug!(target: "babe", "Creating test network from config"); - BabeTestNet { peers: Vec::new() } - } - fn make_block_import( &self, client: PeersClient, @@ -312,12 +303,7 @@ impl TestNetFactory for BabeTestNet { ) } - fn make_verifier( - &self, - client: PeersClient, - _cfg: &ProtocolConfig, - maybe_link: &Option, - ) -> Self::Verifier { + fn make_verifier(&self, client: PeersClient, maybe_link: &Option) -> Self::Verifier { use substrate_test_runtime_client::DefaultTestClientBuilderExt; let client = client.as_client(); diff --git a/client/finality-grandpa/src/tests.rs b/client/finality-grandpa/src/tests.rs index 5083cbfc21d1d..c6fe991964976 100644 --- a/client/finality-grandpa/src/tests.rs +++ b/client/finality-grandpa/src/tests.rs @@ -28,7 +28,7 @@ use sc_consensus::{ BlockImport, BlockImportParams, BoxJustificationImport, ForkChoiceStrategy, ImportResult, ImportedAux, }; -use sc_network::config::{ProtocolConfig, Role}; +use sc_network::config::Role; use sc_network_test::{ Block, BlockImportAdapter, FullPeerConfig, Hash, PassThroughVerifier, Peer, PeersClient, PeersFullClient, TestClient, TestNetFactory, @@ -73,6 +73,7 @@ type GrandpaBlockImport = crate::GrandpaBlockImport< LongestChain, >; +#[derive(Default)] struct GrandpaTestNet { peers: Vec, test_config: TestApi, @@ -110,16 +111,6 @@ impl TestNetFactory for GrandpaTestNet { type PeerData = PeerData; type BlockImport = GrandpaBlockImport; - /// Create new test network with peers and given config. - fn from_config(_config: &ProtocolConfig) -> Self { - GrandpaTestNet { peers: Vec::new(), test_config: Default::default() } - } - - fn default_config() -> ProtocolConfig { - // This is unused. - ProtocolConfig::default() - } - fn add_full_peer(&mut self) { self.add_full_peer_with_config(FullPeerConfig { notifications_protocols: vec![grandpa_protocol_name::NAME.into()], @@ -128,12 +119,7 @@ impl TestNetFactory for GrandpaTestNet { }) } - fn make_verifier( - &self, - _client: PeersClient, - _cfg: &ProtocolConfig, - _: &PeerData, - ) -> Self::Verifier { + fn make_verifier(&self, _client: PeersClient, _: &PeerData) -> Self::Verifier { PassThroughVerifier::new(false) // use non-instant finality. } diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index a8044d2fe9973..5c0432bf0b5cb 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -155,6 +155,19 @@ pub enum PollBlockAnnounceValidation { Skip, } +/// Operation mode. +#[derive(Debug, PartialEq, Eq)] +pub enum SyncMode { + // Sync headers only + Light, + // Sync headers and block bodies + Full, + // Sync headers and the last finalied state + LightState { storage_chain_mode: bool, skip_proofs: bool }, + // Warp sync mode. + Warp, +} + #[derive(Debug)] pub struct Metrics { pub queued_blocks: u32, @@ -164,7 +177,7 @@ pub struct Metrics { // TODO: Make concrete types for `StateRequest` and `StateResponse` that can be converted to network // messages from scheme later -pub trait ChainSync { +pub trait ChainSync: Send { /// Returns the state of the sync of the given peer. /// /// Returns `None` if the peer is unknown. diff --git a/client/network/src/config.rs b/client/network/src/config.rs index cb42550683699..b933313f8a78b 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -31,11 +31,6 @@ pub use sc_network_common::{ pub use libp2p::{build_multiaddr, core::PublicKey, identity}; -// Note: this re-export shouldn't be part of the public API of the crate and will be removed in -// the future. -#[doc(hidden)] -pub use crate::protocol::ProtocolConfig; - use crate::ExHashT; use core::{fmt, iter}; diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 51603a5999c2c..b50ea366fcde7 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -54,14 +54,13 @@ use sc_network_common::{ BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, BlockState, FromBlock, }, - BadPeer, OnBlockData, OnBlockJustification, OnStateData, PollBlockAnnounceValidation, - SyncStatus, + BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, + PollBlockAnnounceValidation, SyncStatus, }, - warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}, + warp_sync::{EncodedProof, WarpProofRequest}, }; -use sc_network_sync::{schema::v1::StateResponse, ChainSync}; use sp_arithmetic::traits::SaturatedConversion; -use sp_consensus::{block_validation::BlockAnnounceValidator, BlockOrigin}; +use sp_consensus::BlockOrigin; use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, CheckedSub, Header as HeaderT, NumberFor, Zero}, @@ -172,11 +171,18 @@ pub struct Protocol { tick_timeout: Pin + Send>>, /// Pending list of messages to return from `poll` as a priority. pending_messages: VecDeque>, - config: ProtocolConfig, + /// Assigned roles. + roles: Roles, genesis_hash: B::Hash, /// State machine that handles the list of in-progress requests. Only full node peers are /// registered. - sync: ChainSync, + chain_sync: Box< + dyn ChainSync< + B, + sc_network_sync::schema::v1::StateRequest, + sc_network_sync::schema::v1::StateResponse, + >, + >, // All connected peers. Contains both full and light node peers. peers: HashMap>, chain: Arc, @@ -236,38 +242,6 @@ pub struct PeerInfo { pub best_number: ::Number, } -/// Configuration for the Substrate-specific part of the networking layer. -#[derive(Clone)] -pub struct ProtocolConfig { - /// Assigned roles. - pub roles: Roles, - /// Maximum number of peers to ask the same blocks in parallel. - pub max_parallel_downloads: u32, - /// Enable state sync. - pub sync_mode: config::SyncMode, -} - -impl ProtocolConfig { - fn sync_mode(&self) -> sc_network_sync::SyncMode { - if self.roles.is_light() { - sc_network_sync::SyncMode::Light - } else { - match self.sync_mode { - config::SyncMode::Full => sc_network_sync::SyncMode::Full, - config::SyncMode::Fast { skip_proofs, storage_chain_mode } => - sc_network_sync::SyncMode::LightState { skip_proofs, storage_chain_mode }, - config::SyncMode::Warp => sc_network_sync::SyncMode::Warp, - } - } - } -} - -impl Default for ProtocolConfig { - fn default() -> ProtocolConfig { - Self { roles: Roles::FULL, max_parallel_downloads: 5, sync_mode: config::SyncMode::Full } - } -} - /// Handshake sent when we open a block announces substream. #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] struct BlockAnnouncesHandshake { @@ -283,12 +257,12 @@ struct BlockAnnouncesHandshake { impl BlockAnnouncesHandshake { fn build( - protocol_config: &ProtocolConfig, + roles: Roles, best_number: NumberFor, best_hash: B::Hash, genesis_hash: B::Hash, ) -> Self { - Self { genesis_hash, roles: protocol_config.roles, best_number, best_hash } + Self { genesis_hash, roles, best_number, best_hash } } } @@ -305,24 +279,21 @@ where { /// Create a new instance. pub fn new( - config: ProtocolConfig, + roles: Roles, chain: Arc, protocol_id: ProtocolId, network_config: &config::NetworkConfiguration, notifications_protocols_handshakes: Vec>, - block_announce_validator: Box + Send>, metrics_registry: Option<&Registry>, - warp_sync_provider: Option>>, + chain_sync: Box< + dyn ChainSync< + B, + sc_network_sync::schema::v1::StateRequest, + sc_network_sync::schema::v1::StateResponse, + >, + >, ) -> error::Result<(Protocol, sc_peerset::PeersetHandle, Vec<(PeerId, Multiaddr)>)> { let info = chain.info(); - let sync = ChainSync::new( - config.sync_mode(), - chain.clone(), - block_announce_validator, - config.max_parallel_downloads, - warp_sync_provider, - ) - .map_err(Box::new)?; let boot_node_ids = { let mut list = HashSet::new(); @@ -410,7 +381,7 @@ where let genesis_hash = info.genesis_hash; let block_announces_handshake = - BlockAnnouncesHandshake::::build(&config, best_number, best_hash, genesis_hash) + BlockAnnouncesHandshake::::build(roles, best_number, best_hash, genesis_hash) .encode(); let sync_protocol_config = notifications::ProtocolConfig { @@ -443,11 +414,11 @@ where let protocol = Self { tick_timeout: Box::pin(interval(TICK_TIMEOUT)), pending_messages: VecDeque::new(), - config, + roles, peers: HashMap::new(), chain, genesis_hash: info.genesis_hash, - sync, + chain_sync, important_peers, default_peers_set_num_full: network_config.default_peers_set_num_full as usize, default_peers_set_num_light: { @@ -515,49 +486,49 @@ where /// Current global sync state. pub fn sync_state(&self) -> SyncStatus { - self.sync.status() + self.chain_sync.status() } /// Target sync block number. pub fn best_seen_block(&self) -> Option> { - self.sync.status().best_seen_block + self.chain_sync.status().best_seen_block } /// Number of peers participating in syncing. pub fn num_sync_peers(&self) -> u32 { - self.sync.status().num_peers + self.chain_sync.status().num_peers } /// Number of blocks in the import queue. pub fn num_queued_blocks(&self) -> u32 { - self.sync.status().queued_blocks + self.chain_sync.status().queued_blocks } /// Number of downloaded blocks. pub fn num_downloaded_blocks(&self) -> usize { - self.sync.num_downloaded_blocks() + self.chain_sync.num_downloaded_blocks() } /// Number of active sync requests. pub fn num_sync_requests(&self) -> usize { - self.sync.num_sync_requests() + self.chain_sync.num_sync_requests() } /// Inform sync about new best imported block. pub fn new_best_block_imported(&mut self, hash: B::Hash, number: NumberFor) { debug!(target: "sync", "New best block imported {:?}/#{}", hash, number); - self.sync.update_chain_info(&hash, number); + self.chain_sync.update_chain_info(&hash, number); self.behaviour.set_notif_protocol_handshake( HARDCODED_PEERSETS_SYNC, - BlockAnnouncesHandshake::::build(&self.config, number, hash, self.genesis_hash) + BlockAnnouncesHandshake::::build(self.roles, number, hash, self.genesis_hash) .encode(), ); } fn update_peer_info(&mut self, who: &PeerId) { - if let Some(info) = self.sync.peer_info(who) { + if let Some(info) = self.chain_sync.peer_info(who) { if let Some(ref mut peer) = self.peers.get_mut(who) { peer.info.best_hash = info.best_hash; peer.info.best_number = info.best_number; @@ -589,7 +560,9 @@ where } if let Some(_peer_data) = self.peers.remove(&peer) { - if let Some(OnBlockData::Import(origin, blocks)) = self.sync.peer_disconnected(&peer) { + if let Some(OnBlockData::Import(origin, blocks)) = + self.chain_sync.peer_disconnected(&peer) + { self.pending_messages .push_back(CustomMessageOutcome::BlockImport(origin, blocks)); } @@ -695,7 +668,7 @@ where ); if request.fields == BlockAttributes::JUSTIFICATION { - match self.sync.on_block_justification(peer_id, block_response) { + match self.chain_sync.on_block_justification(peer_id, block_response) { Ok(OnBlockJustification::Nothing) => CustomMessageOutcome::None, Ok(OnBlockJustification::Import { peer, hash, number, justifications }) => CustomMessageOutcome::JustificationImport(peer, hash, number, justifications), @@ -706,7 +679,7 @@ where }, } } else { - match self.sync.on_block_data(&peer_id, Some(request), block_response) { + match self.chain_sync.on_block_data(&peer_id, Some(request), block_response) { Ok(OnBlockData::Import(origin, blocks)) => CustomMessageOutcome::BlockImport(origin, blocks), Ok(OnBlockData::Request(peer, req)) => self.prepare_block_request(peer, req), @@ -724,9 +697,9 @@ where pub fn on_state_response( &mut self, peer_id: PeerId, - response: StateResponse, + response: sc_network_sync::schema::v1::StateResponse, ) -> CustomMessageOutcome { - match self.sync.on_state_data(&peer_id, response) { + match self.chain_sync.on_state_data(&peer_id, response) { Ok(OnStateData::Import(origin, block)) => CustomMessageOutcome::BlockImport(origin, vec![block]), Ok(OnStateData::Continue) => CustomMessageOutcome::None, @@ -745,7 +718,7 @@ where peer_id: PeerId, response: EncodedProof, ) -> CustomMessageOutcome { - match self.sync.on_warp_sync_data(&peer_id, response) { + match self.chain_sync.on_warp_sync_data(&peer_id, response) { Ok(()) => CustomMessageOutcome::None, Err(BadPeer(id, repu)) => { self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC); @@ -803,7 +776,7 @@ where return Err(()) } - if self.config.roles.is_light() { + if self.roles.is_light() { // we're not interested in light peers if status.roles.is_light() { debug!(target: "sync", "Peer {} is unable to serve light requests", who); @@ -826,14 +799,15 @@ where } } - if status.roles.is_full() && self.sync.num_peers() >= self.default_peers_set_num_full { + if status.roles.is_full() && self.chain_sync.num_peers() >= self.default_peers_set_num_full + { debug!(target: "sync", "Too many full nodes, rejecting {}", who); self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC); return Err(()) } if status.roles.is_light() && - (self.peers.len() - self.sync.num_peers()) >= self.default_peers_set_num_light + (self.peers.len() - self.chain_sync.num_peers()) >= self.default_peers_set_num_light { // Make sure that not all slots are occupied by light clients. debug!(target: "sync", "Too many light nodes, rejecting {}", who); @@ -854,7 +828,7 @@ where }; let req = if peer.info.roles.is_full() { - match self.sync.new_peer(who, peer.info.best_hash, peer.info.best_number) { + match self.chain_sync.new_peer(who, peer.info.best_hash, peer.info.best_number) { Ok(req) => req, Err(BadPeer(id, repu)) => { self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC); @@ -958,7 +932,7 @@ where }; if peer.info.roles.is_full() { - self.sync.push_block_announce_validation(who, hash, announce, is_best); + self.chain_sync.push_block_announce_validation(who, hash, announce, is_best); } } @@ -1015,7 +989,7 @@ where // to import header from announced block let's construct response to request that normally // would have been sent over network (but it is not in our case) - let blocks_to_import = self.sync.on_block_data( + let blocks_to_import = self.chain_sync.on_block_data( &who, None, BlockResponse:: { @@ -1052,7 +1026,7 @@ where /// Call this when a block has been finalized. The sync layer may have some additional /// requesting to perform. pub fn on_block_finalized(&mut self, hash: B::Hash, header: &B::Header) { - self.sync.on_block_finalized(&hash, *header.number()) + self.chain_sync.on_block_finalized(&hash, *header.number()) } /// Request a justification for the given block. @@ -1060,12 +1034,12 @@ where /// Uses `protocol` to queue a new justification request and tries to dispatch all pending /// requests. pub fn request_justification(&mut self, hash: &B::Hash, number: NumberFor) { - self.sync.request_justification(hash, number) + self.chain_sync.request_justification(hash, number) } /// Clear all pending justification requests. pub fn clear_justification_requests(&mut self) { - self.sync.clear_justification_requests(); + self.chain_sync.clear_justification_requests(); } /// Request syncing for the given block from given set of peers. @@ -1077,7 +1051,7 @@ where hash: &B::Hash, number: NumberFor, ) { - self.sync.set_sync_fork_request(peers, hash, number) + self.chain_sync.set_sync_fork_request(peers, hash, number) } /// A batch of blocks have been processed, with or without errors. @@ -1089,7 +1063,7 @@ where count: usize, results: Vec<(Result>, BlockImportError>, B::Hash)>, ) { - let results = self.sync.on_blocks_processed(imported, count, results); + let results = self.chain_sync.on_blocks_processed(imported, count, results); for result in results { match result { Ok((id, req)) => { @@ -1116,7 +1090,7 @@ where number: NumberFor, success: bool, ) { - self.sync.on_justification_import(hash, number, success); + self.chain_sync.on_justification_import(hash, number, success); if !success { info!("💔 Invalid justification provided by {} for #{}", who, hash); self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC); @@ -1238,7 +1212,7 @@ where let n = u64::try_from(self.peers.len()).unwrap_or(std::u64::MAX); metrics.peers.set(n); - let m = self.sync.metrics(); + let m = self.chain_sync.metrics(); metrics.fork_targets.set(m.fork_targets.into()); metrics.queued_blocks.set(m.queued_blocks.into()); @@ -1577,25 +1551,25 @@ where self.tick(); } - for (id, request) in self.sync.block_requests() { + for (id, request) in self.chain_sync.block_requests() { let event = prepare_block_request(&mut self.peers, *id, request); self.pending_messages.push_back(event); } - if let Some((id, request)) = self.sync.state_request() { + if let Some((id, request)) = self.chain_sync.state_request() { let event = prepare_state_request(&mut self.peers, id, request); self.pending_messages.push_back(event); } - for (id, request) in self.sync.justification_requests() { + for (id, request) in self.chain_sync.justification_requests() { let event = prepare_block_request(&mut self.peers, id, request); self.pending_messages.push_back(event); } - if let Some((id, request)) = self.sync.warp_sync_request() { + if let Some((id, request)) = self.chain_sync.warp_sync_request() { let event = prepare_warp_sync_request(&mut self.peers, id, request); self.pending_messages.push_back(event); } // Check if there is any block announcement validation finished. - while let Poll::Ready(result) = self.sync.poll_block_announce_validation(cx) { + while let Poll::Ready(result) = self.chain_sync.poll_block_announce_validation(cx) { match self.process_block_announce_validation_result(result) { CustomMessageOutcome::None => {}, outcome => self.pending_messages.push_back(outcome), @@ -1776,7 +1750,8 @@ where // Make sure that the newly added block announce validation future was // polled once to be registered in the task. - if let Poll::Ready(res) = self.sync.poll_block_announce_validation(cx) { + if let Poll::Ready(res) = self.chain_sync.poll_block_announce_validation(cx) + { self.process_block_announce_validation_result(res) } else { CustomMessageOutcome::None diff --git a/client/network/src/service.rs b/client/network/src/service.rs index c5249c9177ce7..46c0fecee39ff 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -30,7 +30,7 @@ use crate::{ behaviour::{self, Behaviour, BehaviourOut}, bitswap::Bitswap, - config::{parse_str_addr, Params, TransportConfig}, + config::{self, parse_str_addr, Params, TransportConfig}, discovery::DiscoveryConfig, error::Error, network_state::{ @@ -60,7 +60,8 @@ use metrics::{Histogram, HistogramVec, MetricSources, Metrics}; use parking_lot::Mutex; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; -use sc_network_common::sync::{SyncState, SyncStatus}; +use sc_network_common::sync::{SyncMode, SyncState, SyncStatus}; +use sc_network_sync::ChainSync; use sc_peerset::PeersetHandle; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use sp_blockchain::{HeaderBackend, HeaderMetadata}; @@ -207,13 +208,26 @@ where None => (None, None), }; - let (protocol, peerset_handle, mut known_addresses) = Protocol::new( - protocol::ProtocolConfig { - roles: From::from(¶ms.role), - max_parallel_downloads: params.network_config.max_parallel_downloads, - sync_mode: params.network_config.sync_mode.clone(), + let chain_sync = ChainSync::new( + if params.role.is_light() { + SyncMode::Light + } else { + match params.network_config.sync_mode { + config::SyncMode::Full => SyncMode::Full, + config::SyncMode::Fast { skip_proofs, storage_chain_mode } => + SyncMode::LightState { skip_proofs, storage_chain_mode }, + config::SyncMode::Warp => SyncMode::Warp, + } }, params.chain.clone(), + params.block_announce_validator, + params.network_config.max_parallel_downloads, + warp_sync_provider, + ) + .map_err(Box::new)?; + let (protocol, peerset_handle, mut known_addresses) = Protocol::new( + From::from(¶ms.role), + params.chain.clone(), params.protocol_id.clone(), ¶ms.network_config, iter::once(Vec::new()) @@ -222,9 +236,8 @@ where .map(|_| default_notif_handshake_message.clone()), ) .collect(), - params.block_announce_validator, params.metrics_registry.as_ref(), - warp_sync_provider, + Box::new(chain_sync), )?; // List of multiaddresses that we know in the network. diff --git a/client/network/sync/Cargo.toml b/client/network/sync/Cargo.toml index c82895812a18c..f4ab02b53f97f 100644 --- a/client/network/sync/Cargo.toml +++ b/client/network/sync/Cargo.toml @@ -20,7 +20,6 @@ prost-build = "0.9" codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive", ] } -either = "1.5.3" futures = "0.3.21" libp2p = "0.44.0" log = "0.4.16" diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 44a102f4fcb03..e3c26f5ff8fcc 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -43,7 +43,6 @@ use crate::{ warp::{WarpProofImportResult, WarpSync}, }; use codec::Encode; -use either::Either; use extra_requests::ExtraRequests; use futures::{stream::FuturesUnordered, task::Poll, Future, FutureExt, StreamExt}; use libp2p::PeerId; @@ -56,8 +55,8 @@ use sc_network_common::sync::{ FromBlock, }, warp::{EncodedProof, WarpProofRequest, WarpSyncPhase, WarpSyncProgress, WarpSyncProvider}, - BadPeer, Metrics, OnBlockData, OnBlockJustification, OnStateData, PeerInfo, - PollBlockAnnounceValidation, SyncState, SyncStatus, + BadPeer, ChainSync as ChainSyncT, Metrics, OnBlockData, OnBlockJustification, OnStateData, + PeerInfo, PollBlockAnnounceValidation, SyncMode, SyncState, SyncStatus, }; use sp_arithmetic::traits::Saturating; use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata}; @@ -359,19 +358,6 @@ enum PreValidateBlockAnnounce { Skip, } -/// Operation mode. -#[derive(Debug, PartialEq, Eq)] -pub enum SyncMode { - // Sync headers only - Light, - // Sync headers and block bodies - Full, - // Sync headers and the last finalied state - LightState { storage_chain_mode: bool, skip_proofs: bool }, - // Warp sync mode. - Warp, -} - /// Result of [`ChainSync::has_slot_for_block_announce_validation`]. enum HasSlotForBlockAnnounceValidation { /// Yes, there is a slot for the block announce validation. @@ -382,7 +368,7 @@ enum HasSlotForBlockAnnounceValidation { MaximumPeerSlotsReached, } -impl ChainSync +impl ChainSyncT for ChainSync where B: BlockT, Client: HeaderBackend @@ -393,74 +379,13 @@ where + Sync + 'static, { - /// Create a new instance. - pub fn new( - mode: SyncMode, - client: Arc, - block_announce_validator: Box + Send>, - max_parallel_downloads: u32, - warp_sync_provider: Option>>, - ) -> Result { - let mut sync = Self { - client, - peers: HashMap::new(), - blocks: BlockCollection::new(), - best_queued_hash: Default::default(), - best_queued_number: Zero::zero(), - extra_justifications: ExtraRequests::new("justification"), - mode, - queue_blocks: Default::default(), - fork_targets: Default::default(), - allowed_requests: Default::default(), - block_announce_validator, - max_parallel_downloads, - downloaded_blocks: 0, - block_announce_validation: Default::default(), - block_announce_validation_per_peer_stats: Default::default(), - state_sync: None, - warp_sync: None, - warp_sync_provider, - import_existing: false, - gap_sync: None, - }; - sync.reset_sync_start_point()?; - Ok(sync) - } - - fn required_block_attributes(&self) -> BlockAttributes { - match self.mode { - SyncMode::Full => - BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION | BlockAttributes::BODY, - SyncMode::Light => BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION, - SyncMode::LightState { storage_chain_mode: false, .. } | SyncMode::Warp => - BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION | BlockAttributes::BODY, - SyncMode::LightState { storage_chain_mode: true, .. } => - BlockAttributes::HEADER | - BlockAttributes::JUSTIFICATION | - BlockAttributes::INDEXED_BODY, - } - } - - fn skip_execution(&self) -> bool { - match self.mode { - SyncMode::Full => false, - SyncMode::Light => true, - SyncMode::LightState { .. } => true, - SyncMode::Warp => true, - } - } - - /// Returns the state of the sync of the given peer. - /// - /// Returns `None` if the peer is unknown. - pub fn peer_info(&self, who: &PeerId) -> Option> { + fn peer_info(&self, who: &PeerId) -> Option> { self.peers .get(who) .map(|p| PeerInfo { best_hash: p.best_hash, best_number: p.best_number }) } - /// Returns the current sync status. - pub fn status(&self) -> SyncStatus { + fn status(&self) -> SyncStatus { let best_seen = self.peers.values().map(|p| p.best_number).max(); let sync_state = if let Some(n) = best_seen { // A chain is classified as downloading if the provided best block is @@ -496,29 +421,22 @@ where } } - /// Number of active forks requests. This includes - /// requests that are pending or could be issued right away. - pub fn num_sync_requests(&self) -> usize { + fn num_sync_requests(&self) -> usize { self.fork_targets .values() .filter(|f| f.number <= self.best_queued_number) .count() } - /// Number of downloaded blocks. - pub fn num_downloaded_blocks(&self) -> usize { + fn num_downloaded_blocks(&self) -> usize { self.downloaded_blocks } - /// Returns the current number of peers stored within this state machine. - pub fn num_peers(&self) -> usize { + fn num_peers(&self) -> usize { self.peers.len() } - /// Handle a new connected peer. - /// - /// Call this method whenever we connect to a new peer. - pub fn new_peer( + fn new_peer( &mut self, who: PeerId, best_hash: B::Hash, @@ -642,27 +560,21 @@ where } } - /// Signal that a new best block has been imported. - /// `ChainSync` state with that information. - pub fn update_chain_info(&mut self, best_hash: &B::Hash, best_number: NumberFor) { + fn update_chain_info(&mut self, best_hash: &B::Hash, best_number: NumberFor) { self.on_block_queued(best_hash, best_number); } - /// Schedule a justification request for the given block. - pub fn request_justification(&mut self, hash: &B::Hash, number: NumberFor) { + fn request_justification(&mut self, hash: &B::Hash, number: NumberFor) { let client = &self.client; self.extra_justifications .schedule((*hash, number), |base, block| is_descendent_of(&**client, base, block)) } - /// Clear all pending justification requests. - pub fn clear_justification_requests(&mut self) { + fn clear_justification_requests(&mut self) { self.extra_justifications.reset(); } - /// Request syncing for the given block from given set of peers. - // The implementation is similar to on_block_announce with unknown parent hash. - pub fn set_sync_fork_request( + fn set_sync_fork_request( &mut self, mut peers: Vec, hash: &B::Hash, @@ -714,13 +626,12 @@ where .extend(peers); } - /// Get an iterator over all scheduled justification requests. - pub fn justification_requests( + fn justification_requests( &mut self, - ) -> impl Iterator)> + '_ { + ) -> Box)> + '_> { let peers = &mut self.peers; let mut matcher = self.extra_justifications.matcher(); - std::iter::from_fn(move || { + Box::new(std::iter::from_fn(move || { if let Some((peer, request)) = matcher.next(peers) { peers .get_mut(&peer) @@ -740,21 +651,20 @@ where } else { None } - }) + })) } - /// Get an iterator over all block requests of all peers. - pub fn block_requests(&mut self) -> impl Iterator)> + '_ { + fn block_requests(&mut self) -> Box)> + '_> { if self.allowed_requests.is_empty() || self.state_sync.is_some() || self.mode == SyncMode::Warp { - return Either::Left(std::iter::empty()) + return Box::new(std::iter::empty()) } if self.queue_blocks.len() > MAX_IMPORTING_BLOCKS { trace!(target: "sync", "Too many blocks in the queue."); - return Either::Left(std::iter::empty()) + return Box::new(std::iter::empty()) } let major_sync = self.status().state == SyncState::Downloading; let attrs = self.required_block_attributes(); @@ -851,11 +761,10 @@ where None } }); - Either::Right(iter) + Box::new(iter) } - /// Get a state request, if any. - pub fn state_request(&mut self) -> Option<(PeerId, StateRequest)> { + fn state_request(&mut self) -> Option<(PeerId, StateRequest)> { if self.allowed_requests.is_empty() { return None } @@ -900,8 +809,7 @@ where None } - /// Get a warp sync request, if any. - pub fn warp_sync_request(&mut self) -> Option<(PeerId, WarpProofRequest)> { + fn warp_sync_request(&mut self) -> Option<(PeerId, WarpProofRequest)> { if let Some(sync) = &self.warp_sync { if self.allowed_requests.is_empty() || sync.is_complete() || @@ -932,14 +840,7 @@ where None } - /// Handle a response from the remote to a block request that we made. - /// - /// `request` must be the original request that triggered `response`. - /// or `None` if data comes from the block announcement. - /// - /// If this corresponds to a valid block, this outputs the block that - /// must be imported in the import queue. - pub fn on_block_data( + fn on_block_data( &mut self, who: &PeerId, request: Option>, @@ -1163,10 +1064,7 @@ where Ok(self.validate_and_queue_blocks(new_blocks, gap)) } - /// Handle a response from the remote to a state request that we made. - /// - /// Returns next request if any. - pub fn on_state_data( + fn on_state_data( &mut self, who: &PeerId, response: StateResponse, @@ -1226,14 +1124,7 @@ where } } - /// Handle a response from the remote to a warp proof request that we made. - /// - /// Returns next request. - pub fn on_warp_sync_data( - &mut self, - who: &PeerId, - response: EncodedProof, - ) -> Result<(), BadPeer> { + fn on_warp_sync_data(&mut self, who: &PeerId, response: EncodedProof) -> Result<(), BadPeer> { if let Some(peer) = self.peers.get_mut(who) { if let PeerSyncState::DownloadingWarpProof = peer.state { peer.state = PeerSyncState::Available; @@ -1262,51 +1153,7 @@ where } } - fn validate_and_queue_blocks( - &mut self, - mut new_blocks: Vec>, - gap: bool, - ) -> OnBlockData { - let orig_len = new_blocks.len(); - new_blocks.retain(|b| !self.queue_blocks.contains(&b.hash)); - if new_blocks.len() != orig_len { - debug!( - target: "sync", - "Ignoring {} blocks that are already queued", - orig_len - new_blocks.len(), - ); - } - - let origin = if !gap && self.status().state != SyncState::Downloading { - BlockOrigin::NetworkBroadcast - } else { - BlockOrigin::NetworkInitialSync - }; - - if let Some((h, n)) = new_blocks - .last() - .and_then(|b| b.header.as_ref().map(|h| (&b.hash, *h.number()))) - { - trace!( - target:"sync", - "Accepted {} blocks ({:?}) with origin {:?}", - new_blocks.len(), - h, - origin, - ); - self.on_block_queued(h, n) - } - self.queue_blocks.extend(new_blocks.iter().map(|b| b.hash)); - OnBlockData::Import(origin, new_blocks) - } - - /// Handle a response from the remote to a justification request that we made. - /// - /// `request` must be the original request that triggered `response`. - /// - /// Returns `Some` if this produces a justification that must be imported - /// into the import queue. - pub fn on_block_justification( + fn on_block_justification( &mut self, who: PeerId, response: BlockResponse, @@ -1361,18 +1208,12 @@ where Ok(OnBlockJustification::Nothing) } - /// A batch of blocks have been processed, with or without errors. - /// - /// Call this when a batch of blocks have been processed by the import - /// queue, with or without errors. - /// - /// `peer_info` is passed in case of a restart. - pub fn on_blocks_processed( + fn on_blocks_processed( &mut self, imported: usize, count: usize, results: Vec<(Result>, BlockImportError>, B::Hash)>, - ) -> impl Iterator), BadPeer>> { + ) -> Box), BadPeer>>> { trace!(target: "sync", "Imported {} of {}", imported, count); let mut output = Vec::new(); @@ -1511,20 +1352,17 @@ where } self.allowed_requests.set_all(); - output.into_iter() + Box::new(output.into_iter()) } - /// Call this when a justification has been processed by the import queue, - /// with or without errors. - pub fn on_justification_import(&mut self, hash: B::Hash, number: NumberFor, success: bool) { + fn on_justification_import(&mut self, hash: B::Hash, number: NumberFor, success: bool) { let finalization_result = if success { Ok((hash, number)) } else { Err(()) }; self.extra_justifications .try_finalize_root((hash, number), finalization_result, true); self.allowed_requests.set_all(); } - /// Notify about finalization of the given block. - pub fn on_block_finalized(&mut self, hash: &B::Hash, number: NumberFor) { + fn on_block_finalized(&mut self, hash: &B::Hash, number: NumberFor) { let client = &self.client; let r = self.extra_justifications.on_block_finalized(hash, number, |base, block| { is_descendent_of(&**client, base, block) @@ -1562,85 +1400,7 @@ where } } - /// Called when a block has been queued for import. - /// - /// Updates our internal state for best queued block and then goes - /// through all peers to update our view of their state as well. - fn on_block_queued(&mut self, hash: &B::Hash, number: NumberFor) { - if self.fork_targets.remove(hash).is_some() { - trace!(target: "sync", "Completed fork sync {:?}", hash); - } - if let Some(gap_sync) = &mut self.gap_sync { - if number > gap_sync.best_queued_number && number <= gap_sync.target { - gap_sync.best_queued_number = number; - } - } - if number > self.best_queued_number { - self.best_queued_number = number; - self.best_queued_hash = *hash; - // Update common blocks - for (n, peer) in self.peers.iter_mut() { - if let PeerSyncState::AncestorSearch { .. } = peer.state { - // Wait for ancestry search to complete first. - continue - } - let new_common_number = - if peer.best_number >= number { number } else { peer.best_number }; - trace!( - target: "sync", - "Updating peer {} info, ours={}, common={}->{}, their best={}", - n, - number, - peer.common_number, - new_common_number, - peer.best_number, - ); - peer.common_number = new_common_number; - } - } - self.allowed_requests.set_all(); - } - - /// Checks if there is a slot for a block announce validation. - /// - /// The total number and the number per peer of concurrent block announce validations - /// is capped. - /// - /// Returns [`HasSlotForBlockAnnounceValidation`] to inform about the result. - /// - /// # Note - /// - /// It is *required* to call [`Self::peer_block_announce_validation_finished`] when the - /// validation is finished to clear the slot. - fn has_slot_for_block_announce_validation( - &mut self, - peer: &PeerId, - ) -> HasSlotForBlockAnnounceValidation { - if self.block_announce_validation.len() >= MAX_CONCURRENT_BLOCK_ANNOUNCE_VALIDATIONS { - return HasSlotForBlockAnnounceValidation::TotalMaximumSlotsReached - } - - match self.block_announce_validation_per_peer_stats.entry(*peer) { - Entry::Vacant(entry) => { - entry.insert(1); - HasSlotForBlockAnnounceValidation::Yes - }, - Entry::Occupied(mut entry) => { - if *entry.get() < MAX_CONCURRENT_BLOCK_ANNOUNCE_VALIDATIONS_PER_PEER { - *entry.get_mut() += 1; - HasSlotForBlockAnnounceValidation::Yes - } else { - HasSlotForBlockAnnounceValidation::MaximumPeerSlotsReached - } - }, - } - } - - /// Push a block announce validation. - /// - /// It is required that [`ChainSync::poll_block_announce_validation`] is called - /// to check for finished block announce validations. - pub fn push_block_announce_validation( + fn push_block_announce_validation( &mut self, who: PeerId, hash: B::Hash, @@ -1743,16 +1503,7 @@ where ); } - /// Poll block announce validation. - /// - /// Block announce validations can be pushed by using - /// [`ChainSync::push_block_announce_validation`]. - /// - /// This should be polled until it returns [`Poll::Pending`]. - /// - /// If [`PollBlockAnnounceValidation::ImportHeader`] is returned, then the caller MUST try to - /// import passed header (call `on_block_data`). The network request isn't sent in this case. - pub fn poll_block_announce_validation( + fn poll_block_announce_validation( &mut self, cx: &mut std::task::Context, ) -> Poll> { @@ -1765,6 +1516,216 @@ where } } + fn peer_disconnected(&mut self, who: &PeerId) -> Option> { + self.blocks.clear_peer_download(who); + if let Some(gap_sync) = &mut self.gap_sync { + gap_sync.blocks.clear_peer_download(who) + } + self.peers.remove(who); + self.extra_justifications.peer_disconnected(who); + self.allowed_requests.set_all(); + self.fork_targets.retain(|_, target| { + target.peers.remove(who); + !target.peers.is_empty() + }); + let blocks = self.drain_blocks(); + if !blocks.is_empty() { + Some(self.validate_and_queue_blocks(blocks, false)) + } else { + None + } + } + + fn metrics(&self) -> Metrics { + Metrics { + queued_blocks: self.queue_blocks.len().try_into().unwrap_or(std::u32::MAX), + fork_targets: self.fork_targets.len().try_into().unwrap_or(std::u32::MAX), + justifications: self.extra_justifications.metrics(), + } + } +} + +impl ChainSync +where + Self: ChainSyncT, + B: BlockT, + Client: HeaderBackend + + BlockBackend + + HeaderMetadata + + ProofProvider + + Send + + Sync + + 'static, +{ + /// Create a new instance. + pub fn new( + mode: SyncMode, + client: Arc, + block_announce_validator: Box + Send>, + max_parallel_downloads: u32, + warp_sync_provider: Option>>, + ) -> Result { + let mut sync = Self { + client, + peers: HashMap::new(), + blocks: BlockCollection::new(), + best_queued_hash: Default::default(), + best_queued_number: Zero::zero(), + extra_justifications: ExtraRequests::new("justification"), + mode, + queue_blocks: Default::default(), + fork_targets: Default::default(), + allowed_requests: Default::default(), + block_announce_validator, + max_parallel_downloads, + downloaded_blocks: 0, + block_announce_validation: Default::default(), + block_announce_validation_per_peer_stats: Default::default(), + state_sync: None, + warp_sync: None, + warp_sync_provider, + import_existing: false, + gap_sync: None, + }; + sync.reset_sync_start_point()?; + Ok(sync) + } + + fn required_block_attributes(&self) -> BlockAttributes { + match self.mode { + SyncMode::Full => + BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION | BlockAttributes::BODY, + SyncMode::Light => BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION, + SyncMode::LightState { storage_chain_mode: false, .. } | SyncMode::Warp => + BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION | BlockAttributes::BODY, + SyncMode::LightState { storage_chain_mode: true, .. } => + BlockAttributes::HEADER | + BlockAttributes::JUSTIFICATION | + BlockAttributes::INDEXED_BODY, + } + } + + fn skip_execution(&self) -> bool { + match self.mode { + SyncMode::Full => false, + SyncMode::Light => true, + SyncMode::LightState { .. } => true, + SyncMode::Warp => true, + } + } + + fn validate_and_queue_blocks( + &mut self, + mut new_blocks: Vec>, + gap: bool, + ) -> OnBlockData { + let orig_len = new_blocks.len(); + new_blocks.retain(|b| !self.queue_blocks.contains(&b.hash)); + if new_blocks.len() != orig_len { + debug!( + target: "sync", + "Ignoring {} blocks that are already queued", + orig_len - new_blocks.len(), + ); + } + + let origin = if !gap && self.status().state != SyncState::Downloading { + BlockOrigin::NetworkBroadcast + } else { + BlockOrigin::NetworkInitialSync + }; + + if let Some((h, n)) = new_blocks + .last() + .and_then(|b| b.header.as_ref().map(|h| (&b.hash, *h.number()))) + { + trace!( + target:"sync", + "Accepted {} blocks ({:?}) with origin {:?}", + new_blocks.len(), + h, + origin, + ); + self.on_block_queued(h, n) + } + self.queue_blocks.extend(new_blocks.iter().map(|b| b.hash)); + OnBlockData::Import(origin, new_blocks) + } + + /// Called when a block has been queued for import. + /// + /// Updates our internal state for best queued block and then goes + /// through all peers to update our view of their state as well. + fn on_block_queued(&mut self, hash: &B::Hash, number: NumberFor) { + if self.fork_targets.remove(hash).is_some() { + trace!(target: "sync", "Completed fork sync {:?}", hash); + } + if let Some(gap_sync) = &mut self.gap_sync { + if number > gap_sync.best_queued_number && number <= gap_sync.target { + gap_sync.best_queued_number = number; + } + } + if number > self.best_queued_number { + self.best_queued_number = number; + self.best_queued_hash = *hash; + // Update common blocks + for (n, peer) in self.peers.iter_mut() { + if let PeerSyncState::AncestorSearch { .. } = peer.state { + // Wait for ancestry search to complete first. + continue + } + let new_common_number = + if peer.best_number >= number { number } else { peer.best_number }; + trace!( + target: "sync", + "Updating peer {} info, ours={}, common={}->{}, their best={}", + n, + number, + peer.common_number, + new_common_number, + peer.best_number, + ); + peer.common_number = new_common_number; + } + } + self.allowed_requests.set_all(); + } + + /// Checks if there is a slot for a block announce validation. + /// + /// The total number and the number per peer of concurrent block announce validations + /// is capped. + /// + /// Returns [`HasSlotForBlockAnnounceValidation`] to inform about the result. + /// + /// # Note + /// + /// It is *required* to call [`Self::peer_block_announce_validation_finished`] when the + /// validation is finished to clear the slot. + fn has_slot_for_block_announce_validation( + &mut self, + peer: &PeerId, + ) -> HasSlotForBlockAnnounceValidation { + if self.block_announce_validation.len() >= MAX_CONCURRENT_BLOCK_ANNOUNCE_VALIDATIONS { + return HasSlotForBlockAnnounceValidation::TotalMaximumSlotsReached + } + + match self.block_announce_validation_per_peer_stats.entry(*peer) { + Entry::Vacant(entry) => { + entry.insert(1); + HasSlotForBlockAnnounceValidation::Yes + }, + Entry::Occupied(mut entry) => { + if *entry.get() < MAX_CONCURRENT_BLOCK_ANNOUNCE_VALIDATIONS_PER_PEER { + *entry.get_mut() += 1; + HasSlotForBlockAnnounceValidation::Yes + } else { + HasSlotForBlockAnnounceValidation::MaximumPeerSlotsReached + } + }, + } + } + /// Should be called when a block announce validation is finished, to update the slots /// of the peer that send the block announce. fn peer_block_announce_validation_finished( @@ -1922,29 +1883,6 @@ where PollBlockAnnounceValidation::Nothing { is_best, who, announce } } - /// Call when a peer has disconnected. - /// Canceled obsolete block request may result in some blocks being ready for - /// import, so this functions checks for such blocks and returns them. - pub fn peer_disconnected(&mut self, who: &PeerId) -> Option> { - self.blocks.clear_peer_download(who); - if let Some(gap_sync) = &mut self.gap_sync { - gap_sync.blocks.clear_peer_download(who) - } - self.peers.remove(who); - self.extra_justifications.peer_disconnected(who); - self.allowed_requests.set_all(); - self.fork_targets.retain(|_, target| { - target.peers.remove(who); - !target.peers.is_empty() - }); - let blocks = self.drain_blocks(); - if !blocks.is_empty() { - Some(self.validate_and_queue_blocks(blocks, false)) - } else { - None - } - } - /// Restart the sync process. This will reset all pending block requests and return an iterator /// of new block requests to make to peers. Peers that were downloading finality data (i.e. /// their state was `DownloadingJustification`) are unaffected and will stay in the same state. @@ -2047,15 +1985,6 @@ where .any(|(_, p)| p.state == PeerSyncState::DownloadingStale(*hash)) } - /// Return some key metrics. - pub fn metrics(&self) -> Metrics { - Metrics { - queued_blocks: self.queue_blocks.len().try_into().unwrap_or(std::u32::MAX), - fork_targets: self.fork_targets.len().try_into().unwrap_or(std::u32::MAX), - justifications: self.extra_justifications.metrics(), - } - } - /// Drain the downloaded block set up to the first gap. fn drain_blocks(&mut self) -> Vec> { self.blocks diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index d833238f1342c..e6cc282c95b38 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -50,8 +50,8 @@ pub use sc_network::config::EmptyTransactionPool; use sc_network::{ block_request_handler::BlockRequestHandler, config::{ - MultiaddrWithPeerId, NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, - ProtocolConfig, Role, SyncMode, TransportConfig, + MultiaddrWithPeerId, NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, Role, + SyncMode, TransportConfig, }, state_request_handler::StateRequestHandler, warp_request_handler, Multiaddr, NetworkService, NetworkWorker, @@ -686,7 +686,7 @@ pub struct FullPeerConfig { pub storage_chain: bool, } -pub trait TestNetFactory: Sized +pub trait TestNetFactory: Default + Sized where >::Transaction: Send, { @@ -694,14 +694,8 @@ where type BlockImport: BlockImport + Clone + Send + Sync + 'static; type PeerData: Default; - /// These two need to be implemented! - fn from_config(config: &ProtocolConfig) -> Self; - fn make_verifier( - &self, - client: PeersClient, - config: &ProtocolConfig, - peer_data: &Self::PeerData, - ) -> Self::Verifier; + /// This one needs to be implemented! + fn make_verifier(&self, client: PeersClient, peer_data: &Self::PeerData) -> Self::Verifier; /// Get reference to peer. fn peer(&mut self, i: usize) -> &mut Peer; @@ -721,15 +715,10 @@ where Self::PeerData, ); - fn default_config() -> ProtocolConfig { - ProtocolConfig::default() - } - /// Create new test network with this many peers. fn new(n: usize) -> Self { trace!(target: "test_network", "Creating test network"); - let config = Self::default_config(); - let mut net = Self::from_config(&config); + let mut net = Self::default(); for i in 0..n { trace!(target: "test_network", "Adding peer {}", i); @@ -765,11 +754,8 @@ where let (block_import, justification_import, data) = self .make_block_import(PeersClient { client: client.clone(), backend: backend.clone() }); - let verifier = self.make_verifier( - PeersClient { client: client.clone(), backend: backend.clone() }, - &Default::default(), - &data, - ); + let verifier = self + .make_verifier(PeersClient { client: client.clone(), backend: backend.clone() }, &data); let verifier = VerifierAdapter::new(verifier); let import_queue = Box::new(BasicQueue::new( @@ -1010,6 +996,7 @@ where } } +#[derive(Default)] pub struct TestNet { peers: Vec>, } @@ -1019,17 +1006,7 @@ impl TestNetFactory for TestNet { type PeerData = (); type BlockImport = PeersClient; - /// Create new test network with peers and given config. - fn from_config(_config: &ProtocolConfig) -> Self { - TestNet { peers: Vec::new() } - } - - fn make_verifier( - &self, - _client: PeersClient, - _config: &ProtocolConfig, - _peer_data: &(), - ) -> Self::Verifier { + fn make_verifier(&self, _client: PeersClient, _peer_data: &()) -> Self::Verifier { PassThroughVerifier::new(false) } @@ -1079,6 +1056,7 @@ impl JustificationImport for ForceFinalized { } } +#[derive(Default)] pub struct JustificationTestNet(TestNet); impl TestNetFactory for JustificationTestNet { @@ -1086,17 +1064,8 @@ impl TestNetFactory for JustificationTestNet { type PeerData = (); type BlockImport = PeersClient; - fn from_config(config: &ProtocolConfig) -> Self { - JustificationTestNet(TestNet::from_config(config)) - } - - fn make_verifier( - &self, - client: PeersClient, - config: &ProtocolConfig, - peer_data: &(), - ) -> Self::Verifier { - self.0.make_verifier(client, config, peer_data) + fn make_verifier(&self, client: PeersClient, peer_data: &()) -> Self::Verifier { + self.0.make_verifier(client, peer_data) } fn peer(&mut self, i: usize) -> &mut Peer { From d6d0a46a187a88ebf0817545003d9d8626504c86 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 12 May 2022 05:13:04 +0300 Subject: [PATCH 06/15] Introduce `OpaqueStateRequest`/`OpaqueStateResponse` to remove generics from `StateSync` trait --- client/network/common/src/sync.rs | 38 +++++++++++++++++---- client/network/src/behaviour.rs | 33 +++++++++--------- client/network/src/bitswap.rs | 3 +- client/network/src/protocol.rs | 56 ++++++++++++------------------- client/network/sync/src/lib.rs | 46 ++++++++++++++++++++----- 5 files changed, 108 insertions(+), 68 deletions(-) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index 5c0432bf0b5cb..f4bf47fe743e9 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -30,7 +30,7 @@ use sp_runtime::{ traits::{Block as BlockT, NumberFor}, Justifications, }; -use std::{fmt, task::Poll}; +use std::{any::Any, fmt, fmt::Formatter, task::Poll}; use warp::{EncodedProof, WarpProofRequest, WarpSyncProgress}; /// The sync status of a peer we are trying to sync with @@ -175,9 +175,29 @@ pub struct Metrics { pub justifications: extra_requests::Metrics, } -// TODO: Make concrete types for `StateRequest` and `StateResponse` that can be converted to network -// messages from scheme later -pub trait ChainSync: Send { +/// Wrapper for implementation-specific state request. +/// +/// NOTE: Implementation must be able to encode and decode it for network purposes. +pub struct OpaqueStateRequest(pub Box); + +impl fmt::Debug for OpaqueStateRequest { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("OpaqueStateRequest").finish() + } +} + +/// Wrapper for implementation-specific state response. +/// +/// NOTE: Implementation must be able to encode and decode it for network purposes. +pub struct OpaqueStateResponse(pub Box); + +impl fmt::Debug for OpaqueStateResponse { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("OpaqueStateResponse").finish() + } +} + +pub trait ChainSync: Send { /// Returns the state of the sync of the given peer. /// /// Returns `None` if the peer is unknown. @@ -234,7 +254,7 @@ pub trait ChainSync: Send { fn block_requests(&mut self) -> Box)> + '_>; /// Get a state request, if any. - fn state_request(&mut self) -> Option<(PeerId, StateRequest)>; + fn state_request(&mut self) -> Option<(PeerId, OpaqueStateRequest)>; /// Get a warp sync request, if any. fn warp_sync_request(&mut self) -> Option<(PeerId, WarpProofRequest)>; @@ -259,7 +279,7 @@ pub trait ChainSync: Send { fn on_state_data( &mut self, who: &PeerId, - response: StateResponse, + response: OpaqueStateResponse, ) -> Result, BadPeer>; /// Handle a response from the remote to a warp proof request that we made. @@ -337,4 +357,10 @@ pub trait ChainSync: Send { /// Return some key metrics. fn metrics(&self) -> Metrics; + + /// Encode implementation-specific state request. + fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result, String>; + + /// Decode implementation-specific state response. + fn decode_state_response(&self, response: &[u8]) -> Result; } diff --git a/client/network/src/behaviour.rs b/client/network/src/behaviour.rs index 091dd116e4c9c..394de62ce5827 100644 --- a/client/network/src/behaviour.rs +++ b/client/network/src/behaviour.rs @@ -401,23 +401,24 @@ where ); }, CustomMessageOutcome::StateRequest { target, request, pending_response } => { - let mut buf = Vec::with_capacity(request.encoded_len()); - if let Err(err) = request.encode(&mut buf) { - log::warn!( - target: "sync", - "Failed to encode state request {:?}: {:?}", - request, err - ); - return + match self.substrate.encode_state_request(&request) { + Ok(data) => { + self.request_responses.send_request( + &target, + &self.state_request_protocol_name, + data, + pending_response, + IfDisconnected::ImmediateError, + ); + }, + Err(err) => { + log::warn!( + target: "sync", + "Failed to encode state request {:?}: {:?}", + request, err + ); + }, } - - self.request_responses.send_request( - &target, - &self.state_request_protocol_name, - buf, - pending_response, - IfDisconnected::ImmediateError, - ); }, CustomMessageOutcome::WarpSyncRequest { target, request, pending_response } => match &self.warp_sync_protocol_name { diff --git a/client/network/src/bitswap.rs b/client/network/src/bitswap.rs index d5039faaca113..14c730e75981a 100644 --- a/client/network/src/bitswap.rs +++ b/client/network/src/bitswap.rs @@ -118,8 +118,7 @@ where fn upgrade_outbound(self, mut socket: TSocket, _info: Self::Info) -> Self::Future { Box::pin(async move { - let mut data = Vec::with_capacity(self.encoded_len()); - self.encode(&mut data)?; + let data = self.encode_to_vec(); upgrade::write_length_prefixed(&mut socket, data).await }) } diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index b50ea366fcde7..a4003050bc085 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -83,6 +83,7 @@ pub mod event; pub mod message; pub use notifications::{NotificationsSink, NotifsHandlerError, Ready}; +use sc_network_common::sync::{OpaqueStateRequest, OpaqueStateResponse}; use sp_blockchain::HeaderMetadata; /// Interval at which we perform time based maintenance @@ -176,13 +177,7 @@ pub struct Protocol { genesis_hash: B::Hash, /// State machine that handles the list of in-progress requests. Only full node peers are /// registered. - chain_sync: Box< - dyn ChainSync< - B, - sc_network_sync::schema::v1::StateRequest, - sc_network_sync::schema::v1::StateResponse, - >, - >, + chain_sync: Box>, // All connected peers. Contains both full and light node peers. peers: HashMap>, chain: Arc, @@ -285,14 +280,8 @@ where network_config: &config::NetworkConfiguration, notifications_protocols_handshakes: Vec>, metrics_registry: Option<&Registry>, - chain_sync: Box< - dyn ChainSync< - B, - sc_network_sync::schema::v1::StateRequest, - sc_network_sync::schema::v1::StateResponse, - >, - >, - ) -> error::Result<(Protocol, sc_peerset::PeersetHandle, Vec<(PeerId, Multiaddr)>)> { + chain_sync: Box>, + ) -> error::Result<(Self, sc_peerset::PeersetHandle, Vec<(PeerId, Multiaddr)>)> { let info = chain.info(); let boot_node_ids = { @@ -541,14 +530,6 @@ where self.peers.iter().map(|(id, peer)| (id, &peer.info)) } - fn prepare_block_request( - &mut self, - who: PeerId, - request: BlockRequest, - ) -> CustomMessageOutcome { - prepare_block_request::(&mut self.peers, who, request) - } - /// Called by peer when it is disconnecting. /// /// Returns a result if the handshake of this peer was indeed accepted. @@ -682,7 +663,8 @@ where match self.chain_sync.on_block_data(&peer_id, Some(request), block_response) { Ok(OnBlockData::Import(origin, blocks)) => CustomMessageOutcome::BlockImport(origin, blocks), - Ok(OnBlockData::Request(peer, req)) => self.prepare_block_request(peer, req), + Ok(OnBlockData::Request(peer, req)) => + prepare_block_request(&mut self.peers, peer, req), Err(BadPeer(id, repu)) => { self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC); self.peerset_handle.report_peer(id, repu); @@ -697,7 +679,7 @@ where pub fn on_state_response( &mut self, peer_id: PeerId, - response: sc_network_sync::schema::v1::StateResponse, + response: OpaqueStateResponse, ) -> CustomMessageOutcome { match self.chain_sync.on_state_data(&peer_id, response) { Ok(OnStateData::Import(origin, block)) => @@ -847,8 +829,8 @@ where .push_back(CustomMessageOutcome::PeerNewBest(who, status.best_number)); if let Some(req) = req { - let event = self.prepare_block_request(who, req); - self.pending_messages.push_back(event); + self.pending_messages + .push_back(prepare_block_request(&mut self.peers, who, req)); } Ok(()) @@ -1014,7 +996,8 @@ where match blocks_to_import { Ok(OnBlockData::Import(origin, blocks)) => CustomMessageOutcome::BlockImport(origin, blocks), - Ok(OnBlockData::Request(peer, req)) => self.prepare_block_request(peer, req), + Ok(OnBlockData::Request(peer, req)) => + prepare_block_request(&mut self.peers, peer, req), Err(BadPeer(id, repu)) => { self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC); self.peerset_handle.report_peer(id, repu); @@ -1207,6 +1190,11 @@ where } } + /// Encode implementation-specific state request. + pub fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result, String> { + self.chain_sync.encode_state_request(request) + } + fn report_metrics(&self) { if let Some(metrics) = &self.metrics { let n = u64::try_from(self.peers.len()).unwrap_or(std::u64::MAX); @@ -1268,7 +1256,7 @@ fn prepare_block_request( fn prepare_state_request( peers: &mut HashMap>, who: PeerId, - request: sc_network_sync::schema::v1::StateRequest, + request: OpaqueStateRequest, ) -> CustomMessageOutcome { let (tx, rx) = oneshot::channel(); @@ -1331,7 +1319,7 @@ pub enum CustomMessageOutcome { /// A new storage request must be emitted. StateRequest { target: PeerId, - request: sc_network_sync::schema::v1::StateRequest, + request: OpaqueStateRequest, pending_response: oneshot::Sender, RequestFailure>>, }, /// A new warp sync request must be emitted. @@ -1456,10 +1444,8 @@ where finished_block_requests.push((*id, req, protobuf_response)); }, PeerRequest::State => { - let protobuf_response = - match sc_network_sync::schema::v1::StateResponse::decode( - &resp[..], - ) { + let response = + match self.chain_sync.decode_state_response(&resp[..]) { Ok(proto) => proto, Err(e) => { debug!( @@ -1475,7 +1461,7 @@ where }, }; - finished_state_requests.push((*id, protobuf_response)); + finished_state_requests.push((*id, response)); }, PeerRequest::WarpProof => { finished_warp_sync_requests.push((*id, resp)); diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index e3c26f5ff8fcc..7b553a1ece89b 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -47,6 +47,7 @@ use extra_requests::ExtraRequests; use futures::{stream::FuturesUnordered, task::Poll, Future, FutureExt, StreamExt}; use libp2p::PeerId; use log::{debug, error, info, trace, warn}; +use prost::Message; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; use sc_network_common::sync::{ @@ -56,7 +57,8 @@ use sc_network_common::sync::{ }, warp::{EncodedProof, WarpProofRequest, WarpSyncPhase, WarpSyncProgress, WarpSyncProvider}, BadPeer, ChainSync as ChainSyncT, Metrics, OnBlockData, OnBlockJustification, OnStateData, - PeerInfo, PollBlockAnnounceValidation, SyncMode, SyncState, SyncStatus, + OpaqueStateRequest, OpaqueStateResponse, PeerInfo, PollBlockAnnounceValidation, SyncMode, + SyncState, SyncStatus, }; use sp_arithmetic::traits::Saturating; use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata}; @@ -368,7 +370,7 @@ enum HasSlotForBlockAnnounceValidation { MaximumPeerSlotsReached, } -impl ChainSyncT for ChainSync +impl ChainSyncT for ChainSync where B: BlockT, Client: HeaderBackend @@ -764,7 +766,7 @@ where Box::new(iter) } - fn state_request(&mut self) -> Option<(PeerId, StateRequest)> { + fn state_request(&mut self) -> Option<(PeerId, OpaqueStateRequest)> { if self.allowed_requests.is_empty() { return None } @@ -785,7 +787,7 @@ where let request = sync.next_request(); trace!(target: "sync", "New StateRequest for {}: {:?}", id, request); self.allowed_requests.clear(); - return Some((*id, request)) + return Some((*id, OpaqueStateRequest(Box::new(request)))) } } } @@ -801,7 +803,7 @@ where trace!(target: "sync", "New StateRequest for {}: {:?}", id, request); peer.state = PeerSyncState::DownloadingState; self.allowed_requests.clear(); - return Some((*id, request)) + return Some((*id, OpaqueStateRequest(Box::new(request)))) } } } @@ -1067,8 +1069,17 @@ where fn on_state_data( &mut self, who: &PeerId, - response: StateResponse, + response: OpaqueStateResponse, ) -> Result, BadPeer> { + let response: Box = response.0.downcast().map_err(|_error| { + error!( + target: "sync", + "Failed to downcast opaque state response, this is an implementation bug." + ); + + BadPeer(*who, rep::BAD_RESPONSE) + })?; + if let Some(peer) = self.peers.get_mut(who) { if let PeerSyncState::DownloadingState = peer.state { peer.state = PeerSyncState::Available; @@ -1083,7 +1094,7 @@ where response.entries.len(), response.proof.len(), ); - sync.import(response) + sync.import(*response) } else if let Some(sync) = &mut self.warp_sync { debug!( target: "sync", @@ -1092,7 +1103,7 @@ where response.entries.len(), response.proof.len(), ); - sync.import_state(response) + sync.import_state(*response) } else { debug!(target: "sync", "Ignored obsolete state response from {}", who); return Err(BadPeer(*who, rep::NOT_REQUESTED)) @@ -1543,11 +1554,28 @@ where justifications: self.extra_justifications.metrics(), } } + + fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result, String> { + let request: &StateRequest = request.0.downcast_ref().ok_or_else(|| { + "Failed to downcast opaque state response during encoding, this is an \ + implementation bug." + .to_string() + })?; + + Ok(request.encode_to_vec()) + } + + fn decode_state_response(&self, response: &[u8]) -> Result { + let response = StateResponse::decode(response) + .map_err(|error| format!("Failed to decode state response: {error}"))?; + + Ok(OpaqueStateResponse(Box::new(response))) + } } impl ChainSync where - Self: ChainSyncT, + Self: ChainSyncT, B: BlockT, Client: HeaderBackend + BlockBackend From a91cd0297235ad2e2ad8669c83a5f241d155f9f4 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 12 May 2022 06:17:45 +0300 Subject: [PATCH 07/15] Introduce `OpaqueBlockRequest`/`OpaqueBlockResponse`, make `scheme` module of `sc-network-sync` private --- client/network/common/src/sync.rs | 40 ++++++++- client/network/src/behaviour.rs | 35 ++++---- client/network/src/protocol.rs | 133 +++++++++--------------------- client/network/sync/src/lib.rs | 107 +++++++++++++++++++++++- client/network/sync/src/schema.rs | 2 +- 5 files changed, 202 insertions(+), 115 deletions(-) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index f4bf47fe743e9..90e23e148818e 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -23,7 +23,7 @@ pub mod message; pub mod warp; use libp2p::PeerId; -use message::{BlockAnnounce, BlockRequest, BlockResponse}; +use message::{BlockAnnounce, BlockData, BlockRequest, BlockResponse}; use sc_consensus::{BlockImportError, BlockImportStatus, IncomingBlock}; use sp_consensus::BlockOrigin; use sp_runtime::{ @@ -197,6 +197,28 @@ impl fmt::Debug for OpaqueStateResponse { } } +/// Wrapper for implementation-specific block request. +/// +/// NOTE: Implementation must be able to encode and decode it for network purposes. +pub struct OpaqueBlockRequest(pub Box); + +impl fmt::Debug for OpaqueBlockRequest { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("OpaqueBlockRequest").finish() + } +} + +/// Wrapper for implementation-specific block response. +/// +/// NOTE: Implementation must be able to encode and decode it for network purposes. +pub struct OpaqueBlockResponse(pub Box); + +impl fmt::Debug for OpaqueBlockResponse { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("OpaqueBlockResponse").finish() + } +} + pub trait ChainSync: Send { /// Returns the state of the sync of the given peer. /// @@ -358,6 +380,22 @@ pub trait ChainSync: Send { /// Return some key metrics. fn metrics(&self) -> Metrics; + /// Create implementation-specific block request. + fn create_opaque_block_request(&self, request: &BlockRequest) -> OpaqueBlockRequest; + + /// Encode implementation-specific block request. + fn encode_block_request(&self, request: &OpaqueBlockRequest) -> Result, String>; + + /// Decode implementation-specific block response. + fn decode_block_response(&self, response: &[u8]) -> Result; + + /// Access blocks from implementation-specific block response. + fn block_response_into_blocks( + &self, + request: &BlockRequest, + response: OpaqueBlockResponse, + ) -> Result>, String>; + /// Encode implementation-specific state request. fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result, String>; diff --git a/client/network/src/behaviour.rs b/client/network/src/behaviour.rs index 394de62ce5827..515608df13d0f 100644 --- a/client/network/src/behaviour.rs +++ b/client/network/src/behaviour.rs @@ -38,7 +38,7 @@ use libp2p::{ NetworkBehaviour, }; use log::debug; -use prost::Message; + use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::import_queue::{IncomingBlock, Origin}; use sc_network_common::{config::ProtocolId, request_responses::ProtocolConfig}; @@ -382,23 +382,24 @@ where .events .push_back(BehaviourOut::JustificationImport(origin, hash, nb, justification)), CustomMessageOutcome::BlockRequest { target, request, pending_response } => { - let mut buf = Vec::with_capacity(request.encoded_len()); - if let Err(err) = request.encode(&mut buf) { - log::warn!( - target: "sync", - "Failed to encode block request {:?}: {:?}", - request, err - ); - return + match self.substrate.encode_block_request(&request) { + Ok(data) => { + self.request_responses.send_request( + &target, + &self.block_request_protocol_name, + data, + pending_response, + IfDisconnected::ImmediateError, + ); + }, + Err(err) => { + log::warn!( + target: "sync", + "Failed to encode block request {:?}: {:?}", + request, err + ); + }, } - - self.request_responses.send_request( - &target, - &self.block_request_protocol_name, - buf, - pending_response, - IfDisconnected::ImmediateError, - ); }, CustomMessageOutcome::StateRequest { target, request, pending_response } => { match self.substrate.encode_state_request(&request) { diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index a4003050bc085..ac91478e8ab15 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -44,7 +44,6 @@ use message::{ }; use notifications::{Notifications, NotificationsOut}; use prometheus_endpoint::{register, Gauge, GaugeVec, Opts, PrometheusError, Registry, U64}; -use prost::Message as _; use sc_client_api::{BlockBackend, HeaderBackend, ProofProvider}; use sc_consensus::import_queue::{BlockImportError, BlockImportStatus, IncomingBlock, Origin}; use sc_network_common::{ @@ -52,14 +51,15 @@ use sc_network_common::{ sync::{ message::{ BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, BlockState, - FromBlock, }, - BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, - PollBlockAnnounceValidation, SyncStatus, + BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, OpaqueBlockRequest, + OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PollBlockAnnounceValidation, + SyncStatus, }, warp_sync::{EncodedProof, WarpProofRequest}, }; use sp_arithmetic::traits::SaturatedConversion; +use sp_blockchain::HeaderMetadata; use sp_consensus::BlockOrigin; use sp_runtime::{ generic::BlockId, @@ -83,8 +83,6 @@ pub mod event; pub mod message; pub use notifications::{NotificationsSink, NotifsHandlerError, Ready}; -use sc_network_common::sync::{OpaqueStateRequest, OpaqueStateResponse}; -use sp_blockchain::HeaderMetadata; /// Interval at which we perform time based maintenance const TICK_TIMEOUT: time::Duration = time::Duration::from_millis(1100); @@ -564,62 +562,9 @@ where &mut self, peer_id: PeerId, request: BlockRequest, - response: sc_network_sync::schema::v1::BlockResponse, + response: OpaqueBlockResponse, ) -> CustomMessageOutcome { - let blocks = response - .blocks - .into_iter() - .map(|block_data| { - Ok(BlockData:: { - hash: Decode::decode(&mut block_data.hash.as_ref())?, - header: if !block_data.header.is_empty() { - Some(Decode::decode(&mut block_data.header.as_ref())?) - } else { - None - }, - body: if request.fields.contains(BlockAttributes::BODY) { - Some( - block_data - .body - .iter() - .map(|body| Decode::decode(&mut body.as_ref())) - .collect::, _>>()?, - ) - } else { - None - }, - indexed_body: if request.fields.contains(BlockAttributes::INDEXED_BODY) { - Some(block_data.indexed_body) - } else { - None - }, - receipt: if !block_data.receipt.is_empty() { - Some(block_data.receipt) - } else { - None - }, - message_queue: if !block_data.message_queue.is_empty() { - Some(block_data.message_queue) - } else { - None - }, - justification: if !block_data.justification.is_empty() { - Some(block_data.justification) - } else if block_data.is_empty_justification { - Some(Vec::new()) - } else { - None - }, - justifications: if !block_data.justifications.is_empty() { - Some(DecodeAll::decode_all(&mut block_data.justifications.as_ref())?) - } else { - None - }, - }) - }) - .collect::, codec::Error>>(); - - let blocks = match blocks { + let blocks = match self.chain_sync.block_response_into_blocks(&request, response) { Ok(blocks) => blocks, Err(err) => { debug!(target: "sync", "Failed to decode block response from {}: {}", peer_id, err); @@ -664,7 +609,7 @@ where Ok(OnBlockData::Import(origin, blocks)) => CustomMessageOutcome::BlockImport(origin, blocks), Ok(OnBlockData::Request(peer, req)) => - prepare_block_request(&mut self.peers, peer, req), + prepare_block_request(self.chain_sync.as_ref(), &mut self.peers, peer, req), Err(BadPeer(id, repu)) => { self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC); self.peerset_handle.report_peer(id, repu); @@ -829,8 +774,12 @@ where .push_back(CustomMessageOutcome::PeerNewBest(who, status.best_number)); if let Some(req) = req { - self.pending_messages - .push_back(prepare_block_request(&mut self.peers, who, req)); + self.pending_messages.push_back(prepare_block_request( + self.chain_sync.as_ref(), + &mut self.peers, + who, + req, + )); } Ok(()) @@ -997,7 +946,7 @@ where Ok(OnBlockData::Import(origin, blocks)) => CustomMessageOutcome::BlockImport(origin, blocks), Ok(OnBlockData::Request(peer, req)) => - prepare_block_request(&mut self.peers, peer, req), + prepare_block_request(self.chain_sync.as_ref(), &mut self.peers, peer, req), Err(BadPeer(id, repu)) => { self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC); self.peerset_handle.report_peer(id, repu); @@ -1051,6 +1000,7 @@ where match result { Ok((id, req)) => { self.pending_messages.push_back(prepare_block_request( + self.chain_sync.as_ref(), &mut self.peers, id, req, @@ -1190,6 +1140,11 @@ where } } + /// Encode implementation-specific block request. + pub fn encode_block_request(&self, request: &OpaqueBlockRequest) -> Result, String> { + self.chain_sync.encode_block_request(request) + } + /// Encode implementation-specific state request. pub fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result, String> { self.chain_sync.encode_state_request(request) @@ -1226,6 +1181,7 @@ where } fn prepare_block_request( + chain_sync: &dyn ChainSync, peers: &mut HashMap>, who: PeerId, request: BlockRequest, @@ -1236,19 +1192,7 @@ fn prepare_block_request( peer.request = Some((PeerRequest::Block(request.clone()), rx)); } - let request = sc_network_sync::schema::v1::BlockRequest { - fields: request.fields.to_be_u32(), - from_block: match request.from { - FromBlock::Hash(h) => - Some(sc_network_sync::schema::v1::block_request::FromBlock::Hash(h.encode())), - FromBlock::Number(n) => - Some(sc_network_sync::schema::v1::block_request::FromBlock::Number(n.encode())), - }, - to_block: request.to.map(|h| h.encode()).unwrap_or_default(), - direction: request.direction as i32, - max_blocks: request.max.unwrap_or(0), - support_multiple_justifications: true, - }; + let request = chain_sync.create_opaque_block_request(&request); CustomMessageOutcome::BlockRequest { target: who, request, pending_response: tx } } @@ -1313,7 +1257,7 @@ pub enum CustomMessageOutcome { /// A new block request must be emitted. BlockRequest { target: PeerId, - request: sc_network_sync::schema::v1::BlockRequest, + request: OpaqueBlockRequest, pending_response: oneshot::Sender, RequestFailure>>, }, /// A new storage request must be emitted. @@ -1422,10 +1366,8 @@ where let (req, _) = peer.request.take().unwrap(); match req { PeerRequest::Block(req) => { - let protobuf_response = - match sc_network_sync::schema::v1::BlockResponse::decode( - &resp[..], - ) { + let response = + match self.chain_sync.decode_block_response(&resp[..]) { Ok(proto) => proto, Err(e) => { debug!( @@ -1441,7 +1383,7 @@ where }, }; - finished_block_requests.push((*id, req, protobuf_response)); + finished_block_requests.push((*id, req, response)); }, PeerRequest::State => { let response = @@ -1520,12 +1462,12 @@ where } } } - for (id, req, protobuf_response) in finished_block_requests { - let ev = self.on_block_response(id, req, protobuf_response); + for (id, req, response) in finished_block_requests { + let ev = self.on_block_response(id, req, response); self.pending_messages.push_back(ev); } - for (id, protobuf_response) in finished_state_requests { - let ev = self.on_state_response(id, protobuf_response); + for (id, response) in finished_state_requests { + let ev = self.on_state_response(id, response); self.pending_messages.push_back(ev); } for (id, response) in finished_warp_sync_requests { @@ -1537,16 +1479,23 @@ where self.tick(); } - for (id, request) in self.chain_sync.block_requests() { - let event = prepare_block_request(&mut self.peers, *id, request); + for (id, request) in self + .chain_sync + .block_requests() + .map(|(peer_id, request)| (*peer_id, request)) + .collect::>() + { + let event = + prepare_block_request(self.chain_sync.as_ref(), &mut self.peers, id, request); self.pending_messages.push_back(event); } if let Some((id, request)) = self.chain_sync.state_request() { let event = prepare_state_request(&mut self.peers, id, request); self.pending_messages.push_back(event); } - for (id, request) in self.chain_sync.justification_requests() { - let event = prepare_block_request(&mut self.peers, id, request); + for (id, request) in self.chain_sync.justification_requests().collect::>() { + let event = + prepare_block_request(self.chain_sync.as_ref(), &mut self.peers, id, request); self.pending_messages.push_back(event); } if let Some((id, request)) = self.chain_sync.warp_sync_request() { diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 7b553a1ece89b..bcf3f270b9790 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -30,7 +30,7 @@ pub mod block_request_handler; pub mod blocks; -pub mod schema; +mod schema; pub mod state; pub mod state_request_handler; pub mod warp; @@ -42,7 +42,7 @@ use crate::{ state::StateSync, warp::{WarpProofImportResult, WarpSync}, }; -use codec::Encode; +use codec::{Decode, DecodeAll, Encode}; use extra_requests::ExtraRequests; use futures::{stream::FuturesUnordered, task::Poll, Future, FutureExt, StreamExt}; use libp2p::PeerId; @@ -57,8 +57,8 @@ use sc_network_common::sync::{ }, warp::{EncodedProof, WarpProofRequest, WarpSyncPhase, WarpSyncProgress, WarpSyncProvider}, BadPeer, ChainSync as ChainSyncT, Metrics, OnBlockData, OnBlockJustification, OnStateData, - OpaqueStateRequest, OpaqueStateResponse, PeerInfo, PollBlockAnnounceValidation, SyncMode, - SyncState, SyncStatus, + OpaqueBlockRequest, OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PeerInfo, + PollBlockAnnounceValidation, SyncMode, SyncState, SyncStatus, }; use sp_arithmetic::traits::Saturating; use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata}; @@ -1555,6 +1555,105 @@ where } } + /// Create implementation-specific block request. + fn create_opaque_block_request(&self, request: &BlockRequest) -> OpaqueBlockRequest { + OpaqueBlockRequest(Box::new(schema::v1::BlockRequest { + fields: request.fields.to_be_u32(), + from_block: match request.from { + FromBlock::Hash(h) => Some(schema::v1::block_request::FromBlock::Hash(h.encode())), + FromBlock::Number(n) => + Some(schema::v1::block_request::FromBlock::Number(n.encode())), + }, + to_block: request.to.map(|h| h.encode()).unwrap_or_default(), + direction: request.direction as i32, + max_blocks: request.max.unwrap_or(0), + support_multiple_justifications: true, + })) + } + + fn encode_block_request(&self, request: &OpaqueBlockRequest) -> Result, String> { + let request: &schema::v1::BlockRequest = request.0.downcast_ref().ok_or_else(|| { + "Failed to downcast opaque block response during encoding, this is an \ + implementation bug." + .to_string() + })?; + + Ok(request.encode_to_vec()) + } + + fn decode_block_response(&self, response: &[u8]) -> Result { + let response = schema::v1::BlockResponse::decode(response) + .map_err(|error| format!("Failed to decode block response: {error}"))?; + + Ok(OpaqueBlockResponse(Box::new(response))) + } + + fn block_response_into_blocks( + &self, + request: &BlockRequest, + response: OpaqueBlockResponse, + ) -> Result>, String> { + let response: Box = response.0.downcast().map_err(|_error| { + "Failed to downcast opaque block response during encoding, this is an \ + implementation bug." + .to_string() + })?; + + response + .blocks + .into_iter() + .map(|block_data| { + Ok(BlockData:: { + hash: Decode::decode(&mut block_data.hash.as_ref())?, + header: if !block_data.header.is_empty() { + Some(Decode::decode(&mut block_data.header.as_ref())?) + } else { + None + }, + body: if request.fields.contains(BlockAttributes::BODY) { + Some( + block_data + .body + .iter() + .map(|body| Decode::decode(&mut body.as_ref())) + .collect::, _>>()?, + ) + } else { + None + }, + indexed_body: if request.fields.contains(BlockAttributes::INDEXED_BODY) { + Some(block_data.indexed_body) + } else { + None + }, + receipt: if !block_data.receipt.is_empty() { + Some(block_data.receipt) + } else { + None + }, + message_queue: if !block_data.message_queue.is_empty() { + Some(block_data.message_queue) + } else { + None + }, + justification: if !block_data.justification.is_empty() { + Some(block_data.justification) + } else if block_data.is_empty_justification { + Some(Vec::new()) + } else { + None + }, + justifications: if !block_data.justifications.is_empty() { + Some(DecodeAll::decode_all(&mut block_data.justifications.as_ref())?) + } else { + None + }, + }) + }) + .collect::>() + .map_err(|error: codec::Error| error.to_string()) + } + fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result, String> { let request: &StateRequest = request.0.downcast_ref().ok_or_else(|| { "Failed to downcast opaque state response during encoding, this is an \ diff --git a/client/network/sync/src/schema.rs b/client/network/sync/src/schema.rs index aa3eb84621d8f..b31005360d023 100644 --- a/client/network/sync/src/schema.rs +++ b/client/network/sync/src/schema.rs @@ -18,6 +18,6 @@ //! Include sources generated from protobuf definitions. -pub mod v1 { +pub(crate) mod v1 { include!(concat!(env!("OUT_DIR"), "/api.v1.rs")); } From dc4a7214b04b35278f92c81e8c03fbbc29402a23 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 12 May 2022 06:54:40 +0300 Subject: [PATCH 08/15] Surface `sc-network-sync` into `sc-service` and make `sc-network` not depend on it anymore --- Cargo.lock | 2 ++ client/network/Cargo.toml | 2 +- client/network/src/config.rs | 12 +++++++++--- client/network/src/lib.rs | 1 - client/network/src/service.rs | 10 +++------- client/network/src/service/tests.rs | 24 ++++++++++++++++++------ client/network/test/Cargo.toml | 1 + client/network/test/src/lib.rs | 27 +++++++++++++++++++++------ client/service/Cargo.toml | 3 ++- client/service/src/builder.rs | 25 ++++++++++++++++++++----- 10 files changed, 77 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc5122e139513..f8648ef5bceed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8639,6 +8639,7 @@ dependencies = [ "sc-network", "sc-network-common", "sc-network-light", + "sc-network-sync", "sc-service", "sp-blockchain", "sp-consensus", @@ -8822,6 +8823,7 @@ dependencies = [ "sc-network", "sc-network-common", "sc-network-light", + "sc-network-sync", "sc-offchain", "sc-rpc", "sc-rpc-server", diff --git a/client/network/Cargo.toml b/client/network/Cargo.toml index ba250475dfc92..7cb7da6ce047d 100644 --- a/client/network/Cargo.toml +++ b/client/network/Cargo.toml @@ -51,7 +51,6 @@ sc-block-builder = { version = "0.10.0-dev", path = "../block-builder" } sc-client-api = { version = "4.0.0-dev", path = "../api" } sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } sc-network-common = { version = "0.10.0-dev", path = "./common" } -sc-network-sync = { version = "0.10.0-dev", path = "./sync" } sc-peerset = { version = "4.0.0-dev", path = "../peerset" } sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-arithmetic = { version = "5.0.0", path = "../../primitives/arithmetic" } @@ -68,6 +67,7 @@ quickcheck = "1.0.3" rand = "0.7.2" tempfile = "3.1.0" sc-network-light = { version = "0.10.0-dev", path = "./light" } +sc-network-sync = { version = "0.10.0-dev", path = "./sync" } sp-test-primitives = { version = "2.0.0", path = "../../primitives/test-primitives" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } substrate-test-runtime = { version = "2.0.0", path = "../../test-utils/runtime" } diff --git a/client/network/src/config.rs b/client/network/src/config.rs index b933313f8a78b..da7173aab57b9 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -41,7 +41,7 @@ use libp2p::{ }; use prometheus_endpoint::Registry; use sc_consensus::ImportQueue; -use sp_consensus::block_validation::BlockAnnounceValidator; +use sc_network_common::sync::ChainSync; use sp_runtime::traits::Block as BlockT; use std::{ borrow::Cow, @@ -96,8 +96,14 @@ where /// valid. pub import_queue: Box>, - /// Type to check incoming block announcements. - pub block_announce_validator: Box + Send>, + /// Factory function that creates a new instance of chain sync. + pub create_chain_sync: Box< + dyn FnOnce( + sc_network_common::sync::SyncMode, + Arc, + Option>>, + ) -> crate::error::Result>>, + >, /// Registry for recording prometheus metrics to. pub metrics_registry: Option, diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 331f4790e9a6f..83bc1075b8bad 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -270,7 +270,6 @@ pub use sc_network_common::sync::{ warp::{WarpSyncPhase, WarpSyncProgress}, StateDownloadProgress, SyncState, }; -pub use sc_network_sync::{block_request_handler, state_request_handler, warp_request_handler}; pub use service::{ DecodingError, IfDisconnected, KademliaKey, Keypair, NetworkService, NetworkWorker, NotificationSender, NotificationSenderReady, OutboundFailure, PublicKey, RequestFailure, diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 46c0fecee39ff..2f2d1dab58fc7 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -61,7 +61,6 @@ use parking_lot::Mutex; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; use sc_network_common::sync::{SyncMode, SyncState, SyncStatus}; -use sc_network_sync::ChainSync; use sc_peerset::PeersetHandle; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; use sp_blockchain::{HeaderBackend, HeaderMetadata}; @@ -208,7 +207,7 @@ where None => (None, None), }; - let chain_sync = ChainSync::new( + let chain_sync = (params.create_chain_sync)( if params.role.is_light() { SyncMode::Light } else { @@ -220,11 +219,8 @@ where } }, params.chain.clone(), - params.block_announce_validator, - params.network_config.max_parallel_downloads, warp_sync_provider, - ) - .map_err(Box::new)?; + )?; let (protocol, peerset_handle, mut known_addresses) = Protocol::new( From::from(¶ms.role), params.chain.clone(), @@ -237,7 +233,7 @@ where ) .collect(), params.metrics_registry.as_ref(), - Box::new(chain_sync), + chain_sync, )?; // List of multiaddresses that we know in the network. diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 808546d67fc7c..181d58130aa6b 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -16,15 +16,17 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{ - config, state_request_handler::StateRequestHandler, Event, NetworkService, NetworkWorker, -}; +use crate::{config, Event, NetworkService, NetworkWorker}; use futures::prelude::*; use libp2p::PeerId; use sc_network_common::config::ProtocolId; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; -use sc_network_sync::block_request_handler::BlockRequestHandler; +use sc_network_sync::{ + block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, + ChainSync, +}; +use sp_consensus::block_validation::DefaultBlockAnnounceValidator; use sp_runtime::traits::{Block as BlockT, Header as _}; use std::{borrow::Cow, sync::Arc, time::Duration}; use substrate_test_runtime_client::{TestClientBuilder, TestClientBuilderExt as _}; @@ -109,6 +111,7 @@ fn build_test_full_node( protocol_config }; + let max_parallel_downloads = config.max_parallel_downloads; let worker = NetworkWorker::new(config::Params { role: config::Role::Full, executor: None, @@ -120,8 +123,17 @@ fn build_test_full_node( transaction_pool: Arc::new(crate::config::EmptyTransactionPool), protocol_id, import_queue, - block_announce_validator: Box::new( - sp_consensus::block_validation::DefaultBlockAnnounceValidator, + create_chain_sync: Box::new( + move |sync_mode, chain, warp_sync_provider| match ChainSync::new( + sync_mode, + chain, + Box::new(DefaultBlockAnnounceValidator), + max_parallel_downloads, + warp_sync_provider, + ) { + Ok(chain_sync) => Ok(Box::new(chain_sync)), + Err(error) => Err(Box::new(error).into()), + }, ), metrics_registry: None, block_request_protocol_config, diff --git a/client/network/test/Cargo.toml b/client/network/test/Cargo.toml index 78ea2454b90e0..98323989fb98b 100644 --- a/client/network/test/Cargo.toml +++ b/client/network/test/Cargo.toml @@ -27,6 +27,7 @@ sc-consensus = { version = "0.10.0-dev", path = "../../consensus/common" } sc-network = { version = "0.10.0-dev", path = "../" } sc-network-common = { version = "0.10.0-dev", path = "../common" } sc-network-light = { version = "0.10.0-dev", path = "../light" } +sc-network-sync = { version = "0.10.0-dev", path = "../sync" } sc-service = { version = "0.10.0-dev", default-features = false, features = ["test-helpers"], path = "../../service" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index e6cc282c95b38..8b0701d03c5cb 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -48,17 +48,19 @@ use sc_consensus::{ }; pub use sc_network::config::EmptyTransactionPool; use sc_network::{ - block_request_handler::BlockRequestHandler, config::{ MultiaddrWithPeerId, NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, Role, SyncMode, TransportConfig, }, - state_request_handler::StateRequestHandler, - warp_request_handler, Multiaddr, NetworkService, NetworkWorker, + Multiaddr, NetworkService, NetworkWorker, }; pub use sc_network_common::config::ProtocolId; use sc_network_common::warp_sync; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; +use sc_network_sync::{ + block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, + warp_request_handler, ChainSync, +}; use sc_service::client::Client; use sp_blockchain::{ well_known_cache_keys::{self, Id as CacheKeyId}, @@ -829,6 +831,10 @@ where protocol_config }; + let max_parallel_downloads = network_config.max_parallel_downloads; + let block_announce_validator = config + .block_announce_validator + .unwrap_or_else(|| Box::new(DefaultBlockAnnounceValidator)); let network = NetworkWorker::new(sc_network::config::Params { role: if config.is_authority { Role::Authority } else { Role::Full }, executor: None, @@ -840,9 +846,18 @@ where transaction_pool: Arc::new(EmptyTransactionPool), protocol_id, import_queue, - block_announce_validator: config - .block_announce_validator - .unwrap_or_else(|| Box::new(DefaultBlockAnnounceValidator)), + create_chain_sync: Box::new(move |sync_mode, chain, warp_sync_provider| { + match ChainSync::new( + sync_mode, + chain, + block_announce_validator, + max_parallel_downloads, + warp_sync_provider, + ) { + Ok(chain_sync) => Ok(Box::new(chain_sync)), + Err(error) => Err(Box::new(error).into()), + } + }), metrics_registry: None, block_request_protocol_config, state_request_protocol_config, diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index fa578485816ab..64bbc3a0e0849 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -50,9 +50,10 @@ sp-consensus = { version = "0.10.0-dev", path = "../../primitives/consensus/comm sc-consensus = { version = "0.10.0-dev", path = "../../client/consensus/common" } sp-inherents = { version = "4.0.0-dev", path = "../../primitives/inherents" } sp-storage = { version = "6.0.0", path = "../../primitives/storage" } +sc-network = { version = "0.10.0-dev", path = "../network" } sc-network-common = { version = "0.10.0-dev", path = "../network/common" } sc-network-light = { version = "0.10.0-dev", path = "../network/light" } -sc-network = { version = "0.10.0-dev", path = "../network" } +sc-network-sync = { version = "0.10.0-dev", path = "../network/sync" } sc-chain-spec = { version = "4.0.0-dev", path = "../chain-spec" } sc-client-api = { version = "4.0.0-dev", path = "../api" } sp-api = { version = "4.0.0-dev", path = "../../primitives/api" } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index f79fd1b846657..034ac7f288249 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -38,14 +38,17 @@ use sc_consensus::import_queue::ImportQueue; use sc_executor::RuntimeVersionOf; use sc_keystore::LocalKeystore; use sc_network::{ - block_request_handler::{self, BlockRequestHandler}, config::{Role, SyncMode}, - state_request_handler::{self, StateRequestHandler}, - warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler}, NetworkService, }; use sc_network_common::warp_sync::WarpSyncProvider; use sc_network_light::light_client_requests::{self, handler::LightClientRequestHandler}; +use sc_network_sync::{ + block_request_handler::{self, BlockRequestHandler}, + state_request_handler::{self, StateRequestHandler}, + warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler}, + ChainSync, +}; use sc_rpc::{ author::AuthorApiServer, chain::ChainApiServer, @@ -802,6 +805,7 @@ where } }; + let max_parallel_downloads = config.network.max_parallel_downloads; let network_params = sc_network::config::Params { role: config.role.clone(), executor: { @@ -819,9 +823,20 @@ where network_config: config.network.clone(), chain: client.clone(), transaction_pool: transaction_pool_adapter as _, - import_queue: Box::new(import_queue), protocol_id, - block_announce_validator, + import_queue: Box::new(import_queue), + create_chain_sync: Box::new( + move |sync_mode, chain, warp_sync_provider| match ChainSync::new( + sync_mode, + chain, + block_announce_validator, + max_parallel_downloads, + warp_sync_provider, + ) { + Ok(chain_sync) => Ok(Box::new(chain_sync)), + Err(error) => Err(Box::new(error).into()), + }, + ), metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()), block_request_protocol_config, state_request_protocol_config, From 38abfd014a335e45c1cbc834f2a6a85f9afd3a90 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 12 May 2022 07:01:40 +0300 Subject: [PATCH 09/15] Remove now unnecessary dependency from `sc-network` --- Cargo.lock | 1 - client/network/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8648ef5bceed..0f1262b06c592 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8521,7 +8521,6 @@ dependencies = [ "sp-blockchain", "sp-consensus", "sp-core", - "sp-finality-grandpa", "sp-runtime", "sp-test-primitives", "sp-tracing", diff --git a/client/network/Cargo.toml b/client/network/Cargo.toml index 7cb7da6ce047d..4b572c136a1b4 100644 --- a/client/network/Cargo.toml +++ b/client/network/Cargo.toml @@ -57,7 +57,6 @@ sp-arithmetic = { version = "5.0.0", path = "../../primitives/arithmetic" } sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../primitives/consensus/common" } sp-core = { version = "6.0.0", path = "../../primitives/core" } -sp-finality-grandpa = { version = "4.0.0-dev", path = "../../primitives/finality-grandpa" } sp-runtime = { version = "6.0.0", path = "../../primitives/runtime" } [dev-dependencies] From 0583c146f103045ab311d37b48a2bcdfd707d35f Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Sat, 14 May 2022 11:24:27 +0300 Subject: [PATCH 10/15] Replace crate links with just text since dependencies are gone now --- client/network/src/config.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/client/network/src/config.rs b/client/network/src/config.rs index da7173aab57b9..6ef9fc4265a7e 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -115,26 +115,26 @@ where /// block requests, if enabled. /// /// Can be constructed either via - /// [`sc_network_sync::block_request_handler::generate_protocol_config`] allowing outgoing but - /// not incoming requests, or constructed via [`sc_network_sync::block_request_handler:: - /// BlockRequestHandler::new`] allowing both outgoing and incoming requests. + /// `sc_network_sync::block_request_handler::generate_protocol_config` allowing outgoing but + /// not incoming requests, or constructed via `sc_network_sync::block_request_handler:: + /// BlockRequestHandler::new` allowing both outgoing and incoming requests. pub block_request_protocol_config: RequestResponseConfig, /// Request response configuration for the light client request protocol. /// /// Can be constructed either via - /// [`sc_network_light::light_client_requests::generate_protocol_config`] allowing outgoing but + /// `sc_network_light::light_client_requests::generate_protocol_config` allowing outgoing but /// not incoming requests, or constructed via - /// [`sc_network_light::light_client_requests::handler::LightClientRequestHandler::new`] + /// `sc_network_light::light_client_requests::handler::LightClientRequestHandler::new` /// allowing both outgoing and incoming requests. pub light_client_request_protocol_config: RequestResponseConfig, /// Request response configuration for the state request protocol. /// /// Can be constructed either via - /// [`sc_network_sync::block_request_handler::generate_protocol_config`] allowing outgoing but + /// `sc_network_sync::state_request_handler::generate_protocol_config` allowing outgoing but /// not incoming requests, or constructed via - /// [`crate::state_request_handler::StateRequestHandler::new`] allowing + /// `sc_network_sync::state_request_handler::StateRequestHandler::new` allowing /// both outgoing and incoming requests. pub state_request_protocol_config: RequestResponseConfig, From 26b8ab2578c9ea53cb6f9e2f0d39d9da80017861 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 21 Jun 2022 01:16:59 +0300 Subject: [PATCH 11/15] Remove `warp_sync` re-export from `sc-network-common` --- client/finality-grandpa/src/warp_proof.rs | 2 +- client/network/common/src/lib.rs | 3 --- client/network/src/config.rs | 2 +- client/network/src/protocol.rs | 2 +- client/network/sync/src/warp.rs | 8 +++---- .../network/sync/src/warp_request_handler.rs | 2 +- client/network/test/src/lib.rs | 24 ++++++++++--------- client/service/src/builder.rs | 2 +- 8 files changed, 21 insertions(+), 24 deletions(-) diff --git a/client/finality-grandpa/src/warp_proof.rs b/client/finality-grandpa/src/warp_proof.rs index b2088710284ab..a31a0a8b91908 100644 --- a/client/finality-grandpa/src/warp_proof.rs +++ b/client/finality-grandpa/src/warp_proof.rs @@ -23,7 +23,7 @@ use crate::{ BlockNumberOps, GrandpaJustification, SharedAuthoritySet, }; use sc_client_api::Backend as ClientBackend; -use sc_network_common::warp_sync::{EncodedProof, VerificationResult, WarpSyncProvider}; +use sc_network_common::sync::warp::{EncodedProof, VerificationResult, WarpSyncProvider}; use sp_blockchain::{Backend as BlockchainBackend, HeaderBackend}; use sp_finality_grandpa::{AuthorityList, SetId, GRANDPA_ENGINE_ID}; use sp_runtime::{ diff --git a/client/network/common/src/lib.rs b/client/network/common/src/lib.rs index 70260584a6c3b..9fbedc542c123 100644 --- a/client/network/common/src/lib.rs +++ b/client/network/common/src/lib.rs @@ -22,6 +22,3 @@ pub mod config; pub mod message; pub mod request_responses; pub mod sync; - -// TODO: Remove re-export -pub use sync::warp as warp_sync; diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 6ef9fc4265a7e..430efd697a18c 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -26,7 +26,7 @@ pub use sc_network_common::{ request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, - warp_sync::WarpSyncProvider, + sync::warp::WarpSyncProvider, }; pub use libp2p::{build_multiaddr, core::PublicKey, identity}; diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index ac91478e8ab15..fcbe45efe0324 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -52,11 +52,11 @@ use sc_network_common::{ message::{ BlockAnnounce, BlockAttributes, BlockData, BlockRequest, BlockResponse, BlockState, }, + warp::{EncodedProof, WarpProofRequest}, BadPeer, ChainSync, OnBlockData, OnBlockJustification, OnStateData, OpaqueBlockRequest, OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PollBlockAnnounceValidation, SyncStatus, }, - warp_sync::{EncodedProof, WarpProofRequest}, }; use sp_arithmetic::traits::SaturatedConversion; use sp_blockchain::HeaderMetadata; diff --git a/client/network/sync/src/warp.rs b/client/network/sync/src/warp.rs index dc650eb422cb2..f3fad6c1b7fdb 100644 --- a/client/network/sync/src/warp.rs +++ b/client/network/sync/src/warp.rs @@ -23,11 +23,9 @@ use crate::{ state::{ImportResult, StateSync}, }; use sc_client_api::ProofProvider; -use sc_network_common::{ - sync::warp::WarpSyncPhase, - warp_sync::{ - EncodedProof, VerificationResult, WarpProofRequest, WarpSyncProgress, WarpSyncProvider, - }, +use sc_network_common::sync::warp::{ + EncodedProof, VerificationResult, WarpProofRequest, WarpSyncPhase, WarpSyncProgress, + WarpSyncProvider, }; use sp_blockchain::HeaderBackend; use sp_finality_grandpa::{AuthorityList, SetId}; diff --git a/client/network/sync/src/warp_request_handler.rs b/client/network/sync/src/warp_request_handler.rs index 5390c0b6cb944..7dc1b844f7f85 100644 --- a/client/network/sync/src/warp_request_handler.rs +++ b/client/network/sync/src/warp_request_handler.rs @@ -27,7 +27,7 @@ use sc_network_common::{ request_responses::{ IncomingRequest, OutgoingResponse, ProtocolConfig as RequestResponseConfig, }, - warp_sync::{EncodedProof, WarpProofRequest, WarpSyncProvider}, + sync::warp::{EncodedProof, WarpProofRequest, WarpSyncProvider}, }; use sp_runtime::traits::Block as BlockT; use std::{sync::Arc, time::Duration}; diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 35d954a2c7808..d7c83810d521d 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -55,7 +55,9 @@ use sc_network::{ Multiaddr, NetworkService, NetworkWorker, }; pub use sc_network_common::config::ProtocolId; -use sc_network_common::warp_sync; +use sc_network_common::sync::warp::{ + AuthorityList, EncodedProof, SetId, VerificationResult, WarpSyncProvider, +}; use sc_network_light::light_client_requests::handler::LightClientRequestHandler; use sc_network_sync::{ block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler, @@ -641,26 +643,26 @@ impl VerifierAdapter { struct TestWarpSyncProvider(Arc>); -impl warp_sync::WarpSyncProvider for TestWarpSyncProvider { +impl WarpSyncProvider for TestWarpSyncProvider { fn generate( &self, _start: B::Hash, - ) -> Result> { + ) -> Result> { let info = self.0.info(); let best_header = self.0.header(BlockId::hash(info.best_hash)).unwrap().unwrap(); - Ok(warp_sync::EncodedProof(best_header.encode())) + Ok(EncodedProof(best_header.encode())) } fn verify( &self, - proof: &warp_sync::EncodedProof, - _set_id: warp_sync::SetId, - _authorities: warp_sync::AuthorityList, - ) -> Result, Box> { - let warp_sync::EncodedProof(encoded) = proof; + proof: &EncodedProof, + _set_id: SetId, + _authorities: AuthorityList, + ) -> Result, Box> { + let EncodedProof(encoded) = proof; let header = B::Header::decode(&mut encoded.as_slice()).unwrap(); - Ok(warp_sync::VerificationResult::Complete(0, Default::default(), header)) + Ok(VerificationResult::Complete(0, Default::default(), header)) } - fn current_authorities(&self) -> warp_sync::AuthorityList { + fn current_authorities(&self) -> AuthorityList { Default::default() } } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 034ac7f288249..fe0ae5db53e70 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -41,7 +41,7 @@ use sc_network::{ config::{Role, SyncMode}, NetworkService, }; -use sc_network_common::warp_sync::WarpSyncProvider; +use sc_network_common::sync::warp::WarpSyncProvider; use sc_network_light::light_client_requests::{self, handler::LightClientRequestHandler}; use sc_network_sync::{ block_request_handler::{self, BlockRequestHandler}, From 4616d25fec98b203c1381ff0308117764c162113 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 21 Jun 2022 01:18:02 +0300 Subject: [PATCH 12/15] Update copyright in network-related files --- client/network/common/src/sync/warp.rs | 2 +- client/network/src/bitswap.rs | 2 +- client/network/sync/src/warp_request_handler.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/network/common/src/sync/warp.rs b/client/network/common/src/sync/warp.rs index 335a9a978e351..339a4c33a7eeb 100644 --- a/client/network/common/src/sync/warp.rs +++ b/client/network/common/src/sync/warp.rs @@ -1,4 +1,4 @@ -// Copyright 2021 Parity Technologies (UK) Ltd. +// Copyright 2022 Parity Technologies (UK) Ltd. // This file is part of Substrate. // Substrate is free software: you can redistribute it and/or modify diff --git a/client/network/src/bitswap.rs b/client/network/src/bitswap.rs index 14c730e75981a..2dab45adc5618 100644 --- a/client/network/src/bitswap.rs +++ b/client/network/src/bitswap.rs @@ -1,4 +1,4 @@ -// Copyright 2021 Parity Technologies (UK) Ltd. +// Copyright 2022 Parity Technologies (UK) Ltd. // This file is part of Substrate. // Substrate is free software: you can redistribute it and/or modify diff --git a/client/network/sync/src/warp_request_handler.rs b/client/network/sync/src/warp_request_handler.rs index 7dc1b844f7f85..53ec216a1e668 100644 --- a/client/network/sync/src/warp_request_handler.rs +++ b/client/network/sync/src/warp_request_handler.rs @@ -1,4 +1,4 @@ -// Copyright 2021 Parity Technologies (UK) Ltd. +// Copyright 2022 Parity Technologies (UK) Ltd. // This file is part of Substrate. // Substrate is free software: you can redistribute it and/or modify From 2e2e6eeb21f4cea6433d76781f0cd7e5fde82913 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 21 Jun 2022 01:21:57 +0300 Subject: [PATCH 13/15] Address review comments about documentation --- client/network/common/src/sync.rs | 12 +----------- client/network/sync/src/lib.rs | 1 + 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index 90e23e148818e..7bbe4256b40f7 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -219,6 +219,7 @@ impl fmt::Debug for OpaqueBlockResponse { } } +/// Something that represents the syncing strategy to download past and future blocks of the chain. pub trait ChainSync: Send { /// Returns the state of the sync of the given peer. /// @@ -249,7 +250,6 @@ pub trait ChainSync: Send { ) -> Result>, BadPeer>; /// Signal that a new best block has been imported. - /// `ChainSync` state with that information. fn update_chain_info(&mut self, best_hash: &Block::Hash, best_number: NumberFor); /// Schedule a justification request for the given block. @@ -259,7 +259,6 @@ pub trait ChainSync: Send { fn clear_justification_requests(&mut self); /// Request syncing for the given block from given set of peers. - // The implementation is similar to on_block_announce with unknown parent hash. fn set_sync_fork_request( &mut self, peers: Vec, @@ -296,8 +295,6 @@ pub trait ChainSync: Send { ) -> Result, BadPeer>; /// Handle a response from the remote to a state request that we made. - /// - /// Returns next request if any. fn on_state_data( &mut self, who: &PeerId, @@ -305,16 +302,11 @@ pub trait ChainSync: Send { ) -> Result, BadPeer>; /// Handle a response from the remote to a warp proof request that we made. - /// - /// Returns next request. fn on_warp_sync_data(&mut self, who: &PeerId, response: EncodedProof) -> Result<(), BadPeer>; /// Handle a response from the remote to a justification request that we made. /// /// `request` must be the original request that triggered `response`. - /// - /// Returns `Some` if this produces a justification that must be imported - /// into the import queue. fn on_block_justification( &mut self, who: PeerId, @@ -325,8 +317,6 @@ pub trait ChainSync: Send { /// /// Call this when a batch of blocks have been processed by the import /// queue, with or without errors. - /// - /// `peer_info` is passed in case of a restart. fn on_blocks_processed( &mut self, imported: usize, diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index f2a7631af7423..32bbc271423e4 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -577,6 +577,7 @@ where self.extra_justifications.reset(); } + // The implementation is similar to on_block_announce with unknown parent hash. fn set_sync_fork_request( &mut self, mut peers: Vec, From 93849a1b532ccfc4b909568c6228f2cce020a591 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Tue, 21 Jun 2022 01:22:47 +0300 Subject: [PATCH 14/15] Apply review suggestion --- client/network/sync/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 32bbc271423e4..c30e26fc4b95c 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -1542,11 +1542,7 @@ where !target.peers.is_empty() }); let blocks = self.drain_blocks(); - if !blocks.is_empty() { - Some(self.validate_and_queue_blocks(blocks, false)) - } else { - None - } + (!blocks.is_empty()).then(|| self.validate_and_queue_blocks(blocks, false)) } fn metrics(&self) -> Metrics { From 6e1eae42f5419ae8546d6f158596d041a1c985f7 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Fri, 24 Jun 2022 02:02:24 +0300 Subject: [PATCH 15/15] Rename `extra_requests` module to `metrics` --- client/network/common/src/sync.rs | 4 ++-- .../network/common/src/sync/{extra_requests.rs => metrics.rs} | 0 client/network/sync/src/extra_requests.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename client/network/common/src/sync/{extra_requests.rs => metrics.rs} (100%) diff --git a/client/network/common/src/sync.rs b/client/network/common/src/sync.rs index 7bbe4256b40f7..2ee8f8c51814b 100644 --- a/client/network/common/src/sync.rs +++ b/client/network/common/src/sync.rs @@ -18,8 +18,8 @@ //! Abstract interfaces and data structures related to network sync. -pub mod extra_requests; pub mod message; +pub mod metrics; pub mod warp; use libp2p::PeerId; @@ -172,7 +172,7 @@ pub enum SyncMode { pub struct Metrics { pub queued_blocks: u32, pub fork_targets: u32, - pub justifications: extra_requests::Metrics, + pub justifications: metrics::Metrics, } /// Wrapper for implementation-specific state request. diff --git a/client/network/common/src/sync/extra_requests.rs b/client/network/common/src/sync/metrics.rs similarity index 100% rename from client/network/common/src/sync/extra_requests.rs rename to client/network/common/src/sync/metrics.rs diff --git a/client/network/sync/src/extra_requests.rs b/client/network/sync/src/extra_requests.rs index 230e6fb067ff5..6206f8a61bcf4 100644 --- a/client/network/sync/src/extra_requests.rs +++ b/client/network/sync/src/extra_requests.rs @@ -20,7 +20,7 @@ use crate::{PeerSync, PeerSyncState}; use fork_tree::ForkTree; use libp2p::PeerId; use log::{debug, trace, warn}; -use sc_network_common::sync::extra_requests::Metrics; +use sc_network_common::sync::metrics::Metrics; use sp_blockchain::Error as ClientError; use sp_runtime::traits::{Block as BlockT, NumberFor, Zero}; use std::{