rpc: add channel updates to SubscribeChannelEvents#10543
rpc: add channel updates to SubscribeChannelEvents#10543bitromortac wants to merge 8 commits intolightningnetwork:masterfrom
Conversation
We create a deep copy of the channel state as we want to later expose the data structure to the rpc, which already has helper methods to marshal this representation to the rpc representation.
Summary of ChangesHello @bitromortac, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔴 PR Severity: CRITICAL
🔴 Critical (5 files)
🟠 High (1 file)
🟢 Low (5 files)
AnalysisThis PR is classified as CRITICAL because it modifies multiple core critical components:
The PR adds channel update events to the subscription system, which involves coordination across the entire stack from database to RPC layer. This requires expert review from engineers familiar with the channel event notification architecture and state management. To override, add a |
There was a problem hiding this comment.
Code Review
The pull request introduces a new ChannelUpdateEvent and associated notification mechanism to SubscribeChannelEvents. This allows subscribers to receive real-time updates when a channel's state changes, specifically after a CommitSig is processed. The implementation correctly utilizes deep copy methods (copy() for ChannelCommitment and Copy() for OpenChannel) to ensure that channel state passed through events is immutable, preventing potential race conditions and data inconsistencies. New integration tests (testChannelUpdateNotifications) have been added to verify the functionality of these new events. Overall, the changes are well-implemented and adhere to good practices for managing shared state in concurrent environments.
e5126e8 to
cb1ab9a
Compare
🔴 PR Severity: CRITICAL
🔴 Critical (5 files)
🟠 High (3 files)
🟢 Low (7 files)
AnalysisThis PR adds channel update notifications to the
While the individual changes appear focused on adding event notification functionality, the breadth of critical systems involved warrants expert review to ensure:
To override, add a |
cb1ab9a to
b7c9c42
Compare
🔴 PR Severity: CRITICAL
🔴 Critical (5 files)
🟠 High (1 file)
🟡 Medium (1 file)
🟢 Low/Test/Generated (9 files)
AnalysisThis PR is classified as CRITICAL due to modifications across multiple core critical packages:
The PR extends the
Recommendation: This PR requires expert review from engineers familiar with channel state management, the HTLC switch, and the channel notification system. Special attention should be paid to:
To override, add a |
Change Description
This PR extends the
SubscribeChannelEventsRPC to emit the channel state when we receive aCommitSigfor the local commitment. This can be useful to track the exact available balances at each point in time. I have considered to updaterouterrpc.SubscribeHtlcEventswith channel balances, but I think it isn't that clean and channel update events could also be emitted if channel constraints or the channel type changes (dynamic commitments).There is a little cost to this code as we have to make a copy of the channel state to transport it to the RPC. I decided to make a copy of
channeldb.OpenChannelas I wanted to reuse the mashal helpercreateRPCOpenChanneland thelnrpc.Channeltype (which is also used forListChannels).