Merge pull request #2810 from TheBlueMatt/2023-12-arbitrary-fuzz-config

Update `full_stack_target` to take an arbitrary config object

Author: TheBlueMatt

.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
+          RUSTFLAGS="--cfg=require_route_graph_test" cargo test --features hashbrown,ahash
           cd ..
       - name: Run benchmarks on Rust ${{ matrix.toolchain }}
         run: |
@@ -176,7 +176,7 @@ jobs:
   fuzz:
     runs-on: ubuntu-latest
     env:
-      TOOLCHAIN: 1.58
+      TOOLCHAIN: 1.63
     steps:
       - name: Checkout source code
         uses: actions/checkout@v3
@@ -188,6 +188,10 @@ jobs:
         run: |
           sudo apt-get update
           sudo apt-get -y install build-essential binutils-dev libunwind-dev
+      - name: Pin the regex dependency
+        run: |
+          cd fuzz && cargo update -p regex --precise "1.9.6" --verbose && cd ..
+          cd lightning-invoice/fuzz && cargo update -p regex --precise "1.9.6" --verbose
       - name: Sanity check fuzz targets on Rust ${{ env.TOOLCHAIN }}
         run: cd fuzz && RUSTFLAGS="--cfg=fuzzing" cargo test --verbose --color always
       - name: Run fuzzers

bench/Cargo.toml

@@ -9,7 +9,7 @@ name = "bench"
 harness = false
 
 [features]
-hashbrown = ["lightning/hashbrown"]
+hashbrown = ["lightning/hashbrown", "lightning/ahash"]
 
 [dependencies]
 lightning = { path = "../lightning", features = ["_test_utils", "criterion"] }

ci/check-cfg-flags.py

@@ -13,6 +13,8 @@ def check_feature(feature):
         pass
     elif feature == "no-std":
         pass
+    elif feature == "ahash":
+        pass
     elif feature == "hashbrown":
         pass
     elif feature == "backtrace":

fuzz/Cargo.toml

@@ -22,7 +22,7 @@ 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.8"
+hashbrown = "0.13"
 
 afl = { version = "0.12", optional = true }
 honggfuzz = { version = "0.5", optional = true, default-features = false }

fuzz/src/full_stack.rs

@@ -83,7 +83,7 @@ pub fn do_test(data: &[u8]) {
 				Ok(len) => len,
 				Err(_) => return,
 			};
