diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 32c0e94bdc8..faf435ce35b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2900,7 +2900,7 @@ struct PendingFunding { received_funding_txid: Option, /// The feerate used in the last successfully negotiated funding transaction. - /// Used for validating the 25/24 feerate increase rule on RBF attempts. + /// Used for validating the minimum feerate increase rule on RBF attempts. last_funding_feerate_sat_per_1000_weight: Option, /// The funding contributions from splice/RBF rounds where we contributed. @@ -6769,16 +6769,15 @@ fn get_v2_channel_reserve_satoshis( cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } -/// Returns the minimum feerate for our own RBF attempts given a previous feerate. +/// Returns the minimum feerate for RBF attempts given a previous feerate. /// -/// The spec (tx_init_rbf) requires the new feerate to be >= 25/24 of the previous feerate. -/// However, at low feerates that multiplier doesn't always satisfy BIP125's relay requirement of -/// an absolute fee increase, so we take the max of a flat +25 sat/kwu (0.1 sat/vB) increment -/// and the spec's multiplicative rule. We still accept the bare 25/24 rule from counterparties -/// in [`FundedChannel::validate_tx_init_rbf`]. +/// The spec (tx_init_rbf) requires the new feerate to be >= the maximum of 25/24 of the previous +/// feerate and the previous feerate + 25 sat/kwu. The flat +25 sat/kwu increment ensures BIP125's +/// relay requirement of an absolute fee increase is satisfied at low feerates where the +/// multiplicative 25/24 rule alone would be insufficient. fn min_rbf_feerate(prev_feerate: u32) -> FeeRate { let flat_increment = (prev_feerate as u64).saturating_add(25); - let spec_increment = ((prev_feerate as u64) * 25).div_ceil(24); + let spec_increment = (prev_feerate as u64) * 25 / 24; FeeRate::from_sat_per_kwu(cmp::max(flat_increment, spec_increment)) } @@ -13008,17 +13007,17 @@ where }, }; - // Check the 25/24 feerate increase rule let prev_feerate = pending_splice.last_funding_feerate_sat_per_1000_weight.unwrap_or_else(|| { fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::UrgentOnChainSweep) }); - let new_feerate = msg.feerate_sat_per_1000_weight; - if (new_feerate as u64) * 24 < (prev_feerate as u64) * 25 { + let new_feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64); + if new_feerate < min_rbf_feerate(prev_feerate) { return Err(ChannelError::Abort(AbortReason::InsufficientRbfFeerate)); } - if !pending_splice.is_rbf_feerate_sufficient(new_feerate, fee_estimator) { + if !pending_splice.is_rbf_feerate_sufficient(msg.feerate_sat_per_1000_weight, fee_estimator) + { return Err(ChannelError::Abort(AbortReason::InsufficientRbfFeerate)); } diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index c08a0a9f471..f4db0296383 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -1019,9 +1019,9 @@ impl FundingContribution { /// Adjusts the contribution's change output for the minimum RBF feerate. /// - /// When a pending splice exists with negotiated candidates and the contribution's feerate - /// is below the minimum RBF feerate (25/24 of the previous feerate), this adjusts the - /// change output so the initiator pays fees at the minimum RBF feerate. + /// When a pending splice exists with negotiated candidates and the contribution's feerate is + /// below the minimum RBF feerate, this adjusts the change output so the initiator pays fees + /// at the minimum RBF feerate. pub(super) fn for_initiator_at_feerate( self, feerate: FeeRate, holder_balance: Amount, ) -> Result { diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 9957205716c..ca8c4450012 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -136,8 +136,8 @@ pub(crate) enum AbortReason { DuplicateFundingOutput, /// More than one funding (shared) input found. DuplicateFundingInput, - /// The RBF feerate is insufficient (e.g., doesn't satisfy the 25/24 rule or can't accommodate - /// prior contributions). + /// The RBF feerate is insufficient (e.g., doesn't satisfy the minimum feerate increase rule or + /// can't accommodate prior contributions). InsufficientRbfFeerate, /// A funding negotiation is already in progress. NegotiationInProgress, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 9adccd17627..aec7fa9d1e1 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4600,12 +4600,12 @@ fn test_splice_rbf_insufficient_feerate() { let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); assert_eq!(tx_abort.channel_id, channel_id); - // Acceptor-side: a counterparty feerate that satisfies the spec's 25/24 rule (264) is - // accepted, even though our own RBF floor (+25 sat/kwu = 278) is higher. + // Acceptor-side: a counterparty feerate that only satisfies the 25/24 rule (263) is + // rejected — the spec requires max(prev + 25, prev * 25/24) = 278 at low feerates. // After tx_abort the channel remains quiescent, so no need to re-enter quiescence. nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); - let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24) as u32; + let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25 / 24) as u32; let tx_init_rbf = msgs::TxInitRbf { channel_id, locktime: 0, @@ -4613,6 +4613,92 @@ fn test_splice_rbf_insufficient_feerate() { funding_output_contribution: Some(added_value.to_sat() as i64), }; + nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!(tx_abort.channel_id, channel_id); + + // Acceptor-side: prev + 25 = 278 satisfies the combined BIP125 rule and is accepted. + nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + + let min_rbf_feerate = FEERATE_FLOOR_SATS_PER_KW + 25; + let tx_init_rbf = msgs::TxInitRbf { + channel_id, + locktime: 0, + feerate_sat_per_1000_weight: min_rbf_feerate, + funding_output_contribution: Some(added_value.to_sat() as i64), + }; + + nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); + let _tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); +} + +#[test] +fn test_splice_rbf_insufficient_feerate_high() { + // At high feerates (above ~600 sat/kwu) the 25/24 multiplicative rule dominates the +25 + // flat increment. Verify that the counterparty validation rejects a feerate satisfying only + // the flat increment and accepts one satisfying the 25/24 rule. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Complete a splice-in at floor feerate, then RBF to 1000 sat/kwu. + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (_splice_tx, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + provide_utxo_reserves(&nodes, 2, added_value * 2); + let high_feerate = FeeRate::from_sat_per_kwu(1000); + let contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, high_feerate); + complete_rbf_handshake(&nodes[0], &nodes[1]); + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + contribution, + new_funding_script, + ); + let (_, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false); + assert!(splice_locked.is_none()); + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + // prev=1000: flat increment gives 1000+25=1025, 25/24 rule gives 1000*25/24=1041. + // Feerate 1025 satisfies the flat increment but not 25/24 — rejected. + reenter_quiescence(&nodes[0], &nodes[1], &channel_id); + + let tx_init_rbf = msgs::TxInitRbf { + channel_id, + locktime: 0, + feerate_sat_per_1000_weight: 1025, + funding_output_contribution: Some(added_value.to_sat() as i64), + }; + + nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!(tx_abort.channel_id, channel_id); + + // Feerate 1041 satisfies both rules — accepted. + nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + + let tx_init_rbf = msgs::TxInitRbf { + channel_id, + locktime: 0, + feerate_sat_per_1000_weight: 1041, + funding_output_contribution: Some(added_value.to_sat() as i64), + }; + nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); let _tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); } @@ -6167,7 +6253,7 @@ fn test_funding_contributed_rbf_adjustment_insufficient_budget() { funding_template.splice_in_sync(added_value, floor_feerate, FeeRate::MAX, &wallet).unwrap(); // Node 1 initiates a splice at a HIGH feerate (10,000 sat/kwu). The minimum RBF feerate will be - // max(10,000 + 25, ceil(10,000 * 25/24)) = 10,417 sat/kwu — far above what node 0's tight + // max(10,000 + 25, 10,000 * 25/24) = 10,416 sat/kwu — far above what node 0's tight // budget can handle. let high_feerate = FeeRate::from_sat_per_kwu(10_000); let node_1_template = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap();