Merge pull request #130 from gavofyork/gav

Various consensus fixes.

src/account.rs

@@ -205,7 +205,6 @@ impl Account {
 		match (self.code_hash.is_none(), self.code_cache.is_empty()) {
 			(true, true) => self.code_hash = Some(SHA3_EMPTY),
 			(true, false) => {
-				println!("Writing into DB {:?}", self.code_cache);
 				self.code_hash = Some(db.insert(&self.code_cache));
 			},
 			(false, _) => {},

src/block.rs

@@ -126,6 +126,15 @@ impl<'x, 'y> OpenBlock<'x, 'y> {
 	/// Alter the timestamp of the block.
 	pub fn set_timestamp(&mut self, timestamp: u64) { self.block.header.set_timestamp(timestamp); }
 
+	/// Alter the difficulty for the block.
+	pub fn set_difficulty(&mut self, a: U256) { self.block.header.set_difficulty(a); }
+
+	/// Alter the gas limit for the block.
+	pub fn set_gas_limit(&mut self, a: U256) { self.block.header.set_gas_limit(a); }
+
+	/// Alter the gas limit for the block.
+	pub fn set_gas_used(&mut self, a: U256) { self.block.header.set_gas_used(a); }
+
 	/// Alter the extra_data for the block.
 	pub fn set_extra_data(&mut self, extra_data: Bytes) -> Result<(), BlockError> {
 		if extra_data.len() > self.engine.maximum_extra_data_size() {
@@ -158,7 +167,7 @@ impl<'x, 'y> OpenBlock<'x, 'y> {
 			author: self.block.header.author.clone(),
 			timestamp: self.block.header.timestamp,
 			difficulty: self.block.header.difficulty.clone(),
-			last_hashes: self.last_hashes.clone(),
+			last_hashes: self.last_hashes.clone(),		// TODO: should be a reference.
 			gas_used: self.block.archive.last().map(|t| t.receipt.gas_used).unwrap_or(U256::from(0)),
 			gas_limit: self.block.header.gas_limit.clone(),
 		}
@@ -169,6 +178,7 @@ impl<'x, 'y> OpenBlock<'x, 'y> {
 	/// If valid, it will be executed, and archived together with the receipt.
 	pub fn push_transaction(&mut self, t: Transaction, h: Option<H256>) -> Result<&Receipt, Error> {
 		let env_info = self.env_info();
+//		info!("env_info says gas_used={}", env_info.gas_used);
 		match self.block.state.apply(&env_info, self.engine, &t) {
 			Ok(receipt) => {
 				self.block.archive_set.insert(h.unwrap_or_else(||t.hash()));
@@ -255,10 +265,19 @@ impl IsBlock for SealedBlock {
 
 /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header
 pub fn enact<'x, 'y>(block_bytes: &[u8], engine: &'x Engine, db: OverlayDB, parent: &Header, last_hashes: &'y LastHashes) -> Result<ClosedBlock<'x, 'y>, Error> {
+	{
+		let header = BlockView::new(block_bytes).header_view();
+		let s = State::from_existing(db.clone(), parent.state_root().clone(), engine.account_start_nonce());
+		trace!("enact(): root={}, author={}, author_balance={}\n", s.root(), header.author(), s.balance(&header.author()));
+	}
+
 	let block = BlockView::new(block_bytes);
 	let header = block.header_view();
 	let mut b = OpenBlock::new(engine, db, parent, last_hashes, header.author(), header.extra_data());
+	b.set_difficulty(header.difficulty());
+	b.set_gas_limit(header.gas_limit());
 	b.set_timestamp(header.timestamp());
+//	info!("enact: Enacting #{}. env_info={:?}", header.number(), b.env_info());
 	for t in block.transactions().into_iter() { try!(b.push_transaction(t, None)); }
 	for u in block.uncles().into_iter() { try!(b.push_uncle(u)); }
 	Ok(b.close())

src/client.rs

@@ -142,14 +142,13 @@ impl Client {
 			},
 		};
 		// build last hashes
-		let mut last = self.chain.read().unwrap().best_block_hash();
 		let mut last_hashes = LastHashes::new();
 		last_hashes.resize(256, H256::new());
+		last_hashes[0] = header.parent_hash.clone();
 		for i in 0..255 {
-			match self.chain.read().unwrap().block_details(&last) {
+			match self.chain.read().unwrap().block_details(&last_hashes[i]) {
 				Some(details) => {
 					last_hashes[i + 1] = details.parent.clone();
-					last = details.parent.clone();
 				},
 				None => break,
 			}

src/ethereum/ethash.rs

@@ -43,6 +43,8 @@ impl Engine for Ethash {
 				max(gas_floor_target, gas_limit - gas_limit / bound_divisor + x!(1) + (header.gas_used * x!(6) / x!(5)) / bound_divisor)
 			}
 		};
+
+//		info!("ethash: populate_from_parent #{}: difficulty={} and gas_limit={}", header.number, header.difficulty, header.gas_limit);
 	}
 
 	/// Apply the block reward on finalisation of the block.

src/evm/ext.rs

@@ -8,10 +8,10 @@ use env_info::*;
 
 pub trait Ext {
 	/// Returns a value for given key.
-	fn sload(&self, key: &H256) -> H256;
+	fn storage_at(&self, key: &H256) -> H256;
 
 	/// Stores a value for given key.
-	fn sstore(&mut self, key: H256, value: H256);
+	fn set_storage_at(&mut self, key: H256, value: H256);
 
 	/// Returns address balance.
 	fn balance(&self, address: &Address) -> U256;

src/evm/jit.rs

@@ -175,14 +175,14 @@ impl<'a> evmjit::Ext for ExtAdapter<'a> {
 	fn sload(&self, index: *const evmjit::I256, out_value: *mut evmjit::I256) {
 		unsafe {
 			let i = H256::from_jit(&*index);
-			let o = self.ext.sload(&i);
+			let o = self.ext.storage_at(&i);
 			*out_value = o.into_jit();
 		}
 	}
 
 	fn sstore(&mut self, index: *const evmjit::I256, value: *const evmjit::I256) {
 		unsafe {
-			self.ext.sstore(H256::from_jit(&*index), H256::from_jit(&*value));
+			self.ext.set_storage_at(H256::from_jit(&*index), H256::from_jit(&*value));
 		}
 	}
 

src/evm/tests.rs

@@ -26,11 +26,11 @@ impl FakeExt {
 }
 
 impl Ext for FakeExt {
-	fn sload(&self, key: &H256) -> H256 {
+	fn storage_at(&self, key: &H256) -> H256 {
 		self.store.get(key).unwrap_or(&H256::new()).clone()
 	}
 
-	fn sstore(&mut self, key: H256, value: H256) {
+	fn set_storage_at(&mut self, key: H256, value: H256) {
 		self.store.insert(key, value);
 	}
 

src/executive.rs

@@ -171,6 +171,7 @@ impl<'a> Executive<'a> {
 
 		// at first, transfer value to destination
 		self.state.transfer_balance(&params.sender, &params.address, &params.value);
+		debug!("Executive::call(params={:?}) self.env_info={:?}", params, self.info);
 
 		if self.engine.is_builtin(&params.code_address) {
 			// if destination is builtin, try to execute it
@@ -201,7 +202,11 @@ impl<'a> Executive<'a> {
 				let evm = Factory::create();
 				evm.exec(&params, &mut ext)
 			};
+
+			trace!("exec: sstore-clears={}\n", unconfirmed_substate.refunds_count);
+			trace!("exec: substate={:?}; unconfirmed_substate={:?}\n", substate, unconfirmed_substate);
 			self.enact_result(&res, substate, unconfirmed_substate, backup);
+			trace!("exec: new substate={:?}\n", substate);
 			res
 		} else {
 			// otherwise, nothing
@@ -242,20 +247,24 @@ impl<'a> Executive<'a> {
 		let sstore_refunds = U256::from(schedule.sstore_refund_gas) * substate.refunds_count;
 		// refunds from contract suicides
 		let suicide_refunds = U256::from(schedule.suicide_refund_gas) * U256::from(substate.suicides.len());
+		let refunds_bound = sstore_refunds + suicide_refunds;
 
 		// real ammount to refund
-		let gas_left = match &result { &Ok(x) => x, _ => x!(0) };
-		let refund = cmp::min(sstore_refunds + suicide_refunds, (t.gas - gas_left) / U256::from(2)) + gas_left;
-		let refund_value = refund * t.gas_price;
-		trace!("Refunding sender: sstore0s: {}, suicides: {}, gas_left: {}, refund: {}, refund_value: {}, sender: {}", sstore_refunds, suicide_refunds, gas_left, refund, refund_value, t.sender().unwrap());
+		let gas_left_prerefund = match &result { &Ok(x) => x, _ => x!(0) };
+		let refunded = cmp::min(refunds_bound, (t.gas - gas_left_prerefund) / U256::from(2));
+		let gas_left = gas_left_prerefund + refunded;
+
+		let gas_used = t.gas - gas_left;
+		let refund_value = gas_left * t.gas_price;
+		let fees_value = gas_used * t.gas_price;
+
+		trace!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n",
+			t.gas, sstore_refunds, suicide_refunds, refunds_bound, gas_left_prerefund, refunded, gas_left, gas_used, refund_value, fees_value);
+
+		trace!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, t.sender().unwrap());
 		self.state.add_balance(&t.sender().unwrap(), &refund_value);
-		
-		// fees earned by author
-		let fees = t.gas - refund;
-		let fees_value = fees * t.gas_price;
-		let author = &self.info.author;
-		self.state.add_balance(author, &fees_value);
-		trace!("Compensating author: fees: {}, fees_value: {}, author: {}", fees, fees_value, author);
+		trace!("exec::finalize: Compensating author: fees_value={}, author={}\n", fees_value, &self.info.author);
+		self.state.add_balance(&self.info.author, &fees_value);
 
 		// perform suicides
 		for address in substate.suicides.iter() {
@@ -263,30 +272,18 @@ impl<'a> Executive<'a> {
 			self.state.kill_account(address);
 		}
 
-		let gas_used = t.gas - gas_left;
-
 		match result { 
 			Err(evm::Error::Internal) => Err(ExecutionError::Internal),
-			Ok(_) => {
+			_ => {
 				Ok(Executed {
 					gas: t.gas,
 					gas_used: gas_used,
-					refunded: refund,
+					refunded: refunded,
 					cumulative_gas_used: self.info.gas_used + gas_used,
 					logs: substate.logs,
-					contracts_created: substate.contracts_created
+					contracts_created: substate.contracts_created,
 				})
 			},
-			_err => {
-				Ok(Executed {
-					gas: t.gas,
-					gas_used: t.gas,
-					refunded: U256::zero(),
-					cumulative_gas_used: self.info.gas_used + t.gas,
-					logs: vec![],
-					contracts_created: vec![]
-				})
-			}
 		}
 	}
 

src/externalities.rs

@@ -60,15 +60,19 @@ impl<'a> Externalities<'a> {
 }
 
 impl<'a> Ext for Externalities<'a> {
-	fn sload(&self, key: &H256) -> H256 {
+	fn storage_at(&self, key: &H256) -> H256 {
+		trace!("ext: storage_at({}, {}) == {}\n", self.params.address, key, U256::from(self.state.storage_at(&self.params.address, key).as_slice()));
 		self.state.storage_at(&self.params.address, key)
 	}
 
-	fn sstore(&mut self, key: H256, value: H256) {
+	fn set_storage_at(&mut self, key: H256, value: H256) {
+		let old = self.state.storage_at(&self.params.address, &key);
 		// if SSTORE nonzero -> zero, increment refund count
-		if value == H256::new() && self.state.storage_at(&self.params.address, &key) != H256::new() {
+		if value.is_zero() && !old.is_zero() {
+			trace!("ext: additional refund. {} -> {}\n", self.substate.refunds_count, self.substate.refunds_count + x!(1));
 			self.substate.refunds_count = self.substate.refunds_count + U256::one();
 		}
+		trace!("ext: set_storage_at({}, {}): {} -> {}\n", self.params.address, key, U256::from(old.as_slice()), U256::from(value.as_slice()));
 		self.state.set_storage(&self.params.address, key, value)
 	}
 
@@ -80,9 +84,14 @@ impl<'a> Ext for Externalities<'a> {
 		match *number < U256::from(self.info.number) && number.low_u64() >= cmp::max(256, self.info.number) - 256 {
 			true => {
 				let index = self.info.number - number.low_u64() - 1;
-				self.info.last_hashes[index as usize].clone()
+				let r = self.info.last_hashes[index as usize].clone();
+				trace!("ext: blockhash({}) -> {} self.info.number={}\n", number, r, self.info.number);
+				r
+			},
+			false => {
+				trace!("ext: blockhash({}) -> null self.info.number={}\n", number, self.info.number);
+				H256::from(&U256::zero())
 			},
-			false => H256::from(&U256::zero()),
 		}
 	}
 
@@ -124,6 +133,7 @@ impl<'a> Ext for Externalities<'a> {
 			data: &[u8], 
 			code_address: &Address, 
 			output: &mut [u8]) -> Result<(U256, bool), evm::Error> {
+
 		let mut gas_cost = *call_gas;
 		let mut call_gas = *call_gas;
 
@@ -138,7 +148,10 @@ impl<'a> Ext for Externalities<'a> {
 			call_gas = call_gas + U256::from(self.schedule.call_stipend);
 		}
 
+		debug!("Externalities::call(gas={}, call_gas={}, recv={}, value={}, data={}, code={})\n", gas, call_gas, receive_address, value, data.pretty(), code_address);
+
 		if gas_cost > *gas {
+			debug!("Externalities::call: OutOfGas gas_cost={}, gas={}", gas_cost, gas);
 			return Err(evm::Error::OutOfGas);
 		}
 
@@ -146,7 +159,8 @@ impl<'a> Ext for Externalities<'a> {
 
 		// if balance is insufficient or we are too deep, return
 		if self.state.balance(&self.params.address) < *value || self.depth >= self.schedule.max_depth {
-			return Ok((gas + call_gas, true));
+			debug!("Externalities::call: OutOfCash bal({})={}, value={}", self.params.address, self.state.balance(&self.params.address), value);
+			return Ok((gas + call_gas, false));
 		}
 
 		let params = ActionParams {
@@ -161,8 +175,13 @@ impl<'a> Ext for Externalities<'a> {
 			data: Some(data.to_vec()),
 		};
 
-		let mut ex = Executive::from_parent(self.state, self.info, self.engine, self.depth);
-		match ex.call(&params, self.substate, BytesRef::Fixed(output)) {
+
+		trace!("Externalities::call: BEFORE: bal({})={}, bal({})={}\n", params.sender, self.state.balance(&params.sender), params.address, self.state.balance(&params.address));
+		trace!("Externalities::call: CALLING: params={:?}\n", params);
+		let r = Executive::from_parent(self.state, self.info, self.engine, self.depth).call(&params, self.substate, BytesRef::Fixed(output));
+		trace!("Externalities::call: AFTER: bal({})={}, bal({})={}\n", params.sender, self.state.balance(&params.sender), params.address, self.state.balance(&params.address));
+
+		match r {
 			Ok(gas_left) => Ok((gas + gas_left, true)),
 			_ => Ok((gas, false))
 		}

src/header.rs

@@ -67,7 +67,9 @@ impl Header {
 
 	pub fn state_root(&self) -> &H256 { &self.state_root }
 	pub fn receipts_root(&self) -> &H256 { &self.receipts_root }
+	pub fn gas_limit(&self) -> &U256 { &self.gas_limit }
 
+	pub fn difficulty(&self) -> &U256 { &self.difficulty }
 	pub fn seal(&self) -> &Vec<Bytes> { &self.seal }
 
 	// TODO: seal_at, set_seal_at &c.
@@ -79,6 +81,10 @@ impl Header {
 
 	pub fn set_extra_data(&mut self, a: Bytes) { if a != self.extra_data { self.extra_data = a; self.note_dirty(); } }
 
+	pub fn set_gas_used(&mut self, a: U256) { self.gas_used = a; self.note_dirty(); }
+	pub fn set_gas_limit(&mut self, a: U256) { self.gas_limit = a; self.note_dirty(); }
+
+	pub fn set_difficulty(&mut self, a: U256) { self.difficulty = a; self.note_dirty(); }
 	pub fn set_seal(&mut self, a: Vec<Bytes>) { self.seal = a; self.note_dirty(); }
 
 	/// Get the hash of this header (sha3 of the RLP).

src/state.rs

@@ -3,6 +3,7 @@ use engine::Engine;
 use executive::Executive;
 use pod_account::*;
 use pod_state::*;
+use state_diff::*;
 
 pub type ApplyResult = Result<Receipt, Error>;
 
@@ -106,12 +107,16 @@ impl State {
 
 	/// Add `incr` to the balance of account `a`.
 	pub fn add_balance(&mut self, a: &Address, incr: &U256) {
-		self.require(a, false).add_balance(incr)
+		let old = self.balance(a);
+		self.require(a, false).add_balance(incr);
+		trace!("state: add_balance({}, {}): {} -> {}\n", a, incr, old, self.balance(a));
 	}
 
 	/// Subtract `decr` from the balance of account `a`.
 	pub fn sub_balance(&mut self, a: &Address, decr: &U256) {
-		self.require(a, false).sub_balance(decr)
+		let old = self.balance(a);
+		self.require(a, false).sub_balance(decr);
+		trace!("state: sub_balance({}, {}): {} -> {}\n", a, decr, old, self.balance(a));
 	}
 
 	/// Subtracts `by` from the balance of `from` and adds it to that of `to`.
@@ -139,8 +144,13 @@ impl State {
 	/// Execute a given transaction.
 	/// This will change the state accordingly.
 	pub fn apply(&mut self, env_info: &EnvInfo, engine: &Engine, t: &Transaction) -> ApplyResult {
+
+		let old = self.to_pod();
+
 		let e = try!(Executive::new(self, env_info, engine).transact(t));
 		//println!("Executed: {:?}", e);
+
+		debug!("Applied transaction. Diff:\n{}\n", StateDiff::diff_pod(&old, &self.to_pod()));
 		self.commit();
 		let receipt = Receipt::new(self.root().clone(), e.cumulative_gas_used, e.logs);
 		debug!("Transaction receipt: {:?}", receipt);
@@ -219,8 +229,9 @@ impl State {
 	/// Pull account `a` in our cache from the trie DB and return it.
 	/// `require_code` requires that the code be cached, too.
 	fn get(&self, a: &Address, require_code: bool) -> Ref<Option<Account>> {
-		self.cache.borrow_mut().entry(a.clone()).or_insert_with(||
-			SecTrieDB::new(&self.db, &self.root).get(&a).map(|rlp| Account::from_rlp(rlp)));
+		self.cache.borrow_mut().entry(a.clone()).or_insert_with(|| {
+			SecTrieDB::new(&self.db, &self.root).get(&a).map(|rlp| Account::from_rlp(rlp))
+		});
 		if require_code {
 			if let Some(ref mut account) = self.cache.borrow_mut().get_mut(a).unwrap().as_mut() {
 				account.cache_code(&self.db);

src/substate.rs

@@ -2,6 +2,7 @@ use common::*;
 
 /// State changes which should be applied in finalize,
 /// after transaction is fully executed.
+#[derive(Debug)]
 pub struct Substate {
 	/// Any accounts that have suicided.
 	pub suicides: HashSet<Address>,

src/tests/executive.rs

@@ -57,12 +57,12 @@ impl<'a> TestExt<'a> {
 }
 
 impl<'a> Ext for TestExt<'a> {
-	fn sload(&self, key: &H256) -> H256 {
-		self.ext.sload(key)
+	fn storage_at(&self, key: &H256) -> H256 {
+		self.ext.storage_at(key)
 	}
 
-	fn sstore(&mut self, key: H256, value: H256) {
-		self.ext.sstore(key, value)
+	fn set_storage_at(&mut self, key: H256, value: H256) {
+		self.ext.set_storage_at(key, value)
 	}
 
 	fn balance(&self, address: &Address) -> U256 {

src/verification.rs

@@ -115,18 +115,18 @@ pub fn verify_block_family<BC>(header: &Header, bytes: &[u8], engine: &Engine, b
 
 /// Phase 4 verification. Check block information against transaction enactment results,
 pub fn verify_block_final(expected: &Header, got: &Header) -> Result<(), Error> {
-	if expected.state_root != got.state_root {
-		return Err(From::from(BlockError::InvalidStateRoot(Mismatch { expected: expected.state_root.clone(), found: got.state_root.clone() })))
+	if expected.gas_used != got.gas_used {
+		return Err(From::from(BlockError::InvalidGasUsed(Mismatch { expected: expected.gas_used, found: got.gas_used })))
 	}
 	if expected.receipts_root != got.receipts_root {
 		return Err(From::from(BlockError::InvalidReceiptsStateRoot(Mismatch { expected: expected.receipts_root.clone(), found: got.receipts_root.clone() })))
 	}
+	if expected.state_root != got.state_root {
+		return Err(From::from(BlockError::InvalidStateRoot(Mismatch { expected: expected.state_root.clone(), found: got.state_root.clone() })))
+	}
 	if expected.log_bloom != got.log_bloom {
 		return Err(From::from(BlockError::InvalidLogBloom(Mismatch { expected: expected.log_bloom.clone(), found: got.log_bloom.clone() })))
 	}
-	if expected.gas_used != got.gas_used {
-		return Err(From::from(BlockError::InvalidGasUsed(Mismatch { expected: expected.gas_used, found: got.gas_used })))
-	}
 	Ok(())
 }