From 979f4e0617bbb15d8c6c3d454848afd382cdfff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 4 Aug 2016 16:42:29 +0200 Subject: [PATCH] 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 | 10 ++--- rpc/src/v1/impls/mod.rs | 20 ++++++--- rpc/src/v1/tests/mocked/eth_signing.rs | 61 +++++++++++++++++++++++++- rpc/src/v1/traits/eth.rs | 9 ++-- 4 files changed, 85 insertions(+), 15 deletions(-) diff --git a/rpc/src/v1/impls/eth_signing.rs b/rpc/src/v1/impls/eth_signing.rs index c4eb187fe..8cedd865a 100644 --- a/rpc/src/v1/impls/eth_signing.rs +++ b/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(request: TRequest, client: &C, miner: &M) -> FilledRequest where C: MiningBlockChainClient, M: MinerService { @@ -143,7 +143,7 @@ impl EthSigning for EthSigningQueueClient }) } - fn check_transaction(&self, params: Params) -> Result { + fn check_request(&self, params: Params) -> Result { try!(self.active()); let mut pending = self.pending.lock(); from_params::<(RpcU256, )>(params).and_then(|(id, )| { @@ -151,10 +151,10 @@ impl EthSigning for EthSigningQueueClient 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 EthSigning for EthSigningUnsafeClient where Err(signer_disabled_error()) } - fn check_transaction(&self, _: Params) -> Result { + fn check_request(&self, _: Params) -> Result { // We don't support this in non-signer mode. Err(signer_disabled_error()) } diff --git a/rpc/src/v1/impls/mod.rs b/rpc/src/v1/impls/mod.rs index a9dddf843..f0eb4a301 100644 --- a/rpc/src/v1/impls/mod.rs +++ b/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(client: &C, miner: &M, signed_transaction: SignedTransaction) -> Result @@ -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, } } diff --git a/rpc/src/v1/tests/mocked/eth_signing.rs b/rpc/src/v1/tests/mocked/eth_signing.rs index 69a21cab5..5acc935b9 100644 --- a/rpc/src/v1/tests/mocked/eth_signing.rs +++ b/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 diff --git a/rpc/src/v1/traits/eth.rs b/rpc/src/v1/traits/eth.rs index 72dc17962..60b482916 100644 --- a/rpc/src/v1/traits/eth.rs +++ b/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; - /// 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; + fn check_request(&self, _: Params) -> Result; /// Should be used to convert object to io delegate. fn to_delegate(self) -> IoDelegate { 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 } }