Merge pull request #2674 from wpaulino/consider-anchor-outputs-value-balances
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 20 Oct 2023 22:54:08 +0000 (22:54 +0000)
committerGitHub <noreply@github.com>
Fri, 20 Oct 2023 22:54:08 +0000 (22:54 +0000)
Consider anchor outputs value throughout balance checks and computations

fuzz/src/chanmon_consistency.rs
lightning/src/ln/channel.rs
lightning/src/ln/monitor_tests.rs
lightning/src/ln/payment_tests.rs

index 3bdd78167865686c1b264a1bd9a1d7fe9b399997..dddd97cae4554c8f22a09af98bf5d1098ea596fa 100644 (file)
@@ -432,7 +432,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
 }
 
 #[inline]
-pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
+pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
        let out = SearchingOutput::new(underlying_out);
        let broadcast = Arc::new(TestBroadcaster{});
        let router = FuzzRouter {};
@@ -450,6 +450,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
                        let mut config = UserConfig::default();
                        config.channel_config.forwarding_fee_proportional_millionths = 0;
                        config.channel_handshake_config.announced_channel = true;
+                       if anchors {
+                               config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+                               config.manually_accept_inbound_channels = true;
+                       }
                        let network = Network::Bitcoin;
                        let best_block_timestamp = genesis_block(network).header.time;
                        let params = ChainParameters {
@@ -473,6 +477,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
                        let mut config = UserConfig::default();
                        config.channel_config.forwarding_fee_proportional_millionths = 0;
                        config.channel_handshake_config.announced_channel = true;
+                       if anchors {
+                               config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+                               config.manually_accept_inbound_channels = true;
+                       }
 
                        let mut monitors = HashMap::new();
                        let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap();
@@ -509,7 +517,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 
        let mut channel_txn = Vec::new();
        macro_rules! make_channel {
-               ($source: expr, $dest: expr, $chan_id: expr) => { {
+               ($source: expr, $dest: expr, $dest_keys_manager: expr, $chan_id: expr) => { {
                        $source.peer_connected(&$dest.get_our_node_id(), &Init {
                                features: $dest.init_features(), networks: None, remote_network_address: None
                        }, true).unwrap();
@@ -528,6 +536,22 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 
                        $dest.handle_open_channel(&$source.get_our_node_id(), &open_channel);
                        let accept_channel = {
+                               if anchors {
+                                       let events = $dest.get_and_clear_pending_events();
+                                       assert_eq!(events.len(), 1);
+                                       if let events::Event::OpenChannelRequest {
+                                               ref temporary_channel_id, ref counterparty_node_id, ..
+                                       } = events[0] {
+                                               let mut random_bytes = [0u8; 16];
+                                               random_bytes.copy_from_slice(&$dest_keys_manager.get_secure_random_bytes()[..16]);
+                                               let user_channel_id = u128::from_be_bytes(random_bytes);
+                                               $dest.accept_inbound_channel(
+                                                       temporary_channel_id,
+                                                       counterparty_node_id,
+                                                       user_channel_id,
+                                               ).unwrap();
+                                       } else { panic!("Wrong event type"); }
+                               }
                                let events = $dest.get_and_clear_pending_msg_events();
                                assert_eq!(events.len(), 1);
                                if let events::MessageSendEvent::SendAcceptChannel { ref msg, .. } = events[0] {
@@ -639,8 +663,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 
        let mut nodes = [node_a, node_b, node_c];
 
-       let chan_1_funding = make_channel!(nodes[0], nodes[1], 0);
-       let chan_2_funding = make_channel!(nodes[1], nodes[2], 1);
+       let chan_1_funding = make_channel!(nodes[0], nodes[1], keys_manager_b, 0);
+       let chan_2_funding = make_channel!(nodes[1], nodes[2], keys_manager_c, 1);
 
        for node in nodes.iter() {
                confirm_txn!(node);
@@ -1199,7 +1223,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
                        0x6d => { send_hop_payment(&nodes[2], &nodes[1], chan_b, &nodes[0], chan_a, 1, &mut payment_id, &mut payment_idx); },
 
                        0x80 => {
-                               let max_feerate = last_htlc_clear_fee_a * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               let mut max_feerate = last_htlc_clear_fee_a;
+                               if !anchors {
+                                       max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               }
                                if fee_est_a.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
                                        fee_est_a.ret_val.store(max_feerate, atomic::Ordering::Release);
                                }
@@ -1208,7 +1235,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
                        0x81 => { fee_est_a.ret_val.store(253, atomic::Ordering::Release); nodes[0].maybe_update_chan_fees(); },
 
                        0x84 => {
-                               let max_feerate = last_htlc_clear_fee_b * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               let mut max_feerate = last_htlc_clear_fee_b;
+                               if !anchors {
+                                       max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               }
                                if fee_est_b.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
                                        fee_est_b.ret_val.store(max_feerate, atomic::Ordering::Release);
                                }
@@ -1217,7 +1247,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
                        0x85 => { fee_est_b.ret_val.store(253, atomic::Ordering::Release); nodes[1].maybe_update_chan_fees(); },
 
                        0x88 => {
-                               let max_feerate = last_htlc_clear_fee_c * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               let mut max_feerate = last_htlc_clear_fee_c;
+                               if !anchors {
+                                       max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               }
                                if fee_est_c.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
                                        fee_est_c.ret_val.store(max_feerate, atomic::Ordering::Release);
                                }
@@ -1338,10 +1371,12 @@ impl<O: Output> SearchingOutput<O> {
 }
 
 pub fn chanmon_consistency_test<Out: Output>(data: &[u8], out: Out) {
-       do_test(data, out);
+       do_test(data, out.clone(), false);
+       do_test(data, out, true);
 }
 
 #[no_mangle]
 pub extern "C" fn chanmon_consistency_run(data: *const u8, datalen: usize) {
-       do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull{});
+       do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull{}, false);
+       do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull{}, true);
 }
index 6a468f2d6fe9f71209691edfa7f0318a349d084a..f3632b41b471098392bf357e3a9e7220995588ce 100644 (file)
@@ -724,7 +724,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>,
@@ -1673,6 +1673,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.
@@ -1687,14 +1692,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);
@@ -1719,7 +1729,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);
@@ -2119,7 +2129,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
@@ -2766,6 +2776,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).
@@ -2825,30 +2836,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());
@@ -2858,7 +2879,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()));
                        }
                }
