diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 5734c9aa073..302dc87a1e2 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/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; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 40cfdcc46f0..edaec16495f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4323,7 +4323,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 @@ -5750,7 +5750,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, @@ -6061,10 +6061,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); @@ -14202,7 +14198,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}; @@ -14419,26 +14414,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(); @@ -14618,53 +14603,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::>()), - 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); @@ -15299,7 +15237,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!(); } @@ -15318,7 +15256,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] @@ -15338,8 +15276,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 diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 3da1830dfc7..193cdd1582a 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/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, current_height: u32, accept_mpp_keysend: bool, + counterparty_skimmed_fee_msat: Option, current_height: u32 ) -> Result { 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( msg: &msgs::UpdateAddHTLC, node_signer: NS, logger: L, secp_ctx: &Secp256k1, - cur_height: u32, accept_mpp_keysend: bool, allow_skimmed_fees: bool, + cur_height: u32, allow_skimmed_fees: bool, ) -> Result 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 { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 82d47135b09..a6cd55ba056 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/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 { diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 14529327ef2..745e13ae6c3 100644 --- a/lightning/src/util/config.rs +++ b/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)?, }) } diff --git a/pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt b/pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt new file mode 100644 index 00000000000..f52d123d2ac --- /dev/null +++ b/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.