Merge branch 'master' into ao-upgrade-rocksdb

* master:
  Fix limit prefix delete case (#368)
  Add arbitrary trait implementation (#378)

ethbloom/Cargo.toml

@@ -26,6 +26,7 @@ default = ["std", "serialize", "rustc-hex"]
 std = ["fixed-hash/std", "crunchy/std"]
 serialize = ["std", "impl-serde"]
 rustc-hex = ["fixed-hash/rustc-hex"]
+arbitrary = ["fixed-hash/arbitrary"]
 
 [[bench]]
 name = "bloom"

ethereum-types/Cargo.toml

@@ -22,3 +22,4 @@ serde_json = "1.0.41"
 default = ["std", "serialize"]
 std = ["uint-crate/std", "fixed-hash/std", "ethbloom/std", "primitive-types/std"]
 serialize = ["std", "impl-serde", "primitive-types/serde", "ethbloom/serialize"]
+arbitrary = ["ethbloom/arbitrary", "fixed-hash/arbitrary", "uint-crate/arbitrary"]

fixed-hash/Cargo.toml

@@ -19,6 +19,7 @@ quickcheck = { version = "0.9.0", optional = true }
 rand = { version = "0.7.2", optional = true, default-features = false }
 rustc-hex = { version = "2.0.1", optional = true, default-features = false }
 static_assertions = "1.0.0"
+arbitrary = { version = "0.4", optional = true }
 
 [dev-dependencies]
 rand_xorshift = "0.2.0"

fixed-hash/README.md

@@ -64,3 +64,5 @@ fixed-hash = { version = "0.3", default-features = false }
     - Disabled by default.
 - `api-dummy`: Generate a dummy hash type for API documentation.
     - Enabled by default at `docs.rs`
+- `arbitrary`: Allow for creation of a hash from random unstructured input.
+    - Disabled by default.

fixed-hash/src/hash.rs

@@ -316,6 +316,7 @@ macro_rules! construct_fixed_hash {
 		impl_cmp_for_fixed_hash!($name);
 		impl_rustc_hex_for_fixed_hash!($name);
 		impl_quickcheck_for_fixed_hash!($name);
+		impl_arbitrary_for_fixed_hash!($name);
 	}
 }
 
@@ -636,6 +637,42 @@ macro_rules! impl_quickcheck_for_fixed_hash {
 	};
 }
 
