Merge branch 'master' into ao-abstract-kvdb-with-cache

* master:
  change dependencies of `keccak-hash`and `trie-standardmap` (#111)
  bring appveyor back (#118)
  Extract `should_replace` from Scoring and supply pooled txs from same sender (#116)

.travis.yml

@@ -12,8 +12,6 @@ matrix:
       rust: nightly
     - os: osx
       rust: stable
-    - os: windows
-      rust: stable
   allow_failures:
     - rust: nightly
 script:

README.md

@@ -1,7 +1,10 @@
 [![Build Status travis][travis-image]][travis-url]
+[![Build Status appveyor][appveyor-image]][appveyor-url]
 
 [travis-image]: https://travis-ci.org/paritytech/parity-common.svg?branch=master
 [travis-url]: https://travis-ci.org/paritytech/parity-common
+[appveyor-image]: https://ci.appveyor.com/api/projects/status/github/paritytech/parity-common/branch/master?svg=true
+[appveyor-url]: https://ci.appveyor.com/project/paritytech/parity-common/branch/master
 
 # parity-common
 Collection of crates used in [Parity Technologies](https://www.paritytech.io/) projects

appveyor.yml

@@ -0,0 +1,29 @@
+environment:
+  matrix:
+    - FEATURES: ""
+
+platform:
+  - x86_64-pc-windows-msvc
+
+# avoid running tests twice
+branches:
+  only:
+    - master
+
+install:
+  - curl -sSf -o rustup-init.exe https://win.rustup.rs/
+  - rustup-init.exe -y --default-host %PLATFORM%
+  - set PATH=%PATH%;C:\Users\appveyor\.cargo\bin
+  - rustc -vV
+  - cargo -vV
+
+build_script:
+  - cargo check --tests --features "%FEATURES%"
+  - cargo build --all --features "%FEATURES%"
+
+test_script:
+  - cargo test --all --features "%FEATURES%" --exclude uint --exclude fixed-hash
+  - cd fixed-hash/ && cargo test --all-features && cd ..
+  - cd uint/ && cargo test --features=std,quickcheck --release && cd ..
+  - cd plain_hasher/ && cargo test --no-default-features && cd ..
+  - cd parity-util-mem/ && cargo test --features=estimate-heapsize && cd ..

keccak-hash/Cargo.toml

@@ -1,15 +1,15 @@
 [package]
 name = "keccak-hash"
-version = "0.1.2"
+version = "0.1.3"
 description = "`keccak-hash` is a set of utility functions to facilitate working with Keccak hashes (256/512 bits long)."
 authors = ["Parity Technologies <admin@parity.io>"]
 repository = "https://github.com/paritytech/parity-common"
 readme = "README.md"
 license = "GPL-3.0"
 
 [dependencies]
-ethereum-types = "0.4"
 tiny-keccak = "1.4"
+primitive-types = { path = "../primitive-types", version = "0.2" }
 
 [dev-dependencies]
 tempdir = "0.3"

keccak-hash/benches/keccak_256.rs

@@ -17,10 +17,9 @@
 #![feature(test)]
 
 extern crate test;
-extern crate ethereum_types;
 extern crate keccak_hash;
 
-use keccak_hash::{keccak, write_keccak};
+use keccak_hash::keccak;
 use test::Bencher;
 
 #[bench]

keccak-hash/src/lib.rs

@@ -14,14 +14,15 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-extern crate ethereum_types;
+extern crate primitive_types;
 extern crate tiny_keccak;
 
 use std::io;
 use std::slice;
+
 use tiny_keccak::Keccak;
 
-pub use ethereum_types::H256;
+pub use primitive_types::H256;
 
 /// Get the KECCAK (i.e. Keccak) hash of the empty bytes string.
 pub const KECCAK_EMPTY: H256 = H256( [0xc5, 0xd2, 0x46, 0x01, 0x86, 0xf7, 0x23, 0x3c, 0x92, 0x7e, 0x7d, 0xb2, 0xdc, 0xc7, 0x03, 0xc0, 0xe5, 0x00, 0xb6, 0x53, 0xca, 0x82, 0x27, 0x3b, 0x7b, 0xfa, 0xd8, 0x04, 0x5d, 0x85, 0xa4, 0x70] );
@@ -91,6 +92,8 @@ mod tests {
 
 	use std::fs;
 	use std::io::{Write, BufReader};
+	use std::str::FromStr;
+
 	use self::tempdir::TempDir;
 	use super::{keccak, write_keccak, keccak_buffer, KECCAK_EMPTY};
 
@@ -101,7 +104,7 @@ mod tests {
 
 	#[test]
 	fn keccak_as() {
-		assert_eq!(keccak([0x41u8; 32]), From::from("59cad5948673622c1d64e2322488bf01619f7ff45789741b15a9f782ce9290a8"));
+		assert_eq!(keccak([0x41u8; 32]), FromStr::from_str("59cad5948673622c1d64e2322488bf01619f7ff45789741b15a9f782ce9290a8").unwrap());
 	}
 
 	#[test]

transaction-pool/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 description = "Generic transaction pool."
 name = "transaction-pool"
-version = "1.13.3"
+version = "2.0.0"
 license = "GPL-3.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 repository = "https://github.com/paritytech/parity-common"

transaction-pool/src/lib.rs

@@ -85,6 +85,7 @@ mod listener;
 mod options;
 mod pool;
 mod ready;
+mod replace;
 mod status;
 mod transactions;
 mod verifier;
@@ -96,6 +97,7 @@ pub use self::listener::{Listener, NoopListener};
 pub use self::options::Options;
 pub use self::pool::{Pool, PendingIterator, UnorderedIterator, Transaction};
 pub use self::ready::{Ready, Readiness};
+pub use self::replace::{ShouldReplace, ReplaceTransaction};
 pub use self::scoring::Scoring;
 pub use self::status::{LightStatus, Status};
 pub use self::verifier::Verifier;

transaction-pool/src/pool.rs

@@ -22,6 +22,7 @@ use error;
 use listener::{Listener, NoopListener};
 use options::Options;
 use ready::{Ready, Readiness};
+use replace::{ShouldReplace, ReplaceTransaction};
 use scoring::{self, Scoring, ScoreWithRef};
 use status::{LightStatus, Status};
 use transactions::{AddResult, Transactions};
@@ -130,10 +131,12 @@ impl<T, S, L> Pool<T, S, L> where
 	/// NOTE: The transaction may push out some other transactions from the pool
 	/// either because of limits (see `Options`) or because `Scoring` decides that the transaction
 	/// replaces an existing transaction from that sender.
-	/// If any limit is reached the transaction with the lowest `Score` is evicted to make room.
+	///
+	/// If any limit is reached the transaction with the lowest `Score` will be compared with the
+	/// new transaction via the supplied `ShouldReplace` implementation and may be evicted.
 	///
 	/// The `Listener` will be informed on any drops or rejections.
-	pub fn import(&mut self, transaction: T) -> error::Result<Arc<T>, T::Hash> {
+	pub fn import(&mut self, transaction: T, replace: &ShouldReplace<T>) -> error::Result<Arc<T>, T::Hash> {
 		let mem_usage = transaction.mem_usage();
 
 		if self.by_hash.contains_key(transaction.hash()) {
@@ -150,7 +153,7 @@ impl<T, S, L> Pool<T, S, L> where
 		// Avoid using should_replace, but rather use scoring for that.
 		{
 			let remove_worst = |s: &mut Self, transaction| {
-				match s.remove_worst(transaction) {
+				match s.remove_worst(transaction, replace) {
 					Err(err) => {
 						s.listener.rejected(transaction, &err);
 						Err(err)
@@ -283,22 +286,32 @@ impl<T, S, L> Pool<T, S, L> where
 	///
 	/// Returns `None` in case we couldn't decide if the transaction should replace the worst transaction or not.
 	/// In such case we will accept the transaction even though it is going to exceed the limit.
-	fn remove_worst(&mut self, transaction: &Transaction<T>) -> error::Result<Option<Transaction<T>>, T::Hash> {
+	fn remove_worst(&mut self, transaction: &Transaction<T>, replace: &ShouldReplace<T>) -> error::Result<Option<Transaction<T>>, T::Hash> {
 		let to_remove = match self.worst_transactions.iter().next_back() {
 			// No elements to remove? and the pool is still full?
 			None => {
 				warn!("The pool is full but there are no transactions to remove.");
 				return Err(error::Error::TooCheapToEnter(transaction.hash().clone(), "unknown".into()))
 			},
-			Some(old) => match self.scoring.should_replace(&old.transaction, transaction) {
-				// We can't decide which of them should be removed, so accept both.
-				scoring::Choice::InsertNew => None,
-				// New transaction is better than the worst one so we can replace it.
-				scoring::Choice::ReplaceOld => Some(old.clone()),
-				// otherwise fail
-				scoring::Choice::RejectNew => {
-					return Err(error::Error::TooCheapToEnter(transaction.hash().clone(), format!("{:#x}", old.score)))
-				},
+			Some(old) => {
+				let txs = &self.transactions;
+				let get_replace_tx = |tx| {
+					let sender_txs = txs.get(transaction.sender()).map(|txs| txs.iter().as_slice());
+					ReplaceTransaction::new(tx, sender_txs)
+				};
+				let old_replace = get_replace_tx(&old.transaction);
+				let new_replace = get_replace_tx(transaction);
+
+				match replace.should_replace(&old_replace, &new_replace) {
+					// We can't decide which of them should be removed, so accept both.
+					scoring::Choice::InsertNew => None,
+					// New transaction is better than the worst one so we can replace it.
+					scoring::Choice::ReplaceOld => Some(old.clone()),
+					// otherwise fail
+					scoring::Choice::RejectNew => {
+						return Err(error::Error::TooCheapToEnter(transaction.hash().clone(), format!("{:#x}", old.score)))
+					},
+				}
 			},
 		};
 

transaction-pool/src/replace.rs

@@ -0,0 +1,53 @@
+// Copyright 2015-2019 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+//! When queue limits are reached, decide whether to replace an existing transaction from the pool
+
+use pool::Transaction;
+use scoring::Choice;
+
+/// Encapsulates a transaction to be compared, along with pooled transactions from the same sender
+pub struct ReplaceTransaction<'a, T> {
+    /// The transaction to be compared for replacement
+    pub transaction: &'a Transaction<T>,
+    /// Other transactions currently in the pool for the same sender
+    pub pooled_by_sender: Option<&'a [Transaction<T>]>,
+}
+
+impl<'a, T> ReplaceTransaction<'a, T> {
+    /// Creates a new `ReplaceTransaction`
+    pub fn new(transaction: &'a Transaction<T>, pooled_by_sender: Option<&'a [Transaction<T>]>) -> Self {
+        ReplaceTransaction {
+            transaction,
+            pooled_by_sender,
+        }
+    }
+}
+
+impl<'a, T> ::std::ops::Deref for ReplaceTransaction<'a, T> {
+    type Target = Transaction<T>;
+    fn deref(&self) -> &Self::Target {
+        &self.transaction
+    }
+}
+
+/// Chooses whether a new transaction should replace an existing transaction if the pool is full.
+pub trait ShouldReplace<T> {
+    /// Decides if `new` should push out `old` transaction from the pool.
+    ///
+    /// NOTE returning `InsertNew` here can lead to some transactions being accepted above pool limits.
+    fn should_replace(&self, old: &ReplaceTransaction<T>, new: &ReplaceTransaction<T>) -> Choice;
+}

transaction-pool/src/scoring.rs

@@ -72,14 +72,12 @@ pub enum Change<T = ()> {
 /// - Returned `Score`s should match ordering of `compare` method.
 /// - `compare` will be called only within a context of transactions from the same sender.
 /// - `choose` may be called even if `compare` returns `Ordering::Equal`
-/// - `should_replace` is used to decide if new transaction should push out an old transaction already in the queue.
 /// - `Score`s and `compare` should align with `Ready` implementation.
 ///
 /// Example: Natural ordering of Ethereum transactions.
 /// - `compare`: compares transaction `nonce` ()
 /// - `choose`: compares transactions `gasPrice` (decides if old transaction should be replaced)
 /// - `update_scores`: score defined as `gasPrice` if `n==0` and `max(scores[n-1], gasPrice)` if `n>0`
-/// - `should_replace`: compares `gasPrice` (decides if transaction from a different sender is more valuable)
 ///
 pub trait Scoring<T>: fmt::Debug {
 	/// A score of a transaction.
@@ -98,11 +96,6 @@ pub trait Scoring<T>: fmt::Debug {
 	/// (i.e. score at index `i` represents transaction at the same index)
 	fn update_scores(&self, txs: &[Transaction<T>], scores: &mut [Self::Score], change: Change<Self::Event>);
 
-	/// Decides if `new` should push out `old` transaction from the pool.
-	///
-	/// NOTE returning `InsertNew` here can lead to some transactions being accepted above pool limits.
-	fn should_replace(&self, old: &T, new: &T) -> Choice;
-
 	/// Decides if the transaction should ignore per-sender limit in the pool.
 	///
 	/// If you return `true` for given transaction it's going to be accepted even though

transaction-pool/src/tests/helpers.rs

@@ -18,7 +18,7 @@ use std::cmp;
 use std::collections::HashMap;
 
 use ethereum_types::{H160 as Sender, U256};
-use {pool, scoring, Scoring, Ready, Readiness};
+use {pool, scoring, Scoring, ShouldReplace, ReplaceTransaction, Ready, Readiness};
 use super::Transaction;
 
 #[derive(Debug, Default)]
@@ -68,7 +68,13 @@ impl Scoring<Transaction> for DummyScoring {
 		}
 	}
 
-	fn should_replace(&self, old: &Transaction, new: &Transaction) -> scoring::Choice {
+	fn should_ignore_sender_limit(&self, _new: &Transaction) -> bool {
+		self.always_insert
+	}
+}
+
+impl ShouldReplace<Transaction> for DummyScoring {
+	fn should_replace(&self, old: &ReplaceTransaction<Transaction>, new: &ReplaceTransaction<Transaction>) -> scoring::Choice {
 		if self.always_insert {
 			scoring::Choice::InsertNew
 		} else if new.gas_price > old.gas_price {
@@ -77,10 +83,6 @@ impl Scoring<Transaction> for DummyScoring {
 			scoring::Choice::RejectNew
 		}
 	}
-
-	fn should_ignore_sender_limit(&self, _new: &Transaction) -> bool {
-		self.always_insert
-	}
 }
 
 #[derive(Default)]