ExecutedBlock cleanup (#7991)

* ExecutedBlock cleanup

* authority round makes only one call to note_rewards

* move Tracing from block to trace module

* removed BlockRef
This commit is contained in:
Marek Kotewicz 2018-02-27 18:22:56 +01:00 committed by GitHub
parent 6445f12c2d
commit df63341eb4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 138 additions and 183 deletions

View File

@ -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"))
}

View File

@ -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());

View File

@ -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]

View File

@ -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
}
}

View File

@ -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> {

View File

@ -20,7 +20,7 @@ use std::collections::{BTreeMap, HashMap};
use std::cmp;
use std::sync::Arc;
use block::ExecutedBlock;
use block::{ExecutedBlock, IsBlock};
use builtin::Builtin;
use client::BlockChainClient;
use error::Error;
@ -28,7 +28,7 @@ use executive::Executive;
use header::{BlockNumber, Header};
use spec::CommonParams;
use state::{CleanupMode, Substate};
use trace::{NoopTracer, NoopVMTracer, Tracer, ExecutiveTracer, RewardType};
use trace::{NoopTracer, NoopVMTracer, Tracer, ExecutiveTracer, RewardType, Tracing};
use transaction::{self, SYSTEM_ADDRESS, UnverifiedTransaction, SignedTransaction};
use tx_filter::TransactionFilter;
@ -135,7 +135,7 @@ impl EthereumMachine {
env_info
};
let mut state = block.fields_mut().state;
let mut state = block.state_mut();
let params = ActionParams {
code_address: contract_address.clone(),
address: contract_address.clone(),
@ -163,12 +163,12 @@ impl EthereumMachine {
/// Push last known block hash to the state.
fn push_last_hash(&self, block: &mut ExecutedBlock) -> Result<(), Error> {
let params = self.params();
if block.fields().header.number() == params.eip210_transition {
let state = block.fields_mut().state;
if block.header().number() == params.eip210_transition {
let state = block.state_mut();
state.init_code(&params.eip210_contract_address, params.eip210_contract_code.clone())?;
}
if block.fields().header.number() >= params.eip210_transition {
let parent_hash = block.fields().header.parent_hash().clone();
if block.header().number() >= params.eip210_transition {
let parent_hash = block.header().parent_hash().clone();
let _ = self.execute_as_system(
block,
params.eip210_contract_address,
@ -185,8 +185,8 @@ impl EthereumMachine {
self.push_last_hash(block)?;
if let Some(ref ethash_params) = self.ethash_extensions {
if block.fields().header.number() == ethash_params.dao_hardfork_transition {
let state = block.fields_mut().state;
if block.header().number() == ethash_params.dao_hardfork_transition {
let state = block.state_mut();
for child in &ethash_params.dao_hardfork_accounts {
let beneficiary = &ethash_params.dao_hardfork_beneficiary;
state.balance(child)
@ -426,11 +426,11 @@ impl<'a> ::parity_machine::LocalizedMachine<'a> for EthereumMachine {
impl ::parity_machine::WithBalances for EthereumMachine {
fn balance(&self, live: &ExecutedBlock, address: &Address) -> Result<U256, Error> {
live.fields().state.balance(address).map_err(Into::into)
live.state().balance(address).map_err(Into::into)
}
fn add_balance(&self, live: &mut ExecutedBlock, address: &Address, amount: &U256) -> Result<(), Error> {
live.fields_mut().state.add_balance(address, amount, CleanupMode::NoEmpty).map_err(Into::into)
live.state_mut().add_balance(address, amount, CleanupMode::NoEmpty).map_err(Into::into)
}
fn note_rewards(
@ -439,22 +439,20 @@ impl ::parity_machine::WithBalances for EthereumMachine {
direct: &[(Address, U256)],
indirect: &[(Address, U256)],
) -> Result<(), Self::Error> {
use block::IsBlock;
if let Tracing::Enabled(ref mut traces) = *live.traces_mut() {
let mut tracer = ExecutiveTracer::default();
if !live.tracing_enabled() { return Ok(()) }
for &(address, amount) in direct {
tracer.trace_reward(address, amount, RewardType::Block);
}
let mut tracer = ExecutiveTracer::default();
for &(address, amount) in indirect {
tracer.trace_reward(address, amount, RewardType::Uncle);
}
for &(address, amount) in direct {
tracer.trace_reward(address, amount, RewardType::Block);
traces.push(tracer.drain().into());
}
for &(address, amount) in indirect {
tracer.trace_reward(address, amount, RewardType::Uncle);
}
live.fields_mut().push_traces(tracer);
Ok(())
}
}

View File

@ -395,7 +395,7 @@ impl Miner {
} else { None };
let transactions = {self.transaction_queue.read().top_transactions_at(chain_info.best_block_number, chain_info.best_block_timestamp, nonce_cap)};
let mut sealing_work = self.sealing_work.lock();
let last_work_hash = sealing_work.queue.peek_last_ref().map(|pb| pb.block().fields().header.hash());
let last_work_hash = sealing_work.queue.peek_last_ref().map(|pb| pb.block().header().hash());
let best_hash = chain_info.best_block_hash;
// check to see if last ClosedBlock in would_seals is actually same parent block.
@ -404,7 +404,7 @@ impl Miner {
// if at least one was pushed successfully, close and enqueue new ClosedBlock;
// otherwise, leave everything alone.
// otherwise, author a fresh block.
let mut open_block = match sealing_work.queue.pop_if(|b| b.block().fields().header.parent_hash() == &best_hash) {
let mut open_block = match sealing_work.queue.pop_if(|b| b.block().header().parent_hash() == &best_hash) {
Some(old_block) => {
trace!(target: "miner", "prepare_block: Already have previous work; updating and returning");
// add transactions to old_block
@ -431,7 +431,7 @@ impl Miner {
let mut invalid_transactions = HashSet::new();
let mut non_allowed_transactions = HashSet::new();
let mut transactions_to_penalize = HashSet::new();
let block_number = open_block.block().fields().header.number();
let block_number = open_block.block().header().number();
let mut tx_count: usize = 0;
let tx_total = transactions.len();
@ -618,14 +618,14 @@ impl Miner {
fn prepare_work(&self, block: ClosedBlock, original_work_hash: Option<H256>) {
let (work, is_new) = {
let mut sealing_work = self.sealing_work.lock();
let last_work_hash = sealing_work.queue.peek_last_ref().map(|pb| pb.block().fields().header.hash());
trace!(target: "miner", "prepare_work: Checking whether we need to reseal: orig={:?} last={:?}, this={:?}", original_work_hash, last_work_hash, block.block().fields().header.hash());
let (work, is_new) = if last_work_hash.map_or(true, |h| h != block.block().fields().header.hash()) {
trace!(target: "miner", "prepare_work: Pushing a new, refreshed or borrowed pending {}...", block.block().fields().header.hash());
let pow_hash = block.block().fields().header.hash();
let number = block.block().fields().header.number();
let difficulty = *block.block().fields().header.difficulty();
let is_new = original_work_hash.map_or(true, |h| block.block().fields().header.hash() != h);
let last_work_hash = sealing_work.queue.peek_last_ref().map(|pb| pb.block().header().hash());
trace!(target: "miner", "prepare_work: Checking whether we need to reseal: orig={:?} last={:?}, this={:?}", original_work_hash, last_work_hash, block.block().header().hash());
let (work, is_new) = if last_work_hash.map_or(true, |h| h != block.block().header().hash()) {
trace!(target: "miner", "prepare_work: Pushing a new, refreshed or borrowed pending {}...", block.block().header().hash());
let pow_hash = block.block().header().hash();
let number = block.block().header().number();
let difficulty = *block.block().header().difficulty();
let is_new = original_work_hash.map_or(true, |h| block.block().header().hash() != h);
sealing_work.queue.push(block);
// If push notifications are enabled we assume all work items are used.
if !self.notifiers.read().is_empty() && is_new {
@ -635,7 +635,7 @@ impl Miner {
} else {
(None, false)
};
trace!(target: "miner", "prepare_work: leaving (last={:?})", sealing_work.queue.peek_last_ref().map(|b| b.block().fields().header.hash()));
trace!(target: "miner", "prepare_work: leaving (last={:?})", sealing_work.queue.peek_last_ref().map(|b| b.block().header().hash()));
(work, is_new)
};
if is_new {
@ -1125,7 +1125,7 @@ impl MinerService for Miner {
// refuse to seal the first block of the chain if it contains hard forks
// which should be on by default.
if block.block().fields().header.number() == 1 && self.engine.params().contains_bugfix_hard_fork() {
if block.block().header().number() == 1 && self.engine.params().contains_bugfix_hard_fork() {
warn!("{}", NO_NEW_CHAIN_WITH_FORKS);
return;
}
@ -1156,7 +1156,7 @@ impl MinerService for Miner {
trace!(target: "miner", "map_sealing_work: sealing prepared");
let mut sealing_work = self.sealing_work.lock();
let ret = sealing_work.queue.use_last_ref();
trace!(target: "miner", "map_sealing_work: leaving use_last_ref={:?}", ret.as_ref().map(|b| b.block().fields().header.hash()));
trace!(target: "miner", "map_sealing_work: leaving use_last_ref={:?}", ret.as_ref().map(|b| b.block().header().hash()));
ret.map(f)
}
@ -1325,16 +1325,16 @@ mod tests {
let client = TestBlockChainClient::default();
let miner = Miner::with_spec(&Spec::new_test());
let res = miner.map_sealing_work(&client, |b| b.block().fields().header.hash());
let res = miner.map_sealing_work(&client, |b| b.block().header().hash());
assert!(res.is_some());
assert!(miner.submit_seal(&client, res.unwrap(), vec![]).is_ok());
// two more blocks mined, work requested.
client.add_blocks(1, EachBlockWith::Uncle);
miner.map_sealing_work(&client, |b| b.block().fields().header.hash());
miner.map_sealing_work(&client, |b| b.block().header().hash());
client.add_blocks(1, EachBlockWith::Uncle);
miner.map_sealing_work(&client, |b| b.block().fields().header.hash());
miner.map_sealing_work(&client, |b| b.block().header().hash());
// solution to original work submitted.
assert!(miner.submit_seal(&client, res.unwrap(), vec![]).is_ok());

View File

@ -280,8 +280,8 @@ fn change_history_size() {
for _ in 0..20 {
let mut b = client.prepare_open_block(Address::default(), (3141562.into(), 31415620.into()), vec![]);
b.block_mut().fields_mut().state.add_balance(&address, &5.into(), CleanupMode::NoEmpty).unwrap();
b.block_mut().fields_mut().state.commit().unwrap();
b.block_mut().state_mut().add_balance(&address, &5.into(), CleanupMode::NoEmpty).unwrap();
b.block_mut().state_mut().commit().unwrap();
let b = b.close_and_lock().seal(&*test_spec.engine, vec![]).unwrap();
client.import_sealed_block(b).unwrap(); // account change is in the journal overlay
}
@ -339,8 +339,8 @@ fn transaction_proof() {
let test_spec = Spec::new_test();
for _ in 0..20 {
let mut b = client.prepare_open_block(Address::default(), (3141562.into(), 31415620.into()), vec![]);
b.block_mut().fields_mut().state.add_balance(&address, &5.into(), CleanupMode::NoEmpty).unwrap();
b.block_mut().fields_mut().state.commit().unwrap();
b.block_mut().state_mut().add_balance(&address, &5.into(), CleanupMode::NoEmpty).unwrap();
b.block_mut().state_mut().commit().unwrap();
let b = b.close_and_lock().seal(&*test_spec.engine, vec![]).unwrap();
client.import_sealed_block(b).unwrap(); // account change is in the journal overlay
}

View File

@ -30,7 +30,7 @@ pub use self::executive_tracer::{ExecutiveTracer, ExecutiveVMTracer};
pub use self::import::ImportRequest;
pub use self::localized::LocalizedTrace;
pub use self::types::{filter, flat, localized, trace};
pub use self::types::{filter, flat, localized, trace, Tracing};
pub use self::types::error::Error as TraceError;
pub use self::types::trace::{VMTrace, VMOperation, VMExecutedOperation, MemoryDiff, StorageDiff, RewardType};
pub use self::types::flat::{FlatTrace, FlatTransactionTraces, FlatBlockTraces};

View File

@ -21,3 +21,37 @@ pub mod filter;
pub mod flat;
pub mod trace;
pub mod localized;
use self::flat::FlatTransactionTraces;
/// Container for block traces.
#[derive(Clone)]
pub enum Tracing {
/// This variant should be used when tracing is enabled.
Enabled(Vec<FlatTransactionTraces>),
/// Tracing is disabled.
Disabled,
}
impl Tracing {
/// Creates new instance of enabled tracing object.
pub fn enabled() -> Self {
Tracing::Enabled(Default::default())
}
/// Returns true if tracing is enabled.
pub fn is_enabled(&self) -> bool {
match *self {
Tracing::Enabled(_) => true,
Tracing::Disabled => false,
}
}
/// Drain all traces.
pub fn drain(self) -> Vec<FlatTransactionTraces> {
match self {
Tracing::Enabled(traces) => traces,
Tracing::Disabled => vec![],
}
}
}