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

Fix: The current preimage of a invoice's lightning payment method should be available via API #5111

Merged
merged 1 commit into from Jun 23, 2023

Conversation

NicolasDorier
Copy link
Member

@NicolasDorier NicolasDorier commented Jun 22, 2023

We are serializing payment method details in two places: Payments blob and inside the Invoices blob.

Sadly, we aren't using the NBitcoin's serializer for the payment method detail in the Invoices blob. resulting in the unuseful

{
  "PaymentHash": { "Size": 32 }
}

Being saved in the database. Notice the casing is wrong.

We will really need to refactor everything concerning payment methods and payment methods details very soon.

@NicolasDorier NicolasDorier requested a review from Kukks June 22, 2023 14:27
Copy link
Member

@Kukks Kukks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn so we have not had any of this data properly persisted?

@NicolasDorier
Copy link
Member Author

NicolasDorier commented Jun 23, 2023

@Kukks we actually had the data, but at the payment level (we receive preimage when we receive the payment), not invoice level.
I am working on another PR that may be better. That would use proper serialized, and fix the JSON in string at invoice level.

@NicolasDorier
Copy link
Member Author

Giving up the other PR, need complete refactor of the PaymentType stuff, it's too big. Will probably just push this PR.

@NicolasDorier NicolasDorier merged commit 53aafcf into btcpayserver:master Jun 23, 2023
4 checks passed
@NicolasDorier NicolasDorier deleted the oqthqb branch June 23, 2023 10:12
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.

None yet

3 participants