Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Adds personal_signTransaction RPC method #6991

Merged
merged 4 commits into from
Dec 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 38 additions & 25 deletions rpc/src/v1/impls/personal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use v1::helpers::errors;
use v1::helpers::dispatch::{Dispatcher, SignWith};
use v1::helpers::accounts::unwrap_provider;
use v1::traits::Personal;
use v1::types::{H160 as RpcH160, H256 as RpcH256, U128 as RpcU128, TransactionRequest};
use v1::types::{H160 as RpcH160, H256 as RpcH256, U128 as RpcU128, TransactionRequest, RichRawTransaction as RpcRichRawTransaction};
use v1::metadata::Metadata;

/// Account management (personal) rpc implementation.
Expand All @@ -55,6 +55,35 @@ impl<D: Dispatcher> PersonalClient<D> {
}
}

impl<D: Dispatcher + 'static> PersonalClient<D> {
fn do_sign_transaction(&self, meta: Metadata, request: TransactionRequest, password: String) -> BoxFuture<(PendingTransaction, D)> {
let dispatcher = self.dispatcher.clone();
let accounts = try_bf!(self.account_provider());

let default = match request.from.as_ref() {
Some(account) => Ok(account.clone().into()),
None => accounts
.dapp_default_address(meta.dapp_id().into())
.map_err(|e| errors::account("Cannot find default account.", e)),
};

let default = match default {
Ok(default) => default,
Err(e) => return Box::new(future::err(e)),
};

Box::new(dispatcher.fill_optional_fields(request.into(), default, false)
.and_then(move |filled| {
let condition = filled.condition.clone().map(Into::into);
dispatcher.sign(accounts, filled, SignWith::Password(password))
.map(|tx| tx.into_value())
.map(move |tx| PendingTransaction::new(tx, condition))
.map(move |tx| (tx, dispatcher))
})
)
}
}

impl<D: Dispatcher + 'static> Personal for PersonalClient<D> {
type Metadata = Metadata;

Expand Down Expand Up @@ -104,37 +133,21 @@ impl<D: Dispatcher + 'static> Personal for PersonalClient<D> {
}
}

fn send_transaction(&self, meta: Metadata, request: TransactionRequest, password: String) -> BoxFuture<RpcH256> {
let dispatcher = self.dispatcher.clone();
let accounts = try_bf!(self.account_provider());

let default = match request.from.as_ref() {
Some(account) => Ok(account.clone().into()),
None => accounts
.dapp_default_address(meta.dapp_id().into())
.map_err(|e| errors::account("Cannot find default account.", e)),
};

let default = match default {
Ok(default) => default,
Err(e) => return Box::new(future::err(e)),
};
fn sign_transaction(&self, meta: Metadata, request: TransactionRequest, password: String) -> BoxFuture<RpcRichRawTransaction> {
Box::new(self.do_sign_transaction(meta, request, password)
.map(|(pending_tx, dispatcher)| dispatcher.enrich(pending_tx.transaction)))
}

Box::new(dispatcher.fill_optional_fields(request.into(), default, false)
.and_then(move |filled| {
let condition = filled.condition.clone().map(Into::into);
dispatcher.sign(accounts, filled, SignWith::Password(password))
.map(|tx| tx.into_value())
.map(move |tx| PendingTransaction::new(tx, condition))
.map(move |tx| (tx, dispatcher))
})
fn send_transaction(&self, meta: Metadata, request: TransactionRequest, password: String) -> BoxFuture<RpcH256> {
Box::new(self.do_sign_transaction(meta, request, password)
.and_then(|(pending_tx, dispatcher)| {
let chain_id = pending_tx.chain_id();
trace!(target: "miner", "send_transaction: dispatching tx: {} for chain ID {:?}",
::rlp::encode(&*pending_tx).into_vec().pretty(), chain_id);

dispatcher.dispatch_transaction(pending_tx).map(Into::into)
}))
})
)
}

fn sign_and_send_transaction(&self, meta: Metadata, request: TransactionRequest, password: String) -> BoxFuture<RpcH256> {
Expand Down
21 changes: 16 additions & 5 deletions rpc/src/v1/tests/mocked/personal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,28 +96,39 @@ fn new_account() {
assert_eq!(res, Some(response));
}

#[test]
fn sign_and_send_transaction_with_invalid_password() {
fn invalid_password_test(method: &str)
{
let tester = setup();
let address = tester.accounts.new_account("password123").unwrap();

let request = r#"{
"jsonrpc": "2.0",
"method": "personal_sendTransaction",
"method": ""#.to_owned() + method + r#"",
"params": [{
"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
"from": ""# + format!("0x{:?}", address).as_ref() + r#"",
"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
"gas": "0x76c0",
"gasPrice": "0x9184e72a000",
"value": "0x9184e72a"
}, "password321"],
"id": 1
}"#;
}"#;

let response = r#"{"jsonrpc":"2.0","error":{"code":-32021,"message":"Account password is invalid or account does not exist.","data":"SStore(InvalidPassword)"},"id":1}"#;

assert_eq!(tester.io.handle_request_sync(request.as_ref()), Some(response.into()));
}

#[test]
fn sign_transaction_with_invalid_password() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have a positive test case as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't sure how to check if the response is actually valid.

Is there a convenient way to validate the whole JSON response, knowing that it should describe a transaction? I.e. all fields should exist and types should be correct? Maybe I could try to deserialize it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, deserializing it makes sense.

invalid_password_test("personal_signTransaction");
}

#[test]
fn sign_and_send_transaction_with_invalid_password() {
invalid_password_test("personal_sendTransaction");
}

#[test]
fn send_transaction() {
sign_and_send_test("personal_sendTransaction");
Expand Down
6 changes: 5 additions & 1 deletion rpc/src/v1/traits/personal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Personal rpc interface.
use jsonrpc_core::{BoxFuture, Result};

use v1::types::{U128, H160, H256, TransactionRequest};
use v1::types::{U128, H160, H256, TransactionRequest, RichRawTransaction as RpcRichRawTransaction};

build_rpc_trait! {
/// Personal rpc interface. Safe (read-only) functions.
Expand All @@ -37,6 +37,10 @@ build_rpc_trait! {
#[rpc(name = "personal_unlockAccount")]
fn unlock_account(&self, H160, String, Option<U128>) -> Result<bool>;

/// Signs transaction. The account is not unlocked in such case.
#[rpc(meta, name = "personal_signTransaction")]
fn sign_transaction(&self, Self::Metadata, TransactionRequest, String) -> BoxFuture<RpcRichRawTransaction>;

/// Sends transaction and signs it in single call. The account is not unlocked in such case.
#[rpc(meta, name = "personal_sendTransaction")]
fn send_transaction(&self, Self::Metadata, TransactionRequest, String) -> BoxFuture<H256>;
Expand Down