Audit uses of `.unwrap`/`.expect` in shardtree
For the unwrap calls, i see that :
batch.rshas 1 non-test unwraplegacy.rshas 1 non-test unwraplib.rshas 3 non-test unwrapprunable.rshas 8 non-test unwrap callscaching.rshas 15 non-test unwrap calls, that are related to the DB access which could be unstable (for ex. due disk space issues)
~~At least the unwraps for DB access seem like a bug to me.~~
batch.rshas 1 non-test unwrap
This (along with one unwrap in prunable.rs) is due to BatchInsertionResult.max_insert_position being an Option<Position>, but I cannot find anywhere this gets set to None.
legacy.rshas 1 non-test unwrap
This should be justifiable because of the invariant that you cannot construct a witness for an empty tree. However, incrementalmerkletree::witness::IncrementalWitness does permit construction from an empty tree, so we can't currently rely on that invariant. We should fix incrementalmerkletree.
lib.rshas 3 non-test unwrapcaching.rshas 15 non-test unwrap calls, that are related to the DB access which could be unstable (for ex. due disk space issues)At least the unwraps for DB access seem like a bug to me.
These are all justifiable.
prunable.rshas 8 non-test unwrap calls
All except two of these (excluding the one mentioned above) are justifiable. The other two are technically justifiable in the context they get used, but their public APIs don't enforce the necessary invariant, so should be fixed (with the unwrap moving out into the place these APIs get used where the invariant can be justified).
This should be justifiable because of the invariant that you cannot construct a witness for an empty tree. However,
incrementalmerkletree::witness::IncrementalWitnessdoes permit construction from an empty tree, so we can't currently rely on that invariant. We should fixincrementalmerkletree.
Opened #120. (Due to @nuttycom changing how we do crate development in #116, we can't change shardtree until after all changes to incrementalmerkletree are made.)