Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include invoice requests in async payment onions #3207

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

valentinewallace
Copy link
Contributor

@valentinewallace valentinewallace commented Jul 26, 2024

Title. Implements the sending-side of the final commit of lightning/bolts#1149.

Based on #3140.

Copy link

codecov bot commented Jul 26, 2024

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 ⚠️
lightning/src/ln/channelmanager.rs 60.00% 2 Missing ⚠️
lightning/src/ln/peer_handler.rs 0.00% 2 Missing ⚠️
lightning/src/onion_message/functional_tests.rs 0.00% 2 Missing ⚠️
lightning/src/routing/router.rs 0.00% 2 Missing ⚠️
lightning/src/events/mod.rs 0.00% 1 Missing ⚠️
lightning/src/ln/msgs.rs 83.33% 0 Missing and 1 partial ⚠️
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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TheBlueMatt
Copy link
Collaborator

Needs rebase.

We already log the error in ChannelManager::handle_message.
RouteNotFound did not fit here because that error is reserved for failing to
find a route for a payment, whereas here we are failing to create a blinded
path back to ourselves..
Since we started using this error in send_payment_for_bolt12_invoice, this
error type is no longer only used on retry but also on initial send.
"Release" is overloaded in the trait's release_pending_messages method, since
the latter releases pending async payments onion messages to the peer manager,
vs the release_held_htlc method handles the release_held_htlc onion message by
attempting to send an HTLC to the recipient.
Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.

We use an experimental TLV type for this new onion payload field, since the
spec is still not merged in the BOLTs.
Add a new invoice request parameter to onion_utils::build_onion_payloads.
As of this commit it will always be passed in as None, to be updated in future
commits.

Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.
Add a new invoice request parameter to onion_utils::create_payment_onion. As of
this commit it will always be passed in as None, to be updated in future
commits.

Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.
Add a new invoice request parameter to outbound_payments and channelmanager
send-to-route internal utils. As of this commit the invreq will always be
passed in as None, to be updated in future commits.

Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.
When transitioning outbound payments from AwaitingInvoice to
StaticInvoiceReceived, include the invreq in the new state's outbound payment
storage for future inclusion in an async payment onion.

Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.
Past commits have set us up to include invoice requests in outbound async
payment onions. Here we actually pull the invoice request from where it's
stored in outbound_payments and pass it into the correct utility for inclusion
in the onion on initial send.

Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.
While in the last commit we began including invoice requests in async payment
onions on initial send, further work is needed to include them on retry. Here
we begin storing invreqs in our retry data, and pass them along for inclusion
in the onion on payment retry.

Per <lightning/bolts#1149>, when paying a static
invoice we need to include our original invoice request in the HTLC onion since
the recipient wouldn't have received it previously.
Async payments include the original invoice request in the payment onion.
Since invreqs may include blinded paths, it's important to factor them into our
max path length calculations since they may take up a significant portion of
the 1300-byte onion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants