From 3f4f165e0bda7410ca0c0197da9a44b22070fd66 Mon Sep 17 00:00:00 2001
From: Yeastplume <yeastplume@protonmail.com>
Date: Tue, 9 Nov 2021 15:34:10 +0000
Subject: [PATCH] PMMR Backend Support for append_pruned_root (Continued)
 (#3659)

* refactor prune_list with aim of allowing pruned subtree appending

* add test coverage around pmmr::is_leaf() and pmmr::bintree_leaf_pos_iter()

* comments

* cleanup

* implement append pruned subtree for prune_list

* commit

* we can now append to prune_list

* fix our prune_list corruption...

* rework how we rewrite the prune list during compaction

* test coverage for improved prune list api

* continuing to merge

* finish merge, tests passing again

* add function pmmr_leaf_to_insertion_index, and modify bintree_lef_pos_iter to use it. Note there's still an unwrap that needs to be dealt with sanely

* change pmmr_leaf_to_insertion_index to simpler version + handle conversion between 1 and 0 based in bintree_leaf_pos_iter

Co-authored-by: antiochp <30642645+antiochp@users.noreply.github.com>
---
 chain/tests/mine_simple_chain.rs  |   1 +
 core/src/core/pmmr/backend.rs     |   4 +
 core/src/core/pmmr/pmmr.rs        |  51 ++++++--
 core/src/core/pmmr/vec_backend.rs |   4 +
 core/tests/pmmr.rs                |  75 +++++++++++-
 store/src/pmmr.rs                 |  23 +++-
 store/src/prune_list.rs           | 152 +++++++++++++++++++-----
 store/tests/prune_list.rs         | 186 ++++++++++++++++++------------
 8 files changed, 382 insertions(+), 114 deletions(-)

diff --git a/chain/tests/mine_simple_chain.rs b/chain/tests/mine_simple_chain.rs
index d05bbffab8..71fb36ac2f 100644
--- a/chain/tests/mine_simple_chain.rs
+++ b/chain/tests/mine_simple_chain.rs
@@ -754,6 +754,7 @@ fn output_header_mappings() {
 	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 	util::init_test_logger();
 	{
+		clean_output_dir(".grin_header_for_output");
 		let chain = init_chain(
 			".grin_header_for_output",
 			pow::mine_genesis_block().unwrap(),
diff --git a/core/src/core/pmmr/backend.rs b/core/src/core/pmmr/backend.rs
index f1ce870fdf..c7f0de72af 100644
--- a/core/src/core/pmmr/backend.rs
+++ b/core/src/core/pmmr/backend.rs
@@ -29,6 +29,10 @@ pub trait Backend<T: PMMRable> {
 	/// help the implementation.
 	fn append(&mut self, data: &T, hashes: &[Hash]) -> Result<(), String>;
 
+	/// Rebuilding a PMMR locally from PIBD segments requires pruned subtree support.
+	/// This allows us to append an existing pruned subtree directly without the underlying leaf nodes.
+	fn append_pruned_subtree(&mut self, hash: Hash, pos: u64) -> Result<(), String>;
+
 	/// Rewind the backend state to a previous position, as if all append
 	/// operations after that had been canceled. Expects a position in the PMMR
 	/// to rewind to as well as bitmaps representing the positions added and
diff --git a/core/src/core/pmmr/pmmr.rs b/core/src/core/pmmr/pmmr.rs
index 8204237825..a6c227b3c3 100644
--- a/core/src/core/pmmr/pmmr.rs
+++ b/core/src/core/pmmr/pmmr.rs
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{marker, ops::Range, u64};
+use std::{cmp::max, iter, marker, ops::Range, u64};
 
 use croaring::Bitmap;
 
@@ -495,6 +495,19 @@ pub fn insertion_to_pmmr_index(mut sz: u64) -> u64 {
 	2 * sz - sz.count_ones() as u64 + 1
 }
 
+/// Returns the insertion index of the given leaf index
+pub fn pmmr_leaf_to_insertion_index(pos1: u64) -> Option<u64> {
+	if pos1 == 0 {
+		return None;
+	}
+	let (insert_idx, height) = peak_map_height(pos1 - 1);
+	if height == 0 {
+		Some(insert_idx)
+	} else {
+		None
+	}
+}
+
 /// sizes of peaks and height of next node in mmr of given size
 /// Example: on input 5 returns ([3,1], 1) as mmr state before adding 5 was
 ///    2
@@ -568,6 +581,9 @@ pub fn bintree_postorder_height(num: u64) -> u64 {
 /// of any size (somewhat unintuitively but this is how the PMMR is "append
 /// only").
 pub fn is_leaf(pos: u64) -> bool {
+	if pos == 0 {
+		return false;
+	}
 	bintree_postorder_height(pos) == 0
 }
 
@@ -665,14 +681,35 @@ pub fn family_branch(pos: u64, last_pos: u64) -> Vec<(u64, u64)> {
 }
 
 /// Gets the position of the rightmost node (i.e. leaf) beneath the provided subtree root.
-pub fn bintree_rightmost(num: u64) -> u64 {
-	num - bintree_postorder_height(num)
+pub fn bintree_rightmost(pos1: u64) -> u64 {
+	pos1 - bintree_postorder_height(pos1)
 }
 
-/// Gets the position of the rightmost node (i.e. leaf) beneath the provided subtree root.
-pub fn bintree_leftmost(num: u64) -> u64 {
-	let height = bintree_postorder_height(num);
-	num + 2 - (2 << height)
+/// Gets the position of the leftmost node (i.e. leaf) beneath the provided subtree root.
+pub fn bintree_leftmost(pos1: u64) -> u64 {
+	let height = bintree_postorder_height(pos1);
+	pos1 + 2 - (2 << height)
+}
+
+/// Iterator over all leaf pos beneath the provided subtree root (including the root itself).
+pub fn bintree_leaf_pos_iter(pos1: u64) -> Box<dyn Iterator<Item = u64>> {
+	let leaf_start = pmmr_leaf_to_insertion_index(bintree_leftmost(pos1));
+	let leaf_end = pmmr_leaf_to_insertion_index(bintree_rightmost(pos1));
+	let leaf_start = match leaf_start {
+		Some(l) => l,
+		None => return Box::new(iter::empty::<u64>()),
+	};
+	let leaf_end = match leaf_end {
+		Some(l) => l,
+		None => return Box::new(iter::empty::<u64>()),
+	};
+	Box::new((leaf_start..=leaf_end).map(|n| insertion_to_pmmr_index(n + 1)))
+}
+
+/// Iterator over all pos beneath the provided subtree root (including the root itself).
+pub fn bintree_pos_iter(pos1: u64) -> impl Iterator<Item = u64> {
+	let leaf_start = max(1, bintree_leftmost(pos1 as u64));
+	(leaf_start..=pos1).into_iter()
 }
 
 /// All pos in the subtree beneath the provided root, including root itself.
diff --git a/core/src/core/pmmr/vec_backend.rs b/core/src/core/pmmr/vec_backend.rs
index c281be1fa4..ea7f8b70a4 100644
--- a/core/src/core/pmmr/vec_backend.rs
+++ b/core/src/core/pmmr/vec_backend.rs
@@ -43,6 +43,10 @@ impl<T: PMMRable> Backend<T> for VecBackend<T> {
 		Ok(())
 	}
 
+	fn append_pruned_subtree(&mut self, _hash: Hash, _pos: u64) -> Result<(), String> {
+		unimplemented!()
+	}
+
 	fn get_hash(&self, position: u64) -> Option<Hash> {
 		if self.removed.contains(&position) {
 			None
diff --git a/core/tests/pmmr.rs b/core/tests/pmmr.rs
index 70e5549eda..8119966946 100644
--- a/core/tests/pmmr.rs
+++ b/core/tests/pmmr.rs
@@ -20,7 +20,6 @@ use self::core::ser::PMMRIndexHashable;
 use crate::common::TestElem;
 use chrono::prelude::Utc;
 use grin_core as core;
-use std::u64;
 
 #[test]
 fn some_peak_map() {
@@ -128,6 +127,80 @@ fn test_bintree_leftmost() {
 	assert_eq!(pmmr::bintree_leftmost(7), 1);
 }
 
+#[test]
+fn test_bintree_leaf_pos_iter() {
+	assert_eq!(pmmr::bintree_leaf_pos_iter(0).count(), 0);
+	assert_eq!(pmmr::bintree_leaf_pos_iter(1).collect::<Vec<_>>(), [1]);
+	assert_eq!(pmmr::bintree_leaf_pos_iter(2).collect::<Vec<_>>(), [2]);
+	assert_eq!(pmmr::bintree_leaf_pos_iter(3).collect::<Vec<_>>(), [1, 2]);
+	assert_eq!(pmmr::bintree_leaf_pos_iter(4).collect::<Vec<_>>(), [4]);
+	assert_eq!(pmmr::bintree_leaf_pos_iter(5).collect::<Vec<_>>(), [5]);
+	assert_eq!(pmmr::bintree_leaf_pos_iter(6).collect::<Vec<_>>(), [4, 5]);
+	assert_eq!(
+		pmmr::bintree_leaf_pos_iter(7).collect::<Vec<_>>(),
+		[1, 2, 4, 5]
+	);
+}
+
+#[test]
+fn test_bintree_pos_iter() {
+	assert_eq!(pmmr::bintree_pos_iter(0).count(), 0);
+	assert_eq!(pmmr::bintree_pos_iter(1).collect::<Vec<_>>(), [1]);
+	assert_eq!(pmmr::bintree_pos_iter(2).collect::<Vec<_>>(), [2]);
+	assert_eq!(pmmr::bintree_pos_iter(3).collect::<Vec<_>>(), [1, 2, 3]);
+	assert_eq!(pmmr::bintree_pos_iter(4).collect::<Vec<_>>(), [4]);
+	assert_eq!(pmmr::bintree_pos_iter(5).collect::<Vec<_>>(), [5]);
+	assert_eq!(pmmr::bintree_pos_iter(6).collect::<Vec<_>>(), [4, 5, 6]);
+	assert_eq!(
+		pmmr::bintree_pos_iter(7).collect::<Vec<_>>(),
+		[1, 2, 3, 4, 5, 6, 7]
+	);
+}
+
+#[test]
+fn test_is_leaf() {
+	assert_eq!(pmmr::is_leaf(0), false);
+	assert_eq!(pmmr::is_leaf(1), true);
+	assert_eq!(pmmr::is_leaf(2), true);
+	assert_eq!(pmmr::is_leaf(3), false);
+	assert_eq!(pmmr::is_leaf(4), true);
+	assert_eq!(pmmr::is_leaf(5), true);
+	assert_eq!(pmmr::is_leaf(6), false);
+	assert_eq!(pmmr::is_leaf(7), false);
+}
+
+#[test]
+fn test_pmmr_leaf_to_insertion_index() {
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(1), Some(0));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(2), Some(1));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(4), Some(2));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(5), Some(3));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(8), Some(4));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(9), Some(5));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(11), Some(6));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(12), Some(7));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(16), Some(8));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(17), Some(9));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(19), Some(10));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(20), Some(11));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(23), Some(12));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(24), Some(13));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(26), Some(14));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(27), Some(15));
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(32), Some(16));
+
+	// Not a leaf node
+	assert_eq!(pmmr::pmmr_leaf_to_insertion_index(31), None);
+
+	// Sanity check to make sure we don't get an explosion around the u64 max
+	// number of leaves
+	let n_leaves_max_u64 = pmmr::n_leaves(u64::MAX - 256);
+	assert_eq!(
+		pmmr::pmmr_leaf_to_insertion_index(n_leaves_max_u64),
+		Some(4611686018427387884)
+	);
+}
+
 #[test]
 fn test_n_leaves() {
 	// make sure we handle an empty MMR correctly
diff --git a/store/src/pmmr.rs b/store/src/pmmr.rs
index 72f22d6c4d..f62118b0d1 100644
--- a/store/src/pmmr.rs
+++ b/store/src/pmmr.rs
@@ -65,7 +65,6 @@ pub struct PMMRBackend<T: PMMRable> {
 impl<T: PMMRable> Backend<T> for PMMRBackend<T> {
 	/// Append the provided data and hashes to the backend storage.
 	/// Add the new leaf pos to our leaf_set if this is a prunable MMR.
-	#[allow(unused_variables)]
 	fn append(&mut self, data: &T, hashes: &[Hash]) -> Result<(), String> {
 		let size = self
 			.data_file
@@ -86,6 +85,22 @@ impl<T: PMMRable> Backend<T> for PMMRBackend<T> {
 		Ok(())
 	}
 
+	// Supports appending a pruned subtree (single root hash) to an existing hash file.
+	// Update the prune_list "shift cache" to reflect the new pruned leaf pos in the subtree.
+	fn append_pruned_subtree(&mut self, hash: Hash, pos: u64) -> Result<(), String> {
+		if !self.prunable {
+			return Err("Not prunable, cannot append pruned subtree.".into());
+		}
+
+		self.hash_file
+			.append(&hash)
+			.map_err(|e| format!("Failed to append subtree hash to file. {}", e))?;
+
+		self.prune_list.append(pos);
+
+		Ok(())
+	}
+
 	fn get_from_file(&self, position: u64) -> Option<Hash> {
 		if self.is_compacted(position) {
 			return None;
@@ -402,9 +417,9 @@ impl<T: PMMRable> PMMRBackend<T> {
 
 		// Update the prune list and write to disk.
 		{
-			for pos in leaves_removed.iter() {
-				self.prune_list.add(pos.into());
-			}
+			let mut bitmap = self.prune_list.bitmap();
+			bitmap.or_inplace(&leaves_removed);
+			self.prune_list = PruneList::new(Some(self.data_dir.join(PMMR_PRUN_FILE)), bitmap);
 			self.prune_list.flush()?;
 		}
 
diff --git a/store/src/prune_list.rs b/store/src/prune_list.rs
index 0616b19066..8f488e5948 100644
--- a/store/src/prune_list.rs
+++ b/store/src/prune_list.rs
@@ -30,7 +30,7 @@ use std::{
 use croaring::Bitmap;
 use grin_core::core::pmmr;
 
-use crate::core::core::pmmr::{bintree_postorder_height, family};
+use crate::core::core::pmmr::{bintree_leftmost, bintree_postorder_height, family};
 use crate::{read_bitmap, save_via_temp_file};
 
 /// Maintains a list of previously pruned nodes in PMMR, compacting the list as
@@ -44,6 +44,7 @@ use crate::{read_bitmap, save_via_temp_file};
 /// but positions of a node within the PMMR will not match positions in the
 /// backend storage anymore. The PruneList accounts for that mismatch and does
 /// the position translation.
+#[derive(Debug)]
 pub struct PruneList {
 	path: Option<PathBuf>,
 	/// Bitmap representing pruned root node positions.
@@ -54,16 +55,21 @@ pub struct PruneList {
 
 impl PruneList {
 	/// Instantiate a new prune list from the provided path and bitmap.
-	pub fn new(path: Option<PathBuf>, mut bitmap: Bitmap) -> PruneList {
-		// Note: prune list is 1-indexed so remove any 0 value for safety.
-		bitmap.remove(0);
-
-		PruneList {
+	/// Note: Does not flush the bitmap to disk. Caller is responsible for doing this.
+	pub fn new(path: Option<PathBuf>, bitmap: Bitmap) -> PruneList {
+		let mut prune_list = PruneList {
 			path,
-			bitmap,
+			bitmap: Bitmap::create(),
 			shift_cache: vec![],
 			leaf_shift_cache: vec![],
+		};
+
+		for pos in bitmap.iter().filter(|x| *x > 0) {
+			prune_list.append(pos as u64)
 		}
+
+		prune_list.bitmap.run_optimize();
+		prune_list
 	}
 
 	/// Instatiate a new empty prune list.
@@ -72,6 +78,7 @@ impl PruneList {
 	}
 
 	/// Open an existing prune_list or create a new one.
+	/// Takes an optional bitmap of new pruned pos to be combined with existing pos.
 	pub fn open<P: AsRef<Path>>(path: P) -> io::Result<PruneList> {
 		let file_path = PathBuf::from(path.as_ref());
 		let bitmap = if file_path.exists() {
@@ -82,7 +89,7 @@ impl PruneList {
 
 		let mut prune_list = PruneList::new(Some(file_path), bitmap);
 
-		// Now built the shift and pruned caches from the bitmap we read from disk.
+		// Now build the shift caches from the bitmap we read from disk
 		prune_list.init_caches();
 
 		if !prune_list.bitmap.is_empty() {
@@ -105,8 +112,6 @@ impl PruneList {
 	}
 
 	/// Save the prune_list to disk.
-	/// Clears out leaf pos before saving to disk
-	/// as we track these via the leaf_set.
 	pub fn flush(&mut self) -> io::Result<()> {
 		// Run the optimization step on the bitmap.
 		self.bitmap.run_optimize();
@@ -118,10 +123,6 @@ impl PruneList {
 			})?;
 		}
 
-		// Rebuild our "shift caches" here as we are flushing changes to disk
-		// and the contents of our prune_list has likely changed.
-		self.init_caches();
-
 		Ok(())
 	}
 
@@ -179,6 +180,18 @@ impl PruneList {
 		}
 	}
 
+	// Calculate the next shift based on provided pos and the previous shift.
+	fn calculate_next_shift(&self, pos: u64) -> u64 {
+		let prev_shift = self.get_shift(pos.saturating_sub(1));
+		let shift = if self.is_pruned_root(pos) {
+			let height = bintree_postorder_height(pos);
+			2 * ((1 << height) - 1)
+		} else {
+			0
+		};
+		prev_shift + shift
+	}
+
 	/// As above, but only returning the number of leaf nodes to skip for a
 	/// given leaf. Helpful if, for instance, data for each leaf is being stored
 	/// separately in a continuous flat-file.
@@ -225,27 +238,86 @@ impl PruneList {
 		}
 	}
 
-	/// Push the node at the provided position in the prune list. Compacts the
-	/// list if pruning the additional node means a parent can get pruned as
-	/// well.
-	pub fn add(&mut self, pos: u64) {
+	// Calculate the next leaf shift based on provided pos and the previous leaf shift.
+	fn calculate_next_leaf_shift(&self, pos: u64) -> u64 {
+		let prev_shift = self.get_leaf_shift(pos.saturating_sub(1) as u64);
+		let shift = if self.is_pruned_root(pos) {
+			let height = bintree_postorder_height(pos);
+			if height == 0 {
+				0
+			} else {
+				1 << height
+			}
+		} else {
+			0
+		};
+		prev_shift + shift
+	}
+
+	// Remove any existing entries in shift_cache and leaf_shift_cache
+	// for any pos contained in the subtree with provided root.
+	fn cleanup_subtree(&mut self, pos: u64) {
 		assert!(pos > 0, "prune list 1-indexed, 0 not valid pos");
 
-		if self.is_pruned(pos) {
+		let lc = bintree_leftmost(pos) as u32;
+		let last_pos = self.bitmap.maximum().unwrap_or(1);
+
+		// If this subtree does not intersect with existing bitmap then nothing to cleanup.
+		if lc > last_pos {
 			return;
 		}
 
-		let mut current = pos;
-		loop {
-			let (parent, sibling) = family(current);
-			if self.is_pruned_root(sibling) {
-				current = parent;
-			} else {
-				// replace the entire subtree with the single pruned root
-				self.bitmap.remove_range(pmmr::bintree_range(current));
-				self.bitmap.add(current as u32);
-				break;
-			}
+		// Note: We will treat this as a "closed range" below (croaring api weirdness).
+		let cleanup_pos = lc..last_pos;
+
+		// Find point where we can truncate based on bitmap "rank" (index) of pos to the left of subtree.
+		let idx = self.bitmap.rank(lc - 1);
+		self.shift_cache.truncate(idx as usize);
+		self.leaf_shift_cache.truncate(idx as usize);
+
+		self.bitmap.remove_range_closed(cleanup_pos)
+	}
+
+	/// Push the node at the provided position in the prune list.
+	/// Assumes rollup of siblings and children has already been handled.
+	fn append_single(&mut self, pos: u64) {
+		assert!(pos > 0, "prune list 1-indexed, 0 not valid pos");
+		assert!(
+			pos > self.bitmap.maximum().unwrap_or(0) as u64,
+			"prune list append only"
+		);
+
+		// Add this pos to the bitmap (leaf or subtree root)
+		self.bitmap.add(pos as u32);
+
+		// Calculate shift and leaf_shift for this pos.
+		self.shift_cache.push(self.calculate_next_shift(pos));
+		self.leaf_shift_cache
+			.push(self.calculate_next_leaf_shift(pos));
+	}
+
+	/// Push the node at the provided position in the prune list.
+	/// Handles rollup of siblings and children as we go (relatively slow).
+	/// Once we find a subtree root that can not be rolled up any further
+	/// we cleanup everything beneath it and replace it with a single appended node.
+	pub fn append(&mut self, pos: u64) {
+		assert!(pos > 0, "prune list 1-indexed, 0 not valid pos");
+		assert!(
+			pos > self.bitmap.maximum().unwrap_or(0) as u64,
+			"prune list append only - pos={} bitmap.maximum={}",
+			pos,
+			self.bitmap.maximum().unwrap_or(0)
+		);
+
+		let (parent, sibling) = family(pos);
+		if self.is_pruned(sibling) {
+			// Recursively append the parent (removing our sibling in the process).
+			self.append(parent)
+		} else {
+			// Make sure we roll anything beneath this up into this higher level pruned subtree root.
+			// We should have no nested entries in the prune_list.
+			self.cleanup_subtree(pos);
+			self.append_single(pos);
 		}
 	}
 
@@ -276,6 +348,21 @@ impl PruneList {
 		}
 	}
 
+	/// Convert the prune_list to a vec of pos.
+	pub fn to_vec(&self) -> Vec<u64> {
+		self.bitmap.iter().map(|x| x as u64).collect()
+	}
+
+	/// Internal shift cache as slice.
+	pub fn shift_cache(&self) -> &[u64] {
+		self.shift_cache.as_slice()
+	}
+
+	/// Internal leaf shift cache as slice.
+	pub fn leaf_shift_cache(&self) -> &[u64] {
+		self.leaf_shift_cache.as_slice()
+	}
+
 	/// Is the specified position a root of a pruned subtree?
 	pub fn is_pruned_root(&self, pos: u64) -> bool {
 		assert!(pos > 0, "prune list 1-indexed, 0 not valid pos");
@@ -304,6 +391,11 @@ impl PruneList {
 	pub fn unpruned_leaf_iter(&self, cutoff_pos: u64) -> impl Iterator<Item = u64> + '_ {
 		self.unpruned_iter(cutoff_pos).filter(|x| pmmr::is_leaf(*x))
 	}
+
+	/// Return a clone of our internal bitmap.
+	pub fn bitmap(&self) -> Bitmap {
+		self.bitmap.clone()
+	}
 }
 
 struct UnprunedIterator<I> {
diff --git a/store/tests/prune_list.rs b/store/tests/prune_list.rs
index fff180faa8..056118f841 100644
--- a/store/tests/prune_list.rs
+++ b/store/tests/prune_list.rs
@@ -41,7 +41,8 @@ fn test_is_pruned() {
 	assert_eq!(pl.is_pruned(2), false);
 	assert_eq!(pl.is_pruned(3), false);
 
-	pl.add(2);
+	pl.append(2);
+	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [2]);
 	assert_eq!(pl.is_pruned(1), false);
@@ -49,8 +50,10 @@ fn test_is_pruned() {
 	assert_eq!(pl.is_pruned(3), false);
 	assert_eq!(pl.is_pruned(4), false);
 
-	pl.add(2);
-	pl.add(1);
+	let mut pl = PruneList::empty();
+	pl.append(1);
+	pl.append(2);
+	pl.flush().unwrap();
 
 	assert_eq!(pl.len(), 1);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [3]);
@@ -59,26 +62,19 @@ fn test_is_pruned() {
 	assert_eq!(pl.is_pruned(3), true);
 	assert_eq!(pl.is_pruned(4), false);
 
-	pl.add(4);
+	pl.append(4);
+
+	// Flushing the prune_list removes any individual leaf positions.
+	// This assumes we will track these outside the prune_list via the leaf_set.
+	pl.flush().unwrap();
 
 	assert_eq!(pl.len(), 2);
-	assert_eq!(pl.iter().collect::<Vec<_>>(), [3, 4]);
+	assert_eq!(pl.to_vec(), [3, 4]);
 	assert_eq!(pl.is_pruned(1), true);
 	assert_eq!(pl.is_pruned(2), true);
 	assert_eq!(pl.is_pruned(3), true);
 	assert_eq!(pl.is_pruned(4), true);
 	assert_eq!(pl.is_pruned(5), false);
-
-	// Test some poorly organized (out of order, overlapping) pruning.
-	let mut pl = PruneList::empty();
-	pl.add(2);
-	pl.add(4);
-	pl.add(3);
-	assert_eq!(pl.iter().collect::<Vec<_>>(), [3, 4]);
-
-	// now add a higher level pruned root clearing out the subtree.
-	pl.add(7);
-	assert_eq!(pl.iter().collect::<Vec<_>>(), [7]);
 }
 
 #[test]
@@ -95,7 +91,7 @@ fn test_get_leaf_shift() {
 	// now add a single leaf pos to the prune list
 	// leaves will not shift shift anything
 	// we only start shifting after pruning a parent
-	pl.add(1);
+	pl.append(1);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [1]);
@@ -104,10 +100,9 @@ fn test_get_leaf_shift() {
 	assert_eq!(pl.get_leaf_shift(3), 0);
 	assert_eq!(pl.get_leaf_shift(4), 0);
 
-	// now add the sibling leaf pos (pos 1 and pos 2) which will prune the parent
+	// now add the sibling leaf pos (pos 2) which will prune the parent
 	// at pos 3 this in turn will "leaf shift" the leaf at pos 3 by 2
-	pl.add(1);
-	pl.add(2);
+	pl.append(2);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.len(), 1);
@@ -120,7 +115,7 @@ fn test_get_leaf_shift() {
 	// now prune an additional leaf at pos 4
 	// leaf offset of subsequent pos will be 2
 	// 00100120
-	pl.add(4);
+	pl.append(4);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.len(), 2);
@@ -138,8 +133,7 @@ fn test_get_leaf_shift() {
 	// the two smaller subtrees (pos 3 and pos 6) are rolled up to larger subtree
 	// (pos 7) the leaf offset is now 4 to cover entire subtree containing first
 	// 4 leaves 00100120
-	pl.add(4);
-	pl.add(5);
+	pl.append(5);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.len(), 1);
@@ -154,13 +148,13 @@ fn test_get_leaf_shift() {
 	assert_eq!(pl.get_leaf_shift(8), 4);
 	assert_eq!(pl.get_leaf_shift(9), 4);
 
-	// now check we can prune some unconnected nodes in arbitrary order
+	// now check we can prune some unconnected nodes
 	// and that leaf_shift is correct for various pos
 	let mut pl = PruneList::empty();
-	pl.add(5);
-	pl.add(11);
-	pl.add(12);
-	pl.add(4);
+	pl.append(4);
+	pl.append(5);
+	pl.append(11);
+	pl.append(12);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.len(), 2);
@@ -184,7 +178,7 @@ fn test_get_shift() {
 	// prune a single leaf node
 	// pruning only a leaf node does not shift any subsequent pos
 	// we will only start shifting when a parent can be pruned
-	pl.add(1);
+	pl.append(1);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [1]);
@@ -192,21 +186,7 @@ fn test_get_shift() {
 	assert_eq!(pl.get_shift(2), 0);
 	assert_eq!(pl.get_shift(3), 0);
 
-	pl.add(1);
-	pl.add(2);
-	pl.flush().unwrap();
-
-	assert_eq!(pl.iter().collect::<Vec<_>>(), [3]);
-	assert_eq!(pl.get_shift(1), 0);
-	assert_eq!(pl.get_shift(2), 0);
-	assert_eq!(pl.get_shift(3), 2);
-	assert_eq!(pl.get_shift(4), 2);
-	assert_eq!(pl.get_shift(5), 2);
-	assert_eq!(pl.get_shift(6), 2);
-
-	// pos 3 is not a leaf and is already in prune list
-	// prune it and check we are still consistent
-	pl.add(3);
+	pl.append(2);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [3]);
@@ -217,7 +197,7 @@ fn test_get_shift() {
 	assert_eq!(pl.get_shift(5), 2);
 	assert_eq!(pl.get_shift(6), 2);
 
-	pl.add(4);
+	pl.append(4);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [3, 4]);
@@ -228,8 +208,7 @@ fn test_get_shift() {
 	assert_eq!(pl.get_shift(5), 2);
 	assert_eq!(pl.get_shift(6), 2);
 
-	pl.add(4);
-	pl.add(5);
+	pl.append(5);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [7]);
@@ -245,18 +224,21 @@ fn test_get_shift() {
 
 	// prune a bunch more
 	for x in 6..1000 {
-		pl.add(x);
+		if !pl.is_pruned(x) {
+			pl.append(x);
+		}
 	}
 	pl.flush().unwrap();
 
 	// and check we shift by a large number (hopefully the correct number...)
 	assert_eq!(pl.get_shift(1010), 996);
 
+	// now check we can do some sparse pruning
 	let mut pl = PruneList::empty();
-	pl.add(9);
-	pl.add(8);
-	pl.add(5);
-	pl.add(4);
+	pl.append(4);
+	pl.append(5);
+	pl.append(8);
+	pl.append(9);
 	pl.flush().unwrap();
 
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [6, 10]);
@@ -277,33 +259,33 @@ fn test_get_shift() {
 #[test]
 pub fn test_iter() {
 	let mut pl = PruneList::empty();
-	pl.add(1);
-	pl.add(2);
-	pl.add(4);
+	pl.append(1);
+	pl.append(2);
+	pl.append(4);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [3, 4]);
 
 	let mut pl = PruneList::empty();
-	pl.add(1);
-	pl.add(2);
-	pl.add(5);
+	pl.append(1);
+	pl.append(2);
+	pl.append(5);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [3, 5]);
 }
 
 #[test]
 pub fn test_pruned_bintree_range_iter() {
 	let mut pl = PruneList::empty();
-	pl.add(1);
-	pl.add(2);
-	pl.add(4);
+	pl.append(1);
+	pl.append(2);
+	pl.append(4);
 	assert_eq!(
 		pl.pruned_bintree_range_iter().collect::<Vec<_>>(),
 		[1..4, 4..5]
 	);
 
 	let mut pl = PruneList::empty();
-	pl.add(1);
-	pl.add(2);
-	pl.add(5);
+	pl.append(1);
+	pl.append(2);
+	pl.append(5);
 	assert_eq!(
 		pl.pruned_bintree_range_iter().collect::<Vec<_>>(),
 		[1..4, 5..6]
@@ -316,15 +298,15 @@ pub fn test_unpruned_iter() {
 	assert_eq!(pl.unpruned_iter(5).collect::<Vec<_>>(), [1, 2, 3, 4, 5]);
 
 	let mut pl = PruneList::empty();
-	pl.add(2);
+	pl.append(2);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [2]);
 	assert_eq!(pl.pruned_bintree_range_iter().collect::<Vec<_>>(), [2..3]);
 	assert_eq!(pl.unpruned_iter(4).collect::<Vec<_>>(), [1, 3, 4]);
 
 	let mut pl = PruneList::empty();
-	pl.add(2);
-	pl.add(4);
-	pl.add(5);
+	pl.append(2);
+	pl.append(4);
+	pl.append(5);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [2, 6]);
 	assert_eq!(
 		pl.pruned_bintree_range_iter().collect::<Vec<_>>(),
@@ -342,15 +324,15 @@ fn test_unpruned_leaf_iter() {
 	);
 
 	let mut pl = PruneList::empty();
-	pl.add(2);
+	pl.append(2);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [2]);
 	assert_eq!(pl.pruned_bintree_range_iter().collect::<Vec<_>>(), [2..3]);
 	assert_eq!(pl.unpruned_leaf_iter(5).collect::<Vec<_>>(), [1, 4, 5]);
 
 	let mut pl = PruneList::empty();
-	pl.add(2);
-	pl.add(4);
-	pl.add(5);
+	pl.append(2);
+	pl.append(4);
+	pl.append(5);
 	assert_eq!(pl.iter().collect::<Vec<_>>(), [2, 6]);
 	assert_eq!(
 		pl.pruned_bintree_range_iter().collect::<Vec<_>>(),
@@ -358,3 +340,63 @@ fn test_unpruned_leaf_iter() {
 	);
 	assert_eq!(pl.unpruned_leaf_iter(9).collect::<Vec<_>>(), [1, 8, 9]);
 }
+
+pub fn test_append_pruned_subtree() {
+	let mut pl = PruneList::empty();
+
+	// append a pruned leaf pos (shift and leaf shift are unaffected).
+	pl.append(1);
+
+	assert_eq!(pl.to_vec(), [1]);
+	assert_eq!(pl.get_shift(2), 0);
+	assert_eq!(pl.get_leaf_shift(2), 0);
+
+	pl.append(3);
+
+	// subtree beneath root at 3 is pruned
+	// pos 4 is shifted by 2 pruned hashes [1, 2]
+	// pos 4 is shifted by 2 leaves [1, 2]
+	assert_eq!(pl.to_vec(), [3]);
+	assert_eq!(pl.get_shift(4), 2);
+	assert_eq!(pl.get_leaf_shift(4), 2);
+
+	// append another pruned subtree (ancester of previous one)
+	pl.append(7);
+
+	// subtree beneath root at 7 is pruned
+	// pos 8 is shifted by 6 pruned hashes [1, 2, 3, 4, 5, 6]
+	// pos 4 is shifted by 4 leaves [1, 2, 4, 5]
+	assert_eq!(pl.to_vec(), [7]);
+	assert_eq!(pl.get_shift(8), 6);
+	assert_eq!(pl.get_leaf_shift(8), 4);
+
+	// now append another pruned leaf pos
+	pl.append(8);
+
+	// additional pruned leaf does not affect the shift or leaf shift
+	// pos 9 is shifted by 6 pruned hashes [1, 2, 3, 4, 5, 6]
+	// pos 4 is shifted by 4 leaves [1, 2, 4, 5]
+	assert_eq!(pl.to_vec(), [7, 8]);
+	assert_eq!(pl.get_shift(9), 6);
+	assert_eq!(pl.get_leaf_shift(9), 4);
+}
+
+#[test]
+fn test_recreate_prune_list() {
+	let mut pl = PruneList::empty();
+	pl.append(4);
+	pl.append(5);
+	pl.append(11);
+
+	let pl2 = PruneList::new(None, vec![4, 5, 11].into_iter().collect());
+
+	assert_eq!(pl.to_vec(), pl2.to_vec());
+	assert_eq!(pl.shift_cache(), pl2.shift_cache());
+	assert_eq!(pl.leaf_shift_cache(), pl2.leaf_shift_cache());
+
+	let pl3 = PruneList::new(None, vec![6, 11].into_iter().collect());
+
+	assert_eq!(pl.to_vec(), pl3.to_vec());
+	assert_eq!(pl.shift_cache(), pl3.shift_cache());
+	assert_eq!(pl.leaf_shift_cache(), pl3.leaf_shift_cache());
+}