Revertible store may not terminate while resetting

[rnbguy]

When .reset() method is called on the revertible store, it calls the .set() or .delete() method for each logged operation.

basecoin-rs/crates/store/src/impls/revertible.rs

Lines 94 to 101 in aa59797

	while let Some(op) = self.op_log.pop() {
	match op {
	RevertOp::Delete(path) => self.delete(&path),
	RevertOp::Set(path, value) => {
	self.set(path, value).unwrap(); // safety - reset failures are unrecoverable
	}
	}
	}

But the .set() may add a new operation to the op_log, and op_log may never be empty.

basecoin-rs/crates/store/src/impls/revertible.rs

Lines 52 to 62 in aa59797

	fn set(&mut self, path: Path, value: Vec<u8>) -> Result<Option<Vec<u8>>, Self::Error> {
	let old_value = self.store.set(path.clone(), value)?;
	match old_value {
	// None implies this was an insert op, so we record the revert op as delete op
	None => self.op_log.push(RevertOp::Delete(path)),
	// Some old value implies this was an update op, so we record the revert op as a set op
	// with the old value
	Some(ref old_value) => self.op_log.push(RevertOp::Set(path, old_value.clone())),
	}
	Ok(old_value)
	}

A sample test case demonstrating this.

let mut store = RevertibleStore::<InMemoryStore>::default();
store.set("into".to_owned().try_into()?, vec![1])?;
store.set("into".to_owned().try_into()?, vec![2])?;
store.reset(); // doesn't terminate

[rnbguy]

We will not fix this issue, as reverting requires rolling back to the previous root hash. However, while reverting the op_log, if the store adds a new key or deletes an old key - it arbitrarily reorganizes the tree.

Since it is a bug in the RevertibleStore, we will deprecate it in favor of InMemoryStore (which currently supports correct rollback) in #182.