X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=b5f6bce228a3886ee17d337ddcac2f103ba32cd7;hb=6bfab9d30a4ddab27f785643f2d974ffdc8371c0;hp=23696e7b8dbdb9cf89d0671e47f8ed854121b1f3;hpb=925e64228ff0a271b9dc49faa26cef4b27c55e95;p=rust-lightning

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 23696e7b..b5f6bce2 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -510,6 +510,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
 	on_holder_tx_csv: u16,
 
 	commitment_secrets: CounterpartyCommitmentSecrets,
+	/// The set of outpoints in each counterparty commitment transaction. We always need at least
+	/// the payment hash from `HTLCOutputInCommitment` to claim even a revoked commitment
+	/// transaction broadcast as we need to be able to construct the witness script in all cases.
 	counterparty_claimable_outpoints: HashMap<Txid, Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>,
 	/// We cannot identify HTLC-Success or HTLC-Timeout transactions by themselves on the chain.
 	/// Nor can we figure out their commitment numbers without the commitment transaction they are
@@ -1204,6 +1207,18 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 /// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
 /// failing any HTLCs which didn't make it into the broadcasted commitment transaction back
 /// after ANTI_REORG_DELAY blocks.
+///
+/// We always compare against the set of HTLCs in counterparty commitment transactions, as those
+/// are the commitment transactions which are generated by us. The off-chain state machine in
+/// `Channel` will automatically resolve any HTLCs which were never included in a commitment
+/// transaction when it detects channel closure, but it is up to us to ensure any HTLCs which were
+/// included in a remote commitment transaction are failed back if they are not present in the
+/// broadcasted commitment transaction.
+///
+/// Specifically, the removal process for HTLCs in `Channel` is always based on the counterparty
+/// sending a `revoke_and_ack`, which causes us to clear `prev_counterparty_commitment_txid`. Thus,
+/// as long as we examine both the current counterparty commitment transaction and, if it hasn't
+/// been revoked yet, the previous one, we we will never "forget" to resolve an HTLC.
 macro_rules! fail_unbroadcast_htlcs {
 	($self: expr, $commitment_tx_type: expr, $commitment_tx_conf_height: expr, $confirmed_htlcs_list: expr, $logger: expr) => { {
 		macro_rules! check_htlc_fails {
@@ -1780,6 +1795,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height);
 			let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx);
 			append_onchain_update!(res, to_watch);
+			fail_unbroadcast_htlcs!(self, "latest holder", height, self.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
 		} else if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
 			if holder_tx.txid == commitment_txid {
 				is_holder_tx = true;
@@ -1787,45 +1803,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 				let res = self.get_broadcasted_holder_claims(holder_tx, height);
 				let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx);
 				append_onchain_update!(res, to_watch);
-			}
-		}
-
-		macro_rules! fail_dust_htlcs_after_threshold_conf {
-			($holder_tx: expr, $commitment_tx: expr) => {
-				for &(ref htlc, _, ref source) in &$holder_tx.htlc_outputs {
-					if htlc.transaction_output_index.is_none() {
-						if let &Some(ref source) = source {
-							self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
-								if entry.height != height { return true; }
-								match entry.event {
-									OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
-										update_source != source
-									},
-									_ => true,
-								}
-							});
-							let entry = OnchainEventEntry {
-								txid: commitment_txid,
-								height,
-								event: OnchainEvent::HTLCUpdate {
-									source: source.clone(), payment_hash: htlc.payment_hash,
-									onchain_value_satoshis: Some(htlc.amount_msat / 1000)
-								},
-							};
-							log_trace!(logger, "Failing HTLC with payment_hash {} from {} holder commitment tx due to broadcast of transaction, waiting confirmation (at height{})",
-								log_bytes!(htlc.payment_hash.0), $commitment_tx, entry.confirmation_threshold());
-							self.onchain_events_awaiting_threshold_conf.push(entry);
-						}
-					}
-				}
+				fail_unbroadcast_htlcs!(self, "previous holder", height, holder_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
 			}
 		}
 
 		if is_holder_tx {
-			fail_dust_htlcs_after_threshold_conf!(self.current_holder_commitment_tx, "latest");
-			if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
-				fail_dust_htlcs_after_threshold_conf!(holder_tx, "previous");
-			}
 		}
 
 		(claim_requests, (commitment_txid, watch_outputs))