fix(light-rpc): Make light_sync generic (#10238)

* fix(light-rpc): Make `light_sync` generic

The motivation behind this change is to easily mock `light-sync` to make it possible to enable `rpc-integration` tests
for the light-client.

Currently the `rpc's` requires the concrete type `sync::LightSync` which makes it very hard to do so

* fix(bad merge)

Cargo.lock

@@ -1,3 +1,5 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
 [[package]]
 name = "aho-corasick"
 version = "0.6.8"

ethcore/sync/src/api.rs

@@ -51,6 +51,8 @@ use network::IpFilter;
 use private_tx::PrivateTxHandler;
 use types::transaction::UnverifiedTransaction;
 
+use super::light_sync::SyncInfo;
+
 /// Parity sync protocol
 pub const WARP_SYNC_PROTOCOL_ID: ProtocolId = *b"par";
 /// Ethereum sync protocol
@@ -804,6 +806,24 @@ pub trait LightSyncProvider {
 	fn transactions_stats(&self) -> BTreeMap<H256, TransactionStats>;
 }
 
+/// Wrapper around `light_sync::SyncInfo` to expose those methods without the concrete type `LightSync`
+pub trait LightSyncInfo: Send + Sync {
+	/// Get the highest block advertised on the network.
+	fn highest_block(&self) -> Option<u64>;
+
+	/// Get the block number at the time of sync start.
+	fn start_block(&self) -> u64;
+
+	/// Whether major sync is underway.
+	fn is_major_importing(&self) -> bool;
+}
+
+/// Execute a closure with a protocol context.
+pub trait LightNetworkDispatcher {
+	/// Execute a closure with a protocol context.
+	fn with_context<F, T>(&self, f: F) -> Option<T> where F: FnOnce(&::light::net::BasicContext) -> T;
+}
+
 /// Configuration for the light sync.
 pub struct LightSyncParams<L> {
 	/// Network configuration.
@@ -823,7 +843,7 @@ pub struct LightSyncParams<L> {
 /// Service for light synchronization.
 pub struct LightSync {
 	proto: Arc<LightProtocol>,
-	sync: Arc<::light_sync::SyncInfo + Sync + Send>,
+	sync: Arc<SyncInfo + Sync + Send>,
 	attached_protos: Vec<AttachedProtocol>,
 	network: NetworkService,
 	subprotocol_name: [u8; 3],
@@ -874,15 +894,6 @@ impl LightSync {
 		})
 	}
 
-	/// Execute a closure with a protocol context.
-	pub fn with_context<F, T>(&self, f: F) -> Option<T>
-		where F: FnOnce(&::light::net::BasicContext) -> T
-	{
-		self.network.with_context_eval(
-			self.subprotocol_name,
-			move |ctx| self.proto.with_context(&ctx, f),
-		)
-	}
 }
 
 impl ::std::ops::Deref for LightSync {
@@ -891,6 +902,16 @@ impl ::std::ops::Deref for LightSync {
 	fn deref(&self) -> &Self::Target { &*self.sync }
 }
 
+
+impl LightNetworkDispatcher for LightSync {
+	fn with_context<F, T>(&self, f: F) -> Option<T> where F: FnOnce(&::light::net::BasicContext) -> T {
+		self.network.with_context_eval(
+			self.subprotocol_name,
+			move |ctx| self.proto.with_context(&ctx, f),
+		)
+	}
+}
+
 impl ManageNetwork for LightSync {
 	fn accept_unreserved_peers(&self) {
 		self.network.set_non_reserved_mode(NonReservedPeerMode::Accept);
@@ -991,3 +1012,17 @@ impl LightSyncProvider for LightSync {
 		Default::default() // TODO
 	}
 }
+
+impl LightSyncInfo for LightSync {
+	fn highest_block(&self) -> Option<u64> {
+		(*self.sync).highest_block()
+	}
+
+	fn start_block(&self) -> u64 {
+		(*self.sync).start_block()
+	}
+
+	fn is_major_importing(&self) -> bool {
+		(*self.sync).is_major_importing()
+	}
+}

parity/light_helpers/epoch_fetch.rs

@@ -18,7 +18,7 @@ use std::sync::{Arc, Weak};
 
 use ethcore::engines::{EthEngine, StateDependentProof};
 use ethcore::machine::EthereumMachine;
-use sync::LightSync;
+use sync::{LightSync, LightNetworkDispatcher};
 use types::encoded;
 use types::header::Header;
 use types::receipt::Receipt;

parity/light_helpers/queue_cull.rs

@@ -20,7 +20,7 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use ethcore::client::ClientIoMessage;
-use sync::LightSync;
+use sync::{LightSync, LightNetworkDispatcher};
 use io::{IoContext, IoHandler, TimerToken};
 
 use light::client::LightChainClient;

rpc/src/v1/helpers/dispatch/light.rs

@@ -23,7 +23,7 @@ use light::client::LightChainClient;
 use light::on_demand::{request, OnDemand};
 use parking_lot::{Mutex, RwLock};
 use stats::Corpus;
-use sync::LightSync;
+use sync::{LightSyncProvider, LightNetworkDispatcher, ManageNetwork};
 use types::basic_account::BasicAccount;
 use types::ids::BlockId;
 use types::transaction::{SignedTransaction, PendingTransaction, Error as TransactionError};
@@ -37,10 +37,9 @@ use v1::types::{RichRawTransaction as RpcRichRawTransaction,};
 use super::{Dispatcher, Accounts, SignWith, PostSign};
 
 /// Dispatcher for light clients -- fetches default gas price, next nonce, etc. from network.
-#[derive(Clone)]
+pub struct LightDispatcher<S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static> {
-pub struct LightDispatcher {
 	/// Sync service.
-	pub sync: Arc<LightSync>,
+	pub sync: Arc<S>,
 	/// Header chain client.
 	pub client: Arc<LightChainClient>,
 	/// On-demand request service.
@@ -55,12 +54,15 @@ pub struct LightDispatcher {
 	pub gas_price_percentile: usize,
 }
 
-impl LightDispatcher {
+impl<S> LightDispatcher<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	/// Create a new `LightDispatcher` from its requisite parts.
 	///
 	/// For correct operation, the OnDemand service is assumed to be registered as a network handler,
 	pub fn new(
-		sync: Arc<LightSync>,
+		sync: Arc<S>,
 		client: Arc<LightChainClient>,
 		on_demand: Arc<OnDemand>,
 		cache: Arc<Mutex<LightDataCache>>,
@@ -115,7 +117,27 @@ impl LightDispatcher {
 	}
 }
 
-impl Dispatcher for LightDispatcher {
+impl<S> Clone for LightDispatcher<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
+	fn clone(&self) -> Self {
+		Self {
+			sync: self.sync.clone(),
+			client: self.client.clone(),
+			on_demand: self.on_demand.clone(),
+			cache: self.cache.clone(),
+			transaction_queue: self.transaction_queue.clone(),
+			nonces: self.nonces.clone(),
+			gas_price_percentile: self.gas_price_percentile
+		}
+	}
+}
+
+impl<S> Dispatcher for LightDispatcher<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	// Ignore the `force_nonce` flag in order to always query the network when fetching the nonce and
 	// the account state. If the nonce is specified in the transaction use that nonce instead but do the
 	// network request anyway to the account state (balance)
@@ -217,8 +239,8 @@ impl Dispatcher for LightDispatcher {
 
 /// Get a recent gas price corpus.
 // TODO: this could be `impl Trait`.
-pub fn fetch_gas_price_corpus(
+pub fn fetch_gas_price_corpus<S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static>(
-	sync: Arc<LightSync>,
+	sync: Arc<S>,
 	client: Arc<LightChainClient>,
 	on_demand: Arc<OnDemand>,
 	cache: Arc<Mutex<LightDataCache>>,

rpc/src/v1/helpers/light_fetch.rs

@@ -17,6 +17,7 @@
 //! Helpers for fetching blockchain data either from the light client or the network.
 
 use std::cmp;
+use std::clone::Clone;
 use std::sync::Arc;
 
 use types::basic_account::BasicAccount;
@@ -40,7 +41,9 @@ use light::on_demand::{
 use light::on_demand::error::Error as OnDemandError;
 use light::request::Field;
 
-use sync::LightSync;
+
+use sync::{LightNetworkDispatcher, ManageNetwork, LightSyncProvider};
+
 use ethereum_types::{U256, Address};
 use hash::H256;
 use parking_lot::Mutex;
@@ -52,10 +55,12 @@ use v1::helpers::{CallRequest as CallRequestHelper, errors, dispatch};
 use v1::types::{BlockNumber, CallRequest, Log, Transaction};
 
 const NO_INVALID_BACK_REFS_PROOF: &str = "Fails only on invalid back-references; back-references here known to be valid; qed";
-
 const WRONG_RESPONSE_AMOUNT_TYPE_PROOF: &str = "responses correspond directly with requests in amount and type; qed";
 
-pub fn light_all_transactions(dispatch: &Arc<dispatch::LightDispatcher>) -> impl Iterator<Item=PendingTransaction> {
+pub fn light_all_transactions<S>(dispatch: &Arc<dispatch::LightDispatcher<S>>) -> impl Iterator<Item=PendingTransaction>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	let txq = dispatch.transaction_queue.read();
 	let chain_info = dispatch.client.chain_info();
 
@@ -66,20 +71,36 @@ pub fn light_all_transactions(dispatch: &Arc<dispatch::LightDispatcher>) -> impl
 
 /// Helper for fetching blockchain data either from the light client or the network
 /// as necessary.
-#[derive(Clone)]
+pub struct LightFetch<S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static>
-pub struct LightFetch {
+{
 	/// The light client.
 	pub client: Arc<LightChainClient>,
 	/// The on-demand request service.
 	pub on_demand: Arc<OnDemand>,
 	/// Handle to the network.
-	pub sync: Arc<LightSync>,
+	pub sync: Arc<S>,
 	/// The light data cache.
 	pub cache: Arc<Mutex<Cache>>,
 	/// Gas Price percentile
 	pub gas_price_percentile: usize,
 }
 
+impl<S> Clone for LightFetch<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
+	fn clone(&self) -> Self {
+		Self {
+			client: self.client.clone(),
+			on_demand: self.on_demand.clone(),
+			sync: self.sync.clone(),
+			cache: self.cache.clone(),
+			gas_price_percentile: self.gas_price_percentile
+		}
+	}
+}
+
+
 /// Extract a transaction at given index.
 pub fn extract_transaction_at_index(block: encoded::Block, index: usize) -> Option<Transaction> {
 	block.transactions().into_iter().nth(index)
@@ -115,7 +136,10 @@ fn extract_header(res: &[OnDemandResponse], header: HeaderRef) -> Option<encoded
 	}
 }
 
-impl LightFetch {
+impl<S> LightFetch<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	// push the necessary requests onto the request chain to get the header by the given ID.
 	// yield a header reference which other requests can use.
 	fn make_header_requests(&self, id: BlockId, reqs: &mut Vec<OnDemandRequest>) -> Result<HeaderRef> {
@@ -635,20 +659,42 @@ impl LightFetch {
 	}
 }
 
-#[derive(Clone)]
+struct ExecuteParams<S>
-struct ExecuteParams {
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	from: Address,
 	tx: EthTransaction,
 	hdr: encoded::Header,
 	env_info: ::vm::EnvInfo,
 	engine: Arc<::ethcore::engines::EthEngine>,
 	on_demand: Arc<OnDemand>,
-	sync: Arc<LightSync>,
+	sync: Arc<S>,
+}
+
+impl<S> Clone for ExecuteParams<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
+	fn clone(&self) -> Self {
+		Self {
+			from: self.from.clone(),
+			tx: self.tx.clone(),
+			hdr: self.hdr.clone(),
+			env_info: self.env_info.clone(),
+			engine: self.engine.clone(),
+			on_demand: self.on_demand.clone(),
+			sync: self.sync.clone()
+		}
+	}
 }
 
 // Has a peer execute the transaction with given params. If `gas_known` is false, this will set the `gas value` to the
 // `required gas value` unless it exceeds the block gas limit
-fn execute_read_only_tx(gas_known: bool, params: ExecuteParams) -> impl Future<Item = ExecutionResult, Error = Error> + Send {
+fn execute_read_only_tx<S>(gas_known: bool, params: ExecuteParams<S>) -> impl Future<Item = ExecutionResult, Error = Error> + Send
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	if !gas_known {
 		Box::new(future::loop_fn(params, |mut params| {
 			execute_read_only_tx(true, params.clone()).and_then(move |res| {

rpc/src/v1/impls/eth_pubsub.rs

@@ -36,7 +36,9 @@ use light::client::{LightChainClient, LightChainNotify};
 use light::on_demand::OnDemand;
 use parity_runtime::Executor;
 use parking_lot::{RwLock, Mutex};
-use sync::LightSync;
+
+use sync::{LightSyncProvider, LightNetworkDispatcher, ManageNetwork};
+
 use types::encoded;
 use types::filter::Filter as EthFilter;
 
@@ -87,12 +89,15 @@ impl<C> EthPubSubClient<C> {
 	}
 }
 
-impl EthPubSubClient<LightFetch> {
+impl<S> EthPubSubClient<LightFetch<S>>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	/// Creates a new `EthPubSubClient` for `LightClient`.
 	pub fn light(
 		client: Arc<LightChainClient>,
 		on_demand: Arc<OnDemand>,
-		sync: Arc<LightSync>,
+		sync: Arc<S>,
 		cache: Arc<Mutex<Cache>>,
 		executor: Executor,
 		gas_price_percentile: usize,
@@ -189,7 +194,10 @@ pub trait LightClient: Send + Sync {
 	fn logs(&self, filter: EthFilter) -> BoxFuture<Vec<Log>>;
 }
 
-impl LightClient for LightFetch {
+impl<S> LightClient for LightFetch<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	fn block_header(&self, id: BlockId) -> Option<encoded::Header> {
 		self.client.block_header(id)
 	}
@@ -200,10 +208,7 @@ impl LightClient for LightFetch {
 }
 
 impl<C: LightClient> LightChainNotify for ChainNotificationHandler<C> {
-	fn new_headers(
+	fn new_headers(&self, enacted: &[H256]) {
-		&self,
-		enacted: &[H256],
-	) {
 		let headers = enacted
 			.iter()
 			.filter_map(|hash| self.client.block_header(BlockId::Hash(*hash)))

rpc/src/v1/impls/light/eth.rs

@@ -32,7 +32,6 @@ use ethereum_types::{U256, Address};
 use hash::{KECCAK_NULL_RLP, KECCAK_EMPTY_LIST_RLP};
 use parking_lot::{RwLock, Mutex};
 use rlp::Rlp;
-use sync::LightSync;
 use types::transaction::SignedTransaction;
 use types::encoded;
 use types::filter::Filter as EthcoreFilter;
@@ -45,19 +44,22 @@ use v1::helpers::deprecated::{self, DeprecationNotice};
 use v1::helpers::light_fetch::{self, LightFetch};
 use v1::traits::Eth;
 use v1::types::{
-	RichBlock, Block, BlockTransactions, BlockNumber, LightBlockNumber, Bytes, SyncStatus, SyncInfo,
+	RichBlock, Block, BlockTransactions, BlockNumber, LightBlockNumber, Bytes,
+	SyncStatus as RpcSyncStatus, SyncInfo as RpcSyncInfo,
 	Transaction, CallRequest, Index, Filter, Log, Receipt, Work, EthAccount,
 	H64 as RpcH64, H256 as RpcH256, H160 as RpcH160, U256 as RpcU256,
 	U64 as RpcU64,
 };
 use v1::metadata::Metadata;
 
+use sync::{LightSyncInfo, LightSyncProvider, LightNetworkDispatcher, ManageNetwork};
+
 const NO_INVALID_BACK_REFS: &str = "Fails only on invalid back-references; back-references here known to be valid; qed";
 
 /// Light client `ETH` (and filter) RPC.
-pub struct EthClient<T> {
+pub struct EthClient<C, S: LightSyncProvider + LightNetworkDispatcher + 'static> {
-	sync: Arc<LightSync>,
+	sync: Arc<S>,
-	client: Arc<T>,
+	client: Arc<C>,
 	on_demand: Arc<OnDemand>,
 	transaction_queue: Arc<RwLock<TransactionQueue>>,
 	accounts: Arc<Fn() -> Vec<Address> + Send + Sync>,
@@ -68,7 +70,10 @@ pub struct EthClient<T> {
 	deprecation_notice: DeprecationNotice,
 }
 
-impl<T> Clone for EthClient<T> {
+impl<C, S> Clone for EthClient<C, S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + 'static
+{
 	fn clone(&self) -> Self {
 		// each instance should have its own poll manager.
 		EthClient {
@@ -86,12 +91,16 @@ impl<T> Clone for EthClient<T> {
 	}
 }
 
-impl<T: LightChainClient + 'static> EthClient<T> {
+impl<C, S> EthClient<C, S>
+where
+	C: LightChainClient + 'static,
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	/// Create a new `EthClient` with a handle to the light sync instance, client,
 	/// and on-demand request service, which is assumed to be attached as a handler.
 	pub fn new(
-		sync: Arc<LightSync>,
+		sync: Arc<S>,
-		client: Arc<T>,
+		client: Arc<C>,
 		on_demand: Arc<OnDemand>,
 		transaction_queue: Arc<RwLock<TransactionQueue>>,
 		accounts: Arc<Fn() -> Vec<Address> + Send + Sync>,
@@ -114,7 +123,8 @@ impl<T: LightChainClient + 'static> EthClient<T> {
 	}
 
 	/// Create a light data fetcher instance.
-	fn fetcher(&self) -> LightFetch {
+	fn fetcher(&self) -> LightFetch<S>
+	{
 		LightFetch {
 			client: self.client.clone(),
 			on_demand: self.on_demand.clone(),
@@ -211,21 +221,25 @@ impl<T: LightChainClient + 'static> EthClient<T> {
 	}
 }
 
-impl<T: LightChainClient + 'static> Eth for EthClient<T> {
+impl<C, S> Eth for EthClient<C, S>
+where
+	C: LightChainClient + 'static,
+	S: LightSyncInfo + LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	type Metadata = Metadata;
 
 	fn protocol_version(&self) -> Result<String> {
 		Ok(format!("{}", ::light::net::MAX_PROTOCOL_VERSION))
 	}
 
-	fn syncing(&self) -> Result<SyncStatus> {
+	fn syncing(&self) -> Result<RpcSyncStatus> {
 		if self.sync.is_major_importing() {
 			let chain_info = self.client.chain_info();
 			let current_block = U256::from(chain_info.best_block_number);
 			let highest_block = self.sync.highest_block().map(U256::from)
 				.unwrap_or_else(|| current_block);
 
-			Ok(SyncStatus::Info(SyncInfo {
+			Ok(RpcSyncStatus::Info(RpcSyncInfo {
 				starting_block: U256::from(self.sync.start_block()).into(),
 				current_block: current_block.into(),
 				highest_block: highest_block.into(),
@@ -233,7 +247,7 @@ impl<T: LightChainClient + 'static> Eth for EthClient<T> {
 				warp_chunks_processed: None,
 			}))
 		} else {
-			Ok(SyncStatus::None)
+			Ok(RpcSyncStatus::None)
 		}
 	}
 
@@ -524,7 +538,11 @@ impl<T: LightChainClient + 'static> Eth for EthClient<T> {
 }
 
 // This trait implementation triggers a blanked impl of `EthFilter`.
-impl<T: LightChainClient + 'static> Filterable for EthClient<T> {
+impl<C, S> Filterable for EthClient<C, S>
+where
+	C: LightChainClient + 'static,
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	fn best_block_number(&self) -> u64 { self.client.chain_info().best_block_number }
 
 	fn block_hash(&self, id: BlockId) -> Option<::ethereum_types::H256> {

rpc/src/v1/impls/light/parity.rs

@@ -23,7 +23,7 @@ use version::version_data;
 use crypto::DEFAULT_MAC;
 use ethkey::{crypto::ecies, Brain, Generator};
 use ethstore::random_phrase;
-use sync::LightSyncProvider;
+use sync::{LightSyncInfo, LightSyncProvider, LightNetworkDispatcher, ManageNetwork};
 use ethcore_logger::RotatingLogger;
 
 use jsonrpc_core::{Result, BoxFuture};
@@ -46,8 +46,8 @@ use v1::types::{
 use Host;
 
 /// Parity implementation for light client.
-pub struct ParityClient {
+pub struct ParityClient<S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static> {
-	light_dispatch: Arc<LightDispatcher>,
+	light_dispatch: Arc<LightDispatcher<S>>,
 	logger: Arc<RotatingLogger>,
 	settings: Arc<NetworkSettings>,
 	signer: Option<Arc<SignerService>>,
@@ -55,10 +55,13 @@ pub struct ParityClient {
 	gas_price_percentile: usize,
 }
 
-impl ParityClient {
+impl<S> ParityClient<S>
+where
+	S: LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	/// Creates new `ParityClient`.
 	pub fn new(
-		light_dispatch: Arc<LightDispatcher>,
+		light_dispatch: Arc<LightDispatcher<S>>,
 		logger: Arc<RotatingLogger>,
 		settings: Arc<NetworkSettings>,
 		signer: Option<Arc<SignerService>>,
@@ -76,7 +79,8 @@ impl ParityClient {
 	}
 
 	/// Create a light blockchain data fetcher.
-	fn fetcher(&self) -> LightFetch {
+	fn fetcher(&self) -> LightFetch<S>
+	{
 		LightFetch {
 			client: self.light_dispatch.client.clone(),
 			on_demand: self.light_dispatch.on_demand.clone(),
@@ -87,7 +91,10 @@ impl ParityClient {
 	}
 }
 
-impl Parity for ParityClient {
+impl<S> Parity for ParityClient<S>
+where
+	S: LightSyncInfo + LightSyncProvider + LightNetworkDispatcher + ManageNetwork + 'static
+{
 	type Metadata = Metadata;
 
 	fn transactions_limit(&self) -> Result<usize> {
@@ -371,7 +378,7 @@ impl Parity for ParityClient {
 
 	fn status(&self) -> Result<()> {
 		let has_peers = self.settings.is_dev_chain || self.light_dispatch.sync.peer_numbers().connected > 0;
-		let is_importing = self.light_dispatch.sync.is_major_importing();
+		let is_importing = (*self.light_dispatch.sync).is_major_importing();
 
 		if has_peers && !is_importing {
 			Ok(())