From dabb6d8b5b4916a69782edce38830e439fd89b66 Mon Sep 17 00:00:00 2001 From: Toralf Wittner Date: Thu, 8 Aug 2019 18:14:14 +0200 Subject: [PATCH] Make tests compile. --- core/client/src/backend.rs | 3 ++- core/client/src/client.rs | 9 +++------ core/consensus/common/src/block_validation.rs | 11 ++++++----- core/finality-grandpa/src/communication/tests.rs | 4 ++-- core/finality-grandpa/src/until_imported.rs | 1 + core/network/src/protocol.rs | 5 ++--- core/network/src/test/mod.rs | 13 ++++++++----- core/network/src/test/sync.rs | 4 ++-- core/service/src/lib.rs | 6 ++++-- 9 files changed, 30 insertions(+), 26 deletions(-) diff --git a/core/client/src/backend.rs b/core/client/src/backend.rs index ad683ebac1e56..9ea725ab38248 100644 --- a/core/client/src/backend.rs +++ b/core/client/src/backend.rs @@ -42,6 +42,7 @@ pub(crate) struct ImportSummary { pub(crate) is_new_best: bool, pub(crate) storage_changes: Option<(StorageCollection, ChildStorageCollection)>, pub(crate) retracted: Vec, + pub(crate) associated_data: Vec, } /// Import operation wrapper @@ -144,7 +145,7 @@ pub trait Finalizer, B: Backend error::Result<()>; - + /// Finalize a block. This will implicitly finalize all blocks up to it and /// fire finality notifications. /// diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 41050e257d826..d314a7e0a75c7 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1002,7 +1002,7 @@ impl Client where is_new_best, storage_changes, retracted, - Vec::new(), // TODO + associated_data: Vec::new(), // TODO }) } @@ -1173,10 +1173,7 @@ impl Client where Ok(()) } - fn notify_imported( - &self, - notify_import: ImportSummary, - ) -> error::Result<()> { + fn notify_imported(&self, notify_import: ImportSummary) -> error::Result<()> { if let Some(storage_changes) = notify_import.storage_changes { // TODO [ToDr] How to handle re-orgs? Should we re-emit all storage changes? self.storage_notifications.lock() @@ -1193,7 +1190,7 @@ impl Client where header: notify_import.header, is_new_best: notify_import.is_new_best, retracted: notify_import.retracted, - associated_data: associated, + associated_data: notify_import.associated_data, }; self.import_notification_sinks.lock() diff --git a/core/consensus/common/src/block_validation.rs b/core/consensus/common/src/block_validation.rs index 0b3bc1feb1a06..416fce337ab63 100644 --- a/core/consensus/common/src/block_validation.rs +++ b/core/consensus/common/src/block_validation.rs @@ -14,8 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . +//! Block announcement validation. + use crate::BlockStatus; -use sr_primitives::{generic::BlockId, traits::{Block, Header}}; +use sr_primitives::{generic::BlockId, traits::Block}; use std::{error::Error, sync::Arc}; /// A type which provides access to chain information. @@ -39,8 +41,6 @@ pub enum Validation { Success, /// Invalid block announcement. Failure, - /// At this moment, validation is inconclusive. - Unknown } /// Type which checks incoming block announcements. @@ -49,6 +49,7 @@ pub trait BlockAnnounceValidator { fn validate(&mut self, header: &B::Header, data: &[u8]) -> Result>; } +/// Default implementation of `BlockAnnounceValidator`. #[derive(Debug)] pub struct DefaultBlockAnnounceValidator { chain: C @@ -61,7 +62,7 @@ impl DefaultBlockAnnounceValidator { } impl> BlockAnnounceValidator for DefaultBlockAnnounceValidator { - fn validate(&mut self, header: &B::Header, data: &[u8]) -> Result> { - Ok(Validation::Success) // TODO: proper validation + fn validate(&mut self, _h: &B::Header, _d: &[u8]) -> Result> { + Ok(Validation::Success) } } diff --git a/core/finality-grandpa/src/communication/tests.rs b/core/finality-grandpa/src/communication/tests.rs index b537c29dcab9c..c4e2a667e0f4d 100644 --- a/core/finality-grandpa/src/communication/tests.rs +++ b/core/finality-grandpa/src/communication/tests.rs @@ -88,8 +88,8 @@ impl super::Network for TestNetwork { } /// Inform peers that a block with given hash should be downloaded. - fn announce(&self, block: Hash) { - let _ = self.sender.unbounded_send(Event::Announce(block)); + fn announce(&self, block: Hash, associated_data: Vec) { + let _ = self.sender.unbounded_send(Event::Announce(block)); // TODO } } diff --git a/core/finality-grandpa/src/until_imported.rs b/core/finality-grandpa/src/until_imported.rs index 4232bfacdfcbd..45c7c064df5ab 100644 --- a/core/finality-grandpa/src/until_imported.rs +++ b/core/finality-grandpa/src/until_imported.rs @@ -473,6 +473,7 @@ mod tests { header, is_new_best: false, retracted: vec![], + associated_data: vec![], }).unwrap(); } } diff --git a/core/network/src/protocol.rs b/core/network/src/protocol.rs index bc5163f04bf09..4d1d51781e1dc 100644 --- a/core/network/src/protocol.rs +++ b/core/network/src/protocol.rs @@ -1073,10 +1073,9 @@ impl, H: ExHashT> Protocol { match self.block_announce_validator.validate(&header, &announce.data) { Ok(Validation::Success) => (), Ok(Validation::Failure) => return CustomMessageOutcome::None, - Ok(Validation::Unknown) => (), // TODO: Is this correct? Err(e) => { - error!(target: "sync", "block validation failed: {}", e); - return CustomMessageOutcome::None // TODO: not sure, `sync` maps this to "unknown block" + warn!(target: "sync", "block validation failed: {}", e); + return CustomMessageOutcome::None } } diff --git a/core/network/src/test/mod.rs b/core/network/src/test/mod.rs index a025bd663a119..0839e4d59479d 100644 --- a/core/network/src/test/mod.rs +++ b/core/network/src/test/mod.rs @@ -35,6 +35,7 @@ use client::error::Result as ClientResult; use client::block_builder::BlockBuilder; use client::backend::{AuxStore, Backend, Finalizer}; use crate::config::Roles; +use consensus::block_validation::DefaultBlockAnnounceValidator; use consensus::import_queue::BasicQueue; use consensus::import_queue::{ BoxBlockImport, BoxJustificationImport, Verifier, BoxFinalityProofImport, @@ -245,8 +246,8 @@ impl> Peer { } /// Announces an important block on the network. - pub fn announce_block(&self, hash: ::Hash) { - self.network.service().announce_block(hash); + pub fn announce_block(&self, hash: ::Hash, data: Vec) { + self.network.service().announce_block(hash, data); } /// Add blocks to the peer -- edit the block before adding @@ -293,11 +294,11 @@ impl> Peer { Default::default() }; self.block_import.import_block(import_block, cache).expect("block_import failed"); - self.network.on_block_imported(hash, header); + self.network.on_block_imported(hash, header, Vec::new()); // TODO at = hash; } - self.network.service().announce_block(at.clone()); + self.network.service().announce_block(at.clone(), Vec::new()); // TODO at } @@ -530,6 +531,7 @@ pub trait TestNetFactory: Sized { protocol_id: ProtocolId::from(&b"test-protocol-name"[..]), import_queue, specialization: self::SpecializationFactory::create(), + block_announce_validator: Box::new(DefaultBlockAnnounceValidator::new(client.clone())) }).unwrap(); self.mut_peers(|peers| { @@ -593,6 +595,7 @@ pub trait TestNetFactory: Sized { protocol_id: ProtocolId::from(&b"test-protocol-name"[..]), import_queue, specialization: self::SpecializationFactory::create(), + block_announce_validator: Box::new(DefaultBlockAnnounceValidator::new(client.clone())) }).unwrap(); self.mut_peers(|peers| { @@ -655,7 +658,7 @@ pub trait TestNetFactory: Sized { // We poll `imported_blocks_stream`. while let Ok(Async::Ready(Some(notification))) = peer.imported_blocks_stream.poll() { - peer.network.on_block_imported(notification.hash, notification.header); + peer.network.on_block_imported(notification.hash, notification.header, notification.associated_data); } // We poll `finality_notification_stream`, but we only take the last event. diff --git a/core/network/src/test/sync.rs b/core/network/src/test/sync.rs index 6cba21c1719bd..1057165738de6 100644 --- a/core/network/src/test/sync.rs +++ b/core/network/src/test/sync.rs @@ -444,7 +444,7 @@ fn can_sync_small_non_best_forks() { assert!(net.peer(0).client().header(&BlockId::Hash(small_hash)).unwrap().is_some()); assert!(!net.peer(1).client().header(&BlockId::Hash(small_hash)).unwrap().is_some()); - net.peer(0).announce_block(small_hash); + net.peer(0).announce_block(small_hash, Vec::new()); // TODO // after announcing, peer 1 downloads the block. @@ -499,7 +499,7 @@ fn light_peer_imports_header_from_announce() { let mut runtime = current_thread::Runtime::new().unwrap(); fn import_with_announce(net: &mut TestNet, runtime: &mut current_thread::Runtime, hash: H256) { - net.peer(0).announce_block(hash); + net.peer(0).announce_block(hash, Vec::new()); // TODO runtime.block_on(futures::future::poll_fn::<(), (), _>(|| { net.poll(); diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 309668aaac6e6..33c5bc2d8f6fa 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -36,7 +36,6 @@ use futures::sync::mpsc; use parking_lot::Mutex; use client::{runtime_api::BlockT, Client}; -use consensus_common::block_validation::DefaultBlockAnnounceValidator; use exit_future::Signal; use futures::prelude::*; use futures03::stream::{StreamExt as _, TryStreamExt as _}; @@ -189,6 +188,9 @@ macro_rules! new_impl { network::config::ProtocolId::from(protocol_id_full) }; + let block_announce_validator = + Box::new(consensus_common::block_validation::DefaultBlockAnnounceValidator::new(client.clone())); + let network_params = network::config::Params { roles: $config.roles, network_config: $config.network.clone(), @@ -200,7 +202,7 @@ macro_rules! new_impl { import_queue, protocol_id, specialization: network_protocol, - block_announce_validator: Box::new(DefaultBlockAnnounceValidator::new(client.clone())) + block_announce_validator, }; let has_bootnodes = !network_params.network_config.boot_nodes.is_empty();