diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6f44dd5bb..fcf8cbdd4 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -481,7 +481,7 @@ impl BlockChainClient for Client where V: Verifier { } let options = TransactOptions { tracing: false, vm_tracing: analytics.vm_tracing, check_nonce: false }; let mut ret = Executive::new(&mut state, &env_info, self.engine.deref().deref(), &self.vm_factory).transact(t, options); - + // TODO gav move this into Executive. if analytics.state_diffing { if let Ok(ref mut x) = ret { @@ -760,18 +760,16 @@ impl BlockChainClient for Client where V: Verifier { } impl MiningBlockChainClient for Client where V: Verifier { - fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec) - -> (Option, HashSet) { + fn prepare_open_block(&self, author: Address, gas_floor_target: U256, extra_data: Bytes) -> OpenBlock { let engine = self.engine.deref().deref(); let h = self.chain.best_block_hash(); - let mut invalid_transactions = HashSet::new(); - let mut b = OpenBlock::new( + let mut open_block = OpenBlock::new( engine, &self.vm_factory, false, // TODO: this will need to be parameterised once we want to do immediate mining insertion. self.state_db.lock().unwrap().boxed_clone(), - match self.chain.block_header(&h) { Some(ref x) => x, None => { return (None, invalid_transactions) } }, + &self.chain.block_header(&h).expect("h is best block hash: so it's header must exist: qed"), self.build_last_hashes(h.clone()), author, gas_floor_target, @@ -785,44 +783,10 @@ impl MiningBlockChainClient for Client where V: Verifier { .into_iter() .take(engine.maximum_uncle_count()) .foreach(|h| { - b.push_uncle(h).unwrap(); + open_block.push_uncle(h).unwrap(); }); - // Add transactions - let block_number = b.block().header().number(); - let min_tx_gas = U256::from(self.engine.schedule(&b.env_info()).tx_gas); - - for tx in transactions { - // Push transaction to block - let hash = tx.hash(); - let import = b.push_transaction(tx, None); - - match import { - Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, .. })) => { - trace!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?}", hash); - // Exit early if gas left is smaller then min_tx_gas - if gas_limit - gas_used < min_tx_gas { - break; - } - }, - Err(e) => { - invalid_transactions.insert(hash); - trace!(target: "miner", - "Error adding transaction to block: number={}. transaction_hash={:?}, Error: {:?}", - block_number, hash, e); - }, - _ => {} - } - } - - // And close - let b = b.close(); - trace!(target: "miner", "Sealing: number={}, hash={}, diff={}", - b.block().header().number(), - b.hash(), - b.block().header().difficulty() - ); - (Some(b), invalid_transactions) + open_block } fn try_seal(&self, block: LockedBlock, seal: Vec) -> Result { diff --git a/ethcore/src/client/mod.rs b/ethcore/src/client/mod.rs index 0dffb1a1c..7579f75f2 100644 --- a/ethcore/src/client/mod.rs +++ b/ethcore/src/client/mod.rs @@ -31,13 +31,12 @@ pub use self::trace::Filter as TraceFilter; pub use executive::{Executed, Executive, TransactOptions}; pub use env_info::{LastHashes, EnvInfo}; -use std::collections::HashSet; use util::bytes::Bytes; use util::hash::{Address, H256, H2048}; use util::numbers::U256; use blockchain::TreeRoute; use block_queue::BlockQueueInfo; -use block::{ClosedBlock, LockedBlock, SealedBlock}; +use block::{LockedBlock, SealedBlock, OpenBlock}; use header::{BlockNumber, Header}; use transaction::{LocalizedTransaction, SignedTransaction}; use log_entry::LocalizedLogEntry; @@ -199,7 +198,7 @@ pub trait MiningBlockChainClient : BlockChainClient { /// Attempts to seal given block. Returns `SealedBlock` on success and the same block in case of error. fn try_seal(&self, block: LockedBlock, seal: Vec) -> Result; - /// Returns ClosedBlock prepared for sealing. - fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec) - -> (Option, HashSet); + /// Returns OpenBlock prepared for closing. + fn prepare_open_block(&self, author: Address, gas_floor_target: U256, extra_data: Bytes) + -> OpenBlock; } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 17905a905..d02afc0f7 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -31,7 +31,7 @@ use evm::Factory as EvmFactory; use miner::{Miner, MinerService}; use block_queue::BlockQueueInfo; -use block::{SealedBlock, ClosedBlock, LockedBlock}; +use block::{SealedBlock, LockedBlock, OpenBlock}; use executive::Executed; use error::{ExecutionError}; use trace::LocalizedTrace; @@ -245,8 +245,8 @@ impl MiningBlockChainClient for TestBlockChainClient { } - fn prepare_sealing(&self, _author: Address, _gas_floor_target: U256, _extra_data: Bytes, _transactions: Vec) -> (Option, HashSet) { - (None, HashSet::new()) + fn prepare_open_block(&self, _author: Address, _gas_floor_target: U256, _extra_data: Bytes) -> OpenBlock { + unimplemented!(); } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 5dc3864c3..489173996 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -117,90 +117,89 @@ impl Miner { // otherwise, leave everything alone. // otherwise, author a fresh block. */ - - let (b, invalid_transactions) = match sealing_work.pop_if(|b| b.block().fields().header.parent_hash() == &best_hash) { + let mut open_block = match sealing_work.pop_if(|b| b.block().fields().header.parent_hash() == &best_hash) { Some(old_block) => { trace!(target: "miner", "Already have previous work; updating and returning"); // add transactions to old_block let e = self.engine(); - let mut invalid_transactions = HashSet::new(); - let mut block = old_block.reopen(e, chain.vm_factory()); - let block_number = block.block().fields().header.number(); - - // TODO: push new uncles, too. - // TODO: refactor with chain.prepare_sealing - for tx in transactions { - let hash = tx.hash(); - let res = block.push_transaction(tx, None); - match res { - Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, .. })) => { - trace!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?}", hash); - // Exit early if gas left is smaller then min_tx_gas - let min_tx_gas: U256 = 21000.into(); // TODO: figure this out properly. - if gas_limit - gas_used < min_tx_gas { - break; - } - }, - Err(Error::Transaction(TransactionError::AlreadyImported)) => {} // already have transaction - ignore - Err(e) => { - invalid_transactions.insert(hash); - trace!(target: "miner", - "Error adding transaction to block: number={}. transaction_hash={:?}, Error: {:?}", - block_number, hash, e); - }, - _ => {} // imported ok - } - } - (Some(block.close()), invalid_transactions) + old_block.reopen(e, chain.vm_factory()) } None => { // block not found - create it. trace!(target: "miner", "No existing work - making new block"); - chain.prepare_sealing( + chain.prepare_open_block( self.author(), self.gas_floor_target(), - self.extra_data(), - transactions, + self.extra_data() ) } }; + + let mut invalid_transactions = HashSet::new(); + let block_number = open_block.block().fields().header.number(); + // TODO: push new uncles, too. + for tx in transactions { + let hash = tx.hash(); + match open_block.push_transaction(tx, None) { + Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, .. })) => { + trace!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?}", hash); + // Exit early if gas left is smaller then min_tx_gas + let min_tx_gas: U256 = 21000.into(); // TODO: figure this out properly. + if gas_limit - gas_used < min_tx_gas { + break; + } + }, + Err(Error::Transaction(TransactionError::AlreadyImported)) => {} // already have transaction - ignore + Err(e) => { + invalid_transactions.insert(hash); + trace!(target: "miner", + "Error adding transaction to block: number={}. transaction_hash={:?}, Error: {:?}", + block_number, hash, e); + }, + _ => {} // imported ok + } + } + + let block = open_block.close(); + let mut queue = self.transaction_queue.lock().unwrap(); let fetch_account = |a: &Address| AccountDetails { nonce: chain.latest_nonce(a), balance: chain.latest_balance(a), }; + for hash in invalid_transactions.into_iter() { queue.remove_invalid(&hash, &fetch_account); } - if let Some(block) = b { - if !block.transactions().is_empty() { - trace!(target: "miner", "prepare_sealing: block has transaction - attempting internal seal."); - // block with transactions - see if we can seal immediately. - let s = self.engine().generate_seal(block.block(), match self.accounts { - Some(ref x) => Some(&**x), - None => None, - }); - if let Some(seal) = s { - trace!(target: "miner", "prepare_sealing: managed internal seal. importing..."); - if let Ok(sealed) = chain.try_seal(block.lock(), seal) { - if let Ok(_) = chain.import_block(sealed.rlp_bytes()) { - trace!(target: "miner", "prepare_sealing: sealed internally and imported. leaving."); - } else { - warn!("prepare_sealing: ERROR: could not import internally sealed block. WTF?"); - } + + if !block.transactions().is_empty() { + trace!(target: "miner", "prepare_sealing: block has transaction - attempting internal seal."); + // block with transactions - see if we can seal immediately. + let s = self.engine().generate_seal(block.block(), match self.accounts { + Some(ref x) => Some(&**x), + None => None, + }); + if let Some(seal) = s { + trace!(target: "miner", "prepare_sealing: managed internal seal. importing..."); + if let Ok(sealed) = chain.try_seal(block.lock(), seal) { + if let Ok(_) = chain.import_block(sealed.rlp_bytes()) { + trace!(target: "miner", "prepare_sealing: sealed internally and imported. leaving."); } else { - warn!("prepare_sealing: ERROR: try_seal failed when given internally generated seal. WTF?"); + warn!("prepare_sealing: ERROR: could not import internally sealed block. WTF?"); } - return; } else { - trace!(target: "miner", "prepare_sealing: unable to generate seal internally"); + warn!("prepare_sealing: ERROR: try_seal failed when given internally generated seal. WTF?"); } - } - if sealing_work.peek_last_ref().map_or(true, |pb| pb.block().fields().header.hash() != block.block().fields().header.hash()) { - trace!(target: "miner", "Pushing a new, refreshed or borrowed pending {}...", block.block().fields().header.hash()); - sealing_work.push(block); + return; + } else { + trace!(target: "miner", "prepare_sealing: unable to generate seal internally"); } } + if sealing_work.peek_last_ref().map_or(true, |pb| pb.block().fields().header.hash() != block.block().fields().header.hash()) { + trace!(target: "miner", "Pushing a new, refreshed or borrowed pending {}...", block.block().fields().header.hash()); + sealing_work.push(block); + } + trace!(target: "miner", "prepare_sealing: leaving (last={:?})", sealing_work.peek_last_ref().map(|b| b.block().fields().header.hash())); } @@ -282,7 +281,7 @@ impl MinerService for Miner { } let options = TransactOptions { tracing: false, vm_tracing: analytics.vm_tracing, check_nonce: false }; let mut ret = Executive::new(&mut state, &env_info, self.engine(), chain.vm_factory()).transact(t, options); - + // TODO gav move this into Executive. if analytics.state_diffing { if let Ok(ref mut x) = ret { diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index d6fab71f0..789943942 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -137,7 +137,7 @@ fn can_mine() { let client_result = get_test_client_with_blocks(vec![dummy_blocks[0].clone()]); let client = client_result.reference(); - let b = client.prepare_sealing(Address::default(), 31415926.into(), vec![], vec![]).0.unwrap(); + let b = client.prepare_open_block(Address::default(), 31415926.into(), vec![]).close(); assert_eq!(*b.block().header().parent_hash(), BlockView::new(&dummy_blocks[0]).header_view().sha3()); assert!(client.try_seal(b.lock(), vec![]).is_ok());