Merge pull request #2667 from wpaulino/random-htlc-holder-sigs-non-anchors
[rust-lightning] / lightning / src / events / bump_transaction.rs
index dbe1b734e5642339045d89b16b3f9d31b39ba56e..1fd533d2fd27f1e9637096d8de287139456b3b9d 100644 (file)
 use alloc::collections::BTreeMap;
 use core::ops::Deref;
 
-use crate::chain::chaininterface::{BroadcasterInterface, compute_feerate_sat_per_1000_weight, fee_for_weight, FEERATE_FLOOR_SATS_PER_KW};
+use crate::chain::chaininterface::{BroadcasterInterface, fee_for_weight};
 use crate::chain::ClaimId;
 use crate::io_extras::sink;
 use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI;
 use crate::ln::chan_utils;
 use crate::ln::chan_utils::{
        ANCHOR_INPUT_WITNESS_WEIGHT, HTLC_SUCCESS_INPUT_ANCHOR_WITNESS_WEIGHT,
-       HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, ChannelTransactionParameters, HTLCOutputInCommitment
+       HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT, HTLCOutputInCommitment
 };
-use crate::ln::features::ChannelTypeFeatures;
-use crate::ln::PaymentPreimage;
 use crate::prelude::*;
-use crate::sign::{ChannelSigner, EcdsaChannelSigner, SignerProvider};
+use crate::sign::{
+       ChannelDerivationParameters, HTLCDescriptor, EcdsaChannelSigner, SignerProvider,
+       WriteableEcdsaChannelSigner, P2WPKH_WITNESS_WEIGHT
+};
 use crate::sync::Mutex;
 use crate::util::logger::Logger;
 
-use bitcoin::{OutPoint, PackedLockTime, PubkeyHash, Sequence, Script, Transaction, Txid, TxIn, TxOut, Witness, WPubkeyHash};
+use bitcoin::{OutPoint, PackedLockTime, PubkeyHash, Sequence, Script, Transaction, TxIn, TxOut, Witness, WPubkeyHash};
 use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR;
 use bitcoin::consensus::Encodable;
 use bitcoin::secp256k1;
-use bitcoin::secp256k1::{PublicKey, Secp256k1};
+use bitcoin::secp256k1::Secp256k1;
 use bitcoin::secp256k1::ecdsa::Signature;
 
 const EMPTY_SCRIPT_SIG_WEIGHT: u64 = 1 /* empty script_sig */ * WITNESS_SCALE_FACTOR as u64;
@@ -43,20 +44,6 @@ const BASE_INPUT_SIZE: u64 = 32 /* txid */ + 4 /* vout */ + 4 /* sequence */;
 
 const BASE_INPUT_WEIGHT: u64 = BASE_INPUT_SIZE * WITNESS_SCALE_FACTOR as u64;
 
-/// The parameters required to derive a channel signer via [`SignerProvider`].
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct ChannelDerivationParameters {
-       /// The value in satoshis of the channel we're attempting to spend the anchor output of.
-       pub value_satoshis: u64,
-       /// The unique identifier to re-derive the signer for the associated channel.
-       pub keys_id: [u8; 32],
-       /// The necessary channel parameters that need to be provided to the re-derived signer through
-       /// [`ChannelSigner::provide_channel_parameters`].
-       ///
-       /// [`ChannelSigner::provide_channel_parameters`]: crate::sign::ChannelSigner::provide_channel_parameters
-       pub transaction_parameters: ChannelTransactionParameters,
-}
-
 /// A descriptor used to sign for a commitment transaction's anchor output.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct AnchorDescriptor {
@@ -102,109 +89,9 @@ impl AnchorDescriptor {
        }
 
        /// Derives the channel signer required to sign the anchor input.
-       pub fn derive_channel_signer<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::Signer
+       pub fn derive_channel_signer<S: WriteableEcdsaChannelSigner, SP: Deref>(&self, signer_provider: &SP) -> S
        where
