Merge pull request #3933 from tankyleo/2025-07-nondust-htlc-accounting

Correct non-dust HTLC accounting in `next_remote_commit_tx_fee_msat`

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -5191,14 +5191,14 @@ where
 		// committed outbound HTLCs, see below.
 		let mut included_htlcs = 0;
 		for ref htlc in context.pending_inbound_htlcs.iter() {
-			if htlc.amount_msat / 1000 <= real_dust_limit_timeout_sat {
+			if htlc.amount_msat / 1000 < real_dust_limit_timeout_sat {
 				continue
 			}
 			included_htlcs += 1;
 		}
 
 		for ref htlc in context.pending_outbound_htlcs.iter() {
-			if htlc.amount_msat / 1000 <= real_dust_limit_success_sat {
+			if htlc.amount_msat / 1000 < real_dust_limit_success_sat {
 				continue
 			}
 			// We only include outbound HTLCs if it will not be included in their next commitment_signed,

lightning/src/ln/htlc_reserve_unit_tests.rs

@@ -2078,3 +2078,302 @@ pub fn test_update_fulfill_htlc_bolt2_missing_badonion_bit_for_malformed_htlc_me
 	let reason = ClosureReason::ProcessingError { err: err_msg.data };
 	check_closed_event!(nodes[0], 1, reason, [node_b_id], 1000000);
 }
+
+#[xtest(feature = "_externalize_tests")]
+pub fn test_dust_limit_fee_accounting() {
+	do_test_dust_limit_fee_accounting(false);
+	do_test_dust_limit_fee_accounting(true);
+}
+
+pub fn do_test_dust_limit_fee_accounting(can_afford: bool) {
+	// Test that when a channel funder sends HTLCs exactly on the dust limit
+	// of the funder, the fundee correctly accounts for the additional fee on the
+	// funder's commitment transaction due to those additional non-dust HTLCs when
+	// checking for any infrigements on the funder's reserve.
+
+	let channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies();
+
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+
+	let mut default_config = test_default_channel_config();
+	default_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+	default_config.manually_accept_inbound_channels = true;
+
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs =
+		create_node_chanmgrs(2, &node_cfgs, &[Some(default_config.clone()), Some(default_config)]);
+
+	let mut 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();
+
+	// Set a HTLC amount that is equal to the dust limit of the funder
+	const HTLC_AMT_SAT: u64 = 354;
+
+	const CHANNEL_VALUE_SAT: u64 = 100_000;
+
+	const FEERATE_PER_KW: u32 = 253;
+
+	let commit_tx_fee_sat =
+		chan_utils::commit_tx_fee_sat(FEERATE_PER_KW, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
+
+	// By default the reserve is set to 1% or 1000sat, whichever is higher
+	let channel_reserve_satoshis = 1_000;
+
+	// Set node 0's balance to pay for exactly MIN_AFFORDABLE_HTLC_COUNT non-dust HTLCs on the channel, minus some offset
+	let node_0_balance_sat = commit_tx_fee_sat
+		+ channel_reserve_satoshis
+		+ 2 * crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI
+		+ MIN_AFFORDABLE_HTLC_COUNT as u64 * HTLC_AMT_SAT
+		- if can_afford { 0 } else { 1 };
+	let mut node_1_balance_sat = CHANNEL_VALUE_SAT - node_0_balance_sat;
+
+	let chan_id = create_chan_between_nodes_with_value(
+		&nodes[0],
+		&nodes[1],
+		CHANNEL_VALUE_SAT,
+		node_1_balance_sat * 1000,
+	)
+	.3;
+
+	{
+		// Double check the reserve that node 0 has to maintain here
+		let per_peer_state_lock;
+		let mut peer_state_lock;
+		let chan =
+			get_channel_ref!(nodes[1], nodes[0], per_peer_state_lock, peer_state_lock, chan_id);
+		assert_eq!(
+			chan.funding().holder_selected_channel_reserve_satoshis,
+			channel_reserve_satoshis
+		);
+	}
+	{
+		// Double check the dust limit on node 0's commitment transactions; when node 0
+		// adds a HTLC, node 1 will check that the fee on node 0's commitment transaction
+		// does not dip under the node 1 selected reserve.
+		let per_peer_state_lock;
+		let mut peer_state_lock;
+		let chan =
+			get_channel_ref!(nodes[0], nodes[1], per_peer_state_lock, peer_state_lock, chan_id);
+		assert_eq!(chan.context().holder_dust_limit_satoshis, HTLC_AMT_SAT);
+	}
+
+	// Precompute the route to skip any router complaints when sending the last HTLC
+	let (route_0_1, payment_hash_0_1, _, payment_secret_0_1) =
+		get_route_and_payment_hash!(nodes[0], nodes[1], HTLC_AMT_SAT * 1000);
+
+	let mut htlcs = Vec::new();
+	for _ in 0..MIN_AFFORDABLE_HTLC_COUNT - 1 {
+		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 {
+			offered: false,
+			amount_msat: HTLC_AMT_SAT * 1000,
+			// Hard-coded to match the expected value
+			cltv_expiry: 81,
+			payment_hash,
+			transaction_output_index: None,
+		};
+		htlcs.push(accepted_htlc);
+	}
+
+	// Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
+	let secp_ctx = Secp256k1::new();
+	let session_priv = SecretKey::from_slice(&[42; 32]).expect("RNG is bad!");
+
+	let cur_height = nodes[1].node.best_block.read().unwrap().height + 1;
+
+	let onion_keys =
+		onion_utils::construct_onion_keys(&secp_ctx, &route_0_1.paths[0], &session_priv);
+	let recipient_onion_fields = RecipientOnionFields::secret_only(payment_secret_0_1);
+	let (onion_payloads, amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(
+		&route_0_1.paths[0],
+		HTLC_AMT_SAT * 1000,
+		&recipient_onion_fields,
+		cur_height,
+		&None,
+		None,
+		None,
+	)
+	.unwrap();
+	let onion_routing_packet =
+		onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash_0_1)
+			.unwrap();
+	// Double check the hard-coded value
+	assert_eq!(cltv_expiry, 81);
+	let msg = msgs::UpdateAddHTLC {
+		channel_id: chan_id,
+		htlc_id: MIN_AFFORDABLE_HTLC_COUNT as u64 - 1,
+		amount_msat,
+		payment_hash: payment_hash_0_1,
+		cltv_expiry,
+		onion_routing_packet,
+		skimmed_fee_msat: None,
+		blinding_point: None,
+	};
+
+	nodes[1].node.handle_update_add_htlc(node_a_id, &msg);
+
+	if !can_afford {
+		let err = "Remote HTLC add would put them under remote reserve value".to_string();
+		nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", &err, 3);
+		let events = nodes[1].node.get_and_clear_pending_msg_events();
+		assert_eq!(events.len(), 2);
+		let reason = ClosureReason::ProcessingError { err };
+		check_closed_event(&nodes[1], 1, reason, false, &[node_a_id], CHANNEL_VALUE_SAT);
+		check_added_monitors(&nodes[1], 1);
+	} else {
+		// Now manually create the commitment_signed message corresponding to the update_add
+		// nodes[0] just sent. In the code for construction of this message, "local" refers
+		// to the sender of the message, and "remote" refers to the receiver.
+
+		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();
+			let local_chan =
+				chan_lock.channel_by_id.get(&chan_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
+			chan_signer.as_ecdsa().unwrap().get_enforcement_state().last_holder_commitment -= 1;
+
+			(
+				chan_signer
+					.as_ref()
+					.release_commitment_secret(
+						INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64 + 1,
+					)
+					.unwrap(),
+				chan_signer
+					.as_ref()
+					.get_per_commitment_point(
+						INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64,
+						&secp_ctx,
+					)
+					.unwrap(),
+			)
+		};
+		let 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, chan_id);
+			let chan_signer = channel.as_funded().unwrap().get_signer();
+			chan_signer
+				.as_ref()
+				.get_per_commitment_point(
+					INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64,
+					&secp_ctx,
+				)
+				.unwrap()
+		};
+
+		// Build the remote commitment transaction so we can sign it, and then later use the
+		// signature for the commitment_signed message.
+		let local_chan_balance = node_0_balance_sat
+			- HTLC_AMT_SAT * MIN_AFFORDABLE_HTLC_COUNT as u64
+			- 2 * crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI
+			- chan_utils::commit_tx_fee_sat(
+				FEERATE_PER_KW,
+				MIN_AFFORDABLE_HTLC_COUNT,
+				&channel_type,
+			);
+
+		let accepted_htlc_info = chan_utils::HTLCOutputInCommitment {
+			offered: false,
+			amount_msat: HTLC_AMT_SAT * 1000,
+			cltv_expiry,
+			payment_hash: payment_hash_0_1,
+			transaction_output_index: None,
+		};
+		htlcs.push(accepted_htlc_info);
+
+		let commitment_number = INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64;
+
+		let res = {
+			let per_peer_lock;
+			let mut peer_state_lock;
+
+			let channel =
+				get_channel_ref!(nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan_id);
+			let chan_signer = channel.as_funded().unwrap().get_signer();
+
+			let commitment_tx = CommitmentTransaction::new(
+				commitment_number,
+				&remote_point,
+				node_1_balance_sat,
+				local_chan_balance,
+				FEERATE_PER_KW,
+				htlcs,
+				&channel.funding().channel_transaction_parameters.as_counterparty_broadcastable(),
+				&secp_ctx,
+			);
+			let params = &channel.funding().channel_transaction_parameters;
+			chan_signer
+				.as_ecdsa()
+				.unwrap()
+				.sign_counterparty_commitment(
+					params,
+					&commitment_tx,
+					Vec::new(),
+					Vec::new(),
+					&secp_ctx,
+				)
+				.unwrap()
+		};
+
+		let commit_signed_msg = msgs::CommitmentSigned {
+			channel_id: chan_id,
+			signature: res.0,
+			htlc_signatures: res.1,
+			funding_txid: None,
+			#[cfg(taproot)]
+			partial_signature_with_nonce: None,
+		};
+
+		// Send the commitment_signed message to the nodes[1].
+		nodes[1].node.handle_commitment_signed(node_a_id, &commit_signed_msg);
+		let _ = nodes[1].node.get_and_clear_pending_msg_events();
+
+		// Send the RAA to nodes[1].
+		let raa_msg = msgs::RevokeAndACK {
+			channel_id: chan_id,
+			per_commitment_secret: local_secret,
+			next_per_commitment_point: next_local_point,
+			#[cfg(taproot)]
+			next_local_nonce: None,
+		};
+		nodes[1].node.handle_revoke_and_ack(node_a_id, &raa_msg);
+
+		// The HTLC actually fails here in `fn validate_commitment_signed` due to a fee spike buffer
+		// violation. It nonetheless passed all checks in `fn validate_update_add_htlc`.
+
+		expect_pending_htlcs_forwardable!(nodes[1]);
+		expect_htlc_handling_failed_destinations!(
+			nodes[1].node.get_and_clear_pending_events(),
+			&[HTLCHandlingFailureType::Receive { payment_hash: payment_hash_0_1 }]
+		);
+
+		let events = nodes[1].node.get_and_clear_pending_msg_events();
+		assert_eq!(events.len(), 1);
+		// Make sure the HTLC failed in the way we expect.
+		match events[0] {
+			MessageSendEvent::UpdateHTLCs {
+				updates: msgs::CommitmentUpdate { ref update_fail_htlcs, .. },
+				..
+			} => {
+				assert_eq!(update_fail_htlcs.len(), 1);
+				update_fail_htlcs[0].clone()
+			},
+			_ => panic!("Unexpected event"),
+		};
+		nodes[1].logger.assert_log("lightning::ln::channel",
+			format!("Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", raa_msg.channel_id), 1);
+
+		check_added_monitors(&nodes[1], 3);
+	}
+}