From a66e36bf41f9b251cb9e7c70b8fb46754beb77f3 Mon Sep 17 00:00:00 2001 From: Afri Schoedon <5chdn@users.noreply.github.com> Date: Tue, 5 Jun 2018 21:51:37 +0200 Subject: [PATCH] parity-version: bump beta to 1.11.3 (#8806) * parity-version: bump beta to 1.11.3 * Disallow unsigned transactions in case EIP-86 is disabled (#8802) * Disallow unsigned transactions in case EIP-86 is disabled * Add tests for verification * Add disallow unsigned transactions test in machine * Fix ancient blocks queue deadlock (#8751) * Revert "Fix not downloading old blocks (#8642)" This reverts commit d1934363e7c2c953f17cd451bea8476f46f52b82. * Make sure only one thread actually imports old blocks. * Add some trace timers. * Bring back pending hashes set. * Separate locks so that queue can happen while we are importing. * Address grumbles. --- Cargo.lock | 16 ++-- Cargo.toml | 2 +- ethcore/src/client/client.rs | 113 ++++++++++++++--------- ethcore/src/ethereum/mod.rs | 3 + ethcore/src/machine.rs | 19 ++++ ethcore/src/verification/verification.rs | 19 ++++ ethcore/transaction/src/transaction.rs | 4 + mac/Parity.pkgproj | 2 +- miner/src/pool/queue.rs | 2 +- nsis/installer.nsi | 2 +- util/version/Cargo.toml | 4 +- 11 files changed, 128 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5d82a301..155901cd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1866,7 +1866,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "num-integer 0.1.36 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", - "rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.3.20 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1956,7 +1956,7 @@ dependencies = [ [[package]] name = "parity" -version = "1.11.2" +version = "1.11.3" dependencies = [ "ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2007,7 +2007,7 @@ dependencies = [ "parity-rpc 1.11.0", "parity-rpc-client 1.4.0", "parity-updater 1.11.0", - "parity-version 1.11.2", + "parity-version 1.11.3", "parity-whisper 0.1.0", "parking_lot 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "path 0.1.0", @@ -2055,7 +2055,7 @@ dependencies = [ "parity-reactor 0.1.0", "parity-ui 1.11.0", "parity-ui-deprecation 1.10.0", - "parity-version 1.11.2", + "parity-version 1.11.3", "parking_lot 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "registrar 0.0.1", @@ -2211,7 +2211,7 @@ dependencies = [ "order-stat 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "parity-reactor 0.1.0", "parity-updater 1.11.0", - "parity-version 1.11.2", + "parity-version 1.11.3", "parking_lot 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "patricia-trie 0.1.0", "pretty_assertions 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2332,7 +2332,7 @@ dependencies = [ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-hash-fetch 1.11.0", - "parity-version 1.11.2", + "parity-version 1.11.3", "parking_lot 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "path 0.1.0", "rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2343,7 +2343,7 @@ dependencies = [ [[package]] name = "parity-version" -version = "1.11.2" +version = "1.11.3" dependencies = [ "ethcore-bytes 0.1.0", "rlp 0.2.1", @@ -2686,7 +2686,7 @@ dependencies = [ "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", - "rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.3.20 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 98ed070d8..4bf628ea3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ description = "Parity Ethereum client" name = "parity" # NOTE Make sure to update util/version/Cargo.toml as well -version = "1.11.2" +version = "1.11.3" license = "GPL-3.0" authors = ["Parity Technologies "] diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 674775eb8..19121cbd2 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -18,7 +18,7 @@ use std::collections::{HashSet, HashMap, BTreeMap, BTreeSet, VecDeque}; use std::fmt; use std::str::FromStr; use std::sync::{Arc, Weak}; -use std::sync::atomic::{AtomicBool, Ordering as AtomicOrdering}; +use std::sync::atomic::{AtomicUsize, AtomicBool, Ordering as AtomicOrdering}; use std::time::{Instant}; // util @@ -88,6 +88,8 @@ use_contract!(registry, "Registry", "res/contracts/registrar.json"); const MAX_TX_QUEUE_SIZE: usize = 4096; const MAX_ANCIENT_BLOCKS_QUEUE_SIZE: usize = 4096; +// Max number of blocks imported at once. +const MAX_ANCIENT_BLOCKS_TO_IMPORT: usize = 4; const MAX_QUEUE_SIZE_TO_SLEEP_ON: usize = 2; const MIN_HISTORY_SIZE: u64 = 8; @@ -208,8 +210,12 @@ pub struct Client { queue_transactions: IoChannelQueue, /// Ancient blocks import queue queue_ancient_blocks: IoChannelQueue, - /// Hashes of pending ancient block wainting to be included - pending_ancient_blocks: RwLock>, + /// Queued ancient blocks, make sure they are imported in order. + queued_ancient_blocks: Arc, + VecDeque<(Header, Bytes, Bytes)> + )>>, + ancient_blocks_import_lock: Arc>, /// Consensus messages import queue queue_consensus_message: IoChannelQueue, @@ -463,7 +469,6 @@ impl Importer { let hash = header.hash(); let _import_lock = self.import_lock.lock(); - trace!(target: "client", "Trying to import old block #{}", header.number()); { trace_time!("import_old_block"); // verify the block, passing the chain for updating the epoch verifier. @@ -744,7 +749,8 @@ impl Client { notify: RwLock::new(Vec::new()), queue_transactions: IoChannelQueue::new(MAX_TX_QUEUE_SIZE), queue_ancient_blocks: IoChannelQueue::new(MAX_ANCIENT_BLOCKS_QUEUE_SIZE), - pending_ancient_blocks: RwLock::new(HashSet::new()), + queued_ancient_blocks: Default::default(), + ancient_blocks_import_lock: Default::default(), queue_consensus_message: IoChannelQueue::new(usize::max_value()), last_hashes: RwLock::new(VecDeque::new()), factories: factories, @@ -1975,8 +1981,9 @@ impl BlockChainClient for Client { impl IoClient for Client { fn queue_transactions(&self, transactions: Vec, peer_id: usize) { + trace_time!("queue_transactions"); let len = transactions.len(); - self.queue_transactions.queue(&mut self.io_channel.lock(), move |client| { + self.queue_transactions.queue(&mut self.io_channel.lock(), len, move |client| { trace_time!("import_queued_transactions"); let txs: Vec = transactions @@ -1995,6 +2002,7 @@ impl IoClient for Client { } fn queue_ancient_block(&self, block_bytes: Bytes, receipts_bytes: Bytes) -> Result { + trace_time!("queue_ancient_block"); let header: Header = ::rlp::Rlp::new(&block_bytes).val_at(0)?; let hash = header.hash(); @@ -2003,31 +2011,51 @@ impl IoClient for Client { if self.chain.read().is_known(&hash) { bail!(BlockImportErrorKind::Import(ImportErrorKind::AlreadyInChain)); } - - let parent_hash = *header.parent_hash(); - let parent_pending = self.pending_ancient_blocks.read().contains(&parent_hash); - let status = self.block_status(BlockId::Hash(parent_hash)); - if !parent_pending && (status == BlockStatus::Unknown || status == BlockStatus::Pending) { - bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(parent_hash))); + let parent_hash = header.parent_hash(); + // NOTE To prevent race condition with import, make sure to check queued blocks first + // (and attempt to acquire lock) + let is_parent_pending = self.queued_ancient_blocks.read().0.contains(parent_hash); + if !is_parent_pending { + let status = self.block_status(BlockId::Hash(*parent_hash)); + if status == BlockStatus::Unknown || status == BlockStatus::Pending { + bail!(BlockImportErrorKind::Block(BlockError::UnknownParent(*parent_hash))); + } } } - self.pending_ancient_blocks.write().insert(hash); + // we queue blocks here and trigger an IO message. + { + let mut queued = self.queued_ancient_blocks.write(); + queued.0.insert(hash); + queued.1.push_back((header, block_bytes, receipts_bytes)); + } - trace!(target: "client", "Queuing old block #{}", header.number()); - match self.queue_ancient_blocks.queue(&mut self.io_channel.lock(), move |client| { - let result = client.importer.import_old_block( - &header, - &block_bytes, - &receipts_bytes, - &**client.db.read(), - &*client.chain.read() - ); - - client.pending_ancient_blocks.write().remove(&hash); - result.map(|_| ()).unwrap_or_else(|e| { - error!(target: "client", "Error importing ancient block: {}", e); - }); + let queued = self.queued_ancient_blocks.clone(); + let lock = self.ancient_blocks_import_lock.clone(); + match self.queue_ancient_blocks.queue(&mut self.io_channel.lock(), 1, move |client| { + trace_time!("import_ancient_block"); + // Make sure to hold the lock here to prevent importing out of order. + // We use separate lock, cause we don't want to block queueing. + let _lock = lock.lock(); + for _i in 0..MAX_ANCIENT_BLOCKS_TO_IMPORT { + let first = queued.write().1.pop_front(); + if let Some((header, block_bytes, receipts_bytes)) = first { + let hash = header.hash(); + client.importer.import_old_block( + &header, + &block_bytes, + &receipts_bytes, + &**client.db.read(), + &*client.chain.read() + ).ok().map_or((), |e| { + error!(target: "client", "Error importing ancient block: {}", e); + }); + // remove from pending + queued.write().0.remove(&hash); + } else { + break; + } + } }) { Ok(_) => Ok(hash), Err(e) => bail!(BlockImportErrorKind::Other(format!("{}", e))), @@ -2035,7 +2063,7 @@ impl IoClient for Client { } fn queue_consensus_message(&self, message: Bytes) { - match self.queue_consensus_message.queue(&mut self.io_channel.lock(), move |client| { + match self.queue_consensus_message.queue(&mut self.io_channel.lock(), 1, move |client| { if let Err(e) = client.engine().handle_message(&message) { debug!(target: "poa", "Invalid message received: {}", e); } @@ -2446,38 +2474,35 @@ impl fmt::Display for QueueError { /// Queue some items to be processed by IO client. struct IoChannelQueue { - queue: Arc>>>, + currently_queued: Arc, limit: usize, } impl IoChannelQueue { pub fn new(limit: usize) -> Self { IoChannelQueue { - queue: Default::default(), + currently_queued: Default::default(), limit, } } - pub fn queue(&self, channel: &mut IoChannel, fun: F) -> Result<(), QueueError> - where F: Fn(&Client) + Send + Sync + 'static + pub fn queue(&self, channel: &mut IoChannel, count: usize, fun: F) -> Result<(), QueueError> where + F: Fn(&Client) + Send + Sync + 'static, { - { - let mut queue = self.queue.lock(); - let queue_size = queue.len(); - ensure!(queue_size < self.limit, QueueError::Full(self.limit)); + let queue_size = self.currently_queued.load(AtomicOrdering::Relaxed); + ensure!(queue_size < self.limit, QueueError::Full(self.limit)); - queue.push_back(Box::new(fun)); - } - - let queue = self.queue.clone(); + let currently_queued = self.currently_queued.clone(); let result = channel.send(ClientIoMessage::execute(move |client| { - while let Some(fun) = queue.lock().pop_front() { - fun(client); - } + currently_queued.fetch_sub(count, AtomicOrdering::SeqCst); + fun(client); })); match result { - Ok(_) => Ok(()), + Ok(_) => { + self.currently_queued.fetch_add(count, AtomicOrdering::SeqCst); + Ok(()) + }, Err(e) => Err(QueueError::Channel(e)), } } diff --git a/ethcore/src/ethereum/mod.rs b/ethcore/src/ethereum/mod.rs index 5cf646268..dc2b16c3b 100644 --- a/ethcore/src/ethereum/mod.rs +++ b/ethcore/src/ethereum/mod.rs @@ -101,6 +101,9 @@ pub fn new_morden<'a, T: Into>>(params: T) -> Spec { /// Create a new Foundation Frontier-era chain spec as though it never changes to Homestead. pub fn new_frontier_test() -> Spec { load(None, include_bytes!("../../res/ethereum/frontier_test.json")) } +/// Create a new Ropsten chain spec. +pub fn new_ropsten_test() -> Spec { load(None, include_bytes!("../../res/ethereum/ropsten.json")) } + /// Create a new Foundation Homestead-era chain spec as though it never changed from Frontier. pub fn new_homestead_test() -> Spec { load(None, include_bytes!("../../res/ethereum/homestead_test.json")) } diff --git a/ethcore/src/machine.rs b/ethcore/src/machine.rs index dad28a771..e8d486ed2 100644 --- a/ethcore/src/machine.rs +++ b/ethcore/src/machine.rs @@ -503,6 +503,25 @@ mod tests { } } + #[test] + fn should_disallow_unsigned_transactions() { + let rlp = "ea80843b9aca0083015f90948921ebb5f79e9e3920abe571004d0b1d5119c154865af3107a400080038080".into(); + let transaction: UnverifiedTransaction = ::rlp::decode(&::rustc_hex::FromHex::from_hex(rlp).unwrap()).unwrap(); + let spec = ::ethereum::new_ropsten_test(); + let ethparams = get_default_ethash_extensions(); + + let machine = EthereumMachine::with_ethash_extensions( + spec.params().clone(), + Default::default(), + ethparams, + ); + let mut header = ::header::Header::new(); + header.set_number(15); + + let res = machine.verify_transaction_basic(&transaction, &header); + assert_eq!(res, Err(transaction::Error::InvalidSignature("Crypto error (Invalid EC signature)".into()))); + } + #[test] fn ethash_gas_limit_is_multiple_of_determinant() { use ethereum_types::U256; diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index 03a6d6f8d..2aee0dc37 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -574,7 +574,17 @@ mod tests { nonce: U256::from(2) }.sign(keypair.secret(), None); + let tr3 = Transaction { + action: Action::Call(0x0.into()), + value: U256::from(0), + data: Bytes::new(), + gas: U256::from(30_000), + gas_price: U256::from(0), + nonce: U256::zero(), + }.null_sign(0); + let good_transactions = [ tr1.clone(), tr2.clone() ]; + let eip86_transactions = [ tr3.clone() ]; let diff_inc = U256::from(0x40); @@ -610,6 +620,7 @@ mod tests { uncles_rlp.append_list(&good_uncles); let good_uncles_hash = keccak(uncles_rlp.as_raw()); let good_transactions_root = ordered_trie_root(good_transactions.iter().map(|t| ::rlp::encode::(t))); + let eip86_transactions_root = ordered_trie_root(eip86_transactions.iter().map(|t| ::rlp::encode::(t))); let mut parent = good.clone(); parent.set_number(9); @@ -630,6 +641,14 @@ mod tests { check_ok(basic_test(&create_test_block(&good), engine)); + let mut bad_header = good.clone(); + bad_header.set_transactions_root(eip86_transactions_root.clone()); + bad_header.set_uncles_hash(good_uncles_hash.clone()); + match basic_test(&create_test_block_with_data(&bad_header, &eip86_transactions, &good_uncles), engine) { + Err(Error(ErrorKind::Transaction(ref e), _)) if e == &::ethkey::Error::InvalidSignature.into() => (), + e => panic!("Block verification failed.\nExpected: Transaction Error (Invalid Signature)\nGot: {:?}", e), + } + let mut header = good.clone(); header.set_transactions_root(good_transactions_root.clone()); header.set_uncles_hash(good_uncles_hash.clone()); diff --git a/ethcore/transaction/src/transaction.rs b/ethcore/transaction/src/transaction.rs index 6152e61ac..717f0ca0b 100644 --- a/ethcore/transaction/src/transaction.rs +++ b/ethcore/transaction/src/transaction.rs @@ -409,6 +409,10 @@ impl UnverifiedTransaction { if check_low_s && !(allow_empty_signature && self.is_unsigned()) { self.check_low_s()?; } + // Disallow unsigned transactions in case EIP-86 is disabled. + if !allow_empty_signature && self.is_unsigned() { + return Err(ethkey::Error::InvalidSignature.into()); + } // EIP-86: Transactions of this form MUST have gasprice = 0, nonce = 0, value = 0, and do NOT increment the nonce of account 0. if allow_empty_signature && self.is_unsigned() && !(self.gas_price.is_zero() && self.value.is_zero() && self.nonce.is_zero()) { return Err(ethkey::Error::InvalidSignature.into()) diff --git a/mac/Parity.pkgproj b/mac/Parity.pkgproj index 0cb2082dc..b7f821af0 100755 --- a/mac/Parity.pkgproj +++ b/mac/Parity.pkgproj @@ -462,7 +462,7 @@ OVERWRITE_PERMISSIONS VERSION - 1.11.2 + 1.11.3 UUID 2DCD5B81-7BAF-4DA1-9251-6274B089FD36 diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index edc092a11..95385753a 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -174,7 +174,7 @@ impl TransactionQueue { transactions: Vec, ) -> Vec> { // Run verification - let _timer = ::trace_time::PerfTimer::new("queue::verifyAndImport"); + let _timer = ::trace_time::PerfTimer::new("pool::verify_and_import"); let options = self.options.read().clone(); let verifier = verifier::Verifier::new(client, options, self.insertion_id.clone()); diff --git a/nsis/installer.nsi b/nsis/installer.nsi index 4970b3fda..de972d5fa 100644 --- a/nsis/installer.nsi +++ b/nsis/installer.nsi @@ -10,7 +10,7 @@ !define DESCRIPTION "Fast, light, robust Ethereum implementation" !define VERSIONMAJOR 1 !define VERSIONMINOR 11 -!define VERSIONBUILD 2 +!define VERSIONBUILD 3 !define ARGS "" !define FIRST_START_ARGS "--mode=passive ui" diff --git a/util/version/Cargo.toml b/util/version/Cargo.toml index 48ad1e6e2..1c8e7fe58 100644 --- a/util/version/Cargo.toml +++ b/util/version/Cargo.toml @@ -3,7 +3,7 @@ [package] name = "parity-version" # NOTE: this value is used for Parity version string (via env CARGO_PKG_VERSION) -version = "1.11.2" +version = "1.11.3" authors = ["Parity Technologies "] build = "build.rs" @@ -13,7 +13,7 @@ build = "build.rs" track = "beta" # Indicates a critical release in this track (i.e. consensus issue) -critical = false +critical = true # Latest supported fork blocks for various networks. Used ONLY by auto-updater. [package.metadata.forks]