eth_checkTransaction renamed to eth_checkRequest (#1817)

* Making ConfirmationsQueue a bit more generic [WiP]

* Generalizing cofirmations

* New confirmations types - tests

* Separating transaction type in queue. Closes #1310

* Handling sign requests

* Speeding up tests

* Renaming methods

* eth_postSign

* Bumping ui

* Renaming checkRequest methods, adding tests

* Removing duplicate method [ci skip]

* Remove `_posted`

[ci:skip]

* Remove `_posted`

rpc/src/v1/impls/eth_signing.rs

@@ -26,7 +26,7 @@ use ethcore::account_provider::AccountProvider;
 use v1::helpers::{SigningQueue, ConfirmationPromise, ConfirmationResult, ConfirmationsQueue, ConfirmationPayload, TransactionRequest as TRequest, FilledTransactionRequest as FilledRequest};
 use v1::traits::EthSigning;
 use v1::types::{TransactionRequest, H160 as RpcH160, H256 as RpcH256, H520 as RpcH520, U256 as RpcU256};
-use v1::impls::{default_gas_price, sign_and_dispatch, transaction_rejected_error, signer_disabled_error};
+use v1::impls::{default_gas_price, sign_and_dispatch, request_rejected_error, request_not_found_error, signer_disabled_error};
 
 fn fill_optional_fields<C, M>(request: TRequest, client: &C, miner: &M) -> FilledRequest
 	where C: MiningBlockChainClient, M: MinerService {
@@ -143,7 +143,7 @@ impl<C, M> EthSigning for EthSigningQueueClient<C, M>
 		})
 	}
 
