Remove one tab level when accessing a `peer_state`
[rust-lightning] / lightning / src / ln / channel.rs
index 1d1f31a4fabe2bcd6b8be5ca2e52f7586071b565..76dc15065c05cd298b532247de377b757ce5491f 100644 (file)
@@ -27,14 +27,15 @@ use crate::ln::features::{ChannelTypeFeatures, InitFeatures};
 use crate::ln::msgs;
 use crate::ln::msgs::{DecodeError, OptionalField, DataLossProtect};
 use crate::ln::script::{self, ShutdownScript};
-use crate::ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use crate::ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 use crate::ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction};
 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::transaction::{OutPoint, TransactionData};
-use crate::chain::keysinterface::{Sign, KeysInterface, BaseSign};
+use crate::chain::keysinterface::{Sign, EntropySource, KeysInterface, BaseSign, SignerProvider};
 use crate::util::events::ClosureReason;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use crate::util::logger::Logger;
@@ -739,7 +740,7 @@ pub(super) struct Channel<Signer: Sign> {
        channel_ready_event_emitted: bool,
 
        /// The unique identifier used to re-derive the private key material for the channel through
-       /// [`KeysInterface::derive_channel_signer`].
+       /// [`SignerProvider::derive_channel_signer`].
        channel_keys_id: [u8; 32],
 }
 
