Merge pull request #3439 from valentinewallace/2024-12-remove-mpp-keysend-cfg

Remove `UserConfig::accept_mpp_keysend`

Author: TheBlueMatt

lightning/src/ln/blinded_payment_tests.rs

@@ -1214,11 +1214,9 @@ fn conditionally_round_fwd_amt() {
 #[test]
 #[cfg(async_payments)]
 fn blinded_keysend() {
-	let mut mpp_keysend_config = test_default_channel_config();
-	mpp_keysend_config.accept_mpp_keysend = true;
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, Some(mpp_keysend_config)]);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
 	let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;
@@ -1254,11 +1252,9 @@ fn blinded_keysend() {
 #[test]
 #[cfg(async_payments)]
 fn blinded_mpp_keysend() {
-	let mut mpp_keysend_config = test_default_channel_config();
-	mpp_keysend_config.accept_mpp_keysend = true;
 	let chanmon_cfgs = create_chanmon_cfgs(4);
 	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, Some(mpp_keysend_config)]);
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
 	let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
 
 	create_announced_chan_between_nodes(&nodes, 0, 1);
@@ -1315,11 +1311,9 @@ fn blinded_mpp_keysend() {
 #[test]
 #[cfg(async_payments)]
 fn invalid_keysend_payment_secret() {
-	let mut mpp_keysend_config = test_default_channel_config();
-	mpp_keysend_config.accept_mpp_keysend = true;
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, Some(mpp_keysend_config)]);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
 	let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;

lightning/src/ln/channelmanager.rs

@@ -4322,7 +4322,7 @@ where
 				let current_height: u32 = self.best_block.read().unwrap().height;
 				match create_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash,
 					msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat,
