diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 02526d5a98b..ee347733305 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2556,12 +2556,15 @@ impl FundingScope { .funding_pubkey = counterparty_funding_pubkey; // New reserve values are based on the new channel value and are v2-specific - let counterparty_selected_channel_reserve_satoshis = - Some(get_v2_channel_reserve_satoshis(post_channel_value, MIN_CHAN_DUST_LIMIT_SATOSHIS)); + let counterparty_selected_channel_reserve_satoshis = Some( + get_v2_channel_reserve_satoshis(post_channel_value, context.holder_dust_limit_satoshis) + .expect("We ran this exact function in `validate_splice_contributions`"), + ); let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( post_channel_value, context.counterparty_dust_limit_satoshis, - ); + ) + .expect("We ran this exact function in `validate_splice_contributions`"); Self { channel_transaction_parameters: post_channel_transaction_parameters, @@ -2985,6 +2988,9 @@ where /// 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"))] @@ -6442,14 +6448,23 @@ fn get_holder_max_htlc_value_in_flight_msat( /// /// This is used both for outbound and inbound channels and has lower bound /// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS`. +/// +/// Returns `Err` if `channel_value_satoshis` is smaller than +/// `MIN_THEIR_CHAN_RESERVE_SATOSHIS`. pub(crate) fn get_holder_selected_channel_reserve_satoshis( channel_value_satoshis: u64, config: &UserConfig, -) -> u64 { - let counterparty_chan_reserve_prop_mil = - config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64; +) -> Result { + if channel_value_satoshis < MIN_THEIR_CHAN_RESERVE_SATOSHIS { + return Err(()); + } + // 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; - cmp::min(channel_value_satoshis, cmp::max(calculated_reserve, MIN_THEIR_CHAN_RESERVE_SATOSHIS)) + Ok(cmp::max(calculated_reserve, MIN_THEIR_CHAN_RESERVE_SATOSHIS)) } /// This is for legacy reasons, present for forward-compatibility. @@ -6466,14 +6481,21 @@ 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`. -fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satoshis: u64) -> u64 { +/// +/// Returns `Err` if `channel_value_satoshis` is smaller than `dust_limit_satoshis`. +fn get_v2_channel_reserve_satoshis( + channel_value_satoshis: u64, dust_limit_satoshis: u64, +) -> Result { + if channel_value_satoshis < dust_limit_satoshis { + return Err(()); + } // 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)) } fn check_splice_contribution_sufficient( @@ -12163,12 +12185,23 @@ where their_funding_contribution.to_sat(), ); let counterparty_selected_channel_reserve = Amount::from_sat( - get_v2_channel_reserve_satoshis(post_channel_value, MIN_CHAN_DUST_LIMIT_SATOSHIS), + get_v2_channel_reserve_satoshis( + post_channel_value, + self.context.holder_dust_limit_satoshis + ).map_err(|()| format!( + "The post-splice channel value {post_channel_value} is smaller than our dust limit {}", + self.context.holder_dust_limit_satoshis, + ))? + ); + let holder_selected_channel_reserve = Amount::from_sat( + get_v2_channel_reserve_satoshis( + post_channel_value, + self.context.counterparty_dust_limit_satoshis, + ).map_err(|()| format!( + "The post-splice channel value {post_channel_value} is smaller than their dust limit {}", + self.context.counterparty_dust_limit_satoshis, + ))? ); - let holder_selected_channel_reserve = Amount::from_sat(get_v2_channel_reserve_satoshis( - post_channel_value, - self.context.counterparty_dust_limit_satoshis, - )); // We allow parties to draw from their previous reserve, as long as they satisfy their v2 reserve @@ -13370,7 +13403,8 @@ where F::Target: FeeEstimator, L::Target: Logger, { - let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(channel_value_satoshis, config); + let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(channel_value_satoshis, config) + .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 { // Protocol level safety check in place, although it should never happen because // of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` @@ -13742,7 +13776,8 @@ where // support this channel type. 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, config); + let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(msg.common_fields.funding_satoshis, config) + .map_err(|()| ChannelError::close(format!("The channel value {} is smaller than {MIN_THEIR_CHAN_RESERVE_SATOSHIS}", msg.common_fields.funding_satoshis)))?; let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.common_fields.funding_pubkey, revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint), @@ -13984,7 +14019,10 @@ where }); let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( - funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS); + funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS + ).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) @@ -14131,9 +14169,15 @@ where 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, msg.common_fields.dust_limit_satoshis); + channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS).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, MIN_CHAN_DUST_LIMIT_SATOSHIS); + channel_value_satoshis, their_dust_limit_satoshis + ).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)?; @@ -16187,6 +16231,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] @@ -16206,7 +16254,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).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(); @@ -16216,7 +16276,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, /*is_0conf=*/false).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 3fd546aaff7..483cc5d793b 100644 --- a/lightning/src/ln/channel_open_tests.rs +++ b/lightning/src/ln/channel_open_tests.rs @@ -513,7 +513,7 @@ 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, &cfg); + get_holder_selected_channel_reserve_satoshis(channel_value_sat, &cfg).unwrap(); let push_msat = (channel_value_sat - channel_reserve_satoshis) * 1000; // Have node0 initiate a channel to node1 with aforementioned parameters diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 36fb17ff076..95277b07d61 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -407,7 +407,7 @@ 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, &default_config); + let reserve = get_holder_selected_channel_reserve_satoshis(100_000, &default_config).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 dc5d07c180e..533a1099741 100644 --- a/lightning/src/ln/htlc_reserve_unit_tests.rs +++ b/lightning/src/ln/htlc_reserve_unit_tests.rs @@ -48,7 +48,8 @@ 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, &default_config) * 1000; + push_amt -= + get_holder_selected_channel_reserve_satoshis(100_000, &default_config).unwrap() * 1000; let push = if send_from_initiator { 0 } else { push_amt }; let temp_channel_id = @@ -993,7 +994,8 @@ pub fn test_chan_reserve_violation_outbound_htlc_inbound_chan() { &channel_type_features, ); - push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, &default_config) * 1000; + push_amt -= + get_holder_selected_channel_reserve_satoshis(100_000, &default_config).unwrap() * 1000; let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt); @@ -1035,7 +1037,8 @@ pub fn test_chan_reserve_violation_inbound_htlc_outbound_channel() { MIN_AFFORDABLE_HTLC_COUNT as u64, &channel_type_features, ); - push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, &default_config) * 1000; + push_amt -= + get_holder_selected_channel_reserve_satoshis(100_000, &default_config).unwrap() * 1000; let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt); // Send four HTLCs to cover the initial push_msat buffer we're required to include @@ -1111,7 +1114,8 @@ 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, &default_config) * 1000; + push_amt -= + get_holder_selected_channel_reserve_satoshis(100_000, &default_config).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 bab2a16bef9..58ccbbb3b1f 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -4951,7 +4951,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, &config) * 1000; + get_holder_selected_channel_reserve_satoshis(CHAN_AMT, &config).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 d8c71c0ea1a..a8ef080280b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -2119,3 +2119,251 @@ fn test_splice_with_inflight_htlc_forward_and_resolution() { do_test_splice_with_inflight_htlc_forward_and_resolution(true); do_test_splice_with_inflight_htlc_forward_and_resolution(false); } + +/// 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)] +#[derive(Clone, Copy, Debug)] +enum AcceptorBalance { + NoBalance, + BalanceInHTLC, + SettledBalance, +} + +#[cfg(test)] +#[derive(Clone, Copy, Debug)] +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.manually_accept_inbound_channels = true; + 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); + + provide_anchor_reserves(&nodes); + + // 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 = SpliceContribution::SpliceOut { outputs }; + + 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); + let post_splice_channel_value_sat = node_0_balance_leftover_amount.to_sat(); + // 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 = SpliceContribution::SpliceOut { outputs }; + + let res = nodes[0].node.splice_channel(&channel_id, &node_id_1, contribution, 253, None); + + match (validation_case, acceptor_balance) { + (ValidationCase::FailsAtHolder, AcceptorBalance::NoBalance) => { + let err = format!("The post-splice channel value {post_splice_channel_value_sat} \ + is smaller than our dust limit {high_dust_limit_satoshis}"); + assert_eq!(res.unwrap_err(), APIError::APIMisuseError { err }); + return; + }, + (ValidationCase::FailsAtHolder, _) => { + let v2_reserve_amount = Amount::from_sat(high_dust_limit_satoshis); + let err = format!("Channel {channel_id} cannot be spliced out; our \ + post-splice channel balance {node_0_balance_leftover_amount} \ + is smaller than their selected v2 reserve {v2_reserve_amount}"); + assert_eq!(res.unwrap_err(), APIError::APIMisuseError { err }); + return; + }, + _ => (), + } + + // The dummy contribution should have passed the holder's validation + assert!(res.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 cannot_splice_out = if matches!(acceptor_balance, AcceptorBalance::NoBalance) { + format!( + "Got non-closing error: 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 acb913294ab..35d02752fea 100644 --- a/lightning/src/ln/update_fee_tests.rs +++ b/lightning/src/ln/update_fee_tests.rs @@ -414,7 +414,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, &default_config); + get_holder_selected_channel_reserve_satoshis(channel_value, &default_config).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) @@ -892,7 +892,8 @@ 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, &default_config) * 1000; + push_amt -= + get_holder_selected_channel_reserve_satoshis(100_000, &default_config).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 74941ec8a87..e6d598f1aab 100644 --- a/lightning/src/sign/tx_builder.rs +++ b/lightning/src/sign/tx_builder.rs @@ -194,7 +194,6 @@ impl TxBuilder for SpecTxBuilder { if channel_type.supports_anchor_zero_fee_commitments() { debug_assert_eq!(feerate_per_kw, 0); debug_assert_eq!(excess_feerate, 0); - debug_assert_eq!(addl_nondust_htlc_count, 0); } // Calculate inbound htlc count