Repository navigation
docs(core): document hashbrown feature's fixed-key hashing - #2354
Open
Romansage6 wants to merge 1 commit into
Open
Romansage6 wants to merge 1 commit into
Romansage6 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2354 +/- ##
=======================================
Coverage 78.84% 78.84%
=======================================
Files 31 31
Lines 6060 6060
Branches 288 288
=======================================
Hits 4778 4778
Misses 1203 1203
Partials 79 79
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
notmandatory
reviewed
Oct 9, 2026
Romansage6
added a commit
to Romansage6/bdk
that referenced
this pull request
Oct 10, 2026
Address review on bitcoindevkit#2354: `hashbrown` takes precedence over `std`, not just the `no_std` `BTreeMap`/`BTreeSet` fallback — enabling it drops `std`'s per-process `RandomState` seeding too. Make that explicit, and list the crate's other feature flags (`std`, `serde`) alongside it so `hashbrown` isn't documented in isolation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
|
This looks good, but you need to sign your commits, see CONTRIBUTING/contribution-workflow. You should also squash your clarification commit in to your original one. |
Member
|
And you'll need to rebase your PR on #2368 (after it gets merged). |
The `hashbrown` feature pulls in `hashbrown` with `default-features = false` and neither its `runtime-rng` nor `compile-time-rng` enabled, so its `ahash` hasher falls back to fixed, publicly known keys. Hash maps built on it (e.g. `TxGraph` keyed by `Txid`, `SpkTxOutIndex` keyed by `ScriptBuf`) therefore hash identically across processes, machines, and builds. With untrusted high-volume input this lets an attacker force worst-case quadratic insertion and stall a sync. This feature takes precedence over `std`, not just the `no_std` `BTreeMap`/`BTreeSet` fallback: enabling it drops `std`'s per-process `RandomState` seeding too. This tradeoff was undocumented. Add a crate-level "Feature flags" rustdoc section to bdk_core, where the feature is defined, listing it alongside the crate's other features (`std`, `serde`) so it isn't documented in isolation, and so users can make an informed choice before enabling it. Fixes bitcoindevkit#2342 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Romansage6
force-pushed
the
doc/hashbrown-predictable-hashing
branch
from
October 14, 2026 07:03
398c634 to
afae35c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
hashbrownfeature pulls inhashbrownwithdefault-features = falseand neither its
runtime-rngnorcompile-time-rngfeature enabled, so itsahashhasher falls back to fixed, publicly known keys. Every hash map builton it (e.g.
TxGraphkeyed byTxid,SpkTxOutIndexkeyed byScriptBuf)therefore hashes identically across processes, machines, and builds. With
untrusted high-volume input (e.g. a chain source returning crafted entries)
this permits worst-case quadratic insertion, which can stall a sync. This
tradeoff wasn't documented anywhere.
Notes to the reviewers
Documented as a crate-level "Feature flags" rustdoc section in
bdk_core,where the feature is actually defined (
crates/core/src/lib.rs), rather thanin the one-line
README.mdor in thecollectionsmodule (which is#[doc(hidden)]and wouldn't render). Also notes the mitigation: depend onhashbrowndirectly withruntime-rng/compile-time-rngenabled, whichCargo's feature unification propagates into this crate's internal use.
Changelog notice
hashbrownfeature uses fixed hash keys, making hashingdeterministic and exposing untrusted high-volume input to worst-case
quadratic hash-map insertion.
Checklists
All Submissions:
Bugfixes:
Fixes #2342