Add a `RecipientOnionFields` argument to spontaneous payment sends
authorMatt Corallo <git@bluematt.me>
Fri, 24 Mar 2023 01:19:20 +0000 (01:19 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 5 Apr 2023 16:28:14 +0000 (16:28 +0000)
While most lightning nodes don't (currently) support providing a
payment secret or payment metadata for spontaneous payments,
there's no specific technical reason why we shouldn't support
sending those fields to a recipient.

Further, when we eventually move to allowing custom TLV entries in
the recipient's onion TLV stream, we'll want to support it for
spontaneous payments as well.

Here we simply add the new `RecipientOnionFields` struct as an
argument to the spontaneous payment send methods. We don't yet
plumb it through the payment sending logic, which will come when we
plumb the new struct through the sending logic to replace the
existing payment secret arguments.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs

index 5f5703f7d900aeb617761e2c79d9fd37f20c2c2d..3ae634a2d55c86b20d3f922eef1ddae10c62c1bb 100644 (file)
@@ -2717,7 +2717,7 @@ where
        /// 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>, payment_id: PaymentId) -> Result<PaymentHash, PaymentSendFailure> {
+       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();
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
                self.pending_outbound_payments.send_spontaneous_payment_with_route(
@@ -2734,7 +2734,7 @@ where
        /// payments.
        ///
        /// [`PaymentParameters::for_keysend`]: crate::routing::router::PaymentParameters::for_keysend
-       pub fn send_spontaneous_payment_with_retry(&self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<PaymentHash, RetryableSendFailure> {
+       pub fn send_spontaneous_payment_with_retry(&self, payment_preimage: Option<PaymentPreimage>, recipient_onion: RecipientOnionFields, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<PaymentHash, RetryableSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
                self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
@@ -8032,7 +8032,8 @@ mod tests {
                pass_along_path(&nodes[0], &[&nodes[1]], 200_000, our_payment_hash, Some(payment_secret), events.drain(..).next().unwrap(), false, None);
 
                // Next, send a keysend payment with the same payment_hash and make sure it fails.
-               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), PaymentId(payment_preimage.0)).unwrap();
+               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
+                       RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -8152,7 +8153,8 @@ mod tests {
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
                        None, nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
-               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), PaymentId(payment_preimage.0)).unwrap();
+               nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
+                       RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@ -8185,7 +8187,8 @@ mod tests {
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
                        None, nodes[0].logger, &scorer, &random_seed_bytes
                ).unwrap();
-               let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), PaymentId(payment_preimage.0)).unwrap();
+               let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
+                       RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
index f632196a3b249099c70c32eb1b58acaae5214be5..fd9bd7a768de96ecee75958179e80ea52370e19d 100644 (file)
@@ -9492,7 +9492,8 @@ fn test_keysend_payments_to_public_node() {
        let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);
-       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage), PaymentId(test_preimage.0)).unwrap();
+       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage),
+               RecipientOnionFields::spontaneous_empty(), PaymentId(test_preimage.0)).unwrap();
        check_added_monitors!(nodes[0], 1);
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 1);
@@ -9527,7 +9528,8 @@ fn test_keysend_payments_to_private_node() {
        ).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);
-       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage), PaymentId(test_preimage.0)).unwrap();
+       let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage),
+               RecipientOnionFields::spontaneous_empty(), PaymentId(test_preimage.0)).unwrap();
        check_added_monitors!(nodes[0], 1);
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 1);
index c1e3f27c1e88a0d6622d55e3c6632ae156b3b19a..cc511f7b4ba18d17d7440a33056507986d33ff9c 100644 (file)
@@ -417,6 +417,10 @@ 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.
        pub payment_secret: Option<PaymentSecret>,
 }
 
index 6748f49b5bde99113dc8fda06f1cd5290af49801..412e7f774f92c24e9ae3ccb85c8c356f91f8e1e4 100644 (file)
@@ -1087,7 +1087,8 @@ fn claimed_send_payment_idempotent() {
 
                        // Further, if we try to send a spontaneous payment with the same payment_id it should
                        // also be rejected.
-                       let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
+                       let send_result = nodes[0].node.send_spontaneous_payment(
+                               &route, None, RecipientOnionFields::spontaneous_empty(), payment_id);
                        match send_result {
                                Err(PaymentSendFailure::DuplicatePayment) => {},
                                _ => panic!("Unexpected send result: {:?}", send_result),
@@ -1161,7 +1162,8 @@ fn abandoned_send_payment_idempotent() {
 
                        // Further, if we try to send a spontaneous payment with the same payment_id it should
                        // also be rejected.
-                       let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
+                       let send_result = nodes[0].node.send_spontaneous_payment(
+                               &route, None, RecipientOnionFields::spontaneous_empty(), payment_id);
                        match send_result {
                                Err(PaymentSendFailure::DuplicatePayment) => {},
                                _ => panic!("Unexpected send result: {:?}", send_result),
@@ -1671,7 +1673,9 @@ fn do_automatic_retries(test: AutoRetry) {
                pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], amt_msat, payment_hash, Some(payment_secret), msg_events.pop().unwrap(), true, None);
                claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage);
        } else if test == AutoRetry::Spontaneous {
-               nodes[0].node.send_spontaneous_payment_with_retry(Some(payment_preimage), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
+               nodes[0].node.send_spontaneous_payment_with_retry(Some(payment_preimage),
+                       RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params,
+                       Retry::Attempts(1)).unwrap();
                pass_failed_attempt_with_retry_along_path!(channel_id_2, true);
 
                // Open a new channel with liquidity on the second hop so we can find a route for the retry