ExecutedBlock cleanup

ethcore/src/block.rs

@@ -35,7 +35,7 @@ use header::{Header, Seal};
 use receipt::{Receipt, TransactionOutcome};
 use state::State;
 use state_db::StateDB;
-use trace::FlatTrace;
+use trace::Tracing;
 use transaction::{UnverifiedTransaction, SignedTransaction, Error as TransactionError};
 use verification::PreverifiedBlock;
 use views::BlockView;
@@ -93,53 +93,10 @@ pub struct ExecutedBlock {
 	receipts: Vec<Receipt>,
 	transactions_set: HashSet<H256>,
 	state: State<StateDB>,
-	traces: Option<Vec<Vec<FlatTrace>>>,
+	traces: Tracing,
 	last_hashes: Arc<LastHashes>,
 }
 
-/// A set of references to `ExecutedBlock` fields that are publicly accessible.
-pub struct BlockRefMut<'a> {
-	/// Block header.
-	pub header: &'a mut Header,
-	/// Block transactions.
-	pub transactions: &'a [SignedTransaction],
-	/// Block uncles.
-	pub uncles: &'a [Header],
-	/// Transaction receipts.
-	pub receipts: &'a [Receipt],
-	/// State.
-	pub state: &'a mut State<StateDB>,
-	/// Traces.
-	pub traces: &'a mut Option<Vec<Vec<FlatTrace>>>,
-}
-
-impl<'a> BlockRefMut<'a> {
-	/// Add traces if tracing is enabled.
-	pub fn push_traces(&mut self, tracer: ::trace::ExecutiveTracer) {
-		use trace::Tracer;
-
-		if let Some(ref mut traces) = self.traces.as_mut() {
-			traces.push(tracer.drain())
-		}
-	}
-}
-
-/// A set of immutable references to `ExecutedBlock` fields that are publicly accessible.
-pub struct BlockRef<'a> {
-	/// Block header.
-	pub header: &'a Header,
-	/// Block transactions.
-	pub transactions: &'a [SignedTransaction],
-	/// Block uncles.
-	pub uncles: &'a [Header],
-	/// Transaction receipts.
-	pub receipts: &'a [Receipt],
-	/// State.
-	pub state: &'a State<StateDB>,
-	/// Traces.
-	pub traces: &'a Option<Vec<Vec<FlatTrace>>>,
-}
-
 impl ExecutedBlock {
 	/// Create a new block from the given `state`.
 	fn new(state: State<StateDB>, last_hashes: Arc<LastHashes>, tracing: bool) -> ExecutedBlock {
@@ -150,35 +107,15 @@ impl ExecutedBlock {
 			receipts: Default::default(),
 			transactions_set: Default::default(),
 			state: state,
-			traces: if tracing {Some(Vec::new())} else {None},
+			traces: if tracing {
+				Tracing::enabled()
+			} else {
+				Tracing::Disabled
+			},
 			last_hashes: last_hashes,
 		}
 	}
 
-	/// Get a structure containing individual references to all public fields.
-	pub fn fields_mut(&mut self) -> BlockRefMut {
-		BlockRefMut {
-			header: &mut self.header,
-			transactions: &self.transactions,
-			uncles: &self.uncles,
-			state: &mut self.state,
-			receipts: &self.receipts,
-			traces: &mut self.traces,
-		}
-	}
-
-	/// Get a structure containing individual references to all public fields.
-	pub fn fields(&self) -> BlockRef {
-		BlockRef {
-			header: &self.header,
-			transactions: &self.transactions,
-			uncles: &self.uncles,
-			state: &self.state,
-			receipts: &self.receipts,
-			traces: &self.traces,
-		}
-	}
-
 	/// Get the environment info concerning this block.
 	pub fn env_info(&self) -> EnvInfo {
 		// TODO: memoise.
@@ -192,6 +129,16 @@ impl ExecutedBlock {
 			gas_limit: self.header.gas_limit().clone(),
 		}
 	}
+
+	/// Get mutable access to a state.
+	pub fn state_mut(&mut self) -> &mut State<StateDB> {
+		&mut self.state
+	}
+
+	/// Get mutable reference to traces.
+	pub fn traces_mut(&mut self) -> &mut Tracing {
+		&mut self.traces
+	}
 }
 
 /// Trait for a object that is a `ExecutedBlock`.
