Track outpoint creation height in PackageSolvingData

When we have an outpoint to claim which is lock-timed and the
locktime is reached, we add it to
`OnchainTxHandler::claimable_outpoints` to indicate the outpoint is
now being claimed. However, `claimable_outpoints` is supposed to
track when the outpoint first appeared on chain so that we can
remove the claim if the outpoint is reorged out.

Sadly, in the handling for lock-timed packages, we incorrectly
stored the current height in `claimable_outpoints`, causing such
claims to be removed in case of a reorg right after they were
generated, even if the output we intend to claim isn't removed at
all.

Here we start tracking when the outpoint we're spending was created
in `PackageSolvingData`'s constituent types. While we could have
tracked this information in `PackageTemplate`, it would preclude
later merging packages that are spending outpoints included in
different blocks, which we don't necessarily want to do.

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -3514,23 +3514,26 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				(payment_preimage.clone(), payment_info.clone().into_iter().collect())
 			});
 
-		let confirmed_spend_txid = self.funding_spend_confirmed.or_else(|| {
-			self.onchain_events_awaiting_threshold_conf.iter().find_map(|event| match event.event {
-				OnchainEvent::FundingSpendConfirmation { .. } => Some(event.txid),
-				_ => None,
-			})
-		});
-		let confirmed_spend_txid = if let Some(txid) = confirmed_spend_txid {
-			txid
-		} else {
-			return;
-		};
+		let confirmed_spend_info = self.funding_spend_confirmed
+			.map(|txid| (txid, None))
+			.or_else(|| {
+				self.onchain_events_awaiting_threshold_conf.iter().find_map(|event| match event.event {
+					OnchainEvent::FundingSpendConfirmation { .. } => Some((event.txid, Some(event.height))),
+					_ => None,
+				})
+			});
+		let (confirmed_spend_txid, confirmed_spend_height) =
+			if let Some((txid, height)) = confirmed_spend_info {
+				(txid, height)
+			} else {
+				return;
+			};
 
 		// If the channel is force closed, try to claim the output from this preimage.
 		// First check if a counterparty commitment transaction has been broadcasted:
 		macro_rules! claim_htlcs {
 			($commitment_number: expr, $txid: expr, $htlcs: expr) => {
-				let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None, $htlcs);
+				let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None, $htlcs, confirmed_spend_height);
 				let conf_target = self.closure_conf_target();
 				self.onchain_tx_handler.update_claims_view_from_requests(
 					htlc_claim_reqs, self.best_block.height, self.best_block.height, broadcaster,
@@ -4226,6 +4229,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						per_commitment_point, per_commitment_key, outp.value,
 						self.funding.channel_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx(),
 						self.funding.channel_parameters.clone(),
+						height,
 					);
 					let justice_package = PackageTemplate::build_package(
 						commitment_txid, idx as u32,
@@ -4250,6 +4254,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						let revk_htlc_outp = RevokedHTLCOutput::build(
 							per_commitment_point, per_commitment_key, htlc.clone(),
 							self.funding.channel_parameters.clone(),
+							height,
 						);
 						let counterparty_spendable_height = if htlc.offered {
 							htlc.cltv_expiry
@@ -4304,7 +4309,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
 				), logger);
 			let (htlc_claim_reqs, counterparty_output_info) =
