Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Network sync refactoring (part 6) #11940

Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
223940f
Extract `NetworkKVProvider` trait in `sc-authority-discovery` and rem…
nazar-pc Jul 12, 2022
12c6b3e
Extract `NetworkSyncForkRequest` trait in `sc-finality-grandpa`
nazar-pc Jul 12, 2022
21fa1b7
Relax requirements on `SyncOracle` trait, remove extra native methods…
nazar-pc Jul 12, 2022
eff2464
Move `NetworkSigner` trait from `sc-authority-discovery` into `sc-net…
nazar-pc Jul 12, 2022
506b54a
Move `NetworkKVProvider` trait from `sc-authority-discovery` into `sc…
nazar-pc Jul 12, 2022
bb912a4
Minimize `sc-authority-discovery` dependency on `sc-network`
nazar-pc Jul 12, 2022
3db7466
Move `NetworkSyncForkRequest` trait from `sc-finality-grandpa` to `sc…
nazar-pc Jul 12, 2022
2243cbc
Extract `NetworkStatusProvider` trait and de-duplicate methods on `Ne…
nazar-pc Jul 12, 2022
f518b42
Extract `NetworkPeers` trait and de-duplicate methods on `NetworkServ…
nazar-pc Jul 12, 2022
5f58cf9
Extract `NetworkEventStream` trait and de-duplicate methods on `Netwo…
nazar-pc Jul 12, 2022
83a1097
Move more methods from `NetworkService` into `NetworkPeers` trait
nazar-pc Jul 12, 2022
c03ef79
Move `NetworkStateInfo` trait into `sc-network-common`
nazar-pc Jul 12, 2022
89526ee
Extract `NetworkNotification` trait and de-duplicate methods on `Netw…
nazar-pc Jul 12, 2022
6da9f9f
Extract `NetworkRequest` trait and de-duplicate methods on `NetworkSe…
nazar-pc Jul 13, 2022
1f0f450
Remove `NetworkService::local_peer_id()`, it is already provided by `…
nazar-pc Jul 13, 2022
1b7db47
Extract `NetworkTransaction` trait and de-duplicate methods on `Netwo…
nazar-pc Jul 13, 2022
a639e30
Extract `NetworkBlock` trait and de-duplicate methods on `NetworkServ…
nazar-pc Jul 13, 2022
5caf558
Remove dependencies on `NetworkService` from most of the methods of `…
nazar-pc Jul 13, 2022
67e32eb
Merge remote-tracking branch 'upstream/master' into network-sync-refa…
nazar-pc Aug 1, 2022
bea5c93
Address simple review comments
nazar-pc Aug 9, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bin/node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/commo
sc-transaction-pool = { version = "4.0.0-dev", path = "../../../client/transaction-pool" }
sc-transaction-pool-api = { version = "4.0.0-dev", path = "../../../client/transaction-pool/api" }
sc-network = { version = "0.10.0-dev", path = "../../../client/network" }
sc-network-common = { version = "0.10.0-dev", path = "../../../client/network/common" }
sc-consensus-slots = { version = "0.10.0-dev", path = "../../../client/consensus/slots" }
sc-consensus-babe = { version = "0.10.0-dev", path = "../../../client/consensus/babe" }
sc-consensus-uncles = { version = "0.10.0-dev", path = "../../../client/consensus/uncles" }
Expand Down
3 changes: 2 additions & 1 deletion bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ use node_runtime::RuntimeApi;
use sc_client_api::{BlockBackend, ExecutorProvider};
use sc_consensus_babe::{self, SlotProportion};
use sc_executor::NativeElseWasmExecutor;
use sc_network::{Event, NetworkService};
use sc_network::NetworkService;
use sc_network_common::{protocol::event::Event, service::NetworkEventStream};
use sc_service::{config::Configuration, error::Error as ServiceError, RpcHandlers, TaskManager};
use sc_telemetry::{Telemetry, TelemetryWorker};
use sp_api::ProvideRuntimeApi;
Expand Down
2 changes: 1 addition & 1 deletion client/authority-discovery/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ targets = ["x86_64-unknown-linux-gnu"]
prost-build = "0.10"

[dependencies]
async-trait = "0.1"
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false }
futures = "0.3.21"
futures-timer = "3.0.1"
Expand All @@ -29,6 +28,7 @@ rand = "0.7.2"
thiserror = "1.0"
prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.10.0-dev", path = "../../utils/prometheus" }
sc-client-api = { version = "4.0.0-dev", path = "../api" }
sc-network-common = { version = "0.10.0-dev", path = "../network/common" }
sc-network = { version = "0.10.0-dev", path = "../network" }
sp-api = { version = "4.0.0-dev", path = "../../primitives/api" }
sp-authority-discovery = { version = "4.0.0-dev", path = "../../primitives/authority-discovery" }
Expand Down
3 changes: 2 additions & 1 deletion client/authority-discovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ use futures::{
Stream,
};

