Merge pull request #114 from gavofyork/gav

Fix storage/account and add butress test.

src/account.rs

@@ -149,18 +149,14 @@ 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() ||
+		return self.is_cached() ||
-			match db.lookup(&self.code_hash.unwrap()) {	// why doesn't this work? unwrap causes move?!
+			match self.code_hash {
-				Some(x) => { self.code_cache = x.to_vec(); true },
+				Some(ref h) => match db.lookup(h) {
-				_ => { false }
+					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 {
+				_ => false,
-			match db.lookup(&h) {
-				Some(x) => { self.code_cache = x.to_vec(); true },
-				_ => { false }
 			}
-		} else { false }
 	}
 
 	/// return the storage root associated with this account.
@@ -189,12 +185,15 @@ 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
-				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;
 			}
 		}
@@ -266,7 +265,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(()));
@@ -276,12 +275,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));

src/tests/executive.rs

@@ -228,7 +228,6 @@ fn do_json_test(json_data: &[u8]) -> Vec<String> {
 				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<String> {
 				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(callcreate.data == Bytes::from_json(&expected["data"]), "callcreates data is incorrect");
-					fail_unless(is.destination == xjson!(&expected["destination"]), "callcreates destination is incorrect");
+					fail_unless(callcreate.destination == xjson!(&expected["destination"]), "callcreates destination is incorrect");
-					fail_unless(is.value == xjson!(&expected["value"]), "callcreates value 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"]));
+					//println!("name: {:?}, callcreate {:?}, expected: {:?}", name, callcreate.gas_limit, U256::from(&expected["gasLimit"]));
-					//fail_unless(is.gas_limit == U256::from(&expected["gasLimit"]), "callcreates gas_limit is incorrect");
+					//fail_unless(callcreate.gas_limit == U256::from(&expected["gasLimit"]), "callcreates gas_limit is incorrect");
 				}
 			}
 		}