Merge pull request #2693 from Evanfeenstra/next-hop-pubkey-secp-mode
[rust-lightning] / lightning / src / ln / channel.rs
index 82f1de6ee0c75a96e156bc4532de4e41bde25af6..33a46084abb4d1bba4bf3de72168c4d1b3933b7d 100644 (file)
@@ -723,7 +723,7 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
 
        cur_holder_commitment_transaction_number: u64,
        cur_counterparty_commitment_transaction_number: u64,
-       value_to_self_msat: u64, // Excluding all pending_htlcs, excluding fees
+       value_to_self_msat: u64, // Excluding all pending_htlcs, fees, and anchor outputs
        pending_inbound_htlcs: Vec<InboundHTLCOutput>,
        pending_outbound_htlcs: Vec<OutboundHTLCOutput>,
        holding_cell_htlc_updates: Vec<HTLCUpdateAwaitingACK>,
@@ -1168,7 +1168,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                match self.config.options.max_dust_htlc_exposure {
                        MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => {
                                let feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(
-                                       ConfirmationTarget::HighPriority);
+                                       ConfirmationTarget::OnChainSweep);
                                feerate_per_kw as u64 * multiplier
                        },
                        MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
@@ -1672,6 +1672,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 
                let mut available_capacity_msat = outbound_capacity_msat;
 
+               let anchor_outputs_value_msat = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                       ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
+               } else {
+                       0
+               };
                if context.is_outbound() {
                        // We should mind channel commit tx fee when computing how much of the available capacity
                        // can be used in the next htlc. Mirrors the logic in send_htlc.
@@ -1686,14 +1691,19 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        }
 
                        let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered);
-                       let max_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
+                       let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
                        let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered);
-                       let min_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
+                       let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
+                       if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                               max_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
+                               min_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
+                       }
 
                        // We will first subtract the fee as if we were above-dust. Then, if the resulting
                        // value ends up being below dust, we have this fee available again. In that case,
                        // match the value to right-below-dust.