-	fn check_transaction(&self, params: Params) -> Result<Value, Error> {
+	fn check_request(&self, params: Params) -> Result<Value, Error> {
 		try!(self.active());
 		let mut pending = self.pending.lock();
 		from_params::<(RpcU256, )>(params).and_then(|(id, )| {
@@ -151,10 +151,10 @@ impl<C, M> EthSigning for EthSigningQueueClient<C, M>
 			let res = match pending.get(&id) {
 				Some(ref promise) => match promise.result() {
 					ConfirmationResult::Waiting => { return Ok(Value::Null); }
-					ConfirmationResult::Rejected => Err(transaction_rejected_error()),
+					ConfirmationResult::Rejected => Err(request_rejected_error()),
 					ConfirmationResult::Confirmed(rpc_response) => rpc_response,
 				},
-				_ => { return Err(Error::invalid_params()); }
+				_ => { return Err(request_not_found_error()); }
 			};
 			pending.remove(&id);
 			res
@@ -225,7 +225,7 @@ impl<C, M> EthSigning for EthSigningUnsafeClient<C, M> where
 		Err(signer_disabled_error())
 	}
 
-	fn check_transaction(&self, _: Params) -> Result<Value, Error> {
+	fn check_request(&self, _: Params) -> Result<Value, Error> {
 		// We don't support this in non-signer mode.
 		Err(signer_disabled_error())
 	}

rpc/src/v1/impls/mod.rs

@@ -72,10 +72,11 @@ mod error_codes {
 	pub const NO_AUTHOR_CODE: i64 = -32002;
 	pub const UNKNOWN_ERROR: i64 = -32009;
 	pub const TRANSACTION_ERROR: i64 = -32010;
-	pub const TRANSACTION_REJECTED: i64 = -32011;
 	pub const ACCOUNT_LOCKED: i64 = -32020;
 	pub const PASSWORD_INVALID: i64 = -32021;
 	pub const SIGNER_DISABLED: i64 = -32030;
+	pub const REQUEST_REJECTED: i64 = -32040;
+	pub const REQUEST_NOT_FOUND: i64 = -32041;
 }
 
 fn dispatch_transaction<C, M>(client: &C, miner: &M, signed_transaction: SignedTransaction) -> Result<Value, Error>
@@ -171,11 +172,20 @@ fn password_error(error: AccountError) -> Error {
 	}
 }
 
-/// Error returned when transaction is rejected (in Trusted Signer).
-pub fn transaction_rejected_error() -> Error {
+/// Error returned when request is rejected (in Trusted Signer).
+pub fn request_rejected_error() -> Error {
 	Error {
-		code: ErrorCode::ServerError(error_codes::TRANSACTION_REJECTED),
-		message: "Transaction has been rejected.".into(),
+		code: ErrorCode::ServerError(error_codes::REQUEST_REJECTED),
+		message: "Request has been rejected.".into(),
+		data: None,
+	}
+}
+
+/// Error returned when request is not found in queue.
+pub fn request_not_found_error() -> Error {
+	Error {
+		code: ErrorCode::ServerError(error_codes::REQUEST_NOT_FOUND),
+		message: "Request not found.".into(),
 		data: None,
 	}
 }

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

@@ -17,7 +17,7 @@
 use std::str::FromStr;
 use std::sync::Arc;
 use std::time::Duration;
-use jsonrpc_core::IoHandler;
+use jsonrpc_core::{IoHandler, to_value};
 use v1::impls::EthSigningQueueClient;
 use v1::traits::EthSigning;
 use v1::helpers::{ConfirmationsQueue, SigningQueue};
@@ -107,6 +107,65 @@ fn should_post_sign_to_queue() {
 	assert_eq!(tester.queue.requests().len(), 1);
 }
 
+#[test]
+fn should_check_status_of_request() {
+	// given
+	let tester = eth_signing();
+	let address = Address::random();
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_postSign",
+		"params": [
+			""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
+			"0x0000000000000000000000000000000000000000000000000000000000000005"
+		],
+		"id": 1
+	}"#;
+	tester.io.handle_request(&request).expect("Sent");
+
+	// when
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_checkRequest",
+		"params": ["0x1"],
+		"id": 1
+	}"#;
+	let response = r#"{"jsonrpc":"2.0","result":null,"id":1}"#;
+
+	// then
+	assert_eq!(tester.io.handle_request(&request), Some(response.to_owned()));
+}
+
+#[test]
+fn should_check_status_of_request_when_its_resolved() {
+	// given
+	let tester = eth_signing();
+	let address = Address::random();
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_postSign",
+		"params": [
+			""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
+			"0x0000000000000000000000000000000000000000000000000000000000000005"
+		],
+		"id": 1
+	}"#;
+	tester.io.handle_request(&request).expect("Sent");
+	tester.queue.request_confirmed(U256::from(1), to_value(&"Hello World!"));
+
+	// when
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_checkRequest",
+		"params": ["0x1"],
+		"id": 1
+	}"#;
+	let response = r#"{"jsonrpc":"2.0","result":"Hello World!","id":1}"#;
+
+	// then
+	assert_eq!(tester.io.handle_request(&request), Some(response.to_owned()));
+}
+
 #[test]
 fn should_sign_if_account_is_unlocked() {
 	// given

rpc/src/v1/traits/eth.rs

@@ -220,20 +220,21 @@ pub trait EthSigning: Sized + Send + Sync + 'static {
 	/// Will return a transaction ID for later use with check_transaction.
 	fn post_transaction(&self, _: Params) -> Result<Value, Error>;
 
-	/// Checks the progress of a previously posted transaction.
+	/// Checks the progress of a previously posted request (transaction/sign).
 	/// Should be given a valid send_transaction ID.
 	/// Returns the transaction hash, the zero hash (not yet available),
+	/// or the signature,
 	/// or an error.
-	fn check_transaction(&self, _: Params) -> Result<Value, Error>;
+	fn check_request(&self, _: Params) -> Result<Value, Error>;
 
 	/// 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("eth_sign", EthSigning::sign);
-		delegate.add_method("eth_postSign", EthSigning::post_sign);
 		delegate.add_method("eth_sendTransaction", EthSigning::send_transaction);
+		delegate.add_method("eth_postSign", EthSigning::post_sign);
 		delegate.add_method("eth_postTransaction", EthSigning::post_transaction);
-		delegate.add_method("eth_checkTransaction", EthSigning::check_transaction);
+		delegate.add_method("eth_checkRequest", EthSigning::check_request);
 		delegate
 	}
 }