@@ -5721,16 +5742,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::AnchorChannelFee
+               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::NonAnchorChannelFee
+                       (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) });
                }
 
@@ -6347,13 +6368,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 {
index 0399ed38251ba7bc155af80323add37b86c9eef2..dcc0d57409bbd7abc4a5abae8a4970f8ea1c0de7 100644 (file)
@@ -1400,7 +1400,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 
        // Create some initial channels
        let (_, _, chan_id, funding_tx) =
-               create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 11_000_000);
+               create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 12_000_000);
        let funding_outpoint = OutPoint { txid: funding_tx.txid(), index: 0 };
        assert_eq!(funding_outpoint.to_channel_id(), chan_id);
 
@@ -1410,9 +1410,9 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        assert_eq!(revoked_local_txn[0].input.len(), 1);
        assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, funding_tx.txid());
        if anchors {
-               assert_eq!(revoked_local_txn[0].output[4].value, 10000); // to_self output
+               assert_eq!(revoked_local_txn[0].output[4].value, 11000); // to_self output
        } else {
-               assert_eq!(revoked_local_txn[0].output[2].value, 10000); // to_self output
+               assert_eq!(revoked_local_txn[0].output[2].value, 11000); // to_self output
        }
 
        // The to-be-revoked commitment tx should have two HTLCs, an output for each side, and an
