[backport] localchans: populate FundingScript for missing edges #10443
Conversation
So that we can re-use this helper else where.
When creating a missing edge, we need to populate the funding script too so that the graph builder can update its ChainView appropriately. We use the MakeFundingScript helper from the funding package which ensures that we are using the same logic for creating a funding script as is used for any of the channels that we own.
ellemouton
force-pushed
the
backport-fixMissingEdge
branch
from
06edebf to
8416bc0
Compare
Summary of ChangesHello @ellemouton, 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 backports a critical fix that ensures the correct construction of the channel graph for local channels. Previously, validation checks introduced after a graph refactor prevented the addition of certain channel edges due to missing funding script details. This change addresses that by making the funding script generation function public and ensuring it is always included when creating these graph entries, thereby maintaining the integrity and completeness of the network graph. Highlights
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request appears to be a backport of a fix to correctly populate the FundingScript for missing channel edges, which resolves a bug in adding local channels to the graph database. The change involves exporting the makeFundingScript function from the funding package and utilizing it in the routing/localchans package. The implementation seems correct and is well-tested. My feedback focuses on improving the documentation for the newly exported function to align with the style guide and fixing a placeholder in the release notes.
| * Fix a bug where [missing edges for own channels could not be added to the | ||
| graph DB](https://github.com/lightningnetwork/lnd/pull/10443) |
There was a problem hiding this comment.
The link for this bug fix points to pull/xxxxx, which seems to be a placeholder. Please update it with the correct pull request number.
| // MakeFundingScript re-creates the funding script for the funding transaction | ||
| // of the target channel. |
There was a problem hiding this comment.
Per the LND Style Guide, exported functions should have detailed comments. The current comment for MakeFundingScript is quite brief. It would be beneficial to expand it to explain what kind of script is being created (e.g., P2WSH for legacy channels, P2TR for taproot channels) and why it's needed, similar to the detailed example provided in the style guide for DeriveRevocationPubkey.
References
- Exported functions require detailed comments for the caller. The provided example for a 'RIGHT' comment is very descriptive, explaining the 'why' and 'how' of the function. (link)
ellemouton
force-pushed
the
backport-fixMissingEdge
branch
from
8416bc0 to
53994da
Compare
3 participants
backport #10410