From 404005cc937c0b7239a7ab7f194d0691ebb7570f Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 1 Apr 2016 11:26:14 +0200 Subject: [PATCH 01/27] fixed #855 --- ethcore/src/client/client.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index d2bb69267..e61db1878 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -30,7 +30,7 @@ use service::{NetSyncMessage, SyncMessage}; use env_info::LastHashes; use verification::*; use block::*; -use transaction::{LocalizedTransaction, SignedTransaction}; +use transaction::{LocalizedTransaction, SignedTransaction, Action}; use extras::TransactionAddress; use filter::Filter; use log_entry::LocalizedLogEntry; @@ -38,7 +38,7 @@ use block_queue::{BlockQueue, BlockQueueInfo}; use blockchain::{BlockChain, BlockProvider, TreeRoute, ImportRoute}; use client::{BlockId, TransactionId, UncleId, ClientConfig, BlockChainClient}; use env_info::EnvInfo; -use executive::{Executive, Executed}; +use executive::{Executive, Executed, contract_address}; use receipt::LocalizedReceipt; pub use blockchain::CacheSize as BlockChainCacheSize; @@ -573,8 +573,10 @@ impl BlockChainClient for Client where V: Verifier { // TODO: to fix this, query all previous transaction receipts and retrieve their gas usage cumulative_gas_used: receipt.gas_used, gas_used: receipt.gas_used, - // TODO: to fix this, store created contract address in db - contract_address: None, + contract_address: match tx.action { + Action::Call(_) => None, + Action::Create => Some(contract_address(&tx.sender().unwrap(), &tx.nonce)) + }, logs: receipt.logs.into_iter().enumerate().map(|(i, log)| LocalizedLogEntry { entry: log, block_hash: block_hash.clone(), From 9750a313365b60ba682dc2a2d9918ce88727b06e Mon Sep 17 00:00:00 2001 From: NikVolf Date: Fri, 1 Apr 2016 17:56:29 +0300 Subject: [PATCH 02/27] replace popcnt with mov --- util/bigint/src/uint.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/util/bigint/src/uint.rs b/util/bigint/src/uint.rs index 7bc6007ed..330d20e36 100644 --- a/util/bigint/src/uint.rs +++ b/util/bigint/src/uint.rs @@ -340,31 +340,31 @@ macro_rules! uint_overflowing_mul { cmpq $$0, %rcx jne 2f - popcnt $8, %rcx + mov $8, %rcx jrcxz 12f - popcnt $12, %rcx - popcnt $11, %rax + mov $12, %rcx + mov $11, %rax add %rax, %rcx - popcnt $10, %rax + mov $10, %rax add %rax, %rcx jmp 2f 12: - popcnt $12, %rcx + mov $12, %rcx jrcxz 11f - popcnt $7, %rcx - popcnt $6, %rax + movt $7, %rcx + mov $6, %rax add %rax, %rcx cmpq $$0, %rcx jne 2f 11: - popcnt $11, %rcx + mov $11, %rcx jrcxz 2f - popcnt $7, %rcx + mov $7, %rcx 2: " From 4aeaab3a2faa007f3af71b877bedd4832fd4936c Mon Sep 17 00:00:00 2001 From: arkpar Date: Sat, 2 Apr 2016 19:01:41 +0200 Subject: [PATCH 03/27] Fixed bootnode URL and error message --- ethcore/res/ethereum/frontier.json | 2 +- util/src/network/host.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ethcore/res/ethereum/frontier.json b/ethcore/res/ethereum/frontier.json index 8f9209179..41e69d24d 100644 --- a/ethcore/res/ethereum/frontier.json +++ b/ethcore/res/ethereum/frontier.json @@ -30,7 +30,7 @@ "enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@52.16.188.185:30303", "enode://de471bccee3d042261d52e9bff31458daecc406142b401d4cd848f677479f73104b9fdeb090af9583d3391b7f10cb2ba9e26865dd5fca4fcdc0fb1e3b723c786@54.94.239.50:30303", "enode://1118980bf48b0a3640bdba04e0fe78b1add18e1cd99bf22d53daac1fd9972ad650df52176e7c7d89d1114cfef2bc23a2959aa54998a46afcf7d91809f0855082@52.74.57.123:30303", - "enode://248f12bc8b18d5289358085520ac78cd8076485211e6d96ab0bc93d6cd25442db0ce3a937dc404f64f207b0b9aed50e25e98ce32af5ac7cb321ff285b97de485@parity-node-zero.ethcore.io:30303" + "enode://248f12bc8b18d5289358085520ac78cd8076485211e6d96ab0bc93d6cd25442db0ce3a937dc404f64f207b0b9aed50e25e98ce32af5ac7cb321ff285b97de485@zero.parity.io:30303" ], "accounts": { "0000000000000000000000000000000000000001": { "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, diff --git a/util/src/network/host.rs b/util/src/network/host.rs index 02c576424..d0f9e8abf 100644 --- a/util/src/network/host.rs +++ b/util/src/network/host.rs @@ -381,7 +381,7 @@ impl Host where Message: Send + Sync + Clone { pub fn add_node(&mut self, id: &str) { match Node::from_str(id) { - Err(e) => { warn!("Could not add node: {:?}", e); }, + Err(e) => { debug!("Could not add node {}: {:?}", id, e); }, Ok(n) => { let entry = NodeEntry { endpoint: n.endpoint.clone(), id: n.id.clone() }; self.pinned_nodes.push(n.id.clone()); From 584d89f9641eb8b400e9fa9f2ab2cb82bdf06431 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 3 Apr 2016 00:42:33 -0400 Subject: [PATCH 04/27] Find geth data store cross-platform. Fixes #869 --- util/src/keys/geth_import.rs | 32 ++++++++++++++++++++++++++++++++ util/src/keys/store.rs | 8 ++------ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/util/src/keys/geth_import.rs b/util/src/keys/geth_import.rs index 6c684c37d..7f17144bf 100644 --- a/util/src/keys/geth_import.rs +++ b/util/src/keys/geth_import.rs @@ -19,6 +19,7 @@ use common::*; use keys::store::SecretStore; use keys::directory::KeyFileContent; +use std::path::PathBuf; /// Enumerates all geth keys in the directory and returns collection of tuples `(accountId, filename)` pub fn enumerate_geth_keys(path: &Path) -> Result, io::Error> { @@ -93,6 +94,37 @@ pub fn import_geth_keys(secret_store: &mut SecretStore, geth_keyfiles_directory: Ok(()) } + +/// Gets the default geth keystore directory. +/// +/// Based on https://github.com/ethereum/go-ethereum/blob/e553215/common/path.go#L75 +pub fn keystore_dir() -> PathBuf { + #[cfg(target_os = "macos")] + fn data_dir(mut home: PathBuf) -> PathBuf { + home.push("Library"); + home.push("Ethereum"); + home + } + + #[cfg(windows)] + fn data_dir(mut home: PathBuf) -> PathBuf { + home.push("AppData"); + home.push("Roaming"); + home.push("Ethereum"); + home + } + + #[cfg(not(any(target_os = "macos", windows)))] + fn data_dir(mut home: PathBuf) -> PathBuf { + home.push(".ethereum"); + home + } + + let mut data_dir = data_dir(::std::env::home_dir().expect("Failed to get home dir")); + data_dir.push("keystore"); + data_dir +} + #[cfg(test)] mod tests { use super::*; diff --git a/util/src/keys/store.rs b/util/src/keys/store.rs index f98efd4be..a4b6f2c7b 100644 --- a/util/src/keys/store.rs +++ b/util/src/keys/store.rs @@ -150,7 +150,7 @@ impl AccountService { self.secret_store.write().unwrap().collect_garbage(); } - /// Unlocks account for use (no expiration of unlock) + /// Unlocks account for use (no expiration of unlock) pub fn unlock_account_no_expire(&self, account: &Address, pass: &str) -> Result<(), EncryptedHashMapError> { self.secret_store.write().unwrap().unlock_account_with_expiration(account, pass, None) } @@ -183,13 +183,9 @@ impl SecretStore { /// trys to import keys in the known locations pub fn try_import_existing(&mut self) { - use std::path::PathBuf; use keys::geth_import; - let mut import_path = PathBuf::new(); - import_path.push(::std::env::home_dir().expect("Failed to get home dir")); - import_path.push(".ethereum"); - import_path.push("keystore"); + let import_path = geth_import::keystore_dir(); if let Err(e) = geth_import::import_geth_keys(self, &import_path) { trace!(target: "sstore", "Geth key not imported: {:?}", e); } From a7ebc1dd25a0f15212a47c610be35c541a093c61 Mon Sep 17 00:00:00 2001 From: arkpar Date: Sun, 3 Apr 2016 11:36:21 +0200 Subject: [PATCH 05/27] Handle geth keys with lowercase `crypto` key --- ...--63121b431a52f8043c16fcf0d1df9cb7b5f66649 | 1 + util/src/keys/geth_import.rs | 28 ++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) create mode 100644 util/res/geth_keystore/UTC--2016-04-03T08-58-49.834202900Z--63121b431a52f8043c16fcf0d1df9cb7b5f66649 diff --git a/util/res/geth_keystore/UTC--2016-04-03T08-58-49.834202900Z--63121b431a52f8043c16fcf0d1df9cb7b5f66649 b/util/res/geth_keystore/UTC--2016-04-03T08-58-49.834202900Z--63121b431a52f8043c16fcf0d1df9cb7b5f66649 new file mode 100644 index 000000000..08272d43b --- /dev/null +++ b/util/res/geth_keystore/UTC--2016-04-03T08-58-49.834202900Z--63121b431a52f8043c16fcf0d1df9cb7b5f66649 @@ -0,0 +1 @@ +{"address":"63121b431a52f8043c16fcf0d1df9cb7b5f66649","crypto":{"cipher":"aes-128-ctr","ciphertext":"1dd21926c644b9983916d646f3a4f2c7f9362f7e1c9fb1abcb42494dae06fa01","cipherparams":{"iv":"c52c6ee66d89a7aa8c6839f4b6ed29c8"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"96f17c17bbf48db2dc4da00b3e7decce8e21f44a5d7963dadeeff70e1d38ad75"},"mac":"f279f3444585c2817701225e2196c1176386ad549ebaec2bcc4f94f309727fe6"},"id":"15e49cd2-51fb-4316-ba46-c3cf8db4ae44","version":3} \ No newline at end of file diff --git a/util/src/keys/geth_import.rs b/util/src/keys/geth_import.rs index 7f17144bf..9886a61a8 100644 --- a/util/src/keys/geth_import.rs +++ b/util/src/keys/geth_import.rs @@ -22,7 +22,7 @@ use keys::directory::KeyFileContent; use std::path::PathBuf; /// Enumerates all geth keys in the directory and returns collection of tuples `(accountId, filename)` -pub fn enumerate_geth_keys(path: &Path) -> Result, io::Error> { +pub fn enumerate_geth_keys(path: &Path) -> Result, ImportError> { let mut entries = Vec::new(); for entry in try!(fs::read_dir(path)) { let entry = try!(entry); @@ -31,10 +31,8 @@ pub fn enumerate_geth_keys(path: &Path) -> Result, io::Er Some(name) => { let parts: Vec<&str> = name.split("--").collect(); if parts.len() != 3 { continue; } - match Address::from_str(parts[2]) { - Ok(account_id) => { entries.push((account_id, name.to_owned())); } - Err(e) => { panic!("error: {:?}", e); } - } + let account_id = try!(Address::from_str(parts[2]).map_err(|_| ImportError::Format)); + entries.push((account_id, name.to_owned())); }, None => { continue; } }; @@ -69,9 +67,10 @@ pub fn import_geth_key(secret_store: &mut SecretStore, geth_keyfile_path: &Path) Ok(ref mut parsed_json) => try!(parsed_json.as_object_mut().ok_or(ImportError::Format)), Err(_) => { return Err(ImportError::Format); } }; - let crypto_object = try!(json.get("Crypto").and_then(|crypto| crypto.as_object()).ok_or(ImportError::Format)).clone(); - json.insert("crypto".to_owned(), Json::Object(crypto_object)); - json.remove("Crypto"); + if let Some(crypto_object) = json.get("Crypto").and_then(|crypto| crypto.as_object()).cloned() { + json.insert("crypto".to_owned(), Json::Object(crypto_object)); + json.remove("Crypto"); + } match KeyFileContent::load(&Json::Object(json.clone())) { Ok(key_file) => try!(secret_store.import_key(key_file)), Err(_) => { return Err(ImportError::Format); } @@ -145,11 +144,11 @@ mod tests { #[test] fn can_enumerate() { let keys = enumerate_geth_keys(Path::new(test_path())).unwrap(); - assert_eq!(2, keys.len()); + assert_eq!(3, keys.len()); } #[test] - fn can_import() { + fn can_import_geth_old() { let temp = ::devtools::RandomTempPath::create_dir(); let mut secret_store = SecretStore::new_in(temp.as_path()); import_geth_key(&mut secret_store, Path::new(&test_path_param("/UTC--2016-02-17T09-20-45.721400158Z--3f49624084b67849c7b4e805c5988c21a430f9d9"))).unwrap(); @@ -157,6 +156,15 @@ mod tests { assert!(key.is_some()); } + #[test] + fn can_import_geth140() { + let temp = ::devtools::RandomTempPath::create_dir(); + let mut secret_store = SecretStore::new_in(temp.as_path()); + import_geth_key(&mut secret_store, Path::new(&test_path_param("/UTC--2016-04-03T08-58-49.834202900Z--63121b431a52f8043c16fcf0d1df9cb7b5f66649"))).unwrap(); + let key = secret_store.account(&Address::from_str("63121b431a52f8043c16fcf0d1df9cb7b5f66649").unwrap()); + assert!(key.is_some()); + } + #[test] fn can_import_directory() { let temp = ::devtools::RandomTempPath::create_dir(); From 8503aeb2df11d0efc5958e851665046b9b055295 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Mon, 4 Apr 2016 04:03:20 +0400 Subject: [PATCH 06/27] [ci skip] update misleading cli help msg for author --- parity/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parity/main.rs b/parity/main.rs index 9ad018bd9..28422e857 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -327,7 +327,7 @@ impl Configuration { fn author(&self) -> Address { let d = self.args.flag_etherbase.as_ref().unwrap_or(&self.args.flag_author); Address::from_str(clean_0x(d)).unwrap_or_else(|_| { - die!("{}: Invalid address for --author. Must be 40 hex characters, without the 0x at the beginning.", d) + die!("{}: Invalid address for --author. Must be 40 hex characters, with or without the 0x at the beginning.", d) }) } From 66d3d7ec115fc59c6ced5b04af86b9b636f40cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 4 Apr 2016 09:00:22 +0200 Subject: [PATCH 07/27] Fixing typo in bigint --- util/bigint/src/uint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/bigint/src/uint.rs b/util/bigint/src/uint.rs index 330d20e36..1a23ad085 100644 --- a/util/bigint/src/uint.rs +++ b/util/bigint/src/uint.rs @@ -354,7 +354,7 @@ macro_rules! uint_overflowing_mul { mov $12, %rcx jrcxz 11f - movt $7, %rcx + mov $7, %rcx mov $6, %rax add %rax, %rcx From aed708ffd80123d3d08d44bd5a8fcf6be85d6e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 4 Apr 2016 08:55:47 +0200 Subject: [PATCH 08/27] More descriptive expectations to transaction queue consistency. --- miner/src/transaction_queue.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index bfbbfaecf..18eecd0e4 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -213,14 +213,15 @@ impl TransactionSet { self.by_priority .iter() .skip(self.limit) - .map(|order| by_hash.get(&order.hash).expect("Inconsistency in queue detected.")) + .map(|order| by_hash.get(&order.hash) + .expect("All transactions in `self.by_priority` and `self.by_address` are kept in sync with `by_hash`.")) .map(|tx| (tx.sender(), tx.nonce())) .collect() }; for (sender, nonce) in to_drop { - let order = self.drop(&sender, &nonce).expect("Dropping transaction found in priority queue failed."); - by_hash.remove(&order.hash).expect("Inconsistency in queue."); + let order = self.drop(&sender, &nonce).expect("Transaction has just been found in `by_priority`; so it is in `by_address` also."); + by_hash.remove(&order.hash).expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`"); } } @@ -497,7 +498,7 @@ impl TransactionQueue { pub fn top_transactions(&self) -> Vec { self.current.by_priority .iter() - .map(|t| self.by_hash.get(&t.hash).expect("Transaction Queue Inconsistency")) + .map(|t| self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`")) .map(|t| t.transaction.clone()) .collect() } From a3a0de096dc20f0a1aaa33a7d90fd3ba72031a76 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Mon, 4 Apr 2016 11:06:16 +0200 Subject: [PATCH 09/27] replace add with or --- util/bigint/src/uint.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/util/bigint/src/uint.rs b/util/bigint/src/uint.rs index 1a23ad085..0336e8c7f 100644 --- a/util/bigint/src/uint.rs +++ b/util/bigint/src/uint.rs @@ -345,9 +345,9 @@ macro_rules! uint_overflowing_mul { mov $12, %rcx mov $11, %rax - add %rax, %rcx + or %rax, %rcx mov $10, %rax - add %rax, %rcx + or %rax, %rcx jmp 2f 12: @@ -356,7 +356,7 @@ macro_rules! uint_overflowing_mul { mov $7, %rcx mov $6, %rax - add %rax, %rcx + or %rax, %rcx cmpq $$0, %rcx jne 2f From 33d633eed114a00591039e9cd774407aecf6b58e Mon Sep 17 00:00:00 2001 From: debris Date: Wed, 6 Apr 2016 12:15:20 +0200 Subject: [PATCH 10/27] fixed #875 and added tests for eth_sendTransaction --- miner/src/lib.rs | 3 ++ miner/src/miner.rs | 4 ++ miner/src/transaction_queue.rs | 30 +++++++++++ rpc/src/v1/impls/eth.rs | 14 ++++-- rpc/src/v1/impls/personal.rs | 2 +- rpc/src/v1/tests/eth.rs | 53 ++++++++++++++++++-- rpc/src/v1/tests/helpers/account_provider.rs | 33 ++++++++---- rpc/src/v1/tests/helpers/miner_service.rs | 47 +++++++++++++---- rpc/src/v1/tests/personal.rs | 28 ++++++++--- 9 files changed, 178 insertions(+), 36 deletions(-) diff --git a/miner/src/lib.rs b/miner/src/lib.rs index 92811b82e..e45411f9a 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -109,6 +109,9 @@ pub trait MinerService : Send + Sync { /// Query pending transactions for hash fn transaction(&self, hash: &H256) -> Option; + /// Returns highest transaction nonce for given address. + fn last_nonce(&self, address: &Address) -> Option; + /// Suggested gas price fn sensible_gas_price(&self) -> U256 { x!(20000000000u64) } } diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 04e98cecb..dff5ab48d 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -175,6 +175,10 @@ impl MinerService for Miner { queue.find(hash) } + fn last_nonce(&self, address: &Address) -> Option { + self.transaction_queue.lock().unwrap().last_nonce(address) + } + fn update_sealing(&self, chain: &BlockChainClient) { if self.sealing_enabled.load(atomic::Ordering::Relaxed) { let current_no = chain.chain_info().best_block_number; diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 18eecd0e4..d1f468ade 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -524,6 +524,11 @@ impl TransactionQueue { self.last_nonces.clear(); } + /// Returns highest transaction nonce for given address. + pub fn last_nonce(&self, address: &Address) -> Option { + self.last_nonces.get(address).cloned() + } + /// Checks if there are any transactions in `future` that should actually be promoted to `current` /// (because nonce matches). fn move_matching_future_to_current(&mut self, address: Address, mut current_nonce: U256, first_nonce: U256) { @@ -1256,4 +1261,29 @@ mod test { assert_eq!(stats.future, 0); assert_eq!(stats.pending, 1); } + + #[test] + fn should_return_none_when_transaction_from_given_address_does_not_exist() { + // given + let mut txq = TransactionQueue::new(); + + // then + assert_eq!(txq.last_nonce(&Address::default()), None); + } + + #[test] + fn should_return_correct_nonce_when_transactions_from_given_address_exist() { + // given + let mut txq = TransactionQueue::new(); + let tx = new_tx(); + let from = tx.sender().unwrap(); + let nonce = tx.nonce; + let details = |a: &Address| AccountDetails { nonce: nonce, balance: !U256::zero() }; + + // when + txq.add(tx, &details).unwrap(); + + // then + assert_eq!(txq.last_nonce(&from), Some(nonce)); + } } diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 786abac8c..cbb3cdba6 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -178,11 +178,15 @@ impl EthClient fn dispatch_transaction(&self, signed_transaction: SignedTransaction, raw_transaction: Vec) -> Result { let hash = signed_transaction.hash(); - + let import = { + let miner = take_weak!(self.miner); let client = take_weak!(self.client); take_weak!(self.miner).import_transactions(vec![signed_transaction], |a: &Address| AccountDetails { - nonce: client.nonce(a), + nonce: miner + .last_nonce(a) + .map(|nonce| nonce + U256::one()) + .unwrap_or_else(|| client.nonce(a)), balance: client.balance(a), }) }; @@ -483,7 +487,11 @@ impl Eth for EthClient let client = take_weak!(self.client); let miner = take_weak!(self.miner); EthTransaction { - nonce: request.nonce.unwrap_or_else(|| client.nonce(&request.from)), + nonce: request.nonce + .or_else(|| miner + .last_nonce(&request.from) + .map(|nonce| nonce + U256::one())) + .unwrap_or_else(|| client.nonce(&request.from)), action: request.to.map_or(Action::Create, Action::Call), gas: request.gas.unwrap_or_else(default_gas), gas_price: request.gas_price.unwrap_or_else(|| miner.sensible_gas_price()), diff --git a/rpc/src/v1/impls/personal.rs b/rpc/src/v1/impls/personal.rs index 2822059d6..5bb0d3eee 100644 --- a/rpc/src/v1/impls/personal.rs +++ b/rpc/src/v1/impls/personal.rs @@ -49,7 +49,7 @@ impl Personal for PersonalClient where A: AccountProvider + 'static { |(pass, )| { let store = take_weak!(self.accounts); match store.new_account(&pass) { - Ok(address) => Ok(Value::String(format!("0x{:?}", address))), + Ok(address) => to_value(&address), Err(_) => Err(Error::internal_error()) } } diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 5d3a0bf29..c0c45da13 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -23,6 +23,7 @@ use util::numbers::{Uint, U256}; use ethcore::client::{TestBlockChainClient, EachBlockWith, Executed, TransactionId}; use ethcore::log_entry::{LocalizedLogEntry, LogEntry}; use ethcore::receipt::LocalizedReceipt; +use ethcore::transaction::{Transaction, Action}; use v1::{Eth, EthClient}; use v1::tests::helpers::{TestAccount, TestAccountProvider, TestSyncProvider, Config, TestMinerService, TestExternalMiner}; @@ -52,7 +53,7 @@ fn miner_service() -> Arc { struct EthTester { pub client: Arc, pub sync: Arc, - _accounts_provider: Arc, + pub accounts_provider: Arc, miner: Arc, hashrates: Arc>>, pub io: IoHandler, @@ -72,7 +73,7 @@ impl Default for EthTester { EthTester { client: client, sync: sync, - _accounts_provider: ap, + accounts_provider: ap, miner: miner, io: io, hashrates: hashrates, @@ -449,9 +450,53 @@ fn rpc_eth_estimate_gas_default_block() { } #[test] -#[ignore] fn rpc_eth_send_transaction() { - unimplemented!() + let account = TestAccount::new("123"); + let address = account.address(); + let secret = account.secret.clone(); + + let tester = EthTester::default(); + tester.accounts_provider.accounts.write().unwrap().insert(address.clone(), account); + let request = r#"{ + "jsonrpc": "2.0", + "method": "eth_sendTransaction", + "params": [{ + "from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"", + "to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567", + "gas": "0x76c0", + "gasPrice": "0x9184e72a000", + "value": "0x9184e72a" + }], + "id": 1 + }"#; + + let t = Transaction { + nonce: U256::zero(), + gas_price: U256::from(0x9184e72a000u64), + gas: U256::from(0x76c0), + action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), + value: U256::from(0x9184e72au64), + data: vec![] + }.sign(&secret); + + 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)); + + tester.miner.last_nonces.write().unwrap().insert(address.clone(), U256::zero()); + + let t = Transaction { + nonce: U256::one(), + gas_price: U256::from(0x9184e72a000u64), + gas: U256::from(0x76c0), + action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), + value: U256::from(0x9184e72au64), + data: vec![] + }.sign(&secret); + + 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)); } #[test] diff --git a/rpc/src/v1/tests/helpers/account_provider.rs b/rpc/src/v1/tests/helpers/account_provider.rs index 6ef6e2b59..cace69658 100644 --- a/rpc/src/v1/tests/helpers/account_provider.rs +++ b/rpc/src/v1/tests/helpers/account_provider.rs @@ -20,7 +20,7 @@ use std::sync::RwLock; use std::collections::HashMap; use std::io; use util::hash::{Address, H256, FixedHash}; -use util::crypto::{Secret, Signature}; +use util::crypto::{Secret, Signature, KeyPair}; use util::keys::store::{AccountProvider, SigningError, EncryptedHashMapError}; /// Account mock. @@ -30,23 +30,31 @@ pub struct TestAccount { pub unlocked: bool, /// Account's password. pub password: String, + /// Account's secret. + pub secret: Secret, } impl TestAccount { /// Creates new test account. pub fn new(password: &str) -> Self { + let pair = KeyPair::create().unwrap(); TestAccount { unlocked: false, password: password.to_owned(), + secret: pair.secret().clone() } } + + /// Returns account address. + pub fn address(&self) -> Address { + KeyPair::from_secret(self.secret.clone()).unwrap().address() + } } /// Test account provider. pub struct TestAccountProvider { - accounts: RwLock>, - /// Added accounts passwords. - pub adds: RwLock>, + /// Test provider accounts. + pub accounts: RwLock>, } impl TestAccountProvider { @@ -54,7 +62,6 @@ impl TestAccountProvider { pub fn new(accounts: HashMap) -> Self { TestAccountProvider { accounts: RwLock::new(accounts), - adds: RwLock::new(vec![]), } } } @@ -76,14 +83,20 @@ impl AccountProvider for TestAccountProvider { } fn new_account(&self, pass: &str) -> Result { - let mut adds = self.adds.write().unwrap(); - let address = Address::from(adds.len() as u64 + 2); - adds.push(pass.to_owned()); + let account = TestAccount::new(pass); + let address = KeyPair::from_secret(account.secret.clone()).unwrap().address(); + self.accounts.write().unwrap().insert(address.clone(), account); Ok(address) } - fn account_secret(&self, _account: &Address) -> Result { - Ok(Secret::random()) + fn account_secret(&self, address: &Address) -> Result { + // todo: consider checking if account is unlock. some test may need alteration then. + self.accounts + .read() + .unwrap() + .get(address) + .ok_or(SigningError::NoAccount) + .map(|acc| acc.secret.clone()) } fn sign(&self, _account: &Address, _message: &H256) -> Result { diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index d00b0dd1e..fe7858a65 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -16,7 +16,7 @@ //! Test implementation of miner service. -use util::{Address, H256, Bytes}; +use util::{Address, H256, Bytes, U256}; use util::standard::*; use ethcore::error::Error; use ethcore::client::BlockChainClient; @@ -27,19 +27,22 @@ use ethminer::{MinerService, MinerStatus, AccountDetails}; /// Test miner service. pub struct TestMinerService { /// Imported transactions. - pub imported_transactions: RwLock>, + pub imported_transactions: Mutex>, /// Latest closed block. pub latest_closed_block: Mutex>, /// Pre-existed pending transactions pub pending_transactions: Mutex>, + /// Last nonces. + pub last_nonces: RwLock>, } impl Default for TestMinerService { fn default() -> TestMinerService { TestMinerService { - imported_transactions: RwLock::new(Vec::new()), + imported_transactions: Mutex::new(Vec::new()), latest_closed_block: Mutex::new(None), pending_transactions: Mutex::new(HashMap::new()), + last_nonces: RwLock::new(HashMap::new()), } } } @@ -56,20 +59,36 @@ impl MinerService for TestMinerService { } /// Imports transactions to transaction queue. - fn import_transactions(&self, _transactions: Vec, _fetch_account: T) -> Vec> - where T: Fn(&Address) -> AccountDetails { unimplemented!(); } + fn import_transactions(&self, transactions: Vec, _fetch_account: T) -> Vec> + where T: Fn(&Address) -> AccountDetails { + // lets assume that all txs are valid + self.imported_transactions.lock().unwrap().extend_from_slice(&transactions); + + transactions + .iter() + .map(|_| Ok(())) + .collect() + } /// Returns hashes of transactions currently in pending - fn pending_transactions_hashes(&self) -> Vec { vec![] } + fn pending_transactions_hashes(&self) -> Vec { + vec![] + } /// Removes all transactions from the queue and restart mining operation. - fn clear_and_reset(&self, _chain: &BlockChainClient) { unimplemented!(); } + fn clear_and_reset(&self, _chain: &BlockChainClient) { + unimplemented!(); + } /// Called when blocks are imported to chain, updates transactions queue. - fn chain_new_blocks(&self, _chain: &BlockChainClient, _imported: &[H256], _invalid: &[H256], _enacted: &[H256], _retracted: &[H256]) { unimplemented!(); } + fn chain_new_blocks(&self, _chain: &BlockChainClient, _imported: &[H256], _invalid: &[H256], _enacted: &[H256], _retracted: &[H256]) { + unimplemented!(); + } /// New chain head event. Restart mining operation. - fn update_sealing(&self, _chain: &BlockChainClient) { unimplemented!(); } + fn update_sealing(&self, _chain: &BlockChainClient) { + unimplemented!(); + } /// Grab the `ClosedBlock` that we want to be sealed. Comes as a mutex that you have to lock. fn sealing_block(&self, _chain: &BlockChainClient) -> &Mutex> { @@ -77,10 +96,16 @@ impl MinerService for TestMinerService { } fn transaction(&self, hash: &H256) -> Option { - self.pending_transactions.lock().unwrap().get(hash).and_then(|tx_ref| Some(tx_ref.clone())) + self.pending_transactions.lock().unwrap().get(hash).cloned() + } + + fn last_nonce(&self, address: &Address) -> Option { + self.last_nonces.read().unwrap().get(address).cloned() } /// Submit `seal` as a valid solution for the header of `pow_hash`. /// Will check the seal, but not actually insert the block into the chain. - fn submit_seal(&self, _chain: &BlockChainClient, _pow_hash: H256, _seal: Vec) -> Result<(), Error> { unimplemented!(); } + fn submit_seal(&self, _chain: &BlockChainClient, _pow_hash: H256, _seal: Vec) -> Result<(), Error> { + unimplemented!(); + } } diff --git a/rpc/src/v1/tests/personal.rs b/rpc/src/v1/tests/personal.rs index 261527c47..458669b42 100644 --- a/rpc/src/v1/tests/personal.rs +++ b/rpc/src/v1/tests/personal.rs @@ -22,8 +22,7 @@ use util::numbers::*; use std::collections::*; fn accounts_provider() -> Arc { - let mut accounts = HashMap::new(); - accounts.insert(Address::from(1), TestAccount::new("test")); + let accounts = HashMap::new(); let ap = TestAccountProvider::new(accounts); Arc::new(ap) } @@ -38,7 +37,11 @@ fn setup() -> (Arc, IoHandler) { #[test] fn accounts() { - let (_test_provider, io) = setup(); + let (test_provider, io) = setup(); + test_provider.accounts + .write() + .unwrap() + .insert(Address::from(1), TestAccount::new("test")); let request = r#"{"jsonrpc": "2.0", "method": "personal_listAccounts", "params": [], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":["0x0000000000000000000000000000000000000001"],"id":1}"#; @@ -49,11 +52,22 @@ fn accounts() { #[test] fn new_account() { - let (_test_provider, io) = setup(); - + let (test_provider, io) = setup(); let request = r#"{"jsonrpc": "2.0", "method": "personal_newAccount", "params": ["pass"], "id": 1}"#; - let response = r#"{"jsonrpc":"2.0","result":"0x0000000000000000000000000000000000000002","id":1}"#; - assert_eq!(io.handle_request(request), Some(response.to_owned())); + let res = io.handle_request(request); + + let accounts = test_provider.accounts.read().unwrap(); + assert_eq!(accounts.len(), 1); + + let address = accounts + .keys() + .nth(0) + .cloned() + .unwrap(); + + let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"","id":1}"#; + + assert_eq!(res, Some(response)); } From c5553c1f21d99e5d2d424007764f04544bbadb07 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Wed, 6 Apr 2016 14:03:53 +0300 Subject: [PATCH 11/27] passing key path to all invocations --- parity/main.rs | 9 ++++----- util/src/keys/store.rs | 11 ++++++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index 28422e857..2961d4f67 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -77,7 +77,7 @@ Parity. Ethereum Client. Usage: parity daemon [options] - parity account (new | list) + parity account (new | list) [options] parity [options] Protocol Options: @@ -88,7 +88,7 @@ Protocol Options: -d --db-path PATH Specify the database & configuration directory path [default: $HOME/.parity]. --keys-path PATH Specify the path for JSON key files to be found - [default: $HOME/.web3/keys]. + [default: $HOME/.parity/keys]. --identity NAME Specify your node's name. Account Options: @@ -475,7 +475,7 @@ impl Configuration { fn execute_account_cli(&self) { use util::keys::store::SecretStore; use rpassword::read_password; - let mut secret_store = SecretStore::new(); + let mut secret_store = SecretStore::new_in(Path::new(&self.args.flag_keys_path)); if self.args.cmd_new { println!("Please note that password is NOT RECOVERABLE."); println!("Type password: "); @@ -508,8 +508,7 @@ impl Configuration { .collect::>() .into_iter() }).collect::>(); - - let account_service = AccountService::new(); + let account_service = AccountService::new_in(Path::new(&self.args.flag_keys_path)); for d in &self.args.flag_unlock { let a = Address::from_str(clean_0x(&d)).unwrap_or_else(|_| { die!("{}: Invalid address for --unlock. Must be 40 hex characters, without the 0x at the beginning.", d) diff --git a/util/src/keys/store.rs b/util/src/keys/store.rs index a4b6f2c7b..5dec27fc3 100644 --- a/util/src/keys/store.rs +++ b/util/src/keys/store.rs @@ -128,7 +128,7 @@ impl Default for AccountService { } impl AccountService { - /// New account service with the default location + /// New account service with the keys store in default location pub fn new() -> Self { let secret_store = RwLock::new(SecretStore::new()); secret_store.write().unwrap().try_import_existing(); @@ -137,6 +137,15 @@ impl AccountService { } } + /// New account service with the keys store in specific location + pub fn new_in(path: &Path) -> Self { + let secret_store = RwLock::new(SecretStore::new_in(path)); + secret_store.write().unwrap().try_import_existing(); + AccountService { + secret_store: secret_store + } + } + #[cfg(test)] fn new_test(temp: &::devtools::RandomTempPath) -> Self { let secret_store = RwLock::new(SecretStore::new_test(temp)); From 4ed15931af4aba65ed7205055ea2287bac2caeca Mon Sep 17 00:00:00 2001 From: NikVolf Date: Wed, 6 Apr 2016 14:21:19 +0300 Subject: [PATCH 12/27] replacing /home/nikky also --- parity/main.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index 2961d4f67..1c933bf06 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -353,7 +353,7 @@ impl Configuration { } } - fn _keys_path(&self) -> String { + fn keys_path(&self) -> String { self.args.flag_keys_path.replace("$HOME", env::home_dir().unwrap().to_str().unwrap()) } @@ -475,7 +475,7 @@ impl Configuration { fn execute_account_cli(&self) { use util::keys::store::SecretStore; use rpassword::read_password; - let mut secret_store = SecretStore::new_in(Path::new(&self.args.flag_keys_path)); + let mut secret_store = SecretStore::new_in(Path::new(&self.keys_path())); if self.args.cmd_new { println!("Please note that password is NOT RECOVERABLE."); println!("Type password: "); @@ -508,7 +508,8 @@ impl Configuration { .collect::>() .into_iter() }).collect::>(); - let account_service = AccountService::new_in(Path::new(&self.args.flag_keys_path)); + + let account_service = AccountService::new_in(Path::new(&self.keys_path())); for d in &self.args.flag_unlock { let a = Address::from_str(clean_0x(&d)).unwrap_or_else(|_| { die!("{}: Invalid address for --unlock. Must be 40 hex characters, without the 0x at the beginning.", d) From bf24355480b30e6bc55172c952eee592513e305f Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 6 Apr 2016 13:05:58 +0200 Subject: [PATCH 13/27] Fixed eth_call nonce and gas handling --- ethcore/src/client/client.rs | 5 +++-- ethcore/src/executive.rs | 31 +++++++++++++++++++++++-------- ethcore/src/state.rs | 5 +++-- rpc/src/v1/impls/eth.rs | 6 +++++- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index e61db1878..0e8dc9b73 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -38,7 +38,7 @@ use block_queue::{BlockQueue, BlockQueueInfo}; use blockchain::{BlockChain, BlockProvider, TreeRoute, ImportRoute}; use client::{BlockId, TransactionId, UncleId, ClientConfig, BlockChainClient}; use env_info::EnvInfo; -use executive::{Executive, Executed, contract_address}; +use executive::{Executive, Executed, TransactOptions, contract_address}; use receipt::LocalizedReceipt; pub use blockchain::CacheSize as BlockChainCacheSize; @@ -419,7 +419,8 @@ impl BlockChainClient for Client where V: Verifier { // give the sender max balance state.sub_balance(&sender, &balance); state.add_balance(&sender, &U256::max_value()); - Executive::new(&mut state, &env_info, self.engine.deref().deref()).transact(t) + let options = TransactOptions { tracing: false, check_nonce: false }; + Executive::new(&mut state, &env_info, self.engine.deref().deref()).transact(t, options) } // TODO [todr] Should be moved to miner crate eventually. diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 12bc71bb5..17a2fc5e0 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -36,6 +36,14 @@ pub fn contract_address(address: &Address, nonce: &U256) -> Address { From::from(stream.out().sha3()) } +/// Transaction execution options. +pub struct TransactOptions { + /// Enable call tracing. + pub tracing: bool, + /// Check transaction nonce before execution. + pub check_nonce: bool, +} + /// Transaction execution receipt. #[derive(Debug, PartialEq, Clone)] pub struct Executed { @@ -104,7 +112,7 @@ impl<'a> Executive<'a> { } /// This funtion should be used to execute transaction. - pub fn transact(&'a mut self, t: &SignedTransaction) -> Result { + pub fn transact(&'a mut self, t: &SignedTransaction, options: TransactOptions) -> Result { let sender = try!(t.sender()); let nonce = self.state.nonce(&sender); @@ -118,8 +126,10 @@ impl<'a> Executive<'a> { let init_gas = t.gas - base_gas_required; // validate transaction nonce - if t.nonce != nonce { - return Err(From::from(ExecutionError::InvalidNonce { expected: nonce, got: t.nonce })); + if options.check_nonce { + if t.nonce != nonce { + return Err(From::from(ExecutionError::InvalidNonce { expected: nonce, got: t.nonce })); + } } // validate if transaction fits into given block @@ -703,7 +713,8 @@ mod tests { let executed = { let mut ex = Executive::new(&mut state, &info, &engine); - ex.transact(&t).unwrap() + let opts = TransactOptions { check_nonce: true, tracing: false }; + ex.transact(&t, opts).unwrap() }; assert_eq!(executed.gas, U256::from(100_000)); @@ -736,7 +747,8 @@ mod tests { let res = { let mut ex = Executive::new(&mut state, &info, &engine); - ex.transact(&t) + let opts = TransactOptions { check_nonce: true, tracing: false }; + ex.transact(&t, opts) }; match res { @@ -767,7 +779,8 @@ mod tests { let res = { let mut ex = Executive::new(&mut state, &info, &engine); - ex.transact(&t) + let opts = TransactOptions { check_nonce: true, tracing: false }; + ex.transact(&t, opts) }; match res { @@ -800,7 +813,8 @@ mod tests { let res = { let mut ex = Executive::new(&mut state, &info, &engine); - ex.transact(&t) + let opts = TransactOptions { check_nonce: true, tracing: false }; + ex.transact(&t, opts) }; match res { @@ -833,7 +847,8 @@ mod tests { let res = { let mut ex = Executive::new(&mut state, &info, &engine); - ex.transact(&t) + let opts = TransactOptions { check_nonce: true, tracing: false }; + ex.transact(&t, opts) }; match res { diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index cb54654e6..e19e6fe3b 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -16,7 +16,7 @@ use common::*; use engine::Engine; -use executive::Executive; +use executive::{Executive, TransactOptions}; use account_db::*; #[cfg(test)] #[cfg(feature = "json-tests")] @@ -212,7 +212,8 @@ impl State { pub fn apply(&mut self, env_info: &EnvInfo, engine: &Engine, t: &SignedTransaction) -> ApplyResult { // let old = self.to_pod(); - let e = try!(Executive::new(self, env_info, engine).transact(t)); + let options = TransactOptions { tracing: false, check_nonce: true }; + let e = try!(Executive::new(self, env_info, engine).transact(t, options)); // TODO uncomment once to_pod() works correctly. // trace!("Applied transaction. Diff:\n{}\n", StateDiff::diff_pod(&old, &self.to_pod())); diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index cbb3cdba6..c89677689 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -39,6 +39,10 @@ fn default_gas() -> U256 { U256::from(21_000) } +fn default_call_gas() -> U256 { + U256::from(50_000_000) +} + /// Eth rpc implementation. pub struct EthClient where C: BlockChainClient, @@ -169,7 +173,7 @@ impl EthClient Ok(EthTransaction { nonce: request.nonce.unwrap_or_else(|| client.nonce(&from)), action: request.to.map_or(Action::Create, Action::Call), - gas: request.gas.unwrap_or_else(default_gas), + gas: request.gas.unwrap_or_else(default_call_gas), gas_price: request.gas_price.unwrap_or_else(|| miner.sensible_gas_price()), value: request.value.unwrap_or_else(U256::zero), data: request.data.map_or_else(Vec::new, |d| d.to_vec()) From 59a735e2c4e1bf52a69e5f9511b07c094686011f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 6 Apr 2016 10:58:51 +0200 Subject: [PATCH 14/27] Removing match on constant --- ethcore/src/account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index 6901996bc..87f2a05be 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -138,7 +138,7 @@ impl Account { /// get someone who knows to call `note_code`. pub fn code(&self) -> Option<&[u8]> { match self.code_hash { - Some(SHA3_EMPTY) | None if self.code_cache.is_empty() => Some(&self.code_cache), + Some(c) if c == SHA3_EMPTY && self.code_cache.is_empty() => Some(&self.code_cache), Some(_) if !self.code_cache.is_empty() => Some(&self.code_cache), None => Some(&self.code_cache), _ => None, From 0abdcedc038be5675771d5a9ac16599fb687c1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 6 Apr 2016 19:07:27 +0200 Subject: [PATCH 15/27] Additional logging and error messages --- Cargo.lock | 6 ++-- parity/main.rs | 81 ++++++++++++++++++++++++++++++++++---------------- rpc/Cargo.toml | 2 +- rpc/src/lib.rs | 23 +++++++++----- 4 files changed, 76 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab4df68b9..b45f6dbc9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -244,7 +244,7 @@ dependencies = [ "ethminer 1.0.0", "ethsync 1.0.2", "jsonrpc-core 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", - "jsonrpc-http-server 3.0.1 (registry+https://github.com/rust-lang/crates.io-index)", + "jsonrpc-http-server 4.0.0 (git+https://github.com/tomusdrw/jsonrpc-http-server.git)", "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -451,8 +451,8 @@ dependencies = [ [[package]] name = "jsonrpc-http-server" -version = "3.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" +version = "4.0.0" +source = "git+https://github.com/tomusdrw/jsonrpc-http-server.git#46bd4e7cf8352e0efc940cf76d3dff99f1a3da15" dependencies = [ "hyper 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "jsonrpc-core 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/parity/main.rs b/parity/main.rs index 1c933bf06..ebb347cf2 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -258,10 +258,8 @@ fn setup_rpc_server( sync: Arc, secret_store: Arc, miner: Arc, - url: &str, - cors_domain: &str, apis: Vec<&str> -) -> Option> { +) -> Option { use rpc::v1::*; let server = rpc::RpcServer::new(); @@ -279,7 +277,7 @@ fn setup_rpc_server( } } } - Some(server.start_http(url, cors_domain, ::num_cpus::get())) + Some(server) } #[cfg(not(feature = "rpc"))] @@ -288,8 +286,6 @@ fn setup_rpc_server( _sync: Arc, _secret_store: Arc, _miner: Arc, - _url: &str, - _cors_domain: &str, _apis: Vec<&str> ) -> Option> { None @@ -539,7 +535,10 @@ impl Configuration { let account_service = Arc::new(self.account_service()); // Build client - let mut service = ClientService::start(self.client_config(), spec, net_settings, &Path::new(&self.path())).unwrap(); + let mut service = ClientService::start( + self.client_config(), spec, net_settings, &Path::new(&self.path()) + ).unwrap_or_else(|e| die_with_error(e)); + panic_handler.forward_from(&service); let client = service.client(); @@ -554,7 +553,23 @@ impl Configuration { let sync = EthSync::register(service.network(), sync_config, client.clone(), miner.clone()); // Setup rpc - if self.args.flag_jsonrpc || self.args.flag_rpc { + let rpc_server = if self.args.flag_jsonrpc || self.args.flag_rpc { + // TODO: use this as the API list. + let apis = self.args.flag_rpcapi.as_ref().unwrap_or(&self.args.flag_jsonrpc_apis); + setup_rpc_server( + service.client(), + sync.clone(), + account_service.clone(), + miner.clone(), + apis.split(',').collect() + ) + } else { + None + }; + let rpc_handle = rpc_server.map(|server| { + panic_handler.forward_from(&server); + server + }).map(|server| { let url = format!("{}:{}", match self.args.flag_rpcaddr.as_ref().unwrap_or(&self.args.flag_jsonrpc_interface).as_str() { "all" => "0.0.0.0", @@ -564,22 +579,14 @@ impl Configuration { self.args.flag_rpcport.unwrap_or(self.args.flag_jsonrpc_port) ); SocketAddr::from_str(&url).unwrap_or_else(|_| die!("{}: Invalid JSONRPC listen host/port given.", url)); - let cors = self.args.flag_rpccorsdomain.as_ref().unwrap_or(&self.args.flag_jsonrpc_cors); - // TODO: use this as the API list. - let apis = self.args.flag_rpcapi.as_ref().unwrap_or(&self.args.flag_jsonrpc_apis); - let server_handler = setup_rpc_server( - service.client(), - sync.clone(), - account_service.clone(), - miner.clone(), - &url, - cors, - apis.split(',').collect() - ); - if let Some(handler) = server_handler { - panic_handler.forward_from(handler.deref()); + let cors_domain = self.args.flag_rpccorsdomain.as_ref().unwrap_or(&self.args.flag_jsonrpc_cors); + let start_result = server.start_http(&url, cors_domain, ::num_cpus::get()); + match start_result { + Ok(handle) => handle, + Err(rpc::RpcServerError::IoError(err)) => die_with_io_error(err), + Err(e) => die!("{:?}", e), } - } + }); // Register IO handler let io_handler = Arc::new(ClientIoHandler { @@ -591,11 +598,11 @@ impl Configuration { service.io().register_handler(io_handler).expect("Error registering IO handler"); // Handle exit - wait_for_exit(panic_handler); + wait_for_exit(panic_handler, rpc_handle); } } -fn wait_for_exit(panic_handler: Arc) { +fn wait_for_exit(panic_handler: Arc, _rpc_handle: Option) { let exit = Arc::new(Condvar::new()); // Handle possible exits @@ -609,6 +616,30 @@ fn wait_for_exit(panic_handler: Arc) { // Wait for signal let mutex = Mutex::new(()); let _ = exit.wait(mutex.lock().unwrap()).unwrap(); + info!("Closing...."); +} + +fn die_with_error(e: ethcore::error::Error) -> ! { + use ethcore::error::Error; + + match e { + Error::Util(UtilError::StdIo(e)) => die_with_io_error(e), + _ => die!("{:?}", e), + } +} +fn die_with_io_error(e: std::io::Error) -> ! { + match e.kind() { + std::io::ErrorKind::PermissionDenied => { + die!("We don't have permission to bind to this port.") + }, + std::io::ErrorKind::AddrInUse => { + die!("Specified address is already in use. Please make sure that nothing is listening on specified port or use different one") + }, + std::io::ErrorKind::AddrNotAvailable => { + die!("Couldn't use specified interface or given address is invalid.") + }, + _ => die!("{:?}", e), + } } fn main() { diff --git a/rpc/Cargo.toml b/rpc/Cargo.toml index 22a5c876e..cde0832d4 100644 --- a/rpc/Cargo.toml +++ b/rpc/Cargo.toml @@ -13,7 +13,7 @@ log = "0.3" serde = "0.7.0" serde_json = "0.7.0" jsonrpc-core = "2.0" -jsonrpc-http-server = "3.0" +jsonrpc-http-server = { git = "https://github.com/tomusdrw/jsonrpc-http-server.git" } ethcore-util = { path = "../util" } ethcore = { path = "../ethcore" } ethash = { path = "../ethash" } diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 3096a45c9..119d12a0f 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -34,14 +34,16 @@ extern crate transient_hashmap; use std::sync::Arc; use std::thread; -use util::panics::PanicHandler; +use util::panics::{MayPanic, PanicHandler, OnPanicListener}; use self::jsonrpc_core::{IoHandler, IoDelegate}; +pub use jsonrpc_http_server::{Listening, RpcServerError}; pub mod v1; /// Http server. pub struct RpcServer { handler: Arc, + panic_handler: Arc } impl RpcServer { @@ -49,6 +51,7 @@ impl RpcServer { pub fn new() -> RpcServer { RpcServer { handler: Arc::new(IoHandler::new()), + panic_handler: PanicHandler::new_in_arc(), } } @@ -58,17 +61,23 @@ impl RpcServer { } /// Start server asynchronously in new thread and returns panic handler. - pub fn start_http(&self, addr: &str, cors_domain: &str, threads: usize) -> Arc { + pub fn start_http(&self, addr: &str, cors_domain: &str, threads: usize) -> Result { let addr = addr.to_owned(); let cors_domain = cors_domain.to_owned(); - let panic_handler = PanicHandler::new_in_arc(); - let ph = panic_handler.clone(); + let ph = self.panic_handler.clone(); let server = jsonrpc_http_server::Server::new(self.handler.clone()); + thread::Builder::new().name("jsonrpc_http".to_string()).spawn(move || { ph.catch_panic(move || { - server.start(addr.as_ref(), jsonrpc_http_server::AccessControlAllowOrigin::Value(cors_domain), threads); + server.start(addr.as_ref(), jsonrpc_http_server::AccessControlAllowOrigin::Value(cors_domain), threads) }).unwrap() - }).expect("Error while creating jsonrpc http thread"); - panic_handler + }).expect("Error while creating jsonrpc http thread").join().unwrap() } } + +impl MayPanic for RpcServer { + fn on_panic(&self, closure: F) where F: OnPanicListener { + self.panic_handler.on_panic(closure); + } +} + From e6237ed05e6a09a06a713ffdebee278d6c8b80a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 6 Apr 2016 19:14:05 +0200 Subject: [PATCH 16/27] Removing additional thread from JSON-RPC --- parity/main.rs | 4 +--- rpc/src/lib.rs | 18 +----------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index ebb347cf2..a36157618 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -566,10 +566,8 @@ impl Configuration { } else { None }; + let rpc_handle = rpc_server.map(|server| { - panic_handler.forward_from(&server); - server - }).map(|server| { let url = format!("{}:{}", match self.args.flag_rpcaddr.as_ref().unwrap_or(&self.args.flag_jsonrpc_interface).as_str() { "all" => "0.0.0.0", diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 119d12a0f..9787db9cd 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -33,8 +33,6 @@ extern crate ethminer; extern crate transient_hashmap; use std::sync::Arc; -use std::thread; -use util::panics::{MayPanic, PanicHandler, OnPanicListener}; use self::jsonrpc_core::{IoHandler, IoDelegate}; pub use jsonrpc_http_server::{Listening, RpcServerError}; @@ -43,7 +41,6 @@ pub mod v1; /// Http server. pub struct RpcServer { handler: Arc, - panic_handler: Arc } impl RpcServer { @@ -51,7 +48,6 @@ impl RpcServer { pub fn new() -> RpcServer { RpcServer { handler: Arc::new(IoHandler::new()), - panic_handler: PanicHandler::new_in_arc(), } } @@ -64,20 +60,8 @@ impl RpcServer { pub fn start_http(&self, addr: &str, cors_domain: &str, threads: usize) -> Result { let addr = addr.to_owned(); let cors_domain = cors_domain.to_owned(); - let ph = self.panic_handler.clone(); let server = jsonrpc_http_server::Server::new(self.handler.clone()); - thread::Builder::new().name("jsonrpc_http".to_string()).spawn(move || { - ph.catch_panic(move || { - server.start(addr.as_ref(), jsonrpc_http_server::AccessControlAllowOrigin::Value(cors_domain), threads) - }).unwrap() - }).expect("Error while creating jsonrpc http thread").join().unwrap() + server.start(addr.as_ref(), jsonrpc_http_server::AccessControlAllowOrigin::Value(cors_domain), threads) } } - -impl MayPanic for RpcServer { - fn on_panic(&self, closure: F) where F: OnPanicListener { - self.panic_handler.on_panic(closure); - } -} - From 8367be6992e02c828b6b046ad8114e7f0cd8c92d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 6 Apr 2016 19:22:10 +0200 Subject: [PATCH 17/27] Rewriting messages --- parity/main.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index a36157618..0eb32b5bb 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -628,13 +628,13 @@ fn die_with_error(e: ethcore::error::Error) -> ! { fn die_with_io_error(e: std::io::Error) -> ! { match e.kind() { std::io::ErrorKind::PermissionDenied => { - die!("We don't have permission to bind to this port.") + die!("No permissions to bind to specified port.") }, std::io::ErrorKind::AddrInUse => { - die!("Specified address is already in use. Please make sure that nothing is listening on specified port or use different one") + die!("Specified address is already in use. Please make sure that nothing is listening on the same port or try using a different one.") }, std::io::ErrorKind::AddrNotAvailable => { - die!("Couldn't use specified interface or given address is invalid.") + die!("Could not use specified interface or given address is invalid.") }, _ => die!("{:?}", e), } From 3303aa73ff1e7b88307e361cdd7b67fe72e287b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 6 Apr 2016 23:45:19 +0200 Subject: [PATCH 18/27] Tracing shutdown and changed order of IoManager shutdown process --- ethcore/src/block_queue.rs | 2 ++ util/src/io/service.rs | 4 +++- util/src/io/worker.rs | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index 042df1dc1..e827bd9c6 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -431,12 +431,14 @@ impl MayPanic for BlockQueue { impl Drop for BlockQueue { fn drop(&mut self) { + trace!(target: "shutdown", "[BlockQueue] Closing..."); self.clear(); self.deleting.store(true, AtomicOrdering::Release); self.more_to_verify.notify_all(); for t in self.verifiers.drain(..) { t.join().unwrap(); } + trace!(target: "shutdown", "[BlockQueue] Closed."); } } diff --git a/util/src/io/service.rs b/util/src/io/service.rs index 8a34ee80a..24cc1181a 100644 --- a/util/src/io/service.rs +++ b/util/src/io/service.rs @@ -231,8 +231,8 @@ impl Handler for IoManager where Message: Send + Clone + Sync fn notify(&mut self, event_loop: &mut EventLoop, msg: Self::Message) { match msg { IoMessage::Shutdown => { - self.workers.clear(); event_loop.shutdown(); + self.workers.clear(); }, IoMessage::AddHandler { handler } => { let handler_id = { @@ -376,8 +376,10 @@ impl IoService where Message: Send + Sync + Clone + 'static { impl Drop for IoService where Message: Send + Sync + Clone { fn drop(&mut self) { + trace!(target: "shutdown", "[IoService] Closing..."); self.host_channel.send(IoMessage::Shutdown).unwrap(); self.thread.take().unwrap().join().ok(); + trace!(target: "shutdown", "[IoService] Closed."); } } diff --git a/util/src/io/worker.rs b/util/src/io/worker.rs index b874ea0a4..917b1ad79 100644 --- a/util/src/io/worker.rs +++ b/util/src/io/worker.rs @@ -120,10 +120,12 @@ impl Worker { impl Drop for Worker { fn drop(&mut self) { + trace!(target: "shutdown", "[IoWorker] Closing..."); let _ = self.wait_mutex.lock(); self.deleting.store(true, AtomicOrdering::Release); self.wait.notify_all(); let thread = mem::replace(&mut self.thread, None).unwrap(); thread.join().ok(); + trace!(target: "shutdown", "[IoWorker] Closed"); } } From 03fbde9723a020379521a405737f2153efa52e1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 6 Apr 2016 23:58:23 +0200 Subject: [PATCH 19/27] More descriptive message when closing --- parity/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parity/main.rs b/parity/main.rs index 0eb32b5bb..f52a23c45 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -614,7 +614,7 @@ fn wait_for_exit(panic_handler: Arc, _rpc_handle: Option ! { From 771e3998a9c4076299099c897f467ddd19df64f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 7 Apr 2016 10:31:42 +0200 Subject: [PATCH 20/27] Updating rpc comments --- rpc/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 9787db9cd..4de405211 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -44,7 +44,7 @@ pub struct RpcServer { } impl RpcServer { - /// Construct new http server object with given number of threads. + /// Construct new http server object. pub fn new() -> RpcServer { RpcServer { handler: Arc::new(IoHandler::new()), @@ -56,7 +56,7 @@ impl RpcServer { self.handler.add_delegate(delegate); } - /// Start server asynchronously in new thread and returns panic handler. + /// Start server asynchronously and returns result with `Listening` handle on success or an error. pub fn start_http(&self, addr: &str, cors_domain: &str, threads: usize) -> Result { let addr = addr.to_owned(); let cors_domain = cors_domain.to_owned(); From 3f139116b207cd837266966324d4f6eb5687576d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 7 Apr 2016 12:27:54 +0200 Subject: [PATCH 21/27] Reverting order of shutdown event --- util/src/io/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/src/io/service.rs b/util/src/io/service.rs index 24cc1181a..95aa19e47 100644 --- a/util/src/io/service.rs +++ b/util/src/io/service.rs @@ -231,8 +231,8 @@ impl Handler for IoManager where Message: Send + Clone + Sync fn notify(&mut self, event_loop: &mut EventLoop, msg: Self::Message) { match msg { IoMessage::Shutdown => { - event_loop.shutdown(); self.workers.clear(); + event_loop.shutdown(); }, IoMessage::AddHandler { handler } => { let handler_id = { From cc0c54a45cf4b16fc27bc4816ab2e2879cb360d5 Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 6 Apr 2016 23:03:07 +0200 Subject: [PATCH 22/27] Propagate transaction queue --- miner/src/lib.rs | 3 +++ miner/src/miner.rs | 5 ++++ rpc/src/v1/impls/eth.rs | 8 +++---- rpc/src/v1/tests/helpers/miner_service.rs | 4 ++++ rpc/src/v1/tests/helpers/sync_provider.rs | 5 +--- sync/src/chain.rs | 28 +++++++++-------------- sync/src/lib.rs | 10 +------- 7 files changed, 28 insertions(+), 35 deletions(-) diff --git a/miner/src/lib.rs b/miner/src/lib.rs index e45411f9a..5aba775b1 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -109,6 +109,9 @@ pub trait MinerService : Send + Sync { /// Query pending transactions for hash fn transaction(&self, hash: &H256) -> Option; + /// Get a list of all pending transactions. + fn pending_transactions(&self) -> Vec; + /// Returns highest transaction nonce for given address. fn last_nonce(&self, address: &Address) -> Option; diff --git a/miner/src/miner.rs b/miner/src/miner.rs index dff5ab48d..9ea4ecfa4 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -175,6 +175,11 @@ impl MinerService for Miner { queue.find(hash) } + fn pending_transactions(&self) -> Vec { + let queue = self.transaction_queue.lock().unwrap(); + queue.top_transactions() + } + fn last_nonce(&self, address: &Address) -> Option { self.transaction_queue.lock().unwrap().last_nonce(address) } diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index c89677689..b6f2002e2 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -180,7 +180,7 @@ impl EthClient }.fake_sign(from)) } - fn dispatch_transaction(&self, signed_transaction: SignedTransaction, raw_transaction: Vec) -> Result { + fn dispatch_transaction(&self, signed_transaction: SignedTransaction) -> Result { let hash = signed_transaction.hash(); let import = { @@ -197,7 +197,6 @@ impl EthClient match import.into_iter().collect::, _>>() { Ok(_) => { - take_weak!(self.sync).new_transaction(raw_transaction); to_value(&hash) } Err(e) => { @@ -503,8 +502,7 @@ impl Eth for EthClient data: request.data.map_or_else(Vec::new, |d| d.to_vec()), }.sign(&secret) }; - let raw_transaction = encode(&signed_transaction).to_vec(); - self.dispatch_transaction(signed_transaction, raw_transaction) + self.dispatch_transaction(signed_transaction) }, Err(_) => { to_value(&H256::zero()) } } @@ -516,7 +514,7 @@ impl Eth for EthClient .and_then(|(raw_transaction, )| { let raw_transaction = raw_transaction.to_vec(); match UntrustedRlp::new(&raw_transaction).as_val() { - Ok(signed_transaction) => self.dispatch_transaction(signed_transaction, raw_transaction), + Ok(signed_transaction) => self.dispatch_transaction(signed_transaction), Err(_) => to_value(&H256::zero()), } }) diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index fe7858a65..26a2339fb 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -99,6 +99,10 @@ impl MinerService for TestMinerService { self.pending_transactions.lock().unwrap().get(hash).cloned() } + fn pending_transactions(&self) -> Vec { + self.pending_transactions.lock().unwrap().values().cloned().collect() + } + fn last_nonce(&self, address: &Address) -> Option { self.last_nonces.read().unwrap().get(address).cloned() } diff --git a/rpc/src/v1/tests/helpers/sync_provider.rs b/rpc/src/v1/tests/helpers/sync_provider.rs index 59188f0a7..633e0d45b 100644 --- a/rpc/src/v1/tests/helpers/sync_provider.rs +++ b/rpc/src/v1/tests/helpers/sync_provider.rs @@ -16,7 +16,7 @@ //! Test implementation of SyncProvider. -use util::{U256, Bytes}; +use util::{U256}; use ethsync::{SyncProvider, SyncStatus, SyncState}; use std::sync::{RwLock}; @@ -59,8 +59,5 @@ impl SyncProvider for TestSyncProvider { fn status(&self) -> SyncStatus { self.status.read().unwrap().clone() } - - fn new_transaction(&self, _raw_transaction: Bytes) { - } } diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 6229627c4..662387abb 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -217,10 +217,6 @@ pub struct ChainSync { network_id: U256, /// Miner miner: Arc, - - /// Transactions to propagate - // TODO: reconsider where this is in the codebase - seems a little dodgy to have here. - transactions_to_send: Vec, } type RlpResponseResult = Result, PacketDecodeError>; @@ -247,7 +243,6 @@ impl ChainSync { max_download_ahead_blocks: max(MAX_HEADERS_TO_REQUEST, config.max_download_ahead_blocks), network_id: config.network_id, miner: miner, - transactions_to_send: vec![], } } @@ -959,11 +954,6 @@ impl ChainSync { } } - /// Place a new transaction on the wire. - pub fn new_transaction(&mut self, raw_transaction: Bytes) { - self.transactions_to_send.push(raw_transaction); - } - /// Called when peer sends us new transactions fn on_peer_transactions(&mut self, io: &mut SyncIo, peer_id: PeerId, r: &UntrustedRlp) -> Result<(), PacketDecodeError> { // accepting transactions once only fully synced @@ -1305,11 +1295,16 @@ impl ChainSync { return 0; } - let mut packet = RlpStream::new_list(self.transactions_to_send.len()); - for tx in self.transactions_to_send.iter() { - packet.append_raw(tx, 1); + let mut transactions = self.miner.pending_transactions(); + if transactions.is_empty() { + return 0; + } + + let mut packet = RlpStream::new_list(transactions.len()); + let tx_count = transactions.len(); + for tx in transactions.drain(..) { + packet.append(&tx); } - self.transactions_to_send.clear(); let rlp = packet.out(); let lucky_peers = { @@ -1328,13 +1323,12 @@ impl ChainSync { for peer_id in lucky_peers { self.send_packet(io, peer_id, TRANSACTIONS_PACKET, rlp.clone()); } + trace!(target: "sync", "Sent {} transactions to {} peers.", tx_count, sent); sent } fn propagate_latest_blocks(&mut self, io: &mut SyncIo) { - if !self.transactions_to_send.is_empty() { - self.propagate_new_transactions(io); - } + self.propagate_new_transactions(io); let chain_info = io.chain().chain_info(); if (((chain_info.best_block_number as i64) - (self.last_sent_block_number as i64)).abs() as BlockNumber) < MAX_PEER_LAG_PROPAGATION { let blocks = self.propagate_blocks(&chain_info, io); diff --git a/sync/src/lib.rs b/sync/src/lib.rs index ea4a1daea..a4f6eff38 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -66,7 +66,7 @@ use std::ops::*; use std::sync::*; use util::network::{NetworkProtocolHandler, NetworkService, NetworkContext, PeerId}; use util::TimerToken; -use util::{U256, Bytes, ONE_U256}; +use util::{U256, ONE_U256}; use ethcore::client::Client; use ethcore::service::SyncMessage; use ethminer::Miner; @@ -101,9 +101,6 @@ impl Default for SyncConfig { pub trait SyncProvider: Send + Sync { /// Get sync status fn status(&self) -> SyncStatus; - - /// Note that a user has submitted a new transaction. - fn new_transaction(&self, raw_transaction: Bytes); } /// Ethereum network protocol handler @@ -143,11 +140,6 @@ impl SyncProvider for EthSync { fn status(&self) -> SyncStatus { self.sync.read().unwrap().status() } - - /// Note that a user has submitted a new transaction. - fn new_transaction(&self, raw_transaction: Bytes) { - self.sync.write().unwrap().new_transaction(raw_transaction); - } } impl NetworkProtocolHandler for EthSync { From ece348d173e3561a8c61cda850ea252f2f709b0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 7 Apr 2016 00:20:03 +0200 Subject: [PATCH 23/27] Avoid signalling readiness when app is about to be closed --- ethcore/src/block_queue.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index e827bd9c6..e9dc6dd05 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -116,6 +116,7 @@ struct VerifyingBlock { } struct QueueSignal { + deleting: Arc, signalled: AtomicBool, message_channel: IoChannel, } @@ -123,10 +124,16 @@ struct QueueSignal { impl QueueSignal { #[cfg_attr(feature="dev", allow(bool_comparison))] fn set(&self) { + // Do not signal when we are about to close + if self.deleting.load(AtomicOrdering::Relaxed) { + return; + } + if self.signalled.compare_and_swap(false, true, AtomicOrdering::Relaxed) == false { self.message_channel.send(UserMessage(SyncMessage::BlockVerified)).expect("Error sending BlockVerified message"); } } + fn reset(&self) { self.signalled.store(false, AtomicOrdering::Relaxed); } @@ -150,8 +157,12 @@ impl BlockQueue { bad: Mutex::new(HashSet::new()), }); let more_to_verify = Arc::new(Condvar::new()); - let ready_signal = Arc::new(QueueSignal { signalled: AtomicBool::new(false), message_channel: message_channel }); let deleting = Arc::new(AtomicBool::new(false)); + let ready_signal = Arc::new(QueueSignal { + deleting: deleting.clone(), + signalled: AtomicBool::new(false), + message_channel: message_channel + }); let empty = Arc::new(Condvar::new()); let panic_handler = PanicHandler::new_in_arc(); From eae66568a91442429e830432158184c6e2662e35 Mon Sep 17 00:00:00 2001 From: debris Date: Thu, 7 Apr 2016 00:33:55 +0200 Subject: [PATCH 24/27] fixed #895 --- rpc/src/v1/traits/eth.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rpc/src/v1/traits/eth.rs b/rpc/src/v1/traits/eth.rs index 8a48e0dfe..5d36da670 100644 --- a/rpc/src/v1/traits/eth.rs +++ b/rpc/src/v1/traits/eth.rs @@ -190,9 +190,6 @@ pub trait EthFilter: Sized + Send + Sync + 'static { /// Returns filter changes since last poll. fn filter_changes(&self, _: Params) -> Result { rpc_unimplemented!() } - /// Returns filter logs. - fn filter_logs(&self, _: Params) -> Result { rpc_unimplemented!() } - /// Uninstalls filter. fn uninstall_filter(&self, _: Params) -> Result { rpc_unimplemented!() } @@ -203,7 +200,7 @@ pub trait EthFilter: Sized + Send + Sync + 'static { delegate.add_method("eth_newBlockFilter", EthFilter::new_block_filter); delegate.add_method("eth_newPendingTransactionFilter", EthFilter::new_pending_transaction_filter); delegate.add_method("eth_getFilterChanges", EthFilter::filter_changes); - delegate.add_method("eth_getFilterLogs", EthFilter::filter_logs); + delegate.add_method("eth_getFilterLogs", EthFilter::filter_changes); delegate.add_method("eth_uninstallFilter", EthFilter::uninstall_filter); delegate } From 9df9937922909770a97de4a9a324586a52366702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 7 Apr 2016 12:49:20 +0200 Subject: [PATCH 25/27] Fixing compilation with rpc feature disabled --- parity/main.rs | 55 ++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index f52a23c45..80c3feac6 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -39,6 +39,7 @@ extern crate rpassword; #[cfg(feature = "rpc")] extern crate ethcore_rpc as rpc; +use std::any::Any; use std::io::{BufRead, BufReader}; use std::fs::File; use std::net::{SocketAddr, IpAddr}; @@ -258,8 +259,10 @@ fn setup_rpc_server( sync: Arc, secret_store: Arc, miner: Arc, + url: &str, + cors_domain: &str, apis: Vec<&str> -) -> Option { +) -> Option> { use rpc::v1::*; let server = rpc::RpcServer::new(); @@ -277,7 +280,12 @@ fn setup_rpc_server( } } } - Some(server) + let start_result = server.start_http(url, cors_domain, ::num_cpus::get()); + match start_result { + Err(rpc::RpcServerError::IoError(err)) => die_with_io_error(err), + Err(e) => die!("{:?}", e), + Ok(handle) => Some(Box::new(handle)), + } } #[cfg(not(feature = "rpc"))] @@ -286,9 +294,11 @@ fn setup_rpc_server( _sync: Arc, _secret_store: Arc, _miner: Arc, + _url: &str, + _cors_domain: &str, _apis: Vec<&str> -) -> Option> { - None +) -> ! { + die!("Your Parity version has been compiled without JSON-RPC support.") } fn print_version() { @@ -554,20 +564,7 @@ impl Configuration { // Setup rpc let rpc_server = if self.args.flag_jsonrpc || self.args.flag_rpc { - // TODO: use this as the API list. let apis = self.args.flag_rpcapi.as_ref().unwrap_or(&self.args.flag_jsonrpc_apis); - setup_rpc_server( - service.client(), - sync.clone(), - account_service.clone(), - miner.clone(), - apis.split(',').collect() - ) - } else { - None - }; - - let rpc_handle = rpc_server.map(|server| { let url = format!("{}:{}", match self.args.flag_rpcaddr.as_ref().unwrap_or(&self.args.flag_jsonrpc_interface).as_str() { "all" => "0.0.0.0", @@ -578,13 +575,19 @@ impl Configuration { ); SocketAddr::from_str(&url).unwrap_or_else(|_| die!("{}: Invalid JSONRPC listen host/port given.", url)); let cors_domain = self.args.flag_rpccorsdomain.as_ref().unwrap_or(&self.args.flag_jsonrpc_cors); - let start_result = server.start_http(&url, cors_domain, ::num_cpus::get()); - match start_result { - Ok(handle) => handle, - Err(rpc::RpcServerError::IoError(err)) => die_with_io_error(err), - Err(e) => die!("{:?}", e), - } - }); + + setup_rpc_server( + service.client(), + sync.clone(), + account_service.clone(), + miner.clone(), + &url, + &cors_domain, + apis.split(',').collect() + ) + } else { + None + }; // Register IO handler let io_handler = Arc::new(ClientIoHandler { @@ -596,11 +599,11 @@ impl Configuration { service.io().register_handler(io_handler).expect("Error registering IO handler"); // Handle exit - wait_for_exit(panic_handler, rpc_handle); + wait_for_exit(panic_handler, rpc_server); } } -fn wait_for_exit(panic_handler: Arc, _rpc_handle: Option) { +fn wait_for_exit(panic_handler: Arc, _rpc_server: Option>) { let exit = Arc::new(Condvar::new()); // Handle possible exits From 32b0a80a18dddbcc37efcb2e48270b073cb71e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 7 Apr 2016 12:55:06 +0200 Subject: [PATCH 26/27] Removing Option from setup_rpc_server method return type --- parity/main.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index 80c3feac6..f951a283b 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -262,7 +262,7 @@ fn setup_rpc_server( url: &str, cors_domain: &str, apis: Vec<&str> -) -> Option> { +) -> Box { use rpc::v1::*; let server = rpc::RpcServer::new(); @@ -284,7 +284,7 @@ fn setup_rpc_server( match start_result { Err(rpc::RpcServerError::IoError(err)) => die_with_io_error(err), Err(e) => die!("{:?}", e), - Ok(handle) => Some(Box::new(handle)), + Ok(handle) => Box::new(handle), } } @@ -576,7 +576,7 @@ impl Configuration { SocketAddr::from_str(&url).unwrap_or_else(|_| die!("{}: Invalid JSONRPC listen host/port given.", url)); let cors_domain = self.args.flag_rpccorsdomain.as_ref().unwrap_or(&self.args.flag_jsonrpc_cors); - setup_rpc_server( + Some(setup_rpc_server( service.client(), sync.clone(), account_service.clone(), @@ -584,7 +584,7 @@ impl Configuration { &url, &cors_domain, apis.split(',').collect() - ) + )) } else { None }; From 37eb0fb04e62ea6e14ab69b96d4ccfc9efbb5296 Mon Sep 17 00:00:00 2001 From: arkpar Date: Thu, 7 Apr 2016 14:24:52 +0200 Subject: [PATCH 27/27] Use new json RPC server --- Cargo.lock | 58 +++++++++++++++++++++++++++++++++++--------------- parity/main.rs | 20 ++++++++++------- rpc/Cargo.toml | 2 +- rpc/src/lib.rs | 12 +++++------ 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b45f6dbc9..61c548fef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -244,7 +244,7 @@ dependencies = [ "ethminer 1.0.0", "ethsync 1.0.2", "jsonrpc-core 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", - "jsonrpc-http-server 4.0.0 (git+https://github.com/tomusdrw/jsonrpc-http-server.git)", + "jsonrpc-http-server 5.0.0 (git+https://github.com/debris/jsonrpc-http-server.git)", "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -395,22 +395,23 @@ dependencies = [ [[package]] name = "hyper" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" +version = "0.9.0-mio" +source = "git+https://github.com/hyperium/hyper?branch=mio#bf42e7563b7d52f334a1b25ec3dccf031febb990" dependencies = [ "cookie 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "httparse 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "language-tags 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", - "mime 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", - "num_cpus 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", + "mime 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", + "mio 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rotor 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)", - "solicit 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)", "time 0.1.34 (registry+https://github.com/rust-lang/crates.io-index)", "traitobject 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "typeable 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "url 0.5.7 (registry+https://github.com/rust-lang/crates.io-index)", + "vecio 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -451,10 +452,10 @@ dependencies = [ [[package]] name = "jsonrpc-http-server" -version = "4.0.0" -source = "git+https://github.com/tomusdrw/jsonrpc-http-server.git#46bd4e7cf8352e0efc940cf76d3dff99f1a3da15" +version = "5.0.0" +source = "git+https://github.com/debris/jsonrpc-http-server.git#76fa443982b40665721fe6b1ece42fc0a53be996" dependencies = [ - "hyper 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", + "hyper 0.9.0-mio (git+https://github.com/hyperium/hyper?branch=mio)", "jsonrpc-core 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -531,14 +532,6 @@ dependencies = [ "serde 0.6.15 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "mime" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "mio" version = "0.5.0" @@ -689,6 +682,11 @@ dependencies = [ "syntex_syntax 0.30.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "quick-error" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "rand" version = "0.3.14" @@ -732,6 +730,18 @@ dependencies = [ "librocksdb-sys 0.2.3 (git+https://github.com/arkpar/rust-rocksdb.git)", ] +[[package]] +name = "rotor" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "log 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", + "mio 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "quick-error 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "time 0.1.34 (registry+https://github.com/rust-lang/crates.io-index)", + "void 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "rpassword" version = "0.1.3" @@ -987,6 +997,15 @@ dependencies = [ "rustc-serialize 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "vecio" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "winapi 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "ws2_32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "vergen" version = "0.1.0" @@ -996,6 +1015,11 @@ dependencies = [ "time 0.1.34 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "void" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "winapi" version = "0.2.6" diff --git a/parity/main.rs b/parity/main.rs index f951a283b..c62335b4c 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -39,7 +39,6 @@ extern crate rpassword; #[cfg(feature = "rpc")] extern crate ethcore_rpc as rpc; -use std::any::Any; use std::io::{BufRead, BufReader}; use std::fs::File; use std::net::{SocketAddr, IpAddr}; @@ -60,6 +59,8 @@ use ethminer::{Miner, MinerService}; use docopt::Docopt; use daemonize::Daemonize; use number_prefix::{binary_prefix, Standalone, Prefixed}; +#[cfg(feature = "rpc")] +use rpc::Server as RpcServer; fn die_with_message(msg: &str) -> ! { println!("ERROR: {}", msg); @@ -259,10 +260,10 @@ fn setup_rpc_server( sync: Arc, secret_store: Arc, miner: Arc, - url: &str, + url: &SocketAddr, cors_domain: &str, apis: Vec<&str> -) -> Box { +) -> RpcServer { use rpc::v1::*; let server = rpc::RpcServer::new(); @@ -280,14 +281,17 @@ fn setup_rpc_server( } } } - let start_result = server.start_http(url, cors_domain, ::num_cpus::get()); + let start_result = server.start_http(url, cors_domain); match start_result { Err(rpc::RpcServerError::IoError(err)) => die_with_io_error(err), Err(e) => die!("{:?}", e), - Ok(handle) => Box::new(handle), + Ok(server) => server, } } +#[cfg(not(feature = "rpc"))] +struct RpcServer; + #[cfg(not(feature = "rpc"))] fn setup_rpc_server( _client: Arc, @@ -573,7 +577,7 @@ impl Configuration { }, self.args.flag_rpcport.unwrap_or(self.args.flag_jsonrpc_port) ); - SocketAddr::from_str(&url).unwrap_or_else(|_| die!("{}: Invalid JSONRPC listen host/port given.", url)); + let addr = SocketAddr::from_str(&url).unwrap_or_else(|_| die!("{}: Invalid JSONRPC listen host/port given.", url)); let cors_domain = self.args.flag_rpccorsdomain.as_ref().unwrap_or(&self.args.flag_jsonrpc_cors); Some(setup_rpc_server( @@ -581,7 +585,7 @@ impl Configuration { sync.clone(), account_service.clone(), miner.clone(), - &url, + &addr, &cors_domain, apis.split(',').collect() )) @@ -603,7 +607,7 @@ impl Configuration { } } -fn wait_for_exit(panic_handler: Arc, _rpc_server: Option>) { +fn wait_for_exit(panic_handler: Arc, _rpc_server: Option) { let exit = Arc::new(Condvar::new()); // Handle possible exits diff --git a/rpc/Cargo.toml b/rpc/Cargo.toml index cde0832d4..99002fed8 100644 --- a/rpc/Cargo.toml +++ b/rpc/Cargo.toml @@ -13,7 +13,7 @@ log = "0.3" serde = "0.7.0" serde_json = "0.7.0" jsonrpc-core = "2.0" -jsonrpc-http-server = { git = "https://github.com/tomusdrw/jsonrpc-http-server.git" } +jsonrpc-http-server = { git = "https://github.com/debris/jsonrpc-http-server.git" } ethcore-util = { path = "../util" } ethcore = { path = "../ethcore" } ethash = { path = "../ethash" } diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 4de405211..f059750d2 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -33,9 +33,10 @@ extern crate ethminer; extern crate transient_hashmap; use std::sync::Arc; +use std::net::SocketAddr; use self::jsonrpc_core::{IoHandler, IoDelegate}; -pub use jsonrpc_http_server::{Listening, RpcServerError}; +pub use jsonrpc_http_server::{Server, RpcServerError}; pub mod v1; /// Http server. @@ -56,12 +57,9 @@ impl RpcServer { self.handler.add_delegate(delegate); } - /// Start server asynchronously and returns result with `Listening` handle on success or an error. - pub fn start_http(&self, addr: &str, cors_domain: &str, threads: usize) -> Result { - let addr = addr.to_owned(); + /// Start server asynchronously and returns result with `Server` handle on success or an error. + pub fn start_http(&self, addr: &SocketAddr, cors_domain: &str) -> Result { let cors_domain = cors_domain.to_owned(); - let server = jsonrpc_http_server::Server::new(self.handler.clone()); - - server.start(addr.as_ref(), jsonrpc_http_server::AccessControlAllowOrigin::Value(cors_domain), threads) + Server::start(addr, self.handler.clone(), jsonrpc_http_server::AccessControlAllowOrigin::Value(cors_domain)) } }