merge with master

parity/dapps.rs

@@ -22,7 +22,7 @@ use ethsync::EthSync;
 use ethminer::{Miner, ExternalMiner};
 use util::RotatingLogger;
 use util::panics::PanicHandler;
-use util::keys::store::{AccountService};
+use util::keys::store::AccountService;
 use util::network_settings::NetworkSettings;
 use die::*;
 
@@ -100,7 +100,7 @@ pub fn setup_dapps_server(
 	server.add_delegate(NetClient::new(&deps.sync).to_delegate());
 	server.add_delegate(EthClient::new(&deps.client, &deps.sync, &deps.secret_store, &deps.miner, &deps.external_miner).to_delegate());
 	server.add_delegate(EthFilterClient::new(&deps.client, &deps.miner).to_delegate());
-	server.add_delegate(PersonalClient::new(&deps.secret_store).to_delegate());
+	server.add_delegate(PersonalClient::new(&deps.secret_store, &deps.client, &deps.miner).to_delegate());
 	server.add_delegate(EthcoreClient::new(&deps.miner, deps.logger.clone(), deps.settings.clone()).to_delegate());
 
 	let start_result = match auth {

parity/rpc.rs

@@ -24,7 +24,7 @@ use ethsync::EthSync;
 use ethminer::{Miner, ExternalMiner};
 use util::RotatingLogger;
 use util::panics::PanicHandler;
-use util::keys::store::{AccountService};
+use util::keys::store::AccountService;
 use util::network_settings::NetworkSettings;
 use die::*;
 use jsonipc;
@@ -106,7 +106,7 @@ fn setup_rpc_server(apis: Vec<&str>, deps: &Arc<Dependencies>) -> Server {
 			},
 			"personal" => {
 				modules.insert("personal".to_owned(), "1.0".to_owned());
-				server.add_delegate(PersonalClient::new(&deps.secret_store).to_delegate())
+				server.add_delegate(PersonalClient::new(&deps.secret_store, &deps.client, &deps.miner).to_delegate())
 			},
 			"ethcore" => {
 				modules.insert("ethcore".to_owned(), "1.0".to_owned());

rpc/src/v1/impls/eth.rs

@@ -22,11 +22,10 @@ use std::collections::HashSet;
 use std::sync::{Arc, Weak, Mutex};
 use std::ops::Deref;
 use ethsync::{SyncProvider, SyncState};
-use ethminer::{MinerService, AccountDetails, ExternalMinerService};
+use ethminer::{MinerService, ExternalMinerService};
 use jsonrpc_core::*;
 use util::numbers::*;
 use util::sha3::*;
-use util::bytes::ToPretty;
 use util::rlp::{encode, decode, UntrustedRlp, View};
 use ethcore::client::{BlockChainClient, BlockID, TransactionID, UncleID};
 use ethcore::block::IsBlock;
@@ -39,6 +38,7 @@ use self::ethash::SeedHashCompute;
 use v1::traits::{Eth, EthFilter};
 use v1::types::{Block, BlockTransactions, BlockNumber, Bytes, SyncStatus, SyncInfo, Transaction, TransactionRequest, CallRequest, OptionalValue, Index, Filter, Log, Receipt};
 use v1::helpers::{PollFilter, PollManager};
+use v1::impls::{dispatch_transaction, sign_and_dispatch};
 use util::keys::store::AccountProvider;
 use serde;
 
@@ -155,27 +155,6 @@ impl<C, S, A, M, EM> EthClient<C, S, A, M, EM> where
 		}
 	}
 
-	fn sign_and_dispatch(&self, request: TransactionRequest, secret: H256) -> Result<Value, Error> {
-		let signed_transaction = {
-			let client = take_weak!(self.client);
-			let miner = take_weak!(self.miner);
-			EthTransaction {
-				nonce: request.nonce
-					.or_else(|| miner
-							 .last_nonce(&request.from)
-							 .map(|nonce| nonce + U256::one()))
-					.unwrap_or_else(|| client.latest_nonce(&request.from)),
-					action: request.to.map_or(Action::Create, Action::Call),
-					gas: request.gas.unwrap_or_else(|| miner.sensible_gas_limit()),
-					gas_price: request.gas_price.unwrap_or_else(|| miner.sensible_gas_price()),
-					value: request.value.unwrap_or_else(U256::zero),
-					data: request.data.map_or_else(Vec::new, |d| d.to_vec()),
-			}.sign(&secret)
-		};
-		trace!(target: "miner", "send_transaction: dispatching tx: {}", encode(&signed_transaction).to_vec().pretty());
-		self.dispatch_transaction(signed_transaction)
-	}
-
 	fn sign_call(&self, request: CallRequest) -> Result<SignedTransaction, Error> {
 		let client = take_weak!(self.client);
 		let miner = take_weak!(self.miner);
@@ -189,30 +168,6 @@ impl<C, S, A, M, EM> EthClient<C, S, A, M, EM> where
 			data: request.data.map_or_else(Vec::new, |d| d.to_vec())
 		}.fake_sign(from))
 	}
