Be less aggressive in outbound HTLC CLTV timeout checks
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 30e2793dcb9c2145648e32beadab177d45d24f8d..d7395c337fc96a6193f61bb5dd4271ced09179e5 100644 (file)
@@ -190,8 +190,8 @@ pub enum MonitorEvent {
        /// A monitor event containing an HTLCUpdate.
        HTLCEvent(HTLCUpdate),
 
-       /// A monitor event that the Channel's commitment transaction was broadcasted.
-       CommitmentTxBroadcasted(OutPoint),
+       /// A monitor event that the Channel's commitment transaction was confirmed.
+       CommitmentTxConfirmed(OutPoint),
 }
 
 /// Simple structure sent back by `chain::Watch` when an HTLC from a forward channel is detected on
@@ -232,8 +232,13 @@ pub(crate) const CLTV_CLAIM_BUFFER: u32 = 18;
 /// with at worst this delay, so we are not only using this value as a mercy for them but also
 /// us as a safeguard to delay with enough time.
 pub(crate) const LATENCY_GRACE_PERIOD_BLOCKS: u32 = 3;
-/// Number of blocks we wait on seeing a HTLC output being solved before we fail corresponding inbound
-/// HTLCs. This prevents us from failing backwards and then getting a reorg resulting in us losing money.
+/// Number of blocks we wait on seeing a HTLC output being solved before we fail corresponding
+/// inbound HTLCs. This prevents us from failing backwards and then getting a reorg resulting in us
+/// losing money.
+///
+/// Note that this is a library-wide security assumption. If a reorg deeper than this number of
+/// blocks occurs, counterparties may be able to steal funds or claims made by and balances exposed
+/// by a  [`ChannelMonitor`] may be incorrect.
 // We also use this delay to be sure we can remove our in-flight claim txn from bump candidates buffer.
 // It may cause spurious generation of bumped claim txn but that's alright given the outpoint is already
 // solved by a previous claim tx. What we want to avoid is reorg evicting our claim tx and us not
@@ -248,8 +253,6 @@ pub const ANTI_REORG_DELAY: u32 = 6;
 ///    fail this HTLC,
 /// 2) if we receive an HTLC within this many blocks of its expiry (plus one to avoid a race
 ///    condition with the above), we will fail this HTLC without telling the user we received it,
-/// 3) if we are waiting on a connection or a channel state update to send an HTLC to a peer, and
-///    that HTLC expires within this many blocks, we will simply fail the HTLC instead.
 ///
 /// (1) is all about protecting us - we need enough time to update the channel state before we hit
 /// CLTV_CLAIM_BUFFER, at which point we'd go on chain to claim the HTLC with the preimage.
@@ -257,9 +260,6 @@ pub const ANTI_REORG_DELAY: u32 = 6;
 /// (2) is the same, but with an additional buffer to avoid accepting an HTLC which is immediately
 /// in a race condition between the user connecting a block (which would fail it) and the user
 /// providing us the preimage (which would claim it).
-///
-/// (3) is about our counterparty - we don't want to relay an HTLC to a counterparty when they may
-/// end up force-closing the channel on us to claim it.
 pub(crate) const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
 
 // TODO(devrandom) replace this with HolderCommitmentTransaction
@@ -913,7 +913,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                                        0u8.write(writer)?;
                                        upd.write(writer)?;
                                },
-                               MonitorEvent::CommitmentTxBroadcasted(_) => 1u8.write(writer)?
+                               MonitorEvent::CommitmentTxConfirmed(_) => 1u8.write(writer)?
                        }
                }
 
@@ -1782,7 +1782,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        log_info!(logger, "Broadcasting local {}", log_tx!(tx));
                        broadcaster.broadcast_transaction(tx);
                }
-               self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
+               self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
        }
 
        pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError>
@@ -1830,7 +1830,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        } else if !self.holder_tx_signed {
                                                log_error!(logger, "You have a toxic holder commitment transaction avaible in channel monitor, read comment in ChannelMonitor::get_latest_holder_commitment_txn to be informed of manual action to take");
                                        } else {
-                                               // If we generated a MonitorEvent::CommitmentTxBroadcasted, the ChannelManager
+                                               // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
                                                // will still give us a ChannelForceClosed event with !should_broadcast, but we
                                                // shouldn't print the scary warning above.
                                                log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction.");
@@ -2352,7 +2352,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        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());
                        claimable_outpoints.push(commitment_package);
-                       self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
+                       self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
                        let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
                        self.holder_tx_signed = true;
                        // Because we're broadcasting a commitment transaction, we should construct the package
@@ -3105,7 +3105,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                for _ in 0..pending_monitor_events_len {
                        let ev = match <u8 as Readable>::read(reader)? {
                                0 => MonitorEvent::HTLCEvent(Readable::read(reader)?),
-                               1 => MonitorEvent::CommitmentTxBroadcasted(funding_info.0),
+                               1 => MonitorEvent::CommitmentTxConfirmed(funding_info.0),
                                _ => return Err(DecodeError::InvalidValue)
                        };
                        pending_monitor_events.push(ev);