Merge pull request #2156 from alecchendev/2023-04-mpp-keysend
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sat, 10 Jun 2023 19:48:54 +0000 (19:48 +0000)
committerGitHub <noreply@github.com>
Sat, 10 Jun 2023 19:48:54 +0000 (19:48 +0000)
Support MPP Keysend

lightning/src/ln/channelmanager.rs
lightning/src/ln/features.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs
lightning/src/routing/router.rs
lightning/src/util/config.rs

index 50e65db3f67dabc6227491b634e93cb404c51787..7c065f1ba44c4d956f68d791f52ad75c8135c74e 100644 (file)
@@ -112,6 +112,8 @@ pub(super) enum PendingHTLCRouting {
                phantom_shared_secret: Option<[u8; 32]>,
        },
        ReceiveKeysend {
+               /// This was added in 0.0.116 and will break deserialization on downgrades.
+               payment_data: Option<msgs::FinalOnionHopData>,
                payment_preimage: PaymentPreimage,
                payment_metadata: Option<Vec<u8>>,
                incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed
@@ -2457,20 +2459,7 @@ where
                                });
                        },
                        msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage, payment_metadata } => {
-                               if payment_data.is_some() && keysend_preimage.is_some() {
-                                       return Err(ReceiveError {
-                                               err_code: 0x4000|22,
-                                               err_data: Vec::new(),
-                                               msg: "We don't support MPP keysend payments",
-                                       });
-                               } else if let Some(data) = payment_data {
-                                       PendingHTLCRouting::Receive {
-                                               payment_data: data,
-                                               payment_metadata,
-                                               incoming_cltv_expiry: hop_data.outgoing_cltv_value,
-                                               phantom_shared_secret,
-                                       }
-                               } else if let Some(payment_preimage) = keysend_preimage {
+                               if let Some(payment_preimage) = keysend_preimage {
                                        // We need to check that the sender knows the keysend preimage before processing this
                                        // payment further. Otherwise, an intermediary routing hop forwarding non-keysend-HTLC X
                                        // could discover the final destination of X, by probing the adjacent nodes on the route
@@ -2484,12 +2473,26 @@ where
                                                        msg: "Payment preimage didn't match payment hash",
                                                });
                                        }
-
+                                       if !self.default_configuration.accept_mpp_keysend && payment_data.is_some() {
+                                               return Err(ReceiveError {
+                                                       err_code: 0x4000|22,
+                                                       err_data: Vec::new(),
+                                                       msg: "We don't support MPP keysend payments",
+                                               });
+                                       }
                                        PendingHTLCRouting::ReceiveKeysend {
+                                               payment_data,
                                                payment_preimage,
                                                payment_metadata,
                                                incoming_cltv_expiry: hop_data.outgoing_cltv_value,
                                        }
+                               } else if let Some(data) = payment_data {
+                                       PendingHTLCRouting::Receive {
+                                               payment_data: data,
+                                               payment_metadata,
+                                               incoming_cltv_expiry: hop_data.outgoing_cltv_value,
+                                               phantom_shared_secret,
+                                       }
                                } else {
                                        return Err(ReceiveError {
                                                err_code: 0x4000|0x2000|3,
@@ -3039,8 +3042,6 @@ where
        /// Similar to regular payments, you MUST NOT reuse a `payment_preimage` value. See
        /// [`send_payment`] for more information about the risks of duplicate preimage usage.
        ///
-       /// Note that `route` must have exactly one path.
-       ///
        /// [`send_payment`]: Self::send_payment
        pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<PaymentHash, PaymentSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
@@ -3635,16 +3636,19 @@ where
                                                                                (incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data },
                                                                                        Some(payment_data), phantom_shared_secret, onion_fields)
                                                                        },
-                                                                       PendingHTLCRouting::ReceiveKeysend { payment_preimage, payment_metadata, incoming_cltv_expiry } => {
-                                                                               let onion_fields = RecipientOnionFields { payment_secret: None, payment_metadata };
+                                                                       PendingHTLCRouting::ReceiveKeysend { payment_data, payment_preimage, payment_metadata, incoming_cltv_expiry } => {
+                                                                               let onion_fields = RecipientOnionFields {
+                                                                                       payment_secret: payment_data.as_ref().map(|data| data.payment_secret),
+                                                                                       payment_metadata
+                                                                               };
                                                                                (incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage),
-                                                                                       None, None, onion_fields)
+                                                                                       payment_data, None, onion_fields)
                                                                        },
                                                                        _ => {
                                                                                panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
                                                                        }
                                                                };