-				self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option);
+				self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option, Some(height));
 			to_counterparty_output_info = counterparty_output_info;
 			for req in htlc_claim_reqs {
 				claimable_outpoints.push(req);
@@ -4316,8 +4321,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 	/// Returns the HTLC claim package templates and the counterparty output info
 	#[rustfmt::skip]
-	fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>)
-	-> (Vec<PackageTemplate>, CommitmentTxCounterpartyOutputInfo) {
+	fn get_counterparty_output_claim_info(
+		&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>,
+		per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>,
+		confirmation_height: Option<u32>,
+	) -> (Vec<PackageTemplate>, CommitmentTxCounterpartyOutputInfo) {
 		let mut claimable_outpoints = Vec::new();
 		let mut to_counterparty_output_info: CommitmentTxCounterpartyOutputInfo = None;
 
@@ -4374,15 +4382,19 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					let counterparty_htlc_outp = if htlc.offered {
 						PackageSolvingData::CounterpartyOfferedHTLCOutput(
 							CounterpartyOfferedHTLCOutput::build(
-								*per_commitment_point, preimage.unwrap(), htlc.clone(),
+								*per_commitment_point, preimage.unwrap(),
+								htlc.clone(),
 								self.funding.channel_parameters.clone(),
+								confirmation_height,
 							)
 						)
 					} else {
 						PackageSolvingData::CounterpartyReceivedHTLCOutput(
 							CounterpartyReceivedHTLCOutput::build(
-								*per_commitment_point, htlc.clone(),
+								*per_commitment_point,
+								htlc.clone(),
 								self.funding.channel_parameters.clone(),
+								confirmation_height,
 							)
 						)
 					};
@@ -4426,6 +4438,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				let revk_outp = RevokedOutput::build(
 					per_commitment_point, per_commitment_key, tx.output[idx].value, false,
 					self.funding.channel_parameters.clone(),
+					height,
 				);
 				let justice_package = PackageTemplate::build_package(
 					htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp),
@@ -4507,7 +4520,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					.expect("Expected transaction output index for non-dust HTLC");
 				PackageTemplate::build_package(
 					tx.txid(), transaction_output_index,
-					PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(htlc_descriptor)),
+					PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(htlc_descriptor, conf_height)),
 					counterparty_spendable_height,
 				)
 			})
@@ -4687,7 +4700,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				let txid = self.funding.current_holder_commitment_tx.trust().txid();
 				let vout = htlc_descriptor.htlc.transaction_output_index
 					.expect("Expected transaction output index for non-dust HTLC");
-				let htlc_output = HolderHTLCOutput::build(htlc_descriptor);
+				let htlc_output = HolderHTLCOutput::build(htlc_descriptor, 0);
 				if let Some(htlc_tx) = htlc_output.get_maybe_signed_htlc_tx(
 					&mut self.onchain_tx_handler, &::bitcoin::OutPoint { txid, vout },
 				) {

lightning/src/chain/onchaintx.rs

@@ -1379,19 +1379,21 @@ mod tests {
 				holder_commit_txid,
 				htlc.transaction_output_index.unwrap(),
 				PackageSolvingData::HolderHTLCOutput(HolderHTLCOutput::build(HTLCDescriptor {
-					channel_derivation_parameters: ChannelDerivationParameters {
-						value_satoshis: tx_handler.channel_value_satoshis,
-						keys_id: tx_handler.channel_keys_id,
-						transaction_parameters: tx_handler.channel_transaction_parameters.clone(),
+						channel_derivation_parameters: ChannelDerivationParameters {
+							value_satoshis: tx_handler.channel_value_satoshis,
+							keys_id: tx_handler.channel_keys_id,
+							transaction_parameters: tx_handler.channel_transaction_parameters.clone(),
+						},
+						commitment_txid: holder_commit_txid,
+						per_commitment_number: holder_commit.commitment_number(),
+						per_commitment_point: holder_commit.per_commitment_point(),
+						feerate_per_kw: holder_commit.feerate_per_kw(),
+						htlc: htlc.clone(),
+						preimage: None,
+						counterparty_sig: *counterparty_sig,
 					},
-					commitment_txid: holder_commit_txid,
-					per_commitment_number: holder_commit.commitment_number(),
-					per_commitment_point: holder_commit.per_commitment_point(),
-					feerate_per_kw: holder_commit.feerate_per_kw(),
-					htlc: htlc.clone(),
-					preimage: None,
-					counterparty_sig: *counterparty_sig,
-				})),
+					0
+				)),
 				0,
 			));
 		}