graph/db: private taproot V1→V2 channel migration#10676
graph/db: private taproot V1→V2 channel migration#10676ellemouton wants to merge 9 commits intolightningnetwork:masterfrom
Conversation
Summary of ChangesHello, 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 refactors how private taproot channels are stored in the database, moving them from an older V1 workaround to a canonical V2 representation. This change is crucial for improving data consistency and preparing the system for future full V2 gossip support. A transparent shim ensures backward compatibility, allowing existing V1-aware components to seamlessly interact with the newly migrated V2 channels without requiring immediate updates to those components. 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. 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. 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 implements a significant database migration for private Taproot channels, moving them from a V1 workaround storage format to a canonical V2 storage. The changes include defining a new migration version, implementing the core logic to convert channel and policy data, and introducing a compatibility shim to ensure existing V1-aware components can interact with the migrated V2 channels. The shim provides functions for projecting V2 data back to V1 and handling V1 policy updates for V2 channels. Comprehensive unit and integration tests have been added to validate the migration and shim behavior, alongside a development flag to skip the migration for testing. A review comment suggests an optimization to the migratePolicies function to improve efficiency by directly passing V2 node IDs, thereby avoiding redundant database queries.
| func migratePolicies(ctx context.Context, tx SQLQueries, | ||
| v1Chan sqlc.GraphChannel, c taprootMigrationCandidate, | ||
| v2ChanID int64) error { | ||
|
|
||
| // Map V1 node IDs to V2 node IDs via pubkey lookup. We already | ||
| // created the V2 shell nodes in migrateOneChannel. | ||
| v1NodeIDToV2 := make(map[int64]int64) | ||
| for _, entry := range []struct { | ||
| v1NodeID int64 | ||
| pubKey []byte | ||
| }{ | ||
| {v1Chan.NodeID1, c.node1PubKey}, | ||
| {v1Chan.NodeID2, c.node2PubKey}, | ||
| } { | ||
| v2ID, err := tx.GetNodeIDByPubKey( | ||
| ctx, sqlc.GetNodeIDByPubKeyParams{ | ||
| Version: int16(gossipV2), | ||
| PubKey: entry.pubKey, | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf("fetching v2 node ID: %w", err) | ||
| } | ||
|
|
||
| v1NodeIDToV2[entry.v1NodeID] = v2ID | ||
| } |
There was a problem hiding this comment.
The V2 node IDs (v2Node1ID and v2Node2ID) are already available in migrateOneChannel. To improve efficiency and avoid redundant database queries, you could pass these IDs directly to migratePolicies.
This would involve:
- Changing the signature of
migratePoliciesto acceptv2Node1IDandv2Node2ID. - Updating the call site in
migrateOneChannel. - Simplifying the logic inside
migratePoliciesto use the passed-in IDs instead of re-querying them.
For example, you could change migratePolicies to:
func migratePolicies(ctx context.Context, tx SQLQueries,
v1Chan sqlc.GraphChannel, v2ChanID, v2Node1ID, v2Node2ID int64) error {
// Map V1 node IDs to their V2 counterparts.
v1NodeIDToV2 := map[int64]int64{
v1Chan.NodeID1: v2Node1ID,
v1Chan.NodeID2: v2Node2ID,
}
// ... rest of the functionAnd in migrateOneChannel:
// ...
if err := migratePolicies(
ctx, tx, ch, v2ChanID, v2Node1ID, v2Node2ID,
); err != nil {
return fmt.Errorf("migrating policies: %w", err)
}
// ...This refactoring would make the migration process more efficient, especially for nodes with a large number of channels to migrate.
func migratePolicies(ctx context.Context, tx SQLQueries,
v1Chan sqlc.GraphChannel, v2ChanID, v2Node1ID, v2Node2ID int64) error {
// Map V1 node IDs to their V2 counterparts.
v1NodeIDToV2 := map[int64]int64{
v1Chan.NodeID1: v2Node1ID,
v1Chan.NodeID2: v2Node2ID,
}
ellemouton
force-pushed
the
elle-v2-shim-base
branch
from
416fc00 to
ab224df
Compare
ellemouton
force-pushed
the
migrate-tap-chans
branch
from
9f58b16 to
2323ce6
Compare
ellemouton
force-pushed
the
elle-v2-shim-base
branch
from
ab224df to
2b2edb5
Compare
Add projection helpers for the private taproot V1→V2 migration shim. These convert between V1 and V2 representations of channel edge info and policies, enabling the SQL store to internally store migrated private taproot channels as V2 while presenting them as V1 to callers that are not yet V2-aware. Includes helpers for: - Approximate timestamp↔block-height conversion - V2 edge → V1 edge projection (re-adds taproot staging feature bit) - V2 policy → V1 policy projection (converts block height to timestamp) - V1 policy → V2 policy projection (for write shim) - Private taproot channel detection
Add a shim layer to the SQL store that makes migrated V2 private taproot channels visible to V1-only callers. After the SQL migration converts private taproot channels from V1 workaround storage to canonical V2, callers (gossiper, builder, server) still query with GossipVersion1. The shim transparently falls back to V2 and projects results back to V1 with the taproot staging feature bit re-added. Affected methods: - FetchChannelEdgesByID: V2 fallback lookup on V1 miss - FetchChannelEdgesByOutpoint: V2 fallback lookup on V1 miss - HasChannelEdge: V2 fallback lookup on V1 miss - UpdateEdgePolicy: V1→V2 policy translation when channel is V2 - ForEachChannel: includes V2 private taproot channels in V1 iteration - ForEachChannelCacheable: includes V2 channels in cache for pathfinding - ForEachNodeDirectedChannel: includes V2 channels per-node - ForEachNodeChannel: includes V2 channels per-node - ChannelView: includes V2 channels in startup chain filter - FetchChanInfos: V2 fallback for missing V1 channel IDs All shim code is marked with TODO(elle) for removal when the gossiper/builder gain full V2 support.
Add the migration function that converts private taproot channels from V1 workaround storage (V1 + SimpleTaprootChannelsStaging feature bit) to canonical V2 storage. The migration: - Finds V1 channels with the taproot staging feature bit (180/181) - Creates V2 shell nodes for both channel endpoints - Computes and stores the taproot funding script - Converts policies (timestamp → approximate block height) - Copies features (minus taproot staging bits), extras, policy extras - Deletes old V1 rows (CASCADE handles cleanup) The migration is idempotent and runs as a SQL-only data transformation within the existing schema (no DDL changes needed). Also adds RunTaprootV2Migration method on SQLStore for test use, and unit tests covering the happy path, idempotency, and non-taproot channels being untouched.
Register the private taproot migration as database version 17 in the migration config and wire the migration function in config_builder.go. The migration runs after all existing migrations (including KV→SQL graph migration if applicable) and converts private taproot channels from V1 workaround storage to canonical V2. No schema version change is needed since V2 channel columns already exist from the 000009_graph_v2_columns migration.
Add --dev.skip-taproot-v2-migration flag (integration builds only) to allow itests to control when the private taproot V1→V2 migration runs. The flag is always false in production builds.
Add an integration test that verifies a private taproot channel survives the V1→V2 SQL data migration: 1. Start Alice with --dev.skip-taproot-v2-migration 2. Open a private taproot channel, send payment (V1 workaround storage) 3. Restart Alice without the skip flag (migration runs) 4. Verify channel is still active and payments work in both directions
ellemouton
force-pushed
the
migrate-tap-chans
branch
from
2323ce6 to
ef00be1
Compare
2 participants
Summary
Migrates private taproot channels from the V1 gossip workaround (V1
ChannelAnnouncement+SimpleTaprootChannelsRequiredStagingfeature bit) to canonical V2 storage in the SQL graph backend. This is a DB-layer-only change — the gossiper, builder, server, and peer layers are unmodified.Private taproot channels have always been stored as fake V1 gossip objects with a feature bit signalling taproot support. This was necessary because V2 gossip wasn't available when taproot channels were introduced. With V2 column support already in the SQL schema (from
000009_graph_v2_columns), we can now migrate these channels to their correct canonical representation ahead of full V2 gossip support.Approach
SQL migration (version 17): Finds V1 channels with the taproot staging feature bit, computes the taproot funding script, creates V2 shell nodes, converts policies (timestamp → approximate block height), copies features/extras, and deletes the old V1 rows. Idempotent and SQL-only — no schema changes needed.
SQLStore shim: Since callers (gossiper, builder, etc.) still query with
GossipVersion1, the SQL store transparently falls back to V2 for migrated private taproot channels and projects results back to V1 with the taproot feature bit re-added. This covers lookups (FetchChannelEdgesByID,FetchChannelEdgesByOutpoint,HasChannelEdge), iteration (ForEachChannel,ForEachNodeDirectedChannel,ChannelView, etc.), and writes (UpdateEdgePolicyconverts incoming V1 policies to V2). All shim code is markedTODO(elle)for removal when the gossiper/builder gain full V2 support.Policy update compatibility: When a legacy peer sends a V1
ChannelUpdatefor a migrated channel, the shim converts it to V2 and persists it. When we update our own policy, the shim reads the V2 channel (projected as V1), the gossiper signs a V1 update, the shim persists it as V2, and the legacy peer receives a standard V1 wire message.Key design decisions
sql_store.goandtaproot_v2_shim.go.height * 600 + genesis). Acceptable because these are our own private channels and policies get re-signed.isPrivateTaprootV2,len(Signature) == 0) ensure only private channels are affected. No real V2 public channels exist until the gossiper gains V2 support.Test plan
🤖 Generated with Claude Code