Merge pull request #1154 from rphmeier/personal_sign_and_send

Change eth_signAndSendTransaction to personal_SignAndSendTransaction

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() {