-					current_height, self.default_configuration.accept_mpp_keysend)
+					current_height)
 				{
 					Ok(info) => {
 						// Note that we could obviously respond immediately with an update_fulfill_htlc
@@ -5749,7 +5749,7 @@ where
 														match create_recv_pending_htlc_info(hop_data,
 															incoming_shared_secret, payment_hash, outgoing_amt_msat,
 															outgoing_cltv_value, Some(phantom_shared_secret), false, None,
-															current_height, self.default_configuration.accept_mpp_keysend)
+															current_height)
 														{
 															Ok(info) => phantom_receives.push((
 																prev_short_channel_id, prev_counterparty_node_id, prev_funding_outpoint,
@@ -6060,10 +6060,6 @@ where
 											log_trace!(self.logger, "Failing new {} HTLC with payment_hash {} as we already had an existing {} HTLC with the same payment hash", log_keysend(is_keysend), &payment_hash, log_keysend(!is_keysend));
 											fail_htlc!(claimable_htlc, payment_hash);
 										}
-										if !self.default_configuration.accept_mpp_keysend && is_keysend && !claimable_payment.htlcs.is_empty() {
-											log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash and our config states we don't accept MPP keysend", &payment_hash);
-											fail_htlc!(claimable_htlc, payment_hash);
-										}
 										if let Some(earlier_fields) = &mut claimable_payment.onion_fields {
 											if earlier_fields.check_merge(&mut onion_fields).is_err() {
 												fail_htlc!(claimable_htlc, payment_hash);
@@ -14201,7 +14197,6 @@ where
 #[cfg(test)]
 mod tests {
 	use bitcoin::hashes::Hash;
-	use bitcoin::hashes::sha256::Hash as Sha256;
 	use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
 	use core::sync::atomic::Ordering;
 	use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
@@ -14418,26 +14413,16 @@ mod tests {
 
 	#[test]
 	fn test_keysend_dup_payment_hash() {
-		do_test_keysend_dup_payment_hash(false);
-		do_test_keysend_dup_payment_hash(true);
-	}
-
-	fn do_test_keysend_dup_payment_hash(accept_mpp_keysend: bool) {
 		// (1): Test that a keysend payment with a duplicate payment hash to an existing pending
 		//      outbound regular payment fails as expected.
 		// (2): Test that a regular payment with a duplicate payment hash to an existing keysend payment
 		//      fails as expected.
 		// (3): Test that a keysend payment with a duplicate payment hash to an existing keysend
-		//      payment fails as expected. When `accept_mpp_keysend` is false, this tests that we
-		//      reject MPP keysend payments, since in this case where the payment has no payment
-		//      secret, a keysend payment with a duplicate hash is basically an MPP keysend. If
-		//      `accept_mpp_keysend` is true, this tests that we only accept MPP keysends with
-		//      payment secrets and reject otherwise.
+		//      payment fails as expected. We only accept MPP keysends with payment secrets and reject
+		//      otherwise.
 		let chanmon_cfgs = create_chanmon_cfgs(2);
 		let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-		let mut mpp_keysend_cfg = test_default_channel_config();
-		mpp_keysend_cfg.accept_mpp_keysend = accept_mpp_keysend;
-		let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(mpp_keysend_cfg)]);
+		let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 		let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 		create_announced_chan_between_nodes(&nodes, 0, 1);
 		let scorer = test_utils::TestScorer::new();
@@ -14617,53 +14602,6 @@ mod tests {
 		nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Payment preimage didn't match payment hash", 1);
 	}
 
-	#[test]
-	fn test_keysend_msg_with_secret_err() {
-		// Test that we error as expected if we receive a keysend payment that includes a payment
-		// secret when we don't support MPP keysend.
-		let mut reject_mpp_keysend_cfg = test_default_channel_config();
-		reject_mpp_keysend_cfg.accept_mpp_keysend = false;
-		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, Some(reject_mpp_keysend_cfg)]);
-		let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
-		let payer_pubkey = nodes[0].node.get_our_node_id();
-		let payee_pubkey = nodes[1].node.get_our_node_id();
-
-		let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
-		let route_params = RouteParameters::from_payment_params_and_value(
-			PaymentParameters::for_keysend(payee_pubkey, 40, false), 10_000);
-		let network_graph = nodes[0].network_graph;
-		let first_hops = nodes[0].node.list_usable_channels();
-		let scorer = test_utils::TestScorer::new();
-		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
-		let route = find_route(
-			&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
-			nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
-		).unwrap();
-
-		let test_preimage = PaymentPreimage([42; 32]);
-		let test_secret = PaymentSecret([43; 32]);
-		let payment_hash = PaymentHash(Sha256::hash(&test_preimage.0).to_byte_array());
-		let session_privs = nodes[0].node.test_add_new_pending_payment(payment_hash,
-			RecipientOnionFields::secret_only(test_secret), PaymentId(payment_hash.0), &route).unwrap();
-		nodes[0].node.test_send_payment_internal(&route, payment_hash,
-			RecipientOnionFields::secret_only(test_secret), Some(test_preimage),
-			PaymentId(payment_hash.0), None, session_privs).unwrap();
-		check_added_monitors!(nodes[0], 1);
-
-		let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
-		assert_eq!(updates.update_add_htlcs.len(), 1);
-		assert!(updates.update_fulfill_htlcs.is_empty());
-		assert!(updates.update_fail_htlcs.is_empty());
-		assert!(updates.update_fail_malformed_htlcs.is_empty());
-		assert!(updates.update_fee.is_none());
-		nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
-
-		nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "We don't support MPP keysend payments", 1);
-	}
-
 	#[test]
 	fn test_multi_hop_missing_secret() {
 		let chanmon_cfgs = create_chanmon_cfgs(4);
@@ -15298,7 +15236,7 @@ mod tests {
 		if let Err(crate::ln::channelmanager::InboundHTLCErr { err_code, .. }) =
 			create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
 				sender_intended_amt_msat - extra_fee_msat - 1, 42, None, true, Some(extra_fee_msat),
-				current_height, node[0].node.default_configuration.accept_mpp_keysend)
+				current_height)
 		{
 			assert_eq!(err_code, 19);
 		} else { panic!(); }
@@ -15317,7 +15255,7 @@ mod tests {
 		let current_height: u32 = node[0].node.best_block.read().unwrap().height;
 		assert!(create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
 			sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat),
-			current_height, node[0].node.default_configuration.accept_mpp_keysend).is_ok());
+			current_height).is_ok());
 	}
 
 	#[test]
