Move `Channel::build_commitment_transaction` to `ChannelContext` impl
authorDuncan Dean <git@dunxen.dev>
Wed, 7 Jun 2023 10:20:25 +0000 (12:20 +0200)
committerDuncan Dean <git@dunxen.dev>
Wed, 14 Jun 2023 11:42:27 +0000 (13:42 +0200)
This is one of a series of commits to make sure methods are moved by
chunks so they are easily reviewable in diffs. Unfortunately they are
not purely move-only as fields need to be updated for things to
compile, but these should be quite clear.

This commit also uses the `context` field where needed for compilation
and tests to pass due to the above change.

lightning/src/ln/channel.rs

index 74ce190a45dd323aeddf58f8321e6b0d89cc90db..a03a470a5000c142c145c614dace5073e03596a8 100644 (file)
@@ -1054,6 +1054,253 @@ impl<Signer: ChannelSigner> ChannelContext<Signer> {
        pub fn is_funding_initiated(&self) -> bool {
                self.channel_state >= ChannelState::FundingSent as u32
        }
+
+       /// Transaction nomenclature is somewhat confusing here as there are many different cases - a
+       /// transaction is referred to as "a's transaction" implying that a will be able to broadcast
+       /// the transaction. Thus, b will generally be sending a signature over such a transaction to
+       /// a, and a can revoke the transaction by providing b the relevant per_commitment_secret. As
+       /// such, a transaction is generally the result of b increasing the amount paid to a (or adding
+       /// an HTLC to a).
+       /// @local is used only to convert relevant internal structures which refer to remote vs local
+       /// to decide value of outputs and direction of HTLCs.
+       /// @generated_by_local is used to determine *which* HTLCs to include - noting that the HTLC
+       /// state may indicate that one peer has informed the other that they'd like to add an HTLC but
+       /// have not yet committed it. Such HTLCs will only be included in transactions which are being
+       /// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
+       /// which peer generated this transaction and "to whom" this transaction flows.
+       #[inline]
+       fn build_commitment_transaction<L: Deref>(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, logger: &L) -> CommitmentStats
+               where L::Target: Logger
+       {
+               let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
+               let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
+               let mut included_non_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
+
+               let broadcaster_dust_limit_satoshis = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis };
+               let mut remote_htlc_total_msat = 0;
+               let mut local_htlc_total_msat = 0;
+               let mut value_to_self_msat_offset = 0;
+
+               let mut feerate_per_kw = self.feerate_per_kw;
+               if let Some((feerate, update_state)) = self.pending_update_fee {
+                       if match update_state {
+                               // Note that these match the inclusion criteria when scanning
+                               // pending_inbound_htlcs below.
+                               FeeUpdateState::RemoteAnnounced => { debug_assert!(!self.is_outbound()); !generated_by_local },
+                               FeeUpdateState::AwaitingRemoteRevokeToAnnounce => { debug_assert!(!self.is_outbound()); !generated_by_local },
+                               FeeUpdateState::Outbound => { assert!(self.is_outbound());  generated_by_local },
+                       } {
+                               feerate_per_kw = feerate;
+                       }
+               }
+
+               log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...",
+                       commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number),
+                       get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()),
+                       log_bytes!(self.channel_id), if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
+
+               macro_rules! get_htlc_in_commitment {
+                       ($htlc: expr, $offered: expr) => {
+                               HTLCOutputInCommitment {
+                                       offered: $offered,
+                                       amount_msat: $htlc.amount_msat,
+                                       cltv_expiry: $htlc.cltv_expiry,
+                                       payment_hash: $htlc.payment_hash,
+                                       transaction_output_index: None
+                               }
+                       }
+               }
+
+               macro_rules! add_htlc_output {
+                       ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => {
+                               if $outbound == local { // "offered HTLC output"
+                                       let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
+                                       let htlc_tx_fee = if self.opt_anchors() {
+                                               0
+                                       } else {
+                                               feerate_per_kw as u64 * htlc_timeout_tx_weight(false) / 1000
+                                       };
+                                       if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee {
+                                               log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               included_non_dust_htlcs.push((htlc_in_tx, $source));
+                                       } else {
+                                               log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               included_dust_htlcs.push((htlc_in_tx, $source));
+                                       }
+                               } else {
+                                       let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
+                                       let htlc_tx_fee = if self.opt_anchors() {
+                                               0
+                                       } else {
+                                               feerate_per_kw as u64 * htlc_success_tx_weight(false) / 1000
+                                       };
+                                       if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee {
+                                               log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               included_non_dust_htlcs.push((htlc_in_tx, $source));
+                                       } else {
+                                               log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+                                               included_dust_htlcs.push((htlc_in_tx, $source));
+                                       }
+                               }
+                       }
+               }
+
+               for ref htlc in self.pending_inbound_htlcs.iter() {
+                       let (include, state_name) = match htlc.state {
+                               InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
+                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => (!generated_by_local, "AwaitingRemoteRevokeToAnnounce"),
+                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => (true, "AwaitingAnnouncedRemoteRevoke"),
+                               InboundHTLCState::Committed => (true, "Committed"),
+                               InboundHTLCState::LocalRemoved(_) => (!generated_by_local, "LocalRemoved"),
+                       };
+
+                       if include {
+                               add_htlc_output!(htlc, false, None, state_name);
+                               remote_htlc_total_msat += htlc.amount_msat;
+                       } else {
+                               log_trace!(logger, "   ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
+                               match &htlc.state {
+                                       &InboundHTLCState::LocalRemoved(ref reason) => {
+                                               if generated_by_local {
+                                                       if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+                                                               value_to_self_msat_offset += htlc.amount_msat as i64;
+                                                       }
+                                               }
+                                       },
+                                       _ => {},
+                               }
+                       }
+               }
+
+               let mut preimages: Vec<PaymentPreimage> = Vec::new();
+
+               for ref htlc in self.pending_outbound_htlcs.iter() {
+                       let (include, state_name) = match htlc.state {
+                               OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
+                               OutboundHTLCState::Committed => (true, "Committed"),
+                               OutboundHTLCState::RemoteRemoved(_) => (generated_by_local, "RemoteRemoved"),
+                               OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => (generated_by_local, "AwaitingRemoteRevokeToRemove"),
+                               OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => (false, "AwaitingRemovedRemoteRevoke"),
+                       };
+
+                       let preimage_opt = match htlc.state {
+                               OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(p)) => p,
+                               OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(p)) => p,
+                               OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(p)) => p,
+                               _ => None,
+                       };
+
+                       if let Some(preimage) = preimage_opt {
+                               preimages.push(preimage);
+                       }
+
+                       if include {
+                               add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
+                               local_htlc_total_msat += htlc.amount_msat;
+                       } else {
+                               log_trace!(logger, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
+                               match htlc.state {
+                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_))|OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) => {
+                                               value_to_self_msat_offset -= htlc.amount_msat as i64;
+                                       },
+                                       OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_)) => {
+                                               if !generated_by_local {
+                                                       value_to_self_msat_offset -= htlc.amount_msat as i64;
+                                               }
+                                       },
+                                       _ => {},
+                               }
+                       }
+               }
+
+               let mut value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
+               assert!(value_to_self_msat >= 0);
+               // Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie
+               // AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to
+               // "violate" their reserve value by couting those against it. Thus, we have to convert
+               // everything to i64 before subtracting as otherwise we can overflow.
+               let mut value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
+               assert!(value_to_remote_msat >= 0);
+
+               #[cfg(debug_assertions)]
+               {
+                       // Make sure that the to_self/to_remote is always either past the appropriate
+                       // channel_reserve *or* it is making progress towards it.
+                       let mut broadcaster_max_commitment_tx_output = if generated_by_local {
+                               self.holder_max_commitment_tx_output.lock().unwrap()
+                       } else {
+                               self.counterparty_max_commitment_tx_output.lock().unwrap()
+                       };
+                       debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis.unwrap() as i64);
+                       broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
+                       debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= self.holder_selected_channel_reserve_satoshis as i64);
+                       broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
+               }
+
+               let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), self.channel_transaction_parameters.opt_anchors.is_some());
+               let anchors_val = if self.channel_transaction_parameters.opt_anchors.is_some() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 } as i64;
+               let (value_to_self, value_to_remote) = if self.is_outbound() {
+                       (value_to_self_msat / 1000 - anchors_val - total_fee_sat as i64, value_to_remote_msat / 1000)
+               } else {
+                       (value_to_self_msat / 1000, value_to_remote_msat / 1000 - anchors_val - total_fee_sat as i64)
+               };
+
+               let mut value_to_a = if local { value_to_self } else { value_to_remote };
+               let mut value_to_b = if local { value_to_remote } else { value_to_self };
+               let (funding_pubkey_a, funding_pubkey_b) = if local {
+                       (self.get_holder_pubkeys().funding_pubkey, self.get_counterparty_pubkeys().funding_pubkey)
+               } else {
+                       (self.get_counterparty_pubkeys().funding_pubkey, self.get_holder_pubkeys().funding_pubkey)
+               };
+
+               if value_to_a >= (broadcaster_dust_limit_satoshis as i64) {
+                       log_trace!(logger, "   ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
+               } else {
+                       value_to_a = 0;
+               }
+
+               if value_to_b >= (broadcaster_dust_limit_satoshis as i64) {
+                       log_trace!(logger, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
+               } else {
+                       value_to_b = 0;
+               }
+
+               let num_nondust_htlcs = included_non_dust_htlcs.len();
+
+               let channel_parameters =
+                       if local { self.channel_transaction_parameters.as_holder_broadcastable() }
+                       else { self.channel_transaction_parameters.as_counterparty_broadcastable() };
+               let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number,
+                                                                            value_to_a as u64,
+                                                                            value_to_b as u64,
+                                                                            self.channel_transaction_parameters.opt_anchors.is_some(),
+                                                                            funding_pubkey_a,
+                                                                            funding_pubkey_b,
+                                                                            keys.clone(),
+                                                                            feerate_per_kw,
+                                                                            &mut included_non_dust_htlcs,
+                                                                            &channel_parameters
+               );
+               let mut htlcs_included = included_non_dust_htlcs;
+               // The unwrap is safe, because all non-dust HTLCs have been assigned an output index
+               htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap());
+               htlcs_included.append(&mut included_dust_htlcs);
+
+               // For the stats, trimmed-to-0 the value in msats accordingly
+               value_to_self_msat = if (value_to_self_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_self_msat };
+               value_to_remote_msat = if (value_to_remote_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_remote_msat };
+
+               CommitmentStats {
+                       tx,
+                       feerate_per_kw,
+                       total_fee_sat,
+                       num_nondust_htlcs,
+                       htlcs_included,
+                       local_balance_msat: value_to_self_msat as u64,
+                       remote_balance_msat: value_to_remote_msat as u64,
+                       preimages
+               }
+       }
 }
 
 // Internal utility functions for channels
