diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e07ee7fceab..ed40ac79f1c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3777,6 +3777,14 @@ impl ChannelContext { "Funding must be smaller than the total bitcoin supply. It was {channel_value_satoshis}" ))); } + if !channel_type.supports_anchors_zero_fee_htlc_tx() + && !channel_type.supports_anchor_zero_fee_commitments() + && (msg_channel_reserve_satoshis == 0 || holder_selected_channel_reserve_satoshis == 0) + { + return Err(ChannelError::close(format!( + "0-reserve is not allowed on legacy channels" + ))); + } if msg_channel_reserve_satoshis > channel_value_satoshis { return Err(ChannelError::close(format!( "Bogus channel_reserve_satoshis ({msg_channel_reserve_satoshis}). Must be no greater than channel_value_satoshis: {channel_value_satoshis}" @@ -4198,6 +4206,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, channel_context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -4267,6 +4276,14 @@ impl ChannelContext { } let channel_type = get_initial_channel_type(&config, their_features); + if !channel_type.supports_anchors_zero_fee_htlc_tx() + && !channel_type.supports_anchor_zero_fee_commitments() + && holder_selected_channel_reserve_satoshis == 0 + { + return Err(APIError::APIMisuseError { + err: format!("0-reserve is not allowed on legacy channels"), + }); + } debug_assert!(!channel_type.supports_any_optional_bits()); debug_assert!(!channel_type .requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config))); @@ -4494,6 +4511,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, channel_context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| APIError::APIMisuseError { @@ -4812,6 +4830,15 @@ impl ChannelContext { } let channel_type = funding.get_channel_type(); + if !channel_type.supports_anchors_zero_fee_htlc_tx() + && !channel_type.supports_anchor_zero_fee_commitments() + && (channel_reserve_satoshis == 0 + || funding.holder_selected_channel_reserve_satoshis == 0) + { + return Err(ChannelError::close( + "0-reserve is not allowed on legacy channels".to_owned(), + )); + } if common_fields.max_accepted_htlcs > max_htlcs(channel_type) { return Err(ChannelError::close(format!( "max_accepted_htlcs was {}. It must not be larger than {}", @@ -5284,7 +5311,8 @@ impl ChannelContext { fn get_next_local_commitment_stats( &self, funding: &FundingScope, htlc_candidate: Option, include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize, - feerate_per_kw: u32, dust_exposure_limiting_feerate: Option, + feerate_per_kw: u32, assuming_future_increased_feerate: bool, + dust_exposure_limiting_feerate: Option, ) -> Result<(ChannelStats, Vec), ()> { let next_commitment_htlcs = self.get_next_commitment_htlcs( true, @@ -5306,6 +5334,7 @@ impl ChannelContext { &next_commitment_htlcs, addl_nondust_htlc_count, feerate_per_kw, + assuming_future_increased_feerate, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5330,6 +5359,7 @@ impl ChannelContext { &next_commitment_htlcs, 0, feerate_per_kw, + false, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5351,7 +5381,8 @@ impl ChannelContext { fn get_next_remote_commitment_stats( &self, funding: &FundingScope, htlc_candidate: Option, include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize, - feerate_per_kw: u32, dust_exposure_limiting_feerate: Option, + feerate_per_kw: u32, assuming_future_increased_feerate: bool, + dust_exposure_limiting_feerate: Option, ) -> Result<(ChannelStats, Vec), ()> { let next_commitment_htlcs = self.get_next_commitment_htlcs( false, @@ -5373,6 +5404,7 @@ impl ChannelContext { &next_commitment_htlcs, addl_nondust_htlc_count, feerate_per_kw, + assuming_future_increased_feerate, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5397,6 +5429,7 @@ impl ChannelContext { &next_commitment_htlcs, 0, feerate_per_kw, + false, dust_exposure_limiting_feerate, max_dust_htlc_exposure_msat, channel_constraints, @@ -5439,6 +5472,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5497,6 +5531,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5523,6 +5558,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, 0, new_feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5544,6 +5580,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, 0, new_feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5724,6 +5761,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, + false, dust_exposure_limiting_feerate, ) { stats @@ -5763,6 +5801,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, + false, dust_exposure_limiting_feerate, ) { stats @@ -5810,6 +5849,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, feerate, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5826,6 +5866,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, feerate, + false, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -5862,21 +5903,14 @@ impl ChannelContext { if !funding.is_outbound() { // Note that with anchor outputs we are no longer as sensitive to fee spikes, so we don't need // to account for them. - let fee_spike_multiple = - if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 - } else { - 1 - }; - // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let spiked_feerate = feerate.saturating_mul(fee_spike_multiple); let (remote_stats, _remote_htlcs) = self .get_next_remote_commitment_stats( funding, None, include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, - spiked_feerate, + feerate, + true, dust_exposure_limiting_feerate, ) .map_err(|()| { @@ -6231,6 +6265,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map(|(remote_stats, _)| remote_stats.available_balances)?; @@ -6252,6 +6287,7 @@ impl ChannelContext { include_counterparty_unknown_htlcs, addl_nondust_htlc_count, self.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .unwrap(); @@ -6473,17 +6509,15 @@ impl ChannelContext { /// If we receive an error message when attempting to open a channel, it may only be a rejection /// of the channel type we tried, not of our ability to open any channel at all. We can see if a /// downgrade of channel features would be possible so that we can still open the channel. - #[rustfmt::skip] pub(crate) fn maybe_downgrade_channel_features( &mut self, funding: &mut FundingScope, fee_estimator: &LowerBoundedFeeEstimator, user_config: &UserConfig, their_features: &InitFeatures, ) -> Result<(), ()> { - if !funding.is_outbound() || - !matches!( + if !funding.is_outbound() + || !matches!( self.channel_state, ChannelState::NegotiatingFunding(flags) if flags == NegotiatingFundingFlags::OUR_INIT_SENT - ) - { + ) { return Err(()); } if funding.get_channel_type() == &ChannelTypeFeatures::only_static_remote_key() { @@ -6514,11 +6548,17 @@ impl ChannelContext { } let next_channel_type = get_initial_channel_type(user_config, &eligible_features); + if !next_channel_type.supports_anchors_zero_fee_htlc_tx() + && !next_channel_type.supports_anchor_zero_fee_commitments() + && funding.holder_selected_channel_reserve_satoshis == 0 + { + // 0-reserve is not allowed on legacy channels + return Err(()); + } - self.feerate_per_kw = selected_commitment_sat_per_1000_weight( - &fee_estimator, &next_channel_type, - ); - funding.channel_transaction_parameters.channel_type_features = next_channel_type; + self.feerate_per_kw = + selected_commitment_sat_per_1000_weight(&fee_estimator, &next_channel_type); + funding.channel_transaction_parameters.channel_type_features = next_channel_type; Ok(()) } @@ -13372,16 +13412,6 @@ where // We are not interested in dust exposure let dust_exposure_limiting_feerate = None; - // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let feerate_per_kw = if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - // Similar to HTLC additions, require the funder to have enough funds reserved for - // fees such that the feerate can jump without rendering the channel useless. - let spike_mul = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; - self.context.feerate_per_kw.saturating_mul(spike_mul) - } else { - self.context.feerate_per_kw - }; - // Different dust limits on the local and remote commitments cause the commitment // transaction fee to be different depending on the commitment, so we grab the floor // of both balances across both commitments here. @@ -13399,7 +13429,8 @@ where None, // htlc_candidate include_counterparty_unknown_htlcs, addl_nondust_htlc_count, - feerate_per_kw, + self.context.feerate_per_kw, + true, dust_exposure_limiting_feerate, ) .map_err(|()| "Balance exhausted on local commitment")?; @@ -13411,7 +13442,8 @@ where None, // htlc_candidate include_counterparty_unknown_htlcs, addl_nondust_htlc_count, - feerate_per_kw, + self.context.feerate_per_kw, + true, dust_exposure_limiting_feerate, ) .map_err(|()| "Balance exhausted on remote commitment")?; @@ -13451,6 +13483,7 @@ where include_counterparty_unknown_htlcs, 0, self.context.feerate_per_kw, + false, dust_exposure_limiting_feerate, ) .map_err(|()| "Balance exhausted on remote commitment")?; @@ -17186,7 +17219,7 @@ mod tests { // Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass // the dust limit check. let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amount_msat, outbound: true }; - let local_commit_tx_fee = node_a_chan.context.get_next_local_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let local_commit_tx_fee = node_a_chan.context.get_next_local_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.funding.get_channel_type()) * 1000; assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs); @@ -17195,7 +17228,7 @@ mod tests { node_a_chan.funding.channel_transaction_parameters.is_outbound_from_holder = false; let remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.funding.get_channel_type()) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amount_msat, outbound: true }; - let remote_commit_tx_fee = node_a_chan.context.get_next_remote_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let remote_commit_tx_fee = node_a_chan.context.get_next_remote_commitment_stats(&node_a_chan.funding, Some(htlc_candidate), false, 0, node_a_chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs); } @@ -17230,13 +17263,13 @@ mod tests { // counted as dust when it shouldn't be. let htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.holder_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amt_above_timeout, outbound: true }; - let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); // If swapped: this HTLC would be counted as non-dust when it shouldn't be. let dust_htlc_amt_below_success = (htlc_success_tx_fee_sat + chan.context.holder_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: dust_htlc_amt_below_success, outbound: false }; - let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_local_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); chan.funding.channel_transaction_parameters.is_outbound_from_holder = false; @@ -17244,13 +17277,13 @@ mod tests { // If swapped: this HTLC would be counted as non-dust when it shouldn't be. let dust_htlc_amt_above_timeout = (htlc_timeout_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: dust_htlc_amt_above_timeout, outbound: true }; - let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); // If swapped: this HTLC would be counted as dust when it shouldn't be. let htlc_amt_below_success = (htlc_success_tx_fee_sat + chan.context.counterparty_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCAmountDirection { amount_msat: htlc_amt_below_success, outbound: false }; - let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; + let commitment_tx_fee = chan.context.get_next_remote_commitment_stats(&chan.funding, Some(htlc_candidate), false, 0, chan.context.feerate_per_kw, false, None).unwrap().0.commitment_stats.commit_tx_fee_sat * 1000; assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); } diff --git a/lightning/src/ln/channel_open_tests.rs b/lightning/src/ln/channel_open_tests.rs index ac4a1b67994..1e9f1dd4c78 100644 --- a/lightning/src/ln/channel_open_tests.rs +++ b/lightning/src/ln/channel_open_tests.rs @@ -23,7 +23,8 @@ use crate::ln::channelmanager::{ MAX_UNFUNDED_CHANS_PER_PEER, }; use crate::ln::msgs::{ - AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, MessageSendEvent, + AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, ErrorMessage, + MessageSendEvent, }; use crate::ln::types::ChannelId; use crate::ln::{functional_test_utils::*, msgs}; @@ -47,6 +48,7 @@ use bitcoin::{Amount, Sequence, Transaction, TxIn, TxOut, Witness}; use lightning_macros::xtest; use lightning_types::features::ChannelTypeFeatures; +use types::string::UntrustedString; #[test] fn test_outbound_chans_unlimited() { @@ -2488,3 +2490,219 @@ fn test_fund_pending_channel() { }; check_closed_event(&nodes[0], 1, reason, &[node_b_id], 100_000); } + +#[xtest(feature = "_externalize_tests")] +fn test_legacy_0reserve_is_not_allowed() { + { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut channel_config = test_default_channel_config(); + assert!(channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx); + let node_chanmgrs = create_node_chanmgrs( + 2, + &node_cfgs, + &[Some(channel_config.clone()), Some(channel_config.clone())], + ); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let _node_a_id = nodes[0].node.get_our_node_id(); + let node_b_id = nodes[1].node.get_our_node_id(); + + let mut legacy_channel_config = test_default_channel_config(); + legacy_channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + legacy_channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = + false; + + assert_eq!( + nodes[0] + .node + .create_channel_to_trusted_peer_0reserve( + node_b_id, + 100_000, + 0, + 42, + None, + Some(legacy_channel_config) + ) + .unwrap_err(), + APIError::APIMisuseError { + err: "0-reserve is not allowed on legacy channels".to_owned() + } + ); + } + { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut channel_config = test_default_channel_config(); + channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + let node_chanmgrs = create_node_chanmgrs( + 2, + &node_cfgs, + &[Some(channel_config.clone()), Some(channel_config.clone())], + ); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let _node_a_id = nodes[0].node.get_our_node_id(); + let node_b_id = nodes[1].node.get_our_node_id(); + + assert_eq!( + nodes[0] + .node + .create_channel_to_trusted_peer_0reserve(node_b_id, 100_000, 0, 42, None, None) + .unwrap_err(), + APIError::APIMisuseError { + err: "0-reserve is not allowed on legacy channels".to_owned() + } + ); + } + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut channel_config = test_default_channel_config(); + channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + let node_chanmgrs = create_node_chanmgrs( + 2, + &node_cfgs, + &[Some(channel_config.clone()), Some(channel_config.clone())], + ); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_a_id = nodes[0].node.get_our_node_id(); + let node_b_id = nodes[1].node.get_our_node_id(); + + nodes[0].node.create_channel(node_b_id, 100_000, 0, 42, None, None).unwrap(); + let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id); + let mut err_open_channel_msg = open_channel_msg.clone(); + err_open_channel_msg.channel_reserve_satoshis = 0; + assert_eq!( + err_open_channel_msg.common_fields.channel_type, + Some(ChannelTypeFeatures::only_static_remote_key()) + ); + + nodes[1].node.handle_open_channel(node_a_id, &err_open_channel_msg); + let events = nodes[1].node.get_and_clear_pending_events(); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + let error = nodes[1] + .node + .accept_inbound_channel_from_trusted_peer( + &temporary_channel_id, + &node_a_id, + 42, + TrustedChannelFeatures::ZeroReserve, + None, + ) + .unwrap_err(); + assert_eq!( + error, + APIError::ChannelUnavailable { + err: "0-reserve is not allowed on legacy channels".to_owned() + } + ); + }, + _ => panic!("Unexpected event"), + } + let err_msg = get_err_msg(&nodes[1], &node_a_id); + assert_eq!( + err_msg, + ErrorMessage { + channel_id: err_open_channel_msg.common_fields.temporary_channel_id, + data: "0-reserve is not allowed on legacy channels".to_string() + } + ); + + nodes[1].node.handle_open_channel(node_a_id, &err_open_channel_msg); + let events = nodes[1].node.get_and_clear_pending_events(); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + let error = nodes[1] + .node + .accept_inbound_channel(&temporary_channel_id, &node_a_id, 42, None) + .unwrap_err(); + assert_eq!( + error, + APIError::ChannelUnavailable { + err: "0-reserve is not allowed on legacy channels".to_owned() + } + ); + }, + _ => panic!("Unexpected event"), + } + let err_msg = get_err_msg(&nodes[1], &node_a_id); + assert_eq!( + err_msg, + ErrorMessage { + channel_id: err_open_channel_msg.common_fields.temporary_channel_id, + data: "0-reserve is not allowed on legacy channels".to_string() + } + ); + + handle_and_accept_open_channel(&nodes[1], node_a_id, &open_channel_msg); + let mut accept_channel_msg = + get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, node_a_id); + accept_channel_msg.channel_reserve_satoshis = 0; + + nodes[0].node.handle_accept_channel(node_b_id, &accept_channel_msg); + let err_msg = get_err_msg(&nodes[0], &node_b_id); + assert_eq!( + err_msg, + ErrorMessage { + channel_id: open_channel_msg.common_fields.temporary_channel_id, + data: "0-reserve is not allowed on legacy channels".to_string() + } + ); + let err = "0-reserve is not allowed on legacy channels".to_owned(); + let reason = ClosureReason::ProcessingError { err }; + let expected_closing = ExpectedCloseEvent::from_id_reason( + open_channel_msg.common_fields.temporary_channel_id, + false, + reason, + ); + check_closed_events(&nodes[0], &[expected_closing]); +} + +#[xtest(feature = "_externalize_tests")] +fn test_error_if_0reserve_negotiates_down_to_legacy() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let channel_config = test_default_channel_config(); + assert!(channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx); + let node_chanmgrs = create_node_chanmgrs( + 2, + &node_cfgs, + &[Some(channel_config.clone()), Some(channel_config.clone())], + ); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_b_id = nodes[1].node.get_our_node_id(); + + nodes[0] + .node + .create_channel_to_trusted_peer_0reserve(node_b_id, 100_000, 0, 42, None, None) + .unwrap(); + let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id); + assert_eq!( + open_channel_msg.common_fields.channel_type, + Some(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()) + ); + assert_eq!(open_channel_msg.channel_reserve_satoshis, 0); + + let reason = "Don't like your channel".to_owned(); + nodes[0].node.handle_error( + node_b_id, + &ErrorMessage { + channel_id: open_channel_msg.common_fields.temporary_channel_id, + data: reason.clone(), + }, + ); + + let reason = ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(reason) }; + let expected_closing = ExpectedCloseEvent::from_id_reason( + open_channel_msg.common_fields.temporary_channel_id, + false, + reason, + ); + check_closed_events(&nodes[0], &[expected_closing]); +} diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a7a0942f0c8..f7e510fc994 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3609,6 +3609,8 @@ pub enum TrustedChannelFeatures { /// with a revoked commitment transaction *for free*. /// /// Note that there is no guarantee that the counterparty accepts such a channel themselves. + /// + /// The zero-reserve feature is not allowed on legacy / anchorless channels. ZeroReserve, /// Sets the combination of [`TrustedChannelFeatures::ZeroConf`] and [`TrustedChannelFeatures::ZeroReserve`] ZeroConfZeroReserve, @@ -3865,6 +3867,8 @@ impl< /// transaction *for free*. /// /// Note that there is no guarantee that the counterparty accepts such a channel. + /// + /// The zero-reserve feature is not allowed on legacy / anchorless channels. pub fn create_channel_to_trusted_peer_0reserve( &self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_channel_id: u128, temporary_channel_id: Option, diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 45d3cf5950f..ac7fb4c1155 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -3,14 +3,15 @@ use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose}; use crate::ln::chan_utils::{ self, commit_tx_fee_sat, commitment_tx_base_weight, second_stage_tx_fees_sat, - shared_anchor_script_pubkey, CommitmentTransaction, COMMITMENT_TX_WEIGHT_PER_HTLC, - TRUC_CHILD_MAX_WEIGHT, + shared_anchor_script_pubkey, CommitmentTransaction, HTLCOutputInCommitment, + COMMITMENT_TX_WEIGHT_PER_HTLC, TRUC_CHILD_MAX_WEIGHT, }; use crate::ln::channel::{ get_holder_selected_channel_reserve_satoshis, Channel, ANCHOR_OUTPUT_VALUE_SATOSHI, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, MIN_CHAN_DUST_LIMIT_SATOSHIS, }; +use crate::ln::channel_state::ChannelDetails; use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, TrustedChannelFeatures}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; @@ -22,10 +23,11 @@ use crate::sign::ecdsa::EcdsaChannelSigner; use crate::sign::tx_builder::{SpecTxBuilder, TxBuilder}; use crate::sign::ChannelSigner; use crate::types::features::ChannelTypeFeatures; -use crate::types::payment::{PaymentHash, PaymentPreimage}; +use crate::types::payment::PaymentPreimage; use crate::util::config::UserConfig; use crate::util::errors::APIError; +use bolt11_invoice::PaymentHash; use lightning_macros::xtest; use bitcoin::secp256k1::{Secp256k1, SecretKey}; @@ -38,9 +40,7 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) { // in normal testing, we test it explicitly here. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let legacy_cfg = test_legacy_channel_config(); - let node_chanmgrs = - create_node_chanmgrs(2, &node_cfgs, &[Some(legacy_cfg.clone()), Some(legacy_cfg)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let node_a_id = nodes[0].node.get_our_node_id(); @@ -51,12 +51,13 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) { // Have node0 initiate a channel to node1 with aforementioned parameters let mut push_amt = 100_000_000; let feerate_per_kw = 253; - let channel_type_features = ChannelTypeFeatures::only_static_remote_key(); + let channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); push_amt -= feerate_per_kw as u64 * (commitment_tx_base_weight(&channel_type_features) + 4 * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 * 1000; push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000; + push_amt -= 2 * 330_000; let push = if send_from_initiator { 0 } else { push_amt }; let temp_channel_id = @@ -107,10 +108,8 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) { &nodes[0], &[&nodes[1]], 100_000_000 - // Note that for outbound channels we have to consider the commitment tx fee and the - // "fee spike buffer", which is currently a multiple of the total commitment tx fee as - // well as an additional HTLC. - - FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * commit_tx_fee_msat(feerate_per_kw, 2, &channel_type_features), + - commit_tx_fee_msat(feerate_per_kw, 2, &channel_type_features) + - 2 * 330_000, ); } else { send_payment(&nodes[1], &[&nodes[0]], push_amt); @@ -887,7 +886,7 @@ pub fn do_test_fee_spike_buffer(cfg: Option, htlc_fails: bool) { // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let accepted_htlc_info = HTLCOutputInCommitment { offered: false, amount_msat: payment_amt_msat, cltv_expiry: htlc_cltv, @@ -2142,7 +2141,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { let (_payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], HTLC_AMT_SAT * 1000); // Grab a snapshot of these HTLCs to manually build the commitment transaction later... - let accepted_htlc = chan_utils::HTLCOutputInCommitment { + let accepted_htlc = HTLCOutputInCommitment { offered: false, amount_msat: HTLC_AMT_SAT * 1000, // Hard-coded to match the expected value @@ -2256,7 +2255,7 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { &channel_type, ); - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { + let accepted_htlc_info = HTLCOutputInCommitment { offered: false, amount_msat: HTLC_AMT_SAT * 1000, cltv_expiry, @@ -2354,12 +2353,6 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) { fn test_create_channel_to_trusted_peer_0reserve() { let mut config = test_default_channel_config(); - // Legacy channels - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; - config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; - let channel_type = do_test_create_channel_to_trusted_peer_0reserve(config.clone()); - assert_eq!(channel_type, ChannelTypeFeatures::only_static_remote_key()); - // Anchor channels config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; @@ -2412,14 +2405,8 @@ fn do_test_create_channel_to_trusted_peer_0reserve(mut config: UserConfig) -> Ch } else { 0 }; - let spike_multiple = if channel_type == ChannelTypeFeatures::only_static_remote_key() { - FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 - } else { - 1 - }; - let spiked_feerate = spike_multiple * feerate_per_kw; let reserved_commit_tx_fee_sat = chan_utils::commit_tx_fee_sat( - spiked_feerate, + feerate_per_kw, 2, // We reserve space for two HTLCs, the next outbound non-dust HTLC, and the fee spike buffer HTLC &channel_type, ); @@ -2442,12 +2429,6 @@ fn do_test_create_channel_to_trusted_peer_0reserve(mut config: UserConfig) -> Ch fn test_accept_inbound_channel_from_trusted_peer_0reserve() { let mut config = test_default_channel_config(); - // Legacy channels - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; - config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; - let channel_type = do_test_accept_inbound_channel_from_trusted_peer_0reserve(config.clone()); - assert_eq!(channel_type, ChannelTypeFeatures::only_static_remote_key()); - // Anchor channels config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; @@ -2535,14 +2516,8 @@ fn do_test_accept_inbound_channel_from_trusted_peer_0reserve( } else { 0 }; - let spike_multiple = if channel_type == ChannelTypeFeatures::only_static_remote_key() { - FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 - } else { - 1 - }; - let spiked_feerate = spike_multiple * feerate_per_kw; let reserved_commit_tx_fee_sat = chan_utils::commit_tx_fee_sat( - spiked_feerate, + feerate_per_kw, 2, // We reserve space for two HTLCs, the next outbound non-dust HTLC, and the fee spike buffer HTLC &channel_type, ); @@ -2561,20 +2536,8 @@ fn do_test_accept_inbound_channel_from_trusted_peer_0reserve( channel_type } -enum LegacyChannelsNoOutputs { - PaymentSucceeds, - FailsReceiverUpdateAddHTLC, - FailsReceiverCanAcceptHTLCA, - FailsReceiverCanAcceptHTLCB, -} - #[xtest(feature = "_externalize_tests")] fn test_0reserve_no_outputs() { - do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::PaymentSucceeds); - do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCA); - do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCB); - do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::FailsReceiverUpdateAddHTLC); - do_test_0reserve_no_outputs_keyed_anchors(true); do_test_0reserve_no_outputs_keyed_anchors(false); @@ -2674,184 +2637,10 @@ pub(crate) fn setup_0reserve_no_outputs_channels<'a, 'b, 'c, 'd>( (channel_id, tx) } -fn do_test_0reserve_no_outputs_legacy(no_outputs_case: LegacyChannelsNoOutputs) { - let mut config = test_default_channel_config(); - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; - config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; - - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = - 100; - - let channel_type = ChannelTypeFeatures::only_static_remote_key(); - - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - - let node_a_id = nodes[0].node.get_our_node_id(); - let _node_b_id = nodes[1].node.get_our_node_id(); - - let feerate_per_kw = 253; - let spike_multiple = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; - let dust_limit_satoshis: u64 = 546; - let channel_value_sat = 1000; - - let (channel_id, _funding_tx) = - setup_0reserve_no_outputs_channels(&nodes, channel_value_sat, dust_limit_satoshis); - assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); - - // Sending the biggest dust HTLC possible trims our balance output! - let (timeout_tx_fee_sat, success_tx_fee_sat) = - second_stage_tx_fees_sat(&channel_type, spike_multiple * feerate_per_kw); - let max_dust_htlc_sat = dust_limit_satoshis + success_tx_fee_sat - 1; - assert!( - channel_value_sat - .saturating_sub(commit_tx_fee_sat(feerate_per_kw, 0, &channel_type)) - .saturating_sub(max_dust_htlc_sat) - < dust_limit_satoshis - ); - - // We can't afford the fee for an additional non-dust HTLC + the fee spike HTLC, so we can only send - // dust HTLCs... - let min_local_nondust_htlc_sat = dust_limit_satoshis + timeout_tx_fee_sat; - assert!( - channel_value_sat - commit_tx_fee_sat(spike_multiple * feerate_per_kw, 2, &channel_type) - < min_local_nondust_htlc_sat - ); - - // We cannot trim our own balance output, otherwise we'd have no outputs on the commitment. We must - // also reserve enough fees to pay for an incoming non-dust HTLC, aka the fee spike buffer HTLC. - let min_value_sat = core::cmp::max( - commit_tx_fee_sat(spike_multiple * feerate_per_kw, 0, &channel_type) + dust_limit_satoshis, - commit_tx_fee_sat(spike_multiple * feerate_per_kw, 1, &channel_type), - ); - // At this point the tighter requirement is "must have an output" - assert!( - commit_tx_fee_sat(spike_multiple * feerate_per_kw, 0, &channel_type) + dust_limit_satoshis - > commit_tx_fee_sat(spike_multiple * feerate_per_kw, 1, &channel_type) - ); - // But say at 9sat/vb with default dust limit, - // the tighter requirement is actually "must have funds for an inbound HTLC" ! - assert!( - commit_tx_fee_sat(9 * 250, 0, &channel_type) + 354 - < commit_tx_fee_sat(9 * 250, 1, &channel_type) - ); - let sender_amount_msat = (channel_value_sat - min_value_sat) * 1000; - let details_0 = &nodes[0].node.list_channels()[0]; - assert_eq!(details_0.next_outbound_htlc_minimum_msat, 1000); - assert_eq!(details_0.next_outbound_htlc_limit_msat, sender_amount_msat); - assert!(details_0.next_outbound_htlc_limit_msat > details_0.next_outbound_htlc_minimum_msat); - - let (sender_amount_msat, receiver_amount_msat) = match no_outputs_case { - LegacyChannelsNoOutputs::PaymentSucceeds => (sender_amount_msat, sender_amount_msat), - LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCA => { - // A dust HTLC with 1msat added to it will break counterparty `can_accept_incoming_htlc` - // validation, as this dust HTLC would push the holder's balance output below the - // dust limit at the spike multiple feerate. - (sender_amount_msat, sender_amount_msat + 1) - }, - LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCB => { - // In `validate_update_add_htlc`, we check that there is still some output present on - // the commitment given the *current* set of HTLCs, and the *current* feerate. So this - // HTLC will pass at `validate_update_add_htlc`, but will fail in - // `can_accept_incoming_htlc` due to failed fee spike buffer checks. - let receiver_amount_msat = (channel_value_sat - - commit_tx_fee_sat(feerate_per_kw, 0, &channel_type) - - dust_limit_satoshis) - * 1000; - (sender_amount_msat, receiver_amount_msat) - }, - LegacyChannelsNoOutputs::FailsReceiverUpdateAddHTLC => { - // Same value as above, just add 1msat, and this fails at `validate_update_add_htlc` - let receiver_amount_msat = (channel_value_sat - - commit_tx_fee_sat(feerate_per_kw, 0, &channel_type) - - dust_limit_satoshis) - * 1000; - (sender_amount_msat, receiver_amount_msat + 1) - }, - }; - - if let LegacyChannelsNoOutputs::PaymentSucceeds = no_outputs_case { - send_payment(&nodes[0], &[&nodes[1]], sender_amount_msat); - // Node 1 the fundee has 0-reserve too, so whatever they receive, they can send right back! - // Node 0 should *always* have the funds to cover the fee of a single non-dust HTLC from node 1. - assert_eq!( - nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, - sender_amount_msat - ); - send_payment(&nodes[1], &[&nodes[0]], sender_amount_msat); - } else { - let (route, payment_hash, _, payment_secret) = - get_route_and_payment_hash!(nodes[0], nodes[1], sender_amount_msat); - let secp_ctx = Secp256k1::new(); - let session_priv = SecretKey::from_slice(&[42; 32]).unwrap(); - let cur_height = nodes[0].node.best_block.read().unwrap().height + 1; - let onion_keys = - onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv); - let recipient_onion_fields = - RecipientOnionFields::secret_only(payment_secret, sender_amount_msat); - let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::test_build_onion_payloads( - &route.paths[0], - &recipient_onion_fields, - cur_height, - &None, - None, - None, - ) - .unwrap(); - assert_eq!(htlc_msat, sender_amount_msat); - let onion_packet = - onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash) - .unwrap(); - let msg = msgs::UpdateAddHTLC { - channel_id, - htlc_id: 0, - amount_msat: receiver_amount_msat, - payment_hash, - cltv_expiry: htlc_cltv, - onion_routing_packet: onion_packet, - skimmed_fee_msat: None, - blinding_point: None, - hold_htlc: None, - accountable: None, - }; - - nodes[1].node.handle_update_add_htlc(node_a_id, &msg); - - if let LegacyChannelsNoOutputs::FailsReceiverUpdateAddHTLC = no_outputs_case { - nodes[1].logger.assert_log_contains( - "lightning::ln::channelmanager", - "Remote HTLC add would overdraw remaining funds", - 3, - ); - assert_eq!(nodes[1].node.list_channels().len(), 0); - let err_msg = check_closed_broadcast(&nodes[1], 1, true).pop().unwrap(); - assert_eq!(err_msg.data, "Remote HTLC add would overdraw remaining funds"); - let reason = ClosureReason::ProcessingError { - err: "Remote HTLC add would overdraw remaining funds".to_string(), - }; - check_added_monitors(&nodes[1], 1); - check_closed_event(&nodes[1], 1, reason, &[node_a_id], channel_value_sat); - - return; - } - - manually_trigger_update_fail_htlc( - &nodes, - channel_id, - channel_value_sat, - dust_limit_satoshis, - receiver_amount_msat, - htlc_cltv, - payment_hash, - ); - } -} - fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( - nodes: &'a Vec>, channel_id: ChannelId, channel_value_sat: u64, - dust_limit_satoshis: u64, receiver_amount_msat: u64, htlc_cltv: u32, payment_hash: PaymentHash, + nodes: &'a Vec>, channel_id: ChannelId, value_to_self_msat: u64, + dust_limit_satoshis: u64, payment_hash: PaymentHash, + htlcs_in_commitment: Vec, can_afford_but_reserve_is_breached: bool, ) { let node_a_id = nodes[0].node.get_our_node_id(); let node_b_id = nodes[1].node.get_our_node_id(); @@ -2863,8 +2652,6 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( let feerate_per_kw = get_feerate!(nodes[0], nodes[1], channel_id); - const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; - let (local_secret, next_local_point) = { let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(&node_b_id).unwrap().lock().unwrap(); @@ -2872,36 +2659,29 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( chan_lock.channel_by_id.get(&channel_id).and_then(Channel::as_funded).unwrap(); let chan_signer = local_chan.get_signer(); // Make the signer believe we validated another commitment, so we can release the secret + let commit_number = chan_signer.get_enforcement_state().last_holder_commitment; chan_signer.get_enforcement_state().last_holder_commitment -= 1; ( - chan_signer.release_commitment_secret(INITIAL_COMMITMENT_NUMBER).unwrap(), - chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx).unwrap(), + chan_signer.release_commitment_secret(commit_number).unwrap(), + chan_signer.get_per_commitment_point(commit_number - 2, &secp_ctx).unwrap(), ) }; - let remote_point = { + let (remote_commit_number, remote_point) = { let per_peer_lock; let mut peer_state_lock; let channel = get_channel_ref!(nodes[1], nodes[0], per_peer_lock, peer_state_lock, channel_id); let chan_signer = channel.as_funded().unwrap().get_signer(); - chan_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap() + let commit_number = chan_signer.get_enforcement_state().last_holder_commitment; + let remote_point = + chan_signer.get_per_commitment_point(commit_number - 1, &secp_ctx).unwrap(); + (commit_number - 1, remote_point) }; // Build the remote commitment transaction so we can sign it, and then later use the // signature for the commitment_signed message. - let accepted_htlc_info = chan_utils::HTLCOutputInCommitment { - offered: false, - amount_msat: receiver_amount_msat, - cltv_expiry: htlc_cltv, - payment_hash, - transaction_output_index: Some(1), - }; - - let local_chan_balance_msat = channel_value_sat * 1000; - let commitment_number = INITIAL_COMMITMENT_NUMBER - 1; - let res = { let per_peer_lock; let mut peer_state_lock; @@ -2912,12 +2692,12 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( let (commitment_tx, _stats) = SpecTxBuilder {}.build_commitment_transaction( false, - commitment_number, + remote_commit_number, &remote_point, &channel.funding().channel_transaction_parameters, &secp_ctx, - local_chan_balance_msat, - vec![accepted_htlc_info], + value_to_self_msat, + htlcs_in_commitment, feerate_per_kw, dust_limit_satoshis, &nodes[0].logger, @@ -2968,11 +2748,13 @@ fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>( }, _ => panic!("Unexpected event"), }; - nodes[1].logger.assert_log( - "lightning::ln::channel", - "Attempting to fail HTLC due to balance exhausted on remote commitment".to_string(), - 1, - ); + let log_string = + if can_afford_but_reserve_is_breached { + String::from("Attempting to fail HTLC due to fee spike buffer violation. Rebalancing is required.") + } else { + String::from("Attempting to fail HTLC due to balance exhausted on remote commitment") + }; + nodes[1].logger.assert_log("lightning::ln::channel", log_string, 1); check_added_monitors(&nodes[1], 3); } @@ -3406,3 +3188,341 @@ fn test_0reserve_zero_conf_combined() { assert_eq!(node_1_max_htlc, node_0_max_htlc - node_1_reserve * 1000); send_payment(&nodes[1], &[&nodes[0]], node_1_max_htlc); } + +#[xtest(feature = "_externalize_tests")] +fn test_outbound_vs_available_capacity_outbound_htlc_limit_spiked_feerate() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let _node_a_id = nodes[0].node.get_our_node_id(); + let _node_b_id = nodes[1].node.get_our_node_id(); + + const FEERATE: u32 = 253; + const MULTIPLE: u32 = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; + const SPIKED_FEERATE: u32 = FEERATE * MULTIPLE; + const DUST_LIMIT_MSAT: u64 = 354 * 1000; + const CHANNEL_VALUE_MSAT: u64 = 10_000 * 1000; + const NODE_0_VALUE_TO_SELF_MSAT: u64 = 5000 * 1000; + const NODE_1_VALUE_TO_SELF_MSAT: u64 = 5000 * 1000; + const CHANNEL_RESERVE_MSAT: u64 = 1000 * 1000; + + // Find the HTLC amount that will be non-dust at the current feerate, but dust at the spiked feerate + const SPIKED_DUST_HTLC_MSAT: u64 = 688 * 1000; + const HTLC_SPIKE_DUST_LIMIT_MSAT: u64 = 689 * 1000; + let htlc_timeout_spike_tx_fee_msat = + second_stage_tx_fees_sat(&channel_type, SPIKED_FEERATE).1 * 1000; + assert_eq!(HTLC_SPIKE_DUST_LIMIT_MSAT, DUST_LIMIT_MSAT + htlc_timeout_spike_tx_fee_msat); + + let channel_id = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, CHANNEL_VALUE_MSAT / 1000, 0) + .2; + assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); + { + // Quick double-check on the dust limit to make sure HTLCs would be dust at 2x the + // feerate... + let mut per_peer_lock; + let mut peer_state_lock; + + let channel = + get_channel_ref!(nodes[0], nodes[1], per_peer_lock, peer_state_lock, channel_id); + assert_eq!(channel.context().holder_dust_limit_satoshis * 1000, DUST_LIMIT_MSAT); + } + + // Balance the channel so each side has 5_000 sats + send_payment(&nodes[0], &[&nodes[1]], NODE_1_VALUE_TO_SELF_MSAT); + + let count_total_htlcs = |details: &ChannelDetails| { + details.pending_outbound_htlcs.len() + details.pending_inbound_htlcs.len() + }; + let count_node_0_nondust_htlcs = || { + let mut txs = get_local_commitment_txn!(nodes[0], channel_id); + let commitment_tx = &txs[0]; + commitment_tx + .output + .iter() + .filter(|output| output.value.to_sat() * 1000 == SPIKED_DUST_HTLC_MSAT) + .count() + }; + let count_node_1_nondust_htlcs = || { + let mut txs = get_local_commitment_txn!(nodes[1], channel_id); + let commitment_tx = &txs[0]; + commitment_tx + .output + .iter() + .filter(|output| output.value.to_sat() * 1000 == SPIKED_DUST_HTLC_MSAT) + .count() + }; + + // Sanity check + { + let reserved_fee_sat = commit_tx_fee_sat(SPIKED_FEERATE, 2, &channel_type); + let node_0_outbound_capacity_msat = NODE_0_VALUE_TO_SELF_MSAT - CHANNEL_RESERVE_MSAT; + let node_0_available_capacity_msat = + node_0_outbound_capacity_msat - reserved_fee_sat * 1000; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + assert_eq!(count_total_htlcs(&node_0_details), 0); + assert_eq!(count_node_0_nondust_htlcs(), 0); + } + + // Route 2 688sat HTLCs from node 0 to node 1 + for i in 1..3 { + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + + let max_reserved_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 2 + i, &channel_type) * 1000; + let node_0_outbound_capacity_msat = + NODE_0_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * i as u64 - CHANNEL_RESERVE_MSAT; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - max_reserved_fee_msat; + // Node 0 can send non-dust HTLCs throughout + assert!(node_0_available_capacity_msat >= HTLC_SPIKE_DUST_LIMIT_MSAT); + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + assert_eq!(count_total_htlcs(&node_0_details), i); + assert_eq!(count_node_0_nondust_htlcs(), i); + } + + let node_0_details = &nodes[0].node.list_channels()[0]; + let local_nondust_htlc_count = 2; + assert_eq!(count_total_htlcs(&node_0_details), local_nondust_htlc_count); + assert_eq!(count_node_0_nondust_htlcs(), local_nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), local_nondust_htlc_count); + + let node_0_outbound_capacity_msat = node_0_details.outbound_capacity_msat; + + // Route 2 688sat HTLCs from node 1 to node 0 + for i in 1..3 { + route_payment(&nodes[1], &[&nodes[0]], SPIKED_DUST_HTLC_MSAT); + + let node_1_outbound_capacity_msat = + NODE_1_VALUE_TO_SELF_MSAT - SPIKED_DUST_HTLC_MSAT * i as u64 - CHANNEL_RESERVE_MSAT; + assert!(node_1_outbound_capacity_msat >= HTLC_SPIKE_DUST_LIMIT_MSAT); + let node_1_details = &nodes[1].node.list_channels()[0]; + assert_eq!(node_1_details.outbound_capacity_msat, node_1_outbound_capacity_msat); + assert_eq!(node_1_details.next_outbound_htlc_limit_msat, node_1_outbound_capacity_msat); + + let nondust_htlc_count = 2 + i; + // At the current feerate, 688sat HTLCs are present on both commitments + assert_eq!(count_node_0_nondust_htlcs(), nondust_htlc_count); + assert_eq!(count_node_1_nondust_htlcs(), nondust_htlc_count); + + assert_eq!( + nodes[0].node.list_channels()[0].outbound_capacity_msat, + node_0_outbound_capacity_msat + ); + let max_reserved_fee_msat = + commit_tx_fee_sat(SPIKED_FEERATE, nondust_htlc_count + 2, &channel_type) * 1000; + assert_eq!( + nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat, + node_0_outbound_capacity_msat - max_reserved_fee_msat + ); + } +} + +/// Make sure that we do not account for HTLCs going from non-dust to dust at the spiked feerate +/// when checking the fee spike buffer in `can_accept_incoming_htlc`. This is required to make sure +/// that we can afford *any* increase in the feerate between 1x to 2x, instead of checking whether +/// we can afford only the 2x increase in the feerate. +#[xtest(feature = "_externalize_tests")] +fn test_fail_cannot_afford_dust_htlcs_at_spike_multiple_if_nondust_at_base_feerate() { + let mut config = test_default_channel_config(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage = + 100; + + let channel_type = ChannelTypeFeatures::only_static_remote_key(); + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_a_id = nodes[0].node.get_our_node_id(); + let _node_b_id = nodes[1].node.get_our_node_id(); + + const FEERATE: u32 = 253; + const MULTIPLE: u32 = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32; + const SPIKED_FEERATE: u32 = FEERATE * MULTIPLE; + const DUST_LIMIT_MSAT: u64 = 354 * 1000; + const CHANNEL_VALUE_MSAT: u64 = 10_000 * 1000; + const NODE_0_VALUE_TO_SELF_MSAT: u64 = 5_000 * 1000; + const NODE_1_VALUE_TO_SELF_MSAT: u64 = 5_000 * 1000; + const CHANNEL_RESERVE_MSAT: u64 = 1_000 * 1_000; + + let channel_id = create_announced_chan_between_nodes_with_value( + &nodes, + 0, + 1, + CHANNEL_VALUE_MSAT / 1000, + NODE_1_VALUE_TO_SELF_MSAT, + ) + .2; + assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type); + + // Find the HTLC amount that will be non-dust at the current feerate, + // but dust at the spiked feerate. + const SPIKED_DUST_HTLC_MSAT: u64 = 688 * 1000; + const HTLC_SPIKE_DUST_LIMIT_MSAT: u64 = 689 * 1000; + // When checking the fee spike buffer in `can_accept_incoming_htlc`, we check the remote + // commitment, hence inbound HTLCs will be offered HTLCs, and use the timeout dust limit. + let htlc_timeout_spike_tx_fee_msat = + second_stage_tx_fees_sat(&channel_type, SPIKED_FEERATE).1 * 1000; + assert_eq!(HTLC_SPIKE_DUST_LIMIT_MSAT, DUST_LIMIT_MSAT + htlc_timeout_spike_tx_fee_msat); + + // Calculate here the dust limit at the current feerate so we know when node 0 cannot send + // any further non-dust HTLCs at the current feerate. + let htlc_timeout_tx_fee_msat = second_stage_tx_fees_sat(&channel_type, FEERATE).1 * 1000; + let htlc_dust_limit_msat = DUST_LIMIT_MSAT + htlc_timeout_tx_fee_msat; + // Make sure the HTLC will be non-dust at the current feerate + assert!(SPIKED_DUST_HTLC_MSAT > htlc_dust_limit_msat); + + // Place a few non-dust HTLCs on the commitment, these HTLCs would get trimmed upon a 2x + // increase in the feerate. + let mut sent_htlcs_count: usize = 0; + let mut payment_hashes = Vec::new(); + while nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat >= htlc_dust_limit_msat { + let (_preimage, hash, _secret, _id) = + route_payment(&nodes[0], &[&nodes[1]], SPIKED_DUST_HTLC_MSAT); + payment_hashes.push(hash); + sent_htlcs_count += 1; + } + assert_eq!(sent_htlcs_count, 4); + + // Check the outbound and available capacities + let node_0_outbound_capacity_msat = NODE_0_VALUE_TO_SELF_MSAT + - sent_htlcs_count as u64 * SPIKED_DUST_HTLC_MSAT + - CHANNEL_RESERVE_MSAT; + let node_0_details = &nodes[0].node.list_channels()[0]; + assert_eq!(node_0_details.outbound_capacity_msat, node_0_outbound_capacity_msat); + // Node 0 can now only send dust HTLCs, so we reserve the fees for a single additional + // inbound non-dust HTLC. + let min_reserved_fee_msat = + commit_tx_fee_sat(SPIKED_FEERATE, sent_htlcs_count + 1, &channel_type) * 1000; + let node_0_available_capacity_msat = node_0_outbound_capacity_msat - min_reserved_fee_msat; + assert_eq!(node_0_details.next_outbound_htlc_limit_msat, node_0_available_capacity_msat); + + // Then send an identical, 5th non-dust HTLC, bypass the validation from the holder, and + // check that the counterparty fails it due to a fee spike buffer violation. + + // First check the maths + + // Node 0 can afford an exact 2x increase in the feerate + let spiked_commit_tx_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 0, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(spiked_commit_tx_fee_msat) + .is_some()); + // Node 0 can afford a 5th non-dust HTLC at the current feerate, so `update_add_htlc` + // validation will pass. + let real_commit_tx_fee_msat = commit_tx_fee_sat(FEERATE, 5, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(real_commit_tx_fee_msat) + .is_some()); + // But we don't account for the HTLC trimming effect of the spike multiple feerate increase, + // so the 5th HTLC should be rejected at `can_accept_incoming_htlc`! + let expected_commit_tx_fee_msat = commit_tx_fee_sat(SPIKED_FEERATE, 5, &channel_type) * 1000; + assert!((node_0_outbound_capacity_msat - SPIKED_DUST_HTLC_MSAT) + .checked_sub(expected_commit_tx_fee_msat) + .is_none()); + + // Then run the experiment + + let sender_amount_msat = node_0_available_capacity_msat; + let receiver_amount_msat = SPIKED_DUST_HTLC_MSAT; + let (route, payment_hash, _, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], sender_amount_msat); + let secp_ctx = Secp256k1::new(); + let session_priv = SecretKey::from_slice(&[42; 32]).unwrap(); + let cur_height = nodes[0].node.best_block.read().unwrap().height + 1; + let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv); + let recipient_onion_fields = + RecipientOnionFields::secret_only(payment_secret, sender_amount_msat); + let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::test_build_onion_payloads( + &route.paths[0], + &recipient_onion_fields, + cur_height, + &None, + None, + None, + ) + .unwrap(); + assert_eq!(htlc_msat, sender_amount_msat); + let onion_packet = + onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash) + .unwrap(); + let msg = msgs::UpdateAddHTLC { + channel_id, + htlc_id: sent_htlcs_count as u64, + amount_msat: receiver_amount_msat, + payment_hash, + cltv_expiry: htlc_cltv, + onion_routing_packet: onion_packet, + skimmed_fee_msat: None, + blinding_point: None, + hold_htlc: None, + accountable: None, + }; + + nodes[1].node.handle_update_add_htlc(node_a_id, &msg); + + let htlcs_in_tx = vec![ + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x75).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(0), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x64).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(1), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash, + amount_msat: 688_000, + transaction_output_index: Some(2), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x72).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(3), + }, + HTLCOutputInCommitment { + offered: false, + cltv_expiry: 81, + payment_hash: payment_hashes.iter().find(|hash| hash.0[0] == 0x66).unwrap().clone(), + amount_msat: 688_000, + transaction_output_index: Some(4), + }, + ]; + + manually_trigger_update_fail_htlc( + &nodes, + channel_id, + NODE_0_VALUE_TO_SELF_MSAT, + DUST_LIMIT_MSAT / 1000, + payment_hash, + htlcs_in_tx, + true, + ); +} diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a5361358653..5b72c1db50b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -7482,20 +7482,6 @@ fn test_no_disconnect_after_quiescence_on_reconnect() { #[test] fn test_0reserve_splice() { let mut config = test_default_channel_config(); - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; - config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; - let a = do_test_0reserve_splice_holder_validation(false, false, false, config.clone()); - let _b = do_test_0reserve_splice_holder_validation(true, false, false, config.clone()); - let _c = do_test_0reserve_splice_holder_validation(false, true, false, config.clone()); - let _d = do_test_0reserve_splice_holder_validation(true, true, false, config.clone()); - - let _e = do_test_0reserve_splice_holder_validation(false, false, true, config.clone()); - let _f = do_test_0reserve_splice_holder_validation(true, false, true, config.clone()); - let _g = do_test_0reserve_splice_holder_validation(false, true, true, config.clone()); - let _h = do_test_0reserve_splice_holder_validation(true, true, true, config.clone()); - - assert_eq!(a, ChannelTypeFeatures::only_static_remote_key()); - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; let a = do_test_0reserve_splice_holder_validation(false, false, false, config.clone()); @@ -7525,20 +7511,6 @@ fn test_0reserve_splice() { assert_eq!(a, ChannelTypeFeatures::anchors_zero_fee_commitments()); let mut config = test_default_channel_config(); - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false; - config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; - let a = do_test_0reserve_splice_counterparty_validation(false, false, false, config.clone()); - let _b = do_test_0reserve_splice_counterparty_validation(true, false, false, config.clone()); - let _c = do_test_0reserve_splice_counterparty_validation(false, true, false, config.clone()); - let _d = do_test_0reserve_splice_counterparty_validation(true, true, false, config.clone()); - - let _e = do_test_0reserve_splice_counterparty_validation(false, false, true, config.clone()); - let _f = do_test_0reserve_splice_counterparty_validation(true, false, true, config.clone()); - let _g = do_test_0reserve_splice_counterparty_validation(false, true, true, config.clone()); - let _h = do_test_0reserve_splice_counterparty_validation(true, true, true, config.clone()); - - assert_eq!(a, ChannelTypeFeatures::only_static_remote_key()); - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false; let a = do_test_0reserve_splice_counterparty_validation(false, false, false, config.clone()); @@ -7597,11 +7569,6 @@ fn do_test_0reserve_splice_holder_validation( let feerate = if channel_type == ChannelTypeFeatures::anchors_zero_fee_commitments() { 0 } else { 253 }; - let spiked_feerate = if channel_type == ChannelTypeFeatures::only_static_remote_key() { - feerate * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 - } else { - feerate - }; let anchors_sat = if channel_type == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 @@ -7613,7 +7580,7 @@ fn do_test_0reserve_splice_holder_validation( send_payment(&nodes[0], &[&nodes[1]], channel_value_sat / 2 * 1_000); channel_value_sat / 2 } else if !node_0_is_initiator { - let tx_fee_msat = chan_utils::commit_tx_fee_sat(spiked_feerate, 2, &channel_type) * 1000; + let tx_fee_msat = chan_utils::commit_tx_fee_sat(feerate, 2, &channel_type) * 1000; let node_0_details = &nodes[0].node.list_channels()[0]; let outbound_capacity_msat = node_0_details.outbound_capacity_msat; let available_capacity_msat = node_0_details.next_outbound_htlc_limit_msat; @@ -7650,12 +7617,12 @@ fn do_test_0reserve_splice_holder_validation( // The estimated fees to splice out a single output at 253sat/kw let estimated_fees_sat = 183; let mut splice_out_max_value = if counterparty_has_output && node_0_is_initiator { - let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 1, &channel_type); + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(feerate, 1, &channel_type); Amount::from_sat( initiator_value_to_self_sat - commit_tx_fee_sat - anchors_sat - estimated_fees_sat, ) } else if !counterparty_has_output && node_0_is_initiator { - let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 0, &channel_type); + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(feerate, 0, &channel_type); Amount::from_sat( initiator_value_to_self_sat - commit_tx_fee_sat @@ -7744,11 +7711,6 @@ fn do_test_0reserve_splice_counterparty_validation( let feerate = if channel_type == ChannelTypeFeatures::anchors_zero_fee_commitments() { 0 } else { 253 }; - let spiked_feerate = if channel_type == ChannelTypeFeatures::only_static_remote_key() { - feerate * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 - } else { - feerate - }; let anchors_sat = if channel_type == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 @@ -7760,7 +7722,7 @@ fn do_test_0reserve_splice_counterparty_validation( send_payment(&nodes[0], &[&nodes[1]], channel_value_sat / 2 * 1_000); channel_value_sat / 2 } else if !node_0_is_initiator { - let tx_fee_msat = chan_utils::commit_tx_fee_sat(spiked_feerate, 2, &channel_type) * 1000; + let tx_fee_msat = chan_utils::commit_tx_fee_sat(feerate, 2, &channel_type) * 1000; let node_0_details = &nodes[0].node.list_channels()[0]; let outbound_capacity_msat = node_0_details.outbound_capacity_msat; let available_capacity_msat = node_0_details.next_outbound_htlc_limit_msat; @@ -7772,7 +7734,7 @@ fn do_test_0reserve_splice_counterparty_validation( let node_0_to_local_output_msat = channel_value_sat * 1000 - available_capacity_msat - anchors_sat * 1000 - - chan_utils::commit_tx_fee_sat(spiked_feerate, 0, &channel_type) * 1000; + - chan_utils::commit_tx_fee_sat(feerate, 0, &channel_type) * 1000; assert!(node_0_to_local_output_msat / 1000 < dust_limit_satoshis); let commit_tx = &get_local_commitment_txn!(nodes[0], channel_id)[0]; assert_eq!( @@ -7795,10 +7757,10 @@ fn do_test_0reserve_splice_counterparty_validation( }; let mut splice_out_value_incl_fees = if counterparty_has_output && node_0_is_initiator { - let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 1, &channel_type); + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(feerate, 1, &channel_type); Amount::from_sat(initiator_value_to_self_sat - commit_tx_fee_sat - anchors_sat) } else if !counterparty_has_output && node_0_is_initiator { - let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(spiked_feerate, 0, &channel_type); + let commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(feerate, 0, &channel_type); Amount::from_sat( initiator_value_to_self_sat - commit_tx_fee_sat - anchors_sat - dust_limit_satoshis, ) diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index ffb01c571b7..74300cab6ab 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -11,7 +11,7 @@ use crate::ln::chan_utils::{ }; use crate::ln::channel::{ get_v2_channel_reserve_satoshis, CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI, - MIN_CHANNEL_VALUE_SATOSHIS, + FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_CHANNEL_VALUE_SATOSHIS, }; use crate::prelude::*; use crate::types::features::ChannelTypeFeatures; @@ -219,7 +219,7 @@ fn has_output( fn get_next_commitment_stats( local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, next_commitment_htlcs: &[HTLCAmountDirection], - addl_nondust_htlc_count: usize, feerate_per_kw: u32, + addl_nondust_htlc_count: usize, feerate_per_kw: u32, assuming_future_increased_feerate: bool, dust_exposure_limiting_feerate: Option, broadcaster_dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures, ) -> Result { @@ -270,11 +270,17 @@ fn get_next_commitment_stats( channel_type, ); - // Calculate fees on commitment transaction - let nondust_htlc_count = next_commitment_htlcs + let spiked_feerate = + if assuming_future_increased_feerate && !channel_type.supports_anchors_zero_fee_htlc_tx() { + feerate_per_kw.saturating_mul(FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32) + } else { + feerate_per_kw + }; + + let spiked_nondust_htlc_count = next_commitment_htlcs .iter() .filter(|htlc| { - !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + !htlc.is_dust(local, spiked_feerate, broadcaster_dust_limit_satoshis, channel_type) }) .count(); @@ -284,8 +290,8 @@ fn get_next_commitment_stats( is_outbound_from_holder, holder_balance_before_fee_msat, counterparty_balance_before_fee_msat, - feerate_per_kw, - nondust_htlc_count, + spiked_feerate, + spiked_nondust_htlc_count, broadcaster_dust_limit_satoshis, channel_type, ) { @@ -295,8 +301,17 @@ fn get_next_commitment_stats( // 2) Now including any additional non-dust HTLCs (usually the fee spike buffer HTLC), does the funder cover // this bigger transaction fee ? The funder can dip below their dust limit to cover this case, as the // commitment will have at least one output: the non-dust fee spike buffer HTLC offered by the counterparty. + let nondust_htlc_count = next_commitment_htlcs + .iter() + .filter(|htlc| { + !htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_satoshis, channel_type) + }) + .count(); + // Note here we use the htlc count at the current feerate together with the spiked feerate; + // this makes sure that the holder can afford any fee bump between 1x to 2x from the current + // feerate if the fee spike multiple is included. let commit_tx_fee_sat = commit_tx_fee_sat( - feerate_per_kw, + spiked_feerate, nondust_htlc_count + addl_nondust_htlc_count, channel_type, ); @@ -335,9 +350,9 @@ fn get_next_commitment_stats( // 3) s < (100h + 100 - 100d - c) / 99 fn get_next_splice_out_maximum_sat( is_outbound_from_holder: bool, channel_value_satoshis: u64, local_balance_before_fee_msat: u64, - remote_balance_before_fee_msat: u64, spiked_feerate: u32, - spiked_feerate_nondust_htlc_count: usize, post_splice_delta_above_reserve_sat: u64, - channel_constraints: &ChannelConstraints, channel_type: &ChannelTypeFeatures, + remote_balance_before_fee_msat: u64, feerate_per_kw: u32, nondust_htlc_count: usize, + post_splice_delta_above_reserve_sat: u64, channel_constraints: &ChannelConstraints, + channel_type: &ChannelTypeFeatures, ) -> u64 { let local_balance_before_fee_sat = local_balance_before_fee_msat / 1000; let mut next_splice_out_maximum_sat = if channel_constraints @@ -393,23 +408,23 @@ fn get_next_splice_out_maximum_sat( // // If the current `next_splice_out_maximum_sat` would produce a local commitment with no // outputs, bump this maximum such that, after the splice, the holder's balance covers at - // least `dust_limit_satoshis` and, if they are the funder, `current_spiked_tx_fee_sat`. - // We don't include an additional non-dust inbound HTLC in the `current_spiked_tx_fee_sat`, + // least `dust_limit_satoshis` and, if they are the funder, `current_tx_fee_sat`. + // We don't include an additional non-dust inbound HTLC in the `current_tx_fee_sat`, // because we don't mind if the holder dips below their dust limit to cover the fee for that // inbound non-dust HTLC. if !has_output( is_outbound_from_holder, local_balance_before_fee_msat.saturating_sub(next_splice_out_maximum_sat * 1000), remote_balance_before_fee_msat, - spiked_feerate, - spiked_feerate_nondust_htlc_count, + feerate_per_kw, + nondust_htlc_count, channel_constraints.holder_dust_limit_satoshis, channel_type, ) { let dust_limit_satoshis = channel_constraints.holder_dust_limit_satoshis; - let current_spiked_tx_fee_sat = commit_tx_fee_sat(spiked_feerate, 0, channel_type); + let current_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, 0, channel_type); let min_balance_sat = if is_outbound_from_holder { - dust_limit_satoshis.saturating_add(current_spiked_tx_fee_sat) + dust_limit_satoshis.saturating_add(current_tx_fee_sat) } else { dust_limit_satoshis }; @@ -446,25 +461,28 @@ fn get_available_balances( if channel_type.supports_anchor_zero_fee_commitments() { 0 } else { 1 }; // Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop - let spiked_feerate = feerate_per_kw.saturating_mul( - if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() { + let spiked_feerate = + feerate_per_kw.saturating_mul(if !channel_type.supports_anchors_zero_fee_htlc_tx() { crate::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32 } else { 1 - }, - ); + }); let local_nondust_htlc_count = pending_htlcs .iter() .filter(|htlc| { !htlc.is_dust( true, - spiked_feerate, + feerate_per_kw, channel_constraints.holder_dust_limit_satoshis, channel_type, ) }) .count(); + + // Note here we use the htlc count at the current feerate together with the spiked feerate; + // this makes sure that the holder can afford any fee bump between 1x to 2x from the current + // feerate. let local_max_commit_tx_fee_sat = commit_tx_fee_sat( spiked_feerate, local_nondust_htlc_count + fee_spike_buffer_htlc + 1, @@ -526,8 +544,8 @@ fn get_available_balances( channel_value_satoshis, local_balance_before_fee_msat, remote_balance_before_fee_msat, - spiked_feerate, - // The number of non-dust HTLCs on the local commitment at the spiked feerate + feerate_per_kw, + // The number of non-dust HTLCs on the local commitment at the current feerate local_nondust_htlc_count, // The post-splice minimum balance of the holder if is_outbound_from_holder { local_min_commit_tx_fee_sat } else { 0 }, @@ -659,19 +677,7 @@ fn get_available_balances( } // Now adjust our min and max size HTLC to make sure both the local and the remote commitments still have - // at least one output at the spiked feerate. - - let remote_nondust_htlc_count = pending_htlcs - .iter() - .filter(|htlc| { - !htlc.is_dust( - false, - spiked_feerate, - channel_constraints.counterparty_dust_limit_satoshis, - channel_type, - ) - }) - .count(); + // at least one output at the current feerate. let (next_outbound_htlc_minimum_msat, available_capacity_msat) = adjust_boundaries_if_max_dust_htlc_produces_no_output( @@ -679,8 +685,8 @@ fn get_available_balances( is_outbound_from_holder, local_balance_before_fee_msat, remote_balance_before_fee_msat, + feerate_per_kw, local_nondust_htlc_count, - spiked_feerate, channel_constraints.holder_dust_limit_satoshis, channel_type, next_outbound_htlc_minimum_msat, @@ -693,8 +699,8 @@ fn get_available_balances( is_outbound_from_holder, local_balance_before_fee_msat, remote_balance_before_fee_msat, + feerate_per_kw, remote_nondust_htlc_count, - spiked_feerate, channel_constraints.counterparty_dust_limit_satoshis, channel_type, next_outbound_htlc_minimum_msat, @@ -715,13 +721,13 @@ fn get_available_balances( fn adjust_boundaries_if_max_dust_htlc_produces_no_output( local: bool, is_outbound_from_holder: bool, holder_balance_before_fee_msat: u64, - counterparty_balance_before_fee_msat: u64, nondust_htlc_count: usize, spiked_feerate: u32, + counterparty_balance_before_fee_msat: u64, feerate_per_kw: u32, nondust_htlc_count: usize, dust_limit_satoshis: u64, channel_type: &ChannelTypeFeatures, next_outbound_htlc_minimum_msat: u64, available_capacity_msat: u64, ) -> (u64, u64) { // First, determine the biggest dust HTLC we could send let (htlc_success_tx_fee_sat, htlc_timeout_tx_fee_sat) = - second_stage_tx_fees_sat(channel_type, spiked_feerate); + second_stage_tx_fees_sat(channel_type, feerate_per_kw); let min_nondust_htlc_sat = dust_limit_satoshis + if local { htlc_timeout_tx_fee_sat } else { htlc_success_tx_fee_sat }; let max_dust_htlc_msat = (min_nondust_htlc_sat.saturating_mul(1000)).saturating_sub(1); @@ -732,7 +738,7 @@ fn adjust_boundaries_if_max_dust_htlc_produces_no_output( is_outbound_from_holder, holder_balance_before_fee_msat.saturating_sub(max_dust_htlc_msat), counterparty_balance_before_fee_msat, - spiked_feerate, + feerate_per_kw, nondust_htlc_count, dust_limit_satoshis, channel_type, @@ -750,16 +756,15 @@ fn adjust_boundaries_if_max_dust_htlc_produces_no_output( // Note that this will be a dust HTLC. } else { // Remember we've got no non-dust HTLCs on the commitment here - let current_spiked_tx_fee_sat = commit_tx_fee_sat(spiked_feerate, 0, channel_type); - let spike_buffer_tx_fee_sat = commit_tx_fee_sat(spiked_feerate, 1, channel_type); + let current_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, 0, channel_type); + let spike_buffer_tx_fee_sat = commit_tx_fee_sat(feerate_per_kw, 1, channel_type); // In case we are the funder, we must cover the greater of - // 1) The dust_limit_satoshis plus the fee of the existing commitment at the spiked feerate. + // 1) The dust_limit_satoshis plus the fee of the existing commitment at the current feerate. // 2) The fee of the commitment with an additional non-dust HTLC, aka the fee spike buffer HTLC. // In this case we don't mind the holder balance output dropping below the dust limit, as // this additional non-dust HTLC will create the single remaining output on the commitment. let min_balance_msat = if is_outbound_from_holder { - cmp::max(dust_limit_satoshis + current_spiked_tx_fee_sat, spike_buffer_tx_fee_sat) - * 1000 + cmp::max(dust_limit_satoshis + current_tx_fee_sat, spike_buffer_tx_fee_sat) * 1000 // In case we are the fundee, we can send dust HTLCs as long as our own balance output // remains above the dust limit. } else { @@ -787,8 +792,9 @@ pub(crate) trait TxBuilder { &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], addl_nondust_htlc_count: usize, feerate_per_kw: u32, - dust_exposure_limiting_feerate: Option, max_dust_htlc_exposure_msat: u64, - channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, + assuming_future_increased_feerate: bool, dust_exposure_limiting_feerate: Option, + max_dust_htlc_exposure_msat: u64, channel_constraints: ChannelConstraints, + channel_type: &ChannelTypeFeatures, ) -> Result; fn build_commitment_transaction( &self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey, @@ -805,8 +811,9 @@ impl TxBuilder for SpecTxBuilder { &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], addl_nondust_htlc_count: usize, feerate_per_kw: u32, - dust_exposure_limiting_feerate: Option, max_dust_htlc_exposure_msat: u64, - channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, + assuming_future_increased_feerate: bool, dust_exposure_limiting_feerate: Option, + max_dust_htlc_exposure_msat: u64, channel_constraints: ChannelConstraints, + channel_type: &ChannelTypeFeatures, ) -> Result { let commitment_stats = if local { get_next_commitment_stats( @@ -817,6 +824,7 @@ impl TxBuilder for SpecTxBuilder { pending_htlcs, addl_nondust_htlc_count, feerate_per_kw, + assuming_future_increased_feerate, dust_exposure_limiting_feerate, channel_constraints.holder_dust_limit_satoshis, channel_type, @@ -830,6 +838,7 @@ impl TxBuilder for SpecTxBuilder { pending_htlcs, addl_nondust_htlc_count, feerate_per_kw, + assuming_future_increased_feerate, dust_exposure_limiting_feerate, channel_constraints.counterparty_dust_limit_satoshis, channel_type,