Skip to content

Add PaymentMetadataStore for Bolt11 and Bolt12 inbound payments #537

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

Closed
wants to merge 6 commits into from

Conversation

moisesPompilio
Copy link
Contributor

This PR introduces PaymentMetadata to handle inbound Bolt11 and Bolt12 payments more consistently.

Summary of changes:

  • Created PaymentMetadata and PaymentMetadataStore to persist metadata for incoming payments.
  • Introduced PaymentDataWithFallback, a structure that contains both PaymentMetadata and PaymentDetail as Options, along with shared fields like payment status and direction. This helps ensure backward compatibility.
  • Updated Bolt11 and Bolt12 logic so that incoming payments are stored in PaymentMetadataStore instead of PaymentStore.
  • Adjusted related events to follow the new rule: payments are only added to PaymentStore once they are completed (either successfully or failed)when payments are pending they are in the PaymentMetadataStore. Also ensures compatibility with legacy behavior, where payments were stored exclusively in PaymentStore.
  • Added the legacy_payment_store Cargo feature to test legacy behavior and validate backward compatibility.
  • Enabled tests using the legacy_payment_store feature in the CI workflow to ensure everything works as expected.

Ref: #425

…allback

- Add PaymentMetadata to represent metadata for inbound payments
- Add PaymentMetadataStore for persisting PaymentMetadata
- Add PaymentDataWithFallback to handle backward compatibility during the transition from legacy bolt11/bolt12 pending payments to the new PaymentMetadata format

Issue lightningdevkit#425
Introduces the optional `legacy_payment_store` feature to enable testing of legacy payment flows during the transition to `PaymentMetadata` for inbound payments.

Issue lightningdevkit#425
…g paymentStore

Inbound Bolt11 and Bolt12 payments that are pending should now be stored in `PaymentMetadataStore` rather than directly in `PaymentStore`.
Once the payment is completed (successful or failed), it is then moved to `PaymentStore`, and its status is updated in `PaymentMetadata`.
This change maintains backward compatibility with the previous behavior of storing all payment information directly in `PaymentStore`.

issue lightningdevkit#425
…cy behavior

The test was modified to be compatible with the new `PaymentMetadata` logic, while still verifying compatibility with the legacy rule that stores payments directly in `PaymentStore`.

Issue lightningdevkit#425
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented May 6, 2025

I've assigned @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@moisesPompilio moisesPompilio marked this pull request as draft May 6, 2025 15:58
@moisesPompilio moisesPompilio marked this pull request as ready for review May 7, 2025 14:20
@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz May 7, 2025 14:20
@tnull
Copy link
Collaborator

tnull commented May 8, 2025

  • Updated Bolt11 and Bolt12 logic so that incoming payments are stored in PaymentMetadataStore instead of PaymentStore.

  • Added the legacy_payment_store Cargo feature to test legacy behavior and validate backward compatibility.

Excuse the delay here. As noted over at #425 (comment), I think there are some misconceptions about what the payment metadata store is meant to be here. Unfortunately I think this PR is headed in the wrong direction as is. I'm going to close this PR for now, but feel free to reopen once we synced on the approach and are aligned on what approach should be taken.

@tnull tnull closed this May 8, 2025
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.

3 participants