Provide non-dust HTLC sources separately for holder commitment updates

Currently, non-dust HTLCs are duplicated across the commitment
transaction itself, and the full set of HTLCs (dust & non-dust) along
with their `HTLCSource` considered in the commitment transaction. As of
v0.0.15, we've had support for providing non-dust HTLC sources
separately such that we no longer track duplicate non-dust HTLC data,
but only enabled it under testing environments. This commit enables it
such that it always happens.

Note that we still need to support reading
`ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo` updates that
did not separate the non-dust HTLC sources in case they were written in
an older version and they've yet to be processed.

Author: wpaulino

lightning/src/chain/channelmonitor.rs

@@ -531,6 +531,9 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		/// Note that LDK after 0.0.115 supports this only containing dust HTLCs (implying the
 		/// `Signature` field is never filled in). At that point, non-dust HTLCs are implied by the
 		/// HTLC fields in `commitment_tx` and the sources passed via `nondust_htlc_sources`.
+		/// Starting with 0.2, the non-dust HTLC sources will always be provided separately, and
+		/// `htlc_outputs` will only include dust HTLCs. We still have to track the
+		/// `Option<Signature>` for backwards compatibility.
 		htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
 		claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>,
 		nondust_htlc_sources: Vec<HTLCSource>,

lightning/src/ln/channel.rs

@@ -3531,23 +3531,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			return Err(ChannelError::close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), commitment_stats.num_nondust_htlcs)));
 		}
 
-		// Up to LDK 0.0.115, HTLC information was required to be duplicated in the
-		// `htlcs_and_sigs` vec and in the `holder_commitment_tx` itself, both of which were passed
-		// in the `ChannelMonitorUpdate`. In 0.0.115, support for having a separate set of
-		// outbound-non-dust-HTLCSources in the `ChannelMonitorUpdate` was added, however for
-		// backwards compatibility, we never use it in production. To provide test coverage, here,
-		// we randomly decide (in test/fuzzing builds) to use the new vec sometimes.
-		#[allow(unused_assignments, unused_mut)]
-		let mut separate_nondust_htlc_sources = false;
-		#[cfg(all(feature = "std", any(test, fuzzing)))] {
-			use core::hash::{BuildHasher, Hasher};
-			// Get a random value using the only std API to do so - the DefaultHasher
-			let rand_val = std::collections::hash_map::RandomState::new().build_hasher().finish();
-			separate_nondust_htlc_sources = rand_val % 2 == 0;
-		}
-
-		let mut nondust_htlc_sources = Vec::with_capacity(htlcs_cloned.len());
-		let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len());
+		let mut nondust_htlc_sources = Vec::with_capacity(commitment_stats.num_nondust_htlcs);
+		let mut dust_htlcs = Vec::with_capacity(htlcs_cloned.len() - commitment_stats.num_nondust_htlcs);
 		for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() {
 			if let Some(_) = htlc.transaction_output_index {
 				let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw,
@@ -3563,16 +3548,15 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 				if let Err(_) = self.secp_ctx.verify_ecdsa(&htlc_sighash, &msg.htlc_signatures[idx], &keys.countersignatory_htlc_key.to_public_key()) {
 					return Err(ChannelError::close("Invalid HTLC tx signature from peer".to_owned()));
 				}
-				if !separate_nondust_htlc_sources {
-					htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source_opt.take()));
+				if htlc.offered {
+					if let Some(source) = source_opt.take() {
+						nondust_htlc_sources.push(source);
+					} else {
+						debug_assert!(false, "Missing outbound HTLC source");
+					}
 				}
 			} else {
-				htlcs_and_sigs.push((htlc, None, source_opt.take()));
-			}
-			if separate_nondust_htlc_sources {
-				if let Some(source) = source_opt.take() {
-					nondust_htlc_sources.push(source);
-				}
+				dust_htlcs.push((htlc, None, source_opt.take()));
 			}
 			debug_assert!(source_opt.is_none(), "HTLCSource should have been put somewhere");
 		}
@@ -3590,7 +3574,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 
 		Ok(LatestHolderCommitmentTXInfo {
 			commitment_tx: holder_commitment_tx,
-			htlc_outputs: htlcs_and_sigs,
+			htlc_outputs: dust_htlcs,
 			nondust_htlc_sources,
 		})
 	}