From 5223e25aa6ccf82d5eb61533a6c83e6189e2d02e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 8 Feb 2017 16:55:06 +0100 Subject: [PATCH] use generic dispatcher everywhere, squash errors --- rpc/src/v1/helpers/dispatch.rs | 9 ++-- rpc/src/v1/impls/eth.rs | 5 +- rpc/src/v1/impls/light/eth.rs | 1 - rpc/src/v1/impls/signer.rs | 80 +++++++++++++++++------------- rpc/src/v1/impls/signing.rs | 22 +++++--- rpc/src/v1/impls/signing_unsafe.rs | 11 ++-- rpc/src/v1/traits/signer.rs | 9 ++-- 7 files changed, 79 insertions(+), 58 deletions(-) diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index e2bb0b80f..3994efde1 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -94,12 +94,13 @@ impl Dispatcher for FullDispatcher Dispatcher for FullDispatcher Result { let hash = signed_transaction.transaction.hash(); - take_weak!(self.miner).import_own_transaction(take_weak!(self.client), signed_transaction) + take_weak!(self.miner).import_own_transaction(&*take_weak!(self.client), signed_transaction) .map_err(errors::from_transaction_error) .map(|_| hash) } @@ -223,7 +224,7 @@ impl From<(T, Option)> for WithToken { } } -pub fn execute( +pub fn execute( dispatcher: D, accounts: &AccountProvider, payload: ConfirmationPayload, @@ -231,7 +232,7 @@ pub fn execute( ) -> BoxFuture, Error> { match payload { ConfirmationPayload::SendTransaction(request) => { - let condition = request.condition.clone(); + let condition = request.condition.clone().map(Into::into); dispatcher.sign(accounts, request, pass) .map(move |v| v.map(move |tx| PendingTransaction::new(tx, condition))) .map(WithToken::into_tuple) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 2105aed29..53555e475 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -46,7 +46,7 @@ use jsonrpc_core::Error; use jsonrpc_macros::Trailing; use v1::helpers::{CallRequest as CRequest, errors, limit_logs}; -use v1::helpers::dispatch::{dispatch_transaction, default_gas_price}; +use v1::helpers::dispatch::{Dispatcher, FullDispatcher, default_gas_price}; use v1::helpers::block_import::is_major_importing; use v1::traits::Eth; use v1::types::{ @@ -634,7 +634,8 @@ impl Eth for EthClient where .map_err(errors::from_rlp_error) .and_then(|tx| SignedTransaction::new(tx).map_err(errors::from_transaction_error)) .and_then(|signed_transaction| { - dispatch_transaction(&*take_weak!(self.client), &*take_weak!(self.miner), PendingTransaction::new(signed_transaction, None)) + FullDispatcher::new(self.client.clone(), self.miner.clone()) + .dispatch_transaction(signed_transaction.into()) }) .map(Into::into) } diff --git a/rpc/src/v1/impls/light/eth.rs b/rpc/src/v1/impls/light/eth.rs index 711d559eb..0f5dc8e31 100644 --- a/rpc/src/v1/impls/light/eth.rs +++ b/rpc/src/v1/impls/light/eth.rs @@ -37,7 +37,6 @@ use futures::{future, Future, BoxFuture}; use futures::sync::oneshot; use v1::helpers::{CallRequest as CRequest, errors, limit_logs}; -use v1::helpers::dispatch::{dispatch_transaction, default_gas_price}; use v1::helpers::block_import::is_major_importing; use v1::traits::Eth; use v1::types::{ diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index 90a811e37..5e6a1f09b 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -23,46 +23,52 @@ use ethcore::account_provider::AccountProvider; use ethcore::client::MiningBlockChainClient; use ethcore::transaction::{SignedTransaction, PendingTransaction}; use ethcore::miner::MinerService; +use futures::{self, future, BoxFuture, Future, IntoFuture}; use jsonrpc_core::Error; use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload}; -use v1::helpers::dispatch::{self, dispatch_transaction, WithToken}; +use v1::helpers::dispatch::{self, Dispatcher, WithToken}; use v1::traits::Signer; use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, U256, Bytes}; /// Transactions confirmation (personal) rpc implementation. -pub struct SignerClient where C: MiningBlockChainClient, M: MinerService { +pub struct SignerClient { signer: Weak, accounts: Weak, - client: Weak, - miner: Weak, + dispatcher: D } -impl SignerClient where C: MiningBlockChainClient, M: MinerService { +impl SignerClient { /// Create new instance of signer client. pub fn new( store: &Arc, - client: &Arc, - miner: &Arc, + dispatcher: D, signer: &Arc, ) -> Self { SignerClient { signer: Arc::downgrade(signer), accounts: Arc::downgrade(store), - client: Arc::downgrade(client), - miner: Arc::downgrade(miner), + dispatcher: dispatcher, } } - fn confirm_internal(&self, id: U256, modification: TransactionModification, f: F) -> Result, Error> where - F: FnOnce(&C, &M, &AccountProvider, ConfirmationPayload) -> Result, Error>, + fn confirm_internal(&self, id: U256, modification: TransactionModification, f: F) -> BoxFuture, Error> where + F: FnOnce(D, &AccountProvider, ConfirmationPayload) -> T, + T: IntoFuture, Error=Error>, + T::Future: Send + 'static { let id = id.into(); - let accounts = take_weak!(self.accounts); - let signer = take_weak!(self.signer); - let client = take_weak!(self.client); - let miner = take_weak!(self.miner); + let dispatcher = self.dispatcher.clone(); + + let setup = || { + Ok((take_weak!(self.accounts), take_weak!(self.signer))) + }; + + let (accounts, signer) = match setup() { + Ok(x) => x, + Err(e) => return future::err(e).boxed(), + }; signer.peek(&id).map(|confirmation| { let mut payload = confirmation.payload.clone(); @@ -83,17 +89,21 @@ impl SignerClient where C: MiningBlockChainClient, request.condition = condition.clone().map(Into::into); } } - let result = f(&*client, &*miner, &*accounts, payload); - // Execute - if let Ok(ref response) = result { - signer.request_confirmed(id, Ok((*response).clone())); - } - result - }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) + let fut = f(dispatcher, &*accounts, payload); + fut.into_future().then(move |result| { + // Execute + if let Ok(ref response) = result { + signer.request_confirmed(id, Ok((*response).clone())); + } + + result + }).boxed() + }) + .unwrap_or_else(|| future::err(errors::invalid_params("Unknown RequestID", id)).boxed()) } } -impl Signer for SignerClient where C: MiningBlockChainClient, M: MinerService { +impl Signer for SignerClient { fn requests_to_confirm(&self) -> Result, Error> { let signer = take_weak!(self.signer); @@ -107,29 +117,31 @@ impl Signer for SignerClient where C: MiningBlockC // TODO [ToDr] TransactionModification is redundant for some calls // might be better to replace it in future - fn confirm_request(&self, id: U256, modification: TransactionModification, pass: String) -> Result { - self.confirm_internal(id, modification, move |client, miner, accounts, payload| { - dispatch::execute(client, miner, accounts, payload, dispatch::SignWith::Password(pass)) - }).map(|v| v.into_value()) + fn confirm_request(&self, id: U256, modification: TransactionModification, pass: String) + -> BoxFuture + { + self.confirm_internal(id, modification, move |dis, accounts, payload| { + dispatch::execute(dis, accounts, payload, dispatch::SignWith::Password(pass)) + }).map(|v| v.into_value()).boxed() } - fn confirm_request_with_token(&self, id: U256, modification: TransactionModification, token: String) -> Result { - self.confirm_internal(id, modification, move |client, miner, accounts, payload| { - dispatch::execute(client, miner, accounts, payload, dispatch::SignWith::Token(token)) + fn confirm_request_with_token(&self, id: U256, modification: TransactionModification, token: String) + -> BoxFuture + { + self.confirm_internal(id, modification, move |dis, accounts, payload| { + dispatch::execute(dis, accounts, payload, dispatch::SignWith::Token(token)) }).and_then(|v| match v { WithToken::No(_) => Err(errors::internal("Unexpected response without token.", "")), WithToken::Yes(response, token) => Ok(ConfirmationResponseWithToken { result: response, token: token, }), - }) + }).boxed() } fn confirm_request_raw(&self, id: U256, bytes: Bytes) -> Result { let id = id.into(); let signer = take_weak!(self.signer); - let client = take_weak!(self.client); - let miner = take_weak!(self.miner); signer.peek(&id).map(|confirmation| { let result = match confirmation.payload { @@ -150,7 +162,7 @@ impl Signer for SignerClient where C: MiningBlockC // Dispatch if everything is ok if sender_matches && data_matches && value_matches && nonce_matches { let pending_transaction = PendingTransaction::new(signed_transaction, request.condition.map(Into::into)); - dispatch_transaction(&*client, &*miner, pending_transaction) + self.dispatcher.dispatch_transaction(pending_transaction) .map(Into::into) .map(ConfirmationResponse::SendTransaction) } else { diff --git a/rpc/src/v1/impls/signing.rs b/rpc/src/v1/impls/signing.rs index 639dfe09b..86220d65c 100644 --- a/rpc/src/v1/impls/signing.rs +++ b/rpc/src/v1/impls/signing.rs @@ -72,7 +72,7 @@ fn handle_dispatch(res: Result, on_response: } } -impl SigningQueueClient { +impl SigningQueueClient { /// Creates a new signing queue client given shared signing queue. pub fn new(signer: &Arc, dispatcher: D, accounts: &Arc) -> Self { SigningQueueClient { @@ -84,30 +84,34 @@ impl SigningQueueClient { } fn dispatch(&self, payload: RpcConfirmationPayload, default_account: DefaultAccount) -> BoxFuture { - let setup = || { + let setup = move || { let accounts = take_weak!(self.accounts); + let default_account = default_account; let default_account = match default_account { DefaultAccount::Provided(acc) => acc, DefaultAccount::ForDapp(dapp) => accounts.default_address(dapp).ok().unwrap_or_default(), }; - (self.dispatcher.clone(), accounts, default_account) + Ok((self.dispatcher.clone(), accounts, default_account)) }; let weak_signer = self.signer.clone(); - future::done(setup) + future::done(setup()) .and_then(move |(dispatcher, accounts, default_account)| { dispatch::from_rpc(payload, default_account, &dispatcher) .and_then(move |payload| { let sender = payload.sender(); if accounts.is_unlocked(sender) { dispatch::execute(dispatcher, &accounts, payload, dispatch::SignWith::Nothing) + .map(|v| v.into_value()) + .map(DispatchResult::Value) .boxed() } else { - future::lazy(move || take_weak!(weak_signer).add_request(payload)) - .map(DispatchResult::Promise) - .map_err(|_| errors::request_rejected_limit()) - .boxed() + future::lazy(move || + take_weak!(weak_signer).add_request(payload) + .map(DispatchResult::Promise) + .map_err(|_| errors::request_rejected_limit()) + ).boxed() } }) }) @@ -129,6 +133,7 @@ impl ParitySigning for SigningQueueClient { RpcEither::Either(id.into()) }, }) + .boxed() } fn post_transaction(&self, meta: Metadata, request: RpcTransactionRequest) -> BoxFuture, Error> { @@ -142,6 +147,7 @@ impl ParitySigning for SigningQueueClient { RpcEither::Either(id.into()) }, }) + .boxed() } fn check_request(&self, id: RpcU256) -> Result, Error> { diff --git a/rpc/src/v1/impls/signing_unsafe.rs b/rpc/src/v1/impls/signing_unsafe.rs index 53956f23c..c4e6edd66 100644 --- a/rpc/src/v1/impls/signing_unsafe.rs +++ b/rpc/src/v1/impls/signing_unsafe.rs @@ -44,7 +44,7 @@ pub struct SigningUnsafeClient { dispatcher: D, } -impl SigningUnsafeClient { +impl SigningUnsafeClient { /// Creates new SigningUnsafeClient. pub fn new(accounts: &Arc, dispatcher: D) -> Self { SigningUnsafeClient { @@ -56,20 +56,21 @@ impl SigningUnsafeClient { fn handle(&self, payload: RpcConfirmationPayload, account: DefaultAccount) -> BoxFuture { let setup = move || { let accounts = take_weak!(self.accounts); - let default_account = match account { + let default_account = account; + let default_account = match default_account { DefaultAccount::Provided(acc) => acc, DefaultAccount::ForDapp(dapp) => accounts.default_address(dapp).ok().unwrap_or_default(), }; - (accounts, default_account) + Ok((accounts, default_account)) }; let dis = self.dispatcher.clone(); future::done(setup()) .and_then(move |(accounts, default)| { dispatch::from_rpc(payload, default, &dis) - .and_then(|payload| { - dispatch::execute(&dis, &accounts, payload, dispatch::SignWith::Nothing) + .and_then(move |payload| { + dispatch::execute(dis, &accounts, payload, dispatch::SignWith::Nothing) }) .map(|v| v.into_value()) }) diff --git a/rpc/src/v1/traits/signer.rs b/rpc/src/v1/traits/signer.rs index e4f2f0419..1b03f6d8a 100644 --- a/rpc/src/v1/traits/signer.rs +++ b/rpc/src/v1/traits/signer.rs @@ -16,6 +16,7 @@ //! Parity Signer-related rpc interface. use jsonrpc_core::Error; +use futures::BoxFuture; use v1::types::{U256, Bytes, TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken}; @@ -28,12 +29,12 @@ build_rpc_trait! { fn requests_to_confirm(&self) -> Result, Error>; /// Confirm specific request. - #[rpc(name = "signer_confirmRequest")] - fn confirm_request(&self, U256, TransactionModification, String) -> Result; + #[rpc(async, name = "signer_confirmRequest")] + fn confirm_request(&self, U256, TransactionModification, String) -> BoxFuture; /// Confirm specific request with token. - #[rpc(name = "signer_confirmRequestWithToken")] - fn confirm_request_with_token(&self, U256, TransactionModification, String) -> Result; + #[rpc(async, name = "signer_confirmRequestWithToken")] + fn confirm_request_with_token(&self, U256, TransactionModification, String) -> BoxFuture; /// Confirm specific request with already signed data. #[rpc(name = "signer_confirmRequestRaw")]