From 190557035826da022f883b0e3861342563986598 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Jun 2021 20:27:38 +0000 Subject: [PATCH] Clarify when height is the *current* vs a *confirmation* height --- lightning/src/chain/channelmonitor.rs | 13 +++++++------ lightning/src/ln/channel.rs | 4 ++-- lightning/src/ln/channelmanager.rs | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c395f67c..4aef7852 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 ChannelMonitorImpl { } 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 ChannelMonitorImpl { { 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 ChannelMonitorImpl { self.onchain_events_awaiting_threshold_conf.drain(..).collect::>(); 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 ChannelMonitorImpl { false } - fn would_broadcast_at_height(&self, height: u32, logger: &L) -> bool where L::Target: Logger { + fn should_broadcast_holder_commitment_txn(&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 ChannelMonitorImpl { // 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 5f5bc51d..403744c4 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 f72c76e7..766b2394 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; -- 2.30.2