+// When the `arbitrary` feature is disabled.
+//
+// # Note
+//
+// Feature guarded macro definitions instead of feature guarded impl blocks
+// to work around the problems of introducing `arbitrary` crate feature in
+// a user crate.
+#[cfg(not(feature = "arbitrary"))]
+#[macro_export]
+#[doc(hidden)]
+macro_rules! impl_arbitrary_for_fixed_hash {
+	( $name:ident ) => {};
+}
+
+// When the `arbitrary` feature is enabled.
+//
+// # Note
+//
+// Feature guarded macro definitions instead of feature guarded impl blocks
+// to work around the problems of introducing `arbitrary` crate feature in
+// a user crate.
+#[cfg(feature = "arbitrary")]
+#[macro_export]
+#[doc(hidden)]
+macro_rules! impl_arbitrary_for_fixed_hash {
+	( $name:ident ) => {
+		impl $crate::arbitrary::Arbitrary for $name {
+			fn arbitrary(u: &mut $crate::arbitrary::Unstructured<'_>) -> $crate::arbitrary::Result<Self> {
+				let mut res = Self::zero();
+				u.fill_buffer(&mut res.0)?;
+				Ok(Self::from(res))
+			}
+		}
+	};
+}
+
 #[macro_export]
 #[doc(hidden)]
 macro_rules! impl_ops_for_hash {

fixed-hash/src/lib.rs

@@ -46,6 +46,10 @@ pub use rand;
 #[doc(hidden)]
 pub use quickcheck;
 
+#[cfg(feature = "arbitrary")]
+#[doc(hidden)]
+pub use arbitrary;
+
 #[macro_use]
 mod hash;
 

kvdb-memorydb/src/lib.rs

@@ -74,8 +74,11 @@ impl KeyValueDB for InMemory {
 							col.clear();
 						} else {
 							let start_range = Bound::Included(prefix.to_vec());
-							let end_range = Bound::Excluded(kvdb::end_prefix(&prefix[..]));
-							let keys: Vec<_> = col.range((start_range, end_range)).map(|(k, _)| k.clone()).collect();
+							let keys: Vec<_> = if let Some(end_range) = kvdb::end_prefix(&prefix[..]) {
+								col.range((start_range, Bound::Excluded(end_range))).map(|(k, _)| k.clone()).collect()
+							} else {
+								col.range((start_range, Bound::Unbounded)).map(|(k, _)| k.clone()).collect()
+							};
 							for key in keys.into_iter() {
 								col.remove(&key[..]);
 							}

kvdb-rocksdb/src/lib.rs

@@ -448,15 +448,16 @@ impl Database {
 							stats_total_bytes += key.len();
 							batch.delete_cf(cf, &key);
 						}
-						DBOp::DeletePrefix { col: _, prefix } => {
-							if !prefix.is_empty() {
-								let end_range = kvdb::end_prefix(&prefix[..]);
-								batch.delete_range_cf(cf, &prefix[..], &end_range[..]);
-							} else {
-								// Deletes all values in the column.
-								let end_range = &[u8::max_value()];
-								batch.delete_range_cf(cf, &[][..], &end_range[..]);
-								batch.delete_cf(cf, &end_range[..]);
+						DBOp::DeletePrefix { col, prefix } => {
+							let end_prefix = kvdb::end_prefix(&prefix[..]);
+							let no_end = end_prefix.is_none();
+							let end_range = end_prefix.unwrap_or_else(|| vec![u8::max_value(); 16]);
+							batch.delete_range_cf(cf, &prefix[..], &end_range[..]);
+							if no_end {
+								let prefix = if prefix.len() > end_range.len() { &prefix[..] } else { &end_range[..] };
+								for (key, _) in self.iter_with_prefix(col, prefix) {
+									batch.delete_cf(cf, &key[..]);
+								}
 							}
 						}
 					};
@@ -522,9 +523,8 @@ impl Database {
 		let read_lock = self.db.read();
 		let optional = if read_lock.is_some() {
 			let mut read_opts = generate_read_options();
-			let end_prefix = kvdb::end_prefix(prefix);
 			// rocksdb doesn't work with an empty upper bound
-			if !end_prefix.is_empty() {
+			if let Some(end_prefix) = kvdb::end_prefix(prefix) {
 				read_opts.set_iterate_upper_bound(end_prefix);
 			}
 			let guarded = iter::ReadGuardedIterator::new_with_prefix(read_lock, col, prefix, read_opts);

kvdb-shared-tests/src/lib.rs

@@ -175,7 +175,7 @@ pub fn test_io_stats(db: &dyn KeyValueDB) -> io::Result<()> {
 }
 
 /// The number of columns required to run `test_delete_prefix`.
-pub const DELETE_PREFIX_NUM_COLUMNS: u32 = 5;
+pub const DELETE_PREFIX_NUM_COLUMNS: u32 = 7;
 
 /// A test for `KeyValueDB::delete_prefix`.
 pub fn test_delete_prefix(db: &dyn KeyValueDB) -> io::Result<()> {
@@ -190,6 +190,7 @@ pub fn test_delete_prefix(db: &dyn KeyValueDB) -> io::Result<()> {
 		&[2][..],
 		&[2, 0][..],
 		&[2, 255][..],
+		&[255; 16][..],
 	];
 	let init_db = |ix: u32| -> io::Result<()> {
 		let mut batch = db.transaction();
@@ -199,8 +200,8 @@ pub fn test_delete_prefix(db: &dyn KeyValueDB) -> io::Result<()> {
 		db.write(batch)?;
 		Ok(())
 	};
-	let check_db = |ix: u32, content: [bool; 10]| -> io::Result<()> {
-		let mut state = [true; 10];
+	let check_db = |ix: u32, content: [bool; 11]| -> io::Result<()> {
+		let mut state = [true; 11];
 		for (c, key) in keys.iter().enumerate() {
 			state[c] = db.get(ix, key)?.is_some();
 		}
@@ -209,15 +210,19 @@ pub fn test_delete_prefix(db: &dyn KeyValueDB) -> io::Result<()> {
 	};
 	let tests: [_; DELETE_PREFIX_NUM_COLUMNS as usize] = [
 		// standard
-		(&[1u8][..], [true, true, true, false, false, false, false, true, true, true]),
+		(&[1u8][..], [true, true, true, false, false, false, false, true, true, true, true]),
 		// edge
-		(&[1u8, 255, 255][..], [true, true, true, true, true, true, false, true, true, true]),
+		(&[1u8, 255, 255][..], [true, true, true, true, true, true, false, true, true, true, true]),
 		// none 1
-		(&[1, 2][..], [true, true, true, true, true, true, true, true, true, true]),
+		(&[1, 2][..], [true, true, true, true, true, true, true, true, true, true, true]),
 		// none 2
-		(&[8][..], [true, true, true, true, true, true, true, true, true, true]),
+		(&[8][..], [true, true, true, true, true, true, true, true, true, true, true]),
+		// last value
+		(&[255, 255][..], [true, true, true, true, true, true, true, true, true, true, false]),
+		// last value, limit prefix
+		(&[255][..], [true, true, true, true, true, true, true, true, true, true, false]),
 		// all
-		(&[][..], [false, false, false, false, false, false, false, false, false, false]),
+		(&[][..], [false, false, false, false, false, false, false, false, false, false, false]),
 	];
 	for (ix, test) in tests.iter().enumerate() {
 		let ix = ix as u32;

kvdb/src/lib.rs

@@ -143,15 +143,19 @@ pub trait KeyValueDB: Sync + Send + parity_util_mem::MallocSizeOf {
 
 /// For a given start prefix (inclusive), returns the correct end prefix (non-inclusive).
 /// This assumes the key bytes are ordered in lexicographical order.
-pub fn end_prefix(prefix: &[u8]) -> Vec<u8> {
+/// Since key length is not limited, for some case we return `None` because there is
+/// no bounded limit (every keys in the serie `[]`, `[255]`, `[255, 255]` ...).
+pub fn end_prefix(prefix: &[u8]) -> Option<Vec<u8>> {
 	let mut end_range = prefix.to_vec();
 	while let Some(0xff) = end_range.last() {
 		end_range.pop();
 	}
 	if let Some(byte) = end_range.last_mut() {
 		*byte += 1;
+		Some(end_range)
+	} else {
+		None
 	}
-	end_range
 }
 
 #[cfg(test)]
@@ -160,18 +164,18 @@ mod test {
 
 	#[test]
 	fn end_prefix_test() {
-		assert_eq!(end_prefix(&[5, 6, 7]), vec![5, 6, 8]);
-		assert_eq!(end_prefix(&[5, 6, 255]), vec![5, 7]);
+		assert_eq!(end_prefix(&[5, 6, 7]), Some(vec![5, 6, 8]));
+		assert_eq!(end_prefix(&[5, 6, 255]), Some(vec![5, 7]));
 		// This is not equal as the result is before start.
-		assert_ne!(end_prefix(&[5, 255, 255]), vec![5, 255]);
+		assert_ne!(end_prefix(&[5, 255, 255]), Some(vec![5, 255]));
 		// This is equal ([5, 255] will not be deleted because
 		// it is before start).
-		assert_eq!(end_prefix(&[5, 255, 255]), vec![6]);
-		assert_eq!(end_prefix(&[255, 255, 255]), vec![]);
+		assert_eq!(end_prefix(&[5, 255, 255]), Some(vec![6]));
+		assert_eq!(end_prefix(&[255, 255, 255]), None);
 
-		assert_eq!(end_prefix(&[0x00, 0xff]), vec![0x01]);
-		assert_eq!(end_prefix(&[0xff]), vec![]);
-		assert_eq!(end_prefix(b"0"), b"1".to_vec());
-		assert_eq!(end_prefix(&[]), vec![]);
+		assert_eq!(end_prefix(&[0x00, 0xff]), Some(vec![0x01]));
+		assert_eq!(end_prefix(&[0xff]), None);
+		assert_eq!(end_prefix(&[]), None);
+		assert_eq!(end_prefix(b"0"), Some(b"1".to_vec()));
 	}
 }

primitive-types/Cargo.toml

@@ -22,3 +22,4 @@ rustc-hex = ["fixed-hash/rustc-hex"]
 serde = ["std", "impl-serde"]
 codec = ["impl-codec"]
 rlp = ["impl-rlp"]
+arbitrary = ["fixed-hash/arbitrary", "uint/arbitrary"]

uint/Cargo.toml

@@ -16,6 +16,7 @@ qc = { package = "quickcheck", version = "0.9.0", optional = true }
 rand = { version = "0.7.2", default-features = false, optional = true }
 rustc-hex = { version = "2.0.1", default-features = false }
 static_assertions = "1.0.0"
+arbitrary = { version = "0.4", optional = true }
 
 [features]
 default = ["std"]

uint/README.md

@@ -69,3 +69,5 @@ see fuzz [README.md](fuzz/README.md)
 	- Enabled by default.
 - `quickcheck`: Enable quickcheck-style property testing
 	- Use with `cargo test --release --features=quickcheck`.
+- `arbitrary`: Allow for creation of an `uint` object from random unstructured input for use with fuzzers that use the `arbitrary` crate.
+	- Disabled by default.

uint/src/lib.rs

@@ -29,6 +29,10 @@ pub use qc;
 #[doc(hidden)]
 pub use rand;
 
+#[cfg(feature = "arbitrary")]
+#[doc(hidden)]
+pub use arbitrary;
+
 #[doc(hidden)]
 pub use static_assertions;
 

uint/src/uint.rs

@@ -1554,6 +1554,7 @@ macro_rules! construct_uint {
 		// `$n_words * 8` because macro expects bytes and
 		// uints use 64 bit (8 byte) words
 		$crate::impl_quickcheck_arbitrary_for_uint!($name, ($n_words * 8));
+		$crate::impl_arbitrary_for_uint!($name, ($n_words * 8));
 	}
 }
 
@@ -1632,3 +1633,26 @@ macro_rules! impl_quickcheck_arbitrary_for_uint {
 macro_rules! impl_quickcheck_arbitrary_for_uint {
 	($uint: ty, $n_bytes: tt) => {};
 }
+
+
+#[cfg(feature = "arbitrary")]
+#[macro_export]
+#[doc(hidden)]
+macro_rules! impl_arbitrary_for_uint {
+	($uint: ty, $n_bytes: tt) => {
+		impl $crate::arbitrary::Arbitrary for $uint {
+			fn arbitrary(u: &mut $crate::arbitrary::Unstructured<'_>) -> $crate::arbitrary::Result<Self> {
+				let mut res = [0u8; $n_bytes];
+				u.fill_buffer(&mut res)?;
+				Ok(Self::from(res))
+			}
+		}
+	};
+}
+
+#[cfg(not(feature = "arbitrary"))]
+#[macro_export]
+#[doc(hidden)]
+macro_rules! impl_arbitrary_for_uint {
+	($uint: ty, $n_bytes: tt) => {};
+}