Merge pull request #1124 from ethcore/sign-and-send

Sign and send transaction

Cargo.lock

@@ -357,7 +357,7 @@ dependencies = [
  "jsonrpc-http-server 5.1.0 (git+https://github.com/ethcore/jsonrpc-http-server.git)",
  "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-daodapp 0.2.1 (git+https://github.com/ethcore/parity-daodapp-rs.git)",
- "parity-idmanager 0.2.2 (git+https://github.com/ethcore/parity-idmanager-rs.git)",
+ "parity-idmanager 0.3.2 (git+https://github.com/ethcore/parity-idmanager-rs.git)",
  "parity-makerotc 0.1.3 (git+https://github.com/ethcore/parity-makerotc-rs.git)",
  "parity-status 0.4.3 (git+https://github.com/ethcore/parity-status.git)",
  "parity-wallet 0.4.1 (git+https://github.com/ethcore/parity-wallet.git)",
@@ -846,8 +846,8 @@ dependencies = [
 
 [[package]]
 name = "parity-idmanager"
-version = "0.2.2"
+version = "0.3.2"
-source = "git+https://github.com/ethcore/parity-idmanager-rs.git#19dd79ca7b7afb8824ad072b43ca63babe2ba9bc"
+source = "git+https://github.com/ethcore/parity-idmanager-rs.git#5bc85d456455f7b9f6f257021dab9394de9e66b4"
 dependencies = [
  "parity-webapp 0.2.0 (git+https://github.com/ethcore/parity-webapp.git)",
 ]
@@ -863,7 +863,7 @@ dependencies = [
 [[package]]
 name = "parity-status"
 version = "0.4.3"
-source = "git+https://github.com/ethcore/parity-status.git#1d383d74010f6ebcd712b60b8fc5ff547b44f4e5"
+source = "git+https://github.com/ethcore/parity-status.git#606dfe6d272792f2fdb90909318f2439d58330a5"
 dependencies = [
  "parity-webapp 0.2.0 (git+https://github.com/ethcore/parity-webapp.git)",
 ]

rpc/src/v1/impls/eth.rs

@@ -47,7 +47,7 @@ pub struct EthClient<C, S, A, M, EM> where
 	A: AccountProvider,
 	M: MinerService,
 	EM: ExternalMinerService {
-		
+
 	client: Weak<C>,
 	sync: Weak<S>,
 	accounts: Weak<A>,
@@ -153,6 +153,27 @@ 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.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);
@@ -483,27 +504,19 @@ 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) => {
+					Ok(secret) => self.sign_and_dispatch(request, secret),
-						let signed_transaction = {
+					Err(_) => to_value(&H256::zero())
-							let client = take_weak!(self.client);
+				}
-							let miner = take_weak!(self.miner);
+		})
-							EthTransaction {
+	}
-								nonce: request.nonce
+
-									.or_else(|| miner
+	fn sign_and_send_transaction(&self, params: Params) -> Result<Value, Error> {
-											 .last_nonce(&request.from)
+		from_params::<(TransactionRequest, String)>(params)
-											 .map(|nonce| nonce + U256::one()))
+			.and_then(|(request, password)| {
-									.unwrap_or_else(|| client.nonce(&request.from)),
+				let accounts = take_weak!(self.accounts);
-								action: request.to.map_or(Action::Create, Action::Call),
+				match accounts.locked_account_secret(&request.from, &password) {
-								gas: request.gas.unwrap_or_else(|| miner.sensible_gas_limit()),
+					Ok(secret) => self.sign_and_dispatch(request, secret),
-								gas_price: request.gas_price.unwrap_or_else(|| miner.sensible_gas_price()),
+					Err(_) => to_value(&H256::zero()),
-								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)
-					},
-					Err(_) => { to_value(&H256::zero()) }
 				}
 		})
 	}

rpc/src/v1/tests/eth.rs

@@ -521,6 +521,81 @@ 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/traits/eth.rs

@@ -80,6 +80,9 @@ 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!() }
 
@@ -152,6 +155,7 @@ 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);

util/src/keys/store.rs

@@ -58,13 +58,15 @@ pub enum EncryptedHashMapError {
 	InvalidValueFormat(FromBytesError),
 }
 
-/// Error retrieving value from encrypted hashmap
+/// Error while signing a message
 #[derive(Debug)]
 pub enum SigningError {
 	/// Account passed does not exist
 	NoAccount,
 	/// Account passed is not unlocked
 	AccountNotUnlocked,
+	/// Invalid passphrase
+	InvalidPassword,
 	/// Invalid secret in store
 	InvalidSecret
 }