-                                                               let mut claimable_htlc = ClaimableHTLC {
+                                                               let claimable_htlc = ClaimableHTLC {
                                                                        prev_hop: HTLCPreviousHopData {
                                                                                short_channel_id: prev_short_channel_id,
                                                                                outpoint: prev_funding_outpoint,
@@ -3694,13 +3698,11 @@ where
                                                                }
 
                                                                macro_rules! check_total_value {
-                                                                       ($payment_data: expr, $payment_preimage: expr) => {{
+                                                                       ($purpose: expr) => {{
                                                                                let mut payment_claimable_generated = false;
-                                                                               let purpose = || {
-                                                                                       events::PaymentPurpose::InvoicePayment {
-                                                                                               payment_preimage: $payment_preimage,
-                                                                                               payment_secret: $payment_data.payment_secret,
-                                                                                       }
+                                                                               let is_keysend = match $purpose {
+                                                                                       events::PaymentPurpose::SpontaneousPayment(_) => true,
+                                                                                       events::PaymentPurpose::InvoicePayment { .. } => false,
                                                                                };
                                                                                let mut claimable_payments = self.claimable_payments.lock().unwrap();
                                                                                if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) {
@@ -3712,9 +3714,18 @@ where
                                                                                        .or_insert_with(|| {
                                                                                                committed_to_claimable = true;
                                                                                                ClaimablePayment {
-                                                                                                       purpose: purpose(), htlcs: Vec::new(), onion_fields: None,
+                                                                                                       purpose: $purpose.clone(), htlcs: Vec::new(), onion_fields: None,
                                                                                                }
                                                                                        });
+                                                                               if $purpose != claimable_payment.purpose {
+                                                                                       let log_keysend = |keysend| if keysend { "keysend" } else { "non-keysend" };
+                                                                                       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), log_bytes!(payment_hash.0), 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", log_bytes!(payment_hash.0));
+                                                                                       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);
@@ -3723,38 +3734,27 @@ where
                                                                                        claimable_payment.onion_fields = Some(onion_fields);
                                                                                }
                                                                                let ref mut htlcs = &mut claimable_payment.htlcs;
-                                                                               if htlcs.len() == 1 {
-                                                                                       if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload {
-                                                                                               log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0));
-                                                                                               fail_htlc!(claimable_htlc, payment_hash);
-                                                                                       }
-                                                                               }
                                                                                let mut total_value = claimable_htlc.sender_intended_value;
                                                                                let mut earliest_expiry = claimable_htlc.cltv_expiry;
                                                                                for htlc in htlcs.iter() {
                                                                                        total_value += htlc.sender_intended_value;
                                                                                        earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry);
-                                                                                       match &htlc.onion_payload {
-                                                                                               OnionPayload::Invoice { .. } => {
-                                                                                                       if htlc.total_msat != $payment_data.total_msat {
-                                                                                                               log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})",
-                                                                                                                       log_bytes!(payment_hash.0), $payment_data.total_msat, htlc.total_msat);
-                                                                                                               total_value = msgs::MAX_VALUE_MSAT;
-                                                                                                       }
-                                                                                                       if total_value >= msgs::MAX_VALUE_MSAT { break; }
-                                                                                               },
-                                                                                               _ => unreachable!(),
+                                                                                       if htlc.total_msat != claimable_htlc.total_msat {
+                                                                                               log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})",
+                                                                                                       log_bytes!(payment_hash.0), claimable_htlc.total_msat, htlc.total_msat);
+                                                                                               total_value = msgs::MAX_VALUE_MSAT;
                                                                                        }
