From 478cebf42f0ca3496faef29c066464186821db01 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 14 Jan 2016 22:41:39 +0100 Subject: [PATCH 1/4] Cleanup old code. --- src/account.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/account.rs b/src/account.rs index 70c14f5aa..0923692d1 100644 --- a/src/account.rs +++ b/src/account.rs @@ -149,18 +149,11 @@ impl Account { /// Provide a database to lookup `code_hash`. Should not be called if it is a contract without code. pub fn cache_code(&mut self, db: &HashDB) -> bool { // TODO: fill out self.code_cache; -/* return !self.is_cached() || - match db.lookup(&self.code_hash.unwrap()) { // why doesn't this work? unwrap causes move?! - Some(x) => { self.code_cache = x.to_vec(); true }, - _ => { false } - }*/ - if self.is_cached() { return true; } - return if let Some(ref h) = self.code_hash { - match db.lookup(&h) { + return self.is_cached() || + match db.lookup(self.code_hash.as_ref().unwrap()) { // why doesn't this work? unwrap causes move?! Some(x) => { self.code_cache = x.to_vec(); true }, _ => { false } } - } else { false } } /// return the storage root associated with this account. @@ -266,7 +259,7 @@ mod tests { }; let mut a = Account::from_rlp(&rlp); - assert_eq!(a.cache_code(&db), true); + assert!(a.cache_code(&db)); let mut a = Account::from_rlp(&rlp); assert_eq!(a.note_code(vec![0x55, 0x44, 0xffu8]), Ok(())); From 6f5d3838346752dc9bf28bd85bd11648b99aa84b Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 14 Jan 2016 22:45:50 +0100 Subject: [PATCH 2/4] Storage should kill zero entries. --- src/account.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/account.rs b/src/account.rs index 0923692d1..44a07738b 100644 --- a/src/account.rs +++ b/src/account.rs @@ -187,7 +187,10 @@ impl Account { if f == &Filth::Dirty { // cast key and value to trait type, // so we can call overloaded `to_bytes` method - t.insert(k, &encode(&U256::from(v.as_slice()))); + match v.is_zero() { + true => t.remove(k), + false => t.insert(k, &encode(&U256::from(v.as_slice()))), + } *f = Filth::Clean; } } From ea9d333312170788fc0cf1790925770911d3b01b Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 14 Jan 2016 23:13:05 +0100 Subject: [PATCH 3/4] Fix account and add butress test. --- src/account.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/account.rs b/src/account.rs index 44a07738b..658144654 100644 --- a/src/account.rs +++ b/src/account.rs @@ -182,14 +182,14 @@ impl Account { /// Commit the `storage_overlay` to the backing DB and update `storage_root`. pub fn commit_storage(&mut self, db: &mut HashDB) { - let mut t = SecTrieDBMut::new(db, &mut self.storage_root); + let mut t = SecTrieDBMut::from_existing(db, &mut self.storage_root); for (k, &mut (ref mut f, ref mut v)) in self.storage_overlay.borrow_mut().iter_mut() { if f == &Filth::Dirty { // cast key and value to trait type, // so we can call overloaded `to_bytes` method match v.is_zero() { - true => t.remove(k), - false => t.insert(k, &encode(&U256::from(v.as_slice()))), + true => { t.remove(k); }, + false => { t.insert(k, &encode(&U256::from(v.as_slice()))); }, } *f = Filth::Clean; } @@ -272,12 +272,25 @@ mod tests { fn commit_storage() { let mut a = Account::new_contract(U256::from(69u8)); let mut db = OverlayDB::new_temp(); - a.set_storage(H256::from(&U256::from(0x00u64)), H256::from(&U256::from(0x1234u64))); + a.set_storage(x!(0), x!(0x1234)); assert_eq!(a.storage_root(), None); a.commit_storage(&mut db); assert_eq!(a.storage_root().unwrap().hex(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2"); } + #[test] + fn commit_remove_commit_storage() { + let mut a = Account::new_contract(U256::from(69u8)); + let mut db = OverlayDB::new_temp(); + a.set_storage(x!(0), x!(0x1234)); + a.commit_storage(&mut db); + a.set_storage(x!(1), x!(0x1234)); + a.commit_storage(&mut db); + a.set_storage(x!(1), x!(0)); + a.commit_storage(&mut db); + assert_eq!(a.storage_root().unwrap().hex(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2"); + } + #[test] fn commit_code() { let mut a = Account::new_contract(U256::from(69u8)); From d64ff8df5764aabfd8034bbe29e1c5c3a7368d3d Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 14 Jan 2016 23:41:48 +0100 Subject: [PATCH 4/4] Minor renaming, fix for Account::code_cache. --- src/account.rs | 9 ++++++--- src/tests/executive.rs | 13 ++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/account.rs b/src/account.rs index 658144654..68b0fdbc3 100644 --- a/src/account.rs +++ b/src/account.rs @@ -150,9 +150,12 @@ impl Account { pub fn cache_code(&mut self, db: &HashDB) -> bool { // TODO: fill out self.code_cache; return self.is_cached() || - match db.lookup(self.code_hash.as_ref().unwrap()) { // why doesn't this work? unwrap causes move?! - Some(x) => { self.code_cache = x.to_vec(); true }, - _ => { false } + match self.code_hash { + Some(ref h) => match db.lookup(h) { + Some(x) => { self.code_cache = x.to_vec(); true }, + _ => false, + }, + _ => false, } } diff --git a/src/tests/executive.rs b/src/tests/executive.rs index 61c4df7ac..95a953200 100644 --- a/src/tests/executive.rs +++ b/src/tests/executive.rs @@ -228,7 +228,6 @@ fn do_json_test(json_data: &[u8]) -> Vec { fail_unless(gas_left == xjson!(&test["gas"]), "gas_left is incorrect"); fail_unless(output == Bytes::from_json(&test["out"]), "output is incorrect"); - test.find("post").map(|pre| for (addr, s) in pre.as_object().unwrap() { let address = Address::from(addr.as_ref()); @@ -241,16 +240,16 @@ fn do_json_test(json_data: &[u8]) -> Vec { let cc = test["callcreates"].as_array().unwrap(); fail_unless(callcreates.len() == cc.len(), "callcreates does not match"); for i in 0..cc.len() { - let is = &callcreates[i]; + let callcreate = &callcreates[i]; let expected = &cc[i]; - fail_unless(is.data == Bytes::from_json(&expected["data"]), "callcreates data is incorrect"); - fail_unless(is.destination == xjson!(&expected["destination"]), "callcreates destination is incorrect"); - fail_unless(is.value == xjson!(&expected["value"]), "callcreates value is incorrect"); + fail_unless(callcreate.data == Bytes::from_json(&expected["data"]), "callcreates data is incorrect"); + fail_unless(callcreate.destination == xjson!(&expected["destination"]), "callcreates destination is incorrect"); + fail_unless(callcreate.value == xjson!(&expected["value"]), "callcreates value is incorrect"); // TODO: call_gas is calculated in externalities and is not exposed to TestExt. // maybe move it to it's own function to simplify calculation? - //println!("name: {:?}, is {:?}, expected: {:?}", name, is.gas_limit, U256::from(&expected["gasLimit"])); - //fail_unless(is.gas_limit == U256::from(&expected["gasLimit"]), "callcreates gas_limit is incorrect"); + //println!("name: {:?}, callcreate {:?}, expected: {:?}", name, callcreate.gas_limit, U256::from(&expected["gasLimit"])); + //fail_unless(callcreate.gas_limit == U256::from(&expected["gasLimit"]), "callcreates gas_limit is incorrect"); } } }