From 99f9747a3f916cc1ffa7eac1f8d8bf9b5bc94d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Sat, 28 May 2016 18:50:00 +0200 Subject: [PATCH] TransactionConfirmation API --- rpc/src/v1/helpers/mod.rs | 2 +- rpc/src/v1/helpers/signing_queue.rs | 52 ++++--- rpc/src/v1/impls/personal_signer.rs | 36 ++++- rpc/src/v1/mod.rs | 2 +- rpc/src/v1/tests/mocked/eth_signing.rs | 20 ++- rpc/src/v1/tests/mocked/personal.rs | 2 +- rpc/src/v1/tests/mocked/personal_signer.rs | 152 +++++++++++++++++++++ rpc/src/v1/traits/mod.rs | 2 +- rpc/src/v1/traits/personal.rs | 8 +- rpc/src/v1/types/mod.rs.in | 2 +- rpc/src/v1/types/transaction_request.rs | 39 ++++++ 11 files changed, 274 insertions(+), 43 deletions(-) diff --git a/rpc/src/v1/helpers/mod.rs b/rpc/src/v1/helpers/mod.rs index 8e5a5564d..2acf98bf2 100644 --- a/rpc/src/v1/helpers/mod.rs +++ b/rpc/src/v1/helpers/mod.rs @@ -20,4 +20,4 @@ mod signing_queue; pub use self::poll_manager::PollManager; pub use self::poll_filter::PollFilter; -pub use self::signing_queue::SigningQueue; +pub use self::signing_queue::{ConfirmationsQueue, SigningQueue}; diff --git a/rpc/src/v1/helpers/signing_queue.rs b/rpc/src/v1/helpers/signing_queue.rs index c7abd4924..4e26fbaf6 100644 --- a/rpc/src/v1/helpers/signing_queue.rs +++ b/rpc/src/v1/helpers/signing_queue.rs @@ -15,41 +15,57 @@ // along with Parity. If not, see . use std::sync::Mutex; -use std::collections::HashSet; -use v1::types::TransactionRequest; +use std::collections::HashMap; +use v1::types::{TransactionRequest, TransactionConfirmation}; +use util::U256; /// A queue of transactions awaiting to be confirmed and signed. pub trait SigningQueue: Send + Sync { /// Add new request to the queue. - fn add_request(&self, transaction: TransactionRequest); + fn add_request(&self, transaction: TransactionRequest) -> U256; /// Remove request from the queue. - fn remove_request(&self, id: TransactionRequest); + fn remove_request(&self, id: U256) -> Option; /// Return copy of all the requests in the queue. - fn requests(&self) -> HashSet; + fn requests(&self) -> Vec; } -impl SigningQueue for Mutex> { - fn add_request(&self, transaction: TransactionRequest) { - self.lock().unwrap().insert(transaction); +#[derive(Default)] +pub struct ConfirmationsQueue { + id: Mutex, + queue: Mutex>, +} + +impl SigningQueue for ConfirmationsQueue { + fn add_request(&self, transaction: TransactionRequest) -> U256 { + // Increment id + let id = { + let mut last_id = self.id.lock().unwrap(); + *last_id = *last_id + U256::from(1); + *last_id + }; + let mut queue = self.queue.lock().unwrap(); + queue.insert(id, TransactionConfirmation { + id: id, + transaction: transaction, + }); + id } - fn remove_request(&self, id: TransactionRequest) { - self.lock().unwrap().remove(&id); + fn remove_request(&self, id: U256) -> Option { + self.queue.lock().unwrap().remove(&id) } - fn requests(&self) -> HashSet { - let queue = self.lock().unwrap(); - queue.clone() + fn requests(&self) -> Vec { + let queue = self.queue.lock().unwrap(); + queue.values().cloned().collect() } } #[cfg(test)] mod test { - use std::sync::Mutex; - use std::collections::HashSet; use util::hash::Address; use util::numbers::U256; use v1::types::TransactionRequest; @@ -58,7 +74,7 @@ mod test { #[test] fn should_work_for_hashset() { // given - let queue = Mutex::new(HashSet::new()); + let queue = ConfirmationsQueue::default(); let request = TransactionRequest { from: Address::from(1), @@ -76,6 +92,8 @@ mod test { // then assert_eq!(all.len(), 1); - assert!(all.contains(&request)); + let el = all.get(0).unwrap(); + assert_eq!(el.id, U256::from(1)); + assert_eq!(el.transaction, request); } } diff --git a/rpc/src/v1/impls/personal_signer.rs b/rpc/src/v1/impls/personal_signer.rs index bfb3c9eb5..6afedfea9 100644 --- a/rpc/src/v1/impls/personal_signer.rs +++ b/rpc/src/v1/impls/personal_signer.rs @@ -18,8 +18,8 @@ use std::sync::{Arc, Weak}; use jsonrpc_core::*; -use v1::traits::SignerPersonal; -use v1::types::TransactionRequest; +use v1::traits::PersonalSigner; +use v1::types::TransactionModification; use v1::impls::sign_and_dispatch; use v1::helpers::SigningQueue; use util::keys::store::AccountProvider; @@ -50,20 +50,44 @@ impl SignerClient } } -impl SignerPersonal for SignerClient +impl PersonalSigner for SignerClient where A: AccountProvider, C: BlockChainClient, M: MinerService { - fn transactions_to_confirm(&self, params: Params) -> Result { + fn transactions_to_confirm(&self, _params: Params) -> Result { let queue = take_weak!(self.queue); to_value(&queue.requests()) } fn confirm_transaction(&self, params: Params) -> Result { - Err(Error::internal_error()) + from_params::<(U256, TransactionModification, String)>(params).and_then( + |(id, modification, pass)| { + let accounts = take_weak!(self.accounts); + let queue = take_weak!(self.queue); + queue.remove_request(id) + .and_then(|confirmation| { + let mut request = confirmation.transaction; + // apply modification + if let Some(gas_price) = modification.gas_price { + request.gas_price = Some(gas_price); + } + match accounts.locked_account_secret(&request.from, &pass) { + Ok(secret) => Some(sign_and_dispatch(&self.client, &self.miner, request, secret)), + Err(_) => None + } + }) + .unwrap_or_else(|| to_value(&H256::zero())) + } + ) } fn reject_transaction(&self, params: Params) -> Result { - Err(Error::internal_error()) + from_params::<(U256, )>(params).and_then( + |(id, )| { + let queue = take_weak!(self.queue); + let res = queue.remove_request(id); + to_value(&res.is_some()) + } + ) } } diff --git a/rpc/src/v1/mod.rs b/rpc/src/v1/mod.rs index 308e921d2..deb580d2b 100644 --- a/rpc/src/v1/mod.rs +++ b/rpc/src/v1/mod.rs @@ -25,6 +25,6 @@ pub mod traits; pub mod tests; pub mod types; -pub use self::traits::{Web3, Eth, EthFilter, EthSigning, Personal, Net, Ethcore, Traces, Rpc}; +pub use self::traits::{Web3, Eth, EthFilter, EthSigning, Personal, PersonalSigner, Net, Ethcore, Traces, Rpc}; pub use self::impls::*; pub use self::helpers::SigningQueue; diff --git a/rpc/src/v1/tests/mocked/eth_signing.rs b/rpc/src/v1/tests/mocked/eth_signing.rs index 07adc44b5..7522fabec 100644 --- a/rpc/src/v1/tests/mocked/eth_signing.rs +++ b/rpc/src/v1/tests/mocked/eth_signing.rs @@ -14,42 +14,40 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use std::collections::HashSet; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use jsonrpc_core::IoHandler; use v1::impls::EthSigningQueueClient; use v1::traits::EthSigning; -use v1::helpers::SigningQueue; +use v1::helpers::{ConfirmationsQueue, SigningQueue}; use util::keys::TestAccount; -struct EthSignerTester { +struct EthSigningTester { pub queue: Arc, pub io: IoHandler, } -impl Default for EthSignerTester { +impl Default for EthSigningTester { fn default() -> Self { - let queue : Arc = Arc::new(Mutex::new(HashSet::new())); + let queue: Arc = Arc::new(ConfirmationsQueue::default()); let io = IoHandler::new(); io.add_delegate(EthSigningQueueClient::new(&queue).to_delegate()); - EthSignerTester { + EthSigningTester { queue: queue, io: io, } } } -fn eth_signer() -> EthSignerTester { - EthSignerTester::default() +fn eth_signing() -> EthSigningTester { + EthSigningTester::default() } - #[test] fn should_add_transaction_to_queue() { // given - let tester = eth_signer(); + let tester = eth_signing(); let account = TestAccount::new("123"); let address = account.address(); assert_eq!(tester.queue.requests().len(), 0); diff --git a/rpc/src/v1/tests/mocked/personal.rs b/rpc/src/v1/tests/mocked/personal.rs index 991b13cba..8bc3ab3c8 100644 --- a/rpc/src/v1/tests/mocked/personal.rs +++ b/rpc/src/v1/tests/mocked/personal.rs @@ -176,4 +176,4 @@ fn sign_and_send_transaction() { let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#; assert_eq!(tester.io.handle_request(request.as_ref()), Some(response)); -} \ No newline at end of file +} diff --git a/rpc/src/v1/tests/mocked/personal_signer.rs b/rpc/src/v1/tests/mocked/personal_signer.rs index 05ceaf3d3..b6d28b986 100644 --- a/rpc/src/v1/tests/mocked/personal_signer.rs +++ b/rpc/src/v1/tests/mocked/personal_signer.rs @@ -14,4 +14,156 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::sync::Arc; +use std::str::FromStr; +use std::collections::HashMap; +use jsonrpc_core::IoHandler; +use util::numbers::*; +use util::keys::{TestAccount, TestAccountProvider}; +use ethcore::client::TestBlockChainClient; +use ethcore::transaction::{Transaction, Action}; +use v1::{SignerClient, PersonalSigner}; +use v1::tests::helpers::TestMinerService; +use v1::helpers::{SigningQueue, ConfirmationsQueue}; +use v1::types::TransactionRequest; + + +struct PersonalSignerTester { + queue: Arc, + accounts: Arc, + io: IoHandler, + miner: Arc, + // these unused fields are necessary to keep the data alive + // as the handler has only weak pointers. + _client: Arc, +} + +fn blockchain_client() -> Arc { + let client = TestBlockChainClient::new(); + Arc::new(client) +} + +fn accounts_provider() -> Arc { + let accounts = HashMap::new(); + let ap = TestAccountProvider::new(accounts); + Arc::new(ap) +} + +fn miner_service() -> Arc { + Arc::new(TestMinerService::default()) +} + +fn signer_tester() -> PersonalSignerTester { + let queue: Arc = Arc::new(ConfirmationsQueue::default()); + let accounts = accounts_provider(); + let client = blockchain_client(); + let miner = miner_service(); + + let io = IoHandler::new(); + io.add_delegate(SignerClient::new(&accounts, &client, &miner, &queue).to_delegate()); + + PersonalSignerTester { + queue: queue, + accounts: accounts, + io: io, + miner: miner, + _client: client, + } +} + + +#[test] +fn should_return_list_of_transactions_in_queue() { + // given + let tester = signer_tester(); + tester.queue.add_request(TransactionRequest { + from: Address::from(1), + to: Some(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), + gas_price: Some(U256::from(10_000)), + gas: Some(U256::from(10_000_000)), + value: Some(U256::from(1)), + data: None, + nonce: None, + }); + + // when + let request = r#"{"jsonrpc":"2.0","method":"personal_transactionsToConfirm","params":[],"id":1}"#; + let response = r#"{"jsonrpc":"2.0","result":[{"id":"0x01","transaction":{"data":null,"from":"0x0000000000000000000000000000000000000001","gas":"0x989680","gasPrice":"0x2710","nonce":null,"to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","value":"0x01"}}],"id":1}"#; + + // then + assert_eq!(tester.io.handle_request(&request), Some(response.to_owned())); +} + + +#[test] +fn should_reject_transaction_from_queue_without_dispatching() { + // given + let tester = signer_tester(); + tester.queue.add_request(TransactionRequest { + from: Address::from(1), + to: Some(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), + gas_price: Some(U256::from(10_000)), + gas: Some(U256::from(10_000_000)), + value: Some(U256::from(1)), + data: None, + nonce: None, + }); + assert_eq!(tester.queue.requests().len(), 1); + + // when + let request = r#"{"jsonrpc":"2.0","method":"personal_rejectTransaction","params":["0x01"],"id":1}"#; + let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; + + // then + assert_eq!(tester.io.handle_request(&request), Some(response.to_owned())); + assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.miner.imported_transactions.lock().unwrap().len(), 0); +} + +#[test] +fn should_confirm_transaction_and_dispatch() { + // given + let tester = signer_tester(); + let account = TestAccount::new("test"); + let address = account.address(); + let secret = account.secret.clone(); + let recipient = Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap(); + tester.accounts.accounts + .write() + .unwrap() + .insert(address, account); + tester.queue.add_request(TransactionRequest { + from: address, + to: Some(recipient), + gas_price: Some(U256::from(10_000)), + gas: Some(U256::from(10_000_000)), + value: Some(U256::from(1)), + data: None, + nonce: None, + }); + let t = Transaction { + nonce: U256::zero(), + gas_price: U256::from(0x1000), + gas: U256::from(10_000_000), + action: Action::Call(recipient), + value: U256::from(0x1), + data: vec![] + }.sign(&secret); + + assert_eq!(tester.queue.requests().len(), 1); + + // when + let request = r#"{ + "jsonrpc":"2.0", + "method":"personal_confirmTransaction", + "params":["0x01", {"gasPrice":"0x1000"}, "test"], + "id":1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#; + + // then + assert_eq!(tester.io.handle_request(&request), Some(response.to_owned())); + assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.miner.imported_transactions.lock().unwrap().len(), 1); +} diff --git a/rpc/src/v1/traits/mod.rs b/rpc/src/v1/traits/mod.rs index 2355d6137..5384b0ef4 100644 --- a/rpc/src/v1/traits/mod.rs +++ b/rpc/src/v1/traits/mod.rs @@ -31,7 +31,7 @@ pub mod rpc; pub use self::web3::Web3; pub use self::eth::{Eth, EthFilter, EthSigning}; pub use self::net::Net; -pub use self::personal::{Personal, SignerPersonal}; +pub use self::personal::{Personal, PersonalSigner}; pub use self::ethcore::Ethcore; pub use self::traces::Traces; pub use self::rpc::Rpc; diff --git a/rpc/src/v1/traits/personal.rs b/rpc/src/v1/traits/personal.rs index d8eb7ee75..cde66be2c 100644 --- a/rpc/src/v1/traits/personal.rs +++ b/rpc/src/v1/traits/personal.rs @@ -45,7 +45,7 @@ pub trait Personal: Sized + Send + Sync + 'static { } /// Personal extension for transactions confirmations rpc interface. -pub trait SignerPersonal: Sized + Send + Sync + 'static { +pub trait PersonalSigner: Sized + Send + Sync + 'static { /// Returns a list of transactions to confirm. fn transactions_to_confirm(&self, _: Params) -> Result; @@ -59,9 +59,9 @@ pub trait SignerPersonal: Sized + Send + Sync + 'static { /// 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("personal_transactionsToConfirm", SignerPersonal::transactions_to_confirm); - delegate.add_method("personal_confirmTransaction", SignerPersonal::confirm_transaction); - delegate.add_method("personal_rejectTransaction", SignerPersonal::reject_transaction); + delegate.add_method("personal_transactionsToConfirm", PersonalSigner::transactions_to_confirm); + delegate.add_method("personal_confirmTransaction", PersonalSigner::confirm_transaction); + delegate.add_method("personal_rejectTransaction", PersonalSigner::reject_transaction); delegate } } diff --git a/rpc/src/v1/types/mod.rs.in b/rpc/src/v1/types/mod.rs.in index 824a061ef..b4e82a28b 100644 --- a/rpc/src/v1/types/mod.rs.in +++ b/rpc/src/v1/types/mod.rs.in @@ -38,7 +38,7 @@ pub use self::log::Log; pub use self::optionals::OptionalValue; pub use self::sync::{SyncStatus, SyncInfo}; pub use self::transaction::Transaction; -pub use self::transaction_request::TransactionRequest; +pub use self::transaction_request::{TransactionRequest, TransactionConfirmation, TransactionModification}; pub use self::call_request::CallRequest; pub use self::receipt::Receipt; pub use self::trace::Trace; diff --git a/rpc/src/v1/types/transaction_request.rs b/rpc/src/v1/types/transaction_request.rs index 276f14f07..93d6a479b 100644 --- a/rpc/src/v1/types/transaction_request.rs +++ b/rpc/src/v1/types/transaction_request.rs @@ -40,6 +40,24 @@ pub struct TransactionRequest { pub nonce: Option, } +/// Transaction confirmation waiting in a queue +#[derive(Debug, Clone, Default, Eq, PartialEq, Hash, Serialize)] +pub struct TransactionConfirmation { + /// Id of this confirmation + pub id: U256, + /// TransactionRequest + pub transaction: TransactionRequest, +} + +/// Possible modifications to the confirmed transaction sent by SystemUI +#[derive(Debug, PartialEq, Deserialize)] +pub struct TransactionModification { + /// Modified gas price + #[serde(rename="gasPrice")] + pub gas_price: Option, +} + + #[cfg(test)] mod tests { use std::str::FromStr; @@ -135,5 +153,26 @@ mod tests { nonce: None, }); } + + #[test] + fn should_deserialize_modification() { + // given + let s1 = r#"{ + "gasPrice":"0x0ba43b7400" + }"#; + let s2 = r#"{}"#; + + // when + let res1: TransactionModification = serde_json::from_str(s1).unwrap(); + let res2: TransactionModification = serde_json::from_str(s2).unwrap(); + + // then + assert_eq!(res1, TransactionModification { + gas_price: Some(U256::from_str("0ba43b7400").unwrap()), + }); + assert_eq!(res2, TransactionModification { + gas_price: None, + }); + } }