+                                                                                       if total_value >= msgs::MAX_VALUE_MSAT { break; }
                                                                                }
                                                                                // The condition determining whether an MPP is complete must
                                                                                // match exactly the condition used in `timer_tick_occurred`
                                                                                if total_value >= msgs::MAX_VALUE_MSAT {
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
-                                                                               } else if total_value - claimable_htlc.sender_intended_value >= $payment_data.total_msat {
+                                                                               } else if total_value - claimable_htlc.sender_intended_value >= claimable_htlc.total_msat {
                                                                                        log_trace!(self.logger, "Failing HTLC with payment_hash {} as payment is already claimable",
                                                                                                log_bytes!(payment_hash.0));
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
-                                                                               } else if total_value >= $payment_data.total_msat {
+                                                                               } else if total_value >= claimable_htlc.total_msat {
                                                                                        #[allow(unused_assignments)] {
                                                                                                committed_to_claimable = true;
                                                                                        }
@@ -3765,7 +3765,7 @@ where
                                                                                        new_events.push_back((events::Event::PaymentClaimable {
                                                                                                receiver_node_id: Some(receiver_node_id),
                                                                                                payment_hash,
-                                                                                               purpose: purpose(),
+                                                                                               purpose: $purpose,
                                                                                                amount_msat,
                                                                                                via_channel_id: Some(prev_channel_id),
                                                                                                via_user_channel_id: Some(prev_user_channel_id),
@@ -3813,49 +3813,23 @@ where
                                                                                                                fail_htlc!(claimable_htlc, payment_hash);
                                                                                                        }
                                                                                                }
-                                                                                               check_total_value!(payment_data, payment_preimage);
+                                                                                               let purpose = events::PaymentPurpose::InvoicePayment {
+                                                                                                       payment_preimage: payment_preimage.clone(),
+                                                                                                       payment_secret: payment_data.payment_secret,
+                                                                                               };
+                                                                                               check_total_value!(purpose);
                                                                                        },
                                                                                        OnionPayload::Spontaneous(preimage) => {
-                                                                                               let mut claimable_payments = self.claimable_payments.lock().unwrap();
-                                                                                               if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) {
-                                                                                                       fail_htlc!(claimable_htlc, payment_hash);
-                                                                                               }
-                                                                                               match claimable_payments.claimable_payments.entry(payment_hash) {
-                                                                                                       hash_map::Entry::Vacant(e) => {
-                                                                                                               let amount_msat = claimable_htlc.value;
-                                                                                                               claimable_htlc.total_value_received = Some(amount_msat);
-                                                                                                               let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER);
-                                                                                                               let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
-                                                                                                               e.insert(ClaimablePayment {
-                                                                                                                       purpose: purpose.clone(),
-                                                                                                                       onion_fields: Some(onion_fields.clone()),
-                                                                                                                       htlcs: vec![claimable_htlc],
-                                                                                                               });
-                                                                                                               let prev_channel_id = prev_funding_outpoint.to_channel_id();
-                                                                                                               new_events.push_back((events::Event::PaymentClaimable {
-                                                                                                                       receiver_node_id: Some(receiver_node_id),
-                                                                                                                       payment_hash,
-                                                                                                                       amount_msat,
-                                                                                                                       purpose,
-                                                                                                                       via_channel_id: Some(prev_channel_id),
-                                                                                                                       via_user_channel_id: Some(prev_user_channel_id),
-                                                                                                                       claim_deadline,
-                                                                                                                       onion_fields: Some(onion_fields),
-                                                                                                               }, None));
-                                                                                                       },
-                                                                                                       hash_map::Entry::Occupied(_) => {
-                                                                                                               log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} for a duplicative payment hash", log_bytes!(payment_hash.0));
-                                                                                                               fail_htlc!(claimable_htlc, payment_hash);
-                                                                                                       }
-                                                                                               }
+                                                                                               let purpose = events::PaymentPurpose::SpontaneousPayment(preimage);
+                                                                                               check_total_value!(purpose);
                                                                                        }
                                                                                }
                                                                        },
                                                                        hash_map::Entry::Occupied(inbound_payment) => {
-                                                                               if payment_data.is_none() {
+                                                                               if let OnionPayload::Spontaneous(_) = claimable_htlc.onion_payload {
                                                                                        log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", log_bytes!(payment_hash.0));
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
-                                                                               };
+                                                                               }
                                                                                let payment_data = payment_data.unwrap();
                                                                                if inbound_payment.get().payment_secret != payment_data.payment_secret {
                                                                                        log_trace!(self.logger, "Failing new HTLC with payment_hash {} as it didn't match our expected payment secret.", log_bytes!(payment_hash.0));
@@ -3865,7 +3839,11 @@ where
                                                                                                log_bytes!(payment_hash.0), payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap());
                                                                                        fail_htlc!(claimable_htlc, payment_hash);
                                                                                } else {
-                                                                                       let payment_claimable_generated = check_total_value!(payment_data, inbound_payment.get().payment_preimage);
+                                                                                       let purpose = events::PaymentPurpose::InvoicePayment {
+                                                                                               payment_preimage: inbound_payment.get().payment_preimage,
+                                                                                               payment_secret: payment_data.payment_secret,
+                                                                                       };
+                                                                                       let payment_claimable_generated = check_total_value!(purpose);
                                                                                        if payment_claimable_generated {
                                                                                                inbound_payment.remove_entry();
                                                                                        }
@@ -4445,18 +4423,6 @@ where
                                break;
                        }
                        expected_amt_msat = htlc.total_value_received;
