oid: use `std:#️⃣:Hasher::hash` over `Hasher::write`

Previously, the `RepoId` was backed by the `git2::Oid` type, which has a `Hash`
implementation that uses `std:#️⃣:Hasher::hash`. This uses a length prefixed
form for hashing.

When switching to `radicle_oid::Oid` and implementing `Hash` without the length
prefix, this subtly changed the hashes going forward.

These hashes are used in `BloomFilter`, which is the type underlying `Filter`.
This resulted in cases where a previous `Filter` that certainly contained
`rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5` were suddenly reporting that they did not
contain this repository.

Changing the implementation of `Hash` for `radicle_oid::Oid` fixes this, and is
confirmed by adding the regression test in `filter.rs`.
This commit is contained in:
Fintan Halpenny 2025-10-16 10:00:38 +01:00
parent d9ae29deaa
commit cef0ff571d
2 changed files with 48 additions and 1 deletions

View File

@ -350,7 +350,8 @@ mod std {
#[allow(clippy::derived_hash_with_manual_eq)] #[allow(clippy::derived_hash_with_manual_eq)]
impl hash::Hash for Oid { impl hash::Hash for Oid {
fn hash<H: hash::Hasher>(&self, state: &mut H) { fn hash<H: hash::Hasher>(&self, state: &mut H) {
state.write(self.as_ref()); let bytes: &[u8] = self.as_ref();
std::hash::Hash::hash(bytes, state)
} }
} }
} }

View File

@ -130,6 +130,7 @@ impl qcheck::Arbitrary for Filter {
} }
} }
#[allow(clippy::unwrap_used)]
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use super::*; use super::*;
@ -190,4 +191,49 @@ mod test {
let hs = arbitrary::set::<RepoId>(42..=42); let hs = arbitrary::set::<RepoId>(42..=42);
assert_eq!(hs.iter().size_hint(), (42, Some(42))); assert_eq!(hs.iter().size_hint(), (42, Some(42)));
} }
/// Checks that a particular filter extracted from a live deployment of
/// `radicle-node` at `release/1.5.0`, which is known to contain
/// "heartwood", actually also evaluates to contain "heartwood".
///
/// This is to catch regressions in the [`std::hash::Hash`] implementation
/// [`RepoId`] and other breaking changes to [`Filter`].
#[test]
fn compatible() {
let filter = {
let mut filter = [0u8; FILTER_SIZE_S];
#[rustfmt::skip]
const COMPRESSED_FIXTURE: [(usize, u8); 100] = [
(0x002, 0xa8), (0x010, 0x08), (0x016, 0x40), (0x01b, 0x20), (0x04d, 0x04),
(0x050, 0x04), (0x05a, 0x02), (0x05e, 0x80), (0x06d, 0x40), (0x075, 0x08),
(0x082, 0x80), (0x084, 0x80), (0x089, 0x01), (0x08b, 0x08), (0x099, 0x04),
(0x0a1, 0x40), (0x0a7, 0x40), (0x0be, 0x40), (0x0d3, 0x01), (0x0e2, 0x01),
(0x0ee, 0x08), (0x0f2, 0x04), (0x109, 0x08), (0x119, 0x10), (0x15b, 0x40),
(0x160, 0x44), (0x163, 0x01), (0x168, 0x08), (0x16b, 0x01), (0x16d, 0x04),
(0x176, 0x80), (0x17e, 0x40), (0x189, 0x20), (0x18f, 0x04), (0x19f, 0x20),
(0x1b2, 0x08), (0x1b5, 0x04), (0x1b8, 0x20), (0x1ed, 0x10), (0x1f1, 0x40),
(0x1f3, 0x04), (0x1fa, 0x40), (0x20b, 0x08), (0x20e, 0x04), (0x218, 0x01),
(0x231, 0x02), (0x23d, 0x80), (0x248, 0x10), (0x24e, 0x04), (0x250, 0x01),
(0x251, 0x01), (0x255, 0x04), (0x25a, 0x10), (0x265, 0x20), (0x27c, 0x01),
(0x284, 0x04), (0x285, 0x20), (0x28d, 0x81), (0x29f, 0x01), (0x2a6, 0x10),
(0x2ac, 0x40), (0x2ad, 0x10), (0x2b4, 0x04), (0x2b8, 0x02), (0x2cb, 0x01),
(0x2d1, 0x80), (0x2d4, 0x01), (0x2d7, 0x40), (0x2ed, 0x80), (0x2f7, 0x01),
(0x302, 0x80), (0x303, 0x40), (0x307, 0x40), (0x309, 0x04), (0x318, 0x04),
(0x31e, 0x10), (0x335, 0x01), (0x336, 0x40), (0x338, 0x40), (0x351, 0x80),
(0x353, 0x10), (0x359, 0x0c), (0x360, 0x40), (0x367, 0x01), (0x36b, 0x08),
(0x36c, 0x40), (0x37b, 0x10), (0x37d, 0x40), (0x399, 0x02), (0x39f, 0x02),
(0x3a6, 0x02), (0x3a9, 0x04), (0x3ab, 0x01), (0x3cb, 0x04), (0x3e2, 0x01),
(0x3e5, 0x10), (0x3ea, 0x40), (0x3ed, 0x40), (0x3f2, 0x02), (0x3f5, 0x80),
];
for (i, v) in COMPRESSED_FIXTURE.into_iter() {
filter[i] = v;
}
Filter(BloomFilter::from(filter.to_vec()))
};
assert!(filter.contains(&"rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5".parse().unwrap()),);
}
} }