[contract-client] refactor (#7978)

* Refactor usage of registry contract

* remove unsed file

* Rename methods & fix comments

* refactor contract_client:

* Intoduced separate traits for asyncronous and syncronous contract clients
* Removed registrar from ethcore::client
* s/ContractClient/AsyncContractClient

* make the tests compile and pass

* omit changes in ethcore

* Use only one trait for ContractClient

* Use an associated type in the trait to determine syncronous or asyncronous communication
* Export the types from contract-client crate
* Document that "A" in the hash correspons to a DNS A Record A.K.A Address Record

* contract_client -> registrar

* address review feedback

Cargo.lock

@@ -1984,6 +1984,7 @@ dependencies = [
  "path 0.1.0",
  "pretty_assertions 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "registrar 0.0.1",
  "rlp 0.2.1",
  "rpassword 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "rpc-cli 1.4.0",
@@ -2029,6 +2030,7 @@ dependencies = [
  "parity-version 1.11.0",
  "parking_lot 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "registrar 0.0.1",
  "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.27 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_derive 1.0.27 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2083,6 +2085,7 @@ dependencies = [
  "parity-reactor 0.1.0",
  "parking_lot 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "registrar 0.0.1",
  "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -2643,6 +2646,17 @@ name = "regex-syntax"
 version = "0.4.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "registrar"
+version = "0.0.1"
+dependencies = [
+ "ethabi 5.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethabi-contract 5.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethabi-derive 5.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "futures 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
+ "keccak-hash 0.1.0",
+]
+
 [[package]]
 name = "reqwest"
 version = "0.8.1"

Cargo.toml

@@ -72,6 +72,8 @@ journaldb = { path = "util/journaldb" }
 parity-dapps = { path = "dapps", optional = true }
 ethcore-secretstore = { path = "secret_store", optional = true }
 
+registrar = { path = "registrar" }
+
 [build-dependencies]
 rustc_version = "0.2"
 

dapps/Cargo.toml

@@ -37,6 +37,7 @@ parity-reactor = { path = "../util/reactor" }
 parity-ui = { path = "./ui" }
 keccak-hash = { path = "../util/hash" }
 parity-version = { path = "../util/version" }
+registrar = { path = "../registrar" }
 
 [dev-dependencies]
 env_logger = "0.4"

dapps/src/lib.rs

@@ -41,6 +41,7 @@ extern crate parity_hash_fetch as hash_fetch;
 extern crate parity_ui;
 extern crate keccak_hash as hash;
 extern crate parity_version;
+extern crate registrar;
 
 #[macro_use]
 extern crate futures;
@@ -80,7 +81,7 @@ use parking_lot::RwLock;
 use fetch::Fetch;
 use node_health::NodeHealth;
 
-pub use hash_fetch::urlhint::ContractClient;
+pub use registrar::{RegistrarClient, Asynchronous};
 pub use node_health::SyncStatus;
 
 
@@ -155,7 +156,7 @@ impl Middleware {
 		pool: CpuPool,
 		health: NodeHealth,
 		dapps_domain: &str,
-		registrar: Arc<ContractClient>,
+		registrar: Arc<RegistrarClient<Call=Asynchronous>>,
 		sync_status: Arc<SyncStatus>,
 		fetch: F,
 	) -> Self {
@@ -198,7 +199,7 @@ impl Middleware {
 		dapps_path: PathBuf,
 		extra_dapps: Vec<PathBuf>,
 		dapps_domain: &str,
-		registrar: Arc<ContractClient>,
+		registrar: Arc<RegistrarClient<Call=Asynchronous>>,
 		sync_status: Arc<SyncStatus>,
 		web_proxy_tokens: Arc<WebProxyTokens>,
 		fetch: F,

dapps/src/tests/helpers/mod.rs

@@ -24,7 +24,7 @@ use jsonrpc_http_server::{self as http, Host, DomainsValidation};
 use parity_reactor::Remote;
 
 use devtools::http_client;
-use hash_fetch::urlhint::ContractClient;
+use registrar::{RegistrarClient, Asynchronous};
 use fetch::{Fetch, Client as FetchClient};
 use node_health::{NodeHealth, TimeChecker, CpuPool};
 
@@ -144,7 +144,7 @@ pub fn assert_security_headers_for_embed(headers: &[String]) {
 /// Webapps HTTP+RPC server build.
 pub struct ServerBuilder<T: Fetch = FetchClient> {
 	dapps_path: PathBuf,
-	registrar: Arc<ContractClient>,
+	registrar: Arc<RegistrarClient<Call=Asynchronous>>,
 	sync_status: Arc<SyncStatus>,
 	web_proxy_tokens: Arc<WebProxyTokens>,
 	signer_address: Option<(String, u16)>,
@@ -155,7 +155,7 @@ pub struct ServerBuilder<T: Fetch = FetchClient> {
 
 impl ServerBuilder {
 	/// Construct new dapps server
-	pub fn new<P: AsRef<Path>>(dapps_path: P, registrar: Arc<ContractClient>) -> Self {
+	pub fn new<P: AsRef<Path>>(dapps_path: P, registrar: Arc<RegistrarClient<Call=Asynchronous>>) -> Self {
 		ServerBuilder {
 			dapps_path: dapps_path.as_ref().to_owned(),
 			registrar: registrar,
@@ -227,7 +227,7 @@ impl Server {
 		signer_address: Option<(String, u16)>,
 		dapps_path: PathBuf,
 		extra_dapps: Vec<PathBuf>,
-		registrar: Arc<ContractClient>,
+		registrar: Arc<RegistrarClient<Call=Asynchronous>>,
 		sync_status: Arc<SyncStatus>,
 		web_proxy_tokens: Arc<WebProxyTokens>,
 		remote: Remote,

dapps/src/tests/helpers/registrar.rs

@@ -18,10 +18,9 @@ use std::str;
 use std::sync::Arc;
 use std::collections::HashMap;
 
-use futures::Future;
 use ethereum_types::{H256, Address};
 use bytes::{Bytes, ToPretty};
-use hash_fetch::urlhint::ContractClient;
+use registrar::{RegistrarClient, Asynchronous};
 use parking_lot::Mutex;
 use rustc_hex::FromHex;
 
@@ -62,12 +61,14 @@ impl FakeRegistrar {
 	}
 }
 
-impl ContractClient for FakeRegistrar {
-	fn registrar(&self) -> Result<Address, String> {
+impl RegistrarClient for FakeRegistrar {
+	type Call = Asynchronous;
+
+	fn registrar_address(&self) -> Result<Address, String> {
 		Ok(REGISTRAR.parse().unwrap())
 	}
 
-	fn call(&self, address: Address, data: Bytes) -> Box<Future<Item = Bytes, Error = String> + Send> {
+	fn call_contract(&self, address: Address, data: Bytes) -> Self::Call {
 		let call = (address.to_hex(), data.to_hex());
 		self.calls.lock().push(call.clone());
 		let res = self.responses.lock().get(&call).cloned().expect(&format!("No response for call: {:?}", call));

hash-fetch/Cargo.toml

@@ -18,6 +18,7 @@ ethcore-bytes = { path = "../util/bytes" }
 ethereum-types = "0.2"
 parity-reactor = { path = "../util/reactor" }
 keccak-hash = { path = "../util/hash" }
+registrar = { path = "../registrar" }
 
 ethabi = "5.1"
 ethabi-derive = "5.0"

hash-fetch/src/client.rs

@@ -25,7 +25,8 @@ use hash::keccak_buffer;
 use fetch::{Fetch, Response, Error as FetchError, Client as FetchClient};
 use futures::{Future, IntoFuture};
 use parity_reactor::Remote;
-use urlhint::{ContractClient, URLHintContract, URLHint, URLHintResult};
+use urlhint::{URLHintContract, URLHint, URLHintResult};
+use registrar::{RegistrarClient, Asynchronous};
 use ethereum_types::H256;
 
 /// API for fetching by hash.
@@ -120,14 +121,14 @@ pub struct Client<F: Fetch + 'static = FetchClient> {
 
 impl Client {
 	/// Creates new instance of the `Client` given on-chain contract client and task runner.
-	pub fn new(contract: Arc<ContractClient>, remote: Remote) -> Self {
+	pub fn new(contract: Arc<RegistrarClient<Call=Asynchronous>>, remote: Remote) -> Self {
 		Client::with_fetch(contract, FetchClient::new().unwrap(), remote)
 	}
 }
 
 impl<F: Fetch + 'static> Client<F> {
 	/// Creates new instance of the `Client` given on-chain contract client, fetch service and task runner.
-	pub fn with_fetch(contract: Arc<ContractClient>, fetch: F, remote: Remote) -> Self {
+	pub fn with_fetch(contract: Arc<RegistrarClient<Call=Asynchronous>>, fetch: F, remote: Remote) -> Self {
 		Client {
 			contract: URLHintContract::new(contract),
 			fetch: fetch,

hash-fetch/src/lib.rs

@@ -31,6 +31,7 @@ extern crate mime_guess;
 extern crate parity_reactor;
 extern crate rand;
 extern crate rustc_hex;
+extern crate registrar;
 
 pub extern crate fetch;
 

hash-fetch/src/urlhint.rs

@@ -20,31 +20,21 @@ use std::sync::Arc;
 use rustc_hex::ToHex;
 use mime::{self, Mime};
 use mime_guess;
-use hash::keccak;
 
 use futures::{future, Future};
 use futures::future::Either;
 use ethereum_types::{H256, Address};
-use bytes::Bytes;
+use registrar::{Registrar, RegistrarClient, Asynchronous};
 
-use_contract!(registry, "Registry", "res/registrar.json");
 use_contract!(urlhint, "Urlhint", "res/urlhint.json");
 
 const COMMIT_LEN: usize = 20;
+const GITHUB_HINT: &'static str = "githubhint";
 /// GithubHint entries with commit set as `0x0..01` should be treated
 /// as Github Dapp, downloadable zip files, than can be extracted, containing
 /// the manifest.json file along with the dapp
 static GITHUB_DAPP_COMMIT: &[u8; COMMIT_LEN] = &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1];
 
-/// RAW Contract interface.
-/// Should execute transaction using current blockchain state.
-pub trait ContractClient: Send + Sync {
-	/// Get registrar address
-	fn registrar(&self) -> Result<Address, String>;
-	/// Call Contract
-	fn call(&self, address: Address, data: Bytes) -> Box<Future<Item = Bytes, Error = String> + Send>;
-}
-
 /// Github-hosted dapp.
 #[derive(Debug, PartialEq)]
 pub struct GithubApp {
@@ -111,17 +101,17 @@ pub trait URLHint: Send + Sync {
 /// `URLHintContract` API
 pub struct URLHintContract {
 	urlhint: urlhint::Urlhint,
-	registrar: registry::Registry,
-	client: Arc<ContractClient>,
+	registrar: Registrar,
+	client: Arc<RegistrarClient<Call=Asynchronous>>,
 }
 
 impl URLHintContract {
 	/// Creates new `URLHintContract`
-	pub fn new(client: Arc<ContractClient>) -> Self {
+		pub fn new(client: Arc<RegistrarClient<Call=Asynchronous>>) -> Self {
 		URLHintContract {
 			urlhint: urlhint::Urlhint::default(),
-			registrar: registry::Registry::default(),
-			client,
+			registrar: Registrar::new(client.clone()),
+			client: client,
 		}
 	}
 }
@@ -172,28 +162,19 @@ fn decode_urlhint_output(output: (String, [u8; 20], Address)) -> Option<URLHintR
 
 impl URLHint for URLHintContract {
 	fn resolve(&self, id: H256) -> Box<Future<Item = Option<URLHintResult>, Error = String> + Send> {
-		let address = match self.client.registrar() {
-			Ok(a) => a,
-			Err(e) => return Box::new(future::err(e)),
-		};
-
-		let client = self.client.clone();
-		let get_address = self.registrar.functions().get_address();
 		let entries = self.urlhint.functions().entries();
-		let data = get_address.input(keccak("githubhint"), "A");
+		let client = self.client.clone();
 
-		let future = client.call(address, data)
-			.and_then(move |output| get_address.output(&output).map_err(|e| e.to_string()))
+		let future = self.registrar.get_address(GITHUB_HINT)
 			.and_then(move |addr| if !addr.is_zero() {
 				let data = entries.input(id);
-				let result = client.call(addr, data)
+				let result = client.call_contract(addr, data)
 					.and_then(move |output| entries.output(&output).map_err(|e| e.to_string()))
 					.map(decode_urlhint_output);
 				Either::B(result)
 			} else {
 				Either::A(future::ok(None))
 		});
-
 		Box::new(future)
 	}
 }
@@ -257,12 +238,14 @@ pub mod tests {
 		}
 	}
 
-	impl ContractClient for FakeRegistrar {
-		fn registrar(&self) -> Result<Address, String> {
+	impl RegistrarClient for FakeRegistrar {
+		type Call = Asynchronous;
+
+		fn registrar_address(&self) -> Result<Address, String> {
 			Ok(REGISTRAR.parse().unwrap())
 		}
 
-		fn call(&self, address: Address, data: Bytes) -> Box<Future<Item = Bytes, Error = String> + Send> {
+		fn call_contract(&self, address: Address, data: Bytes) -> Self::Call {
 			self.calls.lock().push((address.to_hex(), data.to_hex()));
 			let res = self.responses.lock().remove(0);
 			Box::new(res.into_future())

parity/dapps.rs

@@ -24,7 +24,7 @@ use ethcore::client::{Client, BlockChainClient, BlockId, CallContract};
 use ethsync::LightSync;
 use futures::{Future, future, IntoFuture};
 use hash_fetch::fetch::Client as FetchClient;
-use hash_fetch::urlhint::ContractClient;
+use registrar::{RegistrarClient, Asynchronous};
 use light::client::LightChainClient;
 use light::on_demand::{self, OnDemand};
 use node_health::{SyncStatus, NodeHealth};
@@ -78,13 +78,15 @@ impl FullRegistrar {
 	}
 }
 
-impl ContractClient for FullRegistrar {
-	fn registrar(&self) -> Result<Address, String> {
+impl RegistrarClient for FullRegistrar {
+	type Call = Asynchronous;
+
+	fn registrar_address(&self) -> Result<Address, String> {
 		self.client.registrar_address()
 			 .ok_or_else(|| "Registrar not defined.".into())
 	}
 
-	fn call(&self, address: Address, data: Bytes) -> Box<Future<Item = Bytes, Error = String> + Send> {
+	fn call_contract(&self, address: Address, data: Bytes) -> Self::Call {
 		Box::new(self.client.call_contract(BlockId::Latest, address, data).into_future())
 	}
 }
@@ -99,8 +101,10 @@ pub struct LightRegistrar<T> {
 	pub sync: Arc<LightSync>,
 }
 
-impl<T: LightChainClient + 'static> ContractClient for LightRegistrar<T> {
-	fn registrar(&self) -> Result<Address, String> {
+impl<T: LightChainClient + 'static> RegistrarClient for LightRegistrar<T> {
+	type Call = Box<Future<Item = Bytes, Error = String> + Send>;
+
+	fn registrar_address(&self) -> Result<Address, String> {
 		self.client.engine().additional_params().get("registrar")
 			 .ok_or_else(|| "Registrar not defined.".into())
 			 .and_then(|registrar| {
@@ -108,7 +112,7 @@ impl<T: LightChainClient + 'static> ContractClient for LightRegistrar<T> {
 			 })
 	}
 
-	fn call(&self, address: Address, data: Bytes) -> Box<Future<Item = Bytes, Error = String> + Send> {
+	fn call_contract(&self, address: Address, data: Bytes) -> Self::Call {
 		let header = self.client.best_block_header();
 		let env_info = self.client.env_info(BlockId::Hash(header.hash()))
 			.ok_or_else(|| format!("Cannot fetch env info for header {}", header.hash()));
@@ -154,7 +158,7 @@ impl<T: LightChainClient + 'static> ContractClient for LightRegistrar<T> {
 pub struct Dependencies {
 	pub node_health: NodeHealth,
 	pub sync_status: Arc<SyncStatus>,
-	pub contract_client: Arc<ContractClient>,
+	pub contract_client: Arc<RegistrarClient<Call=Asynchronous>>,
 	pub fetch: FetchClient,
 	pub signer: Arc<SignerService>,
 	pub ui_address: Option<(String, u16)>,

parity/main.rs

@@ -76,6 +76,7 @@ extern crate rpc_cli;
 extern crate node_filter;
 extern crate keccak_hash as hash;
 extern crate journaldb;
+extern crate registrar;
 
 #[macro_use]
 extern crate log as rlog;

registrar/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+description = "Registar for Parity"
+name = "registrar"
+version = "0.0.1"
+license = "GPL-3.0"
+authors = ["Parity Technologies <admin@parity.io>"]
+
+[dependencies]
+futures = "0.1"
+ethabi = "5.1.0"
+ethabi-derive = "5.0.5"
+ethabi-contract = "5.0.3"
+keccak-hash = { path = "../util/hash" }

registrar/src/lib.rs

@@ -0,0 +1,27 @@
+// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+extern crate futures;
+extern crate ethabi;
+extern crate keccak_hash;
+
+#[macro_use]
+extern crate ethabi_derive;
+#[macro_use]
+extern crate ethabi_contract;
+
+mod registrar;
+pub use registrar::{Registrar, RegistrarClient, Synchronous, Asynchronous};

registrar/src/registrar.rs

@@ -0,0 +1,77 @@
+// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+use futures::{Future, future, IntoFuture};
+use ethabi::{Address, Bytes};
+use std::sync::Arc;
+use keccak_hash::keccak;
+
+use_contract!(registry, "Registry", "res/registrar.json");
+
+// Maps a domain name to an Ethereum address
+const DNS_A_RECORD: &'static str = "A";
+
+pub type Asynchronous = Box<Future<Item=Bytes, Error=String> + Send>;
+pub type Synchronous = Result<Bytes, String>;
+
+/// Registrar is dedicated interface to access the registrar contract
+/// which in turn generates an address when a client requests one
+pub struct Registrar {
+	registrar: registry::Registry,
+	client: Arc<RegistrarClient<Call=Asynchronous>>,
+}
+
+impl Registrar {
+	/// Registrar constructor
+	pub fn new(client: Arc<RegistrarClient<Call=Asynchronous>>) -> Self {
+		Self {
+			registrar: registry::Registry::default(),
+			client: client,
+		}
+	}
+
+	/// Generate an address for the given key
+	pub fn get_address<'a>(&self, key: &'a str) -> Box<Future<Item = Address, Error = String> + Send> {
+		// Address of the registrar itself
+		let registrar_address = match self.client.registrar_address() {
+			Ok(a) => a,
+			Err(e) => return Box::new(future::err(e)),
+		};
+
+		let address_fetcher = self.registrar.functions().get_address();
+		let id = address_fetcher.input(keccak(key), DNS_A_RECORD);
+
+		let future = self.client.call_contract(registrar_address, id).and_then(move |address| {
+			address_fetcher.output(&address)
+		}
+		.map_err(|e| e.to_string()));
+
+		Box::new(future)
+	}
+}
+
+/// Registrar contract interface
+/// Should execute transaction using current blockchain state.
+pub trait RegistrarClient: Send + Sync {
+	/// Specifies synchronous or asynchronous communication
+	type Call: IntoFuture<Item=Bytes, Error=String>;
+
+	/// Get registrar address
+	fn registrar_address(&self) -> Result<Address, String>;
+	/// Call Contract
+	fn call_contract(&self, address: Address, data: Bytes) -> Self::Call;
+}
+