Async receive prefactor #3517
Open
Async receive prefactor #3517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this patch, we would take() the invoice request stored for AwaitingInvoice outbound payments when retrying sending the invoice request onion message. This doesn't work for async payments because we need to keep the invoice request stored for inclusion in the payment onion. Therefore, clone it instead of take()ing it.
Prior to this fix, we would attempt to mark outbound async payments as abandoned but silently fail because they were in state AwaitingInvoice, which the mark_abandoned utility doesn't currently work for. These payments would eventually be removed by the remove_stale_payments method, but there would be a delay in generating the PaymentFailed event. Move to manually removing the outbound payment entry.
Useful for creating payment paths for static invoices which are typically amount-less.
Will be useful for static invoices' blinded paths, which may have long expiries. Rather than having a default max_cltv_expiry, we now base it on the invoice expiry.
This context is stored in the blinded payment paths we put in static invoices and is useful to authenticate payments over these paths to the recipient. We can't reuse Bolt12OfferContext for this because we don't have access to the invoice request fields at static invoice creation time.
This context is included in static invoice's blinded message paths, provided back to us in HeldHtlcAvailable onion messages for blinded path authentication. In future work, we will check if this context is valid and respond with a ReleaseHeldHtlc message to release the upstream payment if so. We also add creation methods for the hmac used for authenticating said blinded path.
We can't use our regular offer creation util for receiving async payments because the recipient can't be relied on to be online to service invoice_requests. Therefore, add a new offer creation util that is parameterized by blinded message paths to another node on the network that *is* always-online and can serve static invoices on behalf of the often-offline recipient. Also add a utility for creating static invoices corresponding to these offers. See new utils' docs and BOLTs PR 1149 for more info.
Since adding support for creating static invoices from ChannelManager, it's easier to test these failure cases that went untested when we added support for paying static invoices.
Blinded keysend is only planned to be supported in the async payments context.
Blinded HTLCs are always failed back with the same error, so DRY the test code that fails them backwards. This util will also be used for async payments testing in upcoming commits.
Needed to authenticate that the held_htlc_available message is being sent over a reply path that we originally created and that isn't expired before we reply with release_held_htlc. This context will be used in upcoming commits when we add support for async receive.
Prior to this patch, if we received an expired static invoice we would delay surfacing the payment failure until after the recipient had come online and sent the release_held_htlc OM, which could be a long time later. Now, we'll detect that the invoice is expired as soon as it's received.
Here we bubble up the payment context into PendingHTLCRouting::ReceiveKeysend and check it when receiving a spontaneous payment prior to generating a claimable event. Prior to this patch, we would have accepted out-of-context keysends sent over blinded paths taken from our BOLT 12 invoices. As a side effect of this, our blinded keysend success test cases now fail, so those tests are now removed. Their coverage is re-added in future commits when we add support for async receive, meaning we're able to receive blinded keysends in the correct payment context.
valentinewallace
force-pushed
the
2025-01-async-receive-prefactor
branch
from
e984b03 to
50fb8fe
Compare
valentinewallace
commented
Jan 9, 2025
| @@ -1045,6 +1045,11 @@ impl OutboundPayments { | |||
| abandon_with_entry!(entry, PaymentFailureReason::UnknownRequiredFeatures); | |||
| return Err(Bolt12PaymentError::UnknownRequiredFeatures) | |||
| } | |||
| if duration_since_epoch > invoice.created_at().saturating_add(invoice.relative_expiry()) { | |||
There was a problem hiding this comment.
Note for reviewers: for inbound payments, we buffer expiries by 2 hours to account for inaccurate block times in no-std. So we could do that here too, but it seemed maybe unnecessary since static invoices are expected to have quite long expiries... Let me know if we'd prefer to do that. (Our other outbound expiry checks are std-only, but I wanted no-std here for ease of testing).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some test util refactors, additional test coverage, and one fix in preparation for finalizing support for async receive.
Based on #3408. Prepares for #3440