[graph mig 2]: graph/db: migrate graph channels and policies from kvdb to SQL #10037
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @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 is a crucial step in the ongoing effort to transition the Lightning Network graph data from a key-value store to a more scalable and queryable SQL database. It specifically addresses the migration of channel information and their routing policies, ensuring that this critical network data is accurately and robustly transferred, with safeguards for data integrity.
Highlights
- Graph Migration Expansion: This pull request significantly expands the ongoing graph database migration from KVDB to SQL by introducing the core logic for migrating Lightning Network channels and their associated policies.
- Data Validation and Integrity: Enhanced validation for
ExtraOpaqueData(TLV streams) has been implemented. During migration, channels and policies with invalid TLV data or missing required fields are now gracefully skipped, with warnings logged, ensuring data integrity in the new SQL store. TLV validation for policy updates in the KV store was also moved to an earlier stage. - Comprehensive Testing: Extensive new test cases have been added to verify the channel and policy migration process. This includes tests for various channel/policy configurations, as well as specific edge cases involving invalid or malformed data, ensuring the robustness of the migration.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
| err := edge.ExtraOpaqueData.ValidateTLV() | ||
| if err != nil { | ||
| return fmt.Errorf("%w: %w", | ||
| ErrParsingExtraTLVBytes, err) | ||
| } |
There was a problem hiding this comment.
This validation logic was moved to kv_store.go's UpdateEdgePolicy function, so the error will now be thrown there instead of being handled here. It might be useful to add a comment explaining that this error is now handled elsewhere.
ellemouton
force-pushed
the
elle-graph-mig-1
branch
from
ab4a75e to
0c03a49
Compare
ellemouton
force-pushed
the
graphMig2-channels
branch
from
4737642 to
b067f7d
Compare
Here, we move TLV validation for the KVStore out of `updateEdgePolicy` so that we can re-use `updateEdgePolicy` in our tests to write policies with invalid TLV (since that was possible before the recently added TLV sanity check) so that we can test that our SQL migration behaves correctly for these cases.
Here we factor out some of the crud code in TestEdgePolicyMissingMaxHtcl so that we can re-use it later on.
In this commit, the `MigrateGraphToSQL` function is expanded to migrate the channel and channe policy data. Both of these have the special case where the kvdb store records may contain invalid TLV. If we encounter a channel with invalid TLV, we skip it and its policies. If we encounter a policy with invalid TLV, we skip it. The `TestMigrateGraphToSQL` and `TestMigrationWithChannelDB` tests are updated accordingly.
ellemouton
force-pushed
the
elle-graph-mig-1
branch
from
0c03a49 to
d62aa8f
Compare
ellemouton
force-pushed
the
graphMig2-channels
branch
from
b067f7d to
d566609
Compare
ellemouton
merged commit d566609
into
lightningnetwork:elle-graph-mig-1
|
accidental branch mess up. Will re-open |
This expands the graph migration code& tests to cover channels and channel policies.
Please see #10025 for the final result we are aiming for here.
Part of #9795
Depends on #10036