f docs, naming, major iterator bug

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -1518,7 +1518,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
 	/// Gets the set of outbound HTLCs which are pending resolution in this channel.
 	/// This is used to reconstruct pending outbound payments on restart in the ChannelManager.
-	pub(crate) fn get_pending_htlcs(&self) -> HashMap<HTLCSource, HTLCOutputInCommitment> {
+	pub(crate) fn get_pending_outbound_htlcs(&self) -> HashMap<HTLCSource, HTLCOutputInCommitment> {
 		let mut res = HashMap::new();
 		let us = self.inner.lock().unwrap();
 
@@ -1562,7 +1562,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
 		// We're only concerned with the confirmation count of HTLC transactions, and don't
 		// actually care how many confirmations a commitment transaction may or may not have. Thus,
-		// we look for both a FundingSpendConfirmation event, or a funding_spend_confirmed.
+		// we look for either a FundingSpendConfirmation event or a funding_spend_confirmed.
 		let confirmed_txid = us.funding_spend_confirmed.or_else(|| {
 			us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
 				if let OnchainEvent::FundingSpendConfirmation { .. } = event.event {
@@ -1572,25 +1572,28 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 		});
 		if let Some(txid) = confirmed_txid {
 			if Some(txid) == us.current_counterparty_commitment_txid || Some(txid) == us.prev_counterparty_commitment_txid {
-				walk_htlcs!(false, us.counterparty_claimable_outpoints.get(&txid).unwrap().iter().find_map(|(a, b)| {
+				walk_htlcs!(false, us.counterparty_claimable_outpoints.get(&txid).unwrap().iter().filter_map(|(a, b)| {
 					if let &Some(ref source) = b {
 						Some((a, &**source))
 					} else { None }
 				}));
 			} else if txid == us.current_holder_commitment_tx.txid {
-				walk_htlcs!(true, us.current_holder_commitment_tx.htlc_outputs.iter().find_map(|(a, _, c)| {
+				walk_htlcs!(true, us.current_holder_commitment_tx.htlc_outputs.iter().filter_map(|(a, _, c)| {
 					if let Some(source) = c { Some((a, source)) } else { None }
 				}));
 			} else if let Some(prev_commitment) = &us.prev_holder_signed_commitment_tx {
 				if txid == prev_commitment.txid {
-					walk_htlcs!(true, prev_commitment.htlc_outputs.iter().find_map(|(a, _, c)| {
+					walk_htlcs!(true, prev_commitment.htlc_outputs.iter().filter_map(|(a, _, c)| {
 						if let Some(source) = c { Some((a, source)) } else { None }
 					}));
 				}
 			}
 		} else {
-			macro_rules! check_htlc_fails {
-				($txid: expr, $commitment_tx: expr) => {
+			// If we have not seen a commitment transaction on-chain (ie the channel is not yet
+			// closed), just examine the available counterparty commitment transactions. See docs
+			// on `fail_unbroadcast_htlcs`, below, for justification.
+			macro_rules! walk_counterparty_commitment {
+				($txid: expr) => {
 					if let Some(ref latest_outpoints) = us.counterparty_claimable_outpoints.get($txid) {
 						for &(ref htlc, ref source_option) in latest_outpoints.iter() {
 							if let &Some(ref source) = source_option {
@@ -1601,10 +1604,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 				}
 			}
 			if let Some(ref txid) = us.current_counterparty_commitment_txid {
-				check_htlc_fails!(txid, "current");
+				walk_counterparty_commitment!(txid);
 			}
 			if let Some(ref txid) = us.prev_counterparty_commitment_txid {
-				check_htlc_fails!(txid, "previous");
+				walk_counterparty_commitment!(txid);
 			}
 		}
 

lightning/src/ln/channelmanager.rs

@@ -5909,7 +5909,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 			// 0.0.102+
 			for (_, monitor) in args.channel_monitors {
 				if by_id.get(&monitor.get_funding_txo().0.to_channel_id()).is_none() {
-					for (htlc_source, htlc) in monitor.get_pending_htlcs() {
+					for (htlc_source, htlc) in monitor.get_pending_outbound_htlcs() {
 						if let HTLCSource::OutboundRoute { payment_id, session_priv, path, payment_secret, .. } = htlc_source {
 							if path.is_empty() {
 								log_error!(args.logger, "Got an empty path for a pending payment");