-                       let mut capacity_minus_commitment_fee_msat: i64 = (available_capacity_msat as i64) - (max_reserved_commit_tx_fee_msat as i64);
+                       let mut capacity_minus_commitment_fee_msat: i64 = available_capacity_msat as i64 -
+                               max_reserved_commit_tx_fee_msat as i64 - anchor_outputs_value_msat as i64;
                        if capacity_minus_commitment_fee_msat < (real_dust_limit_timeout_sat as i64) * 1000 {
                                let one_htlc_difference_msat = max_reserved_commit_tx_fee_msat - min_reserved_commit_tx_fee_msat;
                                debug_assert!(one_htlc_difference_msat != 0);
@@ -1718,7 +1728,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        let remote_balance_msat = (context.channel_value_satoshis * 1000 - context.value_to_self_msat)
                                .saturating_sub(inbound_stats.pending_htlcs_value_msat);
 
-                       if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat {
+                       if remote_balance_msat < max_reserved_commit_tx_fee_msat + holder_selected_chan_reserve_msat + anchor_outputs_value_msat {
                                // If another HTLC's fee would reduce the remote's balance below the reserve limit
                                // we've selected for them, we can only send dust HTLCs.
                                available_capacity_msat = cmp::min(available_capacity_msat, real_dust_limit_success_sat * 1000 - 1);
@@ -2122,7 +2132,7 @@ fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_type_feature
 
 // Get the fee cost in MSATS of a commitment tx with a given number of HTLC outputs.
 // Note that num_htlcs should not include dust HTLCs.
-fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 {
+pub(crate) fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 {
        // Note that we need to divide before multiplying to round properly,
        // since the lowest denomination of bitcoin on-chain is the satoshi.
        (commitment_tx_base_weight(channel_type_features) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate_per_kw as u64 / 1000 * 1000
@@ -2159,28 +2169,20 @@ impl<SP: Deref> Channel<SP> where
                // apply to channels supporting anchor outputs since HTLC transactions are pre-signed with a
                // zero fee, so their fee is no longer considered to determine dust limits.
                if !channel_type.supports_anchors_zero_fee_htlc_tx() {
-                       let upper_limit = cmp::max(250 * 25,
-                               fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
+                       let upper_limit =
+                               fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee) as u64;
                        if feerate_per_kw as u64 > upper_limit {
                                return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
                        }
                }
 
-               // We can afford to use a lower bound with anchors than previously since we can now bump
-               // fees when broadcasting our commitment. However, we must still make sure we meet the
-               // minimum mempool feerate, until package relay is deployed, such that we can ensure the
-               // commitment transaction propagates throughout node mempools on its own.
                let lower_limit_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
-                       ConfirmationTarget::MempoolMinimum
+                       ConfirmationTarget::MinAllowedAnchorChannelRemoteFee
                } else {
-                       ConfirmationTarget::Background
+                       ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee
                };
                let lower_limit = fee_estimator.bounded_sat_per_1000_weight(lower_limit_conf_target);
-               // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
-               // occasional issues with feerate disagreements between an initiator that wants a feerate
-               // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
-               // sat/kw before the comparison here.
-               if feerate_per_kw + 250 < lower_limit {
+               if feerate_per_kw < lower_limit {
                        if let Some(cur_feerate) = cur_feerate_per_kw {
                                if feerate_per_kw > cur_feerate {
                                        log_warn!(logger,
@@ -2189,7 +2191,7 @@ impl<SP: Deref> Channel<SP> where
                                        return Ok(());
                                }
                        }
-                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
+                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
                }
                Ok(())
        }
@@ -2777,6 +2779,7 @@ impl<SP: Deref> Channel<SP> where
                if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > self.context.holder_max_htlc_value_in_flight_msat {
                        return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", self.context.holder_max_htlc_value_in_flight_msat)));
                }
+
                // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet
                // the reserve_satoshis we told them to always have as direct payment so that they lose
                // something if we punish them for broadcasting an old state).
@@ -2836,30 +2839,40 @@ impl<SP: Deref> Channel<SP> where
 
                // Check that the remote can afford to pay for this HTLC on-chain at the current
                // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
-               let remote_commit_tx_fee_msat = if self.context.is_outbound() { 0 } else {
-                       let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
-                       self.context.next_remote_commit_tx_fee_msat(htlc_candidate, None) // Don't include the extra fee spike buffer HTLC in calculations
-               };
-               if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
-                       return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
-               };
-
-               if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < self.context.holder_selected_channel_reserve_satoshis * 1000 {
-                       return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
+               {
+                       let remote_commit_tx_fee_msat = if self.context.is_outbound() { 0 } else {
+                               let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
+                               self.context.next_remote_commit_tx_fee_msat(htlc_candidate, None) // Don't include the extra fee spike buffer HTLC in calculations
+                       };
+                       let anchor_outputs_value_msat = if !self.context.is_outbound() && self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                               ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
+                       } else {
+                               0
+                       };
+                       if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat {
+                               return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
+                       };
+                       if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < self.context.holder_selected_channel_reserve_satoshis * 1000 {
+                               return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
+                       }
                }
 
+               let anchor_outputs_value_msat = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                       ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
+               } else {
+                       0
+               };
                if !self.context.is_outbound() {
-                       // `2 *` and `Some(())` is for the fee spike buffer we keep for the remote. This deviates from
-                       // the spec because in the spec, the fee spike buffer requirement doesn't exist on the
-                       // receiver's side, only on the sender's.
-                       // Note that when we eventually remove support for fee updates and switch to anchor output
-                       // fees, we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep
-                       // the extra htlc when calculating the next remote commitment transaction fee as we should
-                       // still be able to afford adding this HTLC plus one more future HTLC, regardless of being
-                       // sensitive to fee spikes.
+                       // `Some(())` is for the fee spike buffer we keep for the remote. This deviates from
+                       // the spec because the fee spike buffer requirement doesn't exist on the receiver's
+                       // side, only on the sender's. Note that with anchor outputs we are no longer as
+                       // sensitive to fee spikes, so we need to account for them.
                        let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
-                       let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.context.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
-                       if pending_remote_value_msat - msg.amount_msat - self.context.holder_selected_channel_reserve_satoshis * 1000 < remote_fee_cost_incl_stuck_buffer_msat {
+                       let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
+                       if !self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                               remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
+                       }
+                       if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(self.context.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat {
                                // Note that if the pending_forward_status is not updated here, then it's because we're already failing
                                // the HTLC, i.e. its status is already set to failing.
                                log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.context.channel_id());
@@ -2869,7 +2882,7 @@ impl<SP: Deref> Channel<SP> where
                        // Check that they won't violate our local required channel reserve by adding this HTLC.
                        let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
                        let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(htlc_candidate, None);
-                       if self.context.value_to_self_msat < self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat {
+                       if self.context.value_to_self_msat < self.context.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat {
                                return Err(ChannelError::Close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
                        }
                }
@@ -4159,8 +4172,10 @@ impl<SP: Deref> Channel<SP> where
                // Propose a range from our current Background feerate to our Normal feerate plus our
                // force_close_avoidance_max_fee_satoshis.
                // If we fail to come to consensus, we'll have to force-close.
-               let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background);
-               let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
+               let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::ChannelCloseMinimum);
+               // Use NonAnchorChannelFee because this should be an estimate for a channel close
+               // that we don't expect to need fee bumping
+               let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
                let mut proposed_max_feerate = if self.context.is_outbound() { normal_feerate } else { u32::max_value() };
 
                // The spec requires that (when the channel does not have anchors) we only send absolute
@@ -5748,16 +5763,16 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                let channel_type = Self::get_initial_channel_type(&config, their_features);
                debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
 
-               let commitment_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
-                       ConfirmationTarget::MempoolMinimum
+               let (commitment_conf_target, anchor_outputs_value_msat)  = if channel_type.supports_anchors_zero_fee_htlc_tx() {
+                       (ConfirmationTarget::AnchorChannelFee, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000)
                } else {
-                       ConfirmationTarget::Normal
+                       (ConfirmationTarget::NonAnchorChannelFee, 0)
                };
                let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target);
 
                let value_to_self_msat = channel_value_satoshis * 1000 - push_msat;
                let commitment_tx_fee = commit_tx_fee_msat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
-               if value_to_self_msat < commitment_tx_fee {
+               if value_to_self_msat.saturating_sub(anchor_outputs_value_msat) < commitment_tx_fee {
                        return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commitment_tx_fee / 1000) });
                }
 
@@ -6041,7 +6056,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                // whatever reason.
                if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() {
                        self.context.channel_type.clear_anchors_zero_fee_htlc_tx();
-                       self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
+                       self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
                        assert!(!self.context.channel_transaction_parameters.channel_type_features.supports_anchors_nonzero_fee_htlc_tx());
                } else if self.context.channel_type.supports_scid_privacy() {
                        self.context.channel_type.clear_scid_privacy();
@@ -6374,13 +6389,18 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 
                // check if the funder's amount for the initial commitment tx is sufficient
                // for full fee payment plus a few HTLCs to ensure the channel will be useful.
+               let anchor_outputs_value = if channel_type.supports_anchors_zero_fee_htlc_tx() {
+                       ANCHOR_OUTPUT_VALUE_SATOSHI * 2
+               } else {
+                       0
+               };
                let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat;
                let commitment_tx_fee = commit_tx_fee_msat(msg.feerate_per_kw, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000;
-               if funders_amount_msat / 1000 < commitment_tx_fee {
-                       return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", funders_amount_msat / 1000, commitment_tx_fee)));
+               if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commitment_tx_fee {
+                       return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commitment_tx_fee)));
                }
 
-               let to_remote_satoshis = funders_amount_msat / 1000 - commitment_tx_fee;
+               let to_remote_satoshis = funders_amount_msat / 1000 - commitment_tx_fee - anchor_outputs_value;
                // While it's reasonable for us to not meet the channel reserve initially (if they don't
                // want to push much to us), our counterparty should always have more than our reserve.
                if to_remote_satoshis < holder_selected_channel_reserve_satoshis {
@@ -8206,7 +8226,7 @@ mod tests {
                use bitcoin::hashes::hex::FromHex;
                use bitcoin::hash_types::Txid;
                use bitcoin::secp256k1::Message;
-               use crate::sign::EcdsaChannelSigner;
+               use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, EcdsaChannelSigner};
                use crate::ln::PaymentPreimage;
                use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys};
                use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
@@ -8331,7 +8351,7 @@ mod tests {
                                        &chan.context.holder_signer.as_ref().pubkeys().funding_pubkey,
                                        chan.context.counterparty_funding_pubkey()
                                );
-                               let (holder_sig, htlc_sigs) = signer.sign_holder_commitment_and_htlcs(&holder_commitment_tx, &secp_ctx).unwrap();
+                               let holder_sig = signer.sign_holder_commitment(&holder_commitment_tx, &secp_ctx).unwrap();
                                assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig, "holder_sig");
 
                                let funding_redeemscript = chan.context.get_funding_redeemscript();
@@ -8339,14 +8359,14 @@ mod tests {
                                assert_eq!(serialize(&tx)[..], hex::decode($tx_hex).unwrap()[..], "tx");
 
                                // ((htlc, counterparty_sig), (index, holder_sig))
-                               let mut htlc_sig_iter = holder_commitment_tx.htlcs().iter().zip(&holder_commitment_tx.counterparty_htlc_sigs).zip(htlc_sigs.iter().enumerate());
+                               let mut htlc_counterparty_sig_iter = holder_commitment_tx.counterparty_htlc_sigs.iter();
 
                                $({
                                        log_trace!(logger, "verifying htlc {}", $htlc_idx);
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
 
                                        let ref htlc = htlcs[$htlc_idx];
-                                       let htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw,
+                                       let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw,
                                                chan.context.get_counterparty_selected_contest_delay().unwrap(),
                                                &htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
                                        let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, $opt_anchors, &keys);
@@ -8366,20 +8386,32 @@ mod tests {
                                                assert!(preimage.is_some());
                                        }
 
-                                       let htlc_sig = htlc_sig_iter.next().unwrap();
+                                       let htlc_counterparty_sig = htlc_counterparty_sig_iter.next().unwrap();
+                                       let htlc_holder_sig = signer.sign_holder_htlc_transaction(&htlc_tx, 0, &HTLCDescriptor {
+                                               channel_derivation_parameters: ChannelDerivationParameters {
+                                                       value_satoshis: chan.context.channel_value_satoshis,
+                                                       keys_id: chan.context.channel_keys_id,
+                                                       transaction_parameters: chan.context.channel_transaction_parameters.clone(),
+                                               },
+                                               commitment_txid: trusted_tx.txid(),
+                                               per_commitment_number: trusted_tx.commitment_number(),
+                                               per_commitment_point: trusted_tx.per_commitment_point(),
+                                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                                               htlc: htlc.clone(),
+                                               preimage: preimage.clone(),
+                                               counterparty_sig: *htlc_counterparty_sig,
+                                       }, &secp_ctx).unwrap();
                                        let num_anchors = if $opt_anchors.supports_anchors_zero_fee_htlc_tx() { 2 } else { 0 };
-                                       assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx + num_anchors), "output index");
+                                       assert_eq!(htlc.transaction_output_index, Some($htlc_idx + num_anchors), "output index");
 
                                        let signature = Signature::from_der(&hex::decode($htlc_sig_hex).unwrap()[..]).unwrap();
-                                       assert_eq!(signature, *(htlc_sig.1).1, "htlc sig");
-                                       let index = (htlc_sig.1).0;
-                                       let channel_parameters = chan.context.channel_transaction_parameters.as_holder_broadcastable();
+                                       assert_eq!(signature, htlc_holder_sig, "htlc sig");
                                        let trusted_tx = holder_commitment_tx.trust();
-                                       log_trace!(logger, "htlc_tx = {}", hex::encode(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))));
-                                       assert_eq!(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))[..],
-                                                       hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx");
+                                       htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness($htlc_idx, htlc_counterparty_sig, &htlc_holder_sig, &preimage);
+                                       log_trace!(logger, "htlc_tx = {}", hex::encode(serialize(&htlc_tx)));
+                                       assert_eq!(serialize(&htlc_tx)[..], hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx");
                                })*
-                               assert!(htlc_sig_iter.next().is_none());
+                               assert!(htlc_counterparty_sig_iter.next().is_none());
                        } }
                }