Skip to content

Commit 9a01841

Browse files
committed
Bump hashbrown dependency to 0.13
While this isn't expected to materially improve performance, it does get us ahash 0.8, which allows us to reduce fuzzing randomness, making our fuzzers much happier. Sadly, by default `ahash` no longer tries to autodetect a randomness source, so we cannot simply rely on `hashbrown` to do randomization for us, but rather have to also explicitly depend on `ahash`.
1 parent 79ba4b2 commit 9a01841

File tree

9 files changed

+74
-25
lines changed

9 files changed

+74
-25
lines changed

ci/check-cfg-flags.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ def check_feature(feature):
1313
pass
1414
elif feature == "no-std":
1515
pass
16+
elif feature == "ahash":
17+
pass
1618
elif feature == "hashbrown":
1719
pass
1820
elif feature == "backtrace":

fuzz/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ lightning = { path = "../lightning", features = ["regex", "hashbrown", "_test_ut
2222
lightning-rapid-gossip-sync = { path = "../lightning-rapid-gossip-sync" }
2323
bitcoin = { version = "0.30.2", features = ["secp-lowmemory"] }
2424
hex = { package = "hex-conservative", version = "0.1.1", default-features = false }
25-
hashbrown = "0.8"
25+
hashbrown = "0.13"
2626

2727
afl = { version = "0.12", optional = true }
2828
honggfuzz = { version = "0.5", optional = true, default-features = false }

lightning-invoice/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ bech32 = { version = "0.9.0", default-features = false }
2424
lightning = { version = "0.0.119", path = "../lightning", default-features = false }
2525
secp256k1 = { version = "0.27.0", default-features = false, features = ["recovery", "alloc"] }
2626
num-traits = { version = "0.2.8", default-features = false }
27-
hashbrown = { version = "0.8", optional = true }
27+
hashbrown = { version = "0.13", optional = true }
2828
serde = { version = "1.0.118", optional = true }
2929
bitcoin = { version = "0.30.2", default-features = false }
3030

lightning/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ unsafe_revoked_tx_signing = []
3131
# Override signing to not include randomness when generating signatures for test vectors.
3232
_test_vectors = []
3333

34-
no-std = ["hashbrown", "bitcoin/no-std", "core2/alloc", "libm"]
34+
no-std = ["hashbrown", "ahash", "bitcoin/no-std", "core2/alloc", "libm"]
3535
std = ["bitcoin/std"]
3636

3737
# Generates low-r bitcoin signatures, which saves 1 byte in 50% of the cases
@@ -42,7 +42,8 @@ default = ["std", "grind_signatures"]
4242
[dependencies]
4343
bitcoin = { version = "0.30.2", default-features = false, features = ["secp-recovery"] }
4444

45-
hashbrown = { version = "0.8", optional = true }
45+
hashbrown = { version = "0.13", optional = true }
46+
ahash = { version = "0.8", optional = true, default-features = false, features = ["runtime-rng"] }
4647
hex = { package = "hex-conservative", version = "0.1.1", default-features = false }
4748
regex = { version = "1.5.6", optional = true }
4849
backtrace = { version = "0.3", optional = true }

lightning/src/chain/onchaintx.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
686686
if let Some(claim_id) = claim_id {
687687
if let Some(claim) = self.pending_claim_requests.remove(&claim_id) {
688688
for outpoint in claim.outpoints() {
689-
self.claimable_outpoints.remove(&outpoint);
689+
self.claimable_outpoints.remove(outpoint);
690690
}
691691
}
692692
} else {

lightning/src/lib.rs

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,19 @@ mod io_extras {
165165
mod prelude {
166166
#[cfg(feature = "hashbrown")]
167167
extern crate hashbrown;
168+
#[cfg(feature = "ahash")]
169+
extern crate ahash;
168170

169171
pub use alloc::{vec, vec::Vec, string::String, collections::VecDeque, boxed::Box};
170172

173+
pub use alloc::borrow::ToOwned;
174+
pub use alloc::string::ToString;
175+
176+
// For no-std builds, we need to use hashbrown, however, by default, it doesn't randomize the
177+
// hashing and is vulnerable to HashDoS attacks. Thus, when not fuzzing, we use its default
178+
// ahash hashing algorithm but randomize, opting to not randomize when fuzzing to avoid false
179+
// positive branch coverage.
180+
171181
#[cfg(not(feature = "hashbrown"))]
172182
mod std_hashtables {
173183
pub(crate) use std::collections::{HashMap, HashSet, hash_map};
@@ -181,29 +191,65 @@ mod prelude {
181191
pub(crate) use std_hashtables::*;
182192

183193
#[cfg(feature = "hashbrown")]
184-
mod hashbrown_tables {
185-
pub(crate) use hashbrown::{HashMap, HashSet, hash_map};
194+
pub(crate) use self::hashbrown::hash_map;
195+
196+
#[cfg(all(feature = "hashbrown", fuzzing))]
197+
mod nonrandomized_hashbrown {
198+
pub(crate) use self::hashbrown::{HashMap, HashSet};
186199

187200
pub(crate) type OccupiedHashMapEntry<'a, K, V> =
188201
hashbrown::hash_map::OccupiedEntry<'a, K, V, hash_map::DefaultHashBuilder>;
189202
pub(crate) type VacantHashMapEntry<'a, K, V> =
190203
hashbrown::hash_map::VacantEntry<'a, K, V, hash_map::DefaultHashBuilder>;
191204
}
192-
#[cfg(feature = "hashbrown")]
193-
pub(crate) use hashbrown_tables::*;
205+
#[cfg(all(feature = "hashbrown", fuzzing))]
206+
pub(crate) use nonrandomized_hashbrown::*;
207+
208+
209+
#[cfg(all(feature = "hashbrown", not(fuzzing)))]
210+
mod randomized_hashtables {
211+
use super::*;
212+
use ahash::RandomState;
213+
214+
pub(crate) type HashMap<K, V> = hashbrown::HashMap<K, V, RandomState>;
215+
pub(crate) type HashSet<K> = hashbrown::HashSet<K, RandomState>;
216+
217+
pub(crate) type OccupiedHashMapEntry<'a, K, V> =
218+
hashbrown::hash_map::OccupiedEntry<'a, K, V, RandomState>;
219+
pub(crate) type VacantHashMapEntry<'a, K, V> =
220+
hashbrown::hash_map::VacantEntry<'a, K, V, RandomState>;
194221

195-
pub(crate) fn new_hash_map<K: core::hash::Hash + Eq, V>() -> HashMap<K, V> { HashMap::new() }
196-
pub(crate) fn hash_map_with_capacity<K: core::hash::Hash + Eq, V>(cap: usize) -> HashMap<K, V> {
197-
HashMap::with_capacity(cap)
222+
pub(crate) fn new_hash_map<K, V>() -> HashMap<K, V> {
223+
HashMap::with_hasher(RandomState::new())
224+
}
225+
pub(crate) fn hash_map_with_capacity<K, V>(cap: usize) -> HashMap<K, V> {
226+
HashMap::with_capacity_and_hasher(cap, RandomState::new())
227+
}
228+
229+
pub(crate) fn new_hash_set<K>() -> HashSet<K> {
230+
HashSet::with_hasher(RandomState::new())
231+
}
232+
pub(crate) fn hash_set_with_capacity<K>(cap: usize) -> HashSet<K> {
233+
HashSet::with_capacity_and_hasher(cap, RandomState::new())
234+
}
198235
}
199236

200-
pub(crate) fn new_hash_set<K: core::hash::Hash + Eq>() -> HashSet<K> { HashSet::new() }
201-
pub(crate) fn hash_set_with_capacity<K: core::hash::Hash + Eq>(cap: usize) -> HashSet<K> {
202-
HashSet::with_capacity(cap)
237+
#[cfg(any(not(feature = "hashbrown"), fuzzing))]
238+
mod randomized_hashtables {
239+
use super::*;
240+
241+
pub(crate) fn new_hash_map<K, V>() -> HashMap<K, V> { HashMap::new() }
242+
pub(crate) fn hash_map_with_capacity<K, V>(cap: usize) -> HashMap<K, V> {
243+
HashMap::with_capacity(cap)
244+
}
245+
246+
pub(crate) fn new_hash_set<K>() -> HashSet<K> { HashSet::new() }
247+
pub(crate) fn hash_set_with_capacity<K>(cap: usize) -> HashSet<K> {
248+
HashSet::with_capacity(cap)
249+
}
203250
}
204251

205-
pub use alloc::borrow::ToOwned;
206-
pub use alloc::string::ToString;
252+
pub(crate) use randomized_hashtables::*;
207253
}
208254

209255
#[cfg(all(not(ldk_bench), feature = "backtrace", feature = "std", test))]

lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5916,7 +5916,7 @@ where
59165916
// TODO: Once we can rely on the counterparty_node_id from the
59175917
// monitor event, this and the outpoint_to_peer map should be removed.
59185918
let outpoint_to_peer = self.outpoint_to_peer.lock().unwrap();
5919-
match outpoint_to_peer.get(&funding_txo) {
5919+
match outpoint_to_peer.get(funding_txo) {
59205920
Some(cp_id) => cp_id.clone(),
59215921
None => return,
59225922
}
@@ -11020,7 +11020,7 @@ where
1102011020
downstream_counterparty_and_funding_outpoint:
1102111021
Some((blocked_node_id, blocked_channel_outpoint, blocking_action)), ..
1102211022
} = action {
11023-
if let Some(blocked_peer_state) = per_peer_state.get(&blocked_node_id) {
11023+
if let Some(blocked_peer_state) = per_peer_state.get(blocked_node_id) {
1102411024
log_trace!(logger,
1102511025
"Holding the next revoke_and_ack from {} until the preimage is durably persisted in the inbound edge's ChannelMonitor",
1102611026
blocked_channel_outpoint.to_channel_id());

lightning/src/routing/router.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,9 +2532,9 @@ where L::Target: Logger {
25322532
let mut aggregate_path_contribution_msat = path_value_msat;
25332533

25342534
for (idx, (hop, prev_hop_id)) in hop_iter.zip(prev_hop_iter).enumerate() {
2535-
let target = private_hop_key_cache.get(&prev_hop_id).unwrap();
2535+
let target = private_hop_key_cache.get(prev_hop_id).unwrap();
25362536

2537-
if let Some(first_channels) = first_hop_targets.get(&target) {
2537+
if let Some(first_channels) = first_hop_targets.get(target) {
25382538
if first_channels.iter().any(|d| d.outbound_scid_alias == Some(hop.short_channel_id)) {
25392539
log_trace!(logger, "Ignoring route hint with SCID {} (and any previous) due to it being a direct channel of ours.",
25402540
hop.short_channel_id);
@@ -2544,7 +2544,7 @@ where L::Target: Logger {
25442544

25452545
let candidate = network_channels
25462546
.get(&hop.short_channel_id)
2547-
.and_then(|channel| channel.as_directed_to(&target))
2547+
.and_then(|channel| channel.as_directed_to(target))
25482548
.map(|(info, _)| CandidateRouteHop::PublicHop(PublicHopCandidate {
25492549
info,
25502550
short_channel_id: hop.short_channel_id,
@@ -2585,7 +2585,7 @@ where L::Target: Logger {
25852585
.saturating_add(1);
25862586

25872587
// Searching for a direct channel between last checked hop and first_hop_targets
2588-
if let Some(first_channels) = first_hop_targets.get_mut(&target) {
2588+
if let Some(first_channels) = first_hop_targets.get_mut(target) {
25892589
sort_first_hop_channels(first_channels, &used_liquidities,
25902590
recommended_value_msat, our_node_pubkey);
25912591
for details in first_channels {

lightning/src/routing/scoring.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ScoreLookUp for Probabilistic
13301330
_ => return 0,
13311331
};
13321332
let source = candidate.source();
1333-
if let Some(penalty) = score_params.manual_node_penalties.get(&target) {
1333+
if let Some(penalty) = score_params.manual_node_penalties.get(target) {
13341334
return *penalty;
13351335
}
13361336

@@ -1360,7 +1360,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ScoreLookUp for Probabilistic
13601360
let amount_msat = usage.amount_msat.saturating_add(usage.inflight_htlc_msat);
13611361
let capacity_msat = usage.effective_capacity.as_msat();
13621362
self.channel_liquidities
1363-
.get(&scid)
1363+
.get(scid)
13641364
.unwrap_or(&ChannelLiquidity::new(Duration::ZERO))
13651365
.as_directed(&source, &target, capacity_msat)
13661366
.penalty_msat(amount_msat, score_params)

0 commit comments

Comments
 (0)