@@ -221,13 +168,13 @@ pub trait IsBlock {
 	fn receipts(&self) -> &[Receipt] { &self.block().receipts }
 
 	/// Get all information concerning transaction tracing in this block.
-	fn traces(&self) -> &Option<Vec<Vec<FlatTrace>>> { &self.block().traces }
+	fn traces(&self) -> &Tracing { &self.block().traces }
 
 	/// Get all uncles in this block.
 	fn uncles(&self) -> &[Header] { &self.block().uncles }
 
 	/// Get tracing enabled flag for this block.
-	fn tracing_enabled(&self) -> bool { self.block().traces.is_some() }
+	fn tracing_enabled(&self) -> bool { self.block().traces.is_enabled() }
 }
 
 /// Trait for a object that has a state database.
@@ -405,12 +352,14 @@ impl<'x> OpenBlock<'x> {
 
 		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.machine(), &t, self.block.traces.is_some()) {
+		match self.block.state.apply(&env_info, self.engine.machine(), &t, self.block.traces.is_enabled()) {
 			Ok(outcome) => {
 				self.block.transactions_set.insert(h.unwrap_or_else(||t.hash()));
 				self.block.transactions.push(t.into());
 				let t = outcome.trace;
-				self.block.traces.as_mut().map(|traces| traces.push(t));
+				if let Tracing::Enabled(ref mut traces) = self.block.traces {
+					traces.push(t.into());
+				}
 				self.block.receipts.push(outcome.receipt);
 				Ok(self.block.receipts.last().expect("receipt just pushed; qed"))
 			}

ethcore/src/client/client.rs

@@ -63,7 +63,6 @@ use state_db::StateDB;
 use state::{self, State};
 use trace;
 use trace::{TraceDB, ImportRequest as TraceImportRequest, LocalizedTrace, Database as TraceDatabase};
-use trace::FlatTransactionTraces;
 use transaction::{self, LocalizedTransaction, UnverifiedTransaction, SignedTransaction, Transaction, PendingTransaction, Action};
 use types::filter::Filter;
 use types::mode::Mode as IpcMode;
@@ -651,10 +650,7 @@ impl Client {
 
 		// Commit results
 		let receipts = block.receipts().to_owned();
-		let traces = block.traces().clone().unwrap_or_else(Vec::new);
-		let traces: Vec<FlatTransactionTraces> = traces.into_iter()
-			.map(Into::into)
-			.collect();
+		let traces = block.traces().clone().drain();
 
 		assert_eq!(header.hash(), BlockView::new(block_data).header_view().hash());
 

ethcore/src/engines/authority_round/mod.rs

@@ -947,7 +947,7 @@ impl Engine<EthereumMachine> for AuthorityRound {
 		if self.immediate_transitions || !epoch_begin { return Ok(()) }
 
 		// genesis is never a new block, but might as well check.
-		let header = block.fields().header.clone();
+		let header = block.header().clone();
 		let first = header.number() == 0;
 
 		let mut call = |to, data| {
@@ -966,37 +966,45 @@ impl Engine<EthereumMachine> for AuthorityRound {
 
 	/// Apply the block reward on finalisation of the block.
 	fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> {
+		use parity_machine::WithBalances;
+
+		let mut rewards = Vec::new();
 		if block.header().number() >= self.empty_steps_transition {
-			let empty_steps =
-				if block.header().seal().is_empty() {
-					// this is a new block, calculate rewards based on the empty steps messages we have accumulated
-					let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) {
-						Some(client) => client,
-						None => {
-							debug!(target: "engine", "Unable to close block: missing client ref.");
-							return Err(EngineError::RequiresClient.into())
-						},
-					};
-
-					let parent = client.block_header(::client::BlockId::Hash(*block.header().parent_hash()))
-						.expect("hash is from parent; parent header must exist; qed")
-						.decode();
-
-					let parent_step = header_step(&parent, self.empty_steps_transition)?;
-					let current_step = self.step.load();
-					self.empty_steps(parent_step.into(), current_step.into(), parent.hash())
-				} else {
-					// we're verifying a block, extract empty steps from the seal
-					header_empty_steps(block.header())?
+			let empty_steps = if block.header().seal().is_empty() {
+				// this is a new block, calculate rewards based on the empty steps messages we have accumulated
+				let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) {
+					Some(client) => client,
+					None => {
+						debug!(target: "engine", "Unable to close block: missing client ref.");
+						return Err(EngineError::RequiresClient.into())
+					},
 				};
 
+				let parent = client.block_header(::client::BlockId::Hash(*block.header().parent_hash()))
+					.expect("hash is from parent; parent header must exist; qed")
+					.decode();
+
+				let parent_step = header_step(&parent, self.empty_steps_transition)?;
+				let current_step = self.step.load();
+				self.empty_steps(parent_step.into(), current_step.into(), parent.hash())
+			} else {
+				// we're verifying a block, extract empty steps from the seal
+				header_empty_steps(block.header())?
+			};
+
 			for empty_step in empty_steps {
-				::engines::common::bestow_block_reward(block, self.block_reward, empty_step.author()?)?;
+				let author = empty_step.author()?;
+				rewards.push((author, self.block_reward));
 			}
 		}
 
 		let author = *block.header().author();
-		::engines::common::bestow_block_reward(block, self.block_reward, author)
+		rewards.push((author, self.block_reward));
+
+		for &(ref author, ref block_reward) in rewards.iter() {
+			self.machine.add_balance(block, author, block_reward)?;
+		}
+		self.machine.note_rewards(block, &rewards, &[])
 	}
 
 	/// Check the number of seal fields.
@@ -1790,13 +1798,13 @@ mod tests {
 		// step 3
 		// the signer of the accumulated empty step message should be rewarded
 		let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap();
-		let addr1_balance = b2.block().fields().state.balance(&addr1).unwrap();
+		let addr1_balance = b2.block().state().balance(&addr1).unwrap();
 
 		// after closing the block `addr1` should be reward twice, one for the included empty step message and another for block creation
 		let b2 = b2.close_and_lock();
 
 		// the spec sets the block reward to 10
-		assert_eq!(b2.block().fields().state.balance(&addr1).unwrap(), addr1_balance + (10 * 2).into())
+		assert_eq!(b2.block().state().balance(&addr1).unwrap(), addr1_balance + (10 * 2).into())
 	}
 
 	#[test]

ethcore/src/engines/mod.rs

@@ -393,35 +393,3 @@ pub trait EthEngine: Engine<::machine::EthereumMachine> {
 
 // convenience wrappers for existing functions.
 impl<T> EthEngine for T where T: Engine<::machine::EthereumMachine> { }
-
-/// Common engine utilities
-pub mod common {
-	use block::ExecutedBlock;
-	use error::Error;
-	use trace::{Tracer, ExecutiveTracer, RewardType};
-	use state::CleanupMode;
-
-	use ethereum_types::{Address, U256};
-
-	/// Give reward and trace.
-	pub fn bestow_block_reward(block: &mut ExecutedBlock, reward: U256, receiver: Address) -> Result<(), Error> {
-		let fields = block.fields_mut();
-
-		// Bestow block reward
-		let res = fields.state.add_balance(&receiver, &reward, CleanupMode::NoEmpty)
-			.map_err(::error::Error::from)
-			.and_then(|_| fields.state.commit());
-
-		fields.traces.as_mut().map(move |traces| {
-  			let mut tracer = ExecutiveTracer::default();
-  			tracer.trace_reward(receiver, reward, RewardType::Block);
-  			traces.push(tracer.drain())
-		});
-
-		if let Err(ref e) = res {
-			warn!("Encountered error on bestowing reward: {}", e);
-		}
-
-		res
-	}
-}

ethcore/src/engines/tendermint/mod.rs

@@ -542,7 +542,7 @@ impl Engine<EthereumMachine> for Tendermint {
 		if !epoch_begin { return Ok(()) }
 
 		// genesis is never a new block, but might as well check.
-		let header = block.fields().header.clone();
+		let header = block.header().clone();
 		let first = header.number() == 0;
 
 		let mut call = |to, data| {
@@ -561,8 +561,10 @@ impl Engine<EthereumMachine> for Tendermint {
 
 	/// Apply the block reward on finalisation of the block.
 	fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error>{
+		use parity_machine::WithBalances;
 		let author = *block.header().author();
-		::engines::common::bestow_block_reward(block, self.block_reward, author)
+		self.machine.add_balance(block, &author, &self.block_reward)?;
+		self.machine.note_rewards(block, &[(author, self.block_reward)], &[])
 	}
 
 	fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> {