From 65108a022f6321df1e1ae35bf56114642d7cf89e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 12 Feb 2024 22:37:09 +0000 Subject: [PATCH 1/3] Add a crate which wraps `getrandom` but always compiles In the next commit we'll drop the `ahash` dependency in favor of directly calling `getrandom` to seed our hash tables. However, we'd like to depend on `getrandom` only on certain platforms *and* only when certain features (no-std) are set. This introduces an indirection crate to do so, allowing us to depend on it only when `no-std` is set but only depending on `getrandom` on platforms which it supports. --- Cargo.toml | 4 ++++ ci/check-cfg-flags.py | 2 ++ no-std-check/Cargo.toml | 3 +++ possiblyrandom/Cargo.toml | 21 +++++++++++++++++++++ possiblyrandom/src/lib.rs | 35 +++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+) create mode 100644 possiblyrandom/Cargo.toml create mode 100644 possiblyrandom/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index ddc82cd5d45..ec9edb3ac33 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ members = [ "lightning-rapid-gossip-sync", "lightning-custom-message", "lightning-transaction-sync", + "possiblyrandom", ] exclude = [ @@ -38,3 +39,6 @@ lto = "off" opt-level = 3 lto = true panic = "abort" + +[patch.crates-io.possiblyrandom] +path = "possiblyrandom" diff --git a/ci/check-cfg-flags.py b/ci/check-cfg-flags.py index d6e8e0a90fe..8952b693a60 100755 --- a/ci/check-cfg-flags.py +++ b/ci/check-cfg-flags.py @@ -15,6 +15,8 @@ def check_feature(feature): pass elif feature == "ahash": pass + elif feature == "getrandom": + pass elif feature == "hashbrown": pass elif feature == "backtrace": diff --git a/no-std-check/Cargo.toml b/no-std-check/Cargo.toml index c9d404c922f..45a70c2a6d1 100644 --- a/no-std-check/Cargo.toml +++ b/no-std-check/Cargo.toml @@ -15,3 +15,6 @@ lightning-background-processor = { path = "../lightning-background-processor", f # Obviously lightning-transaction-sync doesn't support no-std, but it should build # even if lightning is built with no-std. lightning-transaction-sync = { path = "../lightning-transaction-sync", optional = true } + +[patch.crates-io] +possiblyrandom = { path = "../possiblyrandom" } diff --git a/possiblyrandom/Cargo.toml b/possiblyrandom/Cargo.toml new file mode 100644 index 00000000000..e02b59669b1 --- /dev/null +++ b/possiblyrandom/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "possiblyrandom" +version = "0.1.0" +authors = ["Matt Corallo"] +license = "MIT OR Apache-2.0" +repository = "https://github.com/lightningdevkit/rust-lightning/" +description = """ +A crate that wraps getrandom and always compiles, returning 0s when no randomness is available. +""" +edition = "2021" + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] + +[dependencies] +getrandom = { version = "0.2", optional = true, default-features = false } + +# Enable getrandom if we are on a platform that (likely) supports it +[target.'cfg(not(any(target_os = "unknown", target_os = "none")))'.dependencies] +getrandom = { version = "0.2", default-features = false } diff --git a/possiblyrandom/src/lib.rs b/possiblyrandom/src/lib.rs new file mode 100644 index 00000000000..9cbbad7f13d --- /dev/null +++ b/possiblyrandom/src/lib.rs @@ -0,0 +1,35 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! [`getrandom`] provides access to OS randomness, but will fail to compile on platforms that do +//! not support fetching OS randomness. This is exactly what you want when you're doing +//! cryptographic operations, but when you're just opportunistically randomizing, we're fine with +//! compiling and simply disabling randomization. +//! +//! This crate does that, returning only possibly-random data. +//! +//! Note that this crate only enables getrandom on a subset of platforms it supports. As getrandom +//! evolves this crate is unlikely to carefully track all getrandom-supported platforms, however +//! will use random data on popular platforms. + +#![no_std] + +#[cfg(feature = "getrandom")] +extern crate getrandom; + +/// Possibly fills `dest` with random data. May fill it with zeros. +#[inline] +pub fn getpossiblyrandom(dest: &mut [u8]) { + #[cfg(feature = "getrandom")] + if getrandom::getrandom(dest).is_err() { + dest.fill(0); + } + #[cfg(not(feature = "getrandom"))] + dest.fill(0); +} From 3096061bef7e04444dd6c0b4f2331d06a6d8969c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 12 Feb 2024 22:40:49 +0000 Subject: [PATCH 2/3] Drop `ahash` dependency in favor of core's `SipHasher` https://github.com/tkaitchuck/aHash/pull/196 bumped the MSRV of `ahash` in a patch release, which makes it rather difficult for us to have it as a dependency. Further, it seems that `ahash` hasn't been particularly robust in the past, notably https://github.com/tkaitchuck/aHash/issues/163 and https://github.com/tkaitchuck/aHash/issues/166. Luckily, `core` provides `SipHasher` even on no-std (sadly its SipHash-2-4 unlike the SipHash-1-3 used by the `DefaultHasher` in `std`). Thus, we drop the `ahash` dependency entirely here and simply wrap `SipHasher` for our `no-std` HashMaps. --- .github/workflows/build.yml | 2 +- bench/Cargo.toml | 2 +- ci/check-cfg-flags.py | 2 +- fuzz/Cargo.toml | 1 - fuzz/src/chanmon_consistency.rs | 23 ++--- fuzz/src/full_stack.rs | 19 ++-- fuzz/src/indexedmap.rs | 10 +- fuzz/src/router.rs | 24 ++--- lightning/Cargo.toml | 16 +--- lightning/src/lib.rs | 103 +-------------------- lightning/src/util/hash_tables.rs | 149 ++++++++++++++++++++++++++++++ lightning/src/util/mod.rs | 1 + 12 files changed, 193 insertions(+), 159 deletions(-) create mode 100644 lightning/src/util/hash_tables.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ab10b36d25a..cff105470d2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,7 +110,7 @@ jobs: run: | cd lightning RUSTFLAGS="--cfg=require_route_graph_test" cargo test - RUSTFLAGS="--cfg=require_route_graph_test" cargo test --features hashbrown,ahash + RUSTFLAGS="--cfg=require_route_graph_test" cargo test --features hashbrown cd .. - name: Run benchmarks on Rust ${{ matrix.toolchain }} run: | diff --git a/bench/Cargo.toml b/bench/Cargo.toml index ff254e1d9cc..05354890c2a 100644 --- a/bench/Cargo.toml +++ b/bench/Cargo.toml @@ -9,7 +9,7 @@ name = "bench" harness = false [features] -hashbrown = ["lightning/hashbrown", "lightning/ahash"] +hashbrown = ["lightning/hashbrown"] [dependencies] lightning = { path = "../lightning", features = ["_test_utils", "criterion"] } diff --git a/ci/check-cfg-flags.py b/ci/check-cfg-flags.py index 8952b693a60..0b3a4d569ad 100755 --- a/ci/check-cfg-flags.py +++ b/ci/check-cfg-flags.py @@ -13,7 +13,7 @@ def check_feature(feature): pass elif feature == "no-std": pass - elif feature == "ahash": + elif feature == "possiblyrandom": pass elif feature == "getrandom": pass diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 34c8704a7ed..d87be2ef6a4 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -22,7 +22,6 @@ lightning = { path = "../lightning", features = ["regex", "hashbrown", "_test_ut lightning-rapid-gossip-sync = { path = "../lightning-rapid-gossip-sync" } bitcoin = { version = "0.30.2", features = ["secp-lowmemory"] } hex = { package = "hex-conservative", version = "0.1.1", default-features = false } -hashbrown = "0.13" afl = { version = "0.12", optional = true } honggfuzz = { version = "0.5", optional = true, default-features = false } diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index e5ee77a5953..36e7cea8a22 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -51,6 +51,7 @@ use lightning::offers::invoice_request::UnsignedInvoiceRequest; use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath}; use lightning::util::test_channel_signer::{TestChannelSigner, EnforcementState}; use lightning::util::errors::APIError; +use lightning::util::hash_tables::*; use lightning::util::logger::Logger; use lightning::util::config::UserConfig; use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer}; @@ -66,7 +67,6 @@ use bitcoin::secp256k1::schnorr; use std::mem; use std::cmp::{self, Ordering}; -use hashbrown::{HashSet, hash_map, HashMap}; use std::sync::{Arc,Mutex}; use std::sync::atomic; use std::io::Cursor; @@ -157,7 +157,7 @@ impl TestChainMonitor { logger, keys, persister, - latest_monitors: Mutex::new(HashMap::new()), + latest_monitors: Mutex::new(new_hash_map()), } } } @@ -173,16 +173,13 @@ impl chain::Watch for TestChainMonitor { fn update_channel(&self, funding_txo: OutPoint, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus { let mut map_lock = self.latest_monitors.lock().unwrap(); - let mut map_entry = match map_lock.entry(funding_txo) { - hash_map::Entry::Occupied(entry) => entry, - hash_map::Entry::Vacant(_) => panic!("Didn't have monitor on update call"), - }; + let map_entry = map_lock.get_mut(&funding_txo).expect("Didn't have monitor on update call"); let deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor)>:: - read(&mut Cursor::new(&map_entry.get().1), (&*self.keys, &*self.keys)).unwrap().1; + read(&mut Cursor::new(&map_entry.1), (&*self.keys, &*self.keys)).unwrap().1; deserialized_monitor.update_monitor(update, &&TestBroadcaster{}, &&FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }, &self.logger).unwrap(); let mut ser = VecWriter(Vec::new()); deserialized_monitor.write(&mut ser).unwrap(); - map_entry.insert((update.update_id, ser.0)); + *map_entry = (update.update_id, ser.0); self.chain_monitor.update_channel(funding_txo, update) } @@ -467,7 +464,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { ($node_id: expr, $fee_estimator: expr) => { { let logger: Arc = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone())); let node_secret = SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, $node_id]).unwrap(); - let keys_manager = Arc::new(KeyProvider { node_secret, rand_bytes_id: atomic::AtomicU32::new(0), enforcement_states: Mutex::new(HashMap::new()) }); + let keys_manager = Arc::new(KeyProvider { node_secret, rand_bytes_id: atomic::AtomicU32::new(0), enforcement_states: Mutex::new(new_hash_map()) }); let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(), Arc::new(TestPersister { update_ret: Mutex::new(ChannelMonitorUpdateStatus::Completed) @@ -508,13 +505,13 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { config.manually_accept_inbound_channels = true; } - let mut monitors = HashMap::new(); + let mut monitors = new_hash_map(); let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap(); for (outpoint, (update_id, monitor_ser)) in old_monitors.drain() { monitors.insert(outpoint, <(BlockHash, ChannelMonitor)>::read(&mut Cursor::new(&monitor_ser), (&*$keys_manager, &*$keys_manager)).expect("Failed to read monitor").1); chain_monitor.latest_monitors.lock().unwrap().insert(outpoint, (update_id, monitor_ser)); } - let mut monitor_refs = HashMap::new(); + let mut monitor_refs = new_hash_map(); for (outpoint, monitor) in monitors.iter_mut() { monitor_refs.insert(*outpoint, monitor); } @@ -981,7 +978,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { // In case we get 256 payments we may have a hash collision, resulting in the // second claim/fail call not finding the duplicate-hash HTLC, so we have to // deduplicate the calls here. - let mut claim_set = HashSet::new(); + let mut claim_set = new_hash_map(); let mut events = nodes[$node].get_and_clear_pending_events(); // Sort events so that PendingHTLCsForwardable get processed last. This avoids a // case where we first process a PendingHTLCsForwardable, then claim/fail on a @@ -1003,7 +1000,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { for event in events.drain(..) { match event { events::Event::PaymentClaimable { payment_hash, .. } => { - if claim_set.insert(payment_hash.0) { + if claim_set.insert(payment_hash.0, ()).is_none() { if $fail { nodes[$node].fail_htlc_backwards(&payment_hash); } else { diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 2150bcfb73d..89212e58370 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -49,6 +49,7 @@ use lightning::routing::gossip::{P2PGossipSync, NetworkGraph}; use lightning::routing::utxo::UtxoLookup; use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters, Router}; use lightning::util::config::{ChannelConfig, UserConfig}; +use lightning::util::hash_tables::*; use lightning::util::errors::APIError; use lightning::util::test_channel_signer::{TestChannelSigner, EnforcementState}; use lightning::util::logger::Logger; @@ -63,7 +64,6 @@ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature}; use bitcoin::secp256k1::schnorr; use std::cell::RefCell; -use hashbrown::{HashMap, hash_map}; use std::convert::TryInto; use std::cmp; use std::sync::{Arc, Mutex}; @@ -229,7 +229,7 @@ impl<'a> MoneyLossDetector<'a> { peers, funding_txn: Vec::new(), - txids_confirmed: HashMap::new(), + txids_confirmed: new_hash_map(), header_hashes: vec![(genesis_block(Network::Bitcoin).block_hash(), 0)], height: 0, max_height: 0, @@ -241,13 +241,10 @@ impl<'a> MoneyLossDetector<'a> { let mut txdata = Vec::with_capacity(all_txn.len()); for (idx, tx) in all_txn.iter().enumerate() { let txid = tx.txid(); - match self.txids_confirmed.entry(txid) { - hash_map::Entry::Vacant(e) => { - e.insert(self.height); - txdata.push((idx + 1, tx)); - }, - _ => {}, - } + self.txids_confirmed.entry(txid).or_insert_with(|| { + txdata.push((idx + 1, tx)); + self.height + }); } self.blocks_connected += 1; @@ -489,7 +486,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { node_secret: our_network_key.clone(), inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()), counter: AtomicU64::new(0), - signer_state: RefCell::new(HashMap::new()) + signer_state: RefCell::new(new_hash_map()) }); let network = Network::Bitcoin; let best_block_timestamp = genesis_block(network).header.time; @@ -518,7 +515,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { let mut intercepted_htlcs: Vec = Vec::new(); let mut payments_sent: u16 = 0; let mut pending_funding_generation: Vec<(ChannelId, PublicKey, u64, ScriptBuf)> = Vec::new(); - let mut pending_funding_signatures = HashMap::new(); + let mut pending_funding_signatures = new_hash_map(); loop { match get_slice!(1)[0] { diff --git a/fuzz/src/indexedmap.rs b/fuzz/src/indexedmap.rs index 7cbb8957ad2..58775d02467 100644 --- a/fuzz/src/indexedmap.rs +++ b/fuzz/src/indexedmap.rs @@ -9,7 +9,7 @@ use lightning::util::indexed_map::{IndexedMap, self}; use std::collections::{BTreeMap, btree_map}; -use hashbrown::HashSet; +use lightning::util::hash_tables::*; use crate::utils::test_logger; @@ -80,23 +80,23 @@ fn check_eq(btree: &BTreeMap, mut indexed: IndexedMap) { } } - let mut key_set = HashSet::with_capacity(256); + let mut key_set = hash_map_with_capacity(1024); for k in indexed.unordered_keys() { - assert!(key_set.insert(*k)); + assert!(key_set.insert(*k, ()).is_none()); assert!(btree.contains_key(k)); } assert_eq!(key_set.len(), btree.len()); key_set.clear(); for (k, v) in indexed.unordered_iter() { - assert!(key_set.insert(*k)); + assert!(key_set.insert(*k, ()).is_none()); assert_eq!(btree.get(k).unwrap(), v); } assert_eq!(key_set.len(), btree.len()); key_set.clear(); for (k, v) in indexed_clone.unordered_iter_mut() { - assert!(key_set.insert(*k)); + assert!(key_set.insert(*k, ()).is_none()); assert_eq!(btree.get(k).unwrap(), v); } assert_eq!(key_set.len(), btree.len()); diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 23ae3e24cda..bf94c910531 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -23,6 +23,7 @@ use lightning::routing::utxo::{UtxoFuture, UtxoLookup, UtxoLookupError, UtxoResu use lightning::routing::router::{find_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters}; use lightning::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters}; use lightning::util::config::UserConfig; +use lightning::util::hash_tables::*; use lightning::util::ser::Readable; use bitcoin::hashes::Hash; @@ -32,7 +33,6 @@ use bitcoin::network::constants::Network; use crate::utils::test_logger; use std::convert::TryInto; -use hashbrown::HashSet; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -198,7 +198,7 @@ pub fn do_test(data: &[u8], out: Out) { net_graph: &net_graph, }; - let mut node_pks = HashSet::new(); + let mut node_pks = new_hash_map(); let mut scid = 42; macro_rules! first_hops { @@ -208,7 +208,8 @@ pub fn do_test(data: &[u8], out: Out) { count => { for _ in 0..count { scid += 1; - let rnid = node_pks.iter().skip(u16::from_be_bytes(get_slice!(2).try_into().unwrap()) as usize % node_pks.len()).next().unwrap(); + let (rnid, _) = + node_pks.iter().skip(u16::from_be_bytes(get_slice!(2).try_into().unwrap()) as usize % node_pks.len()).next().unwrap(); let capacity = u64::from_be_bytes(get_slice!(8).try_into().unwrap()); $first_hops_vec.push(ChannelDetails { channel_id: ChannelId::new_zero(), @@ -257,7 +258,8 @@ pub fn do_test(data: &[u8], out: Out) { let count = get_slice!(1)[0]; for _ in 0..count { scid += 1; - let rnid = node_pks.iter().skip(slice_to_be16(get_slice!(2))as usize % node_pks.len()).next().unwrap(); + let (rnid, _) = + node_pks.iter().skip(slice_to_be16(get_slice!(2)) as usize % node_pks.len()).next().unwrap(); $last_hops.push(RouteHint(vec![RouteHintHop { src_node_id: *rnid, short_channel_id: scid, @@ -277,7 +279,7 @@ pub fn do_test(data: &[u8], out: Out) { ($first_hops: expr, $node_pks: expr, $route_params: expr) => { let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &net_graph, &logger); let random_seed_bytes: [u8; 32] = [get_slice!(1)[0]; 32]; - for target in $node_pks { + for (target, ()) in $node_pks { let final_value_msat = slice_to_be64(get_slice!(8)); let final_cltv_expiry_delta = slice_to_be32(get_slice!(4)); let route_params = $route_params(final_value_msat, final_cltv_expiry_delta, target); @@ -297,20 +299,20 @@ pub fn do_test(data: &[u8], out: Out) { return; } let msg = decode_msg_with_len16!(msgs::UnsignedNodeAnnouncement, 288); - node_pks.insert(get_pubkey_from_node_id!(msg.node_id)); + node_pks.insert(get_pubkey_from_node_id!(msg.node_id), ()); let _ = net_graph.update_node_from_unsigned_announcement(&msg); }, 1 => { let msg = decode_msg_with_len16!(msgs::UnsignedChannelAnnouncement, 32+8+33*4); - node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1)); - node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2)); + node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1), ()); + node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2), ()); let _ = net_graph.update_channel_from_unsigned_announcement:: <&FuzzChainSource<'_, '_, Out>>(&msg, &None); }, 2 => { let msg = decode_msg_with_len16!(msgs::UnsignedChannelAnnouncement, 32+8+33*4); - node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1)); - node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2)); + node_pks.insert(get_pubkey_from_node_id!(msg.node_id_1), ()); + node_pks.insert(get_pubkey_from_node_id!(msg.node_id_2), ()); let _ = net_graph.update_channel_from_unsigned_announcement(&msg, &Some(&chain_source)); }, 3 => { @@ -367,7 +369,7 @@ pub fn do_test(data: &[u8], out: Out) { }).collect(); let mut features = Bolt12InvoiceFeatures::empty(); features.set_basic_mpp_optional(); - find_routes!(first_hops, vec![dummy_pk].iter(), |final_amt, _, _| { + find_routes!(first_hops, [(dummy_pk, ())].iter(), |final_amt, _, _| { RouteParameters::from_payment_params_and_value(PaymentParameters::blinded(last_hops.clone()) .with_bolt12_features(features.clone()).unwrap(), final_amt) diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index 8214193cc42..96070e4d0dd 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -31,7 +31,7 @@ unsafe_revoked_tx_signing = [] # Override signing to not include randomness when generating signatures for test vectors. _test_vectors = [] -no-std = ["hashbrown", "ahash", "bitcoin/no-std", "core2/alloc", "libm"] +no-std = ["hashbrown", "possiblyrandom", "bitcoin/no-std", "core2/alloc", "libm"] std = ["bitcoin/std"] # Generates low-r bitcoin signatures, which saves 1 byte in 50% of the cases @@ -42,8 +42,8 @@ default = ["std", "grind_signatures"] [dependencies] bitcoin = { version = "0.30.2", default-features = false, features = ["secp-recovery"] } -hashbrown = { version = "0.13", optional = true } -ahash = { version = "0.8", optional = true, default-features = false } +hashbrown = { version = "0.13", optional = true, default-features = false } +possiblyrandom = { version = "0.1", optional = true, default-features = false } hex = { package = "hex-conservative", version = "0.1.1", default-features = false } regex = { version = "1.5.6", optional = true } backtrace = { version = "0.3", optional = true } @@ -51,16 +51,6 @@ backtrace = { version = "0.3", optional = true } core2 = { version = "0.3.0", optional = true, default-features = false } libm = { version = "0.2", optional = true, default-features = false } -# Because ahash no longer (kinda poorly) does it for us, (roughly) list out the targets that -# getrandom supports and turn on ahash's `runtime-rng` feature for them. -[target.'cfg(not(any(target_os = "unknown", target_os = "none")))'.dependencies] -ahash = { version = "0.8", optional = true, default-features = false, features = ["runtime-rng"] } - -# Not sure what target_os gets set to for sgx, so to be safe always enable runtime-rng for x86_64 -# platforms (assuming LDK isn't being used on embedded x86-64 running directly on metal). -[target.'cfg(target_arch = "x86_64")'.dependencies] -ahash = { version = "0.8", optional = true, default-features = false, features = ["runtime-rng"] } - [dev-dependencies] regex = "1.5.6" diff --git a/lightning/src/lib.rs b/lightning/src/lib.rs index 9bc15832870..1adf3786b76 100644 --- a/lightning/src/lib.rs +++ b/lightning/src/lib.rs @@ -165,113 +165,12 @@ mod io_extras { } mod prelude { - #[cfg(feature = "hashbrown")] - extern crate hashbrown; - #[cfg(feature = "ahash")] - extern crate ahash; - pub use alloc::{vec, vec::Vec, string::String, collections::VecDeque, boxed::Box}; pub use alloc::borrow::ToOwned; pub use alloc::string::ToString; - // For no-std builds, we need to use hashbrown, however, by default, it doesn't randomize the - // hashing and is vulnerable to HashDoS attacks. Thus, when not fuzzing, we use its default - // ahash hashing algorithm but randomize, opting to not randomize when fuzzing to avoid false - // positive branch coverage. - - #[cfg(not(feature = "hashbrown"))] - mod std_hashtables { - pub(crate) use std::collections::{HashMap, HashSet, hash_map}; - - pub(crate) type OccupiedHashMapEntry<'a, K, V> = - std::collections::hash_map::OccupiedEntry<'a, K, V>; - pub(crate) type VacantHashMapEntry<'a, K, V> = - std::collections::hash_map::VacantEntry<'a, K, V>; - } - #[cfg(not(feature = "hashbrown"))] - pub(crate) use std_hashtables::*; - - #[cfg(feature = "hashbrown")] - pub(crate) use self::hashbrown::hash_map; - - #[cfg(all(feature = "hashbrown", fuzzing))] - mod nonrandomized_hashbrown { - pub(crate) use hashbrown::{HashMap, HashSet}; - - pub(crate) type OccupiedHashMapEntry<'a, K, V> = - hashbrown::hash_map::OccupiedEntry<'a, K, V, hashbrown::hash_map::DefaultHashBuilder>; - pub(crate) type VacantHashMapEntry<'a, K, V> = - hashbrown::hash_map::VacantEntry<'a, K, V, hashbrown::hash_map::DefaultHashBuilder>; - } - #[cfg(all(feature = "hashbrown", fuzzing))] - pub(crate) use nonrandomized_hashbrown::*; - - - #[cfg(all(feature = "hashbrown", not(fuzzing)))] - mod randomized_hashtables { - use super::*; - use ahash::RandomState; - - pub(crate) type HashMap = hashbrown::HashMap; - pub(crate) type HashSet = hashbrown::HashSet; - - pub(crate) type OccupiedHashMapEntry<'a, K, V> = - hashbrown::hash_map::OccupiedEntry<'a, K, V, RandomState>; - pub(crate) type VacantHashMapEntry<'a, K, V> = - hashbrown::hash_map::VacantEntry<'a, K, V, RandomState>; - - pub(crate) fn new_hash_map() -> HashMap { - HashMap::with_hasher(RandomState::new()) - } - pub(crate) fn hash_map_with_capacity(cap: usize) -> HashMap { - HashMap::with_capacity_and_hasher(cap, RandomState::new()) - } - pub(crate) fn hash_map_from_iter>(iter: I) -> HashMap { - let iter = iter.into_iter(); - let min_size = iter.size_hint().0; - let mut res = HashMap::with_capacity_and_hasher(min_size, RandomState::new()); - res.extend(iter); - res - } - - pub(crate) fn new_hash_set() -> HashSet { - HashSet::with_hasher(RandomState::new()) - } - pub(crate) fn hash_set_with_capacity(cap: usize) -> HashSet { - HashSet::with_capacity_and_hasher(cap, RandomState::new()) - } - pub(crate) fn hash_set_from_iter>(iter: I) -> HashSet { - let iter = iter.into_iter(); - let min_size = iter.size_hint().0; - let mut res = HashSet::with_capacity_and_hasher(min_size, RandomState::new()); - res.extend(iter); - res - } - } - - #[cfg(any(not(feature = "hashbrown"), fuzzing))] - mod randomized_hashtables { - use super::*; - - pub(crate) fn new_hash_map() -> HashMap { HashMap::new() } - pub(crate) fn hash_map_with_capacity(cap: usize) -> HashMap { - HashMap::with_capacity(cap) - } - pub(crate) fn hash_map_from_iter>(iter: I) -> HashMap { - HashMap::from_iter(iter) - } - - pub(crate) fn new_hash_set() -> HashSet { HashSet::new() } - pub(crate) fn hash_set_with_capacity(cap: usize) -> HashSet { - HashSet::with_capacity(cap) - } - pub(crate) fn hash_set_from_iter>(iter: I) -> HashSet { - HashSet::from_iter(iter) - } - } - - pub(crate) use randomized_hashtables::*; + pub(crate) use crate::util::hash_tables::*; } #[cfg(all(not(ldk_bench), feature = "backtrace", feature = "std", test))] diff --git a/lightning/src/util/hash_tables.rs b/lightning/src/util/hash_tables.rs new file mode 100644 index 00000000000..69581e5e20a --- /dev/null +++ b/lightning/src/util/hash_tables.rs @@ -0,0 +1,149 @@ +//! Generally LDK uses `std`'s `HashMap`s, however when building for no-std, LDK uses `hashbrown`'s +//! `HashMap`s with the `std` `SipHasher` and uses `getrandom` to opportunistically randomize it, +//! if randomization is available. +//! +//! This module simply re-exports the `HashMap` used in LDK for public consumption. + +#[cfg(feature = "hashbrown")] +extern crate hashbrown; +#[cfg(feature = "possiblyrandom")] +extern crate possiblyrandom; + +// For no-std builds, we need to use hashbrown, however, by default, it doesn't randomize the +// hashing and is vulnerable to HashDoS attacks. Thus, we use the core SipHasher when not using +// std, but use `getrandom` to randomize it if its available. + +#[cfg(not(feature = "hashbrown"))] +mod std_hashtables { + pub use std::collections::HashMap; + pub use std::collections::hash_map::RandomState; + + pub(crate) use std::collections::{HashSet, hash_map}; + + pub(crate) type OccupiedHashMapEntry<'a, K, V> = + std::collections::hash_map::OccupiedEntry<'a, K, V>; + pub(crate) type VacantHashMapEntry<'a, K, V> = + std::collections::hash_map::VacantEntry<'a, K, V>; + + /// Builds a new [`HashMap`]. + pub fn new_hash_map() -> HashMap { HashMap::new() } + /// Builds a new [`HashMap`] with the given capacity. + pub fn hash_map_with_capacity(cap: usize) -> HashMap { + HashMap::with_capacity(cap) + } + pub(crate) fn hash_map_from_iter>(iter: I) -> HashMap { + HashMap::from_iter(iter) + } + + pub(crate) fn new_hash_set() -> HashSet { HashSet::new() } + pub(crate) fn hash_set_with_capacity(cap: usize) -> HashSet { + HashSet::with_capacity(cap) + } + pub(crate) fn hash_set_from_iter>(iter: I) -> HashSet { + HashSet::from_iter(iter) + } +} +#[cfg(not(feature = "hashbrown"))] +pub use std_hashtables::*; + +#[cfg(feature = "hashbrown")] +pub(crate) use self::hashbrown::hash_map; + +#[cfg(feature = "hashbrown")] +mod hashbrown_tables { + #[cfg(feature = "std")] + mod hasher { + pub use std::collections::hash_map::RandomState; + } + #[cfg(not(feature = "std"))] + mod hasher { + #![allow(deprecated)] // hash::SipHasher was deprecated in favor of something only in std. + use core::hash::{BuildHasher, SipHasher}; + + #[derive(Clone, Copy)] + /// A simple implementation of [`BuildHasher`] that uses `getrandom` to opportunistically + /// randomize, if the platform supports it. + pub struct RandomState { + k0: u64, k1: u64, + } + + impl RandomState { + /// Constructs a new [`RandomState`] which may or may not be random, depending on the + /// target platform. + pub fn new() -> RandomState { + let (k0, k1); + #[cfg(all(not(fuzzing), feature = "possiblyrandom"))] { + let mut keys = [0; 16]; + possiblyrandom::getpossiblyrandom(&mut keys); + + let mut k0_bytes = [0; 8]; + let mut k1_bytes = [0; 8]; + k0_bytes.copy_from_slice(&keys[..8]); + k1_bytes.copy_from_slice(&keys[8..]); + k0 = u64::from_le_bytes(k0_bytes); + k1 = u64::from_le_bytes(k1_bytes); + } + #[cfg(any(fuzzing, not(feature = "possiblyrandom")))] { + k0 = 0; + k1 = 0; + } + RandomState { k0, k1 } + } + } + + impl Default for RandomState { + fn default() -> RandomState { RandomState::new() } + } + + impl BuildHasher for RandomState { + type Hasher = SipHasher; + fn build_hasher(&self) -> SipHasher { + SipHasher::new_with_keys(self.k0, self.k1) + } + } + } + + pub use hasher::*; + use super::*; + + /// The HashMap type used in LDK. + pub type HashMap = hashbrown::HashMap; + pub(crate) type HashSet = hashbrown::HashSet; + + pub(crate) type OccupiedHashMapEntry<'a, K, V> = + hashbrown::hash_map::OccupiedEntry<'a, K, V, RandomState>; + pub(crate) type VacantHashMapEntry<'a, K, V> = + hashbrown::hash_map::VacantEntry<'a, K, V, RandomState>; + + /// Builds a new [`HashMap`]. + pub fn new_hash_map() -> HashMap { + HashMap::with_hasher(RandomState::new()) + } + /// Builds a new [`HashMap`] with the given capacity. + pub fn hash_map_with_capacity(cap: usize) -> HashMap { + HashMap::with_capacity_and_hasher(cap, RandomState::new()) + } + pub(crate) fn hash_map_from_iter>(iter: I) -> HashMap { + let iter = iter.into_iter(); + let min_size = iter.size_hint().0; + let mut res = HashMap::with_capacity_and_hasher(min_size, RandomState::new()); + res.extend(iter); + res + } + + pub(crate) fn new_hash_set() -> HashSet { + HashSet::with_hasher(RandomState::new()) + } + pub(crate) fn hash_set_with_capacity(cap: usize) -> HashSet { + HashSet::with_capacity_and_hasher(cap, RandomState::new()) + } + pub(crate) fn hash_set_from_iter>(iter: I) -> HashSet { + let iter = iter.into_iter(); + let min_size = iter.size_hint().0; + let mut res = HashSet::with_capacity_and_hasher(min_size, RandomState::new()); + res.extend(iter); + res + } +} +#[cfg(feature = "hashbrown")] +pub use hashbrown_tables::*; diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs index 6ce00acab45..31bdf1ca53c 100644 --- a/lightning/src/util/mod.rs +++ b/lightning/src/util/mod.rs @@ -32,6 +32,7 @@ pub(crate) mod atomic_counter; pub(crate) mod byte_utils; pub(crate) mod transaction_utils; pub(crate) mod time; +pub mod hash_tables; pub mod indexed_map; From eecd2cdf4fadc92fa635aa72d32bce1299c470c6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 13 Feb 2024 18:15:20 +0000 Subject: [PATCH 3/3] Drop `lightning-invoice` dependency on hashbrown` --- lightning-invoice/Cargo.toml | 4 ++-- lightning-invoice/src/lib.rs | 7 ------- lightning-invoice/src/utils.rs | 7 ++++--- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lightning-invoice/Cargo.toml b/lightning-invoice/Cargo.toml index 4376ac0dc70..1d2a4cdbfbe 100644 --- a/lightning-invoice/Cargo.toml +++ b/lightning-invoice/Cargo.toml @@ -16,7 +16,7 @@ rustdoc-args = ["--cfg", "docsrs"] [features] default = ["std"] -no-std = ["hashbrown", "lightning/no-std"] +no-std = ["lightning/no-std"] std = ["bitcoin/std", "num-traits/std", "lightning/std", "bech32/std"] [dependencies] @@ -24,7 +24,6 @@ bech32 = { version = "0.9.0", default-features = false } lightning = { version = "0.0.121", path = "../lightning", default-features = false } secp256k1 = { version = "0.27.0", default-features = false, features = ["recovery", "alloc"] } num-traits = { version = "0.2.8", default-features = false } -hashbrown = { version = "0.13", optional = true } serde = { version = "1.0.118", optional = true } bitcoin = { version = "0.30.2", default-features = false } @@ -32,3 +31,4 @@ bitcoin = { version = "0.30.2", default-features = false } lightning = { version = "0.0.121", path = "../lightning", default-features = false, features = ["_test_utils"] } hex = { package = "hex-conservative", version = "0.1.1", default-features = false } serde_json = { version = "1"} +hashbrown = { version = "0.13", default-features = false } diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 610f739e57a..9c6badad6d9 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -79,14 +79,7 @@ mod tb; #[allow(unused_imports)] mod prelude { - #[cfg(feature = "hashbrown")] - extern crate hashbrown; - pub use alloc::{vec, vec::Vec, string::String}; - #[cfg(not(feature = "hashbrown"))] - pub use std::collections::{HashMap, hash_map}; - #[cfg(feature = "hashbrown")] - pub use self::hashbrown::{HashMap, HashSet, hash_map}; pub use alloc::string::ToString; } diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 5e8b72467e5..d45a4e8e646 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -16,6 +16,7 @@ use lightning::routing::gossip::RoutingFees; use lightning::routing::router::{RouteHint, RouteHintHop, Router}; use lightning::util::logger::{Logger, Record}; use secp256k1::PublicKey; +use alloc::collections::{btree_map, BTreeMap}; use core::ops::Deref; use core::time::Duration; use core::iter::Iterator; @@ -603,7 +604,7 @@ fn sort_and_filter_channels( where L::Target: Logger, { - let mut filtered_channels: HashMap = HashMap::new(); + let mut filtered_channels: BTreeMap = BTreeMap::new(); let min_inbound_capacity = min_inbound_capacity_msat.unwrap_or(0); let mut min_capacity_channel_exists = false; let mut online_channel_exists = false; @@ -664,7 +665,7 @@ where } match filtered_channels.entry(channel.counterparty.node_id) { - hash_map::Entry::Occupied(mut entry) => { + btree_map::Entry::Occupied(mut entry) => { let current_max_capacity = entry.get().inbound_capacity_msat; // If this channel is public and the previous channel is not, ensure we replace the // previous channel to avoid announcing non-public channels. @@ -697,7 +698,7 @@ where channel.inbound_capacity_msat); } } - hash_map::Entry::Vacant(entry) => { + btree_map::Entry::Vacant(entry) => { entry.insert(channel); } }