@@ -1098,6 +1345,13 @@ pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis(channe
        cmp::min(channel_value_satoshis, cmp::max(q, 1000))
 }
 
+// Get the fee cost in SATS of a commitment tx with a given number of HTLC outputs.
+// Note that num_htlcs should not include dust HTLCs.
+#[inline]
+fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, opt_anchors: bool) -> u64 {
+       feerate_per_kw as u64 * (commitment_tx_base_weight(opt_anchors) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000
+}
+
 // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
 // has been completed, and then turn into a Channel to get compiler-time enforcement of things like
 // calling channel_id() before we're set up or things like get_outbound_funding_signed on an
@@ -1817,253 +2071,6 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                Ok(chan)
        }
 
-       /// Transaction nomenclature is somewhat confusing here as there are many different cases - a
-       /// transaction is referred to as "a's transaction" implying that a will be able to broadcast
-       /// the transaction. Thus, b will generally be sending a signature over such a transaction to
-       /// a, and a can revoke the transaction by providing b the relevant per_commitment_secret. As
-       /// such, a transaction is generally the result of b increasing the amount paid to a (or adding
-       /// an HTLC to a).
-       /// @local is used only to convert relevant internal structures which refer to remote vs local
-       /// to decide value of outputs and direction of HTLCs.
-       /// @generated_by_local is used to determine *which* HTLCs to include - noting that the HTLC
-       /// state may indicate that one peer has informed the other that they'd like to add an HTLC but
-       /// have not yet committed it. Such HTLCs will only be included in transactions which are being
-       /// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
-       /// which peer generated this transaction and "to whom" this transaction flows.
-       #[inline]
-       fn build_commitment_transaction<L: Deref>(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, logger: &L) -> CommitmentStats
-               where L::Target: Logger
-       {
-               let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
-               let num_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len();
-               let mut included_non_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
-
-               let broadcaster_dust_limit_satoshis = if local { self.context.holder_dust_limit_satoshis } else { self.context.counterparty_dust_limit_satoshis };
-               let mut remote_htlc_total_msat = 0;
-               let mut local_htlc_total_msat = 0;
-               let mut value_to_self_msat_offset = 0;
-
-               let mut feerate_per_kw = self.context.feerate_per_kw;
-               if let Some((feerate, update_state)) = self.context.pending_update_fee {
-                       if match update_state {
-                               // Note that these match the inclusion criteria when scanning
-                               // pending_inbound_htlcs below.
-                               FeeUpdateState::RemoteAnnounced => { debug_assert!(!self.context.is_outbound()); !generated_by_local },
-                               FeeUpdateState::AwaitingRemoteRevokeToAnnounce => { debug_assert!(!self.context.is_outbound()); !generated_by_local },
-                               FeeUpdateState::Outbound => { assert!(self.context.is_outbound());  generated_by_local },
-                       } {
-                               feerate_per_kw = feerate;
-                       }
-               }
-
-               log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...",
-                       commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number),
-                       get_commitment_transaction_number_obscure_factor(&self.context.get_holder_pubkeys().payment_point, &self.context.get_counterparty_pubkeys().payment_point, self.context.is_outbound()),
-                       log_bytes!(self.context.channel_id), if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
-
-               macro_rules! get_htlc_in_commitment {
-                       ($htlc: expr, $offered: expr) => {
-                               HTLCOutputInCommitment {
-                                       offered: $offered,
-                                       amount_msat: $htlc.amount_msat,
-                                       cltv_expiry: $htlc.cltv_expiry,
-                                       payment_hash: $htlc.payment_hash,
-                                       transaction_output_index: None
-                               }
-                       }
-               }
-
-               macro_rules! add_htlc_output {
-                       ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => {
-                               if $outbound == local { // "offered HTLC output"
-                                       let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
-                                       let htlc_tx_fee = if self.context.opt_anchors() {
-                                               0
-                                       } else {
-                                               feerate_per_kw as u64 * htlc_timeout_tx_weight(false) / 1000
-                                       };
-                                       if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee {
-                                               log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
-                                               included_non_dust_htlcs.push((htlc_in_tx, $source));
-                                       } else {
-                                               log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
-                                               included_dust_htlcs.push((htlc_in_tx, $source));
-                                       }
-                               } else {
-                                       let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
-                                       let htlc_tx_fee = if self.context.opt_anchors() {
-                                               0
-                                       } else {
-                                               feerate_per_kw as u64 * htlc_success_tx_weight(false) / 1000
-                                       };
-                                       if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee {
-                                               log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
-                                               included_non_dust_htlcs.push((htlc_in_tx, $source));
-                                       } else {
-                                               log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
-                                               included_dust_htlcs.push((htlc_in_tx, $source));
-                                       }
-                               }
-                       }
-               }
-
-               for ref htlc in self.context.pending_inbound_htlcs.iter() {
-                       let (include, state_name) = match htlc.state {
-                               InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
-                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => (!generated_by_local, "AwaitingRemoteRevokeToAnnounce"),
-                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => (true, "AwaitingAnnouncedRemoteRevoke"),
-                               InboundHTLCState::Committed => (true, "Committed"),
-                               InboundHTLCState::LocalRemoved(_) => (!generated_by_local, "LocalRemoved"),
-                       };
-
-                       if include {
-                               add_htlc_output!(htlc, false, None, state_name);
-                               remote_htlc_total_msat += htlc.amount_msat;
-                       } else {
-                               log_trace!(logger, "   ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
-                               match &htlc.state {
-                                       &InboundHTLCState::LocalRemoved(ref reason) => {
-                                               if generated_by_local {
-                                                       if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
-                                                               value_to_self_msat_offset += htlc.amount_msat as i64;
-                                                       }
-                                               }
-                                       },
-                                       _ => {},
-                               }
-                       }
-               }
-
-               let mut preimages: Vec<PaymentPreimage> = Vec::new();
-
-               for ref htlc in self.context.pending_outbound_htlcs.iter() {
-                       let (include, state_name) = match htlc.state {
-                               OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
-                               OutboundHTLCState::Committed => (true, "Committed"),
-                               OutboundHTLCState::RemoteRemoved(_) => (generated_by_local, "RemoteRemoved"),
-                               OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => (generated_by_local, "AwaitingRemoteRevokeToRemove"),
-                               OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => (false, "AwaitingRemovedRemoteRevoke"),
-                       };
-
-                       let preimage_opt = match htlc.state {
-                               OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(p)) => p,
-                               OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(p)) => p,
-                               OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(p)) => p,
-                               _ => None,
-                       };
-
-                       if let Some(preimage) = preimage_opt {
-                               preimages.push(preimage);
-                       }
-
-                       if include {
-                               add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
-                               local_htlc_total_msat += htlc.amount_msat;
-                       } else {
-                               log_trace!(logger, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
-                               match htlc.state {
-                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_))|OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) => {
-                                               value_to_self_msat_offset -= htlc.amount_msat as i64;
-                                       },
-                                       OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_)) => {
-                                               if !generated_by_local {
-                                                       value_to_self_msat_offset -= htlc.amount_msat as i64;
-                                               }
-                                       },
-                                       _ => {},
-                               }
-                       }
-               }
-
-               let mut value_to_self_msat: i64 = (self.context.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
-               assert!(value_to_self_msat >= 0);
-               // Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie
-               // AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to
-               // "violate" their reserve value by couting those against it. Thus, we have to convert
-               // everything to i64 before subtracting as otherwise we can overflow.
-               let mut value_to_remote_msat: i64 = (self.context.channel_value_satoshis * 1000) as i64 - (self.context.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
-               assert!(value_to_remote_msat >= 0);
-
-               #[cfg(debug_assertions)]
-               {
-                       // Make sure that the to_self/to_remote is always either past the appropriate
-                       // channel_reserve *or* it is making progress towards it.
-                       let mut broadcaster_max_commitment_tx_output = if generated_by_local {
-                               self.context.holder_max_commitment_tx_output.lock().unwrap()
-                       } else {
-                               self.context.counterparty_max_commitment_tx_output.lock().unwrap()
-                       };
-                       debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.context.counterparty_selected_channel_reserve_satoshis.unwrap() as i64);
-                       broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
-                       debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= self.context.holder_selected_channel_reserve_satoshis as i64);
-                       broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
-               }
-
-               let total_fee_sat = Channel::<Signer>::commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), self.context.channel_transaction_parameters.opt_anchors.is_some());
-               let anchors_val = if self.context.channel_transaction_parameters.opt_anchors.is_some() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 } as i64;
-               let (value_to_self, value_to_remote) = if self.context.is_outbound() {
-                       (value_to_self_msat / 1000 - anchors_val - total_fee_sat as i64, value_to_remote_msat / 1000)
-               } else {
-                       (value_to_self_msat / 1000, value_to_remote_msat / 1000 - anchors_val - total_fee_sat as i64)
-               };
-
-               let mut value_to_a = if local { value_to_self } else { value_to_remote };
-               let mut value_to_b = if local { value_to_remote } else { value_to_self };
-               let (funding_pubkey_a, funding_pubkey_b) = if local {
-                       (self.context.get_holder_pubkeys().funding_pubkey, self.context.get_counterparty_pubkeys().funding_pubkey)
-               } else {
-                       (self.context.get_counterparty_pubkeys().funding_pubkey, self.context.get_holder_pubkeys().funding_pubkey)
-               };
-
-               if value_to_a >= (broadcaster_dust_limit_satoshis as i64) {
-                       log_trace!(logger, "   ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
-               } else {
-                       value_to_a = 0;
-               }
-
-               if value_to_b >= (broadcaster_dust_limit_satoshis as i64) {
-                       log_trace!(logger, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
-               } else {
-                       value_to_b = 0;
-               }
-
-               let num_nondust_htlcs = included_non_dust_htlcs.len();
-
-               let channel_parameters =
-                       if local { self.context.channel_transaction_parameters.as_holder_broadcastable() }
-                       else { self.context.channel_transaction_parameters.as_counterparty_broadcastable() };
-               let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number,
-                                                                            value_to_a as u64,
-                                                                            value_to_b as u64,
-                                                                            self.context.channel_transaction_parameters.opt_anchors.is_some(),
-                                                                            funding_pubkey_a,
-                                                                            funding_pubkey_b,
-                                                                            keys.clone(),
-                                                                            feerate_per_kw,
-                                                                            &mut included_non_dust_htlcs,
-                                                                            &channel_parameters
-               );
-               let mut htlcs_included = included_non_dust_htlcs;
-               // The unwrap is safe, because all non-dust HTLCs have been assigned an output index
-               htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap());
-               htlcs_included.append(&mut included_dust_htlcs);
-
-               // For the stats, trimmed-to-0 the value in msats accordingly
-               value_to_self_msat = if (value_to_self_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_self_msat };
-               value_to_remote_msat = if (value_to_remote_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_remote_msat };
-
-               CommitmentStats {
-                       tx,
-                       feerate_per_kw,
-                       total_fee_sat,
-                       num_nondust_htlcs,
-                       htlcs_included,
-                       local_balance_msat: value_to_self_msat as u64,
-                       remote_balance_msat: value_to_remote_msat as u64,
-                       preimages
-               }
-       }
-
        #[inline]
        fn get_closing_scriptpubkey(&self) -> Script {
                // The shutdown scriptpubkey is set on channel opening when option_upfront_shutdown_script
@@ -2611,7 +2618,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                let funding_script = self.get_funding_redeemscript();
 
                let keys = self.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
-               let initial_commitment_tx = self.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, false, logger).tx;
+               let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, false, logger).tx;
                {
                        let trusted_tx = initial_commitment_tx.trust();
                        let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
@@ -2625,7 +2632,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                }
 
                let counterparty_keys = self.build_remote_transaction_keys();
