From 4ba258722619e00a9baa93bdf3b3720ccd547bc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Oct 2017 14:46:36 +0200 Subject: [PATCH 1/3] Fix serialization of non-localized transactions. --- ethcore/src/transaction.rs | 5 +++++ rpc/src/v1/helpers/light_fetch.rs | 23 +++++++++++++++++------ rpc/src/v1/tests/mocked/eth.rs | 4 ++-- rpc/src/v1/tests/mocked/parity_set.rs | 2 +- rpc/src/v1/tests/mocked/signer.rs | 2 +- rpc/src/v1/tests/mocked/signing.rs | 2 +- rpc/src/v1/types/transaction.rs | 2 +- 7 files changed, 28 insertions(+), 12 deletions(-) diff --git a/ethcore/src/transaction.rs b/ethcore/src/transaction.rs index 02f77d5f3..3596f6c21 100644 --- a/ethcore/src/transaction.rs +++ b/ethcore/src/transaction.rs @@ -469,6 +469,11 @@ impl SignedTransaction { pub fn is_unsigned(&self) -> bool { self.transaction.is_unsigned() } + + /// Deconstructs this transaction back into `UnverifiedTransaction` + pub fn deconstruct(self) -> (UnverifiedTransaction, Address, Option) { + (self.transaction, self.sender, self.public) + } } /// Signed Transaction that is a part of canon blockchain. diff --git a/rpc/src/v1/helpers/light_fetch.rs b/rpc/src/v1/helpers/light_fetch.rs index ac5902b51..2544f66b0 100644 --- a/rpc/src/v1/helpers/light_fetch.rs +++ b/rpc/src/v1/helpers/light_fetch.rs @@ -23,7 +23,7 @@ use ethcore::encoded; use ethcore::executed::{Executed, ExecutionError}; use ethcore::ids::BlockId; use ethcore::filter::Filter as EthcoreFilter; -use ethcore::transaction::{Action, Transaction as EthTransaction, SignedTransaction}; +use ethcore::transaction::{Action, Transaction as EthTransaction, SignedTransaction, LocalizedTransaction}; use ethcore::receipt::Receipt; use jsonrpc_core::{BoxFuture, Error}; @@ -65,13 +65,24 @@ pub struct LightFetch { /// Extract a transaction at given index. pub fn extract_transaction_at_index(block: encoded::Block, index: usize, eip86_transition: u64) -> Option { block.transactions().into_iter().nth(index) + // Verify if transaction signature is correct. .and_then(|tx| SignedTransaction::new(tx).ok()) - .map(|tx| Transaction::from_signed(tx, block.number(), eip86_transition)) - .map(|mut tx| { - tx.block_hash = Some(block.hash().into()); - tx.transaction_index = Some(index.into()); - tx + .map(|signed_tx| { + let (signed, sender, _) = signed_tx.deconstruct(); + let block_hash = block.hash(); + let block_number = block.number(); + let transaction_index = index; + let cached_sender = Some(sender); + + LocalizedTransaction { + signed, + block_number, + block_hash, + transaction_index, + cached_sender, + } }) + .map(|tx| Transaction::from_localized(tx, eip86_transition)) } diff --git a/rpc/src/v1/tests/mocked/eth.rs b/rpc/src/v1/tests/mocked/eth.rs index 337ef9513..9969ee286 100644 --- a/rpc/src/v1/tests/mocked/eth.rs +++ b/rpc/src/v1/tests/mocked/eth.rs @@ -547,7 +547,7 @@ fn rpc_eth_pending_transaction_by_hash() { tester.miner.pending_transactions.lock().insert(H256::zero(), tx); } - let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":"0x0","chainId":null,"condition":null,"creates":null,"from":"0x0f65fe9276bc9a24ae7083ae28e2660ef72df99e","gas":"0x5208","gasPrice":"0x1","hash":"0x41df922fd0d4766fcc02e161f8295ec28522f329ae487f14d811e4b64c8d6e31","input":"0x","nonce":"0x0","publicKey":"0x7ae46da747962c2ee46825839c1ef9298e3bd2e70ca2938495c3693a485ec3eaa8f196327881090ff64cf4fbb0a48485d4f83098e189ed3b7a87d5941b59f789","r":"0x48b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353","raw":"0xf85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","s":"0xefffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","standardV":"0x0","to":"0x095e7baea6a6c7c4c2dfeb977efac326af552d87","transactionIndex":null,"v":"0x1b","value":"0xa"},"id":1}"#; + let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":null,"chainId":null,"condition":null,"creates":null,"from":"0x0f65fe9276bc9a24ae7083ae28e2660ef72df99e","gas":"0x5208","gasPrice":"0x1","hash":"0x41df922fd0d4766fcc02e161f8295ec28522f329ae487f14d811e4b64c8d6e31","input":"0x","nonce":"0x0","publicKey":"0x7ae46da747962c2ee46825839c1ef9298e3bd2e70ca2938495c3693a485ec3eaa8f196327881090ff64cf4fbb0a48485d4f83098e189ed3b7a87d5941b59f789","r":"0x48b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353","raw":"0xf85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","s":"0xefffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","standardV":"0x0","to":"0x095e7baea6a6c7c4c2dfeb977efac326af552d87","transactionIndex":null,"v":"0x1b","value":"0xa"},"id":1}"#; let request = r#"{ "jsonrpc": "2.0", "method": "eth_getTransactionByHash", @@ -863,7 +863,7 @@ fn rpc_eth_sign_transaction() { let response = r#"{"jsonrpc":"2.0","result":{"#.to_owned() + r#""raw":"0x"# + &rlp.to_hex() + r#"","# + r#""tx":{"# + - r#""blockHash":null,"blockNumber":"0x0","# + + r#""blockHash":null,"blockNumber":null,"# + &format!("\"chainId\":{},", t.chain_id().map_or("null".to_owned(), |n| format!("{}", n))) + r#""condition":null,"creates":null,"# + &format!("\"from\":\"0x{:?}\",", &address) + diff --git a/rpc/src/v1/tests/mocked/parity_set.rs b/rpc/src/v1/tests/mocked/parity_set.rs index 1653a1908..ed27862ac 100644 --- a/rpc/src/v1/tests/mocked/parity_set.rs +++ b/rpc/src/v1/tests/mocked/parity_set.rs @@ -234,7 +234,7 @@ fn rpc_parity_remove_transaction() { let hash = signed.hash(); let request = r#"{"jsonrpc": "2.0", "method": "parity_removeTransaction", "params":[""#.to_owned() + &format!("0x{:?}", hash) + r#""], "id": 1}"#; - let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":"0x0","chainId":null,"condition":null,"creates":null,"from":"0x0000000000000000000000000000000000000002","gas":"0x76c0","gasPrice":"0x9184e72a000","hash":"0xa2e0da8a8064e0b9f93e95a53c2db6d01280efb8ac72a708d25487e67dd0f8fc","input":"0x","nonce":"0x1","publicKey":null,"r":"0x1","raw":"0xe9018609184e72a0008276c0940000000000000000000000000000000000000005849184e72a80800101","s":"0x1","standardV":"0x4","to":"0x0000000000000000000000000000000000000005","transactionIndex":null,"v":"0x0","value":"0x9184e72a"},"id":1}"#; + let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":null,"chainId":null,"condition":null,"creates":null,"from":"0x0000000000000000000000000000000000000002","gas":"0x76c0","gasPrice":"0x9184e72a000","hash":"0xa2e0da8a8064e0b9f93e95a53c2db6d01280efb8ac72a708d25487e67dd0f8fc","input":"0x","nonce":"0x1","publicKey":null,"r":"0x1","raw":"0xe9018609184e72a0008276c0940000000000000000000000000000000000000005849184e72a80800101","s":"0x1","standardV":"0x4","to":"0x0000000000000000000000000000000000000005","transactionIndex":null,"v":"0x0","value":"0x9184e72a"},"id":1}"#; miner.pending_transactions.lock().insert(hash, signed); assert_eq!(io.handle_request_sync(&request), Some(response.to_owned())); diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index 0211b5f24..3399cd741 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/rpc/src/v1/tests/mocked/signer.rs @@ -456,7 +456,7 @@ fn should_confirm_sign_transaction_with_rlp() { let response = r#"{"jsonrpc":"2.0","result":{"#.to_owned() + r#""raw":"0x"# + &rlp.to_hex() + r#"","# + r#""tx":{"# + - r#""blockHash":null,"blockNumber":"0x0","# + + r#""blockHash":null,"blockNumber":null,"# + &format!("\"chainId\":{},", t.chain_id().map_or("null".to_owned(), |n| format!("{}", n))) + r#""condition":null,"creates":null,"# + &format!("\"from\":\"0x{:?}\",", &address) + diff --git a/rpc/src/v1/tests/mocked/signing.rs b/rpc/src/v1/tests/mocked/signing.rs index 41c8ec7a0..05199e696 100644 --- a/rpc/src/v1/tests/mocked/signing.rs +++ b/rpc/src/v1/tests/mocked/signing.rs @@ -307,7 +307,7 @@ fn should_add_sign_transaction_to_the_queue() { let response = r#"{"jsonrpc":"2.0","result":{"#.to_owned() + r#""raw":"0x"# + &rlp.to_hex() + r#"","# + r#""tx":{"# + - r#""blockHash":null,"blockNumber":"0x0","# + + r#""blockHash":null,"blockNumber":null,"# + &format!("\"chainId\":{},", t.chain_id().map_or("null".to_owned(), |n| format!("{}", n))) + r#""condition":null,"creates":null,"# + &format!("\"from\":\"0x{:?}\",", &address) + diff --git a/rpc/src/v1/types/transaction.rs b/rpc/src/v1/types/transaction.rs index 570c21120..90d512c86 100644 --- a/rpc/src/v1/types/transaction.rs +++ b/rpc/src/v1/types/transaction.rs @@ -213,7 +213,7 @@ impl Transaction { hash: t.hash().into(), nonce: t.nonce.into(), block_hash: None, - block_number: Some(block_number.into()), + block_number: None, transaction_index: None, from: t.sender().into(), to: match t.action { From 42e23a963300bd101a172b54c23d3c2bdee54432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Oct 2017 16:45:06 +0200 Subject: [PATCH 2/3] Return proper SignedTransactions representation. --- rpc/src/v1/helpers/dispatch.rs | 27 ++++++++++++++++++++------- rpc/src/v1/impls/signer.rs | 7 ++++--- rpc/src/v1/tests/mocked/signing.rs | 6 ++++-- rpc/src/v1/types/transaction.rs | 9 ++++----- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index 0da257035..d84bd6cad 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -71,6 +71,9 @@ pub trait Dispatcher: Send + Sync + Clone { fn sign(&self, accounts: Arc, filled: FilledTransactionRequest, password: SignWith) -> BoxFuture, Error>; + /// Converts a `SignedTransaction` into `RichRawTransaction` + fn enrich(&self, SignedTransaction) -> RpcRichRawTransaction; + /// "Dispatch" a local transaction. fn dispatch_transaction(&self, signed_transaction: PendingTransaction) -> Result; } @@ -163,6 +166,11 @@ impl Dispatcher for FullDispatcher RpcRichRawTransaction { + let block_number = self.client.best_block_header().number(); + RpcRichRawTransaction::from_signed(signed_transaction, block_number, self.client.eip86_transition()) + } + fn dispatch_transaction(&self, signed_transaction: PendingTransaction) -> Result { let hash = signed_transaction.transaction.hash(); @@ -261,11 +269,11 @@ impl LightDispatcher { transaction_queue: Arc>, ) -> Self { LightDispatcher { - sync: sync, - client: client, - on_demand: on_demand, - cache: cache, - transaction_queue: transaction_queue, + sync, + client, + on_demand, + cache, + transaction_queue, } } @@ -399,6 +407,11 @@ impl Dispatcher for LightDispatcher { .and_then(move |nonce| with_nonce(filled, nonce))) } + fn enrich(&self, signed_transaction: SignedTransaction) -> RpcRichRawTransaction { + let block_number = self.client.best_block_header().number(); + RpcRichRawTransaction::from_signed(signed_transaction, block_number, self.client.eip86_transition()) + } + fn dispatch_transaction(&self, signed_transaction: PendingTransaction) -> Result { let hash = signed_transaction.transaction.hash(); @@ -510,8 +523,8 @@ pub fn execute( }, ConfirmationPayload::SignTransaction(request) => { Box::new(dispatcher.sign(accounts, request, pass) - .map(|result| result - .map(RpcRichRawTransaction::from) + .map(move |result| result + .map(move |tx| dispatcher.enrich(tx)) .map(ConfirmationResponse::SignTransaction) )) }, diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index c864134fb..cecd1f2db 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -73,8 +73,8 @@ impl SignerClient { SignerClient { signer: signer.clone(), accounts: store.clone(), - dispatcher: dispatcher, - subscribers: subscribers, + dispatcher, + subscribers, } } @@ -205,7 +205,8 @@ impl Signer for SignerClient { }, ConfirmationPayload::SignTransaction(request) => { Self::verify_transaction(bytes, request, |pending_transaction| { - Ok(ConfirmationResponse::SignTransaction(pending_transaction.transaction.into())) + let rich = self.dispatcher.enrich(pending_transaction.transaction); + Ok(ConfirmationResponse::SignTransaction(rich)) }) }, ConfirmationPayload::EthSignMessage(address, data) => { diff --git a/rpc/src/v1/tests/mocked/signing.rs b/rpc/src/v1/tests/mocked/signing.rs index 05199e696..dbe5534ca 100644 --- a/rpc/src/v1/tests/mocked/signing.rs +++ b/rpc/src/v1/tests/mocked/signing.rs @@ -26,7 +26,7 @@ use v1::impls::SigningQueueClient; use v1::metadata::Metadata; use v1::traits::{EthSigning, ParitySigning, Parity}; use v1::helpers::{SignerService, SigningQueue, FullDispatcher}; -use v1::types::ConfirmationResponse; +use v1::types::{ConfirmationResponse, RichRawTransaction}; use v1::tests::helpers::TestMinerService; use v1::tests::mocked::parity; @@ -334,7 +334,9 @@ fn should_add_sign_transaction_to_the_queue() { ::std::thread::spawn(move || loop { if signer.requests().len() == 1 { // respond - signer.request_confirmed(1.into(), Ok(ConfirmationResponse::SignTransaction(t.into()))); + signer.request_confirmed(1.into(), Ok(ConfirmationResponse::SignTransaction( + RichRawTransaction::from_signed(t.into(), 0x0, u64::max_value()) + ))); break } ::std::thread::sleep(Duration::from_millis(100)) diff --git a/rpc/src/v1/types/transaction.rs b/rpc/src/v1/types/transaction.rs index 90d512c86..dbd326488 100644 --- a/rpc/src/v1/types/transaction.rs +++ b/rpc/src/v1/types/transaction.rs @@ -158,11 +158,10 @@ pub struct RichRawTransaction { pub transaction: Transaction } - -impl From for RichRawTransaction { - fn from(t: SignedTransaction) -> Self { - // TODO: change transition to 0 when EIP-86 is commonly used. - let tx: Transaction = Transaction::from_signed(t, 0, u64::max_value()); +impl RichRawTransaction { + /// Creates new `RichRawTransaction` from `SignedTransaction`. + pub fn from_signed(tx: SignedTransaction, block_number: u64, eip86_transition: u64) -> Self { + let tx = Transaction::from_signed(tx, block_number, eip86_transition); RichRawTransaction { raw: tx.raw.clone(), transaction: tx, From 965dff3d3218180aa62d034bd31f93e0827b562a Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 24 Oct 2017 07:09:48 +0200 Subject: [PATCH 3/3] light: get local transactions by hash --- ethcore/light/src/transaction_queue.rs | 5 +++++ rpc/src/v1/impls/light/eth.rs | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/ethcore/light/src/transaction_queue.rs b/ethcore/light/src/transaction_queue.rs index 090919245..7f67c3718 100644 --- a/ethcore/light/src/transaction_queue.rs +++ b/ethcore/light/src/transaction_queue.rs @@ -321,6 +321,11 @@ impl TransactionQueue { self.by_hash.remove(&hash); } } + + /// Get a transaction by hash. + pub fn get(&self, hash: &H256) -> Option<&PendingTransaction> { + self.by_hash.get(&hash) + } } #[cfg(test)] diff --git a/rpc/src/v1/impls/light/eth.rs b/rpc/src/v1/impls/light/eth.rs index b797e76c2..d498c6ccd 100644 --- a/rpc/src/v1/impls/light/eth.rs +++ b/rpc/src/v1/impls/light/eth.rs @@ -392,8 +392,21 @@ impl Eth for EthClient { } fn transaction_by_hash(&self, hash: RpcH256) -> BoxFuture, Error> { + let hash = hash.into(); let eip86 = self.client.eip86_transition(); - Box::new(self.fetcher().transaction_by_hash(hash.into(), eip86).map(|x| x.map(|(tx, _)| tx))) + + { + let tx_queue = self.transaction_queue.read(); + if let Some(tx) = tx_queue.get(&hash) { + return Box::new(future::ok(Some(Transaction::from_pending( + tx.clone(), + self.client.chain_info().best_block_number, + eip86, + )))); + } + } + + Box::new(self.fetcher().transaction_by_hash(hash, eip86).map(|x| x.map(|(tx, _)| tx))) } fn transaction_by_block_hash_and_index(&self, hash: RpcH256, idx: Index) -> BoxFuture, Error> {