More robust block pruning (#1637)

* more robust block deletion

* rustfmt

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;

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()))
 	}
 

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]