@@ -15337,8 +15275,7 @@ mod tests {
 				payment_secret: PaymentSecret([0; 32]), total_msat: 100,
 			}),
 			custom_tlvs: Vec::new(),
-		}, [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height,
-			node[0].node.default_configuration.accept_mpp_keysend);
+		}, [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height);
 
 		// Should not return an error as this condition:
 		// https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334

lightning/src/ln/onion_payment.rs

@@ -131,7 +131,7 @@ pub(super) fn create_fwd_pending_htlc_info(
 pub(super) fn create_recv_pending_htlc_info(
 	hop_data: msgs::InboundOnionPayload, shared_secret: [u8; 32], payment_hash: PaymentHash,
 	amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool,
-	counterparty_skimmed_fee_msat: Option<u64>, current_height: u32, accept_mpp_keysend: bool,
+	counterparty_skimmed_fee_msat: Option<u64>, current_height: u32
 ) -> Result<PendingHTLCInfo, InboundHTLCErr> {
 	let (
 		payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, onion_cltv_expiry,
@@ -227,13 +227,6 @@ pub(super) fn create_recv_pending_htlc_info(
 				msg: "Payment preimage didn't match payment hash",
 			});
 		}
-		if !accept_mpp_keysend && payment_data.is_some() {
-			return Err(InboundHTLCErr {
-				err_code: 0x4000|22,
-				err_data: Vec::new(),
-				msg: "We don't support MPP keysend payments",
-			});
-		}
 		PendingHTLCRouting::ReceiveKeysend {
 			payment_data,
 			payment_preimage,
@@ -282,7 +275,7 @@ pub(super) fn create_recv_pending_htlc_info(
 /// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable
 pub fn peel_payment_onion<NS: Deref, L: Deref, T: secp256k1::Verification>(
 	msg: &msgs::UpdateAddHTLC, node_signer: NS, logger: L, secp_ctx: &Secp256k1<T>,
-	cur_height: u32, accept_mpp_keysend: bool, allow_skimmed_fees: bool,
+	cur_height: u32, allow_skimmed_fees: bool,
 ) -> Result<PendingHTLCInfo, InboundHTLCErr>
 where
 	NS::Target: NodeSigner,
@@ -333,7 +326,7 @@ where
 		onion_utils::Hop::Receive(received_data) => {
 			create_recv_pending_htlc_info(
 				received_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
-				None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height, accept_mpp_keysend,
+				None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height
 			)?
 		}
 	})
@@ -576,7 +569,7 @@ mod tests {
 		let msg = make_update_add_msg(amount_msat, cltv_expiry, payment_hash, onion);
 		let logger = test_utils::TestLogger::with_id("bob".to_string());
 
-		let peeled = peel_payment_onion(&msg, &bob, &logger, &secp_ctx, cur_height, true, false)
+		let peeled = peel_payment_onion(&msg, &bob, &logger, &secp_ctx, cur_height, false)
 			.map_err(|e| e.msg).unwrap();
 
 		let next_onion = match peeled.routing {
@@ -587,7 +580,7 @@ mod tests {
 		};
 
 		let msg2 = make_update_add_msg(amount_msat, cltv_expiry, payment_hash, next_onion);
-		let peeled2 = peel_payment_onion(&msg2, &charlie, &logger, &secp_ctx, cur_height, true, false)
+		let peeled2 = peel_payment_onion(&msg2, &charlie, &logger, &secp_ctx, cur_height, false)
 			.map_err(|e| e.msg).unwrap();
 
 		match peeled2.routing {

lightning/src/ln/payment_tests.rs

@@ -432,11 +432,9 @@ fn do_test_keysend_payments(public_node: bool, with_retry: bool) {
 
 #[test]
 fn test_mpp_keysend() {
-	let mut mpp_keysend_config = test_default_channel_config();
-	mpp_keysend_config.accept_mpp_keysend = true;
 	let chanmon_cfgs = create_chanmon_cfgs(4);
 	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, Some(mpp_keysend_config)]);
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
 	let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
 
 	create_announced_chan_between_nodes(&nodes, 0, 1);
@@ -478,19 +476,14 @@ fn test_mpp_keysend() {
 }
 
 #[test]
-fn test_reject_mpp_keysend_htlc() {
-	// This test enforces that we reject MPP keysend HTLCs if our config states we don't support
-	// MPP keysend. When receiving a payment, if we don't support MPP keysend we'll reject the
-	// payment if it's keysend and has a payment secret, never reaching our payment validation
-	// logic. To check that we enforce rejecting MPP keysends in our payment logic, here we send
+fn test_reject_mpp_keysend_htlc_mismatching_secret() {
+	// This test enforces that we reject MPP keysend HTLCs if the payment_secrets between MPP parts
+	// don't match. To check that we enforce rejecting MPP keysends in our payment logic, here we send
 	// keysend payments without payment secrets, then modify them by adding payment secrets in the
 	// final node in between receiving the HTLCs and actually processing them.
-	let mut reject_mpp_keysend_cfg = test_default_channel_config();
-	reject_mpp_keysend_cfg.accept_mpp_keysend = false;
-
 	let chanmon_cfgs = create_chanmon_cfgs(4);
 	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, Some(reject_mpp_keysend_cfg)]);
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
 	let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
 	let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
 	let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 2).0.contents.short_channel_id;
@@ -571,7 +564,7 @@ fn test_reject_mpp_keysend_htlc() {
 					match forward_info.routing {
 						PendingHTLCRouting::ReceiveKeysend { ref mut payment_data, .. } => {
 							*payment_data = Some(msgs::FinalOnionHopData {
-								payment_secret: PaymentSecret([42; 32]),
+								payment_secret: PaymentSecret([43; 32]), // Doesn't match the secret used above
 								total_msat: amount * 2,
 							});
 						},
@@ -4282,7 +4275,7 @@ fn peel_payment_onion_custom_tlvs() {
 	};
 	let peeled_onion = crate::ln::onion_payment::peel_payment_onion(
 		&update_add, &chanmon_cfgs[1].keys_manager, &chanmon_cfgs[1].logger, &secp_ctx,
-		nodes[1].best_block_info().1, true, false
+		nodes[1].best_block_info().1, false
 	).unwrap();
 	assert_eq!(peeled_onion.incoming_amt_msat, Some(amt_msat));
 	match peeled_onion.routing {

lightning/src/util/config.rs

@@ -844,17 +844,6 @@ pub struct UserConfig {
 	/// [`ChannelManager::get_intercept_scid`]: crate::ln::channelmanager::ChannelManager::get_intercept_scid
 	/// [`Event::HTLCIntercepted`]: crate::events::Event::HTLCIntercepted
 	pub accept_intercept_htlcs: bool,
-	/// If this is set to `false`, when receiving a keysend payment we'll fail it if it has multiple
-	/// parts. If this is set to `true`, we'll accept the payment.
-	///
-	/// Setting this to `true` will break backwards compatibility upon downgrading to an LDK
-	/// version prior to 0.0.116 while receiving an MPP keysend. If we have already received an MPP
-	/// keysend, downgrading will cause us to fail to deserialize [`ChannelManager`].
-	///
-	/// Default value: `false`
-	///
-	/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
-	pub accept_mpp_keysend: bool,
 	/// If this is set to `true`, the user needs to manually pay [`Bolt12Invoice`]s when received.
 	///
 	/// When set to `true`, [`Event::InvoiceReceived`] will be generated for each received
@@ -881,7 +870,6 @@ impl Default for UserConfig {
 			accept_inbound_channels: true,
 			manually_accept_inbound_channels: false,
 			accept_intercept_htlcs: false,
-			accept_mpp_keysend: false,
 			manually_handle_bolt12_invoices: false,
 		}
 	}
@@ -901,7 +889,6 @@ impl Readable for UserConfig {
 			accept_inbound_channels: Readable::read(reader)?,
 			manually_accept_inbound_channels: Readable::read(reader)?,
 			accept_intercept_htlcs: Readable::read(reader)?,
-			accept_mpp_keysend: Readable::read(reader)?,
 			manually_handle_bolt12_invoices: Readable::read(reader)?,
 		})
 	}

pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt

@@ -0,0 +1,3 @@
+# Backwards Compatibility
+* The presence of pending inbound MPP keysend payments breaks downgrades to LDK versions 0.0.115 and
+	earlier.