diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 3bdd7816786..dddd97cae45 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -432,7 +432,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des } #[inline] -pub fn do_test(data: &[u8], underlying_out: Out) { +pub fn do_test(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(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(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(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(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(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(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(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(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 SearchingOutput { } pub fn chanmon_consistency_test(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); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6a468f2d6fe..f3632b41b47 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -724,7 +724,7 @@ pub(super) struct ChannelContext 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, pending_outbound_htlcs: Vec, holding_cell_htlc_updates: Vec, @@ -1673,6 +1673,11 @@ impl ChannelContext 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 ChannelContext 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 ChannelContext 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 Channel 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 Channel 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 Channel 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 OutboundV1Channel 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 InboundV1Channel 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 { diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 0399ed38251..dcc0d57409b 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -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, diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index e25d8f06e45..8882af68362 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -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); +}