Test the fee spike buffer multiple affordability check

Make sure it correctly does not trim HTLCs at the spiked feerate, see
the previous commit for further details.

Author: tankyleo

lightning/src/ln/htlc_reserve_unit_tests.rs

@@ -3,8 +3,8 @@
 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,
@@ -888,7 +888,7 @@ pub fn do_test_fee_spike_buffer(cfg: Option<UserConfig>, 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,
@@ -2143,7 +2143,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
@@ -2257,7 +2257,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,
@@ -2838,21 +2838,30 @@ fn do_test_0reserve_no_outputs_legacy(no_outputs_case: LegacyChannelsNoOutputs)
 			return;
 		}
 
+		let htlcs_in_commitment = vec![HTLCOutputInCommitment {
+			offered: false,
+			amount_msat: receiver_amount_msat,
+			cltv_expiry: htlc_cltv,
+			payment_hash,
+			transaction_output_index: Some(1),
+		}];
+
 		manually_trigger_update_fail_htlc(
 			&nodes,
 			channel_id,
-			channel_value_sat,
+			channel_value_sat * 1000,
 			dust_limit_satoshis,
-			receiver_amount_msat,
-			htlc_cltv,
 			payment_hash,
+			htlcs_in_commitment,
+			false,
 		);
 	}
 }
 
 fn manually_trigger_update_fail_htlc<'a, 'b, 'c, 'd>(
-	nodes: &'a Vec<Node<'b, 'c, 'd>>, channel_id: ChannelId, channel_value_sat: u64,
-	dust_limit_satoshis: u64, receiver_amount_msat: u64, htlc_cltv: u32, payment_hash: PaymentHash,
+	nodes: &'a Vec<Node<'b, 'c, 'd>>, channel_id: ChannelId, value_to_self_msat: u64,
+	dust_limit_satoshis: u64, payment_hash: PaymentHash,
+	htlcs_in_commitment: Vec<HTLCOutputInCommitment>, 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();
@@ -2864,45 +2873,36 @@ 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();
 		let local_chan =
 			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;
@@ -2913,12 +2913,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,
@@ -2969,11 +2969,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);
 }
@@ -3640,3 +3642,199 @@ fn test_0reserve_outbound_vs_available_capacity_outbound_htlc_limit() {
 	assert_eq!(node_1_details.outbound_capacity_msat, 0);
 	assert_eq!(nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat, 0);
 }
+
+/// 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,
+	);
+}