From 3be078c0a9de2c8d1d65ad660f0fe36e083332b8 Mon Sep 17 00:00:00 2001 From: Seun Date: Thu, 16 May 2019 11:22:10 +0100 Subject: [PATCH 1/4] delete BlockDetails from COL_EXTRA --- ethcore/src/client/client.rs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index bf980ac5fd0..0724bee0c95 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -25,7 +25,7 @@ use blockchain::{BlockReceipts, BlockChain, BlockChainDB, BlockProvider, TreeRou use bytes::Bytes; use call_contract::{CallContract, RegistryInfo}; use ethcore_miner::pool::VerifiedTransaction; -use ethereum_types::{H256, Address, U256}; +use ethereum_types::{H256, H264, Address, U256}; use evm::Schedule; use hash::keccak; use io::IoChannel; @@ -86,7 +86,7 @@ pub use types::blockchain_info::BlockChainInfo; pub use types::block_status::BlockStatus; pub use blockchain::CacheSize as BlockChainCacheSize; pub use verification::QueueInfo as BlockQueueInfo; -use db::Writable; +use db::{Writable, Readable, keys::BlockDetails}; use_contract!(registry, "res/contracts/registrar.json"); @@ -1338,27 +1338,41 @@ impl BlockChainReset for Client { for hash in &blocks_to_delete { db_transaction.delete(::db::COL_HEADERS, &hash.hash()); db_transaction.delete(::db::COL_BODIES, &hash.hash()); - db_transaction.delete(::db::COL_EXTRA, &hash.hash()); + Writable::delete::(&mut db_transaction, ::db::COL_EXTRA, &hash.hash()); Writable::delete:: (&mut db_transaction, ::db::COL_EXTRA, &hash.number()); } + let last_hash = blocks_to_delete.last().expect("blocks_to_delete isn't empty; qed").hash(); + let mut best_block_details = Readable::read::( + &**self.db.read().key_value(), + ::db::COL_EXTRA, + &best_block_hash + ).expect("best_block_details should exist; qed"); + // remove the last block as a child so that it can be re-imported + // ethcore/blockchain/src/blockchain.rs:667 + best_block_details.children.retain(|h| *h != last_hash); + db_transaction.write( + ::db::COL_EXTRA, + &best_block_hash, + &best_block_details + ); + // update the new best block hash db_transaction.put(::db::COL_EXTRA, b"best", &*best_block_hash); + let hashes = blocks_to_delete.iter().map(|b| b.hash()).collect::>(); + info!("Deleting block hashes {}", + Colour::Red + .bold() + .paint(format!("{:#?}", hashes)) + ); + self.db.read() .key_value() .write(db_transaction) .map_err(|err| format!("could not complete reset operation; io error occured: {}", err))?; - let hashes = blocks_to_delete.iter().map(|b| b.hash()).collect::>(); - - info!("Deleting block hashes {}", - Colour::Red - .bold() - .paint(format!("{:#?}", hashes)) - ); - info!("New best block hash {}", Colour::Green.bold().paint(format!("{:?}", best_block_hash))); Ok(()) From dc0d60e08aa2ce453316d3e2ebecccafeef36222 Mon Sep 17 00:00:00 2001 From: Seun Date: Thu, 16 May 2019 16:38:10 +0100 Subject: [PATCH 2/4] better proofs --- ethcore/src/client/client.rs | 6 ++++-- parity/blockchain.rs | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 0724bee0c95..181bbb6c377 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1343,12 +1343,14 @@ impl BlockChainReset for Client { (&mut db_transaction, ::db::COL_EXTRA, &hash.number()); } - let last_hash = blocks_to_delete.last().expect("blocks_to_delete isn't empty; qed").hash(); + let last_hash = blocks_to_delete.last() + .expect("num is > 0; blocks_to_delete can't be empty; qed") + .hash(); let mut best_block_details = Readable::read::( &**self.db.read().key_value(), ::db::COL_EXTRA, &best_block_hash - ).expect("best_block_details should exist; qed"); + ).expect("block was previously imported; best_block_details should exist; qed"); // remove the last block as a child so that it can be re-imported // ethcore/blockchain/src/blockchain.rs:667 best_block_details.children.retain(|h| *h != last_hash); diff --git a/parity/blockchain.rs b/parity/blockchain.rs index e4b8752c511..3f66f03f375 100644 --- a/parity/blockchain.rs +++ b/parity/blockchain.rs @@ -730,6 +730,10 @@ fn execute_export_state(cmd: ExportState) -> Result<(), String> { } fn execute_reset(cmd: ResetBlockchain) -> Result<(), String> { + if cmd.num == 0 { + return Err("Invalid reset argument".into()) + } + let service = start_client( cmd.dirs, cmd.spec, From 0a7923952416a27a5d670b912b3a25f91865469b Mon Sep 17 00:00:00 2001 From: Seun Date: Mon, 20 May 2019 15:40:04 +0100 Subject: [PATCH 3/4] added tests --- ethcore/blockchain/src/blockchain.rs | 23 ++++------- ethcore/src/client/client.rs | 59 ++++++++++++++++------------ ethcore/src/tests/client.rs | 21 +++++++++- 3 files changed, 62 insertions(+), 41 deletions(-) diff --git a/ethcore/blockchain/src/blockchain.rs b/ethcore/blockchain/src/blockchain.rs index de6e8c134a6..7ee735f789d 100644 --- a/ethcore/blockchain/src/blockchain.rs +++ b/ethcore/blockchain/src/blockchain.rs @@ -668,21 +668,6 @@ impl BlockChain { self.db.key_value().read_with_cache(db::COL_EXTRA, &self.block_details, parent).map_or(false, |d| d.children.contains(hash)) } - /// fetches the list of blocks from best block to n, and n's parent hash - /// where n > 0 - pub fn block_headers_from_best_block(&self, n: u32) -> Option<(Vec, H256)> { - let mut blocks = Vec::with_capacity(n as usize); - let mut hash = self.best_block_hash(); - - for _ in 0..n { - let current_hash = self.block_header_data(&hash)?; - hash = current_hash.parent_hash(); - blocks.push(current_hash); - } - - Some((blocks, hash)) - } - /// Returns a tree route between `from` and `to`, which is a tuple of: /// /// - a vector of hashes of all blocks, ordered from `from` to `to`. @@ -869,6 +854,14 @@ impl BlockChain { } } + /// clears all caches for testing purposes + pub fn clear_cache(&self) { + self.block_bodies.write().clear(); + self.block_details.write().clear(); + self.block_hashes.write().clear(); + self.block_headers.write().clear(); + } + /// Update the best ancient block to the given hash, after checking that /// it's directly linked to the currently known best ancient block pub fn update_best_ancient_block(&self, hash: &H256) { diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 181bbb6c377..cc3026aeea1 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1327,53 +1327,62 @@ impl BlockChainReset for Client { fn reset(&self, num: u32) -> Result<(), String> { if num as u64 > self.pruning_history() { return Err("Attempting to reset to block with pruned state".into()) + } else if num == 0 { + return Err("invalid number of blocks to reset".into()) } - let (blocks_to_delete, best_block_hash) = self.chain.read() - .block_headers_from_best_block(num) - .ok_or("Attempted to reset past genesis block")?; + let mut blocks_to_delete = Vec::with_capacity(num as usize); + let mut best_block_hash = self.chain.read().best_block_hash(); - let mut db_transaction = DBTransaction::with_capacity((num + 1) as usize); + for _ in 0..=num { + let current_header = self.chain.read().block_header_data(&best_block_hash) + .ok_or("Attempted to reset past genesis block")?; + best_block_hash = current_header.parent_hash(); + blocks_to_delete.push((current_header.number(), current_header.hash())); + } + + let hashes = blocks_to_delete.iter().map(|(_, hash)| hash).collect::>(); + info!("Deleting block hashes {}", + Colour::Red + .bold() + .paint(format!("{:#?}", hashes)) + ); + + let mut db_transaction = DBTransaction::with_capacity(blocks_to_delete.len()); + let (_, best_block_hash) = blocks_to_delete.first() + .ok_or("num is > 0; blocks can't be empty; qed")?; - for hash in &blocks_to_delete { - db_transaction.delete(::db::COL_HEADERS, &hash.hash()); - db_transaction.delete(::db::COL_BODIES, &hash.hash()); - Writable::delete::(&mut db_transaction, ::db::COL_EXTRA, &hash.hash()); + for (number, hash) in &blocks_to_delete { + db_transaction.delete(::db::COL_HEADERS, &*hash); + db_transaction.delete(::db::COL_BODIES, &*hash); + Writable::delete::(&mut db_transaction, ::db::COL_EXTRA, &*hash); Writable::delete:: - (&mut db_transaction, ::db::COL_EXTRA, &hash.number()); + (&mut db_transaction, ::db::COL_EXTRA, &*number); } - let last_hash = blocks_to_delete.last() - .expect("num is > 0; blocks_to_delete can't be empty; qed") - .hash(); let mut best_block_details = Readable::read::( &**self.db.read().key_value(), ::db::COL_EXTRA, - &best_block_hash - ).expect("block was previously imported; best_block_details should exist; qed"); + &*best_block_hash + ).ok_or("block was previously imported; best_block_details should exist; qed")?; + + let (_, last_hash) = blocks_to_delete.last() + .ok_or("num is > 0; blocks can't be empty; qed")?; // remove the last block as a child so that it can be re-imported // ethcore/blockchain/src/blockchain.rs:667 - best_block_details.children.retain(|h| *h != last_hash); + best_block_details.children.retain(|h| *h != *last_hash); db_transaction.write( ::db::COL_EXTRA, - &best_block_hash, + &*best_block_hash, &best_block_details ); - // update the new best block hash db_transaction.put(::db::COL_EXTRA, b"best", &*best_block_hash); - let hashes = blocks_to_delete.iter().map(|b| b.hash()).collect::>(); - info!("Deleting block hashes {}", - Colour::Red - .bold() - .paint(format!("{:#?}", hashes)) - ); - self.db.read() .key_value() .write(db_transaction) - .map_err(|err| format!("could not complete reset operation; io error occured: {}", err))?; + .map_err(|err| format!("could not delete blocks; io error occured: {}", err))?; info!("New best block hash {}", Colour::Green.bold().paint(format!("{:?}", best_block_hash))); diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index 4d12bb1385e..8ea79625754 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -27,7 +27,7 @@ use types::filter::Filter; use types::view; use types::views::BlockView; -use client::{BlockChainClient, Client, ClientConfig, BlockId, ChainInfo, BlockInfo, PrepareOpenBlock, ImportSealedBlock, ImportBlock}; +use client::{BlockChainClient, BlockChainReset, Client, ClientConfig, BlockId, ChainInfo, BlockInfo, PrepareOpenBlock, ImportSealedBlock, ImportBlock}; use ethereum; use executive::{Executive, TransactOptions}; use miner::{Miner, PendingOrdering, MinerService}; @@ -366,3 +366,22 @@ fn transaction_proof() { assert_eq!(state.balance(&Address::default()).unwrap(), 5.into()); assert_eq!(state.balance(&address).unwrap(), 95.into()); } + +#[test] +fn reset_blockchain() { + let client = get_test_client_with_blocks(get_good_dummy_block_seq(20)); + + assert!(client.block_header(BlockId::Number(20)).is_some()); + + assert!(client.reset(5).is_ok()); + + client.chain().clear_cache(); + + assert!(client.block_header(BlockId::Number(20)).is_none()); + assert!(client.block_header(BlockId::Number(19)).is_none()); + assert!(client.block_header(BlockId::Number(18)).is_none()); + assert!(client.block_header(BlockId::Number(17)).is_none()); + assert!(client.block_header(BlockId::Number(16)).is_none()); + + assert!(client.block_header(BlockId::Number(15)).is_some()); +} From 5e59935815a276e0219d62243d4d3d8348bd2110 Mon Sep 17 00:00:00 2001 From: Seun Date: Tue, 21 May 2019 11:56:56 +0100 Subject: [PATCH 4/4] PR suggestions --- ethcore/src/client/client.rs | 46 +++++++++++++++++------------------- ethcore/src/tests/client.rs | 5 ++-- parity/blockchain.rs | 4 ---- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index cc3026aeea1..53407d6092b 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1333,12 +1333,22 @@ impl BlockChainReset for Client { let mut blocks_to_delete = Vec::with_capacity(num as usize); let mut best_block_hash = self.chain.read().best_block_hash(); + let mut batch = DBTransaction::with_capacity(blocks_to_delete.len()); - for _ in 0..=num { + for _ in 0..num { let current_header = self.chain.read().block_header_data(&best_block_hash) - .ok_or("Attempted to reset past genesis block")?; + .expect("best_block_hash was fetched from db; block_header_data should exist in db; qed"); best_block_hash = current_header.parent_hash(); - blocks_to_delete.push((current_header.number(), current_header.hash())); + + let (number, hash) = (current_header.number(), current_header.hash()); + batch.delete(::db::COL_HEADERS, &hash); + batch.delete(::db::COL_BODIES, &hash); + Writable::delete:: + (&mut batch, ::db::COL_EXTRA, &hash); + Writable::delete:: + (&mut batch, ::db::COL_EXTRA, &number); + + blocks_to_delete.push((number, hash)); } let hashes = blocks_to_delete.iter().map(|(_, hash)| hash).collect::>(); @@ -1348,41 +1358,29 @@ impl BlockChainReset for Client { .paint(format!("{:#?}", hashes)) ); - let mut db_transaction = DBTransaction::with_capacity(blocks_to_delete.len()); - let (_, best_block_hash) = blocks_to_delete.first() - .ok_or("num is > 0; blocks can't be empty; qed")?; - - for (number, hash) in &blocks_to_delete { - db_transaction.delete(::db::COL_HEADERS, &*hash); - db_transaction.delete(::db::COL_BODIES, &*hash); - Writable::delete::(&mut db_transaction, ::db::COL_EXTRA, &*hash); - Writable::delete:: - (&mut db_transaction, ::db::COL_EXTRA, &*number); - } - let mut best_block_details = Readable::read::( &**self.db.read().key_value(), ::db::COL_EXTRA, - &*best_block_hash - ).ok_or("block was previously imported; best_block_details should exist; qed")?; + &best_block_hash + ).expect("block was previously imported; best_block_details should exist; qed"); let (_, last_hash) = blocks_to_delete.last() - .ok_or("num is > 0; blocks can't be empty; qed")?; + .expect("num is > 0; blocks_to_delete can't be empty; qed"); // remove the last block as a child so that it can be re-imported - // ethcore/blockchain/src/blockchain.rs:667 + // ethcore/blockchain/src/blockchain.rs/Blockchain::is_known_child() best_block_details.children.retain(|h| *h != *last_hash); - db_transaction.write( + batch.write( ::db::COL_EXTRA, - &*best_block_hash, + &best_block_hash, &best_block_details ); // update the new best block hash - db_transaction.put(::db::COL_EXTRA, b"best", &*best_block_hash); + batch.put(::db::COL_EXTRA, b"best", &best_block_hash); self.db.read() .key_value() - .write(db_transaction) - .map_err(|err| format!("could not delete blocks; io error occured: {}", err))?; + .write(batch) + .map_err(|err| format!("could not delete blocks; io error occurred: {}", err))?; info!("New best block hash {}", Colour::Green.bold().paint(format!("{:?}", best_block_hash))); diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index 8ea79625754..9086d07e08a 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -369,9 +369,10 @@ fn transaction_proof() { #[test] fn reset_blockchain() { - let client = get_test_client_with_blocks(get_good_dummy_block_seq(20)); - + let client = get_test_client_with_blocks(get_good_dummy_block_seq(19)); + // 19 + genesis block assert!(client.block_header(BlockId::Number(20)).is_some()); + assert_eq!(client.block_header(BlockId::Number(20)).unwrap().hash(), client.best_block_header().hash()); assert!(client.reset(5).is_ok()); diff --git a/parity/blockchain.rs b/parity/blockchain.rs index 3f66f03f375..e4b8752c511 100644 --- a/parity/blockchain.rs +++ b/parity/blockchain.rs @@ -730,10 +730,6 @@ fn execute_export_state(cmd: ExportState) -> Result<(), String> { } fn execute_reset(cmd: ResetBlockchain) -> Result<(), String> { - if cmd.num == 0 { - return Err("Invalid reset argument".into()) - } - let service = start_client( cmd.dirs, cmd.spec,