Skip to content

Conversation

@psychemist
Copy link

@psychemist psychemist commented Dec 26, 2025

This change updatesBolt11Invoice's payment_hash function's return type from a stream of bytes (sha256 digest) to PaymentHash, which is now a valid type in lightning_types. Also, tests and other functions that called the function were refactored to reflect that modification.

Closes #4292

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Dec 26, 2025

I've assigned @TheBlueMatt 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.

@codecov
Copy link

codecov bot commented Dec 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.55%. Comparing base (f6b4f0c) to head (9c802c2).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4293      +/-   ##
==========================================
- Coverage   89.36%   86.55%   -2.82%     
==========================================
  Files         180      158      -22     
  Lines      139847   101821   -38026     
  Branches   139847   101821   -38026     
==========================================
- Hits       124975    88128   -36847     
+ Misses      12278    11273    -1005     
+ Partials     2594     2420     -174     
Flag Coverage Δ
fuzzing 35.21% <0.00%> (-0.01%) ⬇️
tests 85.84% <100.00%> (-2.86%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

I did a pass of this PR, and the git history can be squashed into a single message that includes also a small line for the potential breaking change that this PR is introducing.

In addition, if changing to by-value return is what we want, do you think that we should consider updating payment_secret() as well for consistency (perhaps in a follow-up PR, or expand this one's scope).

@psychemist
Copy link
Author

psychemist commented Dec 27, 2025

@vincenzopalazzo updating payment_secret() is ideal for consistency. i can adjust this PR if you create a new issue or update the scope of the existing one

@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@vincenzopalazzo
Copy link
Contributor

updating payment_secret() is ideal for consistency. i can adjust this PR if you reate a new issue or update the scope of the existing one

Probably better wait the input from @TheBlueMatt before considering the payment_secret

@psychemist
Copy link
Author

Probably better wait the input from @TheBlueMatt before considering the payment_secret

alright. that tracks. i'll make the necessary changes you pointed out

@ldk-reviews-bot
Copy link

🔔 2nd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link

🔔 3rd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@psychemist psychemist force-pushed the change-bolt11invoice-payment_hash-return-type branch from 51fc781 to f3c4954 Compare January 4, 2026 18:56
This commit fixes the payment_hash function of Bolt11Invoice to return a
PaymentHash type instead of a sha256 byte stream. Code and test files dependent
on this function have also been modified to adhere to the updated
changes.
@psychemist psychemist force-pushed the change-bolt11invoice-payment_hash-return-type branch from f3c4954 to 9c802c2 Compare January 4, 2026 19:04
@ldk-reviews-bot
Copy link

🔔 4th Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

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.

Bolt11Invoice::payment_hash should return a PaymentHash

3 participants