From: Wilmer Paulino Date: Tue, 28 Feb 2023 18:45:44 +0000 (-0800) Subject: Use CLOSED_CHANNEL_UPDATE_ID in force closing ChannelMonitorUpdates X-Git-Tag: v0.0.115~46^2~5 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=5a90f014f283f20e783c5db32ffb63c019a8dbe6;p=rust-lightning Use CLOSED_CHANNEL_UPDATE_ID in force closing ChannelMonitorUpdates Currently, all that is required to force close a channel is to broadcast either of the available commitment transactions, but this changes with anchor outputs – commitment transactions may need to have additional fees attached in order to confirm in a timely manner. While we may be able to just queue a new update using the channel's next available update ID, this may result in a violation of the `ChannelMonitor` API (each update ID must strictly increase by 1) if the channel had updates that were persisted by its `ChannelMonitor`, but not the `ChannelManager`. Therefore, we choose to re-purpose the existing `CLOSED_CHANNEL_UPDATE_ID` update ID to also apply to `ChannelMonitorUpdate`s that will force close their respective channel by broadcasting the holder's latest commitment transaction. --- diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs index 488378576..01cb4081c 100644 --- a/lightning-persister/src/lib.rs +++ b/lightning-persister/src/lib.rs @@ -141,6 +141,7 @@ mod tests { use bitcoin::{Txid, TxMerkleNode}; use lightning::chain::ChannelMonitorUpdateStatus; use lightning::chain::chainmonitor::Persist; + use lightning::chain::channelmonitor::CLOSED_CHANNEL_UPDATE_ID; use lightning::chain::transaction::OutPoint; use lightning::{check_closed_broadcast, check_closed_event, check_added_monitors}; use lightning::ln::functional_test_utils::*; @@ -253,7 +254,7 @@ mod tests { check_added_monitors!(nodes[1], 1); // Make sure everything is persisted as expected after close. - check_persisted_data!(11); + check_persisted_data!(CLOSED_CHANNEL_UPDATE_ID); } // Test that if the persister's path to channel data is read-only, writing a diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 1ebb65534..cd6b4c941 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -76,27 +76,30 @@ pub struct ChannelMonitorUpdate { pub(crate) updates: Vec, /// The sequence number of this update. Updates *must* be replayed in-order according to this /// sequence number (and updates may panic if they are not). The update_id values are strictly - /// increasing and increase by one for each new update, with one exception specified below. + /// increasing and increase by one for each new update, with two exceptions specified below. /// /// This sequence number is also used to track up to which points updates which returned /// [`ChannelMonitorUpdateStatus::InProgress`] have been applied to all copies of a given /// ChannelMonitor when ChannelManager::channel_monitor_updated is called. /// - /// The only instance where update_id values are not strictly increasing is the case where we - /// allow post-force-close updates with a special update ID of [`CLOSED_CHANNEL_UPDATE_ID`]. See - /// its docs for more details. + /// The only instances we allow where update_id values are not strictly increasing have a + /// special update ID of [`CLOSED_CHANNEL_UPDATE_ID`]. This update ID is used for updates that + /// will force close the channel by broadcasting the latest commitment transaction or + /// special post-force-close updates, like providing preimages necessary to claim outputs on the + /// broadcast commitment transaction. See its docs for more details. /// /// [`ChannelMonitorUpdateStatus::InProgress`]: super::ChannelMonitorUpdateStatus::InProgress pub update_id: u64, } -/// If: -/// (1) a channel has been force closed and -/// (2) we receive a preimage from a forward link that allows us to spend an HTLC output on -/// this channel's (the backward link's) broadcasted commitment transaction -/// then we allow the `ChannelManager` to send a `ChannelMonitorUpdate` with this update ID, -/// with the update providing said payment preimage. No other update types are allowed after -/// force-close. +/// The update ID used for a [`ChannelMonitorUpdate`] that is either: +/// +/// (1) attempting to force close the channel by broadcasting our latest commitment transaction or +/// (2) providing a preimage (after the channel has been force closed) from a forward link that +/// allows us to spend an HTLC output on this channel's (the backward link's) broadcasted +/// commitment transaction. +/// +/// No other [`ChannelMonitorUpdate`]s are allowed after force-close. pub const CLOSED_CHANNEL_UPDATE_ID: u64 = core::u64::MAX; impl Writeable for ChannelMonitorUpdate { @@ -2272,7 +2275,11 @@ impl ChannelMonitorImpl { if updates.update_id == CLOSED_CHANNEL_UPDATE_ID { assert_eq!(updates.updates.len(), 1); match updates.updates[0] { - ChannelMonitorUpdateStep::PaymentPreimage { .. } => {}, + ChannelMonitorUpdateStep::ChannelForceClosed { .. } => {}, + // We should have already seen a `ChannelForceClosed` update if we're trying to + // provide a preimage at this point. + ChannelMonitorUpdateStep::PaymentPreimage { .. } => + debug_assert_eq!(self.latest_update_id, CLOSED_CHANNEL_UPDATE_ID), _ => { log_error!(logger, "Attempted to apply post-force-close ChannelMonitorUpdate of type {}", updates.updates[0].variant_name()); panic!("Attempted to apply post-force-close ChannelMonitorUpdate that wasn't providing a payment preimage"); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7a389b573..ae45f5e8f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -33,7 +33,7 @@ use crate::ln::chan_utils; use crate::ln::onion_utils::HTLCFailReason; use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator}; -use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; +use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS, CLOSED_CHANNEL_UPDATE_ID}; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::keysinterface::{WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; use crate::routing::gossip::NodeId; @@ -6081,7 +6081,7 @@ impl Channel { // monitor update to the user, even if we return one). // See test_duplicate_chan_id and test_pre_lockin_no_chan_closed_update for more. if self.channel_state & (ChannelState::FundingSent as u32 | ChannelState::ChannelReady as u32 | ChannelState::ShutdownComplete as u32) != 0 { - self.latest_monitor_update_id += 1; + self.latest_monitor_update_id = CLOSED_CHANNEL_UPDATE_ID; Some((funding_txo, ChannelMonitorUpdate { update_id: self.latest_monitor_update_id, updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],