Drop fuzztarget feature entirely

Some time ago we started transitioning to `cfg(fuzzing)` instead of
exposing a full feature. Here we complete the transition.

Author: TheBlueMatt

fuzz/Cargo.toml

@@ -5,7 +5,7 @@ authors = ["Automatically generated"]
 publish = false
 # Because the function is unused it gets dropped before we link lightning, so
 # we have to duplicate build.rs here. Note that this is only required for
-# fuzztarget mode.
+# fuzzing mode.
 
 [package.metadata]
 cargo-fuzz = true
@@ -18,7 +18,7 @@ stdin_fuzz = []
 
 [dependencies]
 afl = { version = "0.4", optional = true }
-lightning = { path = "../lightning", features = ["fuzztarget"] }
+lightning = { path = "../lightning", features = ["regex"] }
 bitcoin = { version = "0.27", features = ["fuzztarget", "secp-lowmemory"] }
 hex = "0.3"
 honggfuzz = { version = "0.5", optional = true }

lightning/Cargo.toml

@@ -11,7 +11,6 @@ Still missing tons of error-handling. See GitHub issues for suggested projects i
 """
 
 [features]
-fuzztarget = ["bitcoin/fuzztarget", "regex"]
 # Internal test utilities exposed to other repo crates
 _test_utils = ["hex", "regex", "bitcoin/bitcoinconsensus"]
 # Unlog messages superior at targeted level.

lightning/src/chain/chainmonitor.rs

@@ -474,15 +474,15 @@ where C::Target: chain::Filter,
 	/// This wrapper avoids having to update some of our tests for now as they assume the direct
 	/// chain::Watch API wherein we mark a monitor fully-updated by just calling
 	/// channel_monitor_updated once with the highest ID.
-	#[cfg(any(test, feature = "fuzztarget"))]
+	#[cfg(any(test, fuzzing))]
 	pub fn force_channel_monitor_updated(&self, funding_txo: OutPoint, monitor_update_id: u64) {
 		self.pending_monitor_events.lock().unwrap().push(MonitorEvent::UpdateCompleted {
 			funding_txo,
 			monitor_update_id,
 		});
 	}
 
-	#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
+	#[cfg(any(test, fuzzing, feature = "_test_utils"))]
 	pub fn get_and_clear_pending_events(&self) -> Vec<events::Event> {
 		use util::events::EventsProvider;
 		let events = core::cell::RefCell::new(Vec::new());
@@ -630,9 +630,9 @@ where C::Target: chain::Filter,
 				// We should never ever trigger this from within ChannelManager. Technically a
 				// user could use this object with some proxying in between which makes this
 				// possible, but in tests and fuzzing, this should be a panic.
-				#[cfg(any(test, feature = "fuzztarget"))]
+				#[cfg(any(test, fuzzing))]
 				panic!("ChannelManager generated a channel update for a channel that was not yet registered!");
-				#[cfg(not(any(test, feature = "fuzztarget")))]
+				#[cfg(not(any(test, fuzzing)))]
 				Err(ChannelMonitorUpdateErr::PermanentFailure)
 			},
 			Some(monitor_state) => {

lightning/src/chain/channelmonitor.rs

@@ -59,7 +59,7 @@ use sync::Mutex;
 
 /// An update generated by the underlying Channel itself which contains some new information the
 /// ChannelMonitor should be made aware of.
-#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
+#[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 #[must_use]
 pub struct ChannelMonitorUpdate {
@@ -441,7 +441,7 @@ impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
 
 );
 
-#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
+#[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 pub(crate) enum ChannelMonitorUpdateStep {
 	LatestHolderCommitmentTXInfo {
@@ -711,9 +711,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
 /// Transaction outputs to watch for on-chain spends.
 pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>);
 
-#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
-/// Used only in testing and fuzztarget to check serialization roundtrips don't change the
-/// underlying object
+#[cfg(any(test, fuzzing, feature = "_test_utils"))]
+/// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying
+/// object
 impl<Signer: Sign> PartialEq for ChannelMonitor<Signer> {
 	fn eq(&self, other: &Self) -> bool {
 		let inner = self.inner.lock().unwrap();
@@ -722,9 +722,9 @@ impl<Signer: Sign> PartialEq for ChannelMonitor<Signer> {
 	}
 }
 
-#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
-/// Used only in testing and fuzztarget to check serialization roundtrips don't change the
-/// underlying object
+#[cfg(any(test, fuzzing, feature = "_test_utils"))]
+/// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying
+/// object
 impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
 	fn eq(&self, other: &Self) -> bool {
 		if self.latest_update_id != other.latest_update_id ||

lightning/src/lib.rs

@@ -18,8 +18,8 @@
 //! generated/etc. This makes it a good candidate for tight integration into an existing wallet
 //! instead of having a rather-separate lightning appendage to a wallet.
 
-#![cfg_attr(not(any(test, feature = "fuzztarget", feature = "_test_utils")), deny(missing_docs))]
-#![cfg_attr(not(any(test, feature = "fuzztarget", feature = "_test_utils")), forbid(unsafe_code))]
+#![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), deny(missing_docs))]
+#![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), forbid(unsafe_code))]
 #![deny(broken_intra_doc_links)]
 
 // In general, rust is absolutely horrid at supporting users doing things like,
@@ -43,7 +43,7 @@ extern crate bitcoin;
 extern crate core;
 
 #[cfg(any(test, feature = "_test_utils"))] extern crate hex;
-#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))] extern crate regex;
+#[cfg(any(test, fuzzing, feature = "_test_utils"))] extern crate regex;
 
 #[cfg(not(feature = "std"))] extern crate core2;
 

lightning/src/ln/chan_utils.rs

@@ -1251,7 +1251,7 @@ impl CommitmentTransaction {
 				if let &Some(ref b_htlcout) = b {
 					a_htlcout.cltv_expiry.cmp(&b_htlcout.cltv_expiry)
 						// Note that due to hash collisions, we have to have a fallback comparison
-						// here for fuzztarget mode (otherwise at least chanmon_fail_consistency
+						// here for fuzzing mode (otherwise at least chanmon_fail_consistency
 						// may fail)!
 						.then(a_htlcout.payment_hash.0.cmp(&b_htlcout.payment_hash.0))
 				// For non-HTLC outputs, if they're copying our SPK we don't really care if we

lightning/src/ln/channel.rs

@@ -46,7 +46,7 @@ use io;
 use prelude::*;
 use core::{cmp,mem,fmt};
 use core::ops::Deref;
-#[cfg(any(test, feature = "fuzztarget", debug_assertions))]
+#[cfg(any(test, fuzzing, debug_assertions))]
 use sync::Mutex;
 use bitcoin::hashes::hex::ToHex;
 
@@ -670,9 +670,9 @@ pub(super) struct Channel<Signer: Sign> {
 	// `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will
 	// be, by comparing the cached values to the fee of the tranaction generated by
 	// `build_commitment_transaction`.
-	#[cfg(any(test, feature = "fuzztarget"))]
+	#[cfg(any(test, fuzzing))]
 	next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
-	#[cfg(any(test, feature = "fuzztarget"))]
+	#[cfg(any(test, fuzzing))]
 	next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
 
 	/// lnd has a long-standing bug where, upon reconnection, if the channel is not yet confirmed
@@ -684,7 +684,7 @@ pub(super) struct Channel<Signer: Sign> {
 	/// See-also <https://github.com/lightningnetwork/lnd/issues/4006>
 	pub workaround_lnd_bug_4006: Option<msgs::FundingLocked>,
 
-	#[cfg(any(test, feature = "fuzztarget"))]
+	#[cfg(any(test, fuzzing))]
 	// When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the
 	// corresponding HTLC on the inbound path. If, then, the outbound path channel is
 	// disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack
@@ -697,7 +697,7 @@ pub(super) struct Channel<Signer: Sign> {
 	channel_type: ChannelTypeFeatures,
 }
 
-#[cfg(any(test, feature = "fuzztarget"))]
+#[cfg(any(test, fuzzing))]
 struct CommitmentTxInfoCached {
 	fee: u64,
 	total_pending_htlcs: usize,
@@ -940,14 +940,14 @@ impl<Signer: Sign> Channel<Signer> {
 
 			announcement_sigs: None,
 
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
 			workaround_lnd_bug_4006: None,
 
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills: HashSet::new(),
 
 			// We currently only actually support one channel type, so don't retry with new types
@@ -1245,14 +1245,14 @@ impl<Signer: Sign> Channel<Signer> {
 
 			announcement_sigs: None,
 
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
 			workaround_lnd_bug_4006: None,
 
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills: HashSet::new(),
 
 			channel_type,
@@ -1650,7 +1650,7 @@ impl<Signer: Sign> Channel<Signer> {
 			}
 		}
 		if pending_idx == core::usize::MAX {
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			// If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and
 			// this is simply a duplicate claim, not previously failed and we lost funds.
 			debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
@@ -1676,7 +1676,7 @@ impl<Signer: Sign> Channel<Signer> {
 						if htlc_id_arg == htlc_id {
 							// Make sure we don't leave latest_monitor_update_id incremented here:
 							self.latest_monitor_update_id -= 1;
-							#[cfg(any(test, feature = "fuzztarget"))]
+							#[cfg(any(test, fuzzing))]
 							debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
 							return UpdateFulfillFetch::DuplicateClaim {};
 						}
@@ -1697,11 +1697,11 @@ impl<Signer: Sign> Channel<Signer> {
 			self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
 				payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
 			});
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
 			return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None };
 		}
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
 
 		{
@@ -1782,7 +1782,7 @@ impl<Signer: Sign> Channel<Signer> {
 			}
 		}
 		if pending_idx == core::usize::MAX {
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			// If we failed to find an HTLC to fail, make sure it was previously fulfilled and this
 			// is simply a duplicate fail, not previously failed and we failed-back too early.
 			debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
@@ -1795,7 +1795,7 @@ impl<Signer: Sign> Channel<Signer> {
 				match pending_update {
 					&HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 						if htlc_id_arg == htlc_id {
-							#[cfg(any(test, feature = "fuzztarget"))]
+							#[cfg(any(test, fuzzing))]
 							debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
 							return Ok(None);
 						}
@@ -2367,7 +2367,7 @@ impl<Signer: Sign> Channel<Signer> {
 
 		let num_htlcs = included_htlcs + addl_htlcs;
 		let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs, self.opt_anchors());
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		{
 			let mut fee = res;
 			if fee_spike_buffer_htlc.is_some() {
@@ -2445,7 +2445,7 @@ impl<Signer: Sign> Channel<Signer> {
 
 		let num_htlcs = included_htlcs + addl_htlcs;
 		let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs, self.opt_anchors());
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		{
 			let mut fee = res;
 			if fee_spike_buffer_htlc.is_some() {
@@ -2728,7 +2728,7 @@ impl<Signer: Sign> Channel<Signer> {
 				return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee".to_owned())));
 			}
 		}
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		{
 			if self.is_outbound() {
 				let projected_commit_tx_info = self.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
@@ -3035,7 +3035,7 @@ impl<Signer: Sign> Channel<Signer> {
 			return Err(ChannelError::Close("Received an unexpected revoke_and_ack".to_owned()));
 		}
 
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		{
 			*self.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
 			*self.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
@@ -4466,9 +4466,9 @@ impl<Signer: Sign> Channel<Signer> {
 								// If we generated the funding transaction and it doesn't match what it
 								// should, the client is really broken and we should just panic and
 								// tell them off. That said, because hash collisions happen with high
-								// probability in fuzztarget mode, if we're fuzzing we just close the
+								// probability in fuzzing mode, if we're fuzzing we just close the
 								// channel and move on.
-								#[cfg(not(feature = "fuzztarget"))]
+								#[cfg(not(fuzzing))]
 								panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
 							}
 							self.update_time_counter += 1;
@@ -4480,7 +4480,7 @@ impl<Signer: Sign> Channel<Signer> {
 									if input.witness.is_empty() {
 										// We generated a malleable funding transaction, implying we've
 										// just exposed ourselves to funds loss to our counterparty.
-										#[cfg(not(feature = "fuzztarget"))]
+										#[cfg(not(fuzzing))]
 										panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
 									}
 								}
@@ -4937,9 +4937,9 @@ impl<Signer: Sign> Channel<Signer> {
 		// Prior to static_remotekey, my_current_per_commitment_point was critical to claiming
 		// current to_remote balances. However, it no longer has any use, and thus is now simply
 		// set to a dummy (but valid, as required by the spec) public key.
-		// fuzztarget mode marks a subset of pubkeys as invalid so that we can hit "invalid pubkey"
+		// fuzzing mode marks a subset of pubkeys as invalid so that we can hit "invalid pubkey"
 		// branches, but we unwrap it below, so we arbitrarily select a dummy pubkey which is both
-		// valid, and valid in fuzztarget mode's arbitrary validity criteria:
+		// valid, and valid in fuzzing mode's arbitrary validity criteria:
 		let mut pk = [2; 33]; pk[1] = 0xff;
 		let dummy_pubkey = PublicKey::from_slice(&pk).unwrap();
 		let data_loss_protect = if self.cur_counterparty_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER {
@@ -5225,7 +5225,7 @@ impl<Signer: Sign> Channel<Signer> {
 		let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
 		let (signature, htlc_signatures);
 
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		{
 			if !self.is_outbound() {
 				let projected_commit_tx_info = self.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
@@ -5711,9 +5711,9 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		self.channel_update_status.write(writer)?;
 
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		(self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?;
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		for htlc in self.historical_inbound_htlc_fulfills.iter() {
 			htlc.write(writer)?;
 		}
@@ -5975,9 +5975,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 
 		let channel_update_status = Readable::read(reader)?;
 
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		let mut historical_inbound_htlc_fulfills = HashSet::new();
-		#[cfg(any(test, feature = "fuzztarget"))]
+		#[cfg(any(test, fuzzing))]
 		{
 			let htlc_fulfills_len: u64 = Readable::read(reader)?;
 			for _ in 0..htlc_fulfills_len {
@@ -6151,14 +6151,14 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 
 			announcement_sigs,
 
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
 			workaround_lnd_bug_4006: None,
 
-			#[cfg(any(test, feature = "fuzztarget"))]
+			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills,
 
 			channel_type: channel_type.unwrap(),