Reset blockchain properly (#10669)

* delete BlockDetails from COL_EXTRA

* better proofs

* added tests

* PR suggestions

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<encoded::Header>, 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) {

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");
 
@@ -1327,38 +1327,61 @@ 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()
+		let mut blocks_to_delete = Vec::with_capacity(num as usize);
-			.block_headers_from_best_block(num)
+		let mut best_block_hash = self.chain.read().best_block_hash();
-			.ok_or("Attempted to reset past genesis block")?;
+		let mut batch = DBTransaction::with_capacity(blocks_to_delete.len());
 
-		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)
+				.expect("best_block_hash was fetched from db; block_header_data should exist in db; qed");
+			best_block_hash = current_header.parent_hash();
 
-		for hash in &blocks_to_delete {
+			let (number, hash) = (current_header.number(), current_header.hash());
-			db_transaction.delete(::db::COL_HEADERS, &hash.hash());
+			batch.delete(::db::COL_HEADERS, &hash);
-			db_transaction.delete(::db::COL_BODIES, &hash.hash());
+			batch.delete(::db::COL_BODIES, &hash);
-			db_transaction.delete(::db::COL_EXTRA, &hash.hash());
+			Writable::delete::<BlockDetails, H264>
+				(&mut batch, ::db::COL_EXTRA, &hash);
 			Writable::delete::<H256, BlockNumberKey>
-				(&mut db_transaction, ::db::COL_EXTRA, &hash.number());
+				(&mut batch, ::db::COL_EXTRA, &number);
+
+			blocks_to_delete.push((number, hash));
 		}
 
-		// update the new best block hash
+		let hashes = blocks_to_delete.iter().map(|(_, hash)| hash).collect::<Vec<_>>();
-		db_transaction.put(::db::COL_EXTRA, b"best", &*best_block_hash);
-
-		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::<Vec<_>>();
-
 		info!("Deleting block hashes {}",
 			  Colour::Red
 				  .bold()
 				  .paint(format!("{:#?}", hashes))
 		);
 
+		let mut best_block_details = Readable::read::<BlockDetails, H264>(
+			&**self.db.read().key_value(),
+			::db::COL_EXTRA,
+			&best_block_hash
+		).expect("block was previously imported; best_block_details should exist; qed");
+
+		let (_, last_hash) = blocks_to_delete.last()
+			.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/Blockchain::is_known_child()
+		best_block_details.children.retain(|h| *h != *last_hash);
+		batch.write(
+			::db::COL_EXTRA,
+			&best_block_hash,
+			&best_block_details
+		);
+		// update the new best block hash
+		batch.put(::db::COL_EXTRA, b"best", &best_block_hash);
+
+		self.db.read()
+			.key_value()
+			.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)));
 
 		Ok(())

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,23 @@ 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(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());
+
+	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());
+}