Fix followups to PR2797 #2864
Fix followups to PR2797 #2864
Conversation
WalkthroughThe recent updates focus on streamlining the handling of monitor updates and channel IDs within the Lightning Network's codebase. By eliminating the need to explicitly pass channel IDs in various contexts and refining the usage of channel identifiers, these changes aim to simplify the code and potentially reduce errors. Additionally, adjustments in logging and event handling reflect a broader effort to enhance clarity and efficiency in the network's operation. 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 (
|
![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 (3)
- lightning/src/ln/channelmanager.rs (13 hunks)
- lightning/src/ln/peer_handler.rs (2 hunks)
- lightning/src/util/macro_logger.rs (2 hunks)
Files skipped from review due to trivial changes (1)
- lightning/src/ln/peer_handler.rs
Additional comments: 16
lightning/src/util/macro_logger.rs (3)
- 7-15: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The removal of
OutPointandTxidimports suggests these types are no longer used in this file. Ensure that this change does not affect any other parts of the code where these imports might be required.
- 7-15: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The removal of the
DebugFundingChannelIdstruct and its implementation aligns with the PR's objective to streamline channel identifier handling. Verify that no code segments are left referencing this struct, potentially leading to compile-time errors.Verification successful
The executed script did not find any references to
DebugFundingChannelIdwithin the project. This suggests that the removal of theDebugFundingChannelIdstruct and its implementation has been handled properly, with no leftover references that could lead to compile-time errors.* 7-15: > 📝 **NOTE** > This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to DebugFundingChannelId in the project. rg "DebugFundingChannelId"Length of output: 26
The remaining code, including macros and struct implementations for logging purposes, appears to be correctly implemented and adheres to best practices for clarity and maintainability. Ensure that the new logging structures and macros integrate well with the rest of the codebase, maintaining consistency and performance.
lightning/src/ln/channelmanager.rs (13)
- 1636-1641: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-6]
The addition of
channel_id: Option<ChannelId>to a structure is not shown in the provided code snippet. Ensure that thechannel_idfield is properly added and documented in the structure wherefunding_txois defined.
- 2296-2296: The macro
handle_new_monitor_update!has been simplified by removing the$channel_idparameter. This change streamlines the process of handling monitor updates by relying solely on the$funding_txo. Ensure that all usages of this macro throughout the codebase have been updated accordingly to match the new signature.- 2753-2753: The call to
handle_new_monitor_update!has been updated to remove thechannel_idparameter, aligning with the macro's new signature. This change is consistent across multiple instances, simplifying the identification of channels during monitor updates.- 3414-3414: The call to
handle_new_monitor_update!within a match statement has been updated to match the new macro signature by removing thechannel_idparameter. This change is part of the broader effort to streamline monitor update handling.- 4770-4770: The call to
handle_new_monitor_update!in the context of handling a funded channel has been updated to match the new macro signature. This change ensures consistency in how monitor updates are handled across different channel states.- 5574-5574: The call to
handle_new_monitor_update!during a non-initialization phase has been updated to match the new macro signature. This change is part of the effort to ensure that monitor updates are handled consistently, regardless of the channel's initialization state.- 6570-6570: The call to
handle_new_monitor_update!in the context of handling a shutdown script monitor update has been updated to match the new macro signature. This change is consistent with the overall simplification of monitor update handling.- 6852-6852: The call to
handle_new_monitor_update!following a commitment_signed message handling has been updated to match the new macro signature. This change ensures that monitor updates are handled consistently across different channel operations.- 7031-7031: The call to
handle_new_monitor_update!in the context of handling a monitor update after RAA (Revoke And Acknowledge) message processing has been updated to match the new macro signature. This change is part of the broader effort to streamline monitor update handling.- 7372-7372: The call to
handle_new_monitor_update!in the context of handling a monitor update during a peer loop has been updated to match the new macro signature. This change ensures consistency in how monitor updates are processed across different channel interactions.- 8141-8141: The call to
handle_new_monitor_update!for unlocking monitor updating for a channel has been updated to match the new macro signature. This change is consistent with the effort to simplify and unify the handling of monitor updates.- 9816-9816: The addition of
requiredto the tuple associated withprev_channel_idin the deserialization process indicates a change in howprev_channel_idis handled. Ensure that this change is properly documented and that it aligns with the intended handling of channel identifiers during deserialization.- 11081-11081: The logging and handling of actions blocking RAA monitor updates have been updated to use the
blocked_channel_iddirectly. This change is part of the broader effort to streamline and clarify the handling of monitor updates and their dependencies on channel states.
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2864 +/- ##
==========================================
+ Coverage 89.14% 89.17% +0.02%
==========================================
Files 116 116
Lines 93205 93201 -4
Branches 93205 93201 -4
==========================================
+ Hits 83089 83109 +20
+ Misses 7583 7566 -17
+ Partials 2533 2526 -7 ☔ View full report in Codecov by Sentry. |
Fixes requested for #2797.