incrementalmerkletree: Fix `Address::common_ancestor`

After correcting for differing levels, the tree distance between indices
within a level was being computed via `u64::abs_diff`. This had failures
in cases where the indices were numerically close but the nodes were
much farther apart in the tree (e.g. adjacent non-sibling leaves).

The XOR distance is the correct metric to use here: the leading prefix
bits corresponding to the path from the tree root to the common ancestor
will be identical and thus zeroed out; the number of bits after that
gives the level difference between the compared indices and their common
ancestor.
This commit is contained in:
Jack Grigg 2023-07-06 12:28:58 +00:00
parent 2a5f27eb5f
commit 1292d478b6
2 changed files with 7 additions and 2 deletions

View File

@ -6,6 +6,11 @@ and this project adheres to Rust's notion of
[Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## Unreleased
### Fixed
- `incrementalmerkletree::Address::common_ancestor` no longer produces incorrect
results for some pairs of addresses. It was previously using an arithmetic
distance between indices within a level, instead of a bitwise distance.
### Changed
- `incrementalmerkletree::Hashable` trait now has a `Debug` bound.

View File

@ -386,7 +386,7 @@ impl Address {
pub fn common_ancestor(&self, other: &Self) -> Self {
if self.level >= other.level {
let other_ancestor_idx = other.index >> (self.level.0 - other.level.0);
let index_delta = self.index.abs_diff(other_ancestor_idx);
let index_delta = self.index ^ other_ancestor_idx;
let level_delta = (u64::BITS - index_delta.leading_zeros()) as u8;
Address {
level: self.level + level_delta,
@ -394,7 +394,7 @@ impl Address {
}
} else {
let self_ancestor_idx = self.index >> (other.level.0 - self.level.0);
let index_delta = other.index.abs_diff(self_ancestor_idx);
let index_delta = other.index ^ self_ancestor_idx;
let level_delta = (u64::BITS - index_delta.leading_zeros()) as u8;
Address {
level: other.level + level_delta,