rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Include invoice requests in async payment onions

Open valentinewallace opened this issue 1 year ago • 2 comments

Title. Implements the sending-side of the final commit of https://github.com/lightning/bolts/pull/1149.

  • [x] Address followup https://github.com/lightningdevkit/rust-lightning/pull/3140#discussion_r1747623805
  • [x] Address followup https://github.com/lightningdevkit/rust-lightning/pull/3140#discussion_r1757611006

Based on #3140.

valentinewallace avatar Jul 26 '24 21:07 valentinewallace

Codecov Report

Attention: Patch coverage is 87.50000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 89.62%. Comparing base (cdd1298) to head (b5ccb98).

Files with missing lines Patch % Lines
lightning/src/ln/outbound_payment.rs 94.54% 3 Missing :warning:
lightning/src/ln/channelmanager.rs 60.00% 2 Missing :warning:
lightning/src/ln/peer_handler.rs 0.00% 2 Missing :warning:
lightning/src/onion_message/functional_tests.rs 0.00% 2 Missing :warning:
lightning/src/routing/router.rs 0.00% 2 Missing :warning:
lightning/src/events/mod.rs 0.00% 1 Missing :warning:
lightning/src/ln/msgs.rs 83.33% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3207      +/-   ##
==========================================
- Coverage   89.64%   89.62%   -0.02%     
==========================================
  Files         126      126              
  Lines      102676   102705      +29     
  Branches   102676   102705      +29     
==========================================
+ Hits        92043    92053      +10     
- Misses       7909     7926      +17     
- Partials     2724     2726       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 26 '24 21:07 codecov[bot]

Needs rebase.

TheBlueMatt avatar Sep 17 '24 19:09 TheBlueMatt

Rebased due to conflicts, then pushed an additional typo fix:

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index bc7bbb641..823c5e2c2 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -566,7 +566,7 @@ pub enum PaymentFailureReason {
        /// routes - we tried the payment over a few routes but were not able to find any further
        /// candidate routes beyond those.
        ///
-       /// Also used for [`BlindedPathCreationFailed`] down downgrading to versions prior to 0.0.124.
+       /// Also used for [`BlindedPathCreationFailed`] when downgrading to versions prior to 0.0.124.
        RouteNotFound,
        /// This error should generally never happen. This likely means that there is a problem with
        /// your router.

valentinewallace avatar Oct 24 '24 17:10 valentinewallace

I went ahead and squashed, the diff prior was:

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index c68209e8d..c3fde629a 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -954,7 +954,7 @@ impl OutboundPayments {
 		let onion_session_privs = match outbounds.entry(payment_id) {
 			hash_map::Entry::Occupied(entry) => match entry.get() {
 				PendingOutboundPayment::InvoiceReceived { .. } => {
-					let (retryable_payment, onion_session_privs) = self.create_pending_payment(
+					let (retryable_payment, onion_session_privs) = Self::create_pending_payment(
 						payment_hash, recipient_onion.clone(), keysend_preimage, None, &route,
 						Some(retry_strategy), payment_params, entropy_source, best_block_height
 					);
@@ -965,7 +965,7 @@ impl OutboundPayments {
 					let invreq = if let PendingOutboundPayment::StaticInvoiceReceived { invoice_request, .. } = entry.remove() {
 						invoice_request
 					} else { unreachable!() };
-					let (retryable_payment, onion_session_privs) = self.create_pending_payment(
+					let (retryable_payment, onion_session_privs) = Self::create_pending_payment(
 						payment_hash, recipient_onion.clone(), keysend_preimage, Some(invreq), &route,
 						Some(retry_strategy), payment_params, entropy_source, best_block_height
 					);
@@ -1572,7 +1572,7 @@ impl OutboundPayments {
 		match pending_outbounds.entry(payment_id) {
 			hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
 			hash_map::Entry::Vacant(entry) => {
-				let (payment, onion_session_privs) = self.create_pending_payment(
+				let (payment, onion_session_privs) = Self::create_pending_payment(
 					payment_hash, recipient_onion, keysend_preimage, None, route, retry_strategy,
 					payment_params, entropy_source, best_block_height
 				);
@@ -1583,7 +1583,7 @@ impl OutboundPayments {
 	}
 
 	fn create_pending_payment<ES: Deref>(
-		&self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
+		payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
 		keysend_preimage: Option<PaymentPreimage>, invoice_request: Option<InvoiceRequest>,
 		route: &Route, retry_strategy: Option<Retry>, payment_params: Option<PaymentParameters>,
 		entropy_source: &ES, best_block_height: u32
@@ -2276,8 +2276,8 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(2, retry_strategy, required),
 		(4, max_total_routing_fee_msat, option),
 	},
-	// Added in 0.0.125. Prior versions will drop these outbounds on downgrade, which is safe because
-	// no HTLCs are in-flight.
+	// Added in 0.1. Prior versions will drop these outbounds on downgrade, which is safe because no
+	// HTLCs are in-flight.
 	(9, StaticInvoiceReceived) => {
 		(0, payment_hash, required),
 		(2, keysend_preimage, required),

I also removed the links to https://github.com/lightning/bolts/pull/1149 in the commit messages because I realized they were majorly cluttering up that PR.

valentinewallace avatar Oct 30 '24 20:10 valentinewallace