@@ -1757,27 +1758,27 @@ impl<Signer: Sign> Channel<Signer> {
        /// our counterparty!)
        /// The result is a transaction which we can revoke broadcastership of (ie a "local" transaction)
        /// TODO Some magic rust shit to compile-time check this?
-       fn build_holder_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
+       fn build_holder_transaction_keys(&self, commitment_number: u64) -> TxCreationKeys {
                let per_commitment_point = self.holder_signer.get_per_commitment_point(commitment_number, &self.secp_ctx);
                let delayed_payment_base = &self.get_holder_pubkeys().delayed_payment_basepoint;
                let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint;
                let counterparty_pubkeys = self.get_counterparty_pubkeys();
 
-               Ok(secp_check!(TxCreationKeys::derive_new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys".to_owned()))
+               TxCreationKeys::derive_new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint)
        }
 
        #[inline]
        /// Creates a set of keys for build_commitment_transaction to generate a transaction which we
        /// will sign and send to our counterparty.
        /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
-       fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
+       fn build_remote_transaction_keys(&self) -> TxCreationKeys {
                //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
                //may see payments to it!
                let revocation_basepoint = &self.get_holder_pubkeys().revocation_basepoint;
                let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint;
                let counterparty_pubkeys = self.get_counterparty_pubkeys();
 
-               Ok(secp_check!(TxCreationKeys::derive_new(&self.secp_ctx, &self.counterparty_cur_commitment_point.unwrap(), &counterparty_pubkeys.delayed_payment_basepoint, &counterparty_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys".to_owned()))
+               TxCreationKeys::derive_new(&self.secp_ctx, &self.counterparty_cur_commitment_point.unwrap(), &counterparty_pubkeys.delayed_payment_basepoint, &counterparty_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint)
        }
 
        /// Gets the redeemscript for the funding transaction output (ie the funding transaction output
@@ -1954,9 +1955,26 @@ impl<Signer: Sign> Channel<Signer> {
        /// an HTLC more than once or fulfill once and then attempt to fail after reconnect. We cannot,
        /// however, fail more than once as we wait for an upstream failure to be irrevocably committed
        /// before we fail backwards.
-       /// If we do fail twice, we debug_assert!(false) and return Ok(None). Thus, will always return
-       /// Ok(_) if debug assertions are turned on or preconditions are met.
-       pub fn get_update_fail_htlc<L: Deref>(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, logger: &L) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> where L::Target: Logger {
+       ///
+       /// If we do fail twice, we `debug_assert!(false)` and return `Ok(None)`. Thus, this will always
+       /// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be
+       /// [`ChannelError::Ignore`].
+       pub fn queue_fail_htlc<L: Deref>(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, logger: &L)
+       -> Result<(), ChannelError> where L::Target: Logger {
+               self.fail_htlc(htlc_id_arg, err_packet, true, logger)
+                       .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
+       }
+
+       /// We can only have one resolution per HTLC. In some cases around reconnect, we may fulfill
+       /// an HTLC more than once or fulfill once and then attempt to fail after reconnect. We cannot,
+       /// however, fail more than once as we wait for an upstream failure to be irrevocably committed
+       /// before we fail backwards.
+       ///
+       /// If we do fail twice, we `debug_assert!(false)` and return `Ok(None)`. Thus, this will always
+       /// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be
+       /// [`ChannelError::Ignore`].
+       fn fail_htlc<L: Deref>(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, mut force_holding_cell: bool, logger: &L)
+       -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> where L::Target: Logger {
                if (self.channel_state & (ChannelState::ChannelReady as u32)) != (ChannelState::ChannelReady as u32) {
                        panic!("Was asked to fail an HTLC when channel was not in an operational state");
                }
@@ -1994,8 +2012,13 @@ impl<Signer: Sign> Channel<Signer> {
                        return Ok(None);
                }
 
-               // Now update local state:
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 {
+                       debug_assert!(force_holding_cell, "!force_holding_cell is only called when emptying the holding cell, so we shouldn't end up back in it!");
+                       force_holding_cell = true;
+               }
+
+               // Now update local state:
+               if force_holding_cell {
                        for pending_update in self.holding_cell_htlc_updates.iter() {
                                match pending_update {
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
@@ -2165,7 +2188,7 @@ impl<Signer: Sign> Channel<Signer> {
        fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(Txid, CommitmentTransaction, Signature), ChannelError> where L::Target: Logger {
                let funding_script = self.get_funding_redeemscript();
 
-               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?;
+               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number);
                let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, logger).tx;
                {
                        let trusted_tx = initial_commitment_tx.trust();
@@ -2179,7 +2202,7 @@ impl<Signer: Sign> Channel<Signer> {
                        secp_check!(self.secp_ctx.verify_ecdsa(&sighash, &sig, self.counterparty_funding_pubkey()), "Invalid funding_created signature from peer".to_owned());
                }
 
-               let counterparty_keys = self.build_remote_transaction_keys()?;
+               let counterparty_keys = self.build_remote_transaction_keys();
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
 
                let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
@@ -2198,7 +2221,13 @@ impl<Signer: Sign> Channel<Signer> {
                &self.get_counterparty_pubkeys().funding_pubkey
        }
 
-       pub fn funding_created<L: Deref>(&mut self, msg: &msgs::FundingCreated, best_block: BestBlock, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor<Signer>, Option<msgs::ChannelReady>), ChannelError> where L::Target: Logger {
+       pub fn funding_created<K: Deref, L: Deref>(
+               &mut self, msg: &msgs::FundingCreated, best_block: BestBlock, keys_source: &K, logger: &L
+       ) -> Result<(msgs::FundingSigned, ChannelMonitor<<K::Target as SignerProvider>::Signer>, Option<msgs::ChannelReady>), ChannelError>
+       where
+               K::Target: KeysInterface,
+               L::Target: Logger
+       {
                if self.is_outbound() {
                        return Err(ChannelError::Close("Received funding_created for an outbound channel?".to_owned()));
                }
@@ -2253,7 +2282,9 @@ impl<Signer: Sign> Channel<Signer> {
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
                let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
                let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
-               let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
+               let mut monitor_signer = keys_source.derive_channel_signer(self.channel_value_satoshis, self.channel_keys_id);
+               monitor_signer.provide_channel_parameters(&self.channel_transaction_parameters);
+               let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), monitor_signer,
                                                          shutdown_script, self.get_holder_selected_contest_delay(),
                                                          &self.destination_script, (funding_txo, funding_txo_script.clone()),
                                                          &self.channel_transaction_parameters,
@@ -2278,7 +2309,13 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Handles a funding_signed message from the remote end.
        /// If this call is successful, broadcast the funding transaction (and not before!)
-       pub fn funding_signed<L: Deref>(&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, logger: &L) -> Result<(ChannelMonitor<Signer>, Transaction, Option<msgs::ChannelReady>), ChannelError> where L::Target: Logger {
+       pub fn funding_signed<K: Deref, L: Deref>(
+               &mut self, msg: &msgs::FundingSigned, best_block: BestBlock, keys_source: &K, logger: &L
+       ) -> Result<(ChannelMonitor<<K::Target as SignerProvider>::Signer>, Transaction, Option<msgs::ChannelReady>), ChannelError>
+       where
+               K::Target: KeysInterface,
+               L::Target: Logger
+       {
                if !self.is_outbound() {
                        return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
                }
@@ -2293,7 +2330,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                let funding_script = self.get_funding_redeemscript();
 
-               let counterparty_keys = self.build_remote_transaction_keys()?;
+               let counterparty_keys = self.build_remote_transaction_keys();
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
                let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
                let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
@@ -2301,7 +2338,7 @@ impl<Signer: Sign> Channel<Signer> {
                log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
                        log_bytes!(self.channel_id()), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
 
-               let holder_signer = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?;
+               let holder_signer = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number);
                let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
                {
                        let trusted_tx = initial_commitment_tx.trust();
@@ -2330,7 +2367,9 @@ impl<Signer: Sign> Channel<Signer> {
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
                let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
                let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
-               let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
+               let mut monitor_signer = keys_source.derive_channel_signer(self.channel_value_satoshis, self.channel_keys_id);
+               monitor_signer.provide_channel_parameters(&self.channel_transaction_parameters);
+               let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), monitor_signer,
                                                          shutdown_script, self.get_holder_selected_contest_delay(),
                                                          &self.destination_script, (funding_txo, funding_txo_script),
                                                          &self.channel_transaction_parameters,
@@ -2967,7 +3006,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                let funding_script = self.get_funding_redeemscript();
 
-               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number).map_err(|e| (None, e))?;
+               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number);
 
                let commitment_stats = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, logger);
                let commitment_txid = {
@@ -3154,8 +3193,8 @@ impl<Signer: Sign> Channel<Signer> {
                } else { Ok((None, Vec::new())) }
        }
 
-       /// Used to fulfill holding_cell_htlcs when we get a remote ack (or implicitly get it by them
-       /// fulfilling or failing the last pending HTLC)
+       /// Frees any pending commitment updates in the holding cell, generating the relevant messages
+       /// for our counterparty.
        fn free_holding_cell_htlcs<L: Deref>(&mut self, logger: &L) -> Result<(Option<(msgs::CommitmentUpdate, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>), ChannelError> where L::Target: Logger {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateInProgress as u32, 0);
                if self.holding_cell_htlc_updates.len() != 0 || self.holding_cell_update_fee.is_some() {
@@ -3181,7 +3220,7 @@ impl<Signer: Sign> Channel<Signer> {
                                // to rebalance channels.
                                match &htlc_update {
                                        &HTLCUpdateAwaitingACK::AddHTLC {amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, ..} => {
-                                               match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone(), logger) {
+                                               match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(), onion_routing_packet.clone(), false, logger) {
                                                        Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()),
                                                        Err(e) => {
                                                                match e {
@@ -3217,13 +3256,13 @@ impl<Signer: Sign> Channel<Signer> {
                                                monitor_update.updates.append(&mut additional_monitor_update.updates);
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
-                                               match self.get_update_fail_htlc(htlc_id, err_packet.clone(), logger) {
+                                               match self.fail_htlc(htlc_id, err_packet.clone(), false, logger) {
                                                        Ok(update_fail_msg_option) => {
                                                                // If an HTLC failure was previously added to the holding cell (via
-                                                               // `get_update_fail_htlc`) then generating the fail message itself
-                                                               // must not fail - we should never end up in a state where we
-                                                               // double-fail an HTLC or fail-then-claim an HTLC as it indicates
-                                                               // we didn't wait for a full revocation before failing.
+                                                               // `queue_fail_htlc`) then generating the fail message itself must
+                                                               // not fail - we should never end up in a state where we double-fail
+                                                               // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait
+                                                               // for a full revocation before failing.
                                                                update_fail_htlcs.push(update_fail_msg_option.unwrap())
                                                        },
                                                        Err(e) => {
@@ -3240,7 +3279,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
                        };
@@ -3540,12 +3579,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");
                }
@@ -3559,7 +3608,7 @@ impl<Signer: Sign> Channel<Signer> {
                // Before proposing a feerate update, check that we can actually afford the new fee.
                let inbound_stats = self.get_inbound_pending_htlc_stats(Some(feerate_per_kw));
                let outbound_stats = self.get_outbound_pending_htlc_stats(Some(feerate_per_kw));
-               let keys = if let Ok(keys) = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number) { keys } else { return None; };
+               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number);
                let commitment_stats = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, true, logger);
                let buffer_fee_msat = Channel::<Signer>::commit_tx_fee_sat(feerate_per_kw, commitment_stats.num_nondust_htlcs + outbound_stats.on_holder_tx_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.opt_anchors()) * 1000;
                let holder_balance_msat = commitment_stats.local_balance_msat - outbound_stats.holding_cell_msat;
@@ -3582,6 +3631,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;
                }
@@ -3595,16 +3648,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.
@@ -5229,7 +5272,7 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
        fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
-               let counterparty_keys = self.build_remote_transaction_keys()?;
+               let counterparty_keys = self.build_remote_transaction_keys();
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
                Ok(self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
@@ -5478,8 +5521,26 @@ impl<Signer: Sign> Channel<Signer> {
 
        // Send stuff to our remote peers:
 
+       /// Queues up an outbound HTLC to send 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.
+       ///
+       /// `Err`s will only be [`ChannelError::Ignore`].
+       pub fn queue_add_htlc<L: Deref>(&mut self, amount_msat: u64, payment_hash: PaymentHash, cltv_expiry: u32, source: HTLCSource,
+               onion_routing_packet: msgs::OnionPacket, logger: &L)
+       -> Result<(), ChannelError> where L::Target: Logger {
+               self
+                       .send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, true, logger)
+                       .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
+                       .map_err(|err| {
+                               if let ChannelError::Ignore(_) = err { /* fine */ }
+                               else { debug_assert!(false, "Queueing cannot trigger channel failure"); }
+                               err
+                       })
+       }
+
        /// 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:
@@ -5490,10 +5551,13 @@ 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!
+       /// 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!
-       pub fn send_htlc<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>, ChannelError> where L::Target: Logger {
+       /// `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 {
                if (self.channel_state & (ChannelState::ChannelReady as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelReady as u32) {
                        return Err(ChannelError::Ignore("Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned()));
                }
@@ -5530,7 +5594,7 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat)));
                }
 
-               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?;
+               let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number);
                let commitment_stats = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, true, logger);
                if !self.is_outbound() {
                        // Check that we won't violate the remote channel reserve by adding this HTLC.
@@ -5588,8 +5652,12 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat)));
                }
 
-               // Now update local state:
                if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0 {
+                       force_holding_cell = true;
+               }
+
+               // Now update local state:
+               if force_holding_cell {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
                                amount_msat,
                                payment_hash,
@@ -5622,41 +5690,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...");
@@ -5722,7 +5755,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// Only fails in case of bad keys. Used for channel_reestablish commitment_signed generation
        /// when we shouldn't change HTLC/channel state.
        fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
-               let counterparty_keys = self.build_remote_transaction_keys()?;
+               let counterparty_keys = self.build_remote_transaction_keys();
                let commitment_stats = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
                let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
                let (signature, htlc_signatures);
@@ -5779,10 +5812,11 @@ 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, logger)? {
+               match self.send_htlc(amount_msat, payment_hash, cltv_expiry, source, onion_routing_packet, false, logger)? {
                        Some(update_add_htlc) => {
                                let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?;
                                Ok(Some((update_add_htlc, commitment_signed, monitor_update)))
@@ -5930,15 +5964,16 @@ impl<Signer: Sign> Channel<Signer> {
                (monitor_update, dropped_outbound_htlcs)
        }
 
-       pub fn inflight_htlc_sources(&self) -> impl Iterator<Item=&HTLCSource> {
+       pub fn inflight_htlc_sources(&self) -> impl Iterator<Item=(&HTLCSource, &PaymentHash)> {
                self.holding_cell_htlc_updates.iter()
                        .flat_map(|htlc_update| {
                                match htlc_update {
-                                       HTLCUpdateAwaitingACK::AddHTLC { source, .. } => { Some(source) }
-                                       _ => None
+                                       HTLCUpdateAwaitingACK::AddHTLC { source, payment_hash, .. }
+                                               => Some((source, payment_hash)),
+                                       _ => None,
                                }
                        })
-                       .chain(self.pending_outbound_htlcs.iter().map(|htlc| &htlc.source))
+                       .chain(self.pending_outbound_htlcs.iter().map(|htlc| (&htlc.source, &htlc.payment_hash)))
        }
 }
 
@@ -6294,7 +6329,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 }
 
 const MAX_ALLOC_SIZE: usize = 64*1024;
-impl<'a, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<<K::Target as KeysInterface>::Signer>
+impl<'a, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<<K::Target as SignerProvider>::Signer>
                where K::Target: KeysInterface {
        fn read<R : io::Read>(reader: &mut R, args: (&'a K, u32)) -> Result<Self, DecodeError> {
                let (keys_source, serialized_height) = args;
@@ -6767,7 +6802,7 @@ mod tests {
        use crate::ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight};
        use crate::chain::BestBlock;
        use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget};
-       use crate::chain::keysinterface::{BaseSign, InMemorySigner, Recipient, KeyMaterial, KeysInterface};
+       use crate::chain::keysinterface::{BaseSign, InMemorySigner, Recipient, KeyMaterial, KeysInterface, EntropySource, NodeSigner, SignerProvider};
        use crate::chain::transaction::OutPoint;
        use crate::util::config::UserConfig;
        use crate::util::enforcing_trait_impls::EnforcingSigner;
@@ -6816,12 +6851,39 @@ mod tests {
        struct Keys {
                signer: InMemorySigner,
        }
-       impl KeysInterface for Keys {
-               type Signer = InMemorySigner;
 
+       impl EntropySource for Keys {
+               fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
+       }
+
+       impl NodeSigner for Keys {
                fn get_node_secret(&self, _recipient: Recipient) -> Result<SecretKey, ()> { panic!(); }
+
+               fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
+                       let secp_ctx = Secp256k1::signing_only();
+                       Ok(PublicKey::from_secret_key(&secp_ctx, &self.get_node_secret(recipient)?))
+               }
+
                fn ecdh(&self, _recipient: Recipient, _other_key: &PublicKey, _tweak: Option<&Scalar>) -> Result<SharedSecret, ()> { panic!(); }
+
                fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); }
+
+               fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient) -> Result<RecoverableSignature, ()> { panic!(); }
+       }
+
+       impl SignerProvider for Keys {
+               type Signer = InMemorySigner;
+
+               fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] {
+                       self.signer.channel_keys_id()
+               }
+
+               fn derive_channel_signer(&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32]) -> Self::Signer {
+                       self.signer.clone()
+               }
+
+               fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }
+
                fn get_destination_script(&self) -> Script {
                        let secp_ctx = Secp256k1::signing_only();
                        let channel_monitor_claim_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
@@ -6834,18 +6896,10 @@ mod tests {
                        let channel_close_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
                        ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key))
                }
-
-               fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] {
-                       self.signer.channel_keys_id()
-               }
-               fn derive_channel_signer(&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32]) -> Self::Signer {
-                       self.signer.clone()
-               }
-               fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
-               fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }
-               fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient) -> Result<RecoverableSignature, ()> { panic!(); }
        }
 
+       impl KeysInterface for Keys {}
+
        #[cfg(not(feature = "grind_signatures"))]
        fn public_from_secret_hex(secp_ctx: &Secp256k1<bitcoin::secp256k1::All>, hex: &str) -> PublicKey {
                PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode(hex).unwrap()[..]).unwrap())
@@ -7053,10 +7107,10 @@ mod tests {
                }]};
                let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
                let funding_created_msg = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap();
-               let (funding_signed_msg, _, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&logger).unwrap();
+               let (funding_signed_msg, _, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).unwrap();
 
                // Node B --> Node A: funding signed
-               let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&logger);
+               let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger);
 
                // Now disconnect the two nodes and check that the commitment point in
                // Node B's channel_reestablish message is sane.
@@ -7344,7 +7398,7 @@ mod tests {
                let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
                let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
                let htlc_basepoint = &chan.holder_signer.pubkeys().htlc_basepoint;
-               let keys = TxCreationKeys::derive_new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint).unwrap();
+               let keys = TxCreationKeys::derive_new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint);
 
                macro_rules! test_commitment {
                        ( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $($remain:tt)* ) => {
@@ -7997,16 +8051,16 @@ mod tests {
                let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
                assert_eq!(per_commitment_point.serialize()[..], hex::decode("025f7117a78150fe2ef97db7cfc83bd57b2e2c0d0dd25eaf467a4a1c2a45ce1486").unwrap()[..]);
 
-               assert_eq!(chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &base_point).unwrap().serialize()[..],
+               assert_eq!(chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &base_point).serialize()[..],
                                hex::decode("0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5").unwrap()[..]);
 
-               assert_eq!(chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &base_secret).unwrap(),
+               assert_eq!(chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &base_secret),
                                SecretKey::from_slice(&hex::decode("cbced912d3b21bf196a766651e436aff192362621ce317704ea2f75d87e7be0f").unwrap()[..]).unwrap());
 
-               assert_eq!(chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &base_point).unwrap().serialize()[..],
+               assert_eq!(chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &base_point).serialize()[..],
                                hex::decode("02916e326636d19c33f13e8c0c3a03dd157f332f3e99c317c141dd865eb01f8ff0").unwrap()[..]);
 
-               assert_eq!(chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_secret, &base_secret).unwrap(),
+               assert_eq!(chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_secret, &base_secret),
                                SecretKey::from_slice(&hex::decode("d09ffff62ddb2297ab000cc85bcb4283fdeb6aa052affbc9dddcf33b61078110").unwrap()[..]).unwrap());
        }