-
-                       if let OnionPayload::Spontaneous(_) = &htlc.onion_payload {
-                               // We don't currently support MPP for spontaneous payments, so just check
-                               // that there's one payment here and move on.
-                               if sources.len() != 1 {
-                                       log_error!(self.logger, "Somehow ended up with an MPP spontaneous payment - this should not be reachable!");
-                                       debug_assert!(false);
-                                       valid_mpp = false;
-                                       break;
-                               }
-                       }
-
                        claimable_amt_msat += htlc.value;
                }
                mem::drop(per_peer_state);
@@ -7287,6 +7253,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
                (0, payment_preimage, required),
                (2, incoming_cltv_expiry, required),
                (3, payment_metadata, option),
+               (4, payment_data, option), // Added in 0.0.116
        },
 ;);
 
@@ -8829,13 +8796,26 @@ 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.
                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, None]);
+               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 nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1);
                let scorer = test_utils::TestScorer::new();
@@ -8847,7 +8827,7 @@ mod tests {
 
                // Next, attempt a keysend payment and make sure it fails.
                let route_params = RouteParameters {
-                       payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV),
+                       payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV, false),
                        final_value_msat: 100_000,
                };
                let route = find_route(
@@ -8924,6 +8904,53 @@ mod tests {
 
                // Finally, succeed the keysend payment.
                claim_payment(&nodes[0], &expected_route, payment_preimage);
+
+               // To start (3), send a keysend payment but don't claim it.
+               let payment_id_1 = PaymentId([44; 32]);
+               let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
+                       RecipientOnionFields::spontaneous_empty(), payment_id_1).unwrap();
+               check_added_monitors!(nodes[0], 1);
+               let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+               assert_eq!(events.len(), 1);
+               let event = events.pop().unwrap();
+               let path = vec![&nodes[1]];
+               pass_along_path(&nodes[0], &path, 100_000, payment_hash, None, event, true, Some(payment_preimage));
+
+               // Next, attempt a keysend payment and make sure it fails.
+               let route_params = RouteParameters {
+                       payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV, false),
+                       final_value_msat: 100_000,
+               };
+               let route = find_route(
+                       &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
+                       None, nodes[0].logger, &scorer, &(), &random_seed_bytes
+               ).unwrap();
+               let payment_id_2 = PaymentId([45; 32]);
+               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
+                       RecipientOnionFields::spontaneous_empty(), payment_id_2).unwrap();
+               check_added_monitors!(nodes[0], 1);
+               let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+               assert_eq!(events.len(), 1);
+               let ev = events.drain(..).next().unwrap();
+               let payment_event = SendEvent::from_event(ev);
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+               check_added_monitors!(nodes[1], 0);
+               commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+               expect_pending_htlcs_forwardable!(nodes[1]);
+               expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], vec![HTLCDestination::FailedPayment { payment_hash }]);
+               check_added_monitors!(nodes[1], 1);
+               let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               assert!(updates.update_add_htlcs.is_empty());
+               assert!(updates.update_fulfill_htlcs.is_empty());
+               assert_eq!(updates.update_fail_htlcs.len(), 1);
+               assert!(updates.update_fail_malformed_htlcs.is_empty());
+               assert!(updates.update_fee.is_none());
+               nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
+               commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, true, true);
+               expect_payment_failed!(nodes[0], payment_hash, true);
+
+               // Finally, claim the original payment.
+               claim_payment(&nodes[0], &expected_route, payment_preimage);
        }
 
        #[test]