-
-	fn dispatch_transaction(&self, signed_transaction: SignedTransaction) -> Result<Value, Error> {
-		let hash = signed_transaction.hash();
-
-		let import = {
-			let client = take_weak!(self.client);
-			let miner = take_weak!(self.miner);
-
-			miner.import_own_transaction(client.deref(), signed_transaction, |a: &Address| {
-				AccountDetails {
-					nonce: client.latest_nonce(&a),
-					balance: client.latest_balance(&a),
-				}
-			})
-		};
-
-		match import {
-			Ok(_) => to_value(&hash),
-			Err(e) => {
-				warn!("Error sending transaction: {:?}", e);
-				to_value(&H256::zero())
-			}
-		}
-	}
 }
 
 const MAX_QUEUE_SIZE_TO_MINE_ON: usize = 4;	// because uncles go back 6.
@@ -544,29 +499,18 @@ impl<C, S, A, M, EM> Eth for EthClient<C, S, A, M, EM> where
 			.and_then(|(request, )| {
 				let accounts = take_weak!(self.accounts);
 				match accounts.account_secret(&request.from) {
-					Ok(secret) => self.sign_and_dispatch(request, secret),
+					Ok(secret) => sign_and_dispatch(&self.client, &self.miner, request, secret),
 					Err(_) => to_value(&H256::zero())
 				}
 		})
 	}
 
