-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
routing: repopulate missing edges in graph for local channels #8768
routing: repopulate missing edges in graph for local channels #8768
Conversation
Some channels missed corresponding edges in the graph database. This had a few implications: - `lncli getchaninfo` would return "edge not found" - forwarding htlcs over a channel with a missing edge would use the default channel policy - `lncli updatechanpolicy` would fail to the edge missing This commit adds a flag to LND startup: `repopulate-missing-edges`. When LND is started with this flag, any missing edges will be prefilled in the database with the default routing policy. This allows the user to later update the channel policy of these channels. Note that 784dc8d partly addresses the issue for new channels, but old channels were still affected by this issue.
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
I'm not entirely sure how to test. Ideally I'd like to run an integration test where 2 nodes have a channel open. Then stop one of the nodes, delete the edge from the database and run the node again. Not sure how to access the node's database in the test though, with all the different backends available. |
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.
Do we know the root cause of this issue, like why it's missing in the first place?
// RepopulateMissingEdges is a value indicating whether missing edges of | ||
// local channels should be re-added to the local view of the graph on | ||
// startup. | ||
RepopulateMissingEdges bool `long:"repopulate-missing-edges" description:"Repopulates any missing edges of local channels in the node's local view of the graph. This can help with 'edge not found' issues where LND will always select the default routing policy. Should be set to false after successful execution to avoid repopulating on every restart of lnd."` |
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.
This looks more like a db migration for the affected nodes than a config to me.
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.
If the problem was permanently fixed in #7613 that could be a better solution. What do you think @ellemouton? Was the 'edge not found' issue reliably fixed by that PR?
It happens mostly on zero conf private channels. Also at least one public channel on our node is affected. I don't know what causes this. I assume it has to do with a peer disconnect during the initial channel setup process. |
Another option is to add the missing edge in the default case here if the edge was not updated, but the channel exists, what do you think @yyforyongyu? |
There's a few ways to go with this PR
Please provide some guidance on what I should do. |
Thanks for digging into this! Left some comments in the original issue, I still want to understand why it happened, to make sure we are not hiding the underlying problem by re-populating the policy. Meanwhile,
I'd prefer this option, with a new param in the |
@yyforyongyu I had made a PR for inserting the edge in updatexhanpolicy here #8805. I'll add the flag there. Can you do a review round on that, especially around the way the edge will eventually be persisted? |
@JssDWt, remember to re-request review from reviewers when ready |
Think it's replaced by #8805, feel free to re-open if otherwise. |
Change Description
Some channels missed corresponding edges in the graph database. This had a few implications:
lncli getchaninfo
would return "edge not found"lncli updatechanpolicy
would fail to the edge missingThis commit adds a flag to LND startup:
repopulate-missing-edges
. When LND is started with this flag, any missing edges will be prefilled in the database with the default routing policy. This allows the user to later update the channel policy of these channels.Note that 784dc8d partly addresses the issue for new channels, but old channels were still affected by this issue.
Fixes #7261
Steps to Test
I'm not entirely sure how to test this properly yet. Have a database where the channel edge is missing.
lncli getchaninfo
should fail. Then run LND with therepopulate-missing-edges
flag. It should fix the issue.Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]
in the commit message for small changes.📝 Please see our Contribution Guidelines for further guidance.