From 990c5c8faa6cded59257846bd58039e0c48ad085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 23 Feb 2016 18:44:13 +0100 Subject: [PATCH 1/7] Refactoring client and fixing mark_as_bad & SyncMessage bugs --- ethcore/src/client.rs | 145 +++++++++++++++++++++++++---------------- ethcore/src/service.rs | 7 +- 2 files changed, 95 insertions(+), 57 deletions(-) diff --git a/ethcore/src/client.rs b/ethcore/src/client.rs index c3ec4b4d0..aa71fbbfa 100644 --- a/ethcore/src/client.rs +++ b/ethcore/src/client.rs @@ -22,7 +22,7 @@ use rocksdb::{Options, DB, DBCompactionStyle}; use blockchain::{BlockChain, BlockProvider, CacheSize}; use views::BlockView; use error::*; -use header::BlockNumber; +use header::{BlockNumber, Header}; use state::State; use spec::Spec; use engine::Engine; @@ -243,85 +243,117 @@ impl Client { self.block_queue.write().unwrap().flush(); } + fn build_last_hashes(&self, header: &Header) -> LastHashes { + let mut last_hashes = LastHashes::new(); + last_hashes.resize(256, H256::new()); + last_hashes[0] = header.parent_hash.clone(); + let chain = self.chain.read().unwrap(); + for i in 0..255 { + match chain.block_details(&last_hashes[i]) { + Some(details) => { + last_hashes[i + 1] = details.parent.clone(); + }, + None => break, + } + } + last_hashes + } + /// This is triggered by a message coming from a block queue when the block is ready for insertion pub fn import_verified_blocks(&self, io: &IoChannel) -> usize { - let mut ret = 0; - let mut bad = HashSet::new(); + let max_blocks_to_import = 128; + + let mut imported = 0; + let mut good_blocks = Vec::with_capacity(max_blocks_to_import); + let mut bad_blocks = HashSet::new(); + let engine = self.engine.deref().deref(); + let _import_lock = self.import_lock.lock(); - let blocks = self.block_queue.write().unwrap().drain(128); - let mut good_blocks = Vec::with_capacity(128); + let blocks = self.block_queue.write().unwrap().drain(max_blocks_to_import); + for block in blocks { - if bad.contains(&block.header.parent_hash) { - self.block_queue.write().unwrap().mark_as_bad(&block.header.hash()); - bad.insert(block.header.hash()); + let header = &block.header; + let header_hash = block.header.hash(); + let bad_contains_parent = bad_blocks.contains(&header.parent_hash); + + let mark_block_as_bad = || { + self.block_queue.write().unwrap().mark_as_bad(&header_hash); + bad_blocks.insert(header_hash); + }; + + if bad_contains_parent { + mark_block_as_bad(); continue; } - let header = &block.header; - if let Err(e) = verify_block_family(&header, &block.bytes, self.engine.deref().deref(), self.chain.read().unwrap().deref()) { + // Verify Block Family + let verify_family_result = verify_block_family(&header, &block.bytes, engine, self.chain.read().unwrap().deref()); + if let Err(e) = verify_family_result { warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - self.block_queue.write().unwrap().mark_as_bad(&header.hash()); - bad.insert(block.header.hash()); + mark_block_as_bad(); break; }; - let parent = match self.chain.read().unwrap().block_header(&header.parent_hash) { - Some(p) => p, - None => { - warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); - self.block_queue.write().unwrap().mark_as_bad(&header.hash()); - bad.insert(block.header.hash()); - break; - }, - }; - // build last hashes - let mut last_hashes = LastHashes::new(); - last_hashes.resize(256, H256::new()); - last_hashes[0] = header.parent_hash.clone(); - for i in 0..255 { - match self.chain.read().unwrap().block_details(&last_hashes[i]) { - Some(details) => { - last_hashes[i + 1] = details.parent.clone(); - }, - None => break, - } - } + // Check if Parent is in chain + let chain_has_parent = self.chain.read().unwrap().block_header(&header.parent_hash); + if let None = chain_has_parent { + warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); + mark_block_as_bad(); + break; + }; + + // Enact Verified Block + let parent = chain_has_parent.unwrap(); + let last_hashes = self.build_last_hashes(header); let db = self.state_db.lock().unwrap().clone(); - let result = match enact_verified(&block, self.engine.deref().deref(), db, &parent, &last_hashes) { - Ok(b) => b, - Err(e) => { - warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - bad.insert(block.header.hash()); - self.block_queue.write().unwrap().mark_as_bad(&header.hash()); - break; - } + + let enact_result = enact_verified(&block, engine, db, &parent, &last_hashes); + if let Err(e) = enact_result { + warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); + mark_block_as_bad(); + break; }; - if let Err(e) = verify_block_final(&header, result.block().header()) { + + // Final Verification + let enact_result = enact_result.unwrap(); + if let Err(e) = verify_block_final(&header, enact_result.block().header()) { warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - self.block_queue.write().unwrap().mark_as_bad(&header.hash()); + mark_block_as_bad(); break; } - good_blocks.push(header.hash().clone()); - + // Insert block self.chain.write().unwrap().insert_block(&block.bytes); //TODO: err here? - let ancient = if header.number() >= HISTORY { Some(header.number() - HISTORY) } else { None }; - match result.drain().commit(header.number(), &header.hash(), ancient.map(|n|(n, self.chain.read().unwrap().block_hash(n).unwrap()))) { - Ok(_) => (), - Err(e) => { - warn!(target: "client", "State DB commit failed: {:?}", e); - break; - } + good_blocks.push(header.hash()); + + let ancient = if header.number() >= HISTORY { + let n = header.number() - HISTORY; + let chain = self.chain.read().unwrap(); + Some((n, chain.block_hash(n).unwrap())) + } else { + None + }; + + // Commit results + let commit_result = enact_result.drain().commit(header.number(), &header.hash(), ancient); + if let Err(e) = commit_result { + warn!(target: "client", "State DB commit failed: {:?}", e); + break; } + self.report.write().unwrap().accrue_block(&block); trace!(target: "client", "Imported #{} ({})", header.number(), header.hash()); - ret += 1; + imported += 1; } + self.block_queue.write().unwrap().mark_as_good(&good_blocks); if !good_blocks.is_empty() && self.block_queue.read().unwrap().queue_info().is_empty() { - io.send(NetworkIoMessage::User(SyncMessage::BlockVerified)).unwrap(); + io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { + good: good_blocks, + bad: bad_blocks.into_iter().collect(), + })).unwrap(); } - ret + imported } /// Get a copy of the best block's state. @@ -393,7 +425,7 @@ impl BlockChainClient for Client { None => BlockStatus::Unknown } } - + fn block_total_difficulty(&self, id: BlockId) -> Option { let chain = self.chain.read().unwrap(); Self::block_hash(&chain, id).and_then(|hash| chain.block_details(&hash)).map(|d| d.total_difficulty) @@ -435,6 +467,7 @@ impl BlockChainClient for Client { return Err(ImportError::UnknownParent); } self.block_queue.write().unwrap().import_block(bytes) + // TODO [ToDr] remove transactions } fn queue_info(&self) -> BlockQueueInfo { diff --git a/ethcore/src/service.rs b/ethcore/src/service.rs index 534aab49d..cdf3425e8 100644 --- a/ethcore/src/service.rs +++ b/ethcore/src/service.rs @@ -26,7 +26,12 @@ use client::Client; #[derive(Clone)] pub enum SyncMessage { /// New block has been imported into the blockchain - NewChainBlock(Bytes), //TODO: use Cow + NewChainBlocks { + /// Hashes of blocks imported to blockchain + good: Vec, + /// Hashes of blocks not imported to blockchain + bad: Vec, + }, /// A block is ready BlockVerified, } From 4084acd869f53e0695bed887bcf33578b611c681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 23 Feb 2016 18:51:18 +0100 Subject: [PATCH 2/7] Removing dangling comment --- ethcore/src/client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/ethcore/src/client.rs b/ethcore/src/client.rs index aa71fbbfa..87dba3dd5 100644 --- a/ethcore/src/client.rs +++ b/ethcore/src/client.rs @@ -467,7 +467,6 @@ impl BlockChainClient for Client { return Err(ImportError::UnknownParent); } self.block_queue.write().unwrap().import_block(bytes) - // TODO [ToDr] remove transactions } fn queue_info(&self) -> BlockQueueInfo { From d3fe3f26918c676353f96461a94ac6a8aa6fceb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 24 Feb 2016 10:55:34 +0100 Subject: [PATCH 3/7] Client refactoring [WIP] --- ethcore/src/block_queue.rs | 23 ++++--- ethcore/src/client.rs | 124 ++++++++++++++++++++----------------- 2 files changed, 81 insertions(+), 66 deletions(-) diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index c39f158f0..b802e3c26 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -285,19 +285,22 @@ impl BlockQueue { } /// Mark given block and all its children as bad. Stops verification. - pub fn mark_as_bad(&mut self, hash: &H256) { + pub fn mark_as_bad(&mut self, hashes: &[H256]) { let mut verification_lock = self.verification.lock().unwrap(); let mut verification = verification_lock.deref_mut(); - verification.bad.insert(hash.clone()); - self.processing.write().unwrap().remove(&hash); let mut new_verified = VecDeque::new(); - for block in verification.verified.drain(..) { - if verification.bad.contains(&block.header.parent_hash) { - verification.bad.insert(block.header.hash()); - self.processing.write().unwrap().remove(&block.header.hash()); - } - else { - new_verified.push_back(block); + + for hash in hashes { + verification.bad.insert(hash.clone()); + self.processing.write().unwrap().remove(&hash); + for block in verification.verified.drain(..) { + if verification.bad.contains(&block.header.parent_hash) { + verification.bad.insert(block.header.hash()); + self.processing.write().unwrap().remove(&block.header.hash()); + } + else { + new_verified.push_back(block); + } } } verification.verified = new_verified; diff --git a/ethcore/src/client.rs b/ethcore/src/client.rs index 87dba3dd5..3c44b97bf 100644 --- a/ethcore/src/client.rs +++ b/ethcore/src/client.rs @@ -259,70 +259,73 @@ impl Client { last_hashes } + fn check_and_close_block(&self, block: &PreVerifiedBlock) -> Result { + let engine = self.engine.deref().deref(); + let header = &block.header; + let header_hash = block.header.hash(); + + // Verify Block Family + let verify_family_result = verify_block_family(&header, &block.bytes, engine, self.chain.read().unwrap().deref()); + if let Err(e) = verify_family_result { + warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); + return Err(()); + }; + + // Check if Parent is in chain + let chain_has_parent = self.chain.read().unwrap().block_header(&header.parent_hash); + if let None = chain_has_parent { + warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); + return Err(()); + }; + + // Enact Verified Block + let parent = chain_has_parent.unwrap(); + let last_hashes = self.build_last_hashes(header); + let db = self.state_db.lock().unwrap().clone(); + + let enact_result = enact_verified(&block, engine, db, &parent, last_hashes); + if let Err(e) = enact_result { + warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); + return Err(()); + }; + + // Final Verification + let closed_block = enact_result.unwrap(); + if let Err(e) = verify_block_final(&header, closed_block.block().header()) { + warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); + return Err(()); + } + + Ok(closed_block) + } + /// This is triggered by a message coming from a block queue when the block is ready for insertion pub fn import_verified_blocks(&self, io: &IoChannel) -> usize { let max_blocks_to_import = 128; - let mut imported = 0; let mut good_blocks = Vec::with_capacity(max_blocks_to_import); let mut bad_blocks = HashSet::new(); - let engine = self.engine.deref().deref(); let _import_lock = self.import_lock.lock(); let blocks = self.block_queue.write().unwrap().drain(max_blocks_to_import); for block in blocks { - let header = &block.header; - let header_hash = block.header.hash(); - let bad_contains_parent = bad_blocks.contains(&header.parent_hash); + let header = block.header; - let mark_block_as_bad = || { - self.block_queue.write().unwrap().mark_as_bad(&header_hash); - bad_blocks.insert(header_hash); - }; - - if bad_contains_parent { - mark_block_as_bad(); + if bad_blocks.contains(&header.parent_hash) { + bad_blocks.insert(header.hash()); continue; } - // Verify Block Family - let verify_family_result = verify_block_family(&header, &block.bytes, engine, self.chain.read().unwrap().deref()); - if let Err(e) = verify_family_result { - warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - mark_block_as_bad(); - break; - }; - // Check if Parent is in chain - let chain_has_parent = self.chain.read().unwrap().block_header(&header.parent_hash); - if let None = chain_has_parent { - warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); - mark_block_as_bad(); - break; - }; - - // Enact Verified Block - let parent = chain_has_parent.unwrap(); - let last_hashes = self.build_last_hashes(header); - let db = self.state_db.lock().unwrap().clone(); - - let enact_result = enact_verified(&block, engine, db, &parent, &last_hashes); - if let Err(e) = enact_result { - warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - mark_block_as_bad(); - break; - }; - - // Final Verification - let enact_result = enact_result.unwrap(); - if let Err(e) = verify_block_final(&header, enact_result.block().header()) { - warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - mark_block_as_bad(); + let closed_block = self.check_and_close_block(&block); + if let Err(_) = closed_block { + bad_blocks.insert(header.hash()); break; } // Insert block + let closed_block = closed_block.unwrap(); self.chain.write().unwrap().insert_block(&block.bytes); //TODO: err here? good_blocks.push(header.hash()); @@ -335,24 +338,33 @@ impl Client { }; // Commit results - let commit_result = enact_result.drain().commit(header.number(), &header.hash(), ancient); - if let Err(e) = commit_result { - warn!(target: "client", "State DB commit failed: {:?}", e); - break; - } + closed_block.drain() + .commit(header.number(), &header.hash(), ancient) + .expect("State DB commit failed."); self.report.write().unwrap().accrue_block(&block); trace!(target: "client", "Imported #{} ({})", header.number(), header.hash()); - imported += 1; } - self.block_queue.write().unwrap().mark_as_good(&good_blocks); - if !good_blocks.is_empty() && self.block_queue.read().unwrap().queue_info().is_empty() { - io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { - good: good_blocks, - bad: bad_blocks.into_iter().collect(), - })).unwrap(); + let imported = good_blocks.len(); + let bad_blocks = bad_blocks.into_iter().collect::>(); + + { + let block_queue = self.block_queue.write().unwrap(); + block_queue.mark_as_bad(&bad_blocks); + block_queue.mark_as_good(&good_blocks); } + + { + let block_queue = self.block_queue.read().unwrap(); + if !good_blocks.is_empty() && block_queue.queue_info().is_empty() { + io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { + good: good_blocks, + bad: bad_blocks, + })).unwrap(); + } + } + imported } From d914a27bdfd2ac69035f129600ee719d7337794d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 24 Feb 2016 11:17:25 +0100 Subject: [PATCH 4/7] Removing lifetimes from Blocks --- ethcore/src/block.rs | 38 +++++++++++++++++----------------- ethcore/src/client.rs | 6 ++---- ethcore/src/ethereum/ethash.rs | 4 ++-- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index a1194c665..9051b6d9a 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -146,18 +146,18 @@ impl IsBlock for ExecutedBlock { /// /// It's a bit like a Vec, eccept that whenever a transaction is pushed, we execute it and /// maintain the system `state()`. We also archive execution receipts in preparation for later block creation. -pub struct OpenBlock<'x, 'y> { +pub struct OpenBlock<'x> { block: ExecutedBlock, engine: &'x Engine, - last_hashes: &'y LastHashes, + last_hashes: LastHashes, } /// Just like OpenBlock, except that we've applied `Engine::on_close_block`, finished up the non-seal header fields, /// and collected the uncles. /// /// There is no function available to push a transaction. If you want that you'll need to `reopen()` it. -pub struct ClosedBlock<'x, 'y> { - open_block: OpenBlock<'x, 'y>, +pub struct ClosedBlock<'x> { + open_block: OpenBlock<'x>, uncle_bytes: Bytes, } @@ -169,9 +169,9 @@ pub struct SealedBlock { uncle_bytes: Bytes, } -impl<'x, 'y> OpenBlock<'x, 'y> { +impl<'x> OpenBlock<'x> { /// Create a new OpenBlock ready for transaction pushing. - pub fn new(engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: &'y LastHashes, author: Address, extra_data: Bytes) -> Self { + pub fn new(engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: LastHashes, author: Address, extra_data: Bytes) -> Self { let mut r = OpenBlock { block: ExecutedBlock::new(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce())), engine: engine, @@ -259,7 +259,7 @@ impl<'x, 'y> OpenBlock<'x, 'y> { } /// Turn this into a `ClosedBlock`. A BlockChain must be provided in order to figure out the uncles. - pub fn close(self) -> ClosedBlock<'x, 'y> { + pub fn close(self) -> ClosedBlock<'x> { let mut s = self; s.engine.on_close_block(&mut s.block); s.block.base.header.transactions_root = ordered_trie_root(s.block.base.transactions.iter().map(|ref e| e.rlp_bytes().to_vec()).collect()); @@ -275,16 +275,16 @@ impl<'x, 'y> OpenBlock<'x, 'y> { } } -impl<'x, 'y> IsBlock for OpenBlock<'x, 'y> { +impl<'x> IsBlock for OpenBlock<'x> { fn block(&self) -> &ExecutedBlock { &self.block } } -impl<'x, 'y> IsBlock for ClosedBlock<'x, 'y> { +impl<'x> IsBlock for ClosedBlock<'x> { fn block(&self) -> &ExecutedBlock { &self.open_block.block } } -impl<'x, 'y> ClosedBlock<'x, 'y> { - fn new(open_block: OpenBlock<'x, 'y>, uncle_bytes: Bytes) -> Self { +impl<'x> ClosedBlock<'x> { + fn new(open_block: OpenBlock<'x>, uncle_bytes: Bytes) -> Self { ClosedBlock { open_block: open_block, uncle_bytes: uncle_bytes, @@ -307,7 +307,7 @@ impl<'x, 'y> ClosedBlock<'x, 'y> { } /// Turn this back into an `OpenBlock`. - pub fn reopen(self) -> OpenBlock<'x, 'y> { self.open_block } + pub fn reopen(self) -> OpenBlock<'x> { self.open_block } /// Drop this object and return the underlieing database. pub fn drain(self) -> JournalDB { self.open_block.block.state.drop().1 } @@ -332,7 +332,7 @@ impl IsBlock for SealedBlock { } /// Enact the block given by block header, transactions and uncles -pub fn enact<'x, 'y>(header: &Header, transactions: &[SignedTransaction], uncles: &[Header], engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: &'y LastHashes) -> Result, Error> { +pub fn enact<'x>(header: &Header, transactions: &[SignedTransaction], uncles: &[Header], engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: LastHashes) -> Result, Error> { { if ::log::max_log_level() >= ::log::LogLevel::Trace { let s = State::from_existing(db.clone(), parent.state_root().clone(), engine.account_start_nonce()); @@ -350,20 +350,20 @@ pub fn enact<'x, 'y>(header: &Header, transactions: &[SignedTransaction], uncles } /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header -pub fn enact_bytes<'x, 'y>(block_bytes: &[u8], engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: &'y LastHashes) -> Result, Error> { +pub fn enact_bytes<'x>(block_bytes: &[u8], engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: LastHashes) -> Result, Error> { let block = BlockView::new(block_bytes); let header = block.header(); enact(&header, &block.transactions(), &block.uncles(), engine, db, parent, last_hashes) } /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header -pub fn enact_verified<'x, 'y>(block: &PreVerifiedBlock, engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: &'y LastHashes) -> Result, Error> { +pub fn enact_verified<'x>(block: &PreVerifiedBlock, engine: &'x Engine, db: JournalDB, parent: &Header, last_hashes: LastHashes) -> Result, Error> { let view = BlockView::new(&block.bytes); enact(&block.header, &block.transactions, &view.uncles(), engine, db, parent, last_hashes) } /// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header. Seal the block aferwards -pub fn enact_and_seal(block_bytes: &[u8], engine: &Engine, db: JournalDB, parent: &Header, last_hashes: &LastHashes) -> Result { +pub fn enact_and_seal(block_bytes: &[u8], engine: &Engine, db: JournalDB, parent: &Header, last_hashes: LastHashes) -> Result { let header = BlockView::new(block_bytes).header_view(); Ok(try!(try!(enact_bytes(block_bytes, engine, db, parent, last_hashes)).seal(header.seal()))) } @@ -384,7 +384,7 @@ mod tests { let mut db = db_result.take(); engine.spec().ensure_db_good(&mut db); let last_hashes = vec![genesis_header.hash()]; - let b = OpenBlock::new(engine.deref(), db, &genesis_header, &last_hashes, Address::zero(), vec![]); + let b = OpenBlock::new(engine.deref(), db, &genesis_header, last_hashes, Address::zero(), vec![]); let b = b.close(); let _ = b.seal(vec![]); } @@ -398,14 +398,14 @@ mod tests { let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); engine.spec().ensure_db_good(&mut db); - let b = OpenBlock::new(engine.deref(), db, &genesis_header, &vec![genesis_header.hash()], Address::zero(), vec![]).close().seal(vec![]).unwrap(); + let b = OpenBlock::new(engine.deref(), db, &genesis_header, vec![genesis_header.hash()], Address::zero(), vec![]).close().seal(vec![]).unwrap(); let orig_bytes = b.rlp_bytes(); let orig_db = b.drain(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); engine.spec().ensure_db_good(&mut db); - let e = enact_and_seal(&orig_bytes, engine.deref(), db, &genesis_header, &vec![genesis_header.hash()]).unwrap(); + let e = enact_and_seal(&orig_bytes, engine.deref(), db, &genesis_header, vec![genesis_header.hash()]).unwrap(); assert_eq!(e.rlp_bytes(), orig_bytes); diff --git a/ethcore/src/client.rs b/ethcore/src/client.rs index 3c44b97bf..2d57fbc3d 100644 --- a/ethcore/src/client.rs +++ b/ethcore/src/client.rs @@ -262,7 +262,6 @@ impl Client { fn check_and_close_block(&self, block: &PreVerifiedBlock) -> Result { let engine = self.engine.deref().deref(); let header = &block.header; - let header_hash = block.header.hash(); // Verify Block Family let verify_family_result = verify_block_family(&header, &block.bytes, engine, self.chain.read().unwrap().deref()); @@ -310,14 +309,13 @@ impl Client { let blocks = self.block_queue.write().unwrap().drain(max_blocks_to_import); for block in blocks { - let header = block.header; + let header = &block.header; if bad_blocks.contains(&header.parent_hash) { bad_blocks.insert(header.hash()); continue; } - let closed_block = self.check_and_close_block(&block); if let Err(_) = closed_block { bad_blocks.insert(header.hash()); @@ -350,7 +348,7 @@ impl Client { let bad_blocks = bad_blocks.into_iter().collect::>(); { - let block_queue = self.block_queue.write().unwrap(); + let mut block_queue = self.block_queue.write().unwrap(); block_queue.mark_as_bad(&bad_blocks); block_queue.mark_as_good(&good_blocks); } diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 8c411e7f0..54f02524d 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -282,7 +282,7 @@ mod tests { let mut db = db_result.take(); engine.spec().ensure_db_good(&mut db); let last_hashes = vec![genesis_header.hash()]; - let b = OpenBlock::new(engine.deref(), db, &genesis_header, &last_hashes, Address::zero(), vec![]); + let b = OpenBlock::new(engine.deref(), db, &genesis_header, last_hashes, Address::zero(), vec![]); let b = b.close(); assert_eq!(b.state().balance(&Address::zero()), U256::from_str("4563918244f40000").unwrap()); } @@ -295,7 +295,7 @@ mod tests { let mut db = db_result.take(); engine.spec().ensure_db_good(&mut db); let last_hashes = vec![genesis_header.hash()]; - let mut b = OpenBlock::new(engine.deref(), db, &genesis_header, &last_hashes, Address::zero(), vec![]); + let mut b = OpenBlock::new(engine.deref(), db, &genesis_header, last_hashes, Address::zero(), vec![]); let mut uncle = Header::new(); let uncle_author = address_from_hex("ef2d6d194084c2de36e0dabfce45d046b37d1106"); uncle.author = uncle_author.clone(); From 08647282dff3999353ba7dbcc8c6bac046b1245c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 24 Feb 2016 17:01:29 +0100 Subject: [PATCH 5/7] Fixing mark_as_bad implementation --- ethcore/res/ethereum/tests | 2 +- ethcore/src/block_queue.rs | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/ethcore/res/ethereum/tests b/ethcore/res/ethereum/tests index f32954b3d..3116f85a4 160000 --- a/ethcore/res/ethereum/tests +++ b/ethcore/res/ethereum/tests @@ -1 +1 @@ -Subproject commit f32954b3ddb5af2dc3dc9ec6d9a28bee848fdf70 +Subproject commit 3116f85a499ceaf4dfdc46726060fc056e2d7829 diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index b802e3c26..ff20021f2 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -287,20 +287,23 @@ impl BlockQueue { /// Mark given block and all its children as bad. Stops verification. pub fn mark_as_bad(&mut self, hashes: &[H256]) { let mut verification_lock = self.verification.lock().unwrap(); - let mut verification = verification_lock.deref_mut(); - let mut new_verified = VecDeque::new(); + let mut processing = self.processing.write().unwrap(); + let mut verification = verification_lock.deref_mut(); + + verification.bad.reserve(hashes.len()); for hash in hashes { verification.bad.insert(hash.clone()); - self.processing.write().unwrap().remove(&hash); - for block in verification.verified.drain(..) { - if verification.bad.contains(&block.header.parent_hash) { - verification.bad.insert(block.header.hash()); - self.processing.write().unwrap().remove(&block.header.hash()); - } - else { - new_verified.push_back(block); - } + processing.remove(&hash); + } + + let mut new_verified = VecDeque::new(); + for block in verification.verified.drain(..) { + if verification.bad.contains(&block.header.parent_hash) { + verification.bad.insert(block.header.hash()); + processing.remove(&block.header.hash()); + } else { + new_verified.push_back(block); } } verification.verified = new_verified; From fd63fa6836489039756f105086b8b41fac793dad Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 26 Feb 2016 17:27:56 +0100 Subject: [PATCH 6/7] Update block.rs --- ethcore/src/block.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 9051b6d9a..70c6c2fb2 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -144,7 +144,7 @@ impl IsBlock for ExecutedBlock { /// Block that is ready for transactions to be added. /// -/// It's a bit like a Vec, eccept that whenever a transaction is pushed, we execute it and +/// It's a bit like a Vec, except that whenever a transaction is pushed, we execute it and /// maintain the system `state()`. We also archive execution receipts in preparation for later block creation. pub struct OpenBlock<'x> { block: ExecutedBlock, From f118e30b209074236c4593693d3285c032ed2bc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 26 Feb 2016 19:56:32 +0100 Subject: [PATCH 7/7] Renaming variables to more descriptive --- ethcore/src/block_queue.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ethcore/src/block_queue.rs b/ethcore/src/block_queue.rs index ff20021f2..62763386f 100644 --- a/ethcore/src/block_queue.rs +++ b/ethcore/src/block_queue.rs @@ -285,14 +285,14 @@ impl BlockQueue { } /// Mark given block and all its children as bad. Stops verification. - pub fn mark_as_bad(&mut self, hashes: &[H256]) { + pub fn mark_as_bad(&mut self, block_hashes: &[H256]) { let mut verification_lock = self.verification.lock().unwrap(); let mut processing = self.processing.write().unwrap(); let mut verification = verification_lock.deref_mut(); - verification.bad.reserve(hashes.len()); - for hash in hashes { + verification.bad.reserve(block_hashes.len()); + for hash in block_hashes { verification.bad.insert(hash.clone()); processing.remove(&hash); } @@ -310,10 +310,10 @@ impl BlockQueue { } /// Mark given block as processed - pub fn mark_as_good(&mut self, hashes: &[H256]) { + pub fn mark_as_good(&mut self, block_hashes: &[H256]) { let mut processing = self.processing.write().unwrap(); - for h in hashes { - processing.remove(&h); + for hash in block_hashes { + processing.remove(&hash); } }