-
Notifications
You must be signed in to change notification settings - Fork 345
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
Prefer implementing From<PaymentPreimage>
over Into<PaymentHash>
#2918
Prefer implementing From<PaymentPreimage>
over Into<PaymentHash>
#2918
Conversation
.. as the std library docs state that implementing Into should be avoided: "One should avoid implementing Into and implement From instead. Implementing From automatically provides one with an implementation of Into thanks to the blanket implementation in the standard library."
WalkthroughThe update in the codebase shifts the method of converting a Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- lightning/src/ln/mod.rs (1 hunks)
Additional comments: 1
lightning/src/ln/mod.rs (1)
- 115-117: The implementation of
From<PaymentPreimage> for PaymentHash
correctly follows Rust's idiomatic practices for type conversion, aligning with the guidance from the Rust standard library documentation. By using theFrom
trait, the code benefits from an automaticInto
implementation, simplifying the codebase and enhancing maintainability. The conversion logic, which hashes thePaymentPreimage
using SHA256 to produce aPaymentHash
, is correctly implemented and efficiently utilizes theSha256::hash
function followed byto_byte_array()
to convert the hash result into the expected format. This change effectively improves the code's clarity and alignment with recommended Rust practices.
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2918 +/- ##
==========================================
- Coverage 89.15% 89.13% -0.02%
==========================================
Files 117 117
Lines 94856 94856
Branches 94856 94856
==========================================
- Hits 84568 84554 -14
- Misses 7812 7827 +15
+ Partials 2476 2475 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. There's not a lot of reason to make someone else look at this one, either. Just gonna land.
In #2916 an
Into<PaymentHash>
implementation was added.Here, we change that to
From<PaymentPreimage>
as thestd
library docs state that implementing Into should be avoided: