From efa8f661e558d77fa3f0f3f528f3718c84abf474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 15 Jun 2016 00:17:23 +0200 Subject: [PATCH] More meaningful errors when sending transaction --- Cargo.lock | 42 +++++++++++----------- rpc/src/v1/helpers/signing_queue.rs | 30 +++++++++------- rpc/src/v1/impls/eth_signing.rs | 22 ++++++++---- rpc/src/v1/impls/mod.rs | 55 +++++++++++++++++++++++++++-- rpc/src/v1/impls/personal_signer.rs | 6 ++-- 5 files changed, 109 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c850940c9..5b8be3cc8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -71,7 +71,7 @@ dependencies = [ "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -233,7 +233,7 @@ dependencies = [ "libc 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -279,7 +279,7 @@ dependencies = [ "ethcore-rpc 1.2.0", "ethcore-util 1.2.0", "hyper 0.9.3 (git+https://github.com/ethcore/hyper)", - "jsonrpc-core 2.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 2.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "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)", "mime_guess 1.6.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -290,8 +290,8 @@ dependencies = [ "parity-dapps-status 0.5.0 (git+https://github.com/ethcore/parity-dapps-status-rs.git)", "parity-dapps-wallet 0.6.1 (git+https://github.com/ethcore/parity-dapps-wallet-rs.git)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_codegen 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_codegen 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "syntex 0.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -330,7 +330,7 @@ name = "ethcore-ipc-nano" version = "1.2.0" dependencies = [ "ethcore-ipc 1.2.0", - "jsonrpc-core 2.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 2.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "nanomsg 0.5.1 (git+https://github.com/ethcore/nanomsg.rs.git)", ] @@ -347,12 +347,12 @@ dependencies = [ "ethjson 0.1.0", "ethsync 1.2.0", "json-ipc-server 0.2.2 (git+https://github.com/ethcore/json-ipc-server.git)", - "jsonrpc-core 2.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 2.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "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)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_codegen 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_codegen 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "syntex 0.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "transient-hashmap 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -366,7 +366,7 @@ dependencies = [ "env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore-rpc 1.2.0", "ethcore-util 1.2.0", - "jsonrpc-core 2.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 2.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-minimal-sysui 0.1.0 (git+https://github.com/ethcore/parity-dapps-minimal-sysui-rs.git)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", @@ -401,7 +401,7 @@ dependencies = [ "rust-crypto 0.2.35 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "sha3 0.1.0", "slab 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "target_info 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -416,8 +416,8 @@ version = "0.1.0" dependencies = [ "ethcore-util 1.2.0", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_codegen 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_codegen 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "syntex 0.33.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -575,7 +575,7 @@ source = "git+https://github.com/ethcore/json-ipc-server.git#15ef25e5f859d2d2746 dependencies = [ "bytes 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", - "jsonrpc-core 2.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 2.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "mio 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -593,11 +593,11 @@ dependencies = [ [[package]] name = "jsonrpc-core" -version = "2.0.5" +version = "2.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_codegen 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_codegen 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "syntex 0.33.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -608,7 +608,7 @@ version = "5.1.0" source = "git+https://github.com/ethcore/jsonrpc-http-server.git#6117b1d77b5a60d6fa2dc884f12aa7f5fd4585ca" dependencies = [ "hyper 0.9.3 (git+https://github.com/ethcore/hyper)", - "jsonrpc-core 2.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-core 2.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1185,12 +1185,12 @@ dependencies = [ [[package]] name = "serde" -version = "0.7.7" +version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "serde_codegen" -version = "0.7.7" +version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "aster 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1206,7 +1206,7 @@ version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/rpc/src/v1/helpers/signing_queue.rs b/rpc/src/v1/helpers/signing_queue.rs index 0ded8998c..dd59ce967 100644 --- a/rpc/src/v1/helpers/signing_queue.rs +++ b/rpc/src/v1/helpers/signing_queue.rs @@ -19,8 +19,11 @@ use std::time::{Instant, Duration}; use std::sync::{mpsc, Mutex, RwLock, Arc}; use std::collections::HashMap; use v1::types::{TransactionRequest, TransactionConfirmation}; -use util::{U256, H256}; +use util::U256; +use jsonrpc_core; +/// Result that can be returned from JSON RPC. +pub type RpcResult = Result; /// Possible events happening in the queue that can be listened to. #[derive(Debug, PartialEq)] @@ -59,7 +62,7 @@ pub trait SigningQueue: Send + Sync { /// Removes a request from the queue. /// Notifies possible token holders that transaction was confirmed and given hash was assigned. - fn request_confirmed(&self, id: U256, hash: H256) -> Option; + fn request_confirmed(&self, id: U256, result: RpcResult) -> Option; /// Returns a request if it is contained in the queue. fn peek(&self, id: &U256) -> Option; @@ -75,7 +78,7 @@ enum ConfirmationResult { /// The transaction has been rejected. Rejected, /// The transaction has been confirmed. - Confirmed(H256), + Confirmed(RpcResult), } /// Time you need to confirm the transaction in UI. @@ -100,7 +103,7 @@ pub struct ConfirmationPromise { impl ConfirmationToken { /// Submit solution to all listeners - fn resolve(&self, result: Option) { + fn resolve(&self, result: Option) { let mut res = self.result.lock().unwrap(); *res = result.map_or(ConfirmationResult::Rejected, |h| ConfirmationResult::Confirmed(h)); // Notify listener @@ -119,8 +122,8 @@ impl ConfirmationPromise { /// Blocks current thread and awaits for /// resolution of the transaction (rejected / confirmed) /// Returns `None` if transaction was rejected or timeout reached. - /// Returns `Some(hash)` if transaction was confirmed. - pub fn wait_with_timeout(&self) -> Option { + /// Returns `Some(result)` if transaction was confirmed. + pub fn wait_with_timeout(&self) -> Option { let timeout = Duration::from_secs(QUEUE_TIMEOUT_DURATION_SEC); let deadline = Instant::now() + timeout; @@ -137,7 +140,7 @@ impl ConfirmationPromise { // Check the result match *res { ConfirmationResult::Rejected => return None, - ConfirmationResult::Confirmed(h) => return Some(h), + ConfirmationResult::Confirmed(ref h) => return Some(h.clone()), ConfirmationResult::Waiting => continue, } } @@ -204,12 +207,12 @@ impl ConfirmationsQueue { /// Removes transaction from this queue and notifies `ConfirmationPromise` holders about the result. /// Notifies also a receiver about that event. - fn remove(&self, id: U256, result: Option) -> Option { + fn remove(&self, id: U256, result: Option) -> Option { let token = self.queue.write().unwrap().remove(&id); if let Some(token) = token { // notify receiver about the event - self.notify(result.map_or_else( + self.notify(result.clone().map_or_else( || QueueEvent::RequestRejected(id), |_| QueueEvent::RequestConfirmed(id) )); @@ -265,9 +268,9 @@ impl SigningQueue for ConfirmationsQueue { self.remove(id, None) } - fn request_confirmed(&self, id: U256, hash: H256) -> Option { + fn request_confirmed(&self, id: U256, result: RpcResult) -> Option { debug!(target: "own_tx", "Signer: Transaction confirmed ({:?}).", id); - self.remove(id, Some(hash)) + self.remove(id, Some(result)) } fn requests(&self) -> Vec { @@ -286,6 +289,7 @@ mod test { use util::numbers::{U256, H256}; use v1::types::TransactionRequest; use super::*; + use jsonrpc_core::to_value; fn request() -> TransactionRequest { TransactionRequest { @@ -317,10 +321,10 @@ mod test { // Just wait for the other thread to start thread::sleep(Duration::from_millis(100)); } - queue.request_confirmed(id, H256::from(1)); + queue.request_confirmed(id, to_value(&H256::from(1))); // then - assert_eq!(handle.join().expect("Thread should finish nicely"), H256::from(1)); + assert_eq!(handle.join().expect("Thread should finish nicely"), to_value(&H256::from(1))); } #[test] diff --git a/rpc/src/v1/impls/eth_signing.rs b/rpc/src/v1/impls/eth_signing.rs index f8c3c343d..591eae059 100644 --- a/rpc/src/v1/impls/eth_signing.rs +++ b/rpc/src/v1/impls/eth_signing.rs @@ -25,7 +25,7 @@ use util::keys::store::AccountProvider; use v1::helpers::{SigningQueue, ConfirmationsQueue}; use v1::traits::EthSigning; use v1::types::TransactionRequest; -use v1::impls::sign_and_dispatch; +use v1::impls::{sign_and_dispatch, error_codes}; /// Implementation of functions that require signing when no trusted signer is used. @@ -45,6 +45,7 @@ impl EthSigningQueueClient { impl EthSigning for EthSigningQueueClient { fn sign(&self, _params: Params) -> Result { + warn!("Invoking eth_sign is not yet supported with signer enabled."); // TODO [ToDr] Implement sign when rest of the signing queue is ready. rpc_unimplemented!() } @@ -55,7 +56,7 @@ impl EthSigning for EthSigningQueueClient { let queue = take_weak!(self.queue); let id = queue.add_request(request); let result = id.wait_with_timeout(); - to_value(&result.unwrap_or_else(H256::new)) + result.unwrap_or_else(|| to_value(&H256::new())) }) } } @@ -93,7 +94,9 @@ impl EthSigning for EthSigningUnsafeClient where fn sign(&self, params: Params) -> Result { from_params::<(Address, H256)>(params).and_then(|(addr, msg)| { - to_value(&take_weak!(self.accounts).sign(&addr, &msg).unwrap_or(H520::zero())) + take_weak!(self.accounts).sign(&addr, &msg) + .map(|v| to_value(&v)) + .unwrap_or_else(|e| Err(account_locked(format!("Error: {:?}", e)))) }) } @@ -102,10 +105,17 @@ impl EthSigning for EthSigningUnsafeClient where .and_then(|(request, )| { let accounts = take_weak!(self.accounts); match accounts.account_secret(&request.from) { - Ok(secret) => to_value(&sign_and_dispatch(&*take_weak!(self.client), &*take_weak!(self.miner), request, secret)), - Err(_) => to_value(&H256::zero()) + Ok(secret) => sign_and_dispatch(&*take_weak!(self.client), &*take_weak!(self.miner), request, secret), + Err(e) => Err(account_locked(format!("Error: {:?}", e))), } }) } - +} + +fn account_locked(data: String) -> Error { + Error { + code: ErrorCode::ServerError(error_codes::ACCOUNT_LOCKED), + message: "Your account is locked. Unlock the account via CLI, personal_unlockAccount or use Trusted Signer.".into(), + data: Some(Value::String(data)), + } } diff --git a/rpc/src/v1/impls/mod.rs b/rpc/src/v1/impls/mod.rs index 7fdf57249..03f4b34ed 100644 --- a/rpc/src/v1/impls/mod.rs +++ b/rpc/src/v1/impls/mod.rs @@ -54,14 +54,16 @@ pub use self::traces::TracesClient; pub use self::rpc::RpcClient; use v1::types::TransactionRequest; +use ethcore::error::Error as EthcoreError; use ethcore::miner::{AccountDetails, MinerService}; use ethcore::client::MiningBlockChainClient; use ethcore::transaction::{Action, SignedTransaction, Transaction}; use util::numbers::*; use util::rlp::encode; use util::bytes::ToPretty; +use jsonrpc_core::{Error, ErrorCode, Value, to_value}; -fn dispatch_transaction(client: &C, miner: &M, signed_transaction: SignedTransaction) -> H256 +fn dispatch_transaction(client: &C, miner: &M, signed_transaction: SignedTransaction) -> Result where C: MiningBlockChainClient, M: MinerService { let hash = signed_transaction.hash(); @@ -72,10 +74,12 @@ fn dispatch_transaction(client: &C, miner: &M, signed_transaction: SignedT } }); - import.map(|_| hash).unwrap_or(H256::zero()) + import + .map_err(transaction_error) + .and_then(|_| to_value(&hash)) } -fn sign_and_dispatch(client: &C, miner: &M, request: TransactionRequest, secret: H256) -> H256 +fn sign_and_dispatch(client: &C, miner: &M, request: TransactionRequest, secret: H256) -> Result where C: MiningBlockChainClient, M: MinerService { let signed_transaction = { @@ -97,3 +101,48 @@ fn sign_and_dispatch(client: &C, miner: &M, request: TransactionRequest, s trace!(target: "miner", "send_transaction: dispatching tx: {}", encode(&signed_transaction).to_vec().pretty()); dispatch_transaction(&*client, &*miner, signed_transaction) } + +mod error_codes { + // NOTE [ToDr] Codes from -32000 to -32099 + pub const UNKNOWN_ERROR: i64 = -32002; + pub const TRANSACTION_ERROR: i64 = -32010; + pub const ACCOUNT_LOCKED: i64 = -32020; +} + +fn transaction_error(error: EthcoreError) -> Error { + use ethcore::error::TransactionError::*; + + if let EthcoreError::Transaction(e) = error { + let msg = match e { + AlreadyImported => "Transaction with the same hash was already imported.".into(), + Old => "Transaction nonce is too low. Try incrementing the nonce.".into(), + TooCheapToReplace => { + "Transaction fee is too low. There is another transaction with same nonce in the queue. Try increasing the fee or incrementing the nonce.".into() + }, + LimitReached => { + "There is too many transactions in the queue. Your transaction was dropped due to limit. Try increasing the fee.".into() + }, + InsufficientGasPrice { minimal, got } => { + format!("Transaction fee is to low. It does not satisfy your node's minimal fee (minimal: {}, got: {}). Try increasing the fee.", minimal, got) + }, + InsufficientBalance { balance, cost } => { + format!("Insufficient funds. Account you try to send transaction from does not have enough funds. Required {} and got: {}.", cost, balance) + }, + GasLimitExceeded { limit, got } => { + format!("Transaction cost exceeds current gas limit. Limit: {}, got: {}. Try decreasing supplied gas.", limit, got) + }, + InvalidGasLimit(_) => "Supplied gas is beyond limit.".into(), + }; + Error { + code: ErrorCode::ServerError(error_codes::TRANSACTION_ERROR), + message: msg, + data: None, + } + } else { + Error { + code: ErrorCode::ServerError(error_codes::UNKNOWN_ERROR), + message: "Unknown error when sending transaction.".into(), + data: Some(Value::String(format!("{:?}", error))), + } + } +} diff --git a/rpc/src/v1/impls/personal_signer.rs b/rpc/src/v1/impls/personal_signer.rs index 88bf9a5d1..0c1fcb7f1 100644 --- a/rpc/src/v1/impls/personal_signer.rs +++ b/rpc/src/v1/impls/personal_signer.rs @@ -73,9 +73,9 @@ impl PersonalSigner for SignerClient { - let hash = sign_and_dispatch(&*client, &*miner, request, secret); - queue.request_confirmed(id, hash); - Some(to_value(&hash)) + let res = sign_and_dispatch(&*client, &*miner, request, secret); + queue.request_confirmed(id, res.clone()); + Some(res) }, Err(_) => None }