Skip to content

fix: make Hrp Hash case-insensitive to match PartialEq#278

Open
alexgrad42 wants to merge 2 commits into
rust-bitcoin:masterfrom
alexgrad42:fix/hrp_hash_partial_eq
Open

fix: make Hrp Hash case-insensitive to match PartialEq#278
alexgrad42 wants to merge 2 commits into
rust-bitcoin:masterfrom
alexgrad42:fix/hrp_hash_partial_eq

Conversation

@alexgrad42

Copy link
Copy Markdown
Contributor

Fixes the Hash implementation for the Hrp struct to iterate over lowercase bytes instead of hashing the raw internal buffer. This restores the core invariant where a == b implies hash(a) == hash(b).

Fixes: #275

@apoelstra

Copy link
Copy Markdown
Member

I think this is wrong because if you hash two Hrps this way you'll get the same hash as a concatenation of the HRPs. See https://doc.rust-lang.org/std/hash/trait.Hash.html#prefix-collisions

The std impl for [T] adds a length prefix: https://doc.rust-lang.org/src/core/hash/mod.rs.html#934

@alexgrad42 alexgrad42 force-pushed the fix/hrp_hash_partial_eq branch from 1023b92 to e96fad7 Compare June 17, 2026 14:52
@alexgrad42

alexgrad42 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

I've added the length + extracted the test into separate commit, please check

Fixes the Hash implementation for the Hrp struct to iterate over
lowercase bytes instead of hashing the raw internal buffer. This
restores the core invariant where `a == b` implies `hash(a) == hash(b)`.

Fixes: rust-bitcoin#275
Fixes the Hash implementation for the Hrp struct to iterate over
lowercase bytes instead of hashing the raw internal buffer. This
restores the core invariant where `a == b` implies `hash(a) == hash(b)`.

The commit only adds a new test to reproduce the issue, no real fix.

Fixes: rust-bitcoin#275
@alexgrad42 alexgrad42 force-pushed the fix/hrp_hash_partial_eq branch from e96fad7 to 0232d7b Compare June 18, 2026 08:02
@apoelstra

Copy link
Copy Markdown
Member

In ad99e2f:

now this has two length prefixes and does an unnecessary copy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hrp Hash impl is case-sensitive while PartialEq is case-insensitive

2 participants