Fixed comments after the review

ethcore/res/null_morden.json

@@ -8,8 +8,7 @@
 		"accountStartNonce": "0x0",
 		"maximumExtraDataSize": "0x20",
 		"minGasLimit": "0x1388",
-		"networkID" : "0x2",
+		"networkID" : "0x2"
-		"applyReward": false
 	},
 	"genesis": {
 		"seal": {

ethcore/res/null_morden_with_reward.json

@@ -9,7 +9,7 @@
 		"maximumExtraDataSize": "0x20",
 		"minGasLimit": "0x1388",
 		"networkID" : "0x2",
-		"applyReward": true
+		"blockReward": "0x4563918244F40000"
 	},
 	"genesis": {
 		"seal": {

ethcore/src/block.rs

@@ -21,7 +21,7 @@ use std::sync::Arc;
 use std::collections::HashSet;
 
 use rlp::{UntrustedRlp, RlpStream, Encodable, Decodable, DecoderError};
-use util::{Bytes, Address, Hashable, U256, H256, ordered_trie_root, SHA3_NULL_RLP};
+use util::{Bytes, Address, Hashable, U256, H256, ordered_trie_root, SHA3_NULL_RLP, SHA3_EMPTY_LIST_RLP};
 use util::error::{Mismatch, OutOfBounds};
 
 use basic_types::{LogBloom, Seal};
@@ -91,8 +91,7 @@ pub struct ExecutedBlock {
 	receipts: Vec<Receipt>,
 	transactions_set: HashSet<H256>,
 	state: State<StateDB>,
-	traces: Option<Vec<Vec<FlatTrace>>>,
+	traces: Option<Vec<Vec<FlatTrace>>>
-	tracing_enabled: bool,
 }
 
 /// A set of references to `ExecutedBlock` fields that are publicly accessible.
@@ -109,8 +108,6 @@ pub struct BlockRefMut<'a> {
 	pub state: &'a mut State<StateDB>,
 	/// Traces.
 	pub traces: &'a Option<Vec<Vec<FlatTrace>>>,
-	/// Tracing enabled flag.
-	pub tracing_enabled: &'a mut bool,
 }
 
 /// A set of immutable references to `ExecutedBlock` fields that are publicly accessible.
@@ -127,8 +124,6 @@ pub struct BlockRef<'a> {
 	pub state: &'a State<StateDB>,
 	/// Traces.
 	pub traces: &'a Option<Vec<Vec<FlatTrace>>>,
-	/// Tracing enabled flag.
-	pub tracing_enabled: &'a bool,
 }
 
 impl ExecutedBlock {
@@ -142,7 +137,6 @@ impl ExecutedBlock {
 			transactions_set: Default::default(),
 			state: state,
 			traces: if tracing {Some(Vec::new())} else {None},
-			tracing_enabled: tracing,
 		}
 	}
 
@@ -155,7 +149,6 @@ impl ExecutedBlock {
 			state: &mut self.state,
 			receipts: &self.receipts,
 			traces: &self.traces,
-			tracing_enabled: &mut self.tracing_enabled,
 		}
 	}
 
@@ -168,7 +161,6 @@ impl ExecutedBlock {
 			state: &self.state,
 			receipts: &self.receipts,
 			traces: &self.traces,
-			tracing_enabled: &self.tracing_enabled,
 		}
 	}
 }
@@ -206,7 +198,7 @@ pub trait IsBlock {
 	fn uncles(&self) -> &[Header] { &self.block().uncles }
 
 	/// Get tracing enabled flag for this block.
-	fn tracing_enabled(&self) -> bool { self.block().tracing_enabled }
+	fn tracing_enabled(&self) -> bool { self.block().traces.is_some() }
 }
 
 /// Trait for a object that has a state database.
