From 1f363b22fcd61ee5d4540be07f4b500021453cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 12:17:20 +0100 Subject: [PATCH] Removing invalid transactions from queue --- ethcore/src/client/client.rs | 31 ++++++++++++++++++++++++++----- ethcore/src/client/mod.rs | 4 +++- ethcore/src/client/test_client.rs | 2 +- ethcore/src/tests/client.rs | 2 +- miner/src/miner.rs | 15 ++++++++++++++- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index caa92db97..7818fcdc8 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -391,7 +391,8 @@ impl BlockChainClient for Client where V: Verifier { } // TODO [todr] Should be moved to miner crate eventually. - fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec) -> Option { + fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec) + -> Option<(ClosedBlock, HashSet)> { let engine = self.engine.deref().deref(); let h = self.chain.best_block_hash(); @@ -417,21 +418,41 @@ impl BlockChainClient for Client where V: Verifier { // Add transactions let block_number = b.block().header().number(); + let gas_limit = *b.block().header().gas_limit(); + let mut gas_left = gas_limit; + let mut invalid_transactions = HashSet::new(); + for tx in transactions { + let hash = tx.hash(); + let gas = tx.gas; + // TODO [todr] It seems that calculating gas_left here doesn't really look nice. After moving this function + // to miner crate we should consider rewriting this logic in some better way. + if gas > gas_left { + trace!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?}", hash); + continue; + } + let import = b.push_transaction(tx, None); - if let Err(e) = import { - trace!("Error adding transaction to block: number={}. Error: {:?}", block_number, e); + match import { + Err(e) => { + trace!(target: "miner", "Error adding transaction to block: number={}. transaction_hash={:?}, Error: {:?}", + block_number, hash, e); + invalid_transactions.insert(hash); + }, + Ok(receipt) => { + gas_left = gas_limit - receipt.gas_used; + } } } // And close let b = b.close(); - trace!("Sealing: number={}, hash={}, diff={}", + trace!(target: "miner", "Sealing: number={}, hash={}, diff={}", b.block().header().number(), b.hash(), b.block().header().difficulty() ); - Some(b) + Some((b, invalid_transactions)) } fn block_header(&self, id: BlockId) -> Option { diff --git a/ethcore/src/client/mod.rs b/ethcore/src/client/mod.rs index 88e07d0b1..198e918f7 100644 --- a/ethcore/src/client/mod.rs +++ b/ethcore/src/client/mod.rs @@ -26,6 +26,7 @@ pub use self::config::{ClientConfig, BlockQueueConfig, BlockChainConfig}; pub use self::ids::{BlockId, TransactionId}; pub use self::test_client::{TestBlockChainClient, EachBlockWith}; +use std::collections::HashSet; use util::bytes::Bytes; use util::hash::{Address, H256, H2048}; use util::numbers::U256; @@ -110,7 +111,8 @@ pub trait BlockChainClient : Sync + Send { // TODO [todr] Should be moved to miner crate eventually. /// Returns ClosedBlock prepared for sealing. - fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec) -> Option; + fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec) + -> Option<(ClosedBlock, HashSet)>; // TODO [todr] Should be moved to miner crate eventually. /// Attempts to seal given block. Returns `SealedBlock` on success and the same block in case of error. diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 83511b1cc..e351011f2 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -217,7 +217,7 @@ impl BlockChainClient for TestBlockChainClient { unimplemented!(); } - fn prepare_sealing(&self, _author: Address, _gas_floor_target: U256, _extra_data: Bytes, _transactions: Vec) -> Option { + fn prepare_sealing(&self, _author: Address, _gas_floor_target: U256, _extra_data: Bytes, _transactions: Vec) -> Option<(ClosedBlock, HashSet)> { unimplemented!() } diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index d9fae0527..64a2222b1 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -144,7 +144,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(), x!(31415926), vec![], vec![]).unwrap(); + let b = client.prepare_sealing(Address::default(), x!(31415926), vec![], vec![]).unwrap().0; assert_eq!(*b.block().header().parent_hash(), BlockView::new(&dummy_blocks[0]).header_view().sha3()); assert!(client.try_seal(b, vec![]).is_ok()); diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 7bbde6294..07a5cb8db 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -132,7 +132,20 @@ impl MinerService for Miner { self.extra_data(), transactions, ); - *self.sealing_block.lock().unwrap() = b; + + match b { + None => { + *self.sealing_block.lock().unwrap() = None + }, + Some((block, invalid_transactions)) => { + let mut queue = self.transaction_queue.lock().unwrap(); + queue.remove_all( + &invalid_transactions.into_iter().collect::>(), + |a: &Address| chain.nonce(a) + ); + *self.sealing_block.lock().unwrap() = Some(block) + } + } } fn sealing_block(&self, chain: &BlockChainClient) -> &Mutex> {