Change Bolt11Invoice payment_hash function return type #4293
Conversation
|
I've assigned @TheBlueMatt as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4293 +/- ##
==========================================
- Coverage 89.38% 89.36% -0.02%
==========================================
Files 180 180
Lines 139834 139834
Branches 139834 139834
==========================================
- Hits 124985 124958 -27
- Misses 12262 12284 +22
- Partials 2587 2592 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vincenzopalazzo
left a comment
vincenzopalazzo
left a comment
There was a problem hiding this comment.
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).
| cltv_expiry_delta, | ||
| payment_size_msat, | ||
| "asdf", | ||
| "e2e", |
There was a problem hiding this comment.
This is unrelated to the PR's stated purpose. Why we need this change?
| *intercept_id, | ||
| *expected_outbound_amount_msat, | ||
| PaymentHash(invoice.payment_hash().to_byte_array()), | ||
| PaymentHash(invoice.payment_hash().0), |
There was a problem hiding this comment.
Why are we not just passing the invoice.payment_hash()?
| *intercept_id, | ||
| *expected_outbound_amount_msat, | ||
| PaymentHash(invoice.payment_hash().to_byte_array()), | ||
| PaymentHash(invoice.payment_hash().0), |
There was a problem hiding this comment.
same here and in other several places
3 participants
This change updates
Bolt11Invoice'spayment_hashfunction's return type from a stream of bytes (sha256 digest) toPaymentHash, which is now a valid type inlightning_types. Also, tests and other functions that called the function were refactored to reflect that modification.Closes #4292