Fix a bunch of low work dependency cycles (#4354)

* take test-client off sp-consensus

* use test primitives rather than test client in authority discovery tests

* move runtime-interface tests

* don't forget to remove the dev-dependency

* remove more unneeded dev deps

* add changes_trie_config to test prrimitives

* Separate network crates from its integration tests

* Fix up consensus crates for networking test changes

* remove unnecessary dependencies

* remove unused addition

* remove unnecessary dev-dependencies

* fixing finality grandpa tests

* removing unnecessary executor dependencies

substrate/client/api/Cargo.toml

@@ -31,10 +31,4 @@ trie = { package = "sp-trie", path = "../../primitives/trie" }
 txpool-api = { package = "sp-transaction-pool-api", path = "../../primitives/transaction-pool" }
 
 [dev-dependencies]
-env_logger = "0.7.0"
-tempfile = "3.1.0"
-client-db = { package = "sc-client-db", path = "../db", features = ["kvdb-rocksdb"] }
-test-primitives = { package = "sp-test-primitives", path = "../../primitives/test-primitives" }
-test-client = { package = "substrate-test-runtime-client", path = "../../test/utils/runtime/client" }
-kvdb-memorydb = "0.1.2"
-panic-handler = { package = "sp-panic-handler", path = "../../primitives/panic-handler" }
+test-primitives = { package = "sp-test-primitives", path = "../../primitives/test-primitives" }

substrate/client/authority-discovery/Cargo.toml

@@ -30,5 +30,5 @@ sp-runtime = { path = "../../primitives/runtime" }
 env_logger = "0.7.0"
 parking_lot = "0.9.0"
 peerset = { package = "sc-peerset", path = "../peerset" }
-test-client = { package = "substrate-test-runtime-client", path = "../../test/utils/runtime/client" }
+sp-test-primitives = { path = "../../primitives/test-primitives" }
 sp-api = { path = "../../primitives/api" }

substrate/client/authority-discovery/src/lib.rs

@@ -555,7 +555,7 @@ mod tests {
 	use sp_runtime::traits::Zero;
 	use sp_runtime::traits::{ApiRef, Block as BlockT, NumberFor, ProvideRuntimeApi};
 	use std::sync::{Arc, Mutex};
-	use test_client::runtime::Block;
+	use sp_test_primitives::Block;
 
 	#[test]
 	fn interval_at_with_start_now() {

substrate/client/consensus/aura/Cargo.toml

@@ -34,7 +34,8 @@ sc-telemetry = { path = "../../telemetry" }
 [dev-dependencies]
 keyring = { package = "sp-keyring", path = "../../../primitives/keyring" }
 sc-executor = { path = "../../executor" }
-network = { package = "sc-network", path = "../../network", features = ["test-helpers"]}
+sc-network = { path = "../../network" }
+sc-network-test = { path = "../../network/test" }
 service = { package = "sc-service", path = "../../service" }
 test-client = { package = "substrate-test-runtime-client", path = "../../../test/utils/runtime/client" }
 tokio = "0.1.22"

substrate/client/consensus/aura/src/lib.rs

@@ -748,10 +748,9 @@ pub fn import_queue<B, C, P, T>(
 mod tests {
 	use super::*;
 	use consensus_common::NoNetwork as DummyOracle;
-	use network::test::*;
-	use network::test::{Block as TestBlock, PeersClient, PeersFullClient};
+	use sc_network_test::{Block as TestBlock, *};
 	use sp_runtime::traits::{Block as BlockT, DigestFor};
-	use network::config::ProtocolConfig;
+	use sc_network::config::ProtocolConfig;
 	use parking_lot::Mutex;
 	use tokio::runtime::current_thread;
 	use keyring::sr25519::Keyring;

substrate/client/consensus/babe/Cargo.toml

@@ -43,7 +43,8 @@ derive_more = "0.99.2"
 [dev-dependencies]
 keyring = { package = "sp-keyring", path = "../../../primitives/keyring" }
 sc-executor = { path = "../../executor" }
-network = { package = "sc-network", path = "../../network", features = ["test-helpers"]}
+sc-network = { path = "../../network" }
+sc-network-test = { path = "../../network/test" }
 service = { package = "sc-service", path = "../../service" }
 test-client = { package = "substrate-test-runtime-client", path = "../../../test/utils/runtime/client" }
 block-builder = { package = "sc-block-builder", path = "../../block-builder" }

substrate/client/consensus/babe/src/tests.rs

@@ -28,11 +28,10 @@ use consensus_common::NoNetwork as DummyOracle;
 use consensus_common::import_queue::{
 	BoxBlockImport, BoxJustificationImport, BoxFinalityProofImport,
 };
-use network::test::*;
-use network::test::{Block as TestBlock, PeersClient};
-use network::config::BoxFinalityProofRequestBuilder;
+use sc_network_test::*;
+use sc_network_test::{Block as TestBlock, PeersClient};
+use sc_network::config::{BoxFinalityProofRequestBuilder, ProtocolConfig};
 use sp_runtime::{generic::DigestItem, traits::{Block as BlockT, DigestFor}};
-use network::config::ProtocolConfig;
 use tokio::runtime::current_thread;
 use client_api::BlockchainEvents;
 use test_client;

substrate/client/executor/Cargo.toml

@@ -39,8 +39,6 @@ hex-literal = "0.2.1"
 runtime-test = { package = "sc-runtime-test", path = "runtime-test" }
 test-runtime = { package = "substrate-test-runtime", path = "../../test/utils/runtime" }
 runtime-interface = { package = "sp-runtime-interface", path = "../../primitives/runtime-interface" }
-client-api = { package = "sc-client-api", path = "../api" }
-sc-offchain = { path = "../offchain/" }
 state_machine = { package = "sp-state-machine", path = "../../primitives/state-machine"  }
 test-case = "0.3.3"
 

substrate/client/finality-grandpa/Cargo.toml

@@ -31,7 +31,8 @@ grandpa = { package = "finality-grandpa", version = "0.10.1", features = ["deriv
 
 [dev-dependencies]
 grandpa = { package = "finality-grandpa", version = "0.10.1", features = ["derive-codec", "test-helpers"] }
-network = { package = "sc-network", path = "../network", features = ["test-helpers"] }
+network = { package = "sc-network", path = "../network" }
+sc-network-test = { path = "../network/test" }
 keyring = { package = "sp-keyring", path = "../../primitives/keyring" }
 test-client = { package = "substrate-test-runtime-client", path = "../../test/utils/runtime/client"}
 babe_primitives = { package = "sp-consensus-babe", path = "../../primitives/consensus/babe" }

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

@@ -1506,7 +1506,7 @@ mod tests {
 	use super::*;
 	use super::environment::SharedVoterSetState;
 	use network_gossip::Validator as GossipValidatorT;
-	use network::test::Block;
+	use sc_network_test::Block;
 	use primitives::{crypto::Public, H256};
 
 	// some random config (not really needed)

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

@@ -19,7 +19,7 @@
 use futures::sync::mpsc;
 use futures::prelude::*;
 use network::consensus_gossip as network_gossip;
-use network::test::{Block, Hash};
+use sc_network_test::{Block, Hash};
 use network_gossip::Validator;
 use tokio::runtime::current_thread;
 use std::sync::Arc;

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

@@ -18,8 +18,8 @@
 
 use super::*;
 use environment::HasVoted;
-use network::test::{Block, DummySpecialization, Hash, TestNetFactory, Peer, PeersClient};
-use network::test::{PassThroughVerifier};
+use sc_network_test::{Block, DummySpecialization, Hash, TestNetFactory, Peer, PeersClient};
+use sc_network_test::{PassThroughVerifier};
 use network::config::{ProtocolConfig, Roles, BoxFinalityProofRequestBuilder};
 use parking_lot::Mutex;
 use futures03::{StreamExt as _, TryStreamExt as _};
@@ -1553,7 +1553,7 @@ fn voter_catches_up_to_latest_round_when_behind() {
 #[test]
 fn grandpa_environment_respects_voting_rules() {
 	use grandpa::Chain;
-	use network::test::TestClient;
+	use sc_network_test::TestClient;
 
 	let peers = &[Ed25519Keyring::Alice];
 	let voters = make_ids(peers);

substrate/client/network/Cargo.toml

@@ -51,12 +51,11 @@ zeroize = "1.0.0"
 babe-primitives = { package = "sp-consensus-babe", path = "../../primitives/consensus/babe" }
 
 [dev-dependencies]
+sp-test-primitives = { path = "../../primitives/test-primitives" }
 env_logger = "0.7.0"
 keyring = { package = "sp-keyring", path = "../../primitives/keyring" }
 quickcheck = "0.9.0"
 rand = "0.7.2"
-test-client = { package = "substrate-test-runtime-client", path = "../../test/utils/runtime/client" }
-test_runtime = { package = "substrate-test-runtime", path = "../../test/utils/runtime" }
 tempfile = "3.1.0"
 tokio = "0.1.22"
 

substrate/client/network/src/lib.rs

@@ -181,15 +181,12 @@ mod utils;
 pub mod config;
 pub mod error;
 
-#[cfg(any(test, feature = "test-helpers"))]
-pub mod test;
-
 pub use chain::{Client as ClientHandle, FinalityProofProvider};
 pub use service::{
 	NetworkService, NetworkWorker, TransactionPool, ExHashT, ReportHandle,
 	NetworkStateInfo,
 };
-pub use protocol::{PeerInfo, Context, consensus_gossip, message, specialization};
+pub use protocol::{PeerInfo, Context, ProtocolConfig, consensus_gossip, message, specialization};
 pub use protocol::event::{Event, DhtEvent};
 pub use protocol::sync::SyncState;
 pub use libp2p::{Multiaddr, PeerId};

substrate/client/network/src/protocol/legacy_proto/tests.rs

@@ -27,7 +27,7 @@ use rand::seq::SliceRandom;
 use std::{io, time::Duration, time::Instant};
 use crate::message::Message;
 use crate::protocol::legacy_proto::{LegacyProto, LegacyProtoOut};
-use test_client::runtime::Block;
+use sp_test_primitives::Block;
 
 /// Builds two nodes that have each other as bootstrap nodes.
 /// This is to be used only for testing, and a panic will happen if something goes wrong.

substrate/client/network/src/protocol/light_dispatch.rs

@@ -686,7 +686,7 @@ pub mod tests {
 	use crate::message::{self, BlockAttributes, Direction, FromBlock, RequestId};
 	use libp2p::PeerId;
 	use super::{REQUEST_TIMEOUT, LightDispatch, LightDispatchNetwork, RequestData, StorageProof};
-	use test_client::runtime::{changes_trie_config, Block, Extrinsic, Header};
+	use sp_test_primitives::{changes_trie_config, Block, Extrinsic, Header};
 
 	struct DummyFetchChecker { ok: bool }
 

substrate/client/network/src/protocol/sync/extra_requests.rs

@@ -288,7 +288,7 @@ mod tests {
 	use rand::Rng;
 	use std::collections::{HashMap, HashSet};
 	use super::*;
-	use test_client::runtime::{Block, BlockNumber, Hash};
+	use sp_test_primitives::{Block, BlockNumber, Hash};
 
 	#[test]
 	fn requests_are_processed_in_order() {

substrate/client/network/test/Cargo.toml

@@ -0,0 +1,30 @@
+[package]
+description = "Integration tests for Substrate network protocol"
+name = "sc-network-test"
+version = "2.0.0"
+license = "GPL-3.0"
+authors = ["Parity Technologies <admin@parity.io>"]
+edition = "2018"
+
+[dependencies]
+sc-network = { path = "../" }
+log = "0.4.8"
+parking_lot = "0.9.0"
+futures = "0.1.29"
+futures03 = { package = "futures", version = "0.3.1", features = ["compat"] }
+futures-timer = "0.4.0"
+rand = "0.7.2"
+libp2p = { version = "0.13.0", default-features = false, features = ["libp2p-websocket"] }
+consensus = { package = "sp-consensus", path = "../../../primitives/consensus/common" }
+client = { package = "sc-client", path = "../../" }
+client-api = { package = "sc-client-api", path = "../../api" }
+sp-blockchain = { path = "../../../primitives/blockchain" }
+sp-runtime = { path = "../../../primitives/runtime" }
+primitives = { package = "sp-core", path = "../../../primitives/core" }
+block-builder = { package = "sc-block-builder", path = "../../block-builder" }
+babe-primitives = { package = "sp-consensus-babe", path = "../../../primitives/consensus/babe" }
+env_logger = "0.7.0"
+test-client = { package = "substrate-test-runtime-client", path = "../../../test/utils/runtime/client" }
+test_runtime = { package = "substrate-test-runtime", path = "../../../test/utils/runtime" }
+tempfile = "3.1.0"
+tokio = "0.1.22"

substrate/client/network/test/src/lib.rs

@@ -24,9 +24,9 @@ mod sync;
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use crate::config::build_multiaddr;
+use libp2p::build_multiaddr;
 use log::trace;
-use crate::chain::FinalityProofProvider;
+use sc_network::FinalityProofProvider;
 use sp_blockchain::{
 	Result as ClientResult, well_known_cache_keys::{self, Id as CacheKeyId},
 };
@@ -38,7 +38,7 @@ use client_api::{
 };
 use block_builder::BlockBuilder;
 use client::LongestChain;
-use crate::config::Roles;
+use sc_network::config::Roles;
 use consensus::block_validation::DefaultBlockAnnounceValidator;
 use consensus::import_queue::BasicQueue;
 use consensus::import_queue::{
@@ -49,17 +49,17 @@ use consensus::Error as ConsensusError;
 use consensus::{BlockOrigin, ForkChoiceStrategy, BlockImportParams, BlockCheckParams, JustificationImport};
 use futures::prelude::*;
 use futures03::{StreamExt as _, TryStreamExt as _};
-use crate::{NetworkWorker, NetworkService, ReportHandle, config::ProtocolId};
-use crate::config::{NetworkConfiguration, TransportConfig, BoxFinalityProofRequestBuilder};
+use sc_network::{NetworkWorker, NetworkService, ReportHandle, config::ProtocolId};
+use sc_network::config::{NetworkConfiguration, TransportConfig, BoxFinalityProofRequestBuilder};
 use libp2p::PeerId;
 use parking_lot::Mutex;
 use primitives::H256;
-use crate::protocol::{Context, ProtocolConfig};
+use sc_network::{Context, ProtocolConfig};
 use sp_runtime::generic::{BlockId, OpaqueDigestItemId};
 use sp_runtime::traits::{Block as BlockT, Header, NumberFor};
 use sp_runtime::Justification;
-use crate::service::TransactionPool;
-use crate::specialization::NetworkSpecialization;
+use sc_network::TransactionPool;
+use sc_network::specialization::NetworkSpecialization;
 use test_client::{self, AccountKeyring};
 
 pub use test_client::runtime::{Block, Extrinsic, Hash, Transfer};
@@ -67,13 +67,11 @@ pub use test_client::{TestClient, TestClientBuilder, TestClientBuilderExt};
 
 type AuthorityId = babe_primitives::AuthorityId;
 
-#[cfg(any(test, feature = "test-helpers"))]
 /// A Verifier that accepts all blocks and passes them on with the configured
 /// finality to be imported.
 #[derive(Clone)]
 pub struct PassThroughVerifier(pub bool);
 
-#[cfg(any(test, feature = "test-helpers"))]
 /// This `Verifier` accepts all data as valid.
 impl<B: BlockT> Verifier<B> for PassThroughVerifier {
 	fn verify(
@@ -117,7 +115,7 @@ impl NetworkSpecialization<Block> for DummySpecialization {
 		&mut self,
 		_ctx: &mut dyn Context<Block>,
 		_peer_id: PeerId,
-		_status: crate::message::Status<Block>
+		_status: sc_network::message::Status<Block>
 	) {}
 
 	fn on_disconnect(&mut self, _ctx: &mut dyn Context<Block>, _peer_id: PeerId) {}
@@ -401,8 +399,8 @@ impl TransactionPool<Hash, Block> for EmptyTransactionPool {
 		&self,
 		_report_handle: ReportHandle,
 		_who: PeerId,
-		_rep_change_good: crate::ReputationChange,
-		_rep_change_bad: crate::ReputationChange,
+		_rep_change_good: sc_network::ReputationChange,
+		_rep_change_bad: sc_network::ReputationChange,
 		_transaction: Extrinsic
 	) {}
 
@@ -563,7 +561,7 @@ pub trait TestNetFactory: Sized {
 
 		let listen_addr = build_multiaddr![Memory(rand::random::<u64>())];
 
-		let network = NetworkWorker::new(crate::config::Params {
+		let network = NetworkWorker::new(sc_network::config::Params {
 			roles: config.roles,
 			network_config: NetworkConfiguration {
 				listen_addresses: vec![listen_addr.clone()],
@@ -637,7 +635,7 @@ pub trait TestNetFactory: Sized {
 
 		let listen_addr = build_multiaddr![Memory(rand::random::<u64>())];
 
-		let network = NetworkWorker::new(crate::config::Params {
+		let network = NetworkWorker::new(sc_network::config::Params {
 			roles: config.roles,
 			network_config: NetworkConfiguration {
 				listen_addresses: vec![listen_addr.clone()],

substrate/client/network/test/src/sync.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-use crate::config::Roles;
+use sc_network::config::Roles;
 use consensus::BlockOrigin;
 use futures03::TryFutureExt as _;
 use std::time::Duration;