Lean on the holding cell for commitments when updating fees
[rust-lightning] / lightning / src / ln / channel.rs
index e8bae2e917de3eb506088aafcb52b1a386490878..b9016538212fede1de44439c57dfeeee95655eec 100644 (file)
@@ -3251,7 +3251,7 @@ impl<Signer: Sign> Channel<Signer> {
                                return Ok((None, htlcs_to_fail));
                        }
                        let update_fee = if let Some(feerate) = self.holding_cell_update_fee.take() {
-                               self.send_update_fee(feerate, logger)
+                               self.send_update_fee(feerate, false, logger)
                        } else {
                                None
                        };
@@ -3551,12 +3551,22 @@ impl<Signer: Sign> Channel<Signer> {
                }
        }
 
+       /// Queues up an outbound update fee by placing it in the holding cell. You should call
+       /// [`Self::maybe_free_holding_cell_htlcs`] in order to actually generate and send the
+       /// commitment update.
+       pub fn queue_update_fee<L: Deref>(&mut self, feerate_per_kw: u32, logger: &L) where L::Target: Logger {
+               let msg_opt = self.send_update_fee(feerate_per_kw, true, logger);
+               assert!(msg_opt.is_none(), "We forced holding cell?");
+       }
+
        /// Adds a pending update to this channel. See the doc for send_htlc for
        /// further details on the optionness of the return value.
        /// If our balance is too low to cover the cost of the next commitment transaction at the
        /// new feerate, the update is cancelled.
-       /// You MUST call send_commitment prior to any other calls on this Channel
-       fn send_update_fee<L: Deref>(&mut self, feerate_per_kw: u32, logger: &L) -> Option<msgs::UpdateFee> where L::Target: Logger {
+       ///
+       /// You MUST call [`Self::send_commitment_no_state_update`] prior to any other calls on this
+       /// [`Channel`] if `force_holding_cell` is false.
+       fn send_update_fee<L: Deref>(&mut self, feerate_per_kw: u32, mut force_holding_cell: bool, logger: &L) -> Option<msgs::UpdateFee> where L::Target: Logger {
                if !self.is_outbound() {
                        panic!("Cannot send fee from inbound channel");
                }
@@ -3593,6 +3603,10 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 {
+                       force_holding_cell = true;
+               }
+
+               if force_holding_cell {
                        self.holding_cell_update_fee = Some(feerate_per_kw);
                        return None;
                }
@@ -3606,16 +3620,6 @@ impl<Signer: Sign> Channel<Signer> {
                })
        }
 
-       pub fn send_update_fee_and_commit<L: Deref>(&mut self, feerate_per_kw: u32, logger: &L) -> Result<Option<(msgs::UpdateFee, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
-               match self.send_update_fee(feerate_per_kw, logger) {
-                       Some(update_fee) => {
-                               let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?;
-                               Ok(Some((update_fee, commitment_signed, monitor_update)))
-                       },
-                       None => Ok(None)
-               }
-       }
-
        /// Removes any uncommitted inbound HTLCs and resets the state of uncommitted outbound HTLC
        /// updates, to be used on peer disconnection. After this, update_*_htlc messages need to be
        /// resent.
@@ -5502,7 +5506,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        /// Adds a pending outbound HTLC to this channel, note that you probably want
-       /// send_htlc_and_commit instead cause you'll want both messages at once.
+       /// [`Self::send_htlc_and_commit`] instead cause you'll want both messages at once.
        ///
        /// This returns an optional UpdateAddHTLC as we may be in a state where we cannot add HTLCs on
        /// the wire:
@@ -5513,10 +5517,10 @@ impl<Signer: Sign> Channel<Signer> {
        ///   we may not yet have sent the previous commitment update messages and will need to
        ///   regenerate them.
        ///
-       /// You MUST call send_commitment prior to calling any other methods on this Channel if
-       /// `force_holding_cell` is false.
+       /// You MUST call [`Self::send_commitment_no_state_update`] prior to calling any other methods
+       /// on this [`Channel`] if `force_holding_cell` is false.
        ///
-       /// If an Err is returned, it's a ChannelError::Ignore!
+       /// `Err`s will only be [`ChannelError::Ignore`].
        fn send_htlc<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
                onion_routing_packet: msgs::OnionPacket, mut force_holding_cell: bool, logger: &L)
        -> Result<Option<msgs::UpdateAddHTLC>, ChannelError> where L::Target: Logger {
@@ -5652,41 +5656,6 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(Some(res))
        }
 
-       /// Creates a signed commitment transaction to send to the remote peer.
-       /// Always returns a ChannelError::Close if an immediately-preceding (read: the
-       /// last call to this Channel) send_htlc returned Ok(Some(_)) and there is an Err.
-       /// May panic if called except immediately after a successful, Ok(Some(_))-returning send_htlc.
-       pub fn send_commitment<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
-               if (self.channel_state & (ChannelState::ChannelReady as u32)) != (ChannelState::ChannelReady as u32) {
-                       panic!("Cannot create commitment tx until channel is fully established");
-               }
-               if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
-                       panic!("Cannot create commitment tx until remote revokes their previous commitment");
-               }
-               if (self.channel_state & (ChannelState::PeerDisconnected as u32)) == (ChannelState::PeerDisconnected as u32) {
-                       panic!("Cannot create commitment tx while disconnected, as send_htlc will have returned an Err so a send_commitment precondition has been violated");
-               }
-               if (self.channel_state & (ChannelState::MonitorUpdateInProgress as u32)) == (ChannelState::MonitorUpdateInProgress as u32) {
-                       panic!("Cannot create commitment tx while awaiting monitor update unfreeze, as send_htlc will have returned an Err so a send_commitment precondition has been violated");
-               }
-               let mut have_updates = self.is_outbound() && self.pending_update_fee.is_some();
-               for htlc in self.pending_outbound_htlcs.iter() {
-                       if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
-                               have_updates = true;
-                       }
-                       if have_updates { break; }
-               }
-               for htlc in self.pending_inbound_htlcs.iter() {
-                       if let InboundHTLCState::LocalRemoved(_) = htlc.state {
-                               have_updates = true;
-                       }
-                       if have_updates { break; }
-               }
-               if !have_updates {
-                       panic!("Cannot create commitment tx until we have some updates to send");
-               }
-               self.send_commitment_no_status_check(logger)
-       }
        /// Only fails in case of bad keys
        fn send_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
                log_trace!(logger, "Updating HTLC state for a newly-sent commitment_signed...");
@@ -5809,8 +5778,9 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Adds a pending outbound HTLC to this channel, and creates a signed commitment transaction
        /// to send to the remote peer in one go.
-       /// Shorthand for calling send_htlc() followed by send_commitment(), see docs on those for
-       /// more info.
+       ///
+       /// Shorthand for calling [`Self::send_htlc`] followed by a commitment update, see docs on
+       /// [`Self::send_htlc`] and [`Self::send_commitment_no_state_update`] for more info.
        pub fn send_htlc_and_commit<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource, onion_routing_packet: msgs::OnionPacket, logger: &L) -> Result<Option<(msgs::UpdateAddHTLC, msgs::CommitmentSigned, ChannelMonitorUpdate)>, ChannelError> where L::Target: Logger {
                match self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, false, logger)? {
                        Some(update_add_htlc) => {