chanstate: move channel state kv store#10812
Open
ziggie1984 wants to merge 116 commits into
Open
Conversation
github-actions
Bot
added
the
severity-critical
🔴 PR Severity: CRITICAL
🔴 Critical (31 files)
🟠 High (4 files)
🟡 Medium (39 files)
AnalysisThis PR is a large-scale architectural refactoring that extracts channel state management code from
Reviewers should pay particular attention to the data migration path (if any), the correctness of the serialization code moved from To override, add a |
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 continues the decomposition of the channel state management system by migrating core data types and KV-backed store logic into a dedicated 'chanstate' package. This refactor improves modularity by allowing channel-state callers to depend on a clean domain-driven contract rather than backend-specific implementation details. The changes are structured to be compatibility-preserving, ensuring existing codebases continue to function while providing a foundation for future backend support. Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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 refactors the channel state management by migrating core types, constants, and logic from the channeldb package into a new, dedicated chanstate package. It introduces a Store interface to abstract the persistence layer, allowing for backend-independent channel state operations. Widespread changes across the codebase update references to use the new chanstate types, while channeldb retains compatibility aliases to facilitate an incremental transition. I have no feedback to provide as the review comments were found to be inaccurate regarding the provided code changes.
Collaborator
|
@GeorgeTsagk: review reminder |
Move the small value types referenced by chanstate.Store out of channeldb. This includes ChannelConfig, ChannelStatus, ChannelCloseSummary, ChannelShell, ChanCount, and FinalHtlcInfo. Leave aliases in channeldb so existing callers keep compiling while the backend still lives there. Parameterize the Store facets over the channel type and instantiate current callers with *channeldb.OpenChannel. This removes the chanstate -> channeldb import edge without moving OpenChannel yet, keeping the first step reviewable and backend-neutral.
Move ChannelType and its flag helpers into chanstate while leaving compatibility aliases in channeldb. This is a backend-neutral value type and does not require moving any KV serialization logic. Keep the full type documentation with the moved chanstate definition. The channeldb aliases preserve the existing public surface while later commits continue moving OpenChannel state out of the KV package.
Move the OpenChannel error definitions into chanstate and leave channeldb aliases for existing callers. These errors describe channel state behavior rather than a concrete KV bucket layout. Keeping the aliases preserves the public channeldb API while later commits move more OpenChannel state and receiver logic toward chanstate.
Move the ShutdownInfo state type, constructor, and closer helper into chanstate. The type describes channel shutdown state and is not tied to the concrete KV backend. Keep the TLV encode and decode helpers in channeldb for now, since those functions describe the current persisted format. The channeldb constructor remains as a compatibility wrapper.
Add a lifecycle facet to the chanstate Store contract for refresh, confirmation, open-state, and SCID mutations. Implement the facet on ChannelStateDB using the existing KV persistence code. Update the matching OpenChannel receivers to call through the store methods instead of reaching into the ChannelStateDB backend directly. Also convert fullSync into a channeldb helper so that KV-specific code is no longer an OpenChannel receiver.
Add a status facet to the chanstate Store contract for status bit updates and data-loss commit point handling. Implement the facet on ChannelStateDB using the existing persistence code. Update the matching OpenChannel receivers to call through the store methods. The broadcast path still uses a private channeldb helper until its closing-transaction facet is introduced in a later commit.
Add shutdown and close-transaction facets to the chanstate Store contract. These cover persisted shutdown info plus stored unilateral and cooperative closing transactions. Implement the facets on ChannelStateDB with the existing KV code and update OpenChannel receivers to call through the store methods. The backend-specific key selection remains private to channeldb.
Add pending-channel setup to the chanstate lifecycle store facet. This covers the path that writes a new pending channel and records the funding broadcast height. Move the OpenChannel receiver to call through ChannelStateDB and pass the backend explicitly into the channeldb sync helper. This keeps the link-node persistence detail in channeldb while removing another direct backend reference from OpenChannel.
Move ChannelCommitment and HTLC into chanstate so upcoming store facets can name commitment state without importing channeldb. Leave the KV serialization helpers in channeldb and keep aliases for existing call sites. This preserves the current disk format and keeps backend-specific persistence code out of chanstate for now.
Move LogUpdate into chanstate so commitment store interfaces can refer to pending update state without importing channeldb. Keep the log-update serialization helpers in channeldb. Those helpers remain part of the existing KV disk format and can move with the KV backend implementation later.
Add a commitment-focused store facet for updating local channel commitment state. This lets OpenChannel call through the chanstate store contract instead of reaching directly into the KV backend. Keep the existing KV transaction body on ChannelStateDB for now. The receiver still owns locking and in-memory state updates while the store method owns persistence.
Move CommitDiff and its forwarding reference types into chanstate. This lets the next commitment store facet name pending remote commitment state without importing channeldb. Keep forwarding package persistence and commit-diff serialization in channeldb for now. The aliases preserve existing call sites while the KV backend code remains in place.
Add the remote commitment-chain append method to the chanstate commitment store facet. Move the existing KV transaction body onto ChannelStateDB and have the OpenChannel receiver call through the store. This removes another direct backend dependency from OpenChannel while keeping KV persistence code in channeldb.
Add read-side commitment lookup methods to the chanstate commitment store facet. Move the existing OpenChannel KV view transaction bodies onto ChannelStateDB. Leave the OpenChannel receivers as store-call wrappers. This removes three more direct backend references from the receiver code without changing the persisted data format.
Add the next-revocation persistence method to the chanstate commitment store facet. Move the existing OpenChannel KV update body onto ChannelStateDB. The OpenChannel receiver keeps the external locking behavior and delegates persistence through the store interface.
Move the borked-channel bucket check into the channel state KV open channel file with its existing documentation. Keep channeldb using the same helper name as a wrapper so commitment update paths continue to call through unchanged.
Move simple open channel metadata update paths into the channel state KV open channel file. Keep the ChannelStateDB methods in channeldb as wrappers while the backend-specific transaction logic shifts into chanstate.
Move open channel status update logic into the channel state KV open channel file, including the callback-capable helper used by related status paths. Keep channeldb methods delegating through their existing names while transaction and bucket logic continues moving into chanstate.
Move the channel data-loss status and commit-point fetch paths into the channel state KV data-loss file. Keep channeldb methods as wrappers so callers continue using the existing Store interface while KV transaction logic moves into chanstate.
Move close transaction status updates and close transaction fetch logic into the channel state KV close transaction file. Keep channeldb methods delegating to chanstate while preserving the public store surface used by channel callers.
Move channel shutdown info store and fetch logic into the channel state KV shutdown file. Keep channeldb methods as wrappers while preserving the Store interface and shifting KV transaction handling into chanstate.
Move forwarding package store methods into the channel state KV forwarding package file beside the packager implementation. Keep channeldb Store methods as wrappers so callers retain the same interface while forwarding KV transactions move into chanstate.
Move remote commitment tip, unsigned update reads, and next revocation persistence into the channel state KV commitment file. Keep channeldb methods as wrappers while commitment transaction logic continues moving into chanstate.
Move the commit-chain mutation and read paths into the chanstate KV store implementation. Keep channeldb as a thin wrapper while callers still use the existing ChannelStateDB surface.
Move revocation-log lookup compatibility and tail-height queries into the chanstate KV store implementation. Keep channeldb wrappers for existing tests and callers while the package boundary is being moved.
Move the local commitment update path into the chanstate KV store implementation. Keep channeldb as the public wrapper and pass the final-HTLC storage option through to preserve existing behavior.
Move the close/archive channel paths into the chanstate KV store implementation. Keep channeldb as the public wrapper and preserve the existing sync/tombstone strategy selection at the boundary.
Move the refresh path into the chanstate open-channel KV store implementation. Keep channeldb as a public wrapper and remove private helpers that were only used by the old refresh implementation.
Move the pending open-channel write into the chanstate KV store implementation while keeping link-node creation in channeldb. Preserve the existing transaction boundary used by pending channels and restores.
Fold the channel key and thaw-height helpers into the store files that own them, instead of keeping separate kv files that do not represent their own stores. The open-channel keys and thaw-height helpers now live with kv_open_channel, closed-channel bucket ownership lives with kv_close_summary, and commitment/update keys live with kv_commitment.
Move the KV-backed ChannelSetupStore implementation onto the chanstate KVStore. This includes both channel-opening state and initial forwarding policy persistence, which together make up the setup store facet. Keep the channeldb ChannelStateDB methods as compatibility wrappers so callers can continue using the old package while the concrete KV store implementation moves by facet.
Move the KV-backed FinalHTLCStore implementation onto KVStore. The ChannelStateDB methods keep compatibility wrappers while final HTLC lookup and on-chain outcome writes now live with the final HTLC KV store code. Carry the store-final-resolution option into KVStore so the on-chain outcome path keeps the existing opt-in behavior.
Move the KV-backed OpenChannelFwdPkgStore implementation onto KVStore. ChannelStateDB keeps the existing wrapper methods, but now delegates through the channelstate KV store instead of passing its backend into package-level helpers. This keeps forwarding package persistence grouped with the existing kv_forwarding_package code while removing another direct backend use from the channeldb compatibility layer.
Move the KV-backed OpenChannelShutdownStore methods onto KVStore. ChannelStateDB keeps the existing compatibility methods, but shutdown state now flows through the channelstate KV store rather than backend helper functions. The bucket-level shutdown codec helper stays in kv_shutdown because it is still the local serialization primitive used by the store method.
Move the KV-backed OpenChannelCloseTxStore implementation onto KVStore. ChannelStateDB keeps compatibility wrappers for marking and fetching broadcast close transactions while the KV logic now lives in kv_close_tx. Keep the generic close transaction fetch helper private to KVStore and expose only the domain methods required by the channelstate interface.
Move the KV-backed OpenChannelStatusStore implementation onto KVStore. ChannelStateDB keeps compatibility wrappers while status updates, data-loss commit point lookup, and borked marking now flow through the channelstate KV store. Keep the status write helper private to KVStore so related KV store facets can share the same status mutation path without passing raw backends around.
Move the backend-local open channel lifecycle mutations onto KVStore. ChannelStateDB keeps compatibility wrappers for refreshing channel state and marking confirmation, open, real scid, and scid-alias negotiation state. Leave SyncPendingChannel in channeldb for now because that path still creates link-node records, and LinkNodeDB ownership remains in channeldb for this refactor.
Move the KV-backed open channel commitment store methods onto KVStore. ChannelStateDB keeps compatibility wrappers for commitment updates, pending commit diffs, revocation queries, and previous-state lookups. Carry the no-rev-log-amount-data option into KVStore alongside the existing final HTLC option so revocation log and final-resolution writes preserve their previous configuration behavior.
Move the KV-backed revocation log tail helper onto KVStore. This removes the last direct backend call from the channeldb open channel compatibility methods while keeping the test-facing helper in place.
Move closed-channel summary reads and CloseChannel onto KVStore. ChannelStateDB keeps compatibility wrappers, while the KV code for close summary lookup and close archiving now lives with kv_close_summary. Carry the tombstone-close option into KVStore so the existing close strategy selection remains unchanged. Leave MarkChanFullyClosed and AbandonChannel in channeldb because they still coordinate link-node pruning and cross-store behavior.
Move historical channel bucket lookup and FetchHistoricalChannel onto KVStore. The channeldb wrapper still attaches itself to channel.Db so existing callers continue to receive a compatibility store reference. Move ErrNoHistoricalBucket into chanstate and keep the channeldb error as an alias, matching the other channel-state errors that have moved.
Move the KV-backed open channel read and scan methods onto KVStore. ChannelStateDB keeps compatibility wrappers that attach themselves to returned OpenChannel values so existing receiver methods still use the channeldb store. Leave restore and sync paths in channeldb because they still coordinate link-node creation and repair while LinkNodeDB ownership remains outside chanstate.
Move the KV-backed AbandonChannel implementation onto KVStore. ChannelStateDB keeps a compatibility wrapper while callers still import channeldb for the channel-state store. Keep the link-node-coupled close and repair methods in channeldb so the follow-up LinkNode store can define that boundary explicitly.
ziggie1984
force-pushed
the
chanstate-kv-store-move
branch
from
8be8faf to
13c62c9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Is based on #10808 and #10809
Context
Part of #10680. This PR continues the channel-state decomposition by
moving the OpenChannel type and KV-backed channel-state store facets into
chanstate. The goal is to make channel-state callers depend on the
chanstate store contract instead of backend-specific channeldb details,
while keeping the current KV backend behavior unchanged.
Story
The refactor is structured as a compatibility-preserving move. Existing
callers can still use channeldb.ChannelStateDB, but the concrete KV
implementation for channel-state facets now lives in chanstate.KVStore.
This lets later PRs introduce alternate backends behind the same domain
interfaces without first changing every call site.
The PR also moves OpenChannel and related value types into chanstate, then
updates consumers to use the chanstate type directly. channeldb keeps type
aliases and thin wrappers only as a transition layer for existing imports.
What moved
forwarding-package, final-HTLC, historical-channel, closed-summary, and
open-channel read paths are now implemented on chanstate.KVStore.
chanstate.KVStore.
Intentional boundaries
repair, or prune LinkNodes stay in channeldb until a follow-up LinkNode
store boundary is introduced.
ChannelStateDB and while OpenChannel receiver methods still require a
Store reference. A follow-up should remove OpenChannel.Db by converting
persistence receiver calls to store-method calls.
Verification