Remove all tests covering 0-reserve legacy channels

Author: tankyleo

lightning/src/ln/htlc_reserve_unit_tests.rs

@@ -39,9 +39,7 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) {
 	// in normal testing, we test it explicitly here.
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-	let legacy_cfg = test_legacy_channel_config();
-	let node_chanmgrs =
-		create_node_chanmgrs(2, &node_cfgs, &[Some(legacy_cfg.clone()), Some(legacy_cfg)]);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
 	let node_a_id = nodes[0].node.get_our_node_id();
@@ -52,13 +50,14 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) {
 	// Have node0 initiate a channel to node1 with aforementioned parameters
 	let mut push_amt = 100_000_000;
 	let feerate_per_kw = 253;
-	let channel_type_features = ChannelTypeFeatures::only_static_remote_key();
+	let channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies();
 	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)
 		.unwrap()
 		* 1000;
+	push_amt -= 2 * 330_000;
 
 	let push = if send_from_initiator { 0 } else { push_amt };
 	let temp_channel_id =
@@ -109,10 +108,8 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) {
 			&nodes[0],
 			&[&nodes[1]],
 			100_000_000
-			// Note that for outbound channels we have to consider the commitment tx fee and the
-			// "fee spike buffer", which is currently a multiple of the total commitment tx fee as
-			// well as an additional HTLC.
-			- FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * commit_tx_fee_msat(feerate_per_kw, 2, &channel_type_features),
+				- commit_tx_fee_msat(feerate_per_kw, 2, &channel_type_features)
+				- 2 * 330_000,
 		);
 	} else {
 		send_payment(&nodes[1], &[&nodes[0]], push_amt);
@@ -2358,12 +2355,6 @@ pub fn do_test_dust_limit_fee_accounting(can_afford: bool) {
 fn test_create_channel_to_trusted_peer_0reserve() {
 	let mut config = test_default_channel_config();
 
-	// Legacy channels
-	config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
-	config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
-	let channel_type = do_test_create_channel_to_trusted_peer_0reserve(config.clone());
-	assert_eq!(channel_type, ChannelTypeFeatures::only_static_remote_key());
-
 	// Anchor channels
 	config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
 	config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
@@ -2416,14 +2407,8 @@ fn do_test_create_channel_to_trusted_peer_0reserve(mut config: UserConfig) -> Ch
 		} else {
 			0
 		};
-	let spike_multiple = if channel_type == ChannelTypeFeatures::only_static_remote_key() {
-		FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
-	} else {
-		1
-	};
-	let spiked_feerate = spike_multiple * feerate_per_kw;
 	let reserved_commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(
-		spiked_feerate,
+		feerate_per_kw,
 		2, // We reserve space for two HTLCs, the next outbound non-dust HTLC, and the fee spike buffer HTLC
 		&channel_type,
 	);
@@ -2446,12 +2431,6 @@ fn do_test_create_channel_to_trusted_peer_0reserve(mut config: UserConfig) -> Ch
 fn test_accept_inbound_channel_from_trusted_peer_0reserve() {
 	let mut config = test_default_channel_config();
 
-	// Legacy channels
-	config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
-	config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
-	let channel_type = do_test_accept_inbound_channel_from_trusted_peer_0reserve(config.clone());
-	assert_eq!(channel_type, ChannelTypeFeatures::only_static_remote_key());
-
 	// Anchor channels
 	config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
 	config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
@@ -2539,14 +2518,8 @@ fn do_test_accept_inbound_channel_from_trusted_peer_0reserve(
 		} else {
 			0
 		};
-	let spike_multiple = if channel_type == ChannelTypeFeatures::only_static_remote_key() {
-		FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
-	} else {
-		1
-	};
-	let spiked_feerate = spike_multiple * feerate_per_kw;
 	let reserved_commit_tx_fee_sat = chan_utils::commit_tx_fee_sat(
-		spiked_feerate,
+		feerate_per_kw,
 		2, // We reserve space for two HTLCs, the next outbound non-dust HTLC, and the fee spike buffer HTLC
 		&channel_type,
 	);
@@ -2565,20 +2538,8 @@ fn do_test_accept_inbound_channel_from_trusted_peer_0reserve(
 	channel_type
 }
 
-enum LegacyChannelsNoOutputs {
-	PaymentSucceeds,
-	FailsReceiverUpdateAddHTLC,
-	FailsReceiverCanAcceptHTLCA,
-	FailsReceiverCanAcceptHTLCB,
-}
-
 #[xtest(feature = "_externalize_tests")]
 fn test_0reserve_no_outputs() {
-	do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::PaymentSucceeds);
-	do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCA);
-	do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCB);
-	do_test_0reserve_no_outputs_legacy(LegacyChannelsNoOutputs::FailsReceiverUpdateAddHTLC);
-
 	do_test_0reserve_no_outputs_keyed_anchors(true);
 	do_test_0reserve_no_outputs_keyed_anchors(false);
 
@@ -2678,189 +2639,6 @@ pub(crate) fn setup_0reserve_no_outputs_channels<'a, 'b, 'c, 'd>(
 	(channel_id, tx)
 }
 
-fn do_test_0reserve_no_outputs_legacy(no_outputs_case: LegacyChannelsNoOutputs) {
-	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();
-
-	let feerate_per_kw = 253;
-	let spike_multiple = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
-	let dust_limit_satoshis: u64 = 546;
-	let channel_value_sat = 1000;
-
-	let (channel_id, _funding_tx) =
-		setup_0reserve_no_outputs_channels(&nodes, channel_value_sat, dust_limit_satoshis);
-	assert_eq!(nodes[0].node.list_channels()[0].channel_type.as_ref().unwrap(), &channel_type);
-
-	// Sending the biggest dust HTLC possible trims our balance output!
-	let (timeout_tx_fee_sat, success_tx_fee_sat) =
-		second_stage_tx_fees_sat(&channel_type, spike_multiple * feerate_per_kw);
-	let max_dust_htlc_sat = dust_limit_satoshis + success_tx_fee_sat - 1;
-	assert!(
-		channel_value_sat
-			.saturating_sub(commit_tx_fee_sat(feerate_per_kw, 0, &channel_type))
-			.saturating_sub(max_dust_htlc_sat)
-			< dust_limit_satoshis
-	);
-
-	// We can't afford the fee for an additional non-dust HTLC + the fee spike HTLC, so we can only send
-	// dust HTLCs...
-	let min_local_nondust_htlc_sat = dust_limit_satoshis + timeout_tx_fee_sat;
-	assert!(
-		channel_value_sat - commit_tx_fee_sat(spike_multiple * feerate_per_kw, 2, &channel_type)
-			< min_local_nondust_htlc_sat
-	);
-
-	// We cannot trim our own balance output, otherwise we'd have no outputs on the commitment. We must
-	// also reserve enough fees to pay for an incoming non-dust HTLC, aka the fee spike buffer HTLC.
-	let min_value_sat = core::cmp::max(
-		commit_tx_fee_sat(spike_multiple * feerate_per_kw, 0, &channel_type) + dust_limit_satoshis,
-		commit_tx_fee_sat(spike_multiple * feerate_per_kw, 1, &channel_type),
-	);
-	// At this point the tighter requirement is "must have an output"
-	assert!(
-		commit_tx_fee_sat(spike_multiple * feerate_per_kw, 0, &channel_type) + dust_limit_satoshis
-			> commit_tx_fee_sat(spike_multiple * feerate_per_kw, 1, &channel_type)
-	);
-	// But say at 9sat/vb with default dust limit,
-	// the tighter requirement is actually "must have funds for an inbound HTLC" !
-	assert!(
-		commit_tx_fee_sat(9 * 250, 0, &channel_type) + 354
-			< commit_tx_fee_sat(9 * 250, 1, &channel_type)
-	);
-	let sender_amount_msat = (channel_value_sat - min_value_sat) * 1000;
-	let details_0 = &nodes[0].node.list_channels()[0];
-	assert_eq!(details_0.next_outbound_htlc_minimum_msat, 1000);
-	assert_eq!(details_0.next_outbound_htlc_limit_msat, sender_amount_msat);
-	assert!(details_0.next_outbound_htlc_limit_msat > details_0.next_outbound_htlc_minimum_msat);
-
-	let (sender_amount_msat, receiver_amount_msat) = match no_outputs_case {
-		LegacyChannelsNoOutputs::PaymentSucceeds => (sender_amount_msat, sender_amount_msat),
-		LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCA => {
-			// A dust HTLC with 1msat added to it will break counterparty `can_accept_incoming_htlc`
-			// validation, as this dust HTLC would push the holder's balance output below the
-			// dust limit at the spike multiple feerate.
-			(sender_amount_msat, sender_amount_msat + 1)
-		},
-		LegacyChannelsNoOutputs::FailsReceiverCanAcceptHTLCB => {
-			// In `validate_update_add_htlc`, we check that there is still some output present on
-			// the commitment given the *current* set of HTLCs, and the *current* feerate. So this
-			// HTLC will pass at `validate_update_add_htlc`, but will fail in
-			// `can_accept_incoming_htlc` due to failed fee spike buffer checks.
-			let receiver_amount_msat = (channel_value_sat
-				- commit_tx_fee_sat(feerate_per_kw, 0, &channel_type)
-				- dust_limit_satoshis)
-				* 1000;
-			(sender_amount_msat, receiver_amount_msat)
-		},
-		LegacyChannelsNoOutputs::FailsReceiverUpdateAddHTLC => {
-			// Same value as above, just add 1msat, and this fails at `validate_update_add_htlc`
-			let receiver_amount_msat = (channel_value_sat
-				- commit_tx_fee_sat(feerate_per_kw, 0, &channel_type)
-				- dust_limit_satoshis)
-				* 1000;
-			(sender_amount_msat, receiver_amount_msat + 1)
-		},
-	};
-
-	if let LegacyChannelsNoOutputs::PaymentSucceeds = no_outputs_case {
-		send_payment(&nodes[0], &[&nodes[1]], sender_amount_msat);
-		// Node 1 the fundee has 0-reserve too, so whatever they receive, they can send right back!
-		// Node 0 should *always* have the funds to cover the fee of a single non-dust HTLC from node 1.
-		assert_eq!(
-			nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat,
-			sender_amount_msat
-		);
-		send_payment(&nodes[1], &[&nodes[0]], sender_amount_msat);
-	} else {
-		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: 0,
-			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);
-
-		if let LegacyChannelsNoOutputs::FailsReceiverUpdateAddHTLC = no_outputs_case {
-			nodes[1].logger.assert_log_contains(
-				"lightning::ln::channelmanager",
-				"Remote HTLC add would overdraw remaining funds",
-				3,
-			);
-			assert_eq!(nodes[1].node.list_channels().len(), 0);
-			let err_msg = check_closed_broadcast(&nodes[1], 1, true).pop().unwrap();
-			assert_eq!(err_msg.data, "Remote HTLC add would overdraw remaining funds");
-			let reason = ClosureReason::ProcessingError {
-				err: "Remote HTLC add would overdraw remaining funds".to_string(),
-			};
-			check_added_monitors(&nodes[1], 1);
-			check_closed_event(&nodes[1], 1, reason, &[node_a_id], channel_value_sat);
-
-			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 * 1000,
-			dust_limit_satoshis,
-			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, value_to_self_msat: u64,
 	dust_limit_satoshis: u64, payment_hash: PaymentHash,