From: Matt Corallo <git@bluematt.me>
Date: Fri, 25 Jun 2021 20:27:38 +0000 (+0000)
Subject: Clarify when height is the *current* vs a *confirmation* height
X-Git-Tag: v0.0.99~7^2~1
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=190557035826da022f883b0e3861342563986598;p=rust-lightning

Clarify when height is the *current* vs a *confirmation* height
---

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index c395f67c7..4aef7852b 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -370,8 +370,8 @@ impl OnchainEventEntry {
 		conf_threshold
 	}
 
-	fn has_reached_confirmation_threshold(&self, height: u32) -> bool {
-		height >= self.confirmation_threshold()
+	fn has_reached_confirmation_threshold(&self, best_block: &BestBlock) -> bool {
+		best_block.height() >= self.confirmation_threshold()
 	}
 }
 
@@ -1856,7 +1856,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 				} else if htlc.0.cltv_expiry > self.best_block.height() + 1 {
 					// Don't broadcast HTLC-Timeout transactions immediately as they don't meet the
 					// current locktime requirements on-chain. We will broadcast them in
-					// `block_confirmed` when `would_broadcast_at_height` returns true.
+					// `block_confirmed` when `should_broadcast_holder_commitment_txn` returns true.
 					// Note that we add + 1 as transactions are broadcastable when they can be
 					// confirmed in the next block.
 					continue;
@@ -2035,7 +2035,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 	{
 		debug_assert!(self.best_block.height() >= conf_height);
 
-		let should_broadcast = self.would_broadcast_at_height(self.best_block.height(), logger);
+		let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
 		if should_broadcast {
 			let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone());
 			let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), false, self.best_block.height());
@@ -2056,7 +2056,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			self.onchain_events_awaiting_threshold_conf.drain(..).collect::<Vec<_>>();
 		let mut onchain_events_reaching_threshold_conf = Vec::new();
 		for entry in onchain_events_awaiting_threshold_conf {
-			if entry.has_reached_confirmation_threshold(self.best_block.height()) {
+			if entry.has_reached_confirmation_threshold(&self.best_block) {
 				onchain_events_reaching_threshold_conf.push(entry);
 			} else {
 				self.onchain_events_awaiting_threshold_conf.push(entry);
@@ -2213,7 +2213,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		false
 	}
 
-	fn would_broadcast_at_height<L: Deref>(&self, height: u32, logger: &L) -> bool where L::Target: Logger {
+	fn should_broadcast_holder_commitment_txn<L: Deref>(&self, logger: &L) -> bool where L::Target: Logger {
 		// We need to consider all HTLCs which are:
 		//  * in any unrevoked counterparty commitment transaction, as they could broadcast said
 		//    transactions and we'd end up in a race, or
@@ -2224,6 +2224,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		// to the source, and if we don't fail the channel we will have to ensure that the next
 		// updates that peer sends us are update_fails, failing the channel if not. It's probably
 		// easier to just fail the channel as this case should be rare enough anyway.
+		let height = self.best_block.height();
 		macro_rules! scan_commitment {
 			($htlcs: expr, $holder_tx: expr) => {
 				for ref htlc in $htlcs {
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 5f5bc51db..403744c4f 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -114,8 +114,8 @@ enum InboundHTLCState {
 	/// commitment transaction without it as otherwise we'll have to force-close the channel to
 	/// claim it before the timeout (obviously doesn't apply to revoked HTLCs that we can't claim
 	/// anyway). That said, ChannelMonitor does this for us (see
-	/// ChannelMonitor::would_broadcast_at_height) so we actually remove the HTLC from our own
-	/// local state before then, once we're sure that the next commitment_signed and
+	/// ChannelMonitor::should_broadcast_holder_commitment_txn) so we actually remove the HTLC from
+	/// our own local state before then, once we're sure that the next commitment_signed and
 	/// ChannelMonitor::provide_latest_local_commitment_tx will not include this HTLC.
 	LocalRemoved(InboundHTLCRemovalReason),
 }
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index f72c76e7a..766b2394b 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -626,7 +626,7 @@ pub const MIN_FINAL_CLTV_EXPIRY: u32 = HTLC_FAIL_BACK_BUFFER + 3;
 const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - CLTV_CLAIM_BUFFER - ANTI_REORG_DELAY - LATENCY_GRACE_PERIOD_BLOCKS;
 
 // Check for ability of an attacker to make us fail on-chain by delaying an HTLC claim. See
-// ChannelMontior::would_broadcast_at_height for a description of why this is needed.
+// ChannelMonitor::should_broadcast_holder_commitment_txn for a description of why this is needed.
 #[deny(const_err)]
 #[allow(dead_code)]
 const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;