-               SP::Target: SignerProvider
-       {
-               let mut signer = signer_provider.derive_channel_signer(
-                       self.channel_derivation_parameters.value_satoshis,
-                       self.channel_derivation_parameters.keys_id,
-               );
-               signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters);
-               signer
-       }
-}
-
-/// A descriptor used to sign for a commitment transaction's HTLC output.
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct HTLCDescriptor {
-       /// The parameters required to derive the signer for the HTLC input.
-       pub channel_derivation_parameters: ChannelDerivationParameters,
-       /// The txid of the commitment transaction in which the HTLC output lives.
-       pub commitment_txid: Txid,
-       /// The number of the commitment transaction in which the HTLC output lives.
-       pub per_commitment_number: u64,
-       /// The key tweak corresponding to the number of the commitment transaction in which the HTLC
-       /// output lives. This tweak is applied to all the basepoints for both parties in the channel to
-       /// arrive at unique keys per commitment.
-       ///
-       /// See <https://github.com/lightning/bolts/blob/master/03-transactions.md#keys> for more info.
-       pub per_commitment_point: PublicKey,
-       /// The details of the HTLC as it appears in the commitment transaction.
-       pub htlc: HTLCOutputInCommitment,
-       /// The preimage, if `Some`, to claim the HTLC output with. If `None`, the timeout path must be
-       /// taken.
-       pub preimage: Option<PaymentPreimage>,
-       /// The counterparty's signature required to spend the HTLC output.
-       pub counterparty_sig: Signature
-}
-
-impl HTLCDescriptor {
-       /// Returns the UTXO to be spent by the HTLC input, which can be obtained via
-       /// [`Self::unsigned_tx_input`].
-       pub fn previous_utxo<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> TxOut {
-               TxOut {
-                       script_pubkey: self.witness_script(secp).to_v0_p2wsh(),
-                       value: self.htlc.amount_msat / 1000,
-               }
-       }
-
-       /// Returns the unsigned transaction input spending the HTLC output in the commitment
-       /// transaction.
-       pub fn unsigned_tx_input(&self) -> TxIn {
-               chan_utils::build_htlc_input(&self.commitment_txid, &self.htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies())
-       }
-
-       /// Returns the delayed output created as a result of spending the HTLC output in the commitment
-       /// transaction.
-       pub fn tx_output<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> TxOut {
-               let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable();
-               let broadcaster_keys = channel_params.broadcaster_pubkeys();
-               let counterparty_keys = channel_params.countersignatory_pubkeys();
-               let broadcaster_delayed_key = chan_utils::derive_public_key(
-                       secp, &self.per_commitment_point, &broadcaster_keys.delayed_payment_basepoint
-               );
-               let counterparty_revocation_key = chan_utils::derive_public_revocation_key(
-                       secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint
-               );
-               chan_utils::build_htlc_output(
-                       0 /* feerate_per_kw */, channel_params.contest_delay(), &self.htlc,
-                       &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &broadcaster_delayed_key, &counterparty_revocation_key
-               )
-       }
-
-       /// Returns the witness script of the HTLC output in the commitment transaction.
-       pub fn witness_script<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> Script {
-               let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable();
-               let broadcaster_keys = channel_params.broadcaster_pubkeys();
-               let counterparty_keys = channel_params.countersignatory_pubkeys();
-               let broadcaster_htlc_key = chan_utils::derive_public_key(
-                       secp, &self.per_commitment_point, &broadcaster_keys.htlc_basepoint
-               );
-               let counterparty_htlc_key = chan_utils::derive_public_key(
-                       secp, &self.per_commitment_point, &counterparty_keys.htlc_basepoint
-               );
-               let counterparty_revocation_key = chan_utils::derive_public_revocation_key(
-                       secp, &self.per_commitment_point, &counterparty_keys.revocation_basepoint
-               );
-               chan_utils::get_htlc_redeemscript_with_explicit_keys(
-                       &self.htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &broadcaster_htlc_key, &counterparty_htlc_key,
-                       &counterparty_revocation_key,
-               )
-       }
-
-       /// Returns the fully signed witness required to spend the HTLC output in the commitment
-       /// transaction.
-       pub fn tx_input_witness(&self, signature: &Signature, witness_script: &Script) -> Witness {
-               chan_utils::build_htlc_input_witness(
-                       signature, &self.counterparty_sig, &self.preimage, witness_script, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() /* opt_anchors */
-               )
-       }
-
-       /// Derives the channel signer required to sign the HTLC input.
-       pub fn derive_channel_signer<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::Signer
-       where
-               SP::Target: SignerProvider
+               SP::Target: SignerProvider<Signer = S>
        {
                let mut signer = signer_provider.derive_channel_signer(
                        self.channel_derivation_parameters.value_satoshis,
@@ -310,7 +197,6 @@ pub enum BumpTransactionEvent {
        ///
        /// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
        /// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::EcdsaChannelSigner::sign_holder_htlc_transaction
-       /// [`HTLCDescriptor::tx_input_witness`]: HTLCDescriptor::tx_input_witness
        HTLCResolution {
                /// The unique identifier for the claim of the HTLCs in the confirmed commitment
                /// transaction.
@@ -332,6 +218,7 @@ pub enum BumpTransactionEvent {
 /// An input that must be included in a transaction when performing coin selection through
 /// [`CoinSelectionSource::select_confirmed_utxos`]. It is guaranteed to be a SegWit input, so it
 /// must have an empty [`TxIn::script_sig`] when spent.
+#[derive(Clone, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)]
 pub struct Input {
        /// The unique identifier of the input.
        pub outpoint: OutPoint,
@@ -345,7 +232,7 @@ pub struct Input {
 
 /// An unspent transaction output that is available to spend resulting from a successful
 /// [`CoinSelection`] attempt.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)]
 pub struct Utxo {
        /// The unique identifier of the output.
        pub outpoint: OutPoint,
@@ -358,12 +245,6 @@ pub struct Utxo {
 }
 
 impl Utxo {
-       const P2WPKH_WITNESS_WEIGHT: u64 = 1 /* num stack items */ +
-               1 /* sig length */ +
-               73 /* sig including sighash flag */ +
-               1 /* pubkey length */ +
-               33 /* pubkey */;
-
        /// Returns a `Utxo` with the `satisfaction_weight` estimate for a legacy P2PKH output.
        pub fn new_p2pkh(outpoint: OutPoint, value: u64, pubkey_hash: &PubkeyHash) -> Self {
                let script_sig_size = 1 /* script_sig length */ +
@@ -393,7 +274,7 @@ impl Utxo {
                                value,
                                script_pubkey: Script::new_p2sh(&Script::new_v0_p2wpkh(pubkey_hash).script_hash()),
                        },
-                       satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + Self::P2WPKH_WITNESS_WEIGHT,
+                       satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + P2WPKH_WITNESS_WEIGHT,
                }
        }
 
@@ -405,13 +286,14 @@ impl Utxo {
                                value,
                                script_pubkey: Script::new_v0_p2wpkh(pubkey_hash),
                        },
-                       satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + Self::P2WPKH_WITNESS_WEIGHT,
+                       satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + P2WPKH_WITNESS_WEIGHT,
                }
        }
 }
 
 /// The result of a successful coin selection attempt for a transaction requiring additional UTXOs
 /// to cover its fees.
+#[derive(Clone, Debug)]
 pub struct CoinSelection {
        /// The set of UTXOs (with at least 1 confirmation) to spend and use within a transaction
        /// requiring additional fees.
@@ -455,12 +337,12 @@ pub trait CoinSelectionSource {
        /// which UTXOs to double spend is left to the implementation, but it must strive to keep the
        /// set of other claims being double spent to a minimum.
        fn select_confirmed_utxos(
-               &self, claim_id: ClaimId, must_spend: &[Input], must_pay_to: &[TxOut],
+               &self, claim_id: ClaimId, must_spend: Vec<Input>, must_pay_to: &[TxOut],
                target_feerate_sat_per_1000_weight: u32,
        ) -> Result<CoinSelection, ()>;
        /// Signs and provides the full witness for all inputs within the transaction known to the
        /// trait (i.e., any provided via [`CoinSelectionSource::select_confirmed_utxos`]).
-       fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()>;
+       fn sign_tx(&self, tx: Transaction) -> Result<Transaction, ()>;
 }
 
 /// An alternative to [`CoinSelectionSource`] that can be implemented and used along [`Wallet`] to
@@ -474,25 +356,34 @@ pub trait WalletSource {
        /// Signs and provides the full [`TxIn::script_sig`] and [`TxIn::witness`] for all inputs within
        /// the transaction known to the wallet (i.e., any provided via
        /// [`WalletSource::list_confirmed_utxos`]).
-       fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()>;
+       fn sign_tx(&self, tx: Transaction) -> Result<Transaction, ()>;
 }
 
 /// A wrapper over [`WalletSource`] that implements [`CoinSelection`] by preferring UTXOs that would
 /// avoid conflicting double spends. If not enough UTXOs are available to do so, conflicting double
 /// spends may happen.
-pub struct Wallet<W: Deref> where W::Target: WalletSource {
+pub struct Wallet<W: Deref, L: Deref>
+where
+       W::Target: WalletSource,
+       L::Target: Logger
+{
        source: W,
+       logger: L,
        // TODO: Do we care about cleaning this up once the UTXOs have a confirmed spend? We can do so
        // by checking whether any UTXOs that exist in the map are no longer returned in
        // `list_confirmed_utxos`.
        locked_utxos: Mutex<HashMap<OutPoint, ClaimId>>,
 }
 
-impl<W: Deref> Wallet<W> where W::Target: WalletSource {
+impl<W: Deref, L: Deref> Wallet<W, L>
+where
+       W::Target: WalletSource,
+       L::Target: Logger
+{
        /// Returns a new instance backed by the given [`WalletSource`] that serves as an implementation
        /// of [`CoinSelectionSource`].
-       pub fn new(source: W) -> Self {
-               Self { source, locked_utxos: Mutex::new(HashMap::new()) }
+       pub fn new(source: W, logger: L) -> Self {
+               Self { source, logger, locked_utxos: Mutex::new(HashMap::new()) }
        }
 
        /// Performs coin selection on the set of UTXOs obtained from
@@ -506,12 +397,13 @@ impl<W: Deref> Wallet<W> where W::Target: WalletSource {
        fn select_confirmed_utxos_internal(
                &self, utxos: &[Utxo], claim_id: ClaimId, force_conflicting_utxo_spend: bool,
                tolerate_high_network_feerates: bool, target_feerate_sat_per_1000_weight: u32,
-               preexisting_tx_weight: u64, target_amount_sat: u64,
+               preexisting_tx_weight: u64, input_amount_sat: u64, target_amount_sat: u64,
        ) -> Result<CoinSelection, ()> {
                let mut locked_utxos = self.locked_utxos.lock().unwrap();
                let mut eligible_utxos = utxos.iter().filter_map(|utxo| {
                        if let Some(utxo_claim_id) = locked_utxos.get(&utxo.outpoint) {
                                if *utxo_claim_id != claim_id && !force_conflicting_utxo_spend {
+                                       log_trace!(self.logger, "Skipping UTXO {} to prevent conflicting spend", utxo.outpoint);
                                        return None;
                                }
                        }
@@ -526,12 +418,13 @@ impl<W: Deref> Wallet<W> where W::Target: WalletSource {
                        if should_spend {
                                Some((utxo, fee_to_spend_utxo))
                        } else {
+                               log_trace!(self.logger, "Skipping UTXO {} due to dust proximity after spend", utxo.outpoint);
                                None
                        }
                }).collect::<Vec<_>>();
                eligible_utxos.sort_unstable_by_key(|(utxo, _)| utxo.output.value);
 
-               let mut selected_amount = 0;
+               let mut selected_amount = input_amount_sat;
                let mut total_fees = fee_for_weight(target_feerate_sat_per_1000_weight, preexisting_tx_weight);
                let mut selected_utxos = Vec::new();
                for (utxo, fee_to_spend_utxo) in eligible_utxos {
@@ -543,6 +436,8 @@ impl<W: Deref> Wallet<W> where W::Target: WalletSource {
                        selected_utxos.push(utxo.clone());
                }
                if selected_amount < target_amount_sat + total_fees {
+                       log_debug!(self.logger, "Insufficient funds to meet target feerate {} sat/kW",
+                               target_feerate_sat_per_1000_weight);
                        return Err(());
                }
                for utxo in &selected_utxos {
@@ -559,6 +454,7 @@ impl<W: Deref> Wallet<W> where W::Target: WalletSource {
                );
                let change_output_amount = remaining_amount.saturating_sub(change_output_fee);
                let change_output = if change_output_amount < change_script.dust_value().to_sat() {
+                       log_debug!(self.logger, "Coin selection attempt did not yield change output");
                        None
                } else {
                        Some(TxOut { script_pubkey: change_script, value: change_output_amount })
@@ -571,9 +467,13 @@ impl<W: Deref> Wallet<W> where W::Target: WalletSource {
        }
 }
 
-impl<W: Deref> CoinSelectionSource for Wallet<W> where W::Target: WalletSource {
+impl<W: Deref, L: Deref> CoinSelectionSource for Wallet<W, L>
+where
+       W::Target: WalletSource,
+       L::Target: Logger
+{
        fn select_confirmed_utxos(
-               &self, claim_id: ClaimId, must_spend: &[Input], must_pay_to: &[TxOut],
+               &self, claim_id: ClaimId, must_spend: Vec<Input>, must_pay_to: &[TxOut],
                target_feerate_sat_per_1000_weight: u32,
        ) -> Result<CoinSelection, ()> {
                let utxos = self.source.list_confirmed_utxos()?;
@@ -587,11 +487,14 @@ impl<W: Deref> CoinSelectionSource for Wallet<W> where W::Target: WalletSource {
 
                let preexisting_tx_weight = 2 /* segwit marker & flag */ + total_input_weight +
                        ((BASE_TX_SIZE + total_output_size) * WITNESS_SCALE_FACTOR as u64);
+               let input_amount_sat: u64 = must_spend.iter().map(|input| input.previous_utxo.value).sum();
                let target_amount_sat = must_pay_to.iter().map(|output| output.value).sum();
                let do_coin_selection = |force_conflicting_utxo_spend: bool, tolerate_high_network_feerates: bool| {
+                       log_debug!(self.logger, "Attempting coin selection targeting {} sat/kW (force_conflicting_utxo_spend = {}, tolerate_high_network_feerates = {})",
+                               target_feerate_sat_per_1000_weight, force_conflicting_utxo_spend, tolerate_high_network_feerates);
                        self.select_confirmed_utxos_internal(
                                &utxos, claim_id, force_conflicting_utxo_spend, tolerate_high_network_feerates,
-                               target_feerate_sat_per_1000_weight, preexisting_tx_weight, target_amount_sat,
+                               target_feerate_sat_per_1000_weight, preexisting_tx_weight, input_amount_sat, target_amount_sat,
                        )
                };
                do_coin_selection(false, false)
@@ -600,7 +503,7 @@ impl<W: Deref> CoinSelectionSource for Wallet<W> where W::Target: WalletSource {
                        .or_else(|_| do_coin_selection(true, true))
        }
 
-       fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()> {
+       fn sign_tx(&self, tx: Transaction) -> Result<Transaction, ()> {
                self.source.sign_tx(tx)
        }
 }
@@ -661,6 +564,7 @@ where
                        // match, but we still need to have at least one output in the transaction for it to be
                        // considered standard. We choose to go with an empty OP_RETURN as it is the cheapest
                        // way to include a dummy output.
+                       log_debug!(self.logger, "Including dummy OP_RETURN output since an output is needed and a change output was not provided");
                        tx.output.push(TxOut {
                                value: 0,
                                script_pubkey: Script::new_op_return(&[]),
@@ -676,25 +580,22 @@ where
                commitment_tx: &Transaction, commitment_tx_fee_sat: u64, anchor_descriptor: &AnchorDescriptor,
        ) -> Result<(), ()> {
                // Our commitment transaction already has fees allocated to it, so we should take them into
-               // account. We compute its feerate and subtract it from the package target, using the result
-               // as the target feerate for our anchor transaction. Unfortunately, this results in users
-               // overpaying by a small margin since we don't yet know the anchor transaction size, and
-               // avoiding the small overpayment only makes our API even more complex.
-               let commitment_tx_sat_per_1000_weight: u32 = compute_feerate_sat_per_1000_weight(
-                       commitment_tx_fee_sat, commitment_tx.weight() as u64,
-               );
-               let anchor_target_feerate_sat_per_1000_weight = core::cmp::max(
-                       package_target_feerate_sat_per_1000_weight - commitment_tx_sat_per_1000_weight,
-                       FEERATE_FLOOR_SATS_PER_KW,
-               );
-
+               // account. We do so by pretending the commitment tranasction's fee and weight are part of
+               // the anchor input.
+               let mut anchor_utxo = anchor_descriptor.previous_utxo();
+               anchor_utxo.value += commitment_tx_fee_sat;
                let must_spend = vec![Input {
                        outpoint: anchor_descriptor.outpoint,
-                       previous_utxo: anchor_descriptor.previous_utxo(),
+                       previous_utxo: anchor_utxo,
                        satisfaction_weight: commitment_tx.weight() as u64 + ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT,
                }];
+               #[cfg(debug_assertions)]
+               let must_spend_amount = must_spend.iter().map(|input| input.previous_utxo.value).sum::<u64>();
+
+               log_debug!(self.logger, "Peforming coin selection for commitment package (commitment and anchor transaction) targeting {} sat/kW",
+                       package_target_feerate_sat_per_1000_weight);
                let coin_selection = self.utxo_source.select_confirmed_utxos(
-                       claim_id, &must_spend, &[], anchor_target_feerate_sat_per_1000_weight,
+                       claim_id, must_spend, &[], package_target_feerate_sat_per_1000_weight,
                )?;
 
                let mut anchor_tx = Transaction {
@@ -703,18 +604,24 @@ where
                        input: vec![anchor_descriptor.unsigned_tx_input()],
                        output: vec![],
                };
+
                #[cfg(debug_assertions)]
-               let total_satisfaction_weight =
-                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>() +
-                               ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT;
+               let total_satisfaction_weight = ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>();
+               #[cfg(debug_assertions)]
+               let total_input_amount = must_spend_amount +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum::<u64>();
 
                self.process_coin_selection(&mut anchor_tx, coin_selection);
+               let anchor_txid = anchor_tx.txid();
 
                debug_assert_eq!(anchor_tx.output.len(), 1);
                #[cfg(debug_assertions)]
                let unsigned_tx_weight = anchor_tx.weight() as u64 - (anchor_tx.input.len() as u64 * EMPTY_SCRIPT_SIG_WEIGHT);
 
-               self.utxo_source.sign_tx(&mut anchor_tx)?;
+               log_debug!(self.logger, "Signing anchor transaction {}", anchor_txid);
+               anchor_tx = self.utxo_source.sign_tx(anchor_tx)?;
+
                let signer = anchor_descriptor.derive_channel_signer(&self.signer_provider);
                let anchor_sig = signer.sign_holder_anchor_input(&anchor_tx, 0, &self.secp)?;
                anchor_tx.input[0].witness = anchor_descriptor.tx_input_witness(&anchor_sig);
@@ -722,10 +629,24 @@ where
                #[cfg(debug_assertions)] {
                        let signed_tx_weight = anchor_tx.weight() as u64;
                        let expected_signed_tx_weight = unsigned_tx_weight + total_satisfaction_weight;
-                       // Our estimate should be within a 1% error margin of the actual weight.
-                       assert!(expected_signed_tx_weight - (expected_signed_tx_weight / 100) <= signed_tx_weight);
+                       // Our estimate should be within a 1% error margin of the actual weight and we should
+                       // never underestimate.
+                       assert!(expected_signed_tx_weight >= signed_tx_weight &&
+                               expected_signed_tx_weight - (expected_signed_tx_weight / 100) <= signed_tx_weight);
+
+                       let expected_package_fee = fee_for_weight(package_target_feerate_sat_per_1000_weight,
+                               signed_tx_weight + commitment_tx.weight() as u64);
+                       let package_fee = total_input_amount -
+                               anchor_tx.output.iter().map(|output| output.value).sum::<u64>();
+                       // Our fee should be within a 5% error margin of the expected fee based on the
+                       // feerate and transaction weight and we should never pay less than required.
+                       let fee_error_margin = expected_package_fee * 5 / 100;
+                       assert!(package_fee >= expected_package_fee &&
+                               package_fee - fee_error_margin <= expected_package_fee);
                }
 
+               log_info!(self.logger, "Broadcasting anchor transaction {} to bump channel close with txid {}",
+                       anchor_txid, commitment_tx.txid());
                self.broadcaster.broadcast_transactions(&[&commitment_tx, &anchor_tx]);
                Ok(())
        }
@@ -759,19 +680,34 @@ where
                        htlc_tx.output.push(htlc_output);
                }
 
+               log_debug!(self.logger, "Peforming coin selection for HTLC transaction targeting {} sat/kW",
+                       target_feerate_sat_per_1000_weight);
+
+               #[cfg(debug_assertions)]
+               let must_spend_satisfaction_weight =
+                       must_spend.iter().map(|input| input.satisfaction_weight).sum::<u64>();
+               #[cfg(debug_assertions)]
+               let must_spend_amount = must_spend.iter().map(|input| input.previous_utxo.value).sum::<u64>();
+
                let coin_selection = self.utxo_source.select_confirmed_utxos(
-                       claim_id, &must_spend, &htlc_tx.output, target_feerate_sat_per_1000_weight,
+                       claim_id, must_spend, &htlc_tx.output, target_feerate_sat_per_1000_weight,
                )?;
+
+               #[cfg(debug_assertions)]
+               let total_satisfaction_weight = must_spend_satisfaction_weight +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>();
                #[cfg(debug_assertions)]
-               let total_satisfaction_weight =
-                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>() +
-                               must_spend.iter().map(|input| input.satisfaction_weight).sum::<u64>();
+               let total_input_amount = must_spend_amount +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum::<u64>();
+
                self.process_coin_selection(&mut htlc_tx, coin_selection);
 
                #[cfg(debug_assertions)]
                let unsigned_tx_weight = htlc_tx.weight() as u64 - (htlc_tx.input.len() as u64 * EMPTY_SCRIPT_SIG_WEIGHT);
 
-               self.utxo_source.sign_tx(&mut htlc_tx)?;
+               log_debug!(self.logger, "Signing HTLC transaction {}", htlc_tx.txid());
+               htlc_tx = self.utxo_source.sign_tx(htlc_tx)?;
+
                let mut signers = BTreeMap::new();
                for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() {
                        let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id)
@@ -784,10 +720,22 @@ where
                #[cfg(debug_assertions)] {
                        let signed_tx_weight = htlc_tx.weight() as u64;
                        let expected_signed_tx_weight = unsigned_tx_weight + total_satisfaction_weight;
-                       // Our estimate should be within a 1% error margin of the actual weight.
-                       assert!(expected_signed_tx_weight - (expected_signed_tx_weight / 100) <= signed_tx_weight);
+                       // Our estimate should be within a 1% error margin of the actual weight and we should
+                       // never underestimate.
+                       assert!(expected_signed_tx_weight >= signed_tx_weight &&
+                               expected_signed_tx_weight - (expected_signed_tx_weight / 100) <= signed_tx_weight);
+
+                       let expected_signed_tx_fee = fee_for_weight(target_feerate_sat_per_1000_weight, signed_tx_weight);
+                       let signed_tx_fee = total_input_amount -
+                               htlc_tx.output.iter().map(|output| output.value).sum::<u64>();
+                       // Our fee should be within a 5% error margin of the expected fee based on the
+                       // feerate and transaction weight and we should never pay less than required.
+                       let fee_error_margin = expected_signed_tx_fee * 5 / 100;
+                       assert!(signed_tx_fee >= expected_signed_tx_fee &&
+                               signed_tx_fee - fee_error_margin <= expected_signed_tx_fee);
                }
 
+               log_info!(self.logger, "Broadcasting {}", log_tx!(htlc_tx));
                self.broadcaster.broadcast_transactions(&[&htlc_tx]);
                Ok(())
        }
@@ -799,6 +747,8 @@ where
                                claim_id, package_target_feerate_sat_per_1000_weight, commitment_tx,
                                commitment_tx_fee_satoshis, anchor_descriptor, ..
                        } => {
+                               log_info!(self.logger, "Handling channel close bump (claim_id = {}, commitment_txid = {})",
+                                       log_bytes!(claim_id.0), commitment_tx.txid());
                                if let Err(_) = self.handle_channel_close(
                                        *claim_id, *package_target_feerate_sat_per_1000_weight, commitment_tx,
                                        *commitment_tx_fee_satoshis, anchor_descriptor,
@@ -810,6 +760,8 @@ where
                        BumpTransactionEvent::HTLCResolution {
                                claim_id, target_feerate_sat_per_1000_weight, htlc_descriptors, tx_lock_time,
                        } => {
+                               log_info!(self.logger, "Handling HTLC bump (claim_id = {}, htlcs_to_claim = {})",
+                                       log_bytes!(claim_id.0), log_iter!(htlc_descriptors.iter().map(|d| d.outpoint())));
                                if let Err(_) = self.handle_htlc_resolution(
                                        *claim_id, *target_feerate_sat_per_1000_weight, htlc_descriptors, *tx_lock_time,
                                ) {