Fix a subtree detection bug and comments

This commit is contained in:
teor 2023-09-19 08:14:03 +10:00
parent 82ca619f2b
commit 5a1deb1865
3 changed files with 14 additions and 28 deletions

View File

@ -408,11 +408,6 @@ impl NoteCommitmentTree {
/// Returns true if this tree has at least one new subtree, when compared with `prev_tree`.
pub fn contains_new_subtree(&self, prev_tree: &Self) -> bool {
// A new subtree was completed by the last note commitment in this tree.
if self.is_complete_subtree() {
return true;
}
// Use -1 for the index of the subtree with no notes, so the comparisons are valid.
let index = self.subtree_index().map_or(-1, |index| i32::from(index.0));
let prev_index = prev_tree
@ -438,9 +433,9 @@ impl NoteCommitmentTree {
// - a new subtree at the end of the previous tree, or
// - a new subtree in this tree (but not at the end).
//
// This happens because the subtree index only increases when the first note is added to
// the new subtree. So we need to exclude subtrees completed by the last note commitment in
// the previous tree.
// Spurious index differences happen because the subtree index only increases when the
// first note is added to the new subtree. So we need to exclude subtrees completed by the
// last note commitment in the previous tree.
//
// We also need to exclude empty previous subtrees, because the index changes to zero when
// the first note is added, but a subtree wasn't completed.
@ -502,12 +497,10 @@ impl NoteCommitmentTree {
// If the subtree has no nodes, the remaining number of nodes is the number of nodes in
// a subtree.
None => {
// This position is guaranteed to be in the first subtree.
let first_position = 0.into();
let subtree_address = incrementalmerkletree::Address::above_position(
TRACKED_SUBTREE_HEIGHT.into(),
first_position,
// This position is guaranteed to be in the first subtree.
0.into(),
);
assert_eq!(

View File

@ -389,11 +389,6 @@ impl NoteCommitmentTree {
/// Returns true if this tree has at least one new subtree, when compared with `prev_tree`.
pub fn contains_new_subtree(&self, prev_tree: &Self) -> bool {
// A new subtree was completed by the last note commitment in this tree.
if self.is_complete_subtree() {
return true;
}
// Use -1 for the index of the subtree with no notes, so the comparisons are valid.
let index = self.subtree_index().map_or(-1, |index| i32::from(index.0));
let prev_index = prev_tree
@ -419,9 +414,9 @@ impl NoteCommitmentTree {
// - a new subtree at the end of the previous tree, or
// - a new subtree in this tree (but not at the end).
//
// This happens because the subtree index only increases when the first note is added to
// the new subtree. So we need to exclude subtrees completed by the last note commitment in
// the previous tree.
// Spurious index differences happen because the subtree index only increases when the
// first note is added to the new subtree. So we need to exclude subtrees completed by the
// last note commitment in the previous tree.
//
// We also need to exclude empty previous subtrees, because the index changes to zero when
// the first note is added, but a subtree wasn't completed.
@ -483,12 +478,10 @@ impl NoteCommitmentTree {
// If the subtree has no nodes, the remaining number of nodes is the number of nodes in
// a subtree.
None => {
// This position is guaranteed to be in the first subtree.
let first_position = 0.into();
let subtree_address = incrementalmerkletree::Address::above_position(
TRACKED_SUBTREE_HEIGHT.into(),
first_position,
// This position is guaranteed to be in the first subtree.
0.into(),
);
assert_eq!(

View File

@ -351,7 +351,7 @@ fn check_sapling_subtrees(db: &ZebraDb) -> Result<(), &'static str> {
// Check that the final note has a greater subtree index if it didn't complete a subtree.
else {
let Some(prev_tree) = db.sapling_tree_by_height(&subtree.end.previous()) else {
result = Err("missing note commitment tree at subtree completion height");
result = Err("missing note commitment tree below subtree completion height");
error!(?result, ?subtree.end);
continue;
};
@ -463,7 +463,7 @@ fn check_orchard_subtrees(db: &ZebraDb) -> Result<(), &'static str> {
// Check that the final note has a greater subtree index if it didn't complete a subtree.
else {
let Some(prev_tree) = db.orchard_tree_by_height(&subtree.end.previous()) else {
result = Err("missing note commitment tree at subtree completion height");
result = Err("missing note commitment tree below subtree completion height");
error!(?result, ?subtree.end);
continue;
};
@ -549,7 +549,7 @@ fn calculate_sapling_subtree(
end_height: Height,
tree: Arc<sapling::tree::NoteCommitmentTree>,
) -> NoteCommitmentSubtree<sapling::tree::Node> {
// If a subtree is completed by a note commentment in the block at `end_height`,
// If a subtree is completed by a note commitment in the block at `end_height`,
// then that subtree can be completed in two different ways:
if let Some((index, node)) = tree.completed_subtree_index_and_root() {
// If the subtree is completed by the last note commitment in that block,
@ -674,7 +674,7 @@ fn calculate_orchard_subtree(
end_height: Height,
tree: Arc<orchard::tree::NoteCommitmentTree>,
) -> NoteCommitmentSubtree<orchard::tree::Node> {
// If a subtree is completed by a note commentment in the block at `end_height`,
// If a subtree is completed by a note commitment in the block at `end_height`,
// then that subtree can be completed in two different ways:
if let Some((index, node)) = tree.completed_subtree_index_and_root() {
// If the subtree is completed by the last note commitment in that block,