@@ -1499,11 +1499,11 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 };
        let as_balances = sorted_vec(vec![Balance::ClaimableAwaitingConfirmations {
                        // to_remote output in B's revoked commitment
-                       amount_satoshis: 1_000_000 - 11_000 - 3_000 - commitment_tx_fee - anchor_outputs_value,
+                       amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value,
                        confirmation_height: to_remote_conf_height,
                }, Balance::CounterpartyRevokedOutputClaimable {
                        // to_self output in B's revoked commitment
-                       amount_satoshis: 10_000,
+                       amount_satoshis: 11_000,
                }, Balance::CounterpartyRevokedOutputClaimable { // HTLC 1
                        amount_satoshis: 3_000,
                }, Balance::CounterpartyRevokedOutputClaimable { // HTLC 2
@@ -1544,11 +1544,11 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        mine_transaction(&nodes[0], &as_htlc_claim_tx[0]);
        assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations {
                        // to_remote output in B's revoked commitment
-                       amount_satoshis: 1_000_000 - 11_000 - 3_000 - commitment_tx_fee - anchor_outputs_value,
+                       amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value,
                        confirmation_height: to_remote_conf_height,
                }, Balance::CounterpartyRevokedOutputClaimable {
                        // to_self output in B's revoked commitment
-                       amount_satoshis: 10_000,
+                       amount_satoshis: 11_000,
                }, Balance::CounterpartyRevokedOutputClaimable { // HTLC 2
                        amount_satoshis: 1_000,
                }, Balance::ClaimableAwaitingConfirmations {
@@ -1561,7 +1561,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        test_spendable_output(&nodes[0], &revoked_local_txn[0], false);
        assert_eq!(sorted_vec(vec![Balance::CounterpartyRevokedOutputClaimable {
                        // to_self output to B
-                       amount_satoshis: 10_000,
+                       amount_satoshis: 11_000,
                }, Balance::CounterpartyRevokedOutputClaimable { // HTLC 2
                        amount_satoshis: 1_000,
                }, Balance::ClaimableAwaitingConfirmations {
@@ -1574,7 +1574,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        test_spendable_output(&nodes[0], &as_htlc_claim_tx[0], false);
        assert_eq!(sorted_vec(vec![Balance::CounterpartyRevokedOutputClaimable {
                        // to_self output in B's revoked commitment
-                       amount_satoshis: 10_000,
+                       amount_satoshis: 11_000,
                }, Balance::CounterpartyRevokedOutputClaimable { // HTLC 2
                        amount_satoshis: 1_000,
                }]),
@@ -1623,7 +1623,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
        assert_eq!(sorted_vec(vec![Balance::CounterpartyRevokedOutputClaimable {
                        // to_self output in B's revoked commitment
-                       amount_satoshis: 10_000,
+                       amount_satoshis: 11_000,
                }, Balance::CounterpartyRevokedOutputClaimable { // HTLC 2
                        amount_satoshis: 1_000,
                }]),
@@ -1632,7 +1632,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
        mine_transaction(&nodes[0], &revoked_htlc_timeout_claim);
        assert_eq!(sorted_vec(vec![Balance::CounterpartyRevokedOutputClaimable {
                        // to_self output in B's revoked commitment
-                       amount_satoshis: 10_000,
+                       amount_satoshis: 11_000,
                }, Balance::ClaimableAwaitingConfirmations {
                        amount_satoshis: revoked_htlc_timeout_claim.output[0].value,
                        confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1,
index e25d8f06e45688446b13c2a5db68986ff6403423..8882af6836268f557a304796ee983f23fb750394 100644 (file)
@@ -16,9 +16,9 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER, LATE
 use crate::sign::EntropySource;
 use crate::chain::transaction::OutPoint;
 use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason, PaymentPurpose};
-use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS;
+use crate::ln::channel::{EXPIRE_PREV_CONFIG_TICKS, commit_tx_fee_msat, get_holder_selected_channel_reserve_satoshis, ANCHOR_OUTPUT_VALUE_SATOSHI};
 use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo};
-use crate::ln::features::Bolt11InvoiceFeatures;
+use crate::ln::features::{Bolt11InvoiceFeatures, ChannelTypeFeatures};
 use crate::ln::{msgs, ChannelId, PaymentSecret, PaymentPreimage};
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::ln::outbound_payment::{IDEMPOTENCY_TIMEOUT_TICKS, Retry};
@@ -4093,3 +4093,95 @@ fn test_payment_metadata_consistency() {
        do_test_payment_metadata_consistency(false, true);
        do_test_payment_metadata_consistency(false, false);
 }
+
+#[test]
+fn  test_htlc_forward_considers_anchor_outputs_value() {
+       // Tests that:
+       //
+       // 1) Forwarding nodes don't forward HTLCs that would cause their balance to dip below the
+       //    reserve when considering the value of anchor outputs.
+       //
+       // 2) Recipients of `update_add_htlc` properly reject HTLCs that would cause the initiator's
+       //    balance to dip below the reserve when considering the value of anchor outputs.
+       let mut config = test_default_channel_config();
+       config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+       config.manually_accept_inbound_channels = true;
+       config.channel_config.forwarding_fee_base_msat = 0;
+       config.channel_config.forwarding_fee_proportional_millionths = 0;
+
+       // Set up a test network of three nodes that replicates a production failure leading to the
+       // discovery of this bug.
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]);
+       let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       const CHAN_AMT: u64 = 1_000_000;
+       const PUSH_MSAT: u64 = 900_000_000;
+       create_announced_chan_between_nodes_with_value(&nodes, 0, 1, CHAN_AMT, 500_000_000);
+       let (_, _, chan_id_2, _) = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, CHAN_AMT, PUSH_MSAT);
+
+       let channel_reserve_msat = get_holder_selected_channel_reserve_satoshis(CHAN_AMT, &config) * 1000;
+       let commitment_fee_msat = commit_tx_fee_msat(
+               *nodes[1].fee_estimator.sat_per_kw.lock().unwrap(), 2, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()
+       );
+       let anchor_outpus_value_msat = ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000;
+       let sendable_balance_msat = CHAN_AMT * 1000 - PUSH_MSAT - channel_reserve_msat - commitment_fee_msat - anchor_outpus_value_msat;
+       let channel_details = nodes[1].node.list_channels().into_iter().find(|channel| channel.channel_id == chan_id_2).unwrap();
+       assert!(sendable_balance_msat >= channel_details.next_outbound_htlc_minimum_msat);
+       assert!(sendable_balance_msat <= channel_details.next_outbound_htlc_limit_msat);
+
+       send_payment(&nodes[0], &[&nodes[1], &nodes[2]], sendable_balance_msat);
+       send_payment(&nodes[2], &[&nodes[1], &nodes[0]], sendable_balance_msat);
+
+       // Send out an HTLC that would cause the forwarding node to dip below its reserve when
+       // considering the value of anchor outputs.
+       let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(
+               nodes[0], nodes[2], sendable_balance_msat + anchor_outpus_value_msat
+       );
+       nodes[0].node.send_payment_with_route(
+               &route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
+       ).unwrap();
+       check_added_monitors!(nodes[0], 1);
+
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       let mut update_add_htlc = if let MessageSendEvent::UpdateHTLCs { updates, .. } = events.pop().unwrap() {
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
+               check_added_monitors(&nodes[1], 0);
+               commitment_signed_dance!(nodes[1], nodes[0], &updates.commitment_signed, false);
+               updates.update_add_htlcs[0].clone()
+       } else {
+               panic!("Unexpected event");
+       };
+
+       // The forwarding node should reject forwarding it as expected.
+       expect_pending_htlcs_forwardable!(nodes[1]);
+       expect_pending_htlcs_forwardable_and_htlc_handling_failed!(&nodes[1], vec![HTLCDestination::NextHopChannel {
+               node_id: Some(nodes[2].node.get_our_node_id()),
+               channel_id: chan_id_2
+       }]);
+       check_added_monitors(&nodes[1], 1);
+
+       let mut events = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 1);
+       if let MessageSendEvent::UpdateHTLCs { updates, .. } = events.pop().unwrap() {
+               nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
+               check_added_monitors(&nodes[0], 0);
+               commitment_signed_dance!(nodes[0], nodes[1], &updates.commitment_signed, false);
+       } else {
+               panic!("Unexpected event");
+       }
+
+       expect_payment_failed!(nodes[0], payment_hash, false);
+
+       // Assume that the forwarding node did forward it, and make sure the recipient rejects it as an
+       // invalid update and closes the channel.
+       update_add_htlc.channel_id = chan_id_2;
+       nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &update_add_htlc);
+       check_closed_event(&nodes[2], 1, ClosureReason::ProcessingError {
+               err: "Remote HTLC add would put them under remote reserve value".to_owned()
+       }, false, &[nodes[1].node.get_our_node_id()], 1_000_000);
+       check_closed_broadcast(&nodes[2], 1, true);
+       check_added_monitors(&nodes[2], 1);
+}