-			buf.copy_from_slice(&get_slice!(len as usize + 16));
+			buf[..len as usize + 16].copy_from_slice(&get_slice!(len as usize + 16));
 			match crypter.decrypt_message(&mut buf[..len as usize + 16]) {
 				Ok(_) => {},
 				Err(_) => return,

fuzz/src/peer_crypt.rs

@@ -24,7 +24,7 @@ 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.8", optional = true }
+hashbrown = { version = "0.13", optional = true }
 serde = { version = "1.0.118", optional = true }
 bitcoin = { version = "0.30.2", default-features = false }
 

lightning-invoice/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", "bitcoin/no-std", "core2/alloc", "libm"]
+no-std = ["hashbrown", "ahash", "bitcoin/no-std", "core2/alloc", "libm"]
 std = ["bitcoin/std"]
 
 # Generates low-r bitcoin signatures, which saves 1 byte in 50% of the cases
@@ -42,14 +42,25 @@ default = ["std", "grind_signatures"]
 [dependencies]
 bitcoin = { version = "0.30.2", default-features = false, features = ["secp-recovery"] }
 
-hashbrown = { version = "0.8", optional = true }
+hashbrown = { version = "0.13", optional = true }
+ahash = { version = "0.8", 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 }
 
 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"
 

lightning/Cargo.toml

@@ -43,7 +43,6 @@ use crate::ln::channelmanager::ChannelDetails;
 
 use crate::prelude::*;
 use crate::sync::{RwLock, RwLockReadGuard, Mutex, MutexGuard};
-use core::iter::FromIterator;
 use core::ops::Deref;
 use core::sync::atomic::{AtomicUsize, Ordering};
 use bitcoin::secp256k1::PublicKey;
@@ -318,7 +317,7 @@ where C::Target: chain::Filter,
 		FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
 	{
 		let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down.";
-		let funding_outpoints: HashSet<OutPoint> = HashSet::from_iter(self.monitors.read().unwrap().keys().cloned());
+		let funding_outpoints = hash_set_from_iter(self.monitors.read().unwrap().keys().cloned());
 		for funding_outpoint in funding_outpoints.iter() {
 			let monitor_lock = self.monitors.read().unwrap();
 			if let Some(monitor_state) = monitor_lock.get(funding_outpoint) {
@@ -420,7 +419,7 @@ where C::Target: chain::Filter,
 	/// transactions relevant to the watched channels.
 	pub fn new(chain_source: Option<C>, broadcaster: T, logger: L, feeest: F, persister: P) -> Self {
 		Self {
-			monitors: RwLock::new(HashMap::new()),
+			monitors: RwLock::new(new_hash_map()),
 			sync_persistence_id: AtomicCounter::new(),
 			chain_source,
 			broadcaster,
@@ -486,9 +485,9 @@ where C::Target: chain::Filter,
 	#[cfg(not(c_bindings))]
 	/// Lists the pending updates for each [`ChannelMonitor`] (by `OutPoint` being monitored).
 	pub fn list_pending_monitor_updates(&self) -> HashMap<OutPoint, Vec<MonitorUpdateId>> {
-		self.monitors.read().unwrap().iter().map(|(outpoint, holder)| {
+		hash_map_from_iter(self.monitors.read().unwrap().iter().map(|(outpoint, holder)| {
 			(*outpoint, holder.pending_monitor_updates.lock().unwrap().clone())
-		}).collect()
+		}))
 	}
 
 	#[cfg(c_bindings)]

lightning/src/chain/chainmonitor.rs

@@ -1235,7 +1235,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 			channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx
 		);
 
-		let mut outputs_to_watch = HashMap::new();
+		let mut outputs_to_watch = new_hash_map();
 		outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
 
 		Self::from_impl(ChannelMonitorImpl {
@@ -1262,17 +1262,17 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 			on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
 
 			commitment_secrets: CounterpartyCommitmentSecrets::new(),
-			counterparty_claimable_outpoints: HashMap::new(),
-			counterparty_commitment_txn_on_chain: HashMap::new(),
-			counterparty_hash_commitment_number: HashMap::new(),
-			counterparty_fulfilled_htlcs: HashMap::new(),
+			counterparty_claimable_outpoints: new_hash_map(),
+			counterparty_commitment_txn_on_chain: new_hash_map(),
+			counterparty_hash_commitment_number: new_hash_map(),
+			counterparty_fulfilled_htlcs: new_hash_map(),
 
 			prev_holder_signed_commitment_tx: None,
 			current_holder_commitment_tx: holder_commitment_tx,
 			current_counterparty_commitment_number: 1 << 48,
 			current_holder_commitment_number,
 
-			payment_preimages: HashMap::new(),
+			payment_preimages: new_hash_map(),
 			pending_monitor_events: Vec::new(),
 			pending_events: Vec::new(),
 			is_processing_pending_events: false,
@@ -2174,7 +2174,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// HTLCs which were resolved on-chain (i.e. where the final HTLC resolution was done by an
 	/// event from this `ChannelMonitor`).
 	pub(crate) fn get_all_current_outbound_htlcs(&self) -> HashMap<HTLCSource, (HTLCOutputInCommitment, Option<PaymentPreimage>)> {
-		let mut res = HashMap::new();
+		let mut res = new_hash_map();
 		// Just examine the available counterparty commitment transactions. See docs on
 		// `fail_unbroadcast_htlcs`, below, for justification.
 		let us = self.inner.lock().unwrap();
@@ -2226,7 +2226,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 			return self.get_all_current_outbound_htlcs();
 		}
 
-		let mut res = HashMap::new();
+		let mut res = new_hash_map();
 		macro_rules! walk_htlcs {
 			($holder_commitment: expr, $htlc_iter: expr) => {
 				for (htlc, source) in $htlc_iter {
@@ -3172,7 +3172,11 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 							(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
 						), logger);
 				} else {
-					debug_assert!(false, "We should have per-commitment option for any recognized old commitment txn");
+					// Our fuzzers aren't contrained by pesky things like valid signatures, so can
+					// spend our funding output with a transaction which doesn't match our past
+					// commitment transactions. Thus, we can only debug-assert here when not
+					// fuzzing.
+					debug_assert!(cfg!(fuzzing), "We should have per-commitment option for any recognized old commitment txn");
 					fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, tx, height,
 						block_hash, [].iter().map(|reference| *reference), logger);
 				}
@@ -3679,6 +3683,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						claimable_outpoints.append(&mut new_outpoints);
 						if new_outpoints.is_empty() {
 							if let Some((mut new_outpoints, new_outputs)) = self.check_spend_holder_transaction(&tx, height, &block_hash, &logger) {
+								#[cfg(not(fuzzing))]
 								debug_assert!(commitment_tx_to_counterparty_output.is_none(),
 									"A commitment transaction matched as both a counterparty and local commitment tx?");
 								if !new_outputs.1.is_empty() {
@@ -3935,7 +3940,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// Filters a block's `txdata` for transactions spending watched outputs or for any child
 	/// transactions thereof.
 	fn filter_block<'a>(&self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> {
-		let mut matched_txn = HashSet::new();
+		let mut matched_txn = new_hash_set();
 		txdata.iter().filter(|&&(_, tx)| {
 			let mut matches = self.spends_watched_output(tx);
 			for input in tx.input.iter() {
@@ -4450,7 +4455,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		}
 
 		let counterparty_claimable_outpoints_len: u64 = Readable::read(reader)?;
-		let mut counterparty_claimable_outpoints = HashMap::with_capacity(cmp::min(counterparty_claimable_outpoints_len as usize, MAX_ALLOC_SIZE / 64));
+		let mut counterparty_claimable_outpoints = hash_map_with_capacity(cmp::min(counterparty_claimable_outpoints_len as usize, MAX_ALLOC_SIZE / 64));
 		for _ in 0..counterparty_claimable_outpoints_len {
 			let txid: Txid = Readable::read(reader)?;
 			let htlcs_count: u64 = Readable::read(reader)?;
@@ -4464,7 +4469,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		}
 
 		let counterparty_commitment_txn_on_chain_len: u64 = Readable::read(reader)?;
-		let mut counterparty_commitment_txn_on_chain = HashMap::with_capacity(cmp::min(counterparty_commitment_txn_on_chain_len as usize, MAX_ALLOC_SIZE / 32));
+		let mut counterparty_commitment_txn_on_chain = hash_map_with_capacity(cmp::min(counterparty_commitment_txn_on_chain_len as usize, MAX_ALLOC_SIZE / 32));
 		for _ in 0..counterparty_commitment_txn_on_chain_len {
 			let txid: Txid = Readable::read(reader)?;
 			let commitment_number = <U48 as Readable>::read(reader)?.0;
@@ -4474,7 +4479,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		}
 
 		let counterparty_hash_commitment_number_len: u64 = Readable::read(reader)?;
-		let mut counterparty_hash_commitment_number = HashMap::with_capacity(cmp::min(counterparty_hash_commitment_number_len as usize, MAX_ALLOC_SIZE / 32));
+		let mut counterparty_hash_commitment_number = hash_map_with_capacity(cmp::min(counterparty_hash_commitment_number_len as usize, MAX_ALLOC_SIZE / 32));
 		for _ in 0..counterparty_hash_commitment_number_len {
 			let payment_hash: PaymentHash = Readable::read(reader)?;
 			let commitment_number = <U48 as Readable>::read(reader)?.0;
@@ -4497,7 +4502,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let current_holder_commitment_number = <U48 as Readable>::read(reader)?.0;
 
 		let payment_preimages_len: u64 = Readable::read(reader)?;
-		let mut payment_preimages = HashMap::with_capacity(cmp::min(payment_preimages_len as usize, MAX_ALLOC_SIZE / 32));
+		let mut payment_preimages = hash_map_with_capacity(cmp::min(payment_preimages_len as usize, MAX_ALLOC_SIZE / 32));
 		for _ in 0..payment_preimages_len {
 			let preimage: PaymentPreimage = Readable::read(reader)?;
 			let hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array());
@@ -4537,7 +4542,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		}
 
 		let outputs_to_watch_len: u64 = Readable::read(reader)?;
-		let mut outputs_to_watch = HashMap::with_capacity(cmp::min(outputs_to_watch_len as usize, MAX_ALLOC_SIZE / (mem::size_of::<Txid>() + mem::size_of::<u32>() + mem::size_of::<Vec<ScriptBuf>>())));
+		let mut outputs_to_watch = hash_map_with_capacity(cmp::min(outputs_to_watch_len as usize, MAX_ALLOC_SIZE / (mem::size_of::<Txid>() + mem::size_of::<u32>() + mem::size_of::<Vec<ScriptBuf>>())));
 		for _ in 0..outputs_to_watch_len {
 			let txid = Readable::read(reader)?;
 			let outputs_len: u64 = Readable::read(reader)?;
@@ -4579,7 +4584,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut counterparty_node_id = None;
 		let mut confirmed_commitment_tx_counterparty_output = None;
 		let mut spendable_txids_confirmed = Some(Vec::new());
-		let mut counterparty_fulfilled_htlcs = Some(HashMap::new());
+		let mut counterparty_fulfilled_htlcs = Some(new_hash_map());
 		let mut initial_counterparty_commitment_info = None;
 		let mut channel_id = None;
 		read_tlv_fields!(reader, {

lightning/src/chain/channelmonitor.rs

@@ -374,13 +374,13 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		signer.provide_channel_parameters(&channel_parameters);
 
 		let pending_claim_requests_len: u64 = Readable::read(reader)?;
-		let mut pending_claim_requests = HashMap::with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
+		let mut pending_claim_requests = hash_map_with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
 		for _ in 0..pending_claim_requests_len {
 			pending_claim_requests.insert(Readable::read(reader)?, Readable::read(reader)?);
 		}
 
 		let claimable_outpoints_len: u64 = Readable::read(reader)?;
-		let mut claimable_outpoints = HashMap::with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
+		let mut claimable_outpoints = hash_map_with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
 		for _ in 0..claimable_outpoints_len {
 			let outpoint = Readable::read(reader)?;
 			let ancestor_claim_txid = Readable::read(reader)?;
@@ -445,8 +445,8 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 			prev_holder_commitment: None,
 			signer,
 			channel_transaction_parameters: channel_parameters,
-			pending_claim_requests: HashMap::new(),
-			claimable_outpoints: HashMap::new(),
+			pending_claim_requests: new_hash_map(),
+			claimable_outpoints: new_hash_map(),
 			locktimed_packages: BTreeMap::new(),
 			onchain_events_awaiting_threshold_conf: Vec::new(),
 			pending_claim_events: Vec::new(),
@@ -686,7 +686,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 		if let Some(claim_id) = claim_id {
 			if let Some(claim) = self.pending_claim_requests.remove(&claim_id) {
 				for outpoint in claim.outpoints() {
-					self.claimable_outpoints.remove(&outpoint);
+					self.claimable_outpoints.remove(outpoint);
 				}
 			}
 		} else {
@@ -806,7 +806,9 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 						claim_id
 					},
 				};
-				debug_assert!(self.pending_claim_requests.get(&claim_id).is_none());
+				// Because fuzzing can cause hash collisions, we can end up with conflicting claim
+				// ids here, so we only assert when not fuzzing.
+				debug_assert!(cfg!(fuzzing) || self.pending_claim_requests.get(&claim_id).is_none());
 				for k in req.outpoints() {
 					log_info!(logger, "Registering claiming request for {}:{}", k.txid, k.vout);
 					self.claimable_outpoints.insert(k.clone(), (claim_id, conf_height));
@@ -832,7 +834,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 		F::Target: FeeEstimator,
 	{
 		log_debug!(logger, "Updating claims view at height {} with {} matched transactions in block {}", cur_height, txn_matched.len(), conf_height);
-		let mut bump_candidates = HashMap::new();
+		let mut bump_candidates = new_hash_map();
 		for tx in txn_matched {
 			// Scan all input to verify is one of the outpoint spent is of interest for us
 			let mut claimed_outputs_material = Vec::new();
@@ -1018,7 +1020,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 		where B::Target: BroadcasterInterface,
 			F::Target: FeeEstimator,
 	{
-		let mut bump_candidates = HashMap::new();
+		let mut bump_candidates = new_hash_map();
 		let onchain_events_awaiting_threshold_conf =
 			self.onchain_events_awaiting_threshold_conf.drain(..).collect::<Vec<_>>();
 		for entry in onchain_events_awaiting_threshold_conf {

lightning/src/chain/onchaintx.rs

@@ -391,7 +391,7 @@ where
 	/// Returns a new instance backed by the given [`WalletSource`] that serves as an implementation
 	/// of [`CoinSelectionSource`].
 	pub fn new(source: W, logger: L) -> Self {
-		Self { source, logger, locked_utxos: Mutex::new(HashMap::new()) }
+		Self { source, logger, locked_utxos: Mutex::new(new_hash_map()) }
 	}
 
 	/// Performs coin selection on the set of UTXOs obtained from