Clarify when height is the *current* vs a *confirmation* height
authorMatt Corallo <git@bluematt.me>
Fri, 25 Jun 2021 20:27:38 +0000 (20:27 +0000)
committerMatt Corallo <git@bluematt.me>
Fri, 2 Jul 2021 17:16:12 +0000 (17:16 +0000)
lightning/src/chain/channelmonitor.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs

index c395f67c76a8cc77f9dbe1af93a7d18c461f158b..4aef7852ba138b19c97939d1fbf2bebd5679d1e3 100644 (file)
@@ -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 {
index 5f5bc51dbb14c3b205847994f238131d2e4c2fbb..403744c4f6ef38208d4e87f814d175e814155f5e 100644 (file)
@@ -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),
 }
index f72c76e7ac3eeb7b3944b3f99c42debeab013a90..766b2394b7ad38e3566f56b72906f4efe96800b1 100644 (file)
@@ -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;