@@ -96,6 +98,8 @@ pub trait AccountProvider : Send + Sync {
 	fn new_account(&self, pass: &str) -> Result<Address, ::std::io::Error>;
 	/// Returns secret for unlocked `account`.
 	fn account_secret(&self, account: &Address) -> Result<crypto::Secret, SigningError>;
+	/// Returns secret for locked account given passphrase.
+	fn locked_account_secret(&self, account: &Address, pass: &str) -> Result<crypto::Secret, SigningError>;
 	/// Returns signature when unlocked `account` signs `message`.
 	fn sign(&self, account: &Address, message: &H256) -> Result<crypto::Signature, SigningError> {
 		self.account_secret(account).and_then(|s| crypto::ec::sign(&s, message).map_err(|_| SigningError::InvalidSecret))
@@ -127,7 +131,11 @@ impl AccountProvider for AccountService {
 	fn account_secret(&self, account: &Address) -> Result<crypto::Secret, SigningError> {
 		self.secret_store.read().unwrap().account_secret(account)
 	}
-	/// Returns secret for unlocked account
+	/// Returns secret for locked account given passphrase.
+	fn locked_account_secret(&self, account: &Address, pass: &str) -> Result<crypto::Secret, SigningError> {
+		self.secret_store.read().unwrap().locked_account_secret(account, pass)
+	}
+	/// Signs a message using key of given unlocked account address.
 	fn sign(&self, account: &Address, message: &H256) -> Result<crypto::Signature, SigningError> {
 		self.secret_store.read().unwrap().sign(account, message)
 	}
@@ -317,6 +325,16 @@ impl SecretStore {
 		ret
 	}
 
+	/// Returns secret for unlocked account.
+	pub fn locked_account_secret(&self, account: &Address, pass: &str) -> Result<crypto::Secret, SigningError> {
+		let secret_id = try!(self.account(&account).ok_or(SigningError::NoAccount));
+		self.get(&secret_id, pass).or_else(|e| Err(match e {
+			EncryptedHashMapError::InvalidPassword => SigningError::InvalidPassword,
+			EncryptedHashMapError::UnknownIdentifier => SigningError::NoAccount,
+			EncryptedHashMapError::InvalidValueFormat(_) => SigningError::InvalidSecret,
+		}))
+	}
+
 	/// Makes account unlocks expire and removes unused key files from memory
 	pub fn collect_garbage(&mut self) {
 		let mut garbage_lock = self.unlocks.write().unwrap();
@@ -679,6 +697,29 @@ mod tests {
 		assert_eq!(Address::from(kp.public().sha3()), addr);
 	}
 
+
+	#[test]
+	fn secret_for_locked_account() {
+		// given
+		let temp = RandomTempPath::create_dir();
+		let mut sstore = SecretStore::new_test(&temp);
+		let addr = sstore.new_account("test-pass").unwrap();
+
+		// when
+		// Invalid pass
+		let secret1 = sstore.locked_account_secret(&addr, "test-pass123");
+		// Valid pass
+		let secret2 = sstore.locked_account_secret(&addr, "test-pass");
+		// Account not unlocked
+		let secret3 = sstore.account_secret(&addr);
+
+
+		assert!(secret1.is_err(), "Invalid password should not return secret.");
+		assert!(secret2.is_ok(), "Should return secret provided valid passphrase.");
+		assert!(secret3.is_err(), "Account should still be locked.");
+	}
+
+
 	#[test]
 	fn can_create_service() {
 		let temp = RandomTempPath::create_dir();

util/src/keys/test_account_provider.rs

@@ -103,5 +103,16 @@ impl AccountProvider for TestAccountProvider {
 			.ok_or(SigningError::NoAccount)
 			.map(|acc| acc.secret.clone())
 	}
+
+	fn locked_account_secret(&self, address: &Address, pass: &str) -> Result<Secret, SigningError> {
+		let accounts = self.accounts.read().unwrap();
+		match accounts.get(address) {
+			Some(ref acc) if acc.password == pass => {
+				Ok(acc.secret.clone())
+			},
+			Some(ref _acc) => Err(SigningError::InvalidPassword),
+			_ => Err(SigningError::NoAccount),
+		}
+	}
 }
 

webapp/Cargo.toml

@@ -23,7 +23,7 @@ ethcore-util = { path = "../util" }
 parity-webapp = { git = "https://github.com/ethcore/parity-webapp.git", version = "0.2" }
 # List of apps
 parity-status = { git = "https://github.com/ethcore/parity-status.git", version = "0.4.3" }
-parity-idmanager = { git = "https://github.com/ethcore/parity-idmanager-rs.git", version = "0.2.2" }
+parity-idmanager = { git = "https://github.com/ethcore/parity-idmanager-rs.git", version = "0.3.2" }
 parity-wallet = { git = "https://github.com/ethcore/parity-wallet.git", version = "0.4.1", optional = true }
 parity-daodapp = { git = "https://github.com/ethcore/parity-daodapp-rs.git", version = "0.2.1", optional = true }
 parity-makerotc = { git = "https://github.com/ethcore/parity-makerotc-rs.git", version = "0.1.3", optional = true }