From b1e986cbdd4cd880cc357b652cba333c1698bf90 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Thu, 14 Jan 2016 22:02:10 +0100 Subject: [PATCH 1/4] Fixing multiplication in uints --- src/uint.rs | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/src/uint.rs b/src/uint.rs index b5607390b..f25cd77d0 100644 --- a/src/uint.rs +++ b/src/uint.rs @@ -145,10 +145,15 @@ macro_rules! construct_uint { for i in 0..$n_words { let upper = other as u64 * (arr[i] >> 32); let lower = other as u64 * (arr[i] & 0xFFFFFFFF); - if i < 3 { - carry[i + 1] += upper >> 32; + + ret[i] = lower.wrapping_add(upper << 32); + + if i < $n_words - 1 { + carry[i + 1] = upper >> 32; + if ret[i] < lower { + carry[i + 1] += 1; + } } - ret[i] = lower + (upper << 32); } $name(ret) + $name(carry) } @@ -557,7 +562,9 @@ pub const BAD_U256: U256 = U256([0xffffffffffffffffu64; 4]); #[cfg(test)] mod tests { + use uint::U128; use uint::U256; + use uint::U512; use uint::FromDecStr; use std::str::FromStr; @@ -737,6 +744,60 @@ mod tests { assert_eq!(U256::from(1u64) * U256::from(10u64), U256::from(10u64)); } + #[test] + pub fn uint128_add() { + assert_eq!( + U128::from_str("fffffffffffffffff").unwrap() + U128::from_str("fffffffffffffffff").unwrap(), + U128::from_str("1ffffffffffffffffe").unwrap() + ); + } + + #[test] + pub fn uint128_add_overflow() { + assert_eq!( + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap() + + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap(), + U128::from_str("fffffffffffffffffffffffffffffffe").unwrap() + ); + } + + #[test] + pub fn uint128_mul() { + assert_eq!( + U128::from_str("fffffffff").unwrap() * U128::from_str("fffffffff").unwrap(), + U128::from_str("ffffffffe000000001").unwrap()); + } + + #[test] + pub fn uint512_mul() { + assert_eq!( + U512::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + * + U512::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(), + U512::from_str("3fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0000000000000000000000000000000000000000000000000000000000000001").unwrap() + ); + } + + #[test] + pub fn uint256_mul_overflow() { + assert_eq!( + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + * + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(), + U256::from_str("1").unwrap() + ); + } + + #[test] + pub fn uint256_mul_overflow2() { + assert_eq!( + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + * + U256::from_str("2").unwrap(), + U256::from_str("fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe").unwrap() + ); + } + #[test] fn uint256_div() { assert_eq!(U256::from(10u64) / U256::from(1u64), U256::from(10u64)); From 9b78a89f966c34900f49446490cc5c3608f30788 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Fri, 15 Jan 2016 01:41:08 +0100 Subject: [PATCH 2/4] Overflow semantics changed --- src/lib.rs | 1 + src/uint.rs | 227 ++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 212 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7bfe147f9..f49e04af9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ #![feature(op_assign_traits)] #![feature(associated_consts)] +#![feature(wrapping)] //! Ethcore-util library //! //! ### Rust version: diff --git a/src/uint.rs b/src/uint.rs index f25cd77d0..e1e2dcd42 100644 --- a/src/uint.rs +++ b/src/uint.rs @@ -26,6 +26,7 @@ use std::cmp::*; use std::ops::*; use std::str::FromStr; use std::hash::{Hash, Hasher}; +use std::num::wrapping::OverflowingOps; use rustc_serialize::hex::{FromHex, FromHexError}; pub trait FromDecStr: Sized { @@ -43,6 +44,14 @@ macro_rules! impl_map_from { } } +macro_rules! panic_on_overflow { + ($name:expr) => { + if $name { + panic!("arithmetic operation overflow") + } + } +} + macro_rules! construct_uint { ($name:ident, $n_words:expr) => ( /// Little-endian large integer type @@ -157,6 +166,31 @@ macro_rules! construct_uint { } $name(ret) + $name(carry) } + + /// Overflowing multiplication by u32 + fn overflowing_mul_u32(self, other: u32) -> ($name, bool) { + let $name(ref arr) = self; + let mut carry = [0u64; $n_words]; + let mut ret = [0u64; $n_words]; + let mut overflow = false; + for i in 0..$n_words { + let upper = other as u64 * (arr[i] >> 32); + let lower = other as u64 * (arr[i] & 0xFFFFFFFF); + + ret[i] = lower.wrapping_add(upper << 32); + + if i < $n_words - 1 { + carry[i + 1] = upper >> 32; + if ret[i] < lower { + carry[i + 1] += 1; + } + } else if (upper >> 32) > 0 || ret[i] < lower { + overflow = true + } + } + let (result, add_overflow) = $name(ret).overflowing_add($name(carry)); + (result, add_overflow || overflow) + } } impl From for $name { @@ -214,6 +248,77 @@ macro_rules! construct_uint { } } + impl OverflowingOps for $name { + fn overflowing_add(self, other: $name) -> ($name, bool) { + let $name(ref me) = self; + let $name(ref you) = other; + let mut ret = [0u64; $n_words]; + let mut carry = [0u64; $n_words]; + let mut b_carry = false; + let mut overflow = false; + + for i in 0..$n_words { + ret[i] = me[i].wrapping_add(you[i]); + + if ret[i] < me[i] { + if i < $n_words - 1 { + carry[i + 1] = 1; + b_carry = true; + } else { + overflow = true + } + } + } + if b_carry { + let (ret, add_overflow) = $name(ret).overflowing_add($name(carry)); + (ret, add_overflow || overflow) + } else { + ($name(ret), overflow) + } + } + + fn overflowing_sub(self, other: $name) -> ($name, bool) { + let (res, _overflow) = (!other).overflowing_add(From::from(1u64)); + let (res, _overflow) = self.overflowing_add(res); + (res, self < other) + } + + fn overflowing_mul(self, other: $name) -> ($name, bool) { + let mut res = $name::from(0u64); + let mut overflow = false; + // TODO: be more efficient about this + for i in 0..(2 * $n_words) { + let (v, mul_overflow) = self.overflowing_mul_u32((other >> (32 * i)).low_u32()); + let (new_res, add_overflow) = res.overflowing_add(v << (32 * i)); + res = new_res; + overflow = overflow || mul_overflow || add_overflow; + } + (res, overflow) + } + + fn overflowing_div(self, other: $name) -> ($name, bool) { + (self / other, false) + } + + fn overflowing_rem(self, other: $name) -> ($name, bool) { + (self % other, false) + } + + fn overflowing_neg(self) -> ($name, bool) { + (!self, true) + } + + fn overflowing_shl(self, _shift32: u32) -> ($name, bool) { + // TODO [todr] not used for now + unimplemented!(); + } + + fn overflowing_shr(self, _shift32: u32) -> ($name, bool) { + // TODO [todr] not used for now + unimplemented!(); + } + } + impl Add<$name> for $name { type Output = $name; @@ -224,10 +329,14 @@ macro_rules! construct_uint { let mut carry = [0u64; $n_words]; let mut b_carry = false; for i in 0..$n_words { - ret[i] = me[i].wrapping_add(you[i]); - if i < $n_words - 1 && ret[i] < me[i] { - carry[i + 1] = 1; - b_carry = true; + if i < $n_words - 1 { + ret[i] = me[i].wrapping_add(you[i]); + if ret[i] < me[i] { + carry[i + 1] = 1; + b_carry = true; + } + } else { + ret[i] = me[i] + you[i]; } } if b_carry { $name(ret) + $name(carry) } else { $name(ret) } @@ -239,7 +348,10 @@ macro_rules! construct_uint { #[inline] fn sub(self, other: $name) -> $name { - self + !other + From::from(1u64) + panic_on_overflow!(self < other); + let (res, _overflow) = (!other).overflowing_add(From::from(1u64)); + let (res, _overflow) = self.overflowing_add(res); + res } } @@ -281,7 +393,8 @@ macro_rules! construct_uint { loop { if sub_copy >= shift_copy { ret[shift / 64] |= 1 << (shift % 64); - sub_copy = sub_copy - shift_copy; + let (copy, _overflow) = sub_copy.overflowing_sub(shift_copy); + sub_copy = copy } shift_copy = shift_copy >> 1; if shift == 0 { break; } @@ -370,7 +483,7 @@ macro_rules! construct_uint { let bit_shift = shift % 64; for i in 0..$n_words { // Shift - if bit_shift < 64 && i + word_shift < $n_words { + if i + word_shift < $n_words { ret[i + word_shift] += original[i] << bit_shift; } // Carry @@ -567,6 +680,7 @@ mod tests { use uint::U512; use uint::FromDecStr; use std::str::FromStr; + use std::num::wrapping::OverflowingOps; #[test] pub fn uint256_from() { @@ -692,7 +806,7 @@ mod tests { let incr = shr + U256::from(1u64); assert_eq!(incr, U256([0x7DDE000000000001u64, 0x0001BD5B7DDFBD5B, 0, 0])); // Subtraction - let sub = incr - init; + let (sub, _of) = incr.overflowing_sub(init); assert_eq!(sub, U256([0x9F30411021524112u64, 0x0001BD5B7DDFBD5A, 0, 0])); // Multiplication let mult = sub.mul_u32(300); @@ -740,7 +854,7 @@ mod tests { } #[test] - pub fn uint256_mul() { + pub fn uint256_mul1() { assert_eq!(U256::from(1u64) * U256::from(10u64), U256::from(10u64)); } @@ -755,12 +869,22 @@ mod tests { #[test] pub fn uint128_add_overflow() { assert_eq!( - U128::from_str("ffffffffffffffffffffffffffffffff").unwrap() - + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap(), - U128::from_str("fffffffffffffffffffffffffffffffe").unwrap() + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap() + .overflowing_add( + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap() + ), + (U128::from_str("fffffffffffffffffffffffffffffffe").unwrap(), true) ); } + #[test] + #[should_panic] + pub fn uint128_add_overflow_panic() { + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap() + + + U128::from_str("ffffffffffffffffffffffffffffffff").unwrap(); + } + #[test] pub fn uint128_mul() { assert_eq!( @@ -782,14 +906,85 @@ mod tests { pub fn uint256_mul_overflow() { assert_eq!( U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() - * - U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(), - U256::from_str("1").unwrap() + .overflowing_mul( + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + ), + (U256::from_str("1").unwrap(), true) ); } #[test] - pub fn uint256_mul_overflow2() { + #[should_panic] + pub fn uint256_mul_overflow_panic() { + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + * + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(); + } + + #[test] + pub fn uint256_sub_overflow() { + assert_eq!( + U256::from_str("0").unwrap() + .overflowing_sub( + U256::from_str("1").unwrap() + ), + (U256::from_str("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(), true) + ); + } + + #[test] + #[should_panic] + pub fn uint256_sub_overflow_panic() { + U256::from_str("0").unwrap() + - + U256::from_str("1").unwrap(); + } + + #[ignore] + #[test] + pub fn uint256_shl_overflow() { + assert_eq!( + U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + .overflowing_shl(4), + (U256::from_str("fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0").unwrap(), true) + ); + } + + #[ignore] + #[test] + #[should_panic] + pub fn uint256_shl_overflow2() { + assert_eq!( + U256::from_str("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + .overflowing_shl(4), + (U256::from_str("fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0").unwrap(), false) + ); + } + + #[ignore] + #[test] + pub fn uint256_shr_overflow() { + assert_eq!( + U256::from_str("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() + .overflowing_shr(4), + (U256::from_str("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(), true) + ); + } + + #[ignore] + #[test] + pub fn uint256_shr_overflow2() { + assert_eq!( + U256::from_str("fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0").unwrap() + .overflowing_shr(4), + (U256::from_str("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(), false) + ); + } + + + + #[test] + pub fn uint256_mul() { assert_eq!( U256::from_str("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap() * From 838eea61f9d41b035041c535b868db4a06a91234 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 15 Jan 2016 01:57:14 +0100 Subject: [PATCH 3/4] updated to rocksdb wrapper version 0.3 --- Cargo.toml | 2 +- src/overlaydb.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6e32edcab..f92dcca53 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ mio = "0.5.0" rand = "0.3.12" time = "0.1.34" tiny-keccak = "1.0" -rocksdb = "0.2" +rocksdb = "0.3" lazy_static = "0.1" eth-secp256k1 = { git = "https://github.com/arkpar/rust-secp256k1.git" } rust-crypto = "0.2.34" diff --git a/src/overlaydb.rs b/src/overlaydb.rs index 5539a8893..929a492ef 100644 --- a/src/overlaydb.rs +++ b/src/overlaydb.rs @@ -10,7 +10,7 @@ use std::ops::*; use std::sync::*; use std::env; use std::collections::HashMap; -use rocksdb::{DB, Writable}; +use rocksdb::{DB, Writable, IteratorMode}; #[derive(Clone)] /// Implementation of the HashDB trait for a disk-backed database with a memory overlay. @@ -138,7 +138,7 @@ impl OverlayDB { impl HashDB for OverlayDB { fn keys(&self) -> HashMap { let mut ret: HashMap = HashMap::new(); - for (key, _) in self.backing.iterator().from_start() { + for (key, _) in self.backing.iterator(IteratorMode::Start) { let h = H256::from_slice(key.deref()); let r = self.payload(&h).unwrap().1; ret.insert(h, r as i32); From 23fbe394081cdde8f8460cf32c40959d48c2f5fa Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 15 Jan 2016 04:02:24 +0100 Subject: [PATCH 4/4] Fix for assumption that empty trie root RLP can always be looked up. --- src/hash.rs | 1 - src/memorydb.rs | 31 +++++++++++++++++++++++++++---- src/trie/triedb.rs | 2 +- src/trie/triedbmut.rs | 12 +++++------- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/hash.rs b/src/hash.rs index c17b706db..8c1772b2c 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -195,7 +195,6 @@ macro_rules! impl_hash { fn from_json(json: &Json) -> Self { match json { &Json::String(ref s) => { - println!("s: {}", s); match s.len() % 2 { 0 => FromStr::from_str(clean_0x(s)).unwrap(), _ => FromStr::from_str(&("0".to_string() + &(clean_0x(s).to_string()))[..]).unwrap() diff --git a/src/memorydb.rs b/src/memorydb.rs index 680129670..6f2b2e603 100644 --- a/src/memorydb.rs +++ b/src/memorydb.rs @@ -2,6 +2,7 @@ use hash::*; use bytes::*; +use rlp::*; use sha3::*; use hashdb::*; use std::mem; @@ -53,13 +54,15 @@ use std::collections::HashMap; /// ``` pub struct MemoryDB { data: HashMap, + static_null_rlp: (Bytes, i32), } impl MemoryDB { /// Create a new instance of the memory DB. pub fn new() -> MemoryDB { MemoryDB { - data: HashMap::new() + data: HashMap::new(), + static_null_rlp: (vec![0x80u8; 1], 1), } } @@ -98,6 +101,9 @@ impl MemoryDB { /// Even when Some is returned, the data is only guaranteed to be useful /// when the refs > 0. pub fn raw(&self, key: &H256) -> Option<&(Bytes, i32)> { + if key == &SHA3_NULL_RLP { + return Some(&self.static_null_rlp); + } self.data.get(key) } @@ -108,18 +114,23 @@ impl MemoryDB { } pub fn denote(&self, key: &H256, value: Bytes) -> &(Bytes, i32) { - if self.data.get(&key) == None { + if self.raw(key) == None { unsafe { let p = &self.data as *const HashMap as *mut HashMap; (*p).insert(key.clone(), (value, 0)); } } - self.data.get(key).unwrap() + self.raw(key).unwrap() } } +static NULL_RLP_STATIC: [u8; 1] = [0x80; 1]; + impl HashDB for MemoryDB { fn lookup(&self, key: &H256) -> Option<&[u8]> { + if key == &SHA3_NULL_RLP { + return Some(&NULL_RLP_STATIC); + } match self.data.get(key) { Some(&(ref d, rc)) if rc > 0 => Some(d), _ => None @@ -127,10 +138,13 @@ impl HashDB for MemoryDB { } fn keys(&self) -> HashMap { - self.data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect::>() + self.data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect() } fn exists(&self, key: &H256) -> bool { + if key == &SHA3_NULL_RLP { + return true; + } match self.data.get(key) { Some(&(_, x)) if x > 0 => true, _ => false @@ -138,6 +152,9 @@ impl HashDB for MemoryDB { } fn insert(&mut self, value: &[u8]) -> H256 { + if value == &NULL_RLP { + return SHA3_NULL_RLP.clone(); + } let key = value.sha3(); if match self.data.get_mut(&key) { Some(&mut (ref mut old_value, ref mut rc @ -0x80000000i32 ... 0)) => { @@ -154,6 +171,9 @@ impl HashDB for MemoryDB { } fn emplace(&mut self, key: H256, value: Bytes) { + if value == &NULL_RLP { + return; + } match self.data.get_mut(&key) { Some(&mut (ref mut old_value, ref mut rc @ -0x80000000i32 ... 0)) => { *old_value = value; @@ -168,6 +188,9 @@ impl HashDB for MemoryDB { } fn kill(&mut self, key: &H256) { + if key == &SHA3_NULL_RLP { + return; + } if match self.data.get_mut(key) { Some(&mut (_, ref mut x)) => { *x -= 1; false } None => true diff --git a/src/trie/triedb.rs b/src/trie/triedb.rs index ef07fd47d..862bbb96c 100644 --- a/src/trie/triedb.rs +++ b/src/trie/triedb.rs @@ -81,7 +81,7 @@ impl<'db> TrieDB<'db> { let mut ret = self.db.keys(); for (k, v) in Self::to_map(self.keys()).into_iter() { let keycount = *ret.get(&k).unwrap_or(&0); - match keycount == v as i32 { + match keycount <= v as i32 { true => ret.remove(&k), _ => ret.insert(k, keycount - v as i32), }; diff --git a/src/trie/triedbmut.rs b/src/trie/triedbmut.rs index d205dc8e0..c6f47fa5e 100644 --- a/src/trie/triedbmut.rs +++ b/src/trie/triedbmut.rs @@ -64,16 +64,13 @@ impl<'db> TrieDBMut<'db> { }; // set root rlp - *r.root = r.db.insert(&NULL_RLP); + *r.root = SHA3_NULL_RLP.clone(); r } /// Create a new trie with the backing database `db` and `root` /// Panics, if `root` does not exist pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Self { - if !db.exists(root) && root == &SHA3_NULL_RLP { - *root = db.insert(&NULL_RLP); - } assert!(db.exists(root)); TrieDBMut { db: db, @@ -111,7 +108,7 @@ impl<'db> TrieDBMut<'db> { let mut ret = self.db.keys(); for (k, v) in Self::to_map(self.keys()).into_iter() { let keycount = *ret.get(&k).unwrap_or(&0); - match keycount == v as i32 { + match keycount <= v as i32 { true => ret.remove(&k), _ => ret.insert(k, keycount - v as i32), }; @@ -771,8 +768,9 @@ mod tests { assert!(memtrie.db_items_remaining().is_empty()); unpopulate_trie(&mut memtrie, &x); if *memtrie.root() != SHA3_NULL_RLP || !memtrie.db_items_remaining().is_empty() { - println!("TRIE MISMATCH"); + println!("- TRIE MISMATCH"); println!(""); + println!("remaining: {:?}", memtrie.db_items_remaining()); println!("{:?} vs {:?}", memtrie.root(), real); for i in x.iter() { println!("{:?} -> {:?}", i.0.pretty(), i.1.pretty()); @@ -811,7 +809,7 @@ mod tests { let mut t1 = TrieDBMut::new(&mut memdb, &mut root); t1.insert(&[0x01, 0x23], &big_value.to_vec()); t1.insert(&[0x01, 0x34], &big_value.to_vec()); - trace!("keys remaining {:?}", t1.db_items_remaining()); + println!("********************** keys remaining {:?}", t1.db_items_remaining()); assert!(t1.db_items_remaining().is_empty()); let mut memdb2 = MemoryDB::new(); let mut root2 = H256::new();