Move cryptographic algorithms and utilities to a new crypto mod
#2828
Conversation
WalkthroughThe Rust codebase for a lightning network project has undergone a significant reorganization, centralizing cryptographic functionality into a new 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 (
|
TheBlueMatt
force-pushed
the
2024-01-crypto-module
branch
from
0030f2b
to
42988b2
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2828 +/- ##
==========================================
- Coverage 88.53% 88.52% -0.02%
==========================================
Files 114 115 +1
Lines 92090 92090
Branches 92090 92090
==========================================
- Hits 81535 81520 -15
- Misses 8059 8072 +13
- Partials 2496 2498 +2 ☔ View full report in Codecov by Sentry. |
TheBlueMatt
force-pushed
the
2024-01-crypto-module
branch
from
42988b2
to
6777783
Compare
|
I wonder whether we should rather move this to a dedicated |
|
We've historically avoided that because our ChaCha implementation doesn't actually fully comply with the RFC (missing handling for high bits set in the nonce and doesn't support sending > 4B blocks of data). If we want to arbitrary expose it I'd want to fix those issues first. |
| #[cfg(test)] | ||
| mod test { | ||
| use crate::prelude::*; | ||
| use alloc::vec; | ||
| use alloc::vec::{Vec}; |
There was a problem hiding this comment.
nit: unnecessary braces. Why can't we use prelude?
There was a problem hiding this comment.
#2829 includes these files by symlink, so can't refer to the in-crate stuff.
As we'd generally like the `lightning` crate to, over time, have more modules rather than being very monolithic, we should move the cryptographic things into their own module, which we do here. We also take this opportunity to move stream adapters into their own module and make clear that the ChaChaPoly `decrypt` method is variable time.
TheBlueMatt
force-pushed
the
2024-01-crypto-module
branch
from
6777783
to
4a0abd5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (20)
- lightning/src/blinded_path/message.rs (1 hunks)
- lightning/src/blinded_path/utils.rs (1 hunks)
- lightning/src/chain/channelmonitor.rs (1 hunks)
- lightning/src/crypto/chacha20.rs (2 hunks)
- lightning/src/crypto/chacha20poly1305rfc.rs (1 hunks)
- lightning/src/crypto/mod.rs (1 hunks)
- lightning/src/crypto/poly1305.rs (1 hunks)
- lightning/src/crypto/streams.rs (1 hunks)
- lightning/src/lib.rs (1 hunks)
- lightning/src/ln/chan_utils.rs (1 hunks)
- lightning/src/ln/inbound_payment.rs (1 hunks)
- lightning/src/ln/monitor_tests.rs (1 hunks)
- lightning/src/ln/msgs.rs (1 hunks)
- lightning/src/ln/onion_utils.rs (1 hunks)
- lightning/src/ln/peer_channel_encryptor.rs (2 hunks)
- lightning/src/onion_message/packet.rs (1 hunks)
- lightning/src/routing/router.rs (2 hunks)
- lightning/src/sign/mod.rs (2 hunks)
- lightning/src/util/mod.rs (2 hunks)
- lightning/src/util/scid_utils.rs (1 hunks)
Files skipped from review as they are similar to previous changes (20)
- lightning/src/blinded_path/message.rs
- lightning/src/blinded_path/utils.rs
- lightning/src/chain/channelmonitor.rs
- lightning/src/crypto/chacha20.rs
- lightning/src/crypto/chacha20poly1305rfc.rs
- lightning/src/crypto/mod.rs
- lightning/src/crypto/poly1305.rs
- lightning/src/crypto/streams.rs
- lightning/src/lib.rs
- lightning/src/ln/chan_utils.rs
- lightning/src/ln/inbound_payment.rs
- lightning/src/ln/monitor_tests.rs
- lightning/src/ln/msgs.rs
- lightning/src/ln/onion_utils.rs
- lightning/src/ln/peer_channel_encryptor.rs
- lightning/src/onion_message/packet.rs
- lightning/src/routing/router.rs
- lightning/src/sign/mod.rs
- lightning/src/util/mod.rs
- lightning/src/util/scid_utils.rs
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: extra newline
| assert!(self.finished == false); | ||
| } | ||
|
|
||
| pub(in super::super) fn finish_and_check_tag(&mut self, tag: &[u8]) -> bool { |
There was a problem hiding this comment.
TIL in super::super.
|
Just code moves and a rename so I'm going to land it. Nice cleanup! |
valentinewallace
merged commit a175958
into
lightningdevkit:main
As we'd generally like the
lightningcrate to, over time, have more modules rather than being very monolithic, we should move the cryptographic things into their own module, which we do here.We also take this opportunity to move stream adapters into their own module and make clear that the ChaChaPoly
decryptmethod is variable time.