diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e07ee7fceab..e59855d2892 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2752,16 +2752,14 @@ impl FundingScope { ) -> Result { if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY { return Err(format!( - "Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply", - context.channel_id(), + "Our {} contribution exceeds the total bitcoin supply", our_funding_contribution, )); } if their_funding_contribution.unsigned_abs() > Amount::MAX_MONEY { return Err(format!( - "Channel {} cannot be spliced; their {} contribution exceeds the total bitcoin supply", - context.channel_id(), + "Their {} contribution exceeds the total bitcoin supply", their_funding_contribution, )); } @@ -2821,17 +2819,20 @@ impl FundingScope { // New reserve values are based on the new channel value and are v2-specific let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( post_channel_value_sat, - MIN_CHAN_DUST_LIMIT_SATOSHIS, + context.holder_dust_limit_satoshis, prev_funding .counterparty_selected_channel_reserve_satoshis .expect("counterparty reserve is set") == 0, - ); + ) + .map_err(|()| format!("The post-splice channel value {post_channel_value_sat} is smaller than our dust limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"))?; + let their_dust_limit_satoshis = context.counterparty_dust_limit_satoshis; let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( post_channel_value_sat, - context.counterparty_dust_limit_satoshis, + their_dust_limit_satoshis, prev_funding.holder_selected_channel_reserve_satoshis == 0, - ); + ) + .map_err(|()| format!("The post-splice channel value {post_channel_value_sat} is smaller than their dust limit {their_dust_limit_satoshis}"))?; Ok(Self { channel_transaction_parameters: post_channel_transaction_parameters, @@ -3384,6 +3385,9 @@ pub(super) struct ChannelContext { /// We use this to close if funding is never broadcasted. pub(super) channel_creation_height: u32, + #[cfg(any(test, feature = "_test_utils"))] + pub(crate) counterparty_dust_limit_satoshis: u64, + #[cfg(not(any(test, feature = "_test_utils")))] counterparty_dust_limit_satoshis: u64, #[cfg(any(test, feature = "_test_utils"))] @@ -6746,20 +6750,32 @@ fn get_legacy_default_holder_max_htlc_value_in_flight_msat(channel_value_satoshi /// This is used both for outbound and inbound channels and has lower bound /// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS`, and the `dust_limit_satoshis` of /// the counterparty. +/// +/// Returns `Err` if `channel_value_satoshis` is smaller than +/// `MIN_THEIR_CHAN_RESERVE_SATOSHIS` or the `dust_limit_satoshis` of the +/// counterparty. pub(crate) fn get_holder_selected_channel_reserve_satoshis( channel_value_satoshis: u64, their_dust_limit_satoshis: u64, config: &UserConfig, is_0reserve: bool, -) -> u64 { +) -> Result { + if channel_value_satoshis < MIN_THEIR_CHAN_RESERVE_SATOSHIS + || channel_value_satoshis < their_dust_limit_satoshis + { + return Err(()); + } if is_0reserve { - return 0; + return Ok(0); } - let counterparty_chan_reserve_prop_mil = - config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64; + // As described in the `ChannelHandshakeConfig` docs, we cap this value at 1_000_000. + let counterparty_chan_reserve_prop_mil = cmp::min( + config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64, + 1_000_000, + ); let calculated_reserve = channel_value_satoshis.saturating_mul(counterparty_chan_reserve_prop_mil) / 1_000_000; let channel_reserve_satoshis = cmp::max(calculated_reserve, MIN_THEIR_CHAN_RESERVE_SATOSHIS); let channel_reserve_satoshis = cmp::max(channel_reserve_satoshis, their_dust_limit_satoshis); - cmp::min(channel_value_satoshis, channel_reserve_satoshis) + Ok(channel_reserve_satoshis) } /// This is for legacy reasons, present for forward-compatibility. @@ -6776,19 +6792,24 @@ pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis( /// Returns a minimum channel reserve value each party needs to maintain, fixed in the spec to a /// default of 1% of the total channel value. /// -/// Guaranteed to return a value no larger than channel_value_satoshis +/// Guaranteed to return a value no larger than `channel_value_satoshis` /// /// This is used both for outbound and inbound channels and has lower bound /// of `dust_limit_satoshis`. +/// +/// Returns `Err` if `channel_value_satoshis` is smaller than `dust_limit_satoshis`. pub(crate) fn get_v2_channel_reserve_satoshis( channel_value_satoshis: u64, dust_limit_satoshis: u64, is_0reserve: bool, -) -> u64 { +) -> Result { + if channel_value_satoshis < dust_limit_satoshis { + return Err(()); + } if is_0reserve { - return 0; + return Ok(0); } // Fixed at 1% of channel value by spec. let (q, _) = channel_value_satoshis.overflowing_div(100); - cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) + Ok(cmp::max(q, dust_limit_satoshis)) } /// Returns the minimum feerate for RBF attempts given a previous feerate. @@ -12824,7 +12845,8 @@ where their_funding_contribution, counterparty_funding_pubkey, our_new_holder_keys, - )?; + ) + .map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?; let (post_splice_holder_balance, post_splice_counterparty_balance) = self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err( @@ -14458,12 +14480,19 @@ impl OutboundV1Channel { // a dust limit higher than our selected reserve. let their_dust_limit_satoshis = 0; let is_0reserve = trusted_channel_features.is_some_and(|f| f.is_0reserve()); - let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis( - channel_value_satoshis, - their_dust_limit_satoshis, - config, - is_0reserve, - ); + let holder_selected_channel_reserve_satoshis = + get_holder_selected_channel_reserve_satoshis( + channel_value_satoshis, + their_dust_limit_satoshis, + config, + is_0reserve, + ) + .map_err(|()| APIError::APIMisuseError { + err: format!( + "The channel value {channel_value_satoshis} is smaller than \ + {MIN_THEIR_CHAN_RESERVE_SATOSHIS}" + ), + })?; if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS && !is_0reserve { // Protocol level safety check in place, although it should never happen because // of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` and `MIN_CHANNEL_VALUE_SATOSHIS` @@ -14855,12 +14884,20 @@ impl InboundV1Channel { let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?; - let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis( - msg.common_fields.funding_satoshis, - msg.common_fields.dust_limit_satoshis, - config, - trusted_channel_features.is_some_and(|f| f.is_0reserve()), - ); + let holder_selected_channel_reserve_satoshis = + get_holder_selected_channel_reserve_satoshis( + msg.common_fields.funding_satoshis, + msg.common_fields.dust_limit_satoshis, + config, + trusted_channel_features.is_some_and(|f| f.is_0reserve()), + ) + .map_err(|()| { + ChannelError::close(format!( + "The channel value {} is smaller than either their dust \ + limit {}, or {MIN_THEIR_CHAN_RESERVE_SATOSHIS}", + msg.common_fields.funding_satoshis, msg.common_fields.dust_limit_satoshis, + )) + })?; let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.common_fields.funding_pubkey, revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint), @@ -15117,8 +15154,13 @@ impl PendingV2Channel { }); let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( - funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve())); - + funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve()) + ).map_err(|()| APIError::APIMisuseError { + err: format!( + "The channel value {funding_satoshis} is smaller than their dust \ + limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}" + ) + })?; let funding_feerate_sat_per_1000_weight = fee_estimator.bounded_sat_per_1000_weight(funding_confirmation_target); let funding_tx_locktime = LockTime::from_height(current_chain_height) .map_err(|_| APIError::APIMisuseError { @@ -15257,9 +15299,16 @@ impl PendingV2Channel { let channel_value_satoshis = our_funding_contribution_sats.saturating_add(msg.common_fields.funding_satoshis); let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( - channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some()); + channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some() + ).map_err(|()| ChannelError::close(format!( + "The channel value {channel_value_satoshis} is smaller than our dust limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}" + )))?; + let their_dust_limit_satoshis = msg.common_fields.dust_limit_satoshis; let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( - channel_value_satoshis, msg.common_fields.dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve())); + channel_value_satoshis, their_dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve()) + ).map_err(|()| ChannelError::close(format!( + "The channel value {channel_value_satoshis} is smaller than their dust limit {their_dust_limit_satoshis}" + )))?; let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?; @@ -17450,6 +17499,10 @@ mod tests { // to channel value test_self_and_counterparty_channel_reserve(10_000_000, 0.50, 0.50); test_self_and_counterparty_channel_reserve(10_000_000, 0.60, 0.50); + + // Make sure we correctly handle reserves greater than the channel value + test_self_and_counterparty_channel_reserve(100_000, 1.1, 0.30); + test_self_and_counterparty_channel_reserve(100_000, 0.30, 1.1); } #[rustfmt::skip] @@ -17469,7 +17522,19 @@ mod tests { outbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (outbound_selected_channel_reserve_perc * 1_000_000.0) as u32; let mut chan = OutboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, outbound_node_id, &channelmanager::provided_init_features(&outbound_node_config), channel_value_satoshis, 100_000, 42, &outbound_node_config, 0, 42, None, &logger, None).unwrap(); - let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * outbound_selected_channel_reserve_perc) as u64); + let outbound_capped_reserve_perc = if outbound_selected_channel_reserve_perc.lt(&1.0) { + outbound_selected_channel_reserve_perc + } else { + 1.0 + }; + + let inbound_capped_reserve_perc = if inbound_selected_channel_reserve_perc.lt(&1.0) { + inbound_selected_channel_reserve_perc + } else { + 1.0 + }; + + let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * outbound_capped_reserve_perc) as u64); assert_eq!(chan.funding.holder_selected_channel_reserve_satoshis, expected_outbound_selected_chan_reserve); let chan_open_channel_msg = chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap(); @@ -17479,7 +17544,7 @@ mod tests { if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 { let chan_inbound_node = InboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, None).unwrap(); - let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * inbound_selected_channel_reserve_perc) as u64); + let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * inbound_capped_reserve_perc) as u64); assert_eq!(chan_inbound_node.funding.holder_selected_channel_reserve_satoshis, expected_inbound_selected_chan_reserve); assert_eq!(chan_inbound_node.funding.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve); diff --git a/lightning/src/ln/channel_open_tests.rs b/lightning/src/ln/channel_open_tests.rs index ac4a1b67994..50ef0721e07 100644 --- a/lightning/src/ln/channel_open_tests.rs +++ b/lightning/src/ln/channel_open_tests.rs @@ -16,7 +16,8 @@ use crate::chain::{self, ChannelMonitorUpdateStatus}; use crate::events::{ClosureReason, Event, FundingInfo}; use crate::ln::channel::{ get_holder_selected_channel_reserve_satoshis, ChannelError, InboundV1Channel, - OutboundV1Channel, COINBASE_MATURITY, UNFUNDED_CHANNEL_AGE_LIMIT_TICKS, + OutboundV1Channel, COINBASE_MATURITY, MIN_THEIR_CHAN_RESERVE_SATOSHIS, + UNFUNDED_CHANNEL_AGE_LIMIT_TICKS, }; use crate::ln::channelmanager::{ self, TrustedChannelFeatures, BREAKDOWN_TIMEOUT, MAX_UNFUNDED_CHANNEL_PEERS, @@ -473,7 +474,8 @@ pub fn test_insane_channel_opens() { // funding satoshis let channel_value_sat = 31337; // same as funding satoshis let channel_reserve_satoshis = - get_holder_selected_channel_reserve_satoshis(channel_value_sat, 0, &legacy_cfg, false); + get_holder_selected_channel_reserve_satoshis(channel_value_sat, 0, &legacy_cfg, false) + .unwrap(); let push_msat = (channel_value_sat - channel_reserve_satoshis) * 1000; // Have node0 initiate a channel to node1 with aforementioned parameters @@ -552,7 +554,13 @@ pub fn test_insane_channel_opens() { }, ); - insane_open_helper("Peer never wants payout outputs?", |mut msg| { + let crazy_dust_limit = channel_value_sat + 1; + let expected_error_str = format!( + "Got non-closing error: The channel value \ + {channel_value_sat} is smaller than either their dust limit {crazy_dust_limit}, or \ + {MIN_THEIR_CHAN_RESERVE_SATOSHIS}" + ); + insane_open_helper(&expected_error_str, |mut msg| { msg.common_fields.dust_limit_satoshis = msg.common_fields.funding_satoshis + 1; msg }); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index c8ecb40fa6d..8bbb9b99479 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -415,7 +415,8 @@ pub fn test_inbound_outbound_capacity_is_not_zero() { assert_eq!(channels0.len(), 1); assert_eq!(channels1.len(), 1); - let reserve = get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false); + let reserve = + get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false).unwrap(); assert_eq!(channels0[0].inbound_capacity_msat, 95000000 - reserve * 1000); assert_eq!(channels1[0].outbound_capacity_msat, 95000000 - reserve * 1000); diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs index 45d3cf5950f..a4d92b7a045 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -55,8 +55,9 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) { 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 -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) + .unwrap() + * 1000; let push = if send_from_initiator { 0 } else { push_amt }; let temp_channel_id = @@ -1002,8 +1003,9 @@ pub fn test_chan_reserve_violation_outbound_htlc_inbound_chan() { &channel_type_features, ); - push_amt -= - get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000; + push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) + .unwrap() + * 1000; let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt); @@ -1048,8 +1050,9 @@ pub fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() { MIN_AFFORDABLE_HTLC_COUNT as u64, &channel_type_features, ); - push_amt -= - get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000; + push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) + .unwrap() + * 1000; create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, push_amt); let (htlc_success_tx_fee_sat, _) = diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 5b4f5f93d71..ccb933a95d7 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -5043,7 +5043,7 @@ fn test_htlc_forward_considers_anchor_outputs_value() { 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, 0, &config, false) * 1000; + get_holder_selected_channel_reserve_satoshis(CHAN_AMT, 0, &config, false).unwrap() * 1000; let commitment_fee_msat = chan_utils::commit_tx_fee_sat( *nodes[1].fee_estimator.sat_per_kw.lock().unwrap(), 2, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a5361358653..0de7574d416 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -7865,13 +7865,15 @@ fn do_test_0reserve_splice_counterparty_validation( // They obviously can't afford their contribution, so we fail before even // querying `TxBuilder` format!( - "Got non-closing error: Their contribution candidate {funding_contribution_sat}sat \ + "Got non-closing error: Channel {channel_id} cannot be spliced; \ + Their contribution candidate {funding_contribution_sat}sat \ is greater than their total balance in the channel {initiator_value_to_self_sat}sat" ) } else if post_channel_value_sat < MIN_CHANNEL_VALUE_SATOSHIS { // We require all spliced channels to have a value of at least 1000 satoshis after the splice format!( - "Got non-closing error: Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \ + "Got non-closing error: Channel {channel_id} cannot be spliced; \ + Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \ It would be {post_channel_value_sat}" ) } else { @@ -7888,3 +7890,244 @@ fn do_test_0reserve_splice_counterparty_validation( channel_type } + +/// We previously allowed a splice initiator to splice out funds past their channel reserve if the +/// the acceptor had no balance in the channel, and there were no HTLCs in the channel +#[cfg(test)] +enum AcceptorBalance { + NoBalance, + BalanceInHTLC, + SettledBalance, +} + +#[cfg(test)] +enum ValidationCase { + Passes, + FailsAtHolder, + FailsAtCounterparty, +} + +#[test] +fn test_splice_out_initiator_reserve_breach_zero_fee_commitments() { + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::NoBalance, + ValidationCase::Passes, + ); + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::BalanceInHTLC, + ValidationCase::Passes, + ); + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::SettledBalance, + ValidationCase::Passes, + ); + + // We used to fail this case here + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::NoBalance, + ValidationCase::FailsAtHolder, + ); + + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::BalanceInHTLC, + ValidationCase::FailsAtHolder, + ); + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::SettledBalance, + ValidationCase::FailsAtHolder, + ); + + // We used to fail this case here + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::NoBalance, + ValidationCase::FailsAtCounterparty, + ); + + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::BalanceInHTLC, + ValidationCase::FailsAtCounterparty, + ); + do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + AcceptorBalance::SettledBalance, + ValidationCase::FailsAtCounterparty, + ); +} + +#[cfg(test)] +fn do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( + acceptor_balance: AcceptorBalance, validation_case: ValidationCase, +) { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut config = test_default_channel_config(); + // This reserve breach was only possible in 0FC channels + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + config.channel_handshake_config.our_htlc_minimum_msat = 1; + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config.clone())]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // Node 0 is initiator, node 1 is acceptor + let _node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_1 = nodes[1].node.get_our_node_id(); + + let channel_value_sat = 100_000; + let node_1_settled_balance_msat = + if matches!(acceptor_balance, AcceptorBalance::SettledBalance) { 1 } else { 0 }; + let node_1_htlc_balance_msat = + if matches!(acceptor_balance, AcceptorBalance::BalanceInHTLC) { 1 } else { 0 }; + let node_0_balance_msat = + channel_value_sat * 1000 - node_1_settled_balance_msat - node_1_htlc_balance_msat; + + // Bump initiator's dust limit to the highest value we allow in anchor channels + let high_dust_limit_satoshis = 10_000; + + let (_, _, channel_id, _tx) = create_announced_chan_between_nodes_with_value( + &nodes, + 0, + 1, + channel_value_sat, + node_1_settled_balance_msat, + ); + + if matches!(acceptor_balance, AcceptorBalance::BalanceInHTLC) { + let _ = route_payment(&nodes[0], &[&nodes[1]], node_1_htlc_balance_msat); + } + + { + let 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); + if let Some(chan) = channel.as_funded_mut() { + chan.context.holder_dust_limit_satoshis = high_dust_limit_satoshis; + } else { + panic!("Unexpected Channel phase"); + } + } + + { + 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); + if let Some(chan) = channel.as_funded_mut() { + chan.context.counterparty_dust_limit_satoshis = high_dust_limit_satoshis; + } else { + panic!("Unexpected Channel phase"); + } + } + + if matches!(validation_case, ValidationCase::Passes) { + let node_0_balance_leftover_amount = Amount::from_sat(high_dust_limit_satoshis); + // Estimated fees of a splice_out at 253sat/kw + let estimated_fees = 183; + // Note in 0FC we've got no fee spike buffer, no commit tx fee, no anchors + let splice_out_output_sat = + node_0_balance_msat / 1000 - node_0_balance_leftover_amount.to_sat() - estimated_fees; + let splice_out_output_amount = Amount::from_sat(splice_out_output_sat); + let outputs = vec![TxOut { + value: splice_out_output_amount, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]; + let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs).unwrap(); + + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, contribution); + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + } else { + let node_0_balance_leftover_amount = Amount::from_sat(high_dust_limit_satoshis - 1); + // Note in 0FC we've got no fee spike buffer, no commit tx fee, no anchors + let funding_contribution_sat = + -((node_0_balance_msat / 1000 - node_0_balance_leftover_amount.to_sat()) as i64); + let value = if matches!(validation_case, ValidationCase::FailsAtHolder) { + Amount::from_sat(funding_contribution_sat.unsigned_abs() - 183) + } else if matches!(validation_case, ValidationCase::FailsAtCounterparty) { + // Splice out some dummy amount to get past the initiator's validation, + // we'll modify the message in-flight. + Amount::from_sat(1000) + } else { + panic!("Unexpected test case"); + }; + let outputs = vec![TxOut { + value, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]; + let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); + + if matches!(validation_case, ValidationCase::FailsAtHolder) { + assert_eq!( + contribution.unwrap_err(), + APIError::APIMisuseError { + err: format!("Channel {channel_id} cannot accept funding contribution"), + } + ); + let splice_out_value = value + Amount::from_sat(183); + let splice_out_max = splice_out_value - Amount::ONE_SAT; + let cannot_splice_out = format!( + "Channel {channel_id} cannot be funded: \ + Our splice-out value of {splice_out_value} is greater than the \ + maximum {splice_out_max}" + ); + nodes[0].logger.assert_log("lightning::ln::channel", cannot_splice_out, 1); + return; + } + + // The dummy contribution should have passed the holder's validation + assert!(contribution.is_ok()); + + // When acceptor has no balance, the reserve the initiator should keep should remain + // clamped at its dust limit. We previously allowed the initiator to withdraw past + // this point. + let v2_channel_reserve = Amount::from_sat(high_dust_limit_satoshis); + + let initiator = &nodes[0]; + let acceptor = &nodes[1]; + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + acceptor.node.handle_stfu(node_id_initiator, &stfu_init); + let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + initiator.node.handle_stfu(node_id_acceptor, &stfu_ack); + + let mut splice_init = + get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + // Make the modification here, acceptor should now complain. If the acceptor has no + // balance, we previously would not complain. + splice_init.funding_contribution_satoshis = funding_contribution_sat; + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1); + if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { + assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); + } else { + panic!("Expected MessageSendEvent::HandleError"); + } + let post_splice_channel_value_sat = node_0_balance_leftover_amount.to_sat(); + let cannot_splice_out = if matches!(acceptor_balance, AcceptorBalance::NoBalance) { + format!( + "Got non-closing error: Channel {channel_id} cannot \ + be spliced; The post-splice channel value {post_splice_channel_value_sat} \ + is smaller than their dust limit {high_dust_limit_satoshis}" + ) + } else { + // As soon as we've pushed any sats out of our balance, the channel value + // is now at the dust limit, so we don't complain when determining the new + // dust limits, but later when we check the balances against those new + // dust limits + assert_eq!( + channel_value_sat.checked_add_signed(funding_contribution_sat).unwrap(), + high_dust_limit_satoshis + ); + format!( + "Got non-closing error: Channel {channel_id} cannot \ + be spliced out; their post-splice channel balance \ + {node_0_balance_leftover_amount} is smaller than our selected v2 reserve \ + {v2_channel_reserve}" + ) + }; + acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1); + } +} diff --git a/lightning/src/ln/update_fee_tests.rs b/lightning/src/ln/update_fee_tests.rs index b1f8257088e..1cb04f13a33 100644 --- a/lightning/src/ln/update_fee_tests.rs +++ b/lightning/src/ln/update_fee_tests.rs @@ -410,7 +410,7 @@ pub fn do_test_update_fee_that_funder_cannot_afford(channel_type_features: Chann let channel_id = chan.2; let secp_ctx = Secp256k1::new(); let bs_channel_reserve_sats = - get_holder_selected_channel_reserve_satoshis(channel_value, 0, &cfg, false); + get_holder_selected_channel_reserve_satoshis(channel_value, 0, &cfg, false).unwrap(); let (anchor_outputs_value_sats, outputs_num_no_htlcs) = if channel_type_features.supports_anchors_zero_fee_htlc_tx() { (ANCHOR_OUTPUT_VALUE_SATOSHI * 2, 4) @@ -886,8 +886,9 @@ pub fn test_chan_init_feerate_unaffordability() { // During open, we don't have a "counterparty channel reserve" to check against, so that // requirement only comes into play on the open_channel handling side. - push_amt -= - get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000; + push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) + .unwrap() + * 1000; nodes[0].node.create_channel(node_b_id, 100_000, push_amt, 42, None, None).unwrap(); let mut open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id); diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs index ffb01c571b7..f9a990e8c94 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -365,7 +365,8 @@ fn get_next_splice_out_maximum_sat( channel_value_satoshis, channel_constraints.holder_dust_limit_satoshis, false, - ); + ) + .unwrap(); // If the holder cannot splice out anything, they must be at or // below the v2 reserve debug_assert!(current_balance_sat <= v2_reserve_sat); @@ -374,7 +375,8 @@ fn get_next_splice_out_maximum_sat( channel_value_satoshis.saturating_sub(max_splice_out_sat), channel_constraints.holder_dust_limit_satoshis, false, - ); + ) + .unwrap(); // If the holder can splice out some maximum, splicing out that // maximum lands them at exactly the new v2 reserve + the // `post_splice_delta_above_reserve_sat` @@ -382,40 +384,56 @@ fn get_next_splice_out_maximum_sat( local_balance_before_fee_sat.saturating_sub(max_splice_out_sat), post_splice_reserve_sat.saturating_add(post_splice_delta_above_reserve_sat) ); + // Splice out an additional satoshi, and check that we are offside + let offside_splice_out_sat = max_splice_out_sat + 1; + let post_splice_reserve_sat_result = get_v2_channel_reserve_satoshis( + channel_value_satoshis.saturating_sub(offside_splice_out_sat), + channel_constraints.holder_dust_limit_satoshis, + false, + ); + match post_splice_reserve_sat_result { + Ok(reserve) => debug_assert!( + local_balance_before_fee_sat.saturating_sub(offside_splice_out_sat) + < reserve.saturating_add(post_splice_delta_above_reserve_sat) + ), + Err(()) => (), + } } max_splice_out_sat } else { // In a zero-reserve channel, the holder is free to withdraw up to its `post_splice_delta_above_reserve_sat` - local_balance_before_fee_sat.saturating_sub(post_splice_delta_above_reserve_sat) - }; + let mut next_splice_out_maximum_sat = + local_balance_before_fee_sat.saturating_sub(post_splice_delta_above_reserve_sat); - // We only bother to check the local commitment here, the counterparty will check its own commitment. - // - // 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`, - // 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, - 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 min_balance_sat = if is_outbound_from_holder { - dust_limit_satoshis.saturating_add(current_spiked_tx_fee_sat) - } else { - dust_limit_satoshis - }; - next_splice_out_maximum_sat = - (local_balance_before_fee_msat / 1000).saturating_sub(min_balance_sat); - } + // We only bother to check the local commitment here, the counterparty will check its own commitment. + // + // 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`, + // 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, + 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 min_balance_sat = if is_outbound_from_holder { + dust_limit_satoshis.saturating_add(current_spiked_tx_fee_sat) + } else { + dust_limit_satoshis + }; + next_splice_out_maximum_sat = + (local_balance_before_fee_msat / 1000).saturating_sub(min_balance_sat); + } + next_splice_out_maximum_sat + }; if channel_value_satoshis < next_splice_out_maximum_sat + MIN_CHANNEL_VALUE_SATOSHIS { next_splice_out_maximum_sat =