-               let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
+               let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.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();
@@ -2758,7 +2765,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                let funding_script = self.get_funding_redeemscript();
 
                let counterparty_keys = self.build_remote_transaction_keys();
-               let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
+               let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.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();
 
@@ -2766,7 +2773,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        log_bytes!(self.context.channel_id()), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
 
                let holder_signer = self.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
-               let initial_commitment_tx = self.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
+               let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
                {
                        let trusted_tx = initial_commitment_tx.trust();
                        let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
@@ -3127,13 +3134,6 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                (commitment_tx_base_weight(opt_anchors) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate_per_kw as u64 / 1000 * 1000
        }
 
-       // Get the fee cost in SATS of a commitment tx with a given number of HTLC outputs.
-       // Note that num_htlcs should not include dust HTLCs.
-       #[inline]
-       fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, opt_anchors: bool) -> u64 {
-               feerate_per_kw as u64 * (commitment_tx_base_weight(opt_anchors) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000
-       }
-
        /// Get the commitment tx fee for the local's (i.e. our) next commitment transaction based on the
        /// number of pending HTLCs that are on track to be in our next commitment tx.
        ///
@@ -3562,7 +3562,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
                let keys = self.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
 
-               let commitment_stats = self.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, false, logger);
+               let commitment_stats = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, false, logger);
                let commitment_txid = {
                        let trusted_tx = commitment_stats.tx.trust();
                        let bitcoin_tx = trusted_tx.built_transaction();
@@ -4153,8 +4153,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                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 = self.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
-               let commitment_stats = self.build_commitment_transaction(self.context.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.context.opt_anchors()) * 1000;
+               let commitment_stats = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, true, logger);
+               let buffer_fee_msat = 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.context.opt_anchors()) * 1000;
                let holder_balance_msat = commitment_stats.local_balance_msat - outbound_stats.holding_cell_msat;
                if holder_balance_msat < buffer_fee_msat  + self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
                        //TODO: auto-close after a number of failures?
@@ -5707,7 +5707,7 @@ impl<Signer: WriteableEcdsaChannelSigner> 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_initial_commitment_tx = self.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
+               let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
                Ok(self.context.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
        }
@@ -6159,7 +6159,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
        fn build_commitment_no_state_update<L: Deref>(&self, logger: &L) -> (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger {
                let counterparty_keys = self.build_remote_transaction_keys();
-               let commitment_stats = self.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
+               let commitment_stats = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
                let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
 
                #[cfg(any(test, fuzzing))]
@@ -6191,7 +6191,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                self.build_commitment_no_state_update(logger);
 
                let counterparty_keys = self.build_remote_transaction_keys();
-               let commitment_stats = self.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
+               let commitment_stats = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
                let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
                let (signature, htlc_signatures);
 
@@ -7869,7 +7869,7 @@ mod tests {
                                $( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
                        } ) => { {
                                let (commitment_tx, htlcs): (_, Vec<HTLCOutputInCommitment>) = {
-                                       let mut commitment_stats = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, &logger);
+                                       let mut commitment_stats = chan.context.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, &logger);
 
                                        let htlcs = commitment_stats.htlcs_included.drain(..)
                                                .filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })