From fe6dd131f0f17281dce21b5fabced1bca1ab22a7 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 30 Oct 2018 23:46:27 +0100 Subject: [PATCH] tests compile after changes --- substrate/Cargo.lock | 4 +- substrate/core/finality-grandpa/Cargo.toml | 2 +- .../finality-grandpa/primitives/src/lib.rs | 2 +- substrate/core/finality-grandpa/src/lib.rs | 60 ++++++--- substrate/core/finality-grandpa/src/tests.rs | 114 ++++++++++++------ substrate/core/network/src/import_queue.rs | 32 ++++- substrate/core/network/src/test/mod.rs | 2 +- 7 files changed, 156 insertions(+), 60 deletions(-) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 3b1b27388d..b14022ea3a 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -581,7 +581,6 @@ dependencies = [ [[package]] name = "finality-grandpa" version = "0.3.0" -source = "git+https://github.com/paritytech/finality-grandpa#f1ad8d7ca020e5db0e51e32cd62e8cd3c578d121" dependencies = [ "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3034,7 +3033,7 @@ dependencies = [ name = "substrate-finality-grandpa" version = "0.1.0" dependencies = [ - "finality-grandpa 0.3.0 (git+https://github.com/paritytech/finality-grandpa)", + "finality-grandpa 0.3.0", "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 2.1.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -4202,7 +4201,6 @@ dependencies = [ "checksum failure_derive 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "946d0e98a50d9831f5d589038d2ca7f8f455b1c21028c0db0e84116a12696426" "checksum fake-simd 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed" "checksum fdlimit 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b1ee15a7050e5580b3712877157068ea713b245b080ff302ae2ca973cfcd9baa" -"checksum finality-grandpa 0.3.0 (git+https://github.com/paritytech/finality-grandpa)" = "" "checksum fixed-hash 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0d5ec8112f00ea8a483e04748a85522184418fd1cf02890b626d8fc28683f7de" "checksum fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2fad85553e09a6f881f739c29f0b00b0f01357c743266d478b68951ce23285f3" "checksum foreign-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" diff --git a/substrate/core/finality-grandpa/Cargo.toml b/substrate/core/finality-grandpa/Cargo.toml index 96892ec6f7..fa30e427f8 100644 --- a/substrate/core/finality-grandpa/Cargo.toml +++ b/substrate/core/finality-grandpa/Cargo.toml @@ -18,7 +18,7 @@ substrate-fg-primitives = { path = "primitives" } [dependencies.finality-grandpa] #version = "0.3.0" -git = "https://github.com/paritytech/finality-grandpa" +path = "../../../finality-afg" features = ["derive-codec"] [dev-dependencies] diff --git a/substrate/core/finality-grandpa/primitives/src/lib.rs b/substrate/core/finality-grandpa/primitives/src/lib.rs index ddf924cc2e..649d424e6b 100644 --- a/substrate/core/finality-grandpa/primitives/src/lib.rs +++ b/substrate/core/finality-grandpa/primitives/src/lib.rs @@ -33,7 +33,7 @@ use sr_primitives::traits::{Block as BlockT, DigestFor, NumberFor}; /// A scheduled change of authority set. #[cfg_attr(feature = "std", derive(Debug, PartialEq))] -#[derive(Encode, Decode)] +#[derive(Clone, Encode, Decode)] pub struct ScheduledChange { /// The new authorities after the change, along with their respective weights. pub next_authorities: Vec<(AuthorityId, u64)>, diff --git a/substrate/core/finality-grandpa/src/lib.rs b/substrate/core/finality-grandpa/src/lib.rs index 21d5c054fb..d12b35ea1f 100644 --- a/substrate/core/finality-grandpa/src/lib.rs +++ b/substrate/core/finality-grandpa/src/lib.rs @@ -699,20 +699,58 @@ impl voter::Environment> f } } +/// Client side of the GRANDPA APIs declared in fg-primitives. +pub trait ApiClient { + /// Get the genesis authorities for GRANDPA. + fn genesis_authorities(&self) -> Result, ClientError>; + + /// Check a header's digest for a scheduled change. + fn scheduled_change(&self, header: &Block::Header) + -> Result>>, ClientError>; +} + +impl ApiClient for Arc> where + B: Backend + 'static, + E: CallExecutor + 'static + Clone, + DigestFor: Encode, +{ + fn genesis_authorities(&self) -> Result, ClientError> { + use runtime_primitives::traits::Zero; + + self.call_api_at( + &BlockId::Number(NumberFor::::zero()), + fg_primitives::AUTHORITIES_CALL, + &() + ) + } + + fn scheduled_change(&self, header: &Block::Header) + -> Result>>, ClientError> + { + self.call_api_at( + &BlockId::hash(header.parent_hash().clone()), + ::fg_primitives::PENDING_CHANGE_CALL, + header.digest(), + ) + } +} + /// A block-import handler for GRANDPA. /// /// This scans each imported block for signals of changing authority set. /// When using GRANDPA, the block import worker should be using this block import /// object. -pub struct GrandpaBlockImport { +pub struct GrandpaBlockImport { inner: Arc>, authority_set: SharedAuthoritySet>, + api_client: Api, } -impl BlockImport for GrandpaBlockImport where +impl BlockImport for GrandpaBlockImport where B: Backend + 'static, E: CallExecutor + 'static + Clone, DigestFor: Encode, + Api: ApiClient, { type Error = ClientError; @@ -721,11 +759,7 @@ impl BlockImport for GrandpaBlockImport { use authorities::PendingChange; - let maybe_change: Option>> = self.inner.call_api_at( - &BlockId::hash(block.header.parent_hash().clone()), - ::fg_primitives::PENDING_CHANGE_CALL, - block.header.digest() - )?; + let maybe_change = self.api_client.scheduled_change(&block.header)?; // when we update the authorities, we need to hold the lock // until the block is written to prevent a race if we need to restore @@ -768,14 +802,13 @@ pub struct LinkHalf { /// Make block importer and link half necessary to tie the background voter /// to it. -pub fn block_import(client: Arc>) - -> Result<(GrandpaBlockImport, LinkHalf), ClientError> +pub fn block_import(client: Arc>, api_client: Api) + -> Result<(GrandpaBlockImport, LinkHalf), ClientError> where B: Backend + 'static, E: CallExecutor + 'static, + Api: ApiClient, { - use runtime_primitives::traits::Zero; - let authority_set = match client.backend().get_aux(AUTHORITY_SET_KEY)? { None => { info!(target: "afg", "Loading GRANDPA authorities \ @@ -784,8 +817,7 @@ pub fn block_import(client: Arc>) // no authority set on disk: fetch authorities from genesis state. // if genesis state is not available, we may be a light client, but these // are unsupported for following GRANDPA directly. - let genesis_authorities: Vec<(AuthorityId, u64)> = client - .call_api_at(&BlockId::Number(NumberFor::::zero()), fg_primitives::AUTHORITIES_CALL, &())?; + let genesis_authorities: Vec<(AuthorityId, u64)> = api_client.genesis_authorities()?; let authority_set = SharedAuthoritySet::genesis(genesis_authorities); let encoded = authority_set.inner().read().encode(); @@ -801,7 +833,7 @@ pub fn block_import(client: Arc>) }; Ok(( - GrandpaBlockImport { inner: client.clone(), authority_set: authority_set.clone() }, + GrandpaBlockImport { inner: client.clone(), authority_set: authority_set.clone(), api_client }, LinkHalf { client, authority_set }, )) } diff --git a/substrate/core/finality-grandpa/src/tests.rs b/substrate/core/finality-grandpa/src/tests.rs index 7b3c6a39bf..adc1646413 100644 --- a/substrate/core/finality-grandpa/src/tests.rs +++ b/substrate/core/finality-grandpa/src/tests.rs @@ -6,16 +6,34 @@ use parking_lot::Mutex; use tokio::runtime::current_thread; use keyring::Keyring; use client::BlockchainEvents; -use test_client; +use test_client::{self, runtime::BlockNumber}; type PeerData = Mutex>>; type GrandpaPeer = Peer; struct GrandpaTestNet { peers: Vec>, + test_config: TestApi, started: bool } +impl GrandpaTestNet { + fn new(test_config: TestApi, n_peers: usize) -> Self { + let mut net = GrandpaTestNet { + peers: Vec::with_capacity(n_peers), + started: false, + test_config, + }; + let config = Self::default_config(); + + for _ in 0..n_peers { + net.add_peer(&config); + } + + net + } +} + impl TestNetFactory for GrandpaTestNet { type Verifier = PassThroughVerifier; type PeerData = PeerData; @@ -24,6 +42,7 @@ impl TestNetFactory for GrandpaTestNet { fn from_config(_config: &ProtocolConfig) -> Self { GrandpaTestNet { peers: Vec::new(), + test_config: Default::default(), started: false } } @@ -37,7 +56,7 @@ impl TestNetFactory for GrandpaTestNet { 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."); + let (import, link) = block_import(client, self.test_config.clone()).expect("Could not create block import for fresh peer."); (Arc::new(import), Mutex::new(Some(link))) } @@ -113,43 +132,74 @@ impl Network for MessageRouting { } } +#[derive(Default, Clone)] +struct TestApi { + genesis_authorities: Vec<(AuthorityId, u64)>, + scheduled_changes: HashMap>, +} + +impl TestApi { + fn new(genesis_authorities: Vec<(AuthorityId, u64)>) -> Self { + TestApi { + genesis_authorities, + scheduled_changes: HashMap::new(), + } + } +} + +impl ApiClient for TestApi { + fn genesis_authorities(&self) -> Result, ClientError> { + Ok(self.genesis_authorities.clone()) + } + + fn scheduled_change(&self, header: &::Header) + -> Result>>, ClientError> + { + // we take only scheduled changes at given block number where there are no + // extrinsics. + Ok(self.scheduled_changes.get(header.number()).map(|c| c.clone())) + } +} + const TEST_GOSSIP_DURATION: Duration = Duration::from_millis(500); const TEST_ROUTING_INTERVAL: Duration = Duration::from_millis(50); #[test] -fn finalize_20_unanimous_3_peers() { - let mut net = GrandpaTestNet::new(3); +fn finalize_3_voters_no_observers() { + let peers = &[Keyring::Alice, Keyring::Bob, Keyring::Charlie]; + let voters: Vec<_> = peers.iter() + .map(|key| AuthorityId(key.to_raw_public())) + .map(|id| (id, 1)) + .collect(); + + let mut net = GrandpaTestNet::new(TestApi::new(voters), 3); net.peer(0).push_blocks(20, false); net.sync(); - let net = Arc::new(Mutex::new(net)); - let peers = &[ - (0, Keyring::Alice), - (1, Keyring::Bob), - (2, Keyring::Charlie), - ]; + for i in 0..3 { + assert_eq!(net.peer(i).client().info().unwrap().chain.best_number, 20, + "Peer #{} failed to sync", i); + } - let voters: Vec<_> = peers.iter() - .map(|&(_, ref key)| AuthorityId(key.to_raw_public())) - .collect(); + let net = Arc::new(Mutex::new(net)); let mut finality_notifications = Vec::new(); - let mut runtime = current_thread::Runtime::new().unwrap(); - for (peer_id, key) in peers { + + for (peer_id, key) in peers.iter().enumerate() { 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"); + let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed"); ( - net.peers[*peer_id].client().clone(), + 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(())) + .for_each(|_| Ok(())) ); let voter = run_grandpa( Config { @@ -157,7 +207,7 @@ fn finalize_20_unanimous_3_peers() { local_key: Some(Arc::new(key.clone().into())), }, link, - MessageRouting::new(net.clone(), *peer_id), + MessageRouting::new(net.clone(), peer_id), ).expect("all in order with client and network"); runtime.spawn(voter); @@ -177,31 +227,27 @@ fn finalize_20_unanimous_3_peers() { } #[test] -fn observer_can_finalize() { - let mut net = GrandpaTestNet::new(4); +fn finalize_3_voters_1_observer() { + let peers = &[Keyring::Alice, Keyring::Bob, Keyring::Charlie]; + let voters: Vec<_> = peers.iter() + .map(|key| AuthorityId(key.to_raw_public())) + .map(|id| (id, 1)) + .collect(); + + let mut net = GrandpaTestNet::new(TestApi::new(voters), 4); net.peer(0).push_blocks(20, false); net.sync(); let net = Arc::new(Mutex::new(net)); - let peers = &[ - (0, Keyring::Alice), - (1, Keyring::Bob), - (2, Keyring::Charlie), - ]; - - let voters: HashMap<_, _> = peers.iter() - .map(|&(_, ref key)| (AuthorityId(key.to_raw_public()), 1)) - .collect(); - let mut finality_notifications = Vec::new(); let mut runtime = current_thread::Runtime::new().unwrap(); let all_peers = peers.iter() .cloned() - .map(|(id, key)| (id, Some(Arc::new(key.into())))) - .chain(::std::iter::once((3, None))); + .map(|key| Some(Arc::new(key.into()))) + .chain(::std::iter::once(None)); - for (peer_id, local_key) in all_peers { + for (peer_id, local_key) in all_peers.enumerate() { let (client, link) = { let mut net = net.lock(); let link = net.peers[peer_id].data.lock().take().expect("link initialized at startup; qed"); diff --git a/substrate/core/network/src/import_queue.rs b/substrate/core/network/src/import_queue.rs index 42fcb1b6d6..96496e33ad 100644 --- a/substrate/core/network/src/import_queue.rs +++ b/substrate/core/network/src/import_queue.rs @@ -268,6 +268,11 @@ pub trait Link: Send { fn restart(&self) { } } +/// A link implementation that does nothing. +pub struct NoopLink; + +impl Link for NoopLink { } + /// A link implementation that connects to the network. pub struct NetworkLink> { /// The chain-sync handle @@ -340,9 +345,9 @@ enum BlockImportError { } /// Import a bunch of blocks. -fn import_many_blocks<'a, B: BlockT, L: Link, V: Verifier>( +fn import_many_blocks<'a, B: BlockT, V: Verifier>( import_handle: &BlockImport, - link: &L, + link: &Link, qdata: Option<&AsyncImportQueueData>, blocks: (BlockOrigin, Vec>), verifier: Arc @@ -460,7 +465,7 @@ fn import_single_block>( } /// Process single block import result. -fn process_import_result<'a, B: BlockT>( +fn process_import_result( link: &Link, result: Result::Header as HeaderT>::Number>, BlockImportError> ) -> usize @@ -568,15 +573,30 @@ pub struct SyncImportQueue> { } #[cfg(any(test, feature = "test-helpers"))] -impl> SyncImportQueue { +impl> SyncImportQueue { /// Create a new SyncImportQueue wrapping the given Verifier and block import /// handle. pub fn new(verifier: Arc, block_import: SharedBlockImport) -> Self { - SyncImportQueue { + let queue = SyncImportQueue { verifier, link: ImportCB::new(), block_import, - } + }; + + let v = queue.verifier.clone(); + let import_handle = queue.block_import.clone(); + queue.link.set(Box::new(move |origin, new_blocks| { + let verifier = v.clone(); + import_many_blocks( + &*import_handle, + &NoopLink, + None, + (origin, new_blocks), + verifier, + ) + })); + + queue } } diff --git a/substrate/core/network/src/test/mod.rs b/substrate/core/network/src/test/mod.rs index ac7968ea97..c5addcd4e1 100644 --- a/substrate/core/network/src/test/mod.rs +++ b/substrate/core/network/src/test/mod.rs @@ -258,7 +258,7 @@ impl, D> Peer { body: Some(block.extrinsics), receipt: None, message_queue: None, - justification: None, + justification: Some(Vec::new()), }, }]); }