@@ -8940,7 +8967,7 @@ mod tests {
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
                let route_params = RouteParameters {
-                       payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
+                       payment_params: PaymentParameters::for_keysend(payee_pubkey, 40, false),
                        final_value_msat: 10_000,
                };
                let network_graph = nodes[0].network_graph.clone();
@@ -8973,10 +9000,13 @@ mod tests {
 
        #[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.
+               // 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, None]);
+               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();
@@ -8984,7 +9014,7 @@ mod tests {
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
                let route_params = RouteParameters {
-                       payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
+                       payment_params: PaymentParameters::for_keysend(payee_pubkey, 40, false),
                        final_value_msat: 10_000,
                };
                let network_graph = nodes[0].network_graph.clone();
index b8087546c7445060230a5ad776a9b3f13c113d37..41b15c97c9ae63c5aaae4f4f3ef1a810da11efaa 100644 (file)
@@ -535,11 +535,17 @@ impl InvoiceFeatures {
        /// [`PaymentParameters::for_keysend`], thus omitting the need for payers to manually construct an
        /// `InvoiceFeatures` for [`find_route`].
        ///
+       /// MPP keysend is not widely supported yet, so we parameterize support to allow the user to
+       /// choose whether their router should find multi-part routes.
+       ///
        /// [`PaymentParameters::for_keysend`]: crate::routing::router::PaymentParameters::for_keysend
        /// [`find_route`]: crate::routing::router::find_route
-       pub(crate) fn for_keysend() -> InvoiceFeatures {
+       pub(crate) fn for_keysend(allow_mpp: bool) -> InvoiceFeatures {
                let mut res = InvoiceFeatures::empty();
                res.set_variable_length_onion_optional();
+               if allow_mpp {
+                       res.set_basic_mpp_optional();
+               }
                res
        }
 }
index b7200e102f93fabc90aa31c969bb83d20c04effe..d4ede8eca992054892513cad99a772693f48ac66 100644 (file)
@@ -2115,7 +2115,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p
                                                        },
                                                        PaymentPurpose::SpontaneousPayment(payment_preimage) => {
                                                                assert_eq!(expected_preimage.unwrap(), *payment_preimage);
-                                                               assert!(our_payment_secret.is_none());
+                                                               assert_eq!(our_payment_secret, onion_fields.as_ref().unwrap().payment_secret);
                                                        },
                                                }
                                                assert_eq!(*amount_msat, recv_value);
index 6df1bc1a26a6bacb003ad13b24df3adb138227f1..0775f58bbcadb309508c3291846c36941d742eea 100644 (file)
@@ -9442,7 +9442,7 @@ fn test_keysend_payments_to_public_node() {
        let payer_pubkey = nodes[0].node.get_our_node_id();
        let payee_pubkey = nodes[1].node.get_our_node_id();
        let route_params = RouteParameters {
-               payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
+               payment_params: PaymentParameters::for_keysend(payee_pubkey, 40, false),
                final_value_msat: 10000,
        };
        let scorer = test_utils::TestScorer::new();
@@ -9473,7 +9473,7 @@ fn test_keysend_payments_to_private_node() {
 
        let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
        let route_params = RouteParameters {
-               payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
+               payment_params: PaymentParameters::for_keysend(payee_pubkey, 40, false),
                final_value_msat: 10000,
        };
        let network_graph = nodes[0].network_graph.clone();
index a13a618d83c4c2b0e8988f6103a1a26c2fcb2d63..2ac24baa58b3466315c2628c711f17e4f3d17d73 100644 (file)
@@ -414,9 +414,9 @@ pub struct RecipientOnionFields {
        /// If you do not have one, the [`Route`] you pay over must not contain multiple paths as
        /// multi-path payments require a recipient-provided secret.
        ///
-       /// Note that for spontaneous payments most lightning nodes do not currently support MPP
-       /// receives, thus you should generally never be providing a secret here for spontaneous
-       /// payments.
+       /// Some implementations may reject spontaneous payments with payment secrets, so you may only
+       /// want to provide a secret for a spontaneous payment if MPP is needed and you know your
+       /// recipient will not reject it.
        pub payment_secret: Option<PaymentSecret>,
        /// The payment metadata serves a similar purpose as [`Self::payment_secret`] but is of
        /// arbitrary length. This gives recipients substantially more flexibility to receive
@@ -447,10 +447,13 @@ impl RecipientOnionFields {
        }
 
        /// Creates a new [`RecipientOnionFields`] with no fields. This generally does not create
-       /// payable HTLCs except for spontaneous payments, i.e. this should generally only be used for
-       /// calls to [`ChannelManager::send_spontaneous_payment`].
+       /// payable HTLCs except for single-path spontaneous payments, i.e. this should generally
+       /// only be used for calls to [`ChannelManager::send_spontaneous_payment`]. If you are sending
+       /// a spontaneous MPP this will not work as all MPP require payment secrets; you may
+       /// instead want to use [`RecipientOnionFields::secret_only`].
        ///
        /// [`ChannelManager::send_spontaneous_payment`]: super::channelmanager::ChannelManager::send_spontaneous_payment
+       /// [`RecipientOnionFields::secret_only`]: RecipientOnionFields::secret_only
        pub fn spontaneous_empty() -> Self {
                Self { payment_secret: None, payment_metadata: None }
        }
index 7b28d2a79a81387629bfcecbcfbd45c3cbf0cdc3..f514fa1ed97693733363ff8bfc8540e380efdf55 100644 (file)
@@ -17,13 +17,13 @@ use crate::sign::EntropySource;
 use crate::chain::transaction::OutPoint;
 use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason};
 use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS;
-use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails, RecipientOnionFields};
+use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo};
 use crate::ln::features::InvoiceFeatures;
-use crate::ln::msgs;
+use crate::ln::{msgs, PaymentSecret, PaymentPreimage};
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::ln::outbound_payment::Retry;
 use crate::routing::gossip::{EffectiveCapacity, RoutingFees};
-use crate::routing::router::{get_route, Path, PaymentParameters, Route, Router, RouteHint, RouteHintHop, RouteHop, RouteParameters};
+use crate::routing::router::{get_route, Path, PaymentParameters, Route, Router, RouteHint, RouteHintHop, RouteHop, RouteParameters, find_route};
 use crate::routing::scoring::ChannelUsage;
 use crate::util::test_utils;
 use crate::util::errors::APIError;
@@ -236,6 +236,177 @@ fn mpp_receive_timeout() {
        do_mpp_receive_timeout(false);
 }
 
+#[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 nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+       create_announced_chan_between_nodes(&nodes, 0, 1);
+       create_announced_chan_between_nodes(&nodes, 0, 2);
+       create_announced_chan_between_nodes(&nodes, 1, 3);
+       create_announced_chan_between_nodes(&nodes, 2, 3);
+       let network_graph = nodes[0].network_graph.clone();
+
+       let payer_pubkey = nodes[0].node.get_our_node_id();
+       let payee_pubkey = nodes[3].node.get_our_node_id();
+       let recv_value = 15_000_000;
+       let route_params = RouteParameters {
+               payment_params: PaymentParameters::for_keysend(payee_pubkey, 40, true),
+               final_value_msat: recv_value,
+       };
+       let scorer = test_utils::TestScorer::new();
+       let random_seed_bytes = chanmon_cfgs[0].keys_manager.get_secure_random_bytes();
+       let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger,
+               &scorer, &(), &random_seed_bytes).unwrap();
+
+       let payment_preimage = PaymentPreimage([42; 32]);
+       let payment_secret = PaymentSecret(payment_preimage.0);
+       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
+               RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_preimage.0)).unwrap();
+       check_added_monitors!(nodes[0], 2);
+
+       let expected_route: &[&[&Node]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]];
+       let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events.len(), 2);
+
+       let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
+       pass_along_path(&nodes[0], expected_route[0], recv_value, payment_hash.clone(),
+               Some(payment_secret), ev.clone(), false, Some(payment_preimage));
+
+       let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
+       pass_along_path(&nodes[0], expected_route[1], recv_value, payment_hash.clone(),
+               Some(payment_secret), ev.clone(), true, Some(payment_preimage));
+       claim_payment_along_route(&nodes[0], expected_route, false, payment_preimage);
+}
+
+#[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
+       // 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 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;
+       let chan_3_id = create_announced_chan_between_nodes(&nodes, 1, 3).0.contents.short_channel_id;
+       let (update_a, _, chan_4_channel_id, _) = create_announced_chan_between_nodes(&nodes, 2, 3);
+       let chan_4_id = update_a.contents.short_channel_id;
+       let amount = 40_000;
+       let (mut route, payment_hash, payment_preimage, _) = get_route_and_payment_hash!(nodes[0], nodes[3], amount);
+
+       // Pay along nodes[1]
+       route.paths[0].hops[0].pubkey = nodes[1].node.get_our_node_id();
+       route.paths[0].hops[0].short_channel_id = chan_1_id;
+       route.paths[0].hops[1].short_channel_id = chan_3_id;
+
+       let payment_id_0 = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes());
+       nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), RecipientOnionFields::spontaneous_empty(), payment_id_0).unwrap();
+       check_added_monitors!(nodes[0], 1);
+
+       let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+       let update_add_0 = update_0.update_add_htlcs[0].clone();
+       nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add_0);
+       commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true);
+       expect_pending_htlcs_forwardable!(nodes[1]);
+
+       check_added_monitors!(&nodes[1], 1);
+       let update_1 = get_htlc_update_msgs!(nodes[1], nodes[3].node.get_our_node_id());
+       let update_add_1 = update_1.update_add_htlcs[0].clone();
+       nodes[3].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &update_add_1);
+       commitment_signed_dance!(nodes[3], nodes[1], update_1.commitment_signed, false, true);
+
+       assert!(nodes[3].node.get_and_clear_pending_msg_events().is_empty());
+       for (_, pending_forwards) in nodes[3].node.forward_htlcs.lock().unwrap().iter_mut() {
+               for f in pending_forwards.iter_mut() {
+                       match f {
+                               &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) => {
+                                       match forward_info.routing {
+                                               PendingHTLCRouting::ReceiveKeysend { ref mut payment_data, .. } => {
+                                                       *payment_data = Some(msgs::FinalOnionHopData {
+                                                               payment_secret: PaymentSecret([42; 32]),
+                                                               total_msat: amount * 2,
+                                                       });
+                                               },
+                                               _ => panic!("Expected PendingHTLCRouting::ReceiveKeysend"),
+                                       }
+                               },
+                               _ => {},
+                       }
+               }
+       }
+       expect_pending_htlcs_forwardable!(nodes[3]);
+
+       // Pay along nodes[2]
+       route.paths[0].hops[0].pubkey = nodes[2].node.get_our_node_id();
+       route.paths[0].hops[0].short_channel_id = chan_2_id;
+       route.paths[0].hops[1].short_channel_id = chan_4_id;
+
+       let payment_id_1 = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes());
+       nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), RecipientOnionFields::spontaneous_empty(), payment_id_1).unwrap();
+       check_added_monitors!(nodes[0], 1);
+
+       let update_2 = get_htlc_update_msgs!(nodes[0], nodes[2].node.get_our_node_id());
+       let update_add_2 = update_2.update_add_htlcs[0].clone();
+       nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add_2);
+       commitment_signed_dance!(nodes[2], nodes[0], &update_2.commitment_signed, false, true);
+       expect_pending_htlcs_forwardable!(nodes[2]);
+
+       check_added_monitors!(&nodes[2], 1);
+       let update_3 = get_htlc_update_msgs!(nodes[2], nodes[3].node.get_our_node_id());
+       let update_add_3 = update_3.update_add_htlcs[0].clone();
+       nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &update_add_3);
+       commitment_signed_dance!(nodes[3], nodes[2], update_3.commitment_signed, false, true);
+
+       assert!(nodes[3].node.get_and_clear_pending_msg_events().is_empty());
+       for (_, pending_forwards) in nodes[3].node.forward_htlcs.lock().unwrap().iter_mut() {
+               for f in pending_forwards.iter_mut() {
+                       match f {
+                               &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) => {
+                                       match forward_info.routing {
+                                               PendingHTLCRouting::ReceiveKeysend { ref mut payment_data, .. } => {
+                                                       *payment_data = Some(msgs::FinalOnionHopData {
+                                                               payment_secret: PaymentSecret([42; 32]),
+                                                               total_msat: amount * 2,
+                                                       });
+                                               },
+                                               _ => panic!("Expected PendingHTLCRouting::ReceiveKeysend"),
+                                       }
+                               },
+                               _ => {},
+                       }
+               }
+       }
+       expect_pending_htlcs_forwardable!(nodes[3]);
+       check_added_monitors!(nodes[3], 1);
+
+       // Fail back along nodes[2]
+       let update_fail_0 = get_htlc_update_msgs!(&nodes[3], &nodes[2].node.get_our_node_id());
+       nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &update_fail_0.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[2], nodes[3], update_fail_0.commitment_signed, false);
+       expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_4_channel_id }]);
+       check_added_monitors!(nodes[2], 1);
+
+       let update_fail_1 = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &update_fail_1.update_fail_htlcs[0]);
+       commitment_signed_dance!(nodes[0], nodes[2], update_fail_1.commitment_signed, false);
+
+       expect_payment_failed_conditions(&nodes[0], payment_hash, true, PaymentFailedConditions::new());
+       expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], vec![HTLCDestination::FailedPayment { payment_hash }]);
+}
+
+
 #[test]
 fn no_pending_leak_on_initial_send_failure() {
        // In an earlier version of our payment tracking, we'd have a retry entry even when the initial
index 237cd28347fa6b44a1f53e85f52b12aa513ae11a..f7ed173b901a90a9dbe0d81166dfb188b77ccec9 100644 (file)
@@ -621,8 +621,17 @@ impl PaymentParameters {
        ///
        /// The `final_cltv_expiry_delta` should match the expected final CLTV delta the recipient has
        /// provided.
-       pub fn for_keysend(payee_pubkey: PublicKey, final_cltv_expiry_delta: u32) -> Self {
-               Self::from_node_id(payee_pubkey, final_cltv_expiry_delta).with_bolt11_features(InvoiceFeatures::for_keysend()).expect("PaymentParameters::from_node_id should always initialize the payee as unblinded")
+       ///
+       /// Note that MPP keysend is not widely supported yet. The `allow_mpp` lets you choose
+       /// whether your router will be allowed to find a multi-part route for this payment. If you
+       /// set `allow_mpp` to true, you should ensure a payment secret is set on send, likely via
+       /// [`RecipientOnionFields::secret_only`].
+       ///
+       /// [`RecipientOnionFields::secret_only`]: crate::ln::channelmanager::RecipientOnionFields::secret_only
+       pub fn for_keysend(payee_pubkey: PublicKey, final_cltv_expiry_delta: u32, allow_mpp: bool) -> Self {
+               Self::from_node_id(payee_pubkey, final_cltv_expiry_delta)
+                       .with_bolt11_features(InvoiceFeatures::for_keysend(allow_mpp))
+                       .expect("PaymentParameters::from_node_id should always initialize the payee as unblinded")
        }
 
        /// Includes the payee's features. Errors if the parameters were initialized with blinded payment
index 758306c002d1a031199b0ca38b43a5decddca4ac..8f1f77b32aadd2f29b76be8fb6efc6290f2cb55a 100644 (file)
@@ -607,6 +607,17 @@ 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 < 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,
 }
 
 impl Default for UserConfig {
@@ -619,6 +630,7 @@ impl Default for UserConfig {
                        accept_inbound_channels: true,
                        manually_accept_inbound_channels: false,
                        accept_intercept_htlcs: false,
+                       accept_mpp_keysend: false,
                }
        }
 }