Use proper locktime for HTLC timeout/success transactions

This only applies for such transactions spending our own commitments.
The timelocks are fixed due to the counterparty's signature, so we
shouldn't be modifying it for such inputs. This wasn't an issue for
channels predating anchors since we'd always broadcast the reconstructed
pre-signed HTLC transaction. This changes with anchors though, as it'll
now be the responsibility of the anchors event handler to ensure the
correct transaction locktime is set. To make the experience a bit more
user-friendly, a later commit will expose the transaction locktime, so
we make sure we start using the correct one in this commit.

Author: wpaulino

lightning/src/chain/package.rs

@@ -470,7 +470,14 @@ impl PackageSolvingData {
 			PackageSolvingData::RevokedHTLCOutput(_) => output_conf_height + 1,
 			PackageSolvingData::CounterpartyOfferedHTLCOutput(_) => output_conf_height + 1,
 			PackageSolvingData::CounterpartyReceivedHTLCOutput(ref outp) => cmp::max(outp.htlc.cltv_expiry, output_conf_height + 1),
-			PackageSolvingData::HolderHTLCOutput(ref outp) => cmp::max(outp.cltv_expiry, output_conf_height + 1),
+			// HTLC timeout/success transactions rely on a fixed timelock due to the counterparty's
+			// signature.
+			PackageSolvingData::HolderHTLCOutput(ref outp) => {
+				if outp.preimage.is_some() {
+					debug_assert_eq!(outp.cltv_expiry, 0);
+				}
+				outp.cltv_expiry
+			},
 			PackageSolvingData::HolderFundingOutput(_) => output_conf_height + 1,
 		};
 		absolute_timelock
@@ -639,8 +646,24 @@ impl PackageTemplate {
 		amounts
 	}
 	pub(crate) fn package_timelock(&self) -> u32 {
-		self.inputs.iter().map(|(_, outp)| outp.absolute_tx_timelock(self.height_original))
-			.max().expect("There must always be at least one output to spend in a PackageTemplate")
+		let timelock = self.inputs.iter().map(|(_, outp)| outp.absolute_tx_timelock(self.height_original))
+			.max().expect("There must always be at least one output to spend in a PackageTemplate");
+		// If we ever try to aggregate a `HolderHTLCOutput` for which we have a preimage with
+		// another output type, we'll likely end up with an incorrect transaction locktime since
+		// the counterparty has included it in its HTLC signature. This should never happen unless
+		// we decide to aggregate outputs across different channel commitments.
+		#[cfg(debug_assertions)] {
+			if self.inputs.iter().any(|(_, outp)|
+				if let PackageSolvingData::HolderHTLCOutput(outp) = outp {
+					outp.preimage.is_some()
+				} else {
+					false
+				}
+			) {
+				debug_assert_eq!(timelock, 0);
+			};
+		}
+		timelock
 	}
 	pub(crate) fn package_weight(&self, destination_script: &Script) -> usize {
 		let mut inputs_weight = 0;