From 6d4acb053dab19df517d7d952a9827af30fa91a9 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 29 Oct 2018 17:03:08 +0100 Subject: [PATCH] tests compile using new test framework --- substrate/Cargo.lock | 1 + substrate/core/finality-grandpa/Cargo.toml | 1 + substrate/core/finality-grandpa/src/lib.rs | 184 +++++++++++++++------ substrate/core/network/src/test/mod.rs | 2 +- 4 files changed, 140 insertions(+), 48 deletions(-) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index c988464c3d..3b1b27388d 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -3047,6 +3047,7 @@ dependencies = [ "substrate-keyring 0.1.0", "substrate-network 0.1.0", "substrate-primitives 0.1.0", + "substrate-test-client 0.1.0", "tokio 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/substrate/core/finality-grandpa/Cargo.toml b/substrate/core/finality-grandpa/Cargo.toml index 2181d2f16d..96892ec6f7 100644 --- a/substrate/core/finality-grandpa/Cargo.toml +++ b/substrate/core/finality-grandpa/Cargo.toml @@ -24,3 +24,4 @@ features = ["derive-codec"] [dev-dependencies] substrate-network = { path = "../network", features = ["test-helpers"] } substrate-keyring = { path = "../keyring" } +substrate-test-client = { path = "../test-client"} diff --git a/substrate/core/finality-grandpa/src/lib.rs b/substrate/core/finality-grandpa/src/lib.rs index 2291c0c823..d8fe874a5e 100644 --- a/substrate/core/finality-grandpa/src/lib.rs +++ b/substrate/core/finality-grandpa/src/lib.rs @@ -38,6 +38,9 @@ extern crate substrate_network as network; #[cfg(test)] extern crate substrate_keyring as keyring; +#[cfg(test)] +extern crate substrate_test_client as test_client; + #[macro_use] extern crate parity_codec_derive; @@ -620,6 +623,7 @@ impl voter::Environment> f return Ok(()); } + // lock must be held through writing to DB to avoid race let mut authority_set = self.authority_set.inner().write(); let client = &self.inner; let status = authority_set.apply_changes(number, |canon_number| { @@ -752,32 +756,23 @@ impl BlockImport for GrandpaBlockImport } } -/// Run a GRANDPA voter as a task. This returns two pieces of data: a task to run, -/// and a `BlockImport` implementation. -pub fn run_grandpa( - config: Config, +/// Half of a link between a block-import worker and a the background voter. +// This should remain non-clone. +pub struct LinkHalf { client: Arc>, - voters: HashMap, - network: N, -) -> ::client::error::Result<( - impl Future, - GrandpaBlockImport, -)> where - Block::Hash: Ord, + authority_set: SharedAuthoritySet>, +} + +/// Make block importer and link half necessary to tie the background voter +/// to it. +pub fn block_import(client: Arc>) + -> Result<(GrandpaBlockImport, LinkHalf), ClientError> + where B: Backend + 'static, E: CallExecutor + 'static, - N: Network + 'static, - N::In: 'static, - NumberFor: BlockNumberOps, - DigestFor: Encode, { - use futures::future::{self, Loop as FutureLoop}; - use runtime_primitives::traits::Zero; - let chain_info = client.info()?; - let genesis_hash = chain_info.chain.genesis_hash; - let authority_set = match client.backend().get_aux(AUTHORITY_SET_KEY)? { None => { info!(target: "afg", "Loading GRANDPA authorities \ @@ -802,10 +797,33 @@ pub fn run_grandpa( .into(), }; - let block_import = GrandpaBlockImport { - inner: client.clone(), - authority_set: authority_set.clone(), - }; + Ok(( + GrandpaBlockImport { inner: client.clone(), authority_set: authority_set.clone() }, + LinkHalf { client, authority_set }, + )) +} + +/// Run a GRANDPA voter as a task. Provide configuration and a link to a +/// block import worker that has already been instantiated with `block_import`. +pub fn run_grandpa( + config: Config, + link: LinkHalf, + network: N, +) -> ::client::error::Result> where + Block::Hash: Ord, + B: Backend + 'static, + E: CallExecutor + 'static, + N: Network + 'static, + N::In: 'static, + NumberFor: BlockNumberOps, + DigestFor: Encode, +{ + use futures::future::{self, Loop as FutureLoop}; + use runtime_primitives::traits::Zero; + + let LinkHalf { client, authority_set } = link; + let chain_info = client.info()?; + let genesis_hash = chain_info.chain.genesis_hash; let (last_round_number, last_state) = match client.backend().get_aux(LAST_COMPLETED_KEY)? { None => (0, RoundState::genesis((genesis_hash, >::zero()))), @@ -815,6 +833,10 @@ pub fn run_grandpa( ))? }; + let voters = authority_set.inner().read().current().1.iter() + .cloned() + .collect(); + let initial_environment = Arc::new(Environment { inner: client.clone(), config: config.clone(), @@ -824,7 +846,7 @@ pub fn run_grandpa( authority_set: authority_set.clone(), }); - let voters = future::loop_fn((initial_environment, last_round_number, last_state), move |params| { + let work = future::loop_fn((initial_environment, last_round_number, last_state), move |params| { let (env, last_round_number, last_state) = params; let chain_info = match client.info() { Ok(i) => i, @@ -866,29 +888,84 @@ pub fn run_grandpa( })) }); - let work = voters.map_err(|e| warn!("GRANDPA Voter failed: {:?}", e)); - - Ok((work, block_import)) + Ok(work.map_err(|e| warn!("GRANDPA Voter failed: {:?}", e))) } #[cfg(test)] mod tests { use super::*; - use network::test::*; + use network::test::{Block, Hash, TestNetFactory, Peer, PeersClient}; + use network::import_queue::{PassThroughVerifier}; + use network::ProtocolConfig; use parking_lot::Mutex; use tokio::runtime::current_thread; use keyring::Keyring; use client::BlockchainEvents; + use test_client; + + type PeerData = Mutex>>; + type GrandpaPeer = Peer; + + struct GrandpaTestNet { + peers: Vec>, + started: bool + } + + impl TestNetFactory for GrandpaTestNet { + type Verifier = PassThroughVerifier; + type PeerData = PeerData; + + /// Create new test network with peers and given config. + fn from_config(_config: &ProtocolConfig) -> Self { + GrandpaTestNet { + peers: Vec::new(), + started: false + } + } + + fn make_verifier(&self, _client: Arc, _cfg: &ProtocolConfig) + -> Arc + { + Arc::new(PassThroughVerifier(false)) // use non-instant finality. + } + + fn make_block_import(&self, client: Arc) + -> (Arc + Send + Sync>, PeerData) + { + let (import, link) = block_import(client).expect("Could not create block import for fresh peer."); + (Arc::new(import), Mutex::new(Some(link))) + } + + fn peer(&self, i: usize) -> &GrandpaPeer { + &self.peers[i] + } + + fn peers(&self) -> &Vec> { + &self.peers + } + + fn mut_peers>)>(&mut self, closure: F) { + closure(&mut self.peers); + } + + fn started(&self) -> bool { + self.started + } + + fn set_started(&mut self, new: bool) { + self.started = new; + } + } #[derive(Clone)] - struct TestGrandpaNetwork { - inner: Arc>, + struct MessageRouting { + inner: Arc>, peer_id: usize, } - impl TestGrandpaNetwork { - fn new(inner: Arc>, peer_id: usize,) -> Self { - TestGrandpaNetwork { + impl MessageRouting { + fn new(inner: Arc>, peer_id: usize,) -> Self { + MessageRouting { inner, peer_id, } @@ -904,7 +981,7 @@ mod tests { hash } - impl Network for TestGrandpaNetwork { + impl Network for MessageRouting { type In = Box,Error=()>>; fn messages_for(&self, round: u64) -> Self::In { @@ -936,7 +1013,7 @@ mod tests { #[test] fn finalize_20_unanimous_3_peers() { - let mut net = TestNet::new(3); + let mut net = GrandpaTestNet::new(3); net.peer(0).push_blocks(20, false); net.sync(); @@ -955,20 +1032,27 @@ mod tests { let mut runtime = current_thread::Runtime::new().unwrap(); for (peer_id, key) in peers { - let client = net.lock().peer(*peer_id).client().clone(); + let (client, link) = { + let mut net = net.lock(); + // temporary needed for some reason + let link = net.peers[*peer_id].data.lock().take().expect("link initialized at startup; qed"); + ( + net.peers[*peer_id].client().clone(), + link, + ) + }; finality_notifications.push( client.finality_notification_stream() .take_while(|n| Ok(n.header.number() < &20)) .for_each(move |_| Ok(())) ); - let (voter, _) = run_grandpa( + let voter = run_grandpa( Config { gossip_duration: TEST_GOSSIP_DURATION, local_key: Some(Arc::new(key.clone().into())), }, - client, - voters.iter().map(|&id| (id, 1)).collect(), - TestGrandpaNetwork::new(net.clone(), *peer_id), + link, + MessageRouting::new(net.clone(), *peer_id), ).expect("all in order with client and network"); runtime.spawn(voter); @@ -989,7 +1073,7 @@ mod tests { #[test] fn observer_can_finalize() { - let mut net = TestNet::new(4); + let mut net = GrandpaTestNet::new(4); net.peer(0).push_blocks(20, false); net.sync(); @@ -1013,20 +1097,26 @@ mod tests { .chain(::std::iter::once((3, None))); for (peer_id, local_key) in all_peers { - let client = net.lock().peer(peer_id).client().clone(); + let (client, link) = { + let mut net = net.lock(); + let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed"); + ( + net.peers[peer_id].client().clone(), + link, + ) + }; finality_notifications.push( client.finality_notification_stream() .take_while(|n| Ok(n.header.number() < &20)) .for_each(move |_| Ok(())) ); - let (voter, _) = run_grandpa( + let voter = run_grandpa( Config { gossip_duration: TEST_GOSSIP_DURATION, local_key, }, - client, - voters.clone(), - TestGrandpaNetwork::new(net.clone(), peer_id), + link, + MessageRouting::new(net.clone(), peer_id), ).expect("all in order with client and network"); runtime.spawn(voter); diff --git a/substrate/core/network/src/test/mod.rs b/substrate/core/network/src/test/mod.rs index 735f6c0c13..ac7968ea97 100644 --- a/substrate/core/network/src/test/mod.rs +++ b/substrate/core/network/src/test/mod.rs @@ -324,7 +324,7 @@ pub trait TestNetFactory: Sized { /// Get reference to peer. fn peer(&self, i: usize) -> &Peer; fn peers(&self) -> &Vec>>; - fn mut_peers>>)>(&mut self, closure: F ); + fn mut_peers>>)>(&mut self, closure: F); fn started(&self) -> bool; fn set_started(&mut self, now: bool);