fix gc call block index.

core/state-db/src/branch.rs

@@ -299,15 +299,18 @@ impl RangeSet {
 		full: bool,
 	) {
 
-		if branch_index <= self.treshold {
+		if branch_index < self.treshold {
+			return;
+		}
+		if branch_index == self.treshold && linear_index.is_none() {
 			return;
 		}
 		// range set update
 		let old_treshold = self.treshold;
 		// we do not finalize current branch cause it
 		// can contains other blocks
 		self.treshold = branch_index;
-		if branch_index == 0 || !full {
+		if (branch_index == 0 && linear_index.is_none()) || !full {
 			// remove cached value under treshold only
 			let new_storage = self.storage.split_off(&(self.treshold));
 			self.storage = new_storage;
@@ -321,7 +324,7 @@ impl RangeSet {
 		} else {
 			let new_storage = self.storage.split_off(&(self.treshold));
 			self.storage = new_storage;
-			self.finalize_full(branch_index, None);
+			self.finalize_full(branch_index, linear_index);
 		};
 	}
 

core/state-db/src/lib.rs

@@ -146,6 +146,9 @@ pub struct ChangeSet<H: Hash> {
 /// ),
 /// but it just need to be build from client (no need to change
 /// it here except to extract faster).
+///
+/// This assumes that we only commit block per block (otherwhise
+/// we need to inclued block number value here).
 pub type OffstateChangeSet<H> = Vec<(H, Option<DBValue>)>;
 
 /// Info for pruning offstate: a last prune index and keys to prune.
@@ -259,6 +262,12 @@ impl<BlockHash: Hash, Key: Hash> StateDbSync<BlockHash, Key> {
 				Ok(CommitSet {
 					data: changeset,
 					meta: Default::default(),
+					// TODO EMCH no current support for archive all,
+					// this would require managing branch index at
+					// client level (was implemented in another branch:
+					// client-db-branch-ix)
+					// and use and ordered tuple (branchix, blocknumber)
+					// index to run.
 					offstate: offstate_changeset,
 					offstate_prune: None,
 				})
@@ -278,6 +287,9 @@ impl<BlockHash: Hash, Key: Hash> StateDbSync<BlockHash, Key> {
 			Ok(()) => {
 				if self.mode == PruningMode::ArchiveCanonical {
 					commit.data.deleted.clear();
+					// TODO EMCH use pruning mode in statedb to avoid feeding that
+					// in the first place.
+					commit.offstate_prune = None;
 				}
 			}
 			Err(e) => return Err(e),
@@ -618,15 +630,21 @@ mod tests {
 	fn full_archive_keeps_everything() {
 		let (db, sdb) = make_test_db(PruningMode::ArchiveAll);
 		assert!(db.data_eq(&make_db(&[1, 21, 22, 3, 4, 91, 921, 922, 93, 94])));
-		assert!(db.offstate_eq(&[1, 21, 22, 3, 4, 81, 821, 822, 83, 84]));
+
+		// TODO EMCH implement full for test db and test for offstate
+
 		assert!(!sdb.is_pruned(&H256::from_low_u64_be(0), 0));
 	}
 
 	#[test]
 	fn canonical_archive_keeps_canonical() {
 		let (db, _) = make_test_db(PruningMode::ArchiveCanonical);
 		assert!(db.data_eq(&make_db(&[1, 21, 3, 91, 921, 922, 93, 94])));
-		assert!(db.offstate_eq(&[1, 21, 3, 81, 821, 822, 83, 84]));
+		assert!(db.offstate_eq_at(&[81, 821, 822, 83, 84], Some(0)));
+		assert!(db.offstate_eq_at(&[1, 821, 822, 83, 84], Some(1)));
+		assert!(db.offstate_eq_at(&[21, 822, 83, 84], Some(2)));
+		assert!(db.offstate_eq_at(&[3, 21, 822, 84], Some(3)));
+		assert!(db.offstate_eq(&[3, 21, 822, 84]));
 	}
 
 	#[test]

core/state-db/src/noncanonical.rs

@@ -95,9 +95,12 @@ impl OffstatePendingGC {
 				// TODO EMCH double get is rather inefficient, see if it is possible
 				// to reuse the hash of the hashset into the hashmap
 				for key in self.keys_pending_gc.drain() {
-					offstate_values.get_mut(&key).map(|historied_value| {
-						historied_value.gc(branches.reverse_iter_ranges());
-					});
+					match offstate_values.get_mut(&key).map(|historied_value| {
+						historied_value.gc(branches.reverse_iter_ranges())
+					}) {
+						Some(historied_data::PruneResult::Cleared) => { let _ = offstate_values.remove(&key); },
+						_ => (),
+					}
 				}
 
 				self.pending_canonicalisation_query = None;
@@ -572,12 +575,23 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 			if let Some(range) = self.get_branch_range(hash, block_number) {
 				commit.offstate.extend(
 					overlay.offstate_inserted.iter()
-					.chain(overlay.offstate_deleted.iter())
 					.map(|k| (k.clone(), self.offstate_values.get(k)
 						.expect("For each key in overlays there's a value in values")
 						.get(&range)
 						.expect("For each key in overlays there's a historied entry in values")
 						.clone())));
+				// some deletion can be pruned if in first block so handle is
+				// a bit less restrictive
+				commit.offstate.extend(
+					overlay.offstate_deleted.iter()
+					.filter_map(|k| self.offstate_values.get(k)
+						.map(|v| (k.clone(), v.get(&range)
+							.expect("For each key in overlays there's a historied entry \
+								in values, and pruned empty value are cleared")
+							.clone())
+						)
+					)
+				);
 			}
 		}
 
@@ -633,7 +647,7 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 			if let Some(branch_index_cannonicalize) = last_index {
 				// this needs to be call after parents update
 				// TODO EMCH may be needed in 'canonicalize', and restore or commit here
-				self.branches.update_finalize_treshold(branch_index_cannonicalize, Some(block_number), true);
+				self.branches.update_finalize_treshold(branch_index_cannonicalize, Some(block_number + 1), true);
 				// gc is at the right place
 				self.offstate_gc.set_pending_gc(branch_index_cannonicalize);
 				// try to run the garbage collection (can run later if there is

core/state-db/src/test.rs

@@ -33,6 +33,10 @@ pub struct TestDb {
 	pub data: HashMap<H256, DBValue>,
 	pub meta: HashMap<Vec<u8>, DBValue>,
 	pub offstate: HashMap<OffstateKey, DBValue>,
+	// big heuristic to increment this for canonicalize transaction only
+	// by checking if there is value in offstate change set.
+	// If empty change set needed, this need to be change manually.
+	// TODO EMCH consider changing commit fun to use last block number explicitely.
 	pub last_block: u64,
 }
 
@@ -79,7 +83,9 @@ impl NodeDb for TestDb {
 
 impl TestDb {
 	pub fn commit(&mut self, commit: &CommitSet<H256>) {
-		self.last_block += 1;
+		if commit.offstate.len() > 0 {
+			self.last_block += 1;
+		}
 		self.data.extend(commit.data.inserted.iter().cloned());
 		for k in commit.data.deleted.iter() {
 			self.data.remove(k);

core/utils/historied-data/src/tree.rs

@@ -27,6 +27,7 @@ use crate::linear::{
 	SerializedConfig,
 };
 use crate::HistoriedValue;
+use crate::PruneResult;
 use crate::{as_usize, as_i};
 use rstd::rc::Rc;
 use rstd::vec::Vec;
@@ -639,12 +640,13 @@ impl<V> History<V> {
 
 	/// Gc an historied value other its possible values.
 	/// Iterator need to be reversed ordered by branch index.
-	pub fn gc<IT, S, I>(&mut self, mut states: IT) 
+	pub fn gc<IT, S, I>(&mut self, mut states: IT) -> PruneResult
 		where
 			IT: Iterator<Item = (S, I)>,
 			S: BranchStateTrait<bool, I>,
 			I: Copy + Eq + TryFrom<usize> + TryInto<usize>,
 	{
+		let mut changed = false;
 		// state is likely bigger than history.
 		let mut current_state = states.next();
 		for branch_index in (0..self.0.len()).rev() {
@@ -663,29 +665,45 @@ impl<V> History<V> {
 								if let Ok(node_index) = node_index.try_into() {
 									if !state.0.get_node(node_index) {
 										if history_index == len - 1 {
-											let _ = self.0[branch_index].history.pop();
+											changed = self.0[branch_index]
+												.history.pop().is_some() || changed;
 										} else {
-											let _ = self.0[branch_index].history.remove(history_index);
+											self.0[branch_index]
+												.history.remove(history_index);
+											changed = true;
 										}
 									}
 								}
 							}
 							if self.0[branch_index].history.len() == 0 {
-								let _ = self.0.remove(branch_index);
+								self.0.remove(branch_index);
+								changed = true;
 							}
 							break;
 						} else {
-							let _ = self.0.remove(branch_index);
+							self.0.remove(branch_index);
+							changed = true;
 							break;
 						}
 					}
 					self.0.remove(branch_index);
+					changed = true;
 					break;
 				} else {
 					break;
 				}
 			}
 		}
+		if changed {
+			if self.0.len() == 0 {
+				PruneResult::Cleared
+			} else {
+				PruneResult::Changed
+			}
+
+		} else {
+			PruneResult::Unchanged
+		}
 	}
 
 /*
@@ -795,7 +813,7 @@ impl<'a, F: SerializedConfig> Serialized<'a, F> {
 	}
 
 	/// keep a single with value history before the state.
-	pub fn prune<I>(&mut self, index: I) -> crate::PruneResult
+	pub fn prune<I>(&mut self, index: I) -> PruneResult
 		where
 			I: Copy + Eq + TryFrom<usize> + TryInto<usize>,
 	{
@@ -845,14 +863,14 @@ impl<'a, F: SerializedConfig> Serialized<'a, F> {
 			if last_index_with_value > 0 {
 				println!("IN {}", last_index_with_value);
 				self.0.truncate_until(last_index_with_value);
-				return crate::PruneResult::Changed;
+				return PruneResult::Changed;
 			}
 		} else {
 			self.0.clear();
-			return crate::PruneResult::Cleared;
+			return PruneResult::Cleared;
 		}
 
-		crate::PruneResult::Unchanged
+		PruneResult::Unchanged
 	}
 
 }