@@ -404,12 +396,9 @@ impl<'x> OpenBlock<'x> {
 		let unclosed_state = s.block.state.clone();
 
 		match s.engine.on_close_block(&mut s.block) {
-			Ok(outcome) => match outcome.trace {
+			Ok(outcome) => if let Some(t) = outcome.trace {
-				Some(t) => {
 				s.block.traces.as_mut().map(|traces| traces.push(t));
 			},
-				None => {},
-			},
 			Err(e) => warn!("Encountered error on closing the block: {}", e),
 		}
 		
@@ -453,7 +442,7 @@ impl<'x> OpenBlock<'x> {
 			s.block.header.set_transactions_root(ordered_trie_root(s.block.transactions.iter().map(|e| e.rlp_bytes().into_vec())));
 		}
 		let uncle_bytes = s.block.uncles.iter().fold(RlpStream::new_list(s.block.uncles.len()), |mut s, u| {s.append_raw(&u.rlp(Seal::With), 1); s} ).out();
-		if s.block.header.uncles_hash().is_zero() || s.block.header.receipts_root() == &SHA3_NULL_RLP {
+		if s.block.header.uncles_hash().is_zero() || s.block.header.uncles_hash() == &SHA3_EMPTY_LIST_RLP {
 			s.block.header.set_uncles_hash(uncle_bytes.sha3());
 		}
 		if s.block.header.receipts_root().is_zero() || s.block.header.receipts_root() == &SHA3_NULL_RLP {

ethcore/src/engines/authority_round/mod.rs

@@ -545,6 +545,7 @@ impl Engine for AuthorityRound {
 
 	/// Apply the block reward on finalisation of the block.
 	fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<CloseOutcome, Error> {
+		let tracing_enabled = block.tracing_enabled();
 		let fields = block.fields_mut();
 		let mut tracer = ExecutiveTracer::default();
 		// Bestow block reward
@@ -553,18 +554,19 @@ impl Engine for AuthorityRound {
 			.map_err(::error::Error::from)
 			.and_then(|_| fields.state.commit());
 
-		// Trace it
+		if tracing_enabled {
-		let block_miner = fields.header.author().clone();
+			let block_author = fields.header.author().clone();
-		tracer.trace_reward(block_miner, self.params().block_reward, RewardType::Block);
+			tracer.trace_reward(block_author, self.params().block_reward, RewardType::Block);
+		}
 
 		// Commit state so that we can actually figure out the state root.
 		if let Err(ref e) = res {
 			warn!("Encountered error on closing block: {}", e);
 		}
 		match res {
-			Ok(_) => match *fields.tracing_enabled {
+			Ok(_) => match tracing_enabled {
-				true => Ok(CloseOutcome{trace: Some(tracer.traces())}),
+				true => Ok(CloseOutcome { trace: Some(tracer.traces()) } ),
-				false => Ok(CloseOutcome{trace: None})
+				false => Ok(CloseOutcome { trace: None } )
 			},
 			Err(e) => Err(e)
 		}

ethcore/src/engines/null_engine.rs

@@ -17,7 +17,7 @@
 use std::collections::BTreeMap;
 use util::Address;
 use builtin::Builtin;
-use block::{ExecutedBlock, };
+use block::{ExecutedBlock, IsBlock};
 use util::U256;
 use engines::{Engine, CloseOutcome};
 use spec::CommonParams;
@@ -71,11 +71,12 @@ impl Engine for NullEngine {
 	}
 
 	fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<CloseOutcome, Error> {
-		if !self.params.apply_reward {
+		if self.params.block_reward == U256::zero() {
 			return Ok(CloseOutcome{trace: None});
 		}
 
 		/// Block reward
+		let tracing_enabled = block.tracing_enabled();
 		let fields = block.fields_mut();		
 		let mut tracer = ExecutiveTracer::default();
 
@@ -86,25 +87,27 @@ impl Engine for NullEngine {
 			CleanupMode::NoEmpty
 		)?;
 
-		let block_miner = fields.header.author().clone();
+		if tracing_enabled {
-		tracer.trace_reward(block_miner, result_block_reward, RewardType::Block);
+			let block_author = fields.header.author().clone();
+			tracer.trace_reward(block_author, result_block_reward, RewardType::Block);
+		}
 
 		/// Uncle rewards
 		let result_uncle_reward = U256::from(10000000);
 		for u in fields.uncles.iter() {
-			let uncle_miner = u.author().clone();
+			let uncle_author = u.author().clone();
 			fields.state.add_balance(
 				u.author(),
 				&(result_uncle_reward),
 				CleanupMode::NoEmpty
 			)?;
-			tracer.trace_reward(uncle_miner, result_uncle_reward, RewardType::Uncle);
+			tracer.trace_reward(uncle_author, result_uncle_reward, RewardType::Uncle);
 		}
 
 		fields.state.commit()?;
-		match *fields.tracing_enabled {
+		match tracing_enabled {
-			true => Ok(CloseOutcome{trace: Some(tracer.traces())}),
+			true => Ok(CloseOutcome { trace: Some(tracer.traces()) } ),
-			false => Ok(CloseOutcome{trace: None})
+			false => Ok(CloseOutcome { trace: None } )
 		}
 	}
 }

ethcore/src/engines/tendermint/mod.rs

@@ -540,6 +540,7 @@ impl Engine for Tendermint {
 
 	/// Apply the block reward on finalisation of the block.
 	fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<CloseOutcome, Error>{
+		let tracing_enabled = block.tracing_enabled();
 		let fields = block.fields_mut();
 		let mut tracer = ExecutiveTracer::default();
 		// Bestow block reward
@@ -548,22 +549,22 @@ impl Engine for Tendermint {
 			.map_err(::error::Error::from)
 			.and_then(|_| fields.state.commit());
 
-		// Trace it
+		if tracing_enabled {
-		let block_miner = fields.header.author().clone();
+			let block_author = fields.header.author().clone();
-		tracer.trace_reward(block_miner, self.params().block_reward, RewardType::Block);
+			tracer.trace_reward(block_author, self.params().block_reward, RewardType::Block);
+		}
 
 		// Commit state so that we can actually figure out the state root.
 		if let Err(ref e) = res {
 			warn!("Encountered error on closing block: {}", e);
 		}
 		match res {
-			Ok(_) => match *fields.tracing_enabled {
+			Ok(_) => match tracing_enabled {
-				true => Ok(CloseOutcome{trace: Some(tracer.traces())}),
+				true => Ok(CloseOutcome { trace: Some(tracer.traces()) } ),
-				false => Ok(CloseOutcome{trace: None})
+				false => Ok(CloseOutcome { trace: None } )
 			},
 			Err(e) => Err(e)
 		}
-
 	}
 
 	fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {

ethcore/src/ethereum/ethash.rs

@@ -274,6 +274,7 @@ impl Engine for Arc<Ethash> {
 	/// This assumes that all uncles are valid uncles (i.e. of at least one generation before the current).
 	fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<CloseOutcome, Error> {
 		let reward = self.params().block_reward;
+		let tracing_enabled = block.tracing_enabled();
 		let fields = block.fields_mut();
 		let eras_rounds = self.ethash_params.ecip1017_era_rounds;
 		let (eras, reward) = ecip1017_eras_block_reward(eras_rounds, reward, fields.header.number());
@@ -287,14 +288,15 @@ impl Engine for Arc<Ethash> {
 			CleanupMode::NoEmpty
 		)?;
 
-		// Trace it
+		if tracing_enabled {
-		let block_miner = fields.header.author().clone();
+			let block_author = fields.header.author().clone();
-		tracer.trace_reward(block_miner, result_block_reward, RewardType::Block);
+			tracer.trace_reward(block_author, result_block_reward, RewardType::Block);
+		}
 
 		// Bestow uncle rewards
 		let current_number = fields.header.number();
 		for u in fields.uncles.iter() {
-			let uncle_miner = u.author().clone();
+			let uncle_author = u.author().clone();
 			let result_uncle_reward: U256;
 
 			if eras == 0 {
@@ -314,14 +316,16 @@ impl Engine for Arc<Ethash> {
 			}?;
 
 			// Trace uncle rewards
-			tracer.trace_reward(uncle_miner, result_uncle_reward, RewardType::Uncle);
+			if tracing_enabled {
+				tracer.trace_reward(uncle_author, result_uncle_reward, RewardType::Uncle);
+			}			
 		}
 
 		// Commit state so that we can actually figure out the state root.
 		fields.state.commit()?;
-		match *fields.tracing_enabled {
+		match tracing_enabled {
-			true => Ok(CloseOutcome{trace: Some(tracer.traces())}),
+			true => Ok(CloseOutcome { trace: Some(tracer.traces()) } ),
-			false => Ok(CloseOutcome{trace: None})
+			false => Ok(CloseOutcome { trace: None } )
 		}		
 	}
 

ethcore/src/spec/spec.rs

@@ -98,8 +98,6 @@ pub struct CommonParams {
 	pub block_reward: U256,
 	/// Registrar contract address.
 	pub registrar: Address,
-	/// Apply reward
-	pub apply_reward: bool,
 }
 
 impl CommonParams {
@@ -170,8 +168,7 @@ impl From<ethjson::spec::Params> for CommonParams {
 			wasm: p.wasm.unwrap_or(false),
 			gas_limit_bound_divisor: p.gas_limit_bound_divisor.into(),
 			block_reward: p.block_reward.map_or_else(U256::zero, Into::into),
-			registrar: p.registrar.map_or_else(Address::new, Into::into),
+			registrar: p.registrar.map_or_else(Address::new, Into::into)
-			apply_reward: p.apply_reward.unwrap_or(true),
 		}
 	}
 }

ethcore/src/trace/db.rs

@@ -215,20 +215,11 @@ impl<T> TraceDB<T> where T: DatabaseExtras {
 		block_number: BlockNumber,
 		tx_number: usize
 	) -> Vec<LocalizedTrace> {
-		let trace_tx_number;
+		let (trace_tx_number, trace_tx_hash) = match self.extras.transaction_hash(block_number, tx_number) {
-		let trace_tx_hash;
+			Some(hash) => (Some(tx_number), Some(hash.clone())),
-
-		match self.extras.transaction_hash(block_number, tx_number) {
-			Some(hash) => {
-				trace_tx_hash = Some(hash.clone());
-				trace_tx_number = Some(tx_number);
-			},
-			None => {
 			//None means trace without transaction (reward)
-				trace_tx_hash = None;
+			None => (None, None),
-				trace_tx_number = None;				
+		};
-			}
-		}
 
 		let flat_traces: Vec<FlatTrace> = traces.into();
 		flat_traces.into_iter()
@@ -375,20 +366,11 @@ impl<T> TraceDatabase for TraceDB<T> where T: DatabaseExtras {
 						.map(Into::<Vec<FlatTrace>>::into)
 						.enumerate()
 						.flat_map(|(tx_position, traces)| {
-							let trace_tx_number;
+							let (trace_tx_number, trace_tx_hash) = match self.extras.transaction_hash(block_number, tx_position) {
-							let trace_tx_hash;
+								Some(hash) => (Some(tx_position), Some(hash.clone())),
-
-							match self.extras.transaction_hash(block_number, tx_position) {
-								Some(hash) => {
-									trace_tx_hash = Some(hash.clone());
-									trace_tx_number = Some(tx_position);
-								},
-								None => {
 								//None means trace without transaction (reward)
-									trace_tx_hash = None;
+								None => (None, None),
-									trace_tx_number = None;				
+							};
-								}
-							}
 
 							traces.into_iter()
 								.map(|trace| LocalizedTrace {

ethcore/src/trace/executive_tracer.rs

@@ -159,7 +159,7 @@ impl Tracer for ExecutiveTracer {
 			result: Res::None,
 			trace_address: Default::default(),
 		};
-		debug!(target: "trace", "Traced failed suicide {:?}", trace);
+		debug!(target: "trace", "Traced suicide {:?}", trace);
 		self.traces.push(trace);
 	}
 	
@@ -174,7 +174,7 @@ impl Tracer for ExecutiveTracer {
 			result: Res::None,
 			trace_address: Default::default(),
 		};
-		debug!(target: "trace", "Traced failed reward {:?}", trace);
+		debug!(target: "trace", "Traced reward {:?}", trace);
 		self.traces.push(trace);
 	}
 

ethcore/src/verification/verification.rs

@@ -591,7 +591,6 @@ mod tests {
 		let mut params = CommonParams::default();
 		params.dust_protection_transition = 0;
 		params.nonce_cap_increment = 2;
-		params.apply_reward = false;
 
 		let mut header = Header::default();
 		header.set_number(1);