From 26dd67ebebfec56e073000a67e20eb9179343ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 10:57:09 +0100 Subject: [PATCH 01/34] Speeding up build --- .travis.yml | 67 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6ae41379e..115bf4ec6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,24 +8,46 @@ branches: - /^stable-.*$/ - /^beta$/ - /^stable$/ +git: + depth: 3 matrix: fast_finish: false allow_failures: - rust: nightly include: - rust: stable - env: FEATURES="--features travis-beta" KCOV_FEATURES="" TARGETS="-p ethash -p ethcore-util -p ethcore -p ethsync -p ethcore-rpc -p parity -p ethminer" ARCHIVE_SUFFIX="-${TRAVIS_OS_NAME}-${TRAVIS_TAG}" + env: FEATURES="--features travis-beta" RUN_TESTS="true" - rust: beta - env: FEATURES="--features travis-beta" KCOV_FEATURES="" TARGETS="-p ethash -p ethcore-util -p ethcore -p ethsync -p ethcore-rpc -p parity -p ethminer" ARCHIVE_SUFFIX="-${TRAVIS_OS_NAME}-${TRAVIS_TAG}" + env: FEATURES="--features travis-beta" RUN_TESTS="true" + - rust: stable + env: FEATURES="--features travis-beta" RUN_BUILD="true" + - rust: beta + env: FEATURES="--features travis-beta" RUN_BUILD="true" + - rust: stable + env: FEATURES="--features travis-beta" RUN_COVERAGE="true" - rust: nightly - env: FEATURES="--features travis-nightly" KCOV_FEATURES="" TARGETS="-p ethash -p ethcore-util -p ethcore -p ethsync -p ethcore-rpc -p parity -p ethminer" ARCHIVE_SUFFIX="-${TRAVIS_OS_NAME}-${TRAVIS_TAG}" + env: FEATURES="--features travis-nightly" RUN_BENCHES="true" + - rust: nightly + env: FEATURES="--features travis-nightly" RUN_TESTS="true" +env: + global: + # GH_TOKEN + - secure: bumJASbZSU8bxJ0EyPUJmu16AiV9EXOpyOj86Jlq/Ty9CfwGqsSXt96uDyE+OUJf34RUFQMsw0nk37/zC4lcn6kqk2wpuH3N/o85Zo/cVZY/NusBWLQqtT5VbYWsV+u2Ua4Tmmsw8yVYQhYwU2ZOejNpflL+Cs9XGgORp1L+/gMRMC2y5Se6ZhwnKPQlRJ8LGsG1dzjQULxzADIt3/zuspNBS8a2urJwlHfGMkvHDoUWCviP/GXoSqw3TZR7FmKyxE19I8n9+iSvm9+oZZquvcgfUxMHn8Gq/b44UbPvjtFOg2yam4xdWXF/RyWCHdc/R9EHorSABeCbefIsm+zcUF3/YQxwpSxM4IZEeH2rTiC7dcrsKw3XsO16xFQz5YI5Bay+CT/wTdMmJd7DdYz7Dyf+pOvcM9WOf/zorxYWSBOMYy0uzbusU2iyIghQ82s7E/Ahg+WARtPgkuTLSB5aL1oCTBKHqQscMr7lo5Ti6RpWLxEdTQMBznc+bMr+6dEtkEcG9zqc6cE9XX+ox3wTU6+HVMfQ1ltCntJ4UKcw3A6INEbw9wgocQa812CIASQ2fE+SCAbz6JxBjIAlFUnD1lUB7S8PdMPwn9plfQgKQ2A5YZqg6FnBdf0rQXIJYxQWKHXj/rBHSUCT0tHACDlzTA+EwWggvkP5AGIxRxm8jhw= + - CVER=5 + - NUM_JOBS=1 + # - TARGETS="-p ethash -p ethcore-util -p ethcore -p ethsync -p ethcore-rpc -p parity -p ethminer" + - TARGETS="-p ethcore-util" + - ARCHIVE_SUFFIX="-${TRAVIS_OS_NAME}-${TRAVIS_TAG}" + - KCOV_FEATURES="" + - KCOV_CMD="./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov" + - RUN_TESTS="false" + - RUN_COVERAGE="false" + - RUN_BUILD="false" + - RUN_BENCHES="false" cache: apt: true directories: - - target/debug/deps - - target/debug/build - - target/release/deps - - target/release/build + - $TRAVIS_BUILD_DIR/target - $HOME/.cargo addons: apt: @@ -33,22 +55,27 @@ addons: - libcurl4-openssl-dev - libelf-dev - libdw-dev + script: -- cargo build --release --verbose ${FEATURES} -- cargo test --release --verbose ${FEATURES} ${TARGETS} -#- cargo bench --no-run ${FEATURES} ${TARGETS} -- tar cvzf parity${ARCHIVE_SUFFIX}.tar.gz -C target/release parity + - echo "$TRAVIS_BUILD_DIR/target" + - ls "$TRAVIS_BUILD_DIR/target" + - if [ "$RUN_TESTS" = "true" ]; then cargo test --release --verbose ${FEATURES} ${TARGETS}; fi + - if [ "$RUN_BENCHES" = "true" ]; then cargo bench --no-run ${FEATURES} ${TARGETS}; fi + - if [ "$RUN_BUILD" = "true" ]; then cargo build --release --verbose ${FEATURES}; fi + - if [ "$RUN_BUILD" = "true" ]; then tar cvzf parity${ARCHIVE_SUFFIX}.tar.gz -C target/release parity; fi + after_success: | + [ "$RUN_COVERAGE" = "true" ] && wget https://github.com/SimonKagstrom/kcov/archive/master.tar.gz && tar xzf master.tar.gz && mkdir kcov-master/build && cd kcov-master/build && cmake .. && make && make install DESTDIR=../tmp && cd ../.. && cargo test --no-run ${KCOV_FEATURES} ${TARGETS} && - ./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov target/debug/deps/ethcore_util-* && - ./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov target/debug/deps/ethash-* && - ./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov target/debug/deps/ethcore-* && - ./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov target/debug/deps/ethsync-* && - ./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov target/debug/deps/ethcore_rpc-* && - ./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov target/debug/deps/ethminer-* && - ./kcov-master/tmp/usr/local/bin/kcov --coveralls-id=${TRAVIS_JOB_ID} --exclude-pattern /usr/,/.cargo,/root/.multirust target/kcov target/debug/parity-* && + $KCOV_CMD target/debug/deps/ethcore_util-* && + $KCOV_CMD target/debug/deps/ethash-* && + $KCOV_CMD target/debug/deps/ethcore-* && + $KCOV_CMD target/debug/deps/ethsync-* && + $KCOV_CMD target/debug/deps/ethcore_rpc-* && + $KCOV_CMD target/debug/deps/ethminer-* && + $KCOV_CMD target/debug/parity-* && [ $TRAVIS_BRANCH = master ] && [ $TRAVIS_PULL_REQUEST = false ] && [ $TRAVIS_RUST_VERSION = stable ] && @@ -57,10 +84,6 @@ after_success: | pip install --user ghp-import && /home/travis/.local/bin/ghp-import -n target/doc && git push -fq https://${GH_TOKEN}@github.com/${TRAVIS_REPO_SLUG}.git gh-pages -env: - global: - # GH_TOKEN - - secure: bumJASbZSU8bxJ0EyPUJmu16AiV9EXOpyOj86Jlq/Ty9CfwGqsSXt96uDyE+OUJf34RUFQMsw0nk37/zC4lcn6kqk2wpuH3N/o85Zo/cVZY/NusBWLQqtT5VbYWsV+u2Ua4Tmmsw8yVYQhYwU2ZOejNpflL+Cs9XGgORp1L+/gMRMC2y5Se6ZhwnKPQlRJ8LGsG1dzjQULxzADIt3/zuspNBS8a2urJwlHfGMkvHDoUWCviP/GXoSqw3TZR7FmKyxE19I8n9+iSvm9+oZZquvcgfUxMHn8Gq/b44UbPvjtFOg2yam4xdWXF/RyWCHdc/R9EHorSABeCbefIsm+zcUF3/YQxwpSxM4IZEeH2rTiC7dcrsKw3XsO16xFQz5YI5Bay+CT/wTdMmJd7DdYz7Dyf+pOvcM9WOf/zorxYWSBOMYy0uzbusU2iyIghQ82s7E/Ahg+WARtPgkuTLSB5aL1oCTBKHqQscMr7lo5Ti6RpWLxEdTQMBznc+bMr+6dEtkEcG9zqc6cE9XX+ox3wTU6+HVMfQ1ltCntJ4UKcw3A6INEbw9wgocQa812CIASQ2fE+SCAbz6JxBjIAlFUnD1lUB7S8PdMPwn9plfQgKQ2A5YZqg6FnBdf0rQXIJYxQWKHXj/rBHSUCT0tHACDlzTA+EwWggvkP5AGIxRxm8jhw= deploy: provider: releases From 2cecb1eadabc3ceb7d4d666e67d0aa78120188bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 11:04:48 +0100 Subject: [PATCH 02/34] Cleaning --- .travis.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 115bf4ec6..905f92bbd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,10 +33,7 @@ env: global: # GH_TOKEN - secure: bumJASbZSU8bxJ0EyPUJmu16AiV9EXOpyOj86Jlq/Ty9CfwGqsSXt96uDyE+OUJf34RUFQMsw0nk37/zC4lcn6kqk2wpuH3N/o85Zo/cVZY/NusBWLQqtT5VbYWsV+u2Ua4Tmmsw8yVYQhYwU2ZOejNpflL+Cs9XGgORp1L+/gMRMC2y5Se6ZhwnKPQlRJ8LGsG1dzjQULxzADIt3/zuspNBS8a2urJwlHfGMkvHDoUWCviP/GXoSqw3TZR7FmKyxE19I8n9+iSvm9+oZZquvcgfUxMHn8Gq/b44UbPvjtFOg2yam4xdWXF/RyWCHdc/R9EHorSABeCbefIsm+zcUF3/YQxwpSxM4IZEeH2rTiC7dcrsKw3XsO16xFQz5YI5Bay+CT/wTdMmJd7DdYz7Dyf+pOvcM9WOf/zorxYWSBOMYy0uzbusU2iyIghQ82s7E/Ahg+WARtPgkuTLSB5aL1oCTBKHqQscMr7lo5Ti6RpWLxEdTQMBznc+bMr+6dEtkEcG9zqc6cE9XX+ox3wTU6+HVMfQ1ltCntJ4UKcw3A6INEbw9wgocQa812CIASQ2fE+SCAbz6JxBjIAlFUnD1lUB7S8PdMPwn9plfQgKQ2A5YZqg6FnBdf0rQXIJYxQWKHXj/rBHSUCT0tHACDlzTA+EwWggvkP5AGIxRxm8jhw= - - CVER=5 - - NUM_JOBS=1 - # - TARGETS="-p ethash -p ethcore-util -p ethcore -p ethsync -p ethcore-rpc -p parity -p ethminer" - - TARGETS="-p ethcore-util" + - TARGETS="-p ethash -p ethcore-util -p ethcore -p ethsync -p ethcore-rpc -p parity -p ethminer" - ARCHIVE_SUFFIX="-${TRAVIS_OS_NAME}-${TRAVIS_TAG}" - KCOV_FEATURES="" - KCOV_CMD="./kcov-master/tmp/usr/local/bin/kcov --exclude-pattern /usr/,/.cargo,/root/.multirust,src/tests,util/json-tests,util/src/network/tests,sync/src/tests,ethcore/src/tests,ethcore/src/evm/tests target/kcov" From 7045e9f2f7db6a4dbdb6ff75b4528f7f20a23006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 11:05:31 +0100 Subject: [PATCH 03/34] More cleaning --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 905f92bbd..7ac668d62 100644 --- a/.travis.yml +++ b/.travis.yml @@ -54,8 +54,6 @@ addons: - libdw-dev script: - - echo "$TRAVIS_BUILD_DIR/target" - - ls "$TRAVIS_BUILD_DIR/target" - if [ "$RUN_TESTS" = "true" ]; then cargo test --release --verbose ${FEATURES} ${TARGETS}; fi - if [ "$RUN_BENCHES" = "true" ]; then cargo bench --no-run ${FEATURES} ${TARGETS}; fi - if [ "$RUN_BUILD" = "true" ]; then cargo build --release --verbose ${FEATURES}; fi From 74245be4f58a337c750c5a007ad1401afc35c724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 15:09:12 +0100 Subject: [PATCH 04/34] Enabling fast finish --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 7ac668d62..990b4852f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,7 @@ branches: git: depth: 3 matrix: - fast_finish: false + fast_finish: true allow_failures: - rust: nightly include: From d57b0177a755dbf8828530c70b6d9954d0ba7951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 15:55:37 +0100 Subject: [PATCH 05/34] Disabling benches and beta tests --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 990b4852f..5d6de65c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,16 +17,16 @@ matrix: include: - rust: stable env: FEATURES="--features travis-beta" RUN_TESTS="true" - - rust: beta - env: FEATURES="--features travis-beta" RUN_TESTS="true" + # - rust: beta + # env: FEATURES="--features travis-beta" RUN_TESTS="true" - rust: stable env: FEATURES="--features travis-beta" RUN_BUILD="true" - rust: beta env: FEATURES="--features travis-beta" RUN_BUILD="true" - rust: stable env: FEATURES="--features travis-beta" RUN_COVERAGE="true" - - rust: nightly - env: FEATURES="--features travis-nightly" RUN_BENCHES="true" + # - rust: nightly + # env: FEATURES="--features travis-nightly" RUN_BENCHES="true" - rust: nightly env: FEATURES="--features travis-nightly" RUN_TESTS="true" env: From ab4bfbac0d85c0ee02ed21ca104bee19943ba7fa Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Tue, 15 Mar 2016 17:13:44 +0100 Subject: [PATCH 06/34] adding check for a sync --- rpc/src/v1/impls/eth.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index fda391304..66339241e 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -321,6 +321,14 @@ impl Eth for EthClient fn work(&self, params: Params) -> Result { match params { Params::None => { + // check if we a still syncing and return empty strings int that case + { + let sync = take_weak!(self.sync); + if sync.status().state != SyncState::Idle { + return to_value(&(String::new(), String::new(), String::new())); + } + } + let miner = take_weak!(self.miner); let client = take_weak!(self.client); let u = miner.sealing_block(client.deref()).lock().unwrap(); From 99bae239960ba4a90f1cf1dc98c2011cd55d6e3f Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Tue, 15 Mar 2016 17:56:35 +0100 Subject: [PATCH 07/34] [ci skip] grammar fix --- rpc/src/v1/impls/eth.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 66339241e..0e5d35dca 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -321,7 +321,7 @@ impl Eth for EthClient fn work(&self, params: Params) -> Result { match params { Params::None => { - // check if we a still syncing and return empty strings int that case + // check if we're still syncing and return empty strings int that case { let sync = take_weak!(self.sync); if sync.status().state != SyncState::Idle { From 188e325b2060ab30377ba97764f4fbae6a27da0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 23:01:36 +0100 Subject: [PATCH 08/34] Importing transactions from hashset. Notifying about every block --- ethcore/src/client/client.rs | 2 +- miner/src/miner.rs | 34 +++++++++++++++++++++++----------- miner/src/transaction_queue.rs | 10 ++++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index caa92db97..6857416d3 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -317,7 +317,7 @@ impl Client where V: Verifier { } { - if !imported_blocks.is_empty() && self.block_queue.queue_info().is_empty() { + if !imported_blocks.is_empty() { let (enacted, retracted) = self.calculate_enacted_retracted(import_results); io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { imported: imported_blocks, diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 6d5b3086e..4652ab3db 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -18,6 +18,7 @@ use rayon::prelude::*; use std::sync::{Mutex, RwLock, Arc}; use std::sync::atomic; use std::sync::atomic::AtomicBool; +use std::collections::HashSet; use util::{H256, U256, Address, Bytes, Uint}; use ethcore::views::{BlockView}; @@ -174,21 +175,10 @@ impl MinerService for Miner { let block = BlockView::new(&block); block.transactions() } - { - let in_chain = vec![imported, enacted, invalid]; - let in_chain = in_chain - .par_iter() - .flat_map(|h| h.par_iter().map(|h| fetch_transactions(chain, h))); let out_of_chain = retracted .par_iter() .map(|h| fetch_transactions(chain, h)); - - in_chain.for_each(|txs| { - let mut transaction_queue = self.transaction_queue.lock().unwrap(); - let hashes = txs.iter().map(|tx| tx.hash()).collect::>(); - transaction_queue.remove_all(&hashes, |a| chain.nonce(a)); - }); out_of_chain.for_each(|txs| { // populate sender for tx in &txs { @@ -198,6 +188,28 @@ impl MinerService for Miner { let _ = transaction_queue.add_all(txs, |a| chain.nonce(a)); }); } + // First import all transactions and after that remove old ones + { + let in_chain = { + let mut in_chain = HashSet::new(); + in_chain.extend(imported); + in_chain.extend(enacted); + in_chain.extend(invalid); + in_chain + .into_iter() + .collect::>() + }; + + let in_chain = in_chain + .par_iter() + .map(|h: &H256| fetch_transactions(chain, h)); + + in_chain.for_each(|txs| { + let hashes = txs.iter().map(|tx| tx.hash()).collect::>(); + let mut transaction_queue = self.transaction_queue.lock().unwrap(); + transaction_queue.remove_all(&hashes, |a| chain.nonce(a)); + }); + } if self.sealing_enabled.load(atomic::Ordering::Relaxed) { self.prepare_sealing(chain); diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 324a46364..04febb84d 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -351,6 +351,8 @@ impl TransactionQueue { /// If gap is introduced marks subsequent transactions as future pub fn remove(&mut self, transaction_hash: &H256, fetch_nonce: &T) where T: Fn(&Address) -> U256 { + + println!("Removing transaction: (hash: {:?})", transaction_hash); let transaction = self.by_hash.remove(transaction_hash); if transaction.is_none() { // We don't know this transaction @@ -362,6 +364,8 @@ impl TransactionQueue { let nonce = transaction.nonce(); let current_nonce = fetch_nonce(&sender); + println!("Removing transaction: ({:?}, {:?}, hash: {:?})", sender, nonce, transaction.hash()); + // Remove from future let order = self.future.drop(&sender, &nonce); if order.is_some() { @@ -489,12 +493,14 @@ impl TransactionQueue { fn import_tx(&mut self, tx: VerifiedTransaction, fetch_nonce: &T) where T: Fn(&Address) -> U256 { + if self.by_hash.get(&tx.hash()).is_some() { // Transaction is already imported. trace!(target: "sync", "Dropping already imported transaction with hash: {:?}", tx.hash()); return; } + let address = tx.sender(); let nonce = tx.nonce(); @@ -506,6 +512,7 @@ impl TransactionQueue { // Check height if nonce > next_nonce { + println!("[F] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); // We have a gap - put to future Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash); self.future.enforce_limit(&mut self.by_hash); @@ -515,6 +522,7 @@ impl TransactionQueue { trace!(target: "sync", "Dropping transaction with nonce: {} - expecting: {}", nonce, next_nonce); return; } + println!("[C] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash); self.last_nonces.insert(address, nonce); @@ -540,11 +548,13 @@ impl TransactionQueue { let new_fee = order.gas_price; if old_fee.cmp(&new_fee) == Ordering::Greater { // Put back old transaction since it has greater priority (higher gas_price) + println!("Didn't replace tx (h:{:?}, h:{:?})", hash, old.hash); set.by_address.insert(address, nonce, old); // and remove new one set.by_priority.remove(&order); by_hash.remove(&hash); } else { + println!("Replaced h:{:?} with h:{:?}, ", old.hash, hash); // Make sure we remove old transaction entirely set.by_priority.remove(&old); by_hash.remove(&old.hash); From 974222aabd114ba9e96c125aa622cf66ef408876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 23:13:53 +0100 Subject: [PATCH 09/34] Removing printlns --- miner/src/transaction_queue.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 04febb84d..4b365bba2 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -352,7 +352,6 @@ impl TransactionQueue { pub fn remove(&mut self, transaction_hash: &H256, fetch_nonce: &T) where T: Fn(&Address) -> U256 { - println!("Removing transaction: (hash: {:?})", transaction_hash); let transaction = self.by_hash.remove(transaction_hash); if transaction.is_none() { // We don't know this transaction @@ -364,7 +363,6 @@ impl TransactionQueue { let nonce = transaction.nonce(); let current_nonce = fetch_nonce(&sender); - println!("Removing transaction: ({:?}, {:?}, hash: {:?})", sender, nonce, transaction.hash()); // Remove from future let order = self.future.drop(&sender, &nonce); @@ -512,7 +510,6 @@ impl TransactionQueue { // Check height if nonce > next_nonce { - println!("[F] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); // We have a gap - put to future Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash); self.future.enforce_limit(&mut self.by_hash); @@ -522,7 +519,6 @@ impl TransactionQueue { trace!(target: "sync", "Dropping transaction with nonce: {} - expecting: {}", nonce, next_nonce); return; } - println!("[C] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash); self.last_nonces.insert(address, nonce); @@ -548,13 +544,11 @@ impl TransactionQueue { let new_fee = order.gas_price; if old_fee.cmp(&new_fee) == Ordering::Greater { // Put back old transaction since it has greater priority (higher gas_price) - println!("Didn't replace tx (h:{:?}, h:{:?})", hash, old.hash); set.by_address.insert(address, nonce, old); // and remove new one set.by_priority.remove(&order); by_hash.remove(&hash); } else { - println!("Replaced h:{:?} with h:{:?}, ", old.hash, hash); // Make sure we remove old transaction entirely set.by_priority.remove(&old); by_hash.remove(&old.hash); From 8427e99c73f6ed3d0db427ef4a9e42ef0410b0ba Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Tue, 15 Mar 2016 23:58:46 +0100 Subject: [PATCH 10/34] checking queue also --- rpc/src/v1/impls/eth.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 0e5d35dca..32e9b44b1 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -321,10 +321,11 @@ impl Eth for EthClient fn work(&self, params: Params) -> Result { match params { Params::None => { + let client = take_weak!(self.client); // check if we're still syncing and return empty strings int that case { let sync = take_weak!(self.sync); - if sync.status().state != SyncState::Idle { + if sync.status().state != SyncState::Idle && client.queue_info().is_empty() { return to_value(&(String::new(), String::new(), String::new())); } } From bd892026f632df65e4b5b075295a670eac85d422 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Wed, 16 Mar 2016 10:37:08 +0100 Subject: [PATCH 11/34] tests --- rpc/src/v1/impls/eth.rs | 2 +- rpc/src/v1/tests/eth.rs | 28 +++++++++++++++++++---- rpc/src/v1/tests/helpers/miner_service.rs | 20 +++++++++++++--- rpc/src/v1/tests/helpers/sync_provider.rs | 9 ++++---- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 32e9b44b1..520b37045 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -340,7 +340,7 @@ impl Eth for EthClient let seed_hash = Ethash::get_seedhash(b.block().header().number()); to_value(&(pow_hash, seed_hash, target)) } - _ => Err(Error::invalid_params()) + _ => Err(Error::internal_error()) } }, _ => Err(Error::invalid_params()) diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 6bc929709..280557e4b 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -43,12 +43,12 @@ fn sync_provider() -> Arc { } fn miner_service() -> Arc { - Arc::new(TestMinerService) + Arc::new(TestMinerService::default()) } struct EthTester { - client: Arc, - _sync: Arc, + pub client: Arc, + pub sync: Arc, _accounts_provider: Arc, _miner: Arc, hashrates: Arc>>, @@ -68,7 +68,7 @@ impl Default for EthTester { io.add_delegate(eth); EthTester { client: client, - _sync: sync, + sync: sync, _accounts_provider: ap, _miner: miner, io: io, @@ -346,5 +346,25 @@ fn rpc_eth_compile_serpent() { assert_eq!(EthTester::default().io.handle_request(request), Some(response.to_owned())); } +#[test] +fn returns_no_work_if_cant_mine() { + let eth_tester = EthTester::default(); + let request = r#"{"jsonrpc": "2.0", "method": "eth_getWork", "params": [], "id": 1}"#; + let response = r#"{"jsonrpc":"2.0","result":["","",""],"id":1}"#; + assert_eq!(eth_tester.io.handle_request(request), Some(response.to_owned())); +} + +#[test] +fn returns_error_if_can_mine_and_no_closed_block() { + use ethsync::{SyncState}; + + let eth_tester = EthTester::default(); + eth_tester.sync.status.write().unwrap().state = SyncState::Idle; + + let request = r#"{"jsonrpc": "2.0", "method": "eth_getWork", "params": [], "id": 1}"#; + let response = r#"{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error","data":null},"id":1}"#; + + assert_eq!(eth_tester.io.handle_request(request), Some(response.to_owned())); +} diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 0cddf2a1e..3a60bd136 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -22,7 +22,19 @@ use ethcore::block::ClosedBlock; use ethcore::transaction::SignedTransaction; use ethminer::{MinerService, MinerStatus}; -pub struct TestMinerService; +pub struct TestMinerService { + pub imported_transactions: RwLock>, + pub latest_closed_block: Mutex>, +} + +impl Default for TestMinerService { + fn default() -> TestMinerService { + TestMinerService { + imported_transactions: RwLock::new(Vec::new()), + latest_closed_block: Mutex::new(None), + } + } +} impl MinerService for TestMinerService { @@ -45,9 +57,11 @@ impl MinerService for TestMinerService { fn prepare_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> { unimplemented!(); } + fn sealing_block(&self, _chain: &BlockChainClient) -> &Mutex> { + &self.latest_closed_block + } /// 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!(); } -} \ No newline at end of file +} diff --git a/rpc/src/v1/tests/helpers/sync_provider.rs b/rpc/src/v1/tests/helpers/sync_provider.rs index 631752dfc..bfbfad44b 100644 --- a/rpc/src/v1/tests/helpers/sync_provider.rs +++ b/rpc/src/v1/tests/helpers/sync_provider.rs @@ -15,6 +15,7 @@ // along with Parity. If not, see . use ethsync::{SyncProvider, SyncStatus, SyncState}; +use std::sync::{RwLock}; pub struct Config { pub protocol_version: u8, @@ -22,13 +23,13 @@ pub struct Config { } pub struct TestSyncProvider { - status: SyncStatus, + pub status: RwLock, } impl TestSyncProvider { pub fn new(config: Config) -> Self { TestSyncProvider { - status: SyncStatus { + status: RwLock::new(SyncStatus { state: SyncState::NotSynced, protocol_version: config.protocol_version, start_block_number: 0, @@ -39,14 +40,14 @@ impl TestSyncProvider { num_peers: config.num_peers, num_active_peers: 0, mem_used: 0, - }, + }) } } } impl SyncProvider for TestSyncProvider { fn status(&self) -> SyncStatus { - self.status.clone() + self.status.read().unwrap().clone() } } From fdba8de6000e8d629d7bc4a504a8cedb03d6168f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 10:40:33 +0100 Subject: [PATCH 12/34] Validating senders balance before importing transaction to queue --- ethcore/src/error.rs | 9 ++- miner/src/lib.rs | 8 +-- miner/src/miner.rs | 20 ++++-- miner/src/transaction_queue.rs | 81 +++++++++++++++-------- rpc/src/v1/impls/eth.rs | 7 +- rpc/src/v1/tests/helpers/miner_service.rs | 7 +- sync/src/chain.rs | 9 ++- 7 files changed, 94 insertions(+), 47 deletions(-) diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 72127c754..482a8de01 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -70,7 +70,14 @@ pub enum TransactionError { /// Minimal expected gas price minimal: U256, /// Transaction gas price - got: U256 + got: U256, + }, + /// Sender doesn't have enough funds to pay for this transaction + InsufficientBalance { + /// Senders balance + balance: U256, + /// Transaction cost + cost: U256, }, /// Transaction's gas limit (aka gas) is invalid. InvalidGasLimit(OutOfBounds), diff --git a/miner/src/lib.rs b/miner/src/lib.rs index a431bd44e..3c9775268 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -62,11 +62,11 @@ extern crate rayon; mod miner; mod transaction_queue; -pub use transaction_queue::TransactionQueue; +pub use transaction_queue::{TransactionQueue, AccountDetails}; pub use miner::{Miner}; use std::sync::Mutex; -use util::{H256, U256, Address, Bytes}; +use util::{H256, Address, Bytes}; use ethcore::client::{BlockChainClient}; use ethcore::block::{ClosedBlock}; use ethcore::error::{Error}; @@ -79,8 +79,8 @@ pub trait MinerService : Send + Sync { fn status(&self) -> MinerStatus; /// Imports transactions to transaction queue. - fn import_transactions(&self, transactions: Vec, fetch_nonce: T) -> Result<(), Error> - where T: Fn(&Address) -> U256; + fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails; /// Returns hashes of transactions currently in pending fn pending_transactions_hashes(&self) -> Vec; diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 4652ab3db..27cf2ded2 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -26,7 +26,7 @@ use ethcore::client::{BlockChainClient, BlockId}; use ethcore::block::{ClosedBlock}; use ethcore::error::{Error}; use ethcore::transaction::SignedTransaction; -use super::{MinerService, MinerStatus, TransactionQueue}; +use super::{MinerService, MinerStatus, TransactionQueue, AccountDetails}; /// Keeps track of transactions using priority queue and holds currently mined block. pub struct Miner { @@ -72,7 +72,7 @@ impl Miner { /// Get the extra_data that we will seal blocks wuth. fn gas_floor_target(&self) -> U256 { - self.gas_floor_target.read().unwrap().clone() + *self.gas_floor_target.read().unwrap() } /// Set the author that we will seal blocks as. @@ -111,10 +111,10 @@ impl MinerService for Miner { } } - fn import_transactions(&self, transactions: Vec, fetch_nonce: T) -> Result<(), Error> - where T: Fn(&Address) -> U256 { + fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { let mut transaction_queue = self.transaction_queue.lock().unwrap(); - transaction_queue.add_all(transactions, fetch_nonce) + transaction_queue.add_all(transactions, fetch_account) } fn pending_transactions_hashes(&self) -> Vec { @@ -185,7 +185,10 @@ impl MinerService for Miner { let _sender = tx.sender(); } let mut transaction_queue = self.transaction_queue.lock().unwrap(); - let _ = transaction_queue.add_all(txs, |a| chain.nonce(a)); + let _ = transaction_queue.add_all(txs, |a| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a) + }); }); } // First import all transactions and after that remove old ones @@ -207,7 +210,10 @@ impl MinerService for Miner { in_chain.for_each(|txs| { let hashes = txs.iter().map(|tx| tx.hash()).collect::>(); let mut transaction_queue = self.transaction_queue.lock().unwrap(); - transaction_queue.remove_all(&hashes, |a| chain.nonce(a)); + transaction_queue.remove_all(&hashes, |a| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a) + }); }); } diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 4b365bba2..ca4a269ae 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -232,8 +232,6 @@ impl TransactionSet { } } -// Will be used when rpc merged -#[allow(dead_code)] #[derive(Debug)] /// Current status of the queue pub struct TransactionQueueStatus { @@ -243,6 +241,14 @@ pub struct TransactionQueueStatus { pub future: usize, } +/// Details of account +pub struct AccountDetails { + /// Most recent account nonce + pub nonce: U256, + /// Current account balance + pub balance: U256, +} + /// TransactionQueue implementation pub struct TransactionQueue { /// Gas Price threshold for transactions that can be imported to this queue (defaults to 0) @@ -308,49 +314,63 @@ impl TransactionQueue { } /// Adds all signed transactions to queue to be verified and imported - pub fn add_all(&mut self, txs: Vec, fetch_nonce: T) -> Result<(), Error> - where T: Fn(&Address) -> U256 { + pub fn add_all(&mut self, txs: Vec, fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { for tx in txs.into_iter() { - try!(self.add(tx, &fetch_nonce)); + try!(self.add(tx, &fetch_account)); } Ok(()) } /// Add signed transaction to queue to be verified and imported - pub fn add(&mut self, tx: SignedTransaction, fetch_nonce: &T) -> Result<(), Error> - where T: Fn(&Address) -> U256 { + pub fn add(&mut self, tx: SignedTransaction, fetch_account: &T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { + + trace!(target: "miner", "Importing: {:?}", tx.hash()); if tx.gas_price < self.minimal_gas_price { - trace!(target: "sync", + trace!(target: "miner", "Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})", tx.hash(), tx.gas_price, self.minimal_gas_price ); return Err(Error::Transaction(TransactionError::InsufficientGasPrice{ minimal: self.minimal_gas_price, - got: tx.gas_price + got: tx.gas_price, })); } - self.import_tx(try!(VerifiedTransaction::new(tx)), fetch_nonce); + let vtx = try!(VerifiedTransaction::new(tx)); + let account = fetch_account(&vtx.sender()); + + if account.balance < vtx.transaction.value { + trace!(target: "miner", "Dropping transaction without sufficient balance: {:?} ({} < {})", + vtx.hash(), account.balance, vtx.transaction.value); + return Err(Error::Transaction(TransactionError::InsufficientBalance { + cost: vtx.transaction.value, + balance: account.balance + })); + } + + self.import_tx(vtx, account.nonce); Ok(()) } /// Removes all transactions identified by hashes given in slice /// /// If gap is introduced marks subsequent transactions as future - pub fn remove_all(&mut self, transaction_hashes: &[H256], fetch_nonce: T) - where T: Fn(&Address) -> U256 { + pub fn remove_all(&mut self, transaction_hashes: &[H256], fetch_account: T) + where T: Fn(&Address) -> AccountDetails { for hash in transaction_hashes { - self.remove(&hash, &fetch_nonce); + self.remove(&hash, &fetch_account); } } /// Removes transaction identified by hashes from queue. /// /// If gap is introduced marks subsequent transactions as future - pub fn remove(&mut self, transaction_hash: &H256, fetch_nonce: &T) - where T: Fn(&Address) -> U256 { + pub fn remove(&mut self, transaction_hash: &H256, fetch_account: &T) + where T: Fn(&Address) -> AccountDetails { let transaction = self.by_hash.remove(transaction_hash); if transaction.is_none() { @@ -361,7 +381,7 @@ impl TransactionQueue { let transaction = transaction.unwrap(); let sender = transaction.sender(); let nonce = transaction.nonce(); - let current_nonce = fetch_nonce(&sender); + let current_nonce = fetch_account(&sender).nonce; // Remove from future @@ -403,6 +423,7 @@ impl TransactionQueue { if k >= current_nonce { self.future.insert(*sender, k, order.update_height(k, current_nonce)); } else { + trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); // Remove the transaction completely self.by_hash.remove(&order.hash); } @@ -423,6 +444,7 @@ impl TransactionQueue { if k >= current_nonce { self.future.insert(*sender, k, order.update_height(k, current_nonce)); } else { + trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); self.by_hash.remove(&order.hash); } } @@ -488,13 +510,11 @@ impl TransactionQueue { /// /// It ignores transactions that has already been imported (same `hash`) and replaces the transaction /// iff `(address, nonce)` is the same but `gas_price` is higher. - fn import_tx(&mut self, tx: VerifiedTransaction, fetch_nonce: &T) - where T: Fn(&Address) -> U256 { - + fn import_tx(&mut self, tx: VerifiedTransaction, state_nonce: U256) { if self.by_hash.get(&tx.hash()).is_some() { // Transaction is already imported. - trace!(target: "sync", "Dropping already imported transaction with hash: {:?}", tx.hash()); + trace!(target: "miner", "Dropping already imported transaction: {:?}", tx.hash()); return; } @@ -502,7 +522,6 @@ impl TransactionQueue { let address = tx.sender(); let nonce = tx.nonce(); - let state_nonce = fetch_nonce(&address); let next_nonce = self.last_nonces .get(&address) .cloned() @@ -516,7 +535,7 @@ impl TransactionQueue { return; } else if nonce < state_nonce { // Droping transaction - trace!(target: "sync", "Dropping transaction with nonce: {} - expecting: {}", nonce, next_nonce); + trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", tx.hash(), nonce, next_nonce); return; } @@ -525,6 +544,8 @@ impl TransactionQueue { // But maybe there are some more items waiting in future? self.move_matching_future_to_current(address, nonce + U256::one(), state_nonce); self.current.enforce_limit(&mut self.by_hash); + + trace!(target: "miner", "status: {:?}", self.status()); } /// Replaces transaction in given set (could be `future` or `current`). @@ -583,8 +604,11 @@ mod test { new_unsigned_tx(U256::from(123)).sign(&keypair.secret()) } - fn default_nonce(_address: &Address) -> U256 { - U256::from(123) + fn default_nonce(_address: &Address) -> AccountDetails { + AccountDetails { + nonce: U256::from(123), + balance: !U256::zero() + } } fn new_txs(second_nonce: U256) -> (SignedTransaction, SignedTransaction) { @@ -953,7 +977,8 @@ mod test { #[test] fn should_not_move_to_future_if_state_nonce_is_higher() { // given - let next_nonce = |a: &Address| default_nonce(a) + U256::one(); + let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: + !U256::zero() }; let mut txq = TransactionQueue::new(); let (tx, tx2) = new_txs(U256::from(1)); let tx3 = new_tx(); @@ -1028,8 +1053,10 @@ mod test { #[test] fn should_recalculate_height_when_removing_from_future() { // given - let previous_nonce = |a: &Address| default_nonce(a) - U256::one(); - let next_nonce = |a: &Address| default_nonce(a) + U256::one(); + let previous_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance: + !U256::zero() }; + let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: + !U256::zero() }; let mut txq = TransactionQueue::new(); let (tx1, tx2) = new_txs(U256::one()); txq.add(tx1.clone(), &previous_nonce).unwrap(); diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index fda391304..04b25d5b2 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -19,7 +19,7 @@ use std::collections::HashSet; use std::sync::{Arc, Weak, Mutex}; use std::ops::Deref; use ethsync::{SyncProvider, SyncState}; -use ethminer::{MinerService}; +use ethminer::{MinerService, AccountDetails}; use jsonrpc_core::*; use util::numbers::*; use util::sha3::*; @@ -370,7 +370,10 @@ impl Eth for EthClient let signed_transaction = transaction.sign(&secret); let hash = signed_transaction.hash(); - let import = miner.import_transactions(vec![signed_transaction], |a: &Address| client.nonce(a)); + let import = miner.import_transactions(vec![signed_transaction], |a: &Address| AccountDetails { + nonce: client.nonce(a), + balance: client.balance(a) + }); match import { Ok(_) => to_value(&hash), Err(e) => { diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 0cddf2a1e..b2aebb29c 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -20,7 +20,7 @@ use ethcore::error::Error; use ethcore::client::BlockChainClient; use ethcore::block::ClosedBlock; use ethcore::transaction::SignedTransaction; -use ethminer::{MinerService, MinerStatus}; +use ethminer::{MinerService, MinerStatus, AccountDetails}; pub struct TestMinerService; @@ -30,7 +30,8 @@ impl MinerService for TestMinerService { fn status(&self) -> MinerStatus { unimplemented!(); } /// Imports transactions to transaction queue. - fn import_transactions(&self, _transactions: Vec, _fetch_nonce: T) -> Result<(), Error> where T: Fn(&Address) -> U256 { unimplemented!(); } + fn import_transactions(&self, _transactions: Vec, _fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { unimplemented!(); } /// Returns hashes of transactions currently in pending fn pending_transactions_hashes(&self) -> Vec { unimplemented!(); } @@ -50,4 +51,4 @@ impl MinerService for TestMinerService { /// 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!(); } -} \ No newline at end of file +} diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 8f0194289..4f8001ce7 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -38,7 +38,7 @@ use range_collection::{RangeCollection, ToUsize, FromUsize}; use ethcore::error::*; use ethcore::transaction::SignedTransaction; use ethcore::block::Block; -use ethminer::{Miner, MinerService}; +use ethminer::{Miner, MinerService, AccountDetails}; use io::SyncIo; use time; use super::SyncConfig; @@ -940,8 +940,11 @@ impl ChainSync { transactions.push(tx); } let chain = io.chain(); - let fetch_nonce = |a: &Address| chain.nonce(a); - let _ = self.miner.import_transactions(transactions, fetch_nonce); + let fetch_account = |a: &Address| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a) + }; + let _ = self.miner.import_transactions(transactions, fetch_account); Ok(()) } From d54c95da9d3cae135a43316c9a62f9bf9f81beb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 10:48:31 +0100 Subject: [PATCH 13/34] Removing unused import --- rpc/src/v1/tests/helpers/miner_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index b2aebb29c..7a1ed3809 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use util::{Address, H256, U256, Bytes}; +use util::{Address, H256, Bytes}; use util::standard::*; use ethcore::error::Error; use ethcore::client::BlockChainClient; From 8741a85443ff6a131cab4bf45af3c941f370e483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 16:47:36 +0100 Subject: [PATCH 14/34] Fixing build --- miner/src/transaction_queue.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index ca4a269ae..abd862a02 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -777,8 +777,10 @@ mod test { #[test] fn should_correctly_update_futures_when_removing() { // given - let prev_nonce = |a: &Address| default_nonce(a) - U256::one(); - let next2_nonce = |a: &Address| default_nonce(a) + U256::from(2); + let prev_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance: + !U256::zero() }; + let next2_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::from(2), balance: + !U256::zero() }; let mut txq = TransactionQueue::new(); @@ -923,7 +925,7 @@ mod test { let mut txq = TransactionQueue::new(); let tx = new_tx(); let last_nonce = tx.nonce + U256::one(); - let fetch_last_nonce = |_a: &Address| last_nonce; + let fetch_last_nonce = |_a: &Address| AccountDetails{ nonce: last_nonce, balance: !U256::zero() }; // when txq.add(tx, &fetch_last_nonce).unwrap(); @@ -937,7 +939,8 @@ mod test { #[test] fn should_not_insert_same_transaction_twice() { // given - let nonce = |a: &Address| default_nonce(a) + U256::one(); + let nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce + U256::one(), + balance: !U256::zero() }; let mut txq = TransactionQueue::new(); let (_tx1, tx2) = new_txs(U256::from(1)); txq.add(tx2.clone(), &default_nonce).unwrap(); @@ -977,7 +980,7 @@ mod test { #[test] fn should_not_move_to_future_if_state_nonce_is_higher() { // given - let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: + let next_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce + U256::one(), balance: !U256::zero() }; let mut txq = TransactionQueue::new(); let (tx, tx2) = new_txs(U256::from(1)); From 0925968840d3d8ed027587c10ae0fdca253c250f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 17:17:49 +0100 Subject: [PATCH 15/34] Adding test --- miner/src/transaction_queue.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index abd862a02..480d6550a 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -34,7 +34,7 @@ //! use util::crypto::KeyPair; //! use util::hash::Address; //! use util::numbers::{Uint, U256}; -//! use ethminer::TransactionQueue; +//! use ethminer::{TransactionQueue, AccountDetails}; //! use ethcore::transaction::*; //! use rustc_serialize::hex::FromHex; //! @@ -47,7 +47,10 @@ //! //! let st1 = t1.sign(&key.secret()); //! let st2 = t2.sign(&key.secret()); -//! let default_nonce = |_a: &Address| U256::from(10); +//! let default_nonce = |_a: &Address| AccountDetails { +//! nonce: U256::from(10), +//! balance: U256::from(10_000), +//! }; //! //! let mut txq = TransactionQueue::new(); //! txq.add(st2.clone(), &default_nonce); @@ -677,6 +680,25 @@ mod test { assert_eq!(stats.pending, 1); } + #[test] + fn should_drop_transactions_from_senders_without_balance() { + // given + let mut txq = TransactionQueue::new(); + let tx = new_tx(); + let account = |a: &Address| AccountDetails { + nonce: default_nonce(a).nonce, + balance: U256::one() + }; + + // when + txq.add(tx, &account).unwrap_err(); + + // then + let stats = txq.status(); + assert_eq!(stats.pending, 0); + assert_eq!(stats.future, 0); + } + #[test] fn should_not_import_transaction_below_min_gas_price_threshold() { // given From 81c36499ea8bd8bde112de40100793efabc08ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 10:20:35 +0100 Subject: [PATCH 16/34] Fixing sync test --- sync/src/chain.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 8d392dc1c..0af6008f2 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -1301,6 +1301,7 @@ mod tests { use ::SyncConfig; use util::*; use super::{PeerInfo, PeerAsking}; + use ethcore::views::BlockView; use ethcore::header::*; use ethcore::client::*; use ethminer::{Miner, MinerService}; @@ -1631,6 +1632,13 @@ mod tests { let good_blocks = vec![client.block_hash_delta_minus(2)]; let retracted_blocks = vec![client.block_hash_delta_minus(1)]; + // Add some balance to clients + for h in vec![good_blocks[0], retracted_blocks[0]] { + let block = client.block(BlockId::Hash(h)).unwrap(); + let view = BlockView::new(&block); + client.set_balance(view.transactions()[0].sender().unwrap(), U256::from(10_000)); + } + let mut queue = VecDeque::new(); let mut io = TestIo::new(&mut client, &mut queue, None); From 95dda4aa68ac87c0320a551df7b08c069848ab27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 10:23:09 +0100 Subject: [PATCH 17/34] Full transaction cost --- miner/src/transaction_queue.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 480d6550a..1842f5f35 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -343,14 +343,16 @@ impl TransactionQueue { })); } + let vtx = try!(VerifiedTransaction::new(tx)); let account = fetch_account(&vtx.sender()); - if account.balance < vtx.transaction.value { + let cost = vtx.transaction.value + vtx.transaction.gas_price * vtx.transaction.gas; + if account.balance < cost { trace!(target: "miner", "Dropping transaction without sufficient balance: {:?} ({} < {})", - vtx.hash(), account.balance, vtx.transaction.value); + vtx.hash(), account.balance, cost); return Err(Error::Transaction(TransactionError::InsufficientBalance { - cost: vtx.transaction.value, + cost: cost, balance: account.balance })); } From 884f2dd873d8a7703dcf959d4c87d39f3dfb6ea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:19:12 +0100 Subject: [PATCH 18/34] Returning number of transactions pending in block not queue --- miner/src/lib.rs | 2 ++ miner/src/miner.rs | 4 +++- rpc/src/v1/impls/eth.rs | 2 +- rpc/src/v1/tests/eth.rs | 14 ++++++++++++++ rpc/src/v1/tests/helpers/miner_service.rs | 10 ++++++++-- 5 files changed, 28 insertions(+), 4 deletions(-) diff --git a/miner/src/lib.rs b/miner/src/lib.rs index a431bd44e..9d2635bef 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -108,4 +108,6 @@ pub struct MinerStatus { pub transaction_queue_pending: usize, /// Number of transactions in queue with state `future` (not yet ready to be included in block) pub transaction_queue_future: usize, + /// Number of transactions included in currently mined block + pub block_pending: usize, } diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 6d5b3086e..d9fb0a271 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -22,7 +22,7 @@ use std::sync::atomic::AtomicBool; use util::{H256, U256, Address, Bytes, Uint}; use ethcore::views::{BlockView}; use ethcore::client::{BlockChainClient, BlockId}; -use ethcore::block::{ClosedBlock}; +use ethcore::block::{ClosedBlock, IsBlock}; use ethcore::error::{Error}; use ethcore::transaction::SignedTransaction; use super::{MinerService, MinerStatus, TransactionQueue}; @@ -104,9 +104,11 @@ impl MinerService for Miner { fn status(&self) -> MinerStatus { let status = self.transaction_queue.lock().unwrap().status(); + let block = self.sealing_block.lock().unwrap(); MinerStatus { transaction_queue_pending: status.pending, transaction_queue_future: status.future, + block_pending: block.as_ref().map_or(0, |b| b.transactions().len()), } } diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index fda391304..0f9582955 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -236,7 +236,7 @@ impl Eth for EthClient fn block_transaction_count_by_number(&self, params: Params) -> Result { from_params::<(BlockNumber,)>(params) .and_then(|(block_number,)| match block_number { - BlockNumber::Pending => to_value(&U256::from(take_weak!(self.miner).status().transaction_queue_pending)), + BlockNumber::Pending => to_value(&U256::from(take_weak!(self.miner).status().block_pending)), _ => to_value(&take_weak!(self.client).block(block_number.into()) .map_or_else(U256::zero, |bytes| U256::from(BlockView::new(&bytes).transactions_count()))) }) diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 6bc929709..2fe39ed63 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -242,6 +242,20 @@ fn rpc_eth_transaction_count_by_number() { assert_eq!(EthTester::default().io.handle_request(request), Some(response.to_owned())); } +#[test] +fn rpc_eth_transaction_count_by_number_pending() { + let request = r#"{ + "jsonrpc": "2.0", + "method": "eth_getBlockTransactionCountByNumber", + "params": ["pending"], + "id": 1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":"0x01","id":1}"#; + + assert_eq!(EthTester::default().io.handle_request(request), Some(response.to_owned())); +} + + #[test] fn rpc_eth_uncle_count_by_block_hash() { let request = r#"{ diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 0cddf2a1e..3afaa3674 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -27,7 +27,13 @@ pub struct TestMinerService; impl MinerService for TestMinerService { /// Returns miner's status. - fn status(&self) -> MinerStatus { unimplemented!(); } + fn status(&self) -> MinerStatus { + MinerStatus { + transaction_queue_pending: 0, + transaction_queue_future: 0, + block_pending: 1 + } + } /// Imports transactions to transaction queue. fn import_transactions(&self, _transactions: Vec, _fetch_nonce: T) -> Result<(), Error> where T: Fn(&Address) -> U256 { unimplemented!(); } @@ -50,4 +56,4 @@ impl MinerService for TestMinerService { /// 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!(); } -} \ No newline at end of file +} From 4050462ad4e7b24846965c384dad8e78bb2ad3e6 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 17 Mar 2016 11:23:30 +0100 Subject: [PATCH 19/34] Update sync_provider.rs --- rpc/src/v1/tests/helpers/sync_provider.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/src/v1/tests/helpers/sync_provider.rs b/rpc/src/v1/tests/helpers/sync_provider.rs index bfbfad44b..5b4b5b099 100644 --- a/rpc/src/v1/tests/helpers/sync_provider.rs +++ b/rpc/src/v1/tests/helpers/sync_provider.rs @@ -40,7 +40,7 @@ impl TestSyncProvider { num_peers: config.num_peers, num_active_peers: 0, mem_used: 0, - }) + }), } } } From 0e7778a7b79be72d25c57fce129098abfa68af44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:27:38 +0100 Subject: [PATCH 20/34] Increasing balance in tests --- sync/src/chain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 0af6008f2..06fbcd5d1 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -403,7 +403,7 @@ impl ChainSync { self.remove_downloaded_blocks(number + 1); } if self.have_common_block && number < self.current_base_block() + 1 { - // unkown header + // unkown header debug!(target: "sync", "Old block header {:?} ({}) is unknown, restarting sync", hash, number); self.restart(io); return Ok(()); @@ -1636,7 +1636,7 @@ mod tests { for h in vec![good_blocks[0], retracted_blocks[0]] { let block = client.block(BlockId::Hash(h)).unwrap(); let view = BlockView::new(&block); - client.set_balance(view.transactions()[0].sender().unwrap(), U256::from(10_000)); + client.set_balance(view.transactions()[0].sender().unwrap(), U256::from(1_000_000_000)); } let mut queue = VecDeque::new(); From b1557b547b04abbef6ce8d116c80814c6d710540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:47:41 +0100 Subject: [PATCH 21/34] Reverting check if block queue is empty --- ethcore/src/client/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6857416d3..caa92db97 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -317,7 +317,7 @@ impl Client where V: Verifier { } { - if !imported_blocks.is_empty() { + if !imported_blocks.is_empty() && self.block_queue.queue_info().is_empty() { let (enacted, retracted) = self.calculate_enacted_retracted(import_results); io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { imported: imported_blocks, From bc04e0c71304237a2f5aba9c59cb14923169e302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:49:56 +0100 Subject: [PATCH 22/34] Adding missing commas --- rpc/src/v1/impls/eth.rs | 2 +- sync/src/chain.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 04b25d5b2..b566dd848 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -372,7 +372,7 @@ impl Eth for EthClient let import = miner.import_transactions(vec![signed_transaction], |a: &Address| AccountDetails { nonce: client.nonce(a), - balance: client.balance(a) + balance: client.balance(a), }); match import { Ok(_) => to_value(&hash), diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 06fbcd5d1..492481722 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -948,7 +948,7 @@ impl ChainSync { let chain = io.chain(); let fetch_account = |a: &Address| AccountDetails { nonce: chain.nonce(a), - balance: chain.balance(a) + balance: chain.balance(a), }; let _ = self.miner.import_transactions(transactions, fetch_account); Ok(()) From 03ca9d2c0662c706d1b37ce0d1e71fa03ccfcc12 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Thu, 17 Mar 2016 12:17:53 +0100 Subject: [PATCH 23/34] adding message from client to sync and disabling sending transactions to the queue while syncing --- ethcore/src/client/client.rs | 6 ++++ ethcore/src/service.rs | 2 ++ rpc/src/v1/impls/eth.rs | 2 +- sync/src/chain.rs | 58 ++++++++++++++++++++++++++++++++++-- sync/src/lib.rs | 3 ++ 5 files changed, 67 insertions(+), 4 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index caa92db97..d8c48e696 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -334,6 +334,12 @@ impl Client where V: Verifier { } } + { + if self.queue_info().is_empty() { + io.send(NetworkIoMessage::User(SyncMessage::BlockQueueEmpty)).expect("error sending message to sync module"); + } + } + imported } diff --git a/ethcore/src/service.rs b/ethcore/src/service.rs index bcfe7724f..1c2385934 100644 --- a/ethcore/src/service.rs +++ b/ethcore/src/service.rs @@ -40,6 +40,8 @@ pub enum SyncMessage { NewChainHead, /// A block is ready BlockVerified, + /// blocks queue is empty + BlockQueueEmpty, } /// IO Message type used for Network service diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 520b37045..b0e31ba97 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -151,7 +151,7 @@ impl Eth for EthClient Params::None => { let status = take_weak!(self.sync).status(); let res = match status.state { - SyncState::NotSynced | SyncState::Idle => SyncStatus::None, + SyncState::NotSynced | SyncState::Idle | SyncState::FullySynced => SyncStatus::None, SyncState::Waiting | SyncState::Blocks | SyncState::NewBlocks => SyncStatus::Info(SyncInfo { starting_block: U256::from(status.start_block_number), current_block: U256::from(take_weak!(self.client).chain_info().best_block_number), diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 363597d40..5af299b34 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -116,6 +116,8 @@ pub enum SyncState { Blocks, /// Downloading blocks learned from NewHashes packet NewBlocks, + /// Once has an event that client finished processing new blocks + FullySynced, } /// Syncing status and statistics @@ -930,6 +932,11 @@ impl ChainSync { } /// 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 + if self.state != SyncState::FullySynced { + return Ok(()); + } + let item_count = r.item_count(); trace!(target: "sync", "{} -> Transactions ({} entries)", peer_id, item_count); @@ -1272,15 +1279,31 @@ impl ChainSync { /// called when block is imported to chain, updates transactions queue and propagates the blocks pub fn chain_new_blocks(&mut self, io: &mut SyncIo, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256]) { - // Notify miner - self.miner.chain_new_blocks(io.chain(), imported, invalid, enacted, retracted); + if self.state == SyncState::FullySynced { + // Notify miner + self.miner.chain_new_blocks(io.chain(), imported, invalid, enacted, retracted); + } // Propagate latests blocks self.propagate_latest_blocks(io); // TODO [todr] propagate transactions? } pub fn chain_new_head(&mut self, io: &mut SyncIo) { - self.miner.prepare_sealing(io.chain()); + if self.state == SyncState::FullySynced { + self.miner.prepare_sealing(io.chain()); + } + } + + // called once has nothing to download and client reports that all that downloaded is imported + pub fn set_fully_synced(&mut self) { + self.state = SyncState::FullySynced; + } + + // handles event from client about empty blow_queue + pub fn client_block_queue_empty(&mut self) { + if self.state == SyncState::Idle { + self.set_fully_synced(); + } } } @@ -1616,6 +1639,7 @@ mod tests { client.add_blocks(1, EachBlockWith::UncleAndTransaction); client.add_blocks(1, EachBlockWith::Transaction); let mut sync = dummy_sync_with_peer(client.block_hash_delta_minus(5)); + sync.state = SyncState::FullySynced; let good_blocks = vec![client.block_hash_delta_minus(2)]; let retracted_blocks = vec![client.block_hash_delta_minus(1)]; @@ -1635,6 +1659,34 @@ mod tests { assert_eq!(status.transaction_queue_future, 0); } + #[test] + fn should_not_add_transactions_to_queue_if_not_synced() { + // given + let mut client = TestBlockChainClient::new(); + client.add_blocks(98, EachBlockWith::Uncle); + client.add_blocks(1, EachBlockWith::UncleAndTransaction); + client.add_blocks(1, EachBlockWith::Transaction); + let mut sync = dummy_sync_with_peer(client.block_hash_delta_minus(5)); + sync.state = SyncState::Idle; + + let good_blocks = vec![client.block_hash_delta_minus(2)]; + let retracted_blocks = vec![client.block_hash_delta_minus(1)]; + + let mut queue = VecDeque::new(); + let mut io = TestIo::new(&mut client, &mut queue, None); + + // when + sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks); + assert_eq!(sync.miner.status().transaction_queue_future, 0); + assert_eq!(sync.miner.status().transaction_queue_pending, 0); + sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); + + // then + let status = sync.miner.status(); + assert_eq!(status.transaction_queue_pending, 0); + assert_eq!(status.transaction_queue_future, 0); + } + #[test] fn returns_requested_block_headers() { let mut client = TestBlockChainClient::new(); diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 1c87da2de..d78ac3b89 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -173,6 +173,9 @@ impl NetworkProtocolHandler for EthSync { SyncMessage::NewChainHead => { let mut sync_io = NetSyncIo::new(io, self.chain.deref()); self.sync.write().unwrap().chain_new_head(&mut sync_io); + }, + SyncMessage::BlockQueueEmpty => { + self.sync.write().unwrap().client_block_queue_empty(); } _ => {/* Ignore other messages */}, } From 7247f9e27f9eb59f2d11a666de3e9f1afed90a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 12:23:15 +0100 Subject: [PATCH 24/34] Fixing doctest --- miner/src/transaction_queue.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 1842f5f35..a1fc20b0f 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -49,12 +49,12 @@ //! let st2 = t2.sign(&key.secret()); //! let default_nonce = |_a: &Address| AccountDetails { //! nonce: U256::from(10), -//! balance: U256::from(10_000), +//! balance: U256::from(1_000_000), //! }; //! //! let mut txq = TransactionQueue::new(); -//! txq.add(st2.clone(), &default_nonce); -//! txq.add(st1.clone(), &default_nonce); +//! txq.add(st2.clone(), &default_nonce).unwrap(); +//! txq.add(st1.clone(), &default_nonce).unwrap(); //! //! // Check status //! assert_eq!(txq.status().pending, 2); From e1c3ab18462096672de67fd19c5f79106abdd9f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:19:12 +0100 Subject: [PATCH 25/34] Renaming status fields to something more descriptive. --- miner/src/lib.rs | 8 ++++---- miner/src/miner.rs | 6 +++--- rpc/src/v1/impls/eth.rs | 4 +++- rpc/src/v1/tests/helpers/miner_service.rs | 6 +++--- sync/src/chain.rs | 10 +++++----- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/miner/src/lib.rs b/miner/src/lib.rs index 9d2635bef..5821c9815 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -42,7 +42,7 @@ //! //! let miner: Miner = Miner::default(); //! // get status -//! assert_eq!(miner.status().transaction_queue_pending, 0); +//! assert_eq!(miner.status().transactions_in_pending_queue, 0); //! //! // Check block for sealing //! miner.prepare_sealing(client.deref()); @@ -105,9 +105,9 @@ pub trait MinerService : Send + Sync { /// Mining status pub struct MinerStatus { /// Number of transactions in queue with state `pending` (ready to be included in block) - pub transaction_queue_pending: usize, + pub transactions_in_pending_queue: usize, /// Number of transactions in queue with state `future` (not yet ready to be included in block) - pub transaction_queue_future: usize, + pub transactions_in_future_queue: usize, /// Number of transactions included in currently mined block - pub block_pending: usize, + pub transactions_in_pending_block: usize, } diff --git a/miner/src/miner.rs b/miner/src/miner.rs index d9fb0a271..ccf45f61a 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -106,9 +106,9 @@ impl MinerService for Miner { let status = self.transaction_queue.lock().unwrap().status(); let block = self.sealing_block.lock().unwrap(); MinerStatus { - transaction_queue_pending: status.pending, - transaction_queue_future: status.future, - block_pending: block.as_ref().map_or(0, |b| b.transactions().len()), + transactions_in_pending_queue: status.pending, + transactions_in_future_queue: status.future, + transactions_in_pending_block: block.as_ref().map_or(0, |b| b.transactions().len()), } } diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 0f9582955..c8306c2c9 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -236,7 +236,9 @@ impl Eth for EthClient fn block_transaction_count_by_number(&self, params: Params) -> Result { from_params::<(BlockNumber,)>(params) .and_then(|(block_number,)| match block_number { - BlockNumber::Pending => to_value(&U256::from(take_weak!(self.miner).status().block_pending)), + BlockNumber::Pending =>to_value( + &U256::from(take_weak!(self.miner).status().transactions_in_pending_block) + ), _ => to_value(&take_weak!(self.client).block(block_number.into()) .map_or_else(U256::zero, |bytes| U256::from(BlockView::new(&bytes).transactions_count()))) }) diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 3afaa3674..776a04f75 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -29,9 +29,9 @@ impl MinerService for TestMinerService { /// Returns miner's status. fn status(&self) -> MinerStatus { MinerStatus { - transaction_queue_pending: 0, - transaction_queue_future: 0, - block_pending: 1 + transactions_in_pending_queue: 0, + transactions_in_future_queue: 0, + transactions_in_pending_block: 1 } } diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 4f6c2100a..206dd5dd2 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -403,7 +403,7 @@ impl ChainSync { self.remove_downloaded_blocks(number + 1); } if self.have_common_block && number < self.current_base_block() + 1 { - // unkown header + // unkown header debug!(target: "sync", "Old block header {:?} ({}) is unknown, restarting sync", hash, number); self.restart(io); return Ok(()); @@ -1633,14 +1633,14 @@ mod tests { // when sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks); - assert_eq!(sync.miner.status().transaction_queue_future, 0); - assert_eq!(sync.miner.status().transaction_queue_pending, 1); + assert_eq!(sync.miner.status().transactions_in_future_queue, 0); + assert_eq!(sync.miner.status().transactions_in_pending_queue, 1); sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); // then let status = sync.miner.status(); - assert_eq!(status.transaction_queue_pending, 1); - assert_eq!(status.transaction_queue_future, 0); + assert_eq!(status.transactions_in_pending_queue, 1); + assert_eq!(status.transactions_in_future_queue, 0); } #[test] From 83f5cc6aa69bdc262e76235460c509d3fd3ef488 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Thu, 17 Mar 2016 13:14:06 +0100 Subject: [PATCH 26/34] adding helper can_sync --- sync/src/chain.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 8697c94a7..f7a03a7bd 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -405,7 +405,7 @@ impl ChainSync { self.remove_downloaded_blocks(number + 1); } if self.have_common_block && number < self.current_base_block() + 1 { - // unkown header + // unkown header debug!(target: "sync", "Old block header {:?} ({}) is unknown, restarting sync", hash, number); self.restart(io); return Ok(()); @@ -627,6 +627,13 @@ impl ChainSync { self.state = SyncState::Waiting; } + fn can_sync(&self) -> bool { + match self.state { + SyncState::Idle | SyncState::NotSynced | SyncState::FullySynced => true, + _ => false + } + } + /// Find something to do for a peer. Called for a new peer or when a peer is done with it's task. fn sync_peer(&mut self, io: &mut SyncIo, peer_id: PeerId, force: bool) { let (peer_latest, peer_difficulty) = { @@ -646,7 +653,7 @@ impl ChainSync { if force || peer_difficulty > syncing_difficulty { // start sync self.syncing_difficulty = peer_difficulty; - if self.state == SyncState::Idle || self.state == SyncState::NotSynced { + if self.can_sync() { self.state = SyncState::Blocks; } trace!(target: "sync", "Starting sync with better chain"); From caedb64ade3b202e47c5c7220cd8639370db6f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 13:18:26 +0100 Subject: [PATCH 27/34] Adding missing space --- rpc/src/v1/impls/eth.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index c8306c2c9..6c434a66b 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -236,7 +236,7 @@ impl Eth for EthClient fn block_transaction_count_by_number(&self, params: Params) -> Result { from_params::<(BlockNumber,)>(params) .and_then(|(block_number,)| match block_number { - BlockNumber::Pending =>to_value( + BlockNumber::Pending => to_value( &U256::from(take_weak!(self.miner).status().transactions_in_pending_block) ), _ => to_value(&take_weak!(self.client).block(block_number.into()) From dec69651fddee226bce347893ad4724183ebacbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 13:41:08 +0100 Subject: [PATCH 28/34] Attempting to add all transactions to mined block --- miner/src/miner.rs | 5 +---- miner/src/transaction_queue.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 832dc5d02..7bbde6294 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -125,10 +125,7 @@ impl MinerService for Miner { } fn prepare_sealing(&self, chain: &BlockChainClient) { - let no_of_transactions = 128; - // TODO: should select transactions orm queue according to gas limit of block. - let transactions = self.transaction_queue.lock().unwrap().top_transactions(no_of_transactions); - + let transactions = self.transaction_queue.lock().unwrap().top_transactions(); let b = chain.prepare_sealing( self.author(), self.gas_floor_target(), diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index a1fc20b0f..71d845e38 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -59,7 +59,7 @@ //! // Check status //! assert_eq!(txq.status().pending, 2); //! // Check top transactions -//! let top = txq.top_transactions(3); +//! let top = txq.top_transactions(); //! assert_eq!(top.len(), 2); //! assert_eq!(top[0], st1); //! assert_eq!(top[1], st2); @@ -69,7 +69,7 @@ //! txq.remove(&st1.hash(), &default_nonce); //! assert_eq!(txq.status().pending, 0); //! assert_eq!(txq.status().future, 1); -//! assert_eq!(txq.top_transactions(3).len(), 0); +//! assert_eq!(txq.top_transactions().len(), 0); //! } //! ``` //! @@ -459,10 +459,9 @@ impl TransactionQueue { // Will be used when mining merged #[allow(dead_code)] /// Returns top transactions from the queue ordered by priority. - pub fn top_transactions(&self, size: usize) -> Vec { + pub fn top_transactions(&self) -> Vec { self.current.by_priority .iter() - .take(size) .map(|t| self.by_hash.get(&t.hash).expect("Transaction Queue Inconsistency")) .map(|t| t.transaction.clone()) .collect() @@ -754,7 +753,7 @@ mod test { txq.add(tx2.clone(), &default_nonce).unwrap(); // then - let top = txq.top_transactions(5); + let top = txq.top_transactions(); assert_eq!(top[0], tx); assert_eq!(top[1], tx2); assert_eq!(top.len(), 2); @@ -793,7 +792,7 @@ mod test { let stats = txq.status(); assert_eq!(stats.pending, 1); assert_eq!(stats.future, 1); - let top = txq.top_transactions(5); + let top = txq.top_transactions(); assert_eq!(top.len(), 1); assert_eq!(top[0], tx); } @@ -920,7 +919,7 @@ mod test { txq.add(tx2.clone(), &default_nonce).unwrap(); // then - let t = txq.top_transactions(2); + let t = txq.top_transactions(); assert_eq!(txq.status().pending, 1); assert_eq!(t.len(), 1); assert_eq!(t[0], tx); @@ -1044,7 +1043,7 @@ mod test { let stats = txq.status(); assert_eq!(stats.pending, 1); assert_eq!(stats.future, 0); - assert_eq!(txq.top_transactions(1)[0].gas_price, U256::from(200)); + assert_eq!(txq.top_transactions()[0].gas_price, U256::from(200)); } #[test] @@ -1074,7 +1073,7 @@ mod test { let stats = txq.status(); assert_eq!(stats.future, 0); assert_eq!(stats.pending, 2); - assert_eq!(txq.top_transactions(2)[1].gas_price, U256::from(200)); + assert_eq!(txq.top_transactions()[1].gas_price, U256::from(200)); } #[test] From a285fbab6d7f807626b44a49271159cfc1d29872 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Thu, 17 Mar 2016 14:11:32 +0100 Subject: [PATCH 29/34] overhaul to flag --- ethcore/src/client/client.rs | 7 +----- ethcore/src/service.rs | 4 ++-- rpc/src/v1/impls/eth.rs | 2 +- sync/src/chain.rs | 41 ++++++++++++------------------------ sync/src/lib.rs | 7 ++---- sync/src/tests/helpers.rs | 2 +- 6 files changed, 21 insertions(+), 42 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index d8c48e696..6ee1b88d5 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -324,6 +324,7 @@ impl Client where V: Verifier { invalid: invalid_blocks, enacted: enacted, retracted: retracted, + is_last: self.queue_info().is_empty() })).unwrap(); } } @@ -334,12 +335,6 @@ impl Client where V: Verifier { } } - { - if self.queue_info().is_empty() { - io.send(NetworkIoMessage::User(SyncMessage::BlockQueueEmpty)).expect("error sending message to sync module"); - } - } - imported } diff --git a/ethcore/src/service.rs b/ethcore/src/service.rs index 1c2385934..a46581e3c 100644 --- a/ethcore/src/service.rs +++ b/ethcore/src/service.rs @@ -35,13 +35,13 @@ pub enum SyncMessage { retracted: Vec, /// Hashes of blocks that are now included in cannonical chain enacted: Vec, + /// Set when blockqueue is empty + is_last: bool, }, /// Best Block Hash in chain has been changed NewChainHead, /// A block is ready BlockVerified, - /// blocks queue is empty - BlockQueueEmpty, } /// IO Message type used for Network service diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index b0e31ba97..520b37045 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -151,7 +151,7 @@ impl Eth for EthClient Params::None => { let status = take_weak!(self.sync).status(); let res = match status.state { - SyncState::NotSynced | SyncState::Idle | SyncState::FullySynced => SyncStatus::None, + SyncState::NotSynced | SyncState::Idle => SyncStatus::None, SyncState::Waiting | SyncState::Blocks | SyncState::NewBlocks => SyncStatus::Info(SyncInfo { starting_block: U256::from(status.start_block_number), current_block: U256::from(take_weak!(self.client).chain_info().best_block_number), diff --git a/sync/src/chain.rs b/sync/src/chain.rs index f7a03a7bd..def9ad7f4 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -116,8 +116,6 @@ pub enum SyncState { Blocks, /// Downloading blocks learned from NewHashes packet NewBlocks, - /// Once has an event that client finished processing new blocks - FullySynced, } /// Syncing status and statistics @@ -217,6 +215,8 @@ pub struct ChainSync { network_id: U256, /// Miner miner: Arc, + /// Fully-synced flag + is_fully_synced: bool, } type RlpResponseResult = Result, PacketDecodeError>; @@ -243,6 +243,7 @@ impl ChainSync { max_download_ahead_blocks: max(MAX_HEADERS_TO_REQUEST, config.max_download_ahead_blocks), network_id: config.network_id, miner: miner, + is_fully_synced: true, } } @@ -629,7 +630,7 @@ impl ChainSync { fn can_sync(&self) -> bool { match self.state { - SyncState::Idle | SyncState::NotSynced | SyncState::FullySynced => true, + SyncState::Idle | SyncState::NotSynced => true, _ => false } } @@ -947,7 +948,7 @@ impl ChainSync { /// 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 - if self.state != SyncState::FullySynced { + if !self.is_fully_synced { return Ok(()); } @@ -1292,8 +1293,10 @@ impl ChainSync { } /// called when block is imported to chain, updates transactions queue and propagates the blocks - pub fn chain_new_blocks(&mut self, io: &mut SyncIo, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256]) { - if self.state == SyncState::FullySynced { + pub fn chain_new_blocks(&mut self, io: &mut SyncIo, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256], is_last: bool) { + // Set the state in which it can accept transactions from the net + self.is_fully_synced = is_last; + if self.is_fully_synced { // Notify miner self.miner.chain_new_blocks(io.chain(), imported, invalid, enacted, retracted); } @@ -1303,21 +1306,7 @@ impl ChainSync { } pub fn chain_new_head(&mut self, io: &mut SyncIo) { - if self.state == SyncState::FullySynced { - self.miner.prepare_sealing(io.chain()); - } - } - - // called once has nothing to download and client reports that all that downloaded is imported - pub fn set_fully_synced(&mut self) { - self.state = SyncState::FullySynced; - } - - // handles event from client about empty blow_queue - pub fn client_block_queue_empty(&mut self) { - if self.state == SyncState::Idle { - self.set_fully_synced(); - } + self.miner.prepare_sealing(io.chain()); } } @@ -1654,7 +1643,6 @@ mod tests { client.add_blocks(1, EachBlockWith::UncleAndTransaction); client.add_blocks(1, EachBlockWith::Transaction); let mut sync = dummy_sync_with_peer(client.block_hash_delta_minus(5)); - sync.state = SyncState::FullySynced; let good_blocks = vec![client.block_hash_delta_minus(2)]; let retracted_blocks = vec![client.block_hash_delta_minus(1)]; @@ -1663,10 +1651,10 @@ mod tests { let mut io = TestIo::new(&mut client, &mut queue, None); // when - sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks); + sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks, true); assert_eq!(sync.miner.status().transaction_queue_future, 0); assert_eq!(sync.miner.status().transaction_queue_pending, 1); - sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); + sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks, true); // then let status = sync.miner.status(); @@ -1682,7 +1670,6 @@ mod tests { client.add_blocks(1, EachBlockWith::UncleAndTransaction); client.add_blocks(1, EachBlockWith::Transaction); let mut sync = dummy_sync_with_peer(client.block_hash_delta_minus(5)); - sync.state = SyncState::Idle; let good_blocks = vec![client.block_hash_delta_minus(2)]; let retracted_blocks = vec![client.block_hash_delta_minus(1)]; @@ -1691,10 +1678,10 @@ mod tests { let mut io = TestIo::new(&mut client, &mut queue, None); // when - sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks); + sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks, false); assert_eq!(sync.miner.status().transaction_queue_future, 0); assert_eq!(sync.miner.status().transaction_queue_pending, 0); - sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); + sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks, false); // then let status = sync.miner.status(); diff --git a/sync/src/lib.rs b/sync/src/lib.rs index d78ac3b89..8ab6a23e9 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -166,17 +166,14 @@ impl NetworkProtocolHandler for EthSync { fn message(&self, io: &NetworkContext, message: &SyncMessage) { match *message { - SyncMessage::NewChainBlocks { ref imported, ref invalid, ref enacted, ref retracted } => { + SyncMessage::NewChainBlocks { ref imported, ref invalid, ref enacted, ref retracted, is_last } => { let mut sync_io = NetSyncIo::new(io, self.chain.deref()); - self.sync.write().unwrap().chain_new_blocks(&mut sync_io, imported, invalid, enacted, retracted); + self.sync.write().unwrap().chain_new_blocks(&mut sync_io, imported, invalid, enacted, retracted, is_last); }, SyncMessage::NewChainHead => { let mut sync_io = NetSyncIo::new(io, self.chain.deref()); self.sync.write().unwrap().chain_new_head(&mut sync_io); }, - SyncMessage::BlockQueueEmpty => { - self.sync.write().unwrap().client_block_queue_empty(); - } _ => {/* Ignore other messages */}, } } diff --git a/sync/src/tests/helpers.rs b/sync/src/tests/helpers.rs index b3e62ccc6..42ef728b9 100644 --- a/sync/src/tests/helpers.rs +++ b/sync/src/tests/helpers.rs @@ -168,6 +168,6 @@ impl TestNet { pub fn trigger_chain_new_blocks(&mut self, peer_id: usize) { let mut peer = self.peer_mut(peer_id); - peer.sync.chain_new_blocks(&mut TestIo::new(&mut peer.chain, &mut peer.queue, None), &[], &[], &[], &[]); + peer.sync.chain_new_blocks(&mut TestIo::new(&mut peer.chain, &mut peer.queue, None), &[], &[], &[], &[], true); } } From 0f96ce9bd250d8718bd7c97657172dbae5bef96e Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Thu, 17 Mar 2016 14:56:19 +0100 Subject: [PATCH 30/34] no flag also --- ethcore/src/client/client.rs | 1 - ethcore/src/service.rs | 2 -- sync/src/chain.rs | 19 +++++++------------ sync/src/io.rs | 4 ++++ sync/src/lib.rs | 4 ++-- sync/src/tests/helpers.rs | 2 +- 6 files changed, 14 insertions(+), 18 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6ee1b88d5..caa92db97 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -324,7 +324,6 @@ impl Client where V: Verifier { invalid: invalid_blocks, enacted: enacted, retracted: retracted, - is_last: self.queue_info().is_empty() })).unwrap(); } } diff --git a/ethcore/src/service.rs b/ethcore/src/service.rs index a46581e3c..bcfe7724f 100644 --- a/ethcore/src/service.rs +++ b/ethcore/src/service.rs @@ -35,8 +35,6 @@ pub enum SyncMessage { retracted: Vec, /// Hashes of blocks that are now included in cannonical chain enacted: Vec, - /// Set when blockqueue is empty - is_last: bool, }, /// Best Block Hash in chain has been changed NewChainHead, diff --git a/sync/src/chain.rs b/sync/src/chain.rs index c94e89445..b68b2b250 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -215,8 +215,6 @@ pub struct ChainSync { network_id: U256, /// Miner miner: Arc, - /// Fully-synced flag - is_fully_synced: bool, } type RlpResponseResult = Result, PacketDecodeError>; @@ -243,7 +241,6 @@ impl ChainSync { max_download_ahead_blocks: max(MAX_HEADERS_TO_REQUEST, config.max_download_ahead_blocks), network_id: config.network_id, miner: miner, - is_fully_synced: true, } } @@ -948,7 +945,7 @@ impl ChainSync { /// 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 - if !self.is_fully_synced { + if !io.is_chain_queue_empty() { return Ok(()); } @@ -1296,10 +1293,8 @@ impl ChainSync { } /// called when block is imported to chain, updates transactions queue and propagates the blocks - pub fn chain_new_blocks(&mut self, io: &mut SyncIo, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256], is_last: bool) { - // Set the state in which it can accept transactions from the net - self.is_fully_synced = is_last; - if self.is_fully_synced { + pub fn chain_new_blocks(&mut self, io: &mut SyncIo, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256]) { + if io.is_chain_queue_empty() { // Notify miner self.miner.chain_new_blocks(io.chain(), imported, invalid, enacted, retracted); } @@ -1662,10 +1657,10 @@ mod tests { let mut io = TestIo::new(&mut client, &mut queue, None); // when - sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks, true); + sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks); assert_eq!(sync.miner.status().transactions_in_future_queue, 0); assert_eq!(sync.miner.status().transactions_in_pending_queue, 1); - sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks, true); + sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); // then @@ -1690,10 +1685,10 @@ mod tests { let mut io = TestIo::new(&mut client, &mut queue, None); // when - sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks, false); + sync.chain_new_blocks(&mut io, &[], &[], &[], &good_blocks); assert_eq!(sync.miner.status().transactions_in_future_queue, 0); assert_eq!(sync.miner.status().transactions_in_pending_queue, 0); - sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks, false); + sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); // then let status = sync.miner.status(); diff --git a/sync/src/io.rs b/sync/src/io.rs index 00ee49be4..84697a021 100644 --- a/sync/src/io.rs +++ b/sync/src/io.rs @@ -37,6 +37,10 @@ pub trait SyncIo { fn peer_info(&self, peer_id: PeerId) -> String { peer_id.to_string() } + /// Returns if the chain block queue empty + fn is_chain_queue_empty(&self) -> bool { + self.chain().queue_info().is_empty() + } } /// Wraps `NetworkContext` and the blockchain client diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 8ab6a23e9..a4f6eff38 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -166,9 +166,9 @@ impl NetworkProtocolHandler for EthSync { fn message(&self, io: &NetworkContext, message: &SyncMessage) { match *message { - SyncMessage::NewChainBlocks { ref imported, ref invalid, ref enacted, ref retracted, is_last } => { + SyncMessage::NewChainBlocks { ref imported, ref invalid, ref enacted, ref retracted } => { let mut sync_io = NetSyncIo::new(io, self.chain.deref()); - self.sync.write().unwrap().chain_new_blocks(&mut sync_io, imported, invalid, enacted, retracted, is_last); + self.sync.write().unwrap().chain_new_blocks(&mut sync_io, imported, invalid, enacted, retracted); }, SyncMessage::NewChainHead => { let mut sync_io = NetSyncIo::new(io, self.chain.deref()); diff --git a/sync/src/tests/helpers.rs b/sync/src/tests/helpers.rs index 42ef728b9..b3e62ccc6 100644 --- a/sync/src/tests/helpers.rs +++ b/sync/src/tests/helpers.rs @@ -168,6 +168,6 @@ impl TestNet { pub fn trigger_chain_new_blocks(&mut self, peer_id: usize) { let mut peer = self.peer_mut(peer_id); - peer.sync.chain_new_blocks(&mut TestIo::new(&mut peer.chain, &mut peer.queue, None), &[], &[], &[], &[], true); + peer.sync.chain_new_blocks(&mut TestIo::new(&mut peer.chain, &mut peer.queue, None), &[], &[], &[], &[]); } } From 7929af67c8b875dd62d644f18c092b1dd44b421a Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Thu, 17 Mar 2016 15:02:18 +0100 Subject: [PATCH 31/34] propagation is out --- sync/src/chain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index b68b2b250..e1f0032a1 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -1297,9 +1297,9 @@ impl ChainSync { if io.is_chain_queue_empty() { // Notify miner self.miner.chain_new_blocks(io.chain(), imported, invalid, enacted, retracted); + // Propagate latests blocks + self.propagate_latest_blocks(io); } - // Propagate latests blocks - self.propagate_latest_blocks(io); // TODO [todr] propagate transactions? } From 85b08e6e7bd63ec3ef165d81484f027ebea697f4 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Thu, 17 Mar 2016 15:09:08 +0100 Subject: [PATCH 32/34] get rid of the function --- sync/src/chain.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index e1f0032a1..57eb101cb 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -625,13 +625,6 @@ impl ChainSync { self.state = SyncState::Waiting; } - fn can_sync(&self) -> bool { - match self.state { - SyncState::Idle | SyncState::NotSynced => true, - _ => false - } - } - /// Find something to do for a peer. Called for a new peer or when a peer is done with it's task. fn sync_peer(&mut self, io: &mut SyncIo, peer_id: PeerId, force: bool) { let (peer_latest, peer_difficulty) = { @@ -651,7 +644,7 @@ impl ChainSync { if force || peer_difficulty > syncing_difficulty { // start sync self.syncing_difficulty = peer_difficulty; - if self.can_sync() { + if self.state == SyncState::Idle || self.state == SyncState::NotSynced { self.state = SyncState::Blocks; } trace!(target: "sync", "Starting sync with better chain"); @@ -1662,7 +1655,6 @@ mod tests { assert_eq!(sync.miner.status().transactions_in_pending_queue, 1); sync.chain_new_blocks(&mut io, &good_blocks, &[], &[], &retracted_blocks); - // then let status = sync.miner.status(); assert_eq!(status.transactions_in_pending_queue, 1); From b1793fcb166ff8f8e49329a37fc621276b4795b3 Mon Sep 17 00:00:00 2001 From: arkpar Date: Thu, 17 Mar 2016 18:41:55 +0100 Subject: [PATCH 33/34] Prettier version wo git dir; Use rustc compile time version --- util/Cargo.toml | 2 +- util/build.rs | 14 ++++++++++++++ util/src/lib.rs | 2 -- util/src/misc.rs | 12 +++++++++--- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/util/Cargo.toml b/util/Cargo.toml index 641036191..ae99ef20b 100644 --- a/util/Cargo.toml +++ b/util/Cargo.toml @@ -29,7 +29,6 @@ sha3 = { path = "sha3" } serde = "0.7.0" clippy = { version = "0.0.50", optional = true } json-tests = { path = "json-tests" } -rustc_version = "0.1.0" igd = "0.4.2" ethcore-devtools = { path = "../devtools" } libc = "0.2.7" @@ -44,3 +43,4 @@ dev = ["clippy"] [build-dependencies] vergen = "*" +rustc_version = "0.1.0" diff --git a/util/build.rs b/util/build.rs index b0b64a380..f033e52e0 100644 --- a/util/build.rs +++ b/util/build.rs @@ -15,9 +15,23 @@ // along with Parity. If not, see . extern crate vergen; +extern crate rustc_version; use vergen::*; +use std::env; +use std::fs::File; +use std::io::Write; +use std::path::Path; fn main() { vergen(OutputFns::all()).unwrap(); + let out_dir = env::var("OUT_DIR").unwrap(); + let dest_path = Path::new(&out_dir).join("rustc_version.rs"); + let mut f = File::create(&dest_path).unwrap(); + f.write_all(format!(" + /// Returns compiler version. + pub fn rustc_version() -> &'static str {{ + \"{}\" + }} + ", rustc_version::version()).as_bytes()).unwrap(); } diff --git a/util/src/lib.rs b/util/src/lib.rs index cdc3a3f19..6abf6485d 100644 --- a/util/src/lib.rs +++ b/util/src/lib.rs @@ -109,9 +109,7 @@ extern crate log as rlog; extern crate igd; extern crate ethcore_devtools as devtools; extern crate libc; -extern crate rustc_version; extern crate target_info; -extern crate vergen; extern crate bigint; extern crate chrono; diff --git a/util/src/misc.rs b/util/src/misc.rs index 8dcd25988..76accf93b 100644 --- a/util/src/misc.rs +++ b/util/src/misc.rs @@ -20,9 +20,9 @@ use std::fs::File; use common::*; use rlp::{Stream, RlpStream}; use target_info::Target; -use rustc_version; include!(concat!(env!("OUT_DIR"), "/version.rs")); +include!(concat!(env!("OUT_DIR"), "/rustc_version.rs")); #[derive(Debug,Clone,PartialEq,Eq)] /// Diff type for specifying a change (or not). @@ -70,7 +70,13 @@ pub fn contents(name: &str) -> Result { /// Get the standard version string for this software. pub fn version() -> String { - format!("Parity/v{}-unstable-{}-{}/{}-{}-{}/rustc{}", env!("CARGO_PKG_VERSION"), short_sha(), commit_date().replace("-", ""), Target::arch(), Target::os(), Target::env(), rustc_version::version()) + let sha3 = short_sha(); + let sha3_dash = if sha3.is_empty() { "" } else { "-" }; + let commit_date = commit_date().replace("-", ""); + let date_dash = if commit_date.is_empty() { "" } else { "-" }; + let env = Target::env(); + let env_dash = if env.is_empty() { "" } else { "-" }; + format!("Parity/v{}-unstable{}{}{}{}/{}-{}{}{}/rustc{}", env!("CARGO_PKG_VERSION"), sha3_dash, sha3, date_dash, commit_date, Target::arch(), Target::os(), env_dash, env, rustc_version()) } /// Get the standard version data for this software. @@ -82,7 +88,7 @@ pub fn version_data() -> Bytes { u32::from_str(env!("CARGO_PKG_VERSION_PATCH")).unwrap(); s.append(&v); s.append(&"Parity"); - s.append(&format!("{}", rustc_version::version())); + s.append(&format!("{}", rustc_version())); s.append(&&Target::os()[0..2]); s.out() } From a61d1d8d51c9fd3c17c792f3bf4c268e7c8c44a6 Mon Sep 17 00:00:00 2001 From: arkpar Date: Thu, 17 Mar 2016 18:43:01 +0100 Subject: [PATCH 34/34] Indent --- util/build.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/util/build.rs b/util/build.rs index f033e52e0..1ec89f704 100644 --- a/util/build.rs +++ b/util/build.rs @@ -26,12 +26,12 @@ use std::path::Path; fn main() { vergen(OutputFns::all()).unwrap(); let out_dir = env::var("OUT_DIR").unwrap(); - let dest_path = Path::new(&out_dir).join("rustc_version.rs"); - let mut f = File::create(&dest_path).unwrap(); + let dest_path = Path::new(&out_dir).join("rustc_version.rs"); + let mut f = File::create(&dest_path).unwrap(); f.write_all(format!(" /// Returns compiler version. - pub fn rustc_version() -> &'static str {{ + pub fn rustc_version() -> &'static str {{ \"{}\" - }} - ", rustc_version::version()).as_bytes()).unwrap(); + }} + ", rustc_version::version()).as_bytes()).unwrap(); }