use libp2p::{Multiaddr, PeerId};
use sc_client_api::blockchain::HeaderBackend;
use sc_network::{DhtEvent, Multiaddr, PeerId};
use sc_network_common::protocol::event::DhtEvent;
use sp_api::ProvideRuntimeApi;
use sp_authority_discovery::{AuthorityDiscoveryApi, AuthorityId};
use sp_runtime::traits::Block as BlockT;
Expand Down
2 changes: 1 addition & 1 deletion client/authority-discovery/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use futures::{
SinkExt,
};

use sc_network::{Multiaddr, PeerId};
use libp2p::{Multiaddr, PeerId};
use sp_authority_discovery::AuthorityId;

/// Service to interact with the [`crate::Worker`].
Expand Down
4 changes: 2 additions & 2 deletions client/authority-discovery/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ fn get_addresses_and_authority_id() {
fn cryptos_are_compatible() {
use sp_core::crypto::Pair;

let libp2p_secret = sc_network::Keypair::generate_ed25519();
let libp2p_secret = libp2p::identity::Keypair::generate_ed25519();
let libp2p_public = libp2p_secret.public();

let sp_core_secret = {
let libp2p_ed_secret = match libp2p_secret.clone() {
sc_network::Keypair::Ed25519(x) => x,
libp2p::identity::Keypair::Ed25519(x) => x,
_ => panic!("generate_ed25519 should have generated an Ed25519 key ¯\\_(ツ)_/¯"),
};
sp_core::ed25519::Pair::from_seed_slice(&libp2p_ed_secret.secret().as_ref()).unwrap()
Expand Down
76 changes: 18 additions & 58 deletions client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,22 @@ use std::{
use futures::{channel::mpsc, future, stream::Fuse, FutureExt, Stream, StreamExt};

use addr_cache::AddrCache;
use async_trait::async_trait;
use codec::Decode;
use ip_network::IpNetwork;
use libp2p::{
core::multiaddr,
multihash::{Multihash, MultihashDigest},
Multiaddr, PeerId,
};
use log::{debug, error, log_enabled};
use prometheus_endpoint::{register, Counter, CounterVec, Gauge, Opts, U64};
use prost::Message;
use rand::{seq::SliceRandom, thread_rng};
use sc_client_api::blockchain::HeaderBackend;
use sc_network::{DhtEvent, ExHashT, Multiaddr, NetworkStateInfo, PeerId};
use sc_network_common::{
protocol::event::DhtEvent,
service::{KademliaKey, NetworkKVProvider, NetworkSigner, NetworkStateInfo, Signature},
};
use sp_api::ProvideRuntimeApi;
use sp_authority_discovery::{
AuthorityDiscoveryApi, AuthorityId, AuthorityPair, AuthoritySignature,
Expand Down Expand Up @@ -136,7 +139,7 @@ pub struct Worker<Client, Network, Block, DhtEventStream> {
/// Queue of throttled lookups pending to be passed to the network.
pending_lookups: Vec<AuthorityId>,
/// Set of in-flight lookups.
in_flight_lookups: HashMap<sc_network::KademliaKey, AuthorityId>,
in_flight_lookups: HashMap<KademliaKey, AuthorityId>,

addr_cache: addr_cache::AddrCache,

Expand Down Expand Up @@ -464,10 +467,7 @@ where
}
}

fn handle_dht_value_found_event(
&mut self,
values: Vec<(sc_network::KademliaKey, Vec<u8>)>,
) -> Result<()> {
fn handle_dht_value_found_event(&mut self, values: Vec<(KademliaKey, Vec<u8>)>) -> Result<()> {
// Ensure `values` is not empty and all its keys equal.
let remote_key = single(values.iter().map(|(key, _)| key.clone()))
.map_err(|_| Error::ReceivingDhtValueFoundEventWithDifferentKeys)?
Expand Down Expand Up @@ -523,11 +523,11 @@ where
// properly signed by the owner of the PeerId

if let Some(peer_signature) = peer_signature {
let public_key =
sc_network::PublicKey::from_protobuf_encoding(&peer_signature.public_key)
.map_err(Error::ParsingLibp2pIdentity)?;
let signature =
sc_network::Signature { public_key, bytes: peer_signature.signature };
let public_key = libp2p::identity::PublicKey::from_protobuf_encoding(
&peer_signature.public_key,
)
.map_err(Error::ParsingLibp2pIdentity)?;
let signature = Signature { public_key, bytes: peer_signature.signature };

if !signature.verify(record, &remote_peer_id) {
return Err(Error::VerifyingDhtPayload)
Expand Down Expand Up @@ -590,55 +590,15 @@ where
}
}

pub trait NetworkSigner {
/// Sign a message in the name of `self.local_peer_id()`
fn sign_with_local_identity(
&self,
msg: impl AsRef<[u8]>,
) -> std::result::Result<sc_network::Signature, sc_network::SigningError>;
}

/// NetworkProvider provides [`Worker`] with all necessary hooks into the
/// underlying Substrate networking. Using this trait abstraction instead of
/// [`sc_network::NetworkService`] directly is necessary to unit test [`Worker`].
#[async_trait]
pub trait NetworkProvider: NetworkStateInfo + NetworkSigner {
/// Start putting a value in the Dht.
fn put_value(&self, key: sc_network::KademliaKey, value: Vec<u8>);

/// Start getting a value from the Dht.
fn get_value(&self, key: &sc_network::KademliaKey);
}
/// `sc_network::NetworkService` directly is necessary to unit test [`Worker`].
pub trait NetworkProvider: NetworkKVProvider + NetworkStateInfo + NetworkSigner {}

impl<B, H> NetworkSigner for sc_network::NetworkService<B, H>
where
B: BlockT + 'static,
H: ExHashT,
{
fn sign_with_local_identity(
&self,
msg: impl AsRef<[u8]>,
) -> std::result::Result<sc_network::Signature, sc_network::SigningError> {
self.sign_with_local_identity(msg)
}
}

#[async_trait::async_trait]
impl<B, H> NetworkProvider for sc_network::NetworkService<B, H>
where
B: BlockT + 'static,
H: ExHashT,
{
fn put_value(&self, key: sc_network::KademliaKey, value: Vec<u8>) {
self.put_value(key, value)
}
fn get_value(&self, key: &sc_network::KademliaKey) {
self.get_value(key)
}
}
impl<T> NetworkProvider for T where T: NetworkKVProvider + NetworkStateInfo + NetworkSigner {}

fn hash_authority_id(id: &[u8]) -> sc_network::KademliaKey {
sc_network::KademliaKey::new(&libp2p::multihash::Code::Sha2_256.digest(id).digest())
fn hash_authority_id(id: &[u8]) -> KademliaKey {
KademliaKey::new(&libp2p::multihash::Code::Sha2_256.digest(id).digest())
}

// Makes sure all values are the same and returns it
Expand Down Expand Up @@ -685,7 +645,7 @@ async fn sign_record_with_authority_ids(
peer_signature: Option<schema::PeerSignature>,
key_store: &dyn CryptoStore,
keys: Vec<CryptoTypePublicPair>,
) -> Result<Vec<(sc_network::KademliaKey, Vec<u8>)>> {
) -> Result<Vec<(KademliaKey, Vec<u8>)>> {
let signatures = key_store
.sign_with_all(key_types::AUTHORITY_DISCOVERY, keys.clone(), &serialized_record)
.await
Expand Down
7 changes: 4 additions & 3 deletions client/authority-discovery/src/worker/addr_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use libp2p::core::multiaddr::{Multiaddr, Protocol};

use sc_network::PeerId;
use libp2p::{
core::multiaddr::{Multiaddr, Protocol},
PeerId,
};
use sp_authority_discovery::AuthorityId;
use std::collections::{hash_map::Entry, HashMap, HashSet};

Expand Down
5 changes: 2 additions & 3 deletions client/authority-discovery/src/worker/schema/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ mod schema_v1 {
}

use super::*;
use libp2p::multiaddr::Multiaddr;
use libp2p::{multiaddr::Multiaddr, PeerId};
use prost::Message;
use sc_network::PeerId;

#[test]
fn v2_decodes_v1() {
Expand Down Expand Up @@ -56,7 +55,7 @@ fn v2_decodes_v1() {

#[test]
fn v1_decodes_v2() {
let peer_secret = sc_network::Keypair::generate_ed25519();
let peer_secret = libp2p::identity::Keypair::generate_ed25519();
let peer_public = peer_secret.public();
let peer_id = peer_public.to_peer_id();
let multiaddress: Multiaddr =
Expand Down
Loading