From 4d70968e70219b0dbf038ba4bfcbd5cb51e74827 Mon Sep 17 00:00:00 2001
From: Antioch Peverell <apeverell@protonmail.com>
Date: Tue, 2 Oct 2018 16:17:15 +0100
Subject: [PATCH] More robust block pruning (#1637)

* more robust block deletion

* rustfmt
---
 chain/src/chain.rs           |  4 +---
 chain/src/store.rs           | 16 +++++++++++++---
 chain/tests/store_indices.rs | 22 ++++++++++++++++++++++
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/chain/src/chain.rs b/chain/src/chain.rs
index 3970adbed..fedc2d469 100644
--- a/chain/src/chain.rs
+++ b/chain/src/chain.rs
@@ -776,10 +776,8 @@ impl Chain {
 		loop {
 			match self.store.get_block(&current.hash()) {
 				Ok(b) => {
-					count += 1;
 					batch.delete_block(&b.hash())?;
-					batch.delete_block_input_bitmap(&b.hash())?;
-					batch.delete_block_sums(&b.hash())?;
+					count += 1;
 				}
 				Err(NotFoundErr(_)) => {
 					break;
diff --git a/chain/src/store.rs b/chain/src/store.rs
index e7c380be7..1f87b6b1a 100644
--- a/chain/src/store.rs
+++ b/chain/src/store.rs
@@ -243,7 +243,17 @@ impl<'a> Batch<'a> {
 	/// Delete a full block. Does not delete any record associated with a block
 	/// header.
 	pub fn delete_block(&self, bh: &Hash) -> Result<(), Error> {
-		self.db.delete(&to_key(BLOCK_PREFIX, &mut bh.to_vec())[..])
+		self.db
+			.delete(&to_key(BLOCK_PREFIX, &mut bh.to_vec())[..])?;
+
+		// Best effort at deleting associated data for this block.
+		// Not an error if these fail.
+		{
+			let _ = self.delete_block_sums(bh);
+			let _ = self.delete_block_input_bitmap(bh);
+		}
+
+		Ok(())
 	}
 
 	pub fn save_block_header(&self, bh: &BlockHeader) -> Result<(), Error> {
@@ -297,7 +307,7 @@ impl<'a> Batch<'a> {
 		)
 	}
 
-	pub fn delete_block_input_bitmap(&self, bh: &Hash) -> Result<(), Error> {
+	fn delete_block_input_bitmap(&self, bh: &Hash) -> Result<(), Error> {
 		self.db
 			.delete(&to_key(BLOCK_INPUT_BITMAP_PREFIX, &mut bh.to_vec()))
 	}
@@ -315,7 +325,7 @@ impl<'a> Batch<'a> {
 		)
 	}
 
-	pub fn delete_block_sums(&self, bh: &Hash) -> Result<(), Error> {
+	fn delete_block_sums(&self, bh: &Hash) -> Result<(), Error> {
 		self.db.delete(&to_key(BLOCK_SUMS_PREFIX, &mut bh.to_vec()))
 	}
 
diff --git a/chain/tests/store_indices.rs b/chain/tests/store_indices.rs
index 71361aa93..1e6664e6a 100644
--- a/chain/tests/store_indices.rs
+++ b/chain/tests/store_indices.rs
@@ -79,6 +79,28 @@ fn test_various_store_indices() {
 
 	let block_header = chain_store.get_header_by_height(1).unwrap();
 	assert_eq!(block_header.hash(), block_hash);
+
+	// Test we can retrive the block from the db and that we can safely delete the
+	// block from the db even though the block_sums are missing.
+	{
+		// Block exists in the db.
+		assert!(chain_store.get_block(&block_hash).is_ok());
+
+		// Block sums do not exist (we never set them up).
+		assert!(chain_store.get_block_sums(&block_hash).is_err());
+
+		{
+			// Start a new batch and delete the block.
+			let batch = chain_store.batch().unwrap();
+			assert!(batch.delete_block(&block_hash).is_ok());
+
+			// Block is deleted within this batch.
+			assert!(batch.get_block(&block_hash).is_err());
+		}
+
+		// Check the batch did not commit any changes to the store .
+		assert!(chain_store.get_block(&block_hash).is_ok());
+	}
 }
 
 #[test]