-	fn sign_and_send_transaction(&self, params: Params) -> Result<Value, Error> {
-		from_params::<(TransactionRequest, String)>(params)
-			.and_then(|(request, password)| {
-				let accounts = take_weak!(self.accounts);
-				match accounts.locked_account_secret(&request.from, &password) {
-					Ok(secret) => self.sign_and_dispatch(request, secret),
-					Err(_) => to_value(&H256::zero()),
-				}
-		})
-	}
-
 	fn send_raw_transaction(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(Bytes, )>(params)
 			.and_then(|(raw_transaction, )| {
 				let raw_transaction = raw_transaction.to_vec();
 				match UntrustedRlp::new(&raw_transaction).as_val() {
-					Ok(signed_transaction) => self.dispatch_transaction(signed_transaction),
+					Ok(signed_transaction) => dispatch_transaction(&*take_weak!(self.client), &*take_weak!(self.miner), signed_transaction),
 					Err(_) => to_value(&H256::zero()),
 				}
 		})

rpc/src/v1/impls/mod.rs

@@ -41,3 +41,59 @@ pub use self::ethcore::EthcoreClient;
 pub use self::traces::TracesClient;
 pub use self::rpc::RpcClient;
 
+use v1::types::TransactionRequest;
+use std::sync::Weak;
+use ethminer::{AccountDetails, MinerService};
+use ethcore::client::BlockChainClient;
+use ethcore::transaction::{Action, SignedTransaction, Transaction};
+use util::numbers::*;
+use util::rlp::encode;
+use util::bytes::ToPretty;
+use jsonrpc_core::{Error, to_value, Value};
+
+fn dispatch_transaction<C, M>(client: &C, miner: &M, signed_transaction: SignedTransaction) -> Result<Value, Error>
+	where C: BlockChainClient, M: MinerService {
+	let hash = signed_transaction.hash();
+
+	let import = {
+		miner.import_own_transaction(client, signed_transaction, |a: &Address| {
+			AccountDetails {
+				nonce: client.latest_nonce(&a),
+				balance: client.latest_balance(&a),
+			}
+		})
+	};
+
+	match import {
+		Ok(_) => to_value(&hash),
+		Err(e) => {
+			warn!("Error sending transaction: {:?}", e);
+			to_value(&H256::zero())
+		}
+	}
+}
+
+fn sign_and_dispatch<C, M>(client: &Weak<C>, miner: &Weak<M>, request: TransactionRequest, secret: H256) -> Result<Value, Error>
+	where C: BlockChainClient, M: MinerService {
+	let client = take_weak!(client);
+	let miner = take_weak!(miner);
+
+	let signed_transaction = {
+		Transaction {
+			nonce: request.nonce
+				.or_else(|| miner
+						 .last_nonce(&request.from)
+						 .map(|nonce| nonce + U256::one()))
+				.unwrap_or_else(|| client.latest_nonce(&request.from)),
+
+			action: request.to.map_or(Action::Create, Action::Call),
+			gas: request.gas.unwrap_or_else(|| miner.sensible_gas_limit()),
+			gas_price: request.gas_price.unwrap_or_else(|| miner.sensible_gas_price()),
+			value: request.value.unwrap_or_else(U256::zero),
+			data: request.data.map_or_else(Vec::new, |b| b.to_vec()),
+		}.sign(&secret)
+	};
+
+	trace!(target: "miner", "send_transaction: dispatching tx: {}", encode(&signed_transaction).to_vec().pretty());
+	dispatch_transaction(&*client, &*miner, signed_transaction)
+}

rpc/src/v1/impls/personal.rs

@@ -18,24 +18,35 @@
 use std::sync::{Arc, Weak};
 use jsonrpc_core::*;
 use v1::traits::Personal;
+use v1::types::TransactionRequest;
+use v1::impls::sign_and_dispatch;
 use util::keys::store::*;
-use util::Address;
+use util::numbers::*;
+use ethcore::client::BlockChainClient;
+use ethminer::MinerService;
 
 /// Account management (personal) rpc implementation.
-pub struct PersonalClient<A> where A: AccountProvider {
+pub struct PersonalClient<A, C, M>
+	where A: AccountProvider, C: BlockChainClient, M: MinerService {
 	accounts: Weak<A>,
+	client: Weak<C>,
+	miner: Weak<M>,
 }
 
-impl<A> PersonalClient<A> where A: AccountProvider {
+impl<A, C, M> PersonalClient<A, C, M>
+	where A: AccountProvider, C: BlockChainClient, M: MinerService {
 	/// Creates new PersonalClient
-	pub fn new(store: &Arc<A>) -> Self {
+	pub fn new(store: &Arc<A>, client: &Arc<C>, miner: &Arc<M>) -> Self {
 		PersonalClient {
 			accounts: Arc::downgrade(store),
+			client: Arc::downgrade(client),
+			miner: Arc::downgrade(miner),
 		}
 	}
 }
 
-impl<A> Personal for PersonalClient<A> where A: AccountProvider + 'static {
+impl<A: 'static, C: 'static, M: 'static> Personal for PersonalClient<A, C, M>
+	where A: AccountProvider, C: BlockChainClient, M: MinerService {
 	fn accounts(&self, _: Params) -> Result<Value, Error> {
 		let store = take_weak!(self.accounts);
 		match store.accounts() {
@@ -66,4 +77,15 @@ impl<A> Personal for PersonalClient<A> where A: AccountProvider + 'static {
 				}
 			})
 	}
+
+	fn sign_and_send_transaction(&self, params: Params) -> Result<Value, Error> {
+		from_params::<(TransactionRequest, String)>(params)
+			.and_then(|(request, password)| {
+				let accounts = take_weak!(self.accounts);
+				match accounts.locked_account_secret(&request.from, &password) {
+					Ok(secret) => sign_and_dispatch(&self.client, &self.miner, request, secret),
+					Err(_) => to_value(&H256::zero()),
+				}
+		})
+	}
 }

rpc/src/v1/tests/mocked/eth.rs

@@ -521,81 +521,6 @@ fn rpc_eth_send_transaction() {
 	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
 }
 
-#[test]
-fn rpc_eth_sign_and_send_transaction_with_invalid_password() {
-	let account = TestAccount::new("password123");
-	let address = account.address();
-
-	let tester = EthTester::default();
-	tester.accounts_provider.accounts.write().unwrap().insert(address.clone(), account);
-	let request = r#"{
-		"jsonrpc": "2.0",
-		"method": "eth_signAndSendTransaction",
-		"params": [{
-			"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
-			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
-			"gas": "0x76c0",
-			"gasPrice": "0x9184e72a000",
-			"value": "0x9184e72a"
-		}, "password321"],
-		"id": 1
-	}"#;
-
-	let response = r#"{"jsonrpc":"2.0","result":"0x0000000000000000000000000000000000000000000000000000000000000000","id":1}"#;
-
-	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response.into()));
-}
-
-#[test]
-fn rpc_eth_sign_and_send_transaction() {
-	let account = TestAccount::new("password123");
-	let address = account.address();
-	let secret = account.secret.clone();
-
-	let tester = EthTester::default();
-	tester.accounts_provider.accounts.write().unwrap().insert(address.clone(), account);
-	let request = r#"{
-		"jsonrpc": "2.0",
-		"method": "eth_signAndSendTransaction",
-		"params": [{
-			"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
-			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
-			"gas": "0x76c0",
-			"gasPrice": "0x9184e72a000",
-			"value": "0x9184e72a"
-		}, "password123"],
-		"id": 1
-	}"#;
-
-	let t = Transaction {
-		nonce: U256::zero(),
-		gas_price: U256::from(0x9184e72a000u64),
-		gas: U256::from(0x76c0),
-		action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
-		value: U256::from(0x9184e72au64),
-		data: vec![]
-	}.sign(&secret);
-
-	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#;
-
-	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
-
-	tester.miner.last_nonces.write().unwrap().insert(address.clone(), U256::zero());
-
-	let t = Transaction {
-		nonce: U256::one(),
-		gas_price: U256::from(0x9184e72a000u64),
-		gas: U256::from(0x76c0),
-		action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
-		value: U256::from(0x9184e72au64),
-		data: vec![]
-	}.sign(&secret);
-
-	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#;
-
-	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
-}
-
 #[test]
 #[ignore]
 fn rpc_eth_send_raw_transaction() {

rpc/src/v1/tests/mocked/personal.rs

@@ -15,11 +15,29 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::sync::Arc;
+use std::str::FromStr;
+use std::collections::HashMap;
 use jsonrpc_core::IoHandler;
 use util::numbers::*;
 use util::keys::{TestAccount, TestAccountProvider};
 use v1::{PersonalClient, Personal};
-use std::collections::*;
+use v1::tests::helpers::TestMinerService;
+use ethcore::client::TestBlockChainClient;
+use ethcore::transaction::{Action, Transaction};
+
+struct PersonalTester {
+	accounts: Arc<TestAccountProvider>,
+	io: IoHandler,
+	miner: Arc<TestMinerService>,
+	// these unused fields are necessary to keep the data alive
+	// as the handler has only weak pointers.
+	_client: Arc<TestBlockChainClient>,
+}
+
+fn blockchain_client() -> Arc<TestBlockChainClient> {
+	let client = TestBlockChainClient::new();
+	Arc::new(client)
+}
 
 fn accounts_provider() -> Arc<TestAccountProvider> {
 	let accounts = HashMap::new();
@@ -27,18 +45,33 @@ fn accounts_provider() -> Arc<TestAccountProvider> {
 	Arc::new(ap)
 }
 
-fn setup() -> (Arc<TestAccountProvider>, IoHandler) {
+fn miner_service() -> Arc<TestMinerService> {
-	let test_provider = accounts_provider();
+	Arc::new(TestMinerService::default())
-	let personal = PersonalClient::new(&test_provider);
+}
+
+fn setup() -> PersonalTester {
+	let accounts = accounts_provider();
+	let client = blockchain_client();
+	let miner = miner_service();
+	let personal = PersonalClient::new(&accounts, &client, &miner);
+
 	let io = IoHandler::new();
 	io.add_delegate(personal.to_delegate());
-	(test_provider, io)
+
+	let tester = PersonalTester {
+		accounts: accounts,
+		io: io,
+		miner: miner,
+		_client: client,
+	};
+
+	tester
 }
 
 #[test]
 fn accounts() {
-	let (test_provider, io) = setup();
+	let tester = setup();
-	test_provider.accounts
+	tester.accounts.accounts
 		.write()
 		.unwrap()
 		.insert(Address::from(1), TestAccount::new("test"));
@@ -46,17 +79,17 @@ fn accounts() {
 	let request = r#"{"jsonrpc": "2.0", "method": "personal_listAccounts", "params": [], "id": 1}"#;
 	let response = r#"{"jsonrpc":"2.0","result":["0x0000000000000000000000000000000000000001"],"id":1}"#;
 
-	assert_eq!(io.handle_request(request), Some(response.to_owned()));
+	assert_eq!(tester.io.handle_request(request), Some(response.to_owned()));
 }
 
 #[test]
 fn new_account() {
-	let (test_provider, io) = setup();
+	let tester = setup();
 	let request = r#"{"jsonrpc": "2.0", "method": "personal_newAccount", "params": ["pass"], "id": 1}"#;
 
-	let res = io.handle_request(request);
+	let res = tester.io.handle_request(request);
 
-	let accounts = test_provider.accounts.read().unwrap();
+	let accounts = tester.accounts.accounts.read().unwrap();
 	assert_eq!(accounts.len(), 1);
 
 	let address = accounts
@@ -70,3 +103,77 @@ fn new_account() {
 	assert_eq!(res, Some(response));
 }
 
+#[test]
+fn sign_and_send_transaction_with_invalid_password() {
+	let account = TestAccount::new("password123");
+	let address = account.address();
+
+	let tester = setup();
+	tester.accounts.accounts.write().unwrap().insert(address.clone(), account);
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "personal_signAndSendTransaction",
+		"params": [{
+			"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
+			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
+			"gas": "0x76c0",
+			"gasPrice": "0x9184e72a000",
+			"value": "0x9184e72a"
+		}, "password321"],
+		"id": 1
+	}"#;
+
+	let response = r#"{"jsonrpc":"2.0","result":"0x0000000000000000000000000000000000000000000000000000000000000000","id":1}"#;
+
+	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response.into()));
+}
+
+#[test]
+fn sign_and_send_transaction() {
+	let account = TestAccount::new("password123");
+	let address = account.address();
+	let secret = account.secret.clone();
+
+	let tester = setup();
+	tester.accounts.accounts.write().unwrap().insert(address.clone(), account);
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "personal_signAndSendTransaction",
+		"params": [{
+			"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
+			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
+			"gas": "0x76c0",
+			"gasPrice": "0x9184e72a000",
+			"value": "0x9184e72a"
+		}, "password123"],
+		"id": 1
+	}"#;
+
+	let t = Transaction {
+		nonce: U256::zero(),
+		gas_price: U256::from(0x9184e72a000u64),
+		gas: U256::from(0x76c0),
+		action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
+		value: U256::from(0x9184e72au64),
+		data: vec![]
+	}.sign(&secret);
+
+	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#;
+
+	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
+
+	tester.miner.last_nonces.write().unwrap().insert(address.clone(), U256::zero());
+
+	let t = Transaction {
+		nonce: U256::one(),
+		gas_price: U256::from(0x9184e72a000u64),
+		gas: U256::from(0x76c0),
+		action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
+		value: U256::from(0x9184e72au64),
+		data: vec![]
+	}.sign(&secret);
+
+	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#;
+
+	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
+}

rpc/src/v1/traits/eth.rs

@@ -80,9 +80,6 @@ pub trait Eth: Sized + Send + Sync + 'static {
 	/// Sends transaction.
 	fn send_transaction(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
 
-	/// Sends transaction and signs it in single call. The account is not unlocked in such case.
-	fn sign_and_send_transaction(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
-
 	/// Sends signed transaction.
 	fn send_raw_transaction(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
 
@@ -155,7 +152,6 @@ pub trait Eth: Sized + Send + Sync + 'static {
 		delegate.add_method("eth_getCode", Eth::code_at);
 		delegate.add_method("eth_sign", Eth::sign);
 		delegate.add_method("eth_sendTransaction", Eth::send_transaction);
-		delegate.add_method("eth_signAndSendTransaction", Eth::sign_and_send_transaction);
 		delegate.add_method("eth_sendRawTransaction", Eth::send_raw_transaction);
 		delegate.add_method("eth_call", Eth::call);
 		delegate.add_method("eth_estimateGas", Eth::estimate_gas);

rpc/src/v1/traits/personal.rs

@@ -30,12 +30,16 @@ pub trait Personal: Sized + Send + Sync + 'static {
 	/// Unlocks specified account for use (can only be one unlocked account at one moment)
 	fn unlock_account(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
 
+	/// Sends transaction and signs it in single call. The account is not unlocked in such case.
+	fn sign_and_send_transaction(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
+
 	/// Should be used to convert object to io delegate.
 	fn to_delegate(self) -> IoDelegate<Self> {
 		let mut delegate = IoDelegate::new(Arc::new(self));
 		delegate.add_method("personal_listAccounts", Personal::accounts);
 		delegate.add_method("personal_newAccount", Personal::new_account);
 		delegate.add_method("personal_unlockAccount", Personal::unlock_account);
+		delegate.add_method("personal_signAndSendTransaction", Personal::sign_and_send_transaction);
 		delegate
 	}
 }

rpc/src/v1/types/bytes.rs

@@ -28,7 +28,7 @@ impl Bytes {
 	pub fn new(bytes: Vec<u8>) -> Bytes {
 		Bytes(bytes)
 	}
-	pub fn to_vec(self) -> Vec<u8> { let Bytes(x) = self; x }
+	pub fn to_vec(self) -> Vec<u8> { self.0 }
 }
 
 impl Serialize for Bytes {