chanstate: move OpenChannel behind store interface#10808
Conversation
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 FwdState, PkgFilter, and FwdPkg into chanstate with their existing comments and helper methods. Leave channeldb aliases for the moved value types and constructors so current callers keep compiling. The KV forwarding package persistence code stays in channeldb.
Add the commitment-tail advancement method to the chanstate commitment store facet. Move the existing AdvanceCommitChainTail KV transaction body onto ChannelStateDB. The OpenChannel receiver now keeps locking and restored channel checks before delegating persistence through the store.
Add a forwarding-package store facet to chanstate.Store. Move the existing OpenChannel forwarding-package KV transaction bodies onto ChannelStateDB. The OpenChannel receivers keep their locking behavior and delegate package loading, acking, filtering, and removal through the store.
Add commitment-height, latest-commitment, and remote revocation store lookups to the chanstate commitment store facet. Move the existing OpenChannel KV view transaction bodies onto ChannelStateDB. This leaves the receivers as store-call wrappers while keeping the persisted format and read behavior unchanged.
Move the remaining OpenChannel revocation-log KV reads onto ChannelStateDB. This keeps FindPreviousState and the unit-test tail-height helper as OpenChannel wrappers. It removes direct backend access from the receiver methods while leaving RevocationLog in channeldb for now.
Move the revocation-log value types and TLV serialization helpers into chanstate. Leave channeldb aliases and wrapper functions for the existing KV persistence code and tests. Bucket keys, errors, and transaction helpers stay in channeldb, so this commit only moves backend-neutral state data.
Add FindPreviousState to the chanstate commitment store facet now that RevocationLog is a chanstate value type. This extends the store contract without changing runtime behavior. The existing ChannelStateDB method already satisfies the new method.
Keep the revocation-log tail-height helper on ChannelStateDB instead of the OpenChannel receiver. The helper is only used by channeldb tests, so it should not become part of the backend-independent chanstate store contract. The tests now call the concrete helper directly.
Change OpenChannel.Db to the composed chanstate Store interface while keeping the existing field name. Tests that need raw channeldb access now assert the concrete test backend explicitly instead of reaching through OpenChannel.Db. This keeps backend setup out of the store contract.
Convert the KV-only OpenChannel helpers for TLV aux data and borked-state lookup into package-level channeldb helpers. This keeps serialization and bucket inspection code tied to the KV backend while leaving the OpenChannel receiver set closer to the future chanstate type.
Add transitional OpenChannel accessors for the channel status and confirmed SCID fields used by KV store code. These helpers keep the fields private while allowing channeldb backend code to continue hydrating and serializing channel state after OpenChannel moves to chanstate.
Remove the KV forwarding packager from OpenChannel and derive a ChannelPackager inside the channeldb store methods that need one. This keeps the backend-specific kvdb transaction helper in channeldb, so the OpenChannel type no longer carries that dependency toward chanstate.
Move the backend-neutral ChannelSnapshot value type into chanstate and leave channeldb with a compatibility alias. This keeps the future OpenChannel Snapshot receiver close to its return type without changing existing channeldb callers.
Move the backend-neutral taproot shachain and verification nonce helpers into chanstate with the thaw-height threshold they support. Leave channeldb aliases for existing callers while OpenChannel and its receiver methods are moved across the package boundary.
Add a transitional non-locking status predicate for channeldb store code and use it from KV serialization helpers. This avoids calling an unexported OpenChannel helper from channeldb after the type moves into chanstate.
Move OpenChannel and its backend-neutral receiver methods into the chanstate package. channeldb now keeps a compatibility alias while retaining the KV store implementation and serialization helpers. Tests that used private channel status fields now use store-facing accessors.
Drop the temporary channel type parameter from the channel-state store interfaces now that OpenChannel lives in chanstate. The domain store facets now refer to *OpenChannel directly while retaining the same backend-independent shape. Update callers and compatibility aliases to use the concrete Store and ChannelShell types.
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 initiates a structural refactor to decouple channel-state storage from the existing KV implementation in 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 performs a significant refactor by moving channel state logic, types, and interfaces from channeldb into a new chanstate package. This change decouples the core channel state management from the specific key-value database implementation, introducing a more modular Store interface. Feedback highlights a critical bug in the newly moved HTLC.Copy method, where missing field assignments and unallocated slices for Signature and ExtraData result in data loss during copying operations.
github-actions
Bot
added
the
severity-critical
PR Severity: CRITICALAutomated classification | 23 non-test files | ~6900 lines changed Critical files (8): channeldb/channel.go, channeldb/chanstate_assertions.go, channeldb/db.go, channeldb/forwarding_package.go, channeldb/revocation_log.go, htlcswitch/test_utils.go, lnwallet/test_utils.go, peer/test_utils.go Medium files (15): New chanstate package - chanstate/channel.go, channel_status.go, channel_type.go, close_summary.go, commitment.go, config.go, errors.go, forwarding.go, interface.go, open_channel.go, open_channel_types.go, revocation_log.go, shutdown.go, snapshot.go, taproot.go Test files excluded (8): channeldb/channel_test.go, channeldb/close_channel_test.go, channeldb/db_test.go, contractcourt/breach_arbitrator_test.go, contractcourt/utils_test.go, htlcswitch/link_test.go, lnwallet/taproot_test_vectors_test.go, lnwallet/transactions_test.go Analysis: This PR refactors channel state persistence by extracting types from channeldb into a new chanstate package. channeldb/ files are CRITICAL - core channel state persistence with ~2500 lines net change to channel.go, revocation_log.go, and forwarding_package.go. htlcswitch/, lnwallet/, peer/ test utilities also touched. Three bump triggers present (all moot at CRITICAL): 23 non-test files, ~6900 lines, multiple critical packages. Reviewers should verify chanstate/ extraction preserves all channeldb invariants around serialization, revocation logs, and HTLC forwarding. To override: add a severity-override-{critical,high,medium,low} label. |
Copy all HTLC fields when cloning channel commitment state. The old copy method only copied a subset of scalar fields and copied into nil slices for Signature and ExtraData. Allocate those slices and deep-copy custom record values so snapshots and channel copies retain complete HTLC metadata.
Code Review — Main FindingsVerdict: approve with minor follow-ups. No blockers. Build / Findings below are framed as either Phase-2 prep (cheaper to fix before a SQL backend lands) or minor polish. Major
Minor
Nits
Risk note
|
Correction: pre-existing findingsziggie pointed out (correctly) that this PR is mostly code-movement and most of my findings reproduce conditions already in Withdrawn — pre-existing on
|
1 participant
Change Description
This PR prepares the channel-state code for backend-independent storage by
moving the backend-neutral
OpenChannelmodel and related value types into thenew
chanstatepackage.The end goal is to support multiple channel-state backends, such as the current
KV backend and a future native SQL backend, without forcing channel consumers to
depend on the concrete
channeldb.ChannelStateDBimplementation. This PR isthe first decomposition step: it introduces the package boundary and interface
shape while intentionally keeping the KV implementation in
channeldb.Review Story
The commits are structured to make the refactor reviewable in small steps:
Create the
chanstate.Storecontract and split it into domain-focusedfacets instead of one unstructured interface. The store covers open channel
records, lifecycle updates, status flags, shutdown state, close
transactions, commitment state, forwarding packages, closed channel
summaries, final HTLC outcomes, setup state, and link-node maintenance.
Move backend-neutral channel-state value types from
channeldbintochanstate, keeping compatibility aliases inchanneldbfor existingcallers. This includes channel type flags, open channel errors, shutdown
metadata, channel configs, commitments, log updates, commitment diffs,
forwarding package types, revocation log types, snapshots, and taproot
helpers.
Move KV transaction bodies out of
OpenChannelreceiver methods and ontoChannelStateDB. The receivers now call store interface methods instead ofreaching into the concrete KV backend.
Add narrow migration hooks for fields that are private to
OpenChannelbutstill need to be read or written by the KV store implementation while it
remains in
channeldb. These helpers are documented as preliminary storehooks and avoid exposing the fields as normal caller API.
Move
OpenChanneland its backend-neutral receiver methods intochanstate.channeldb.OpenChannelremains as a compatibility alias, sobroad caller churn is deferred to a later PR.
Remove the temporary generic parameter from the store interfaces once
OpenChannellives inchanstate. The final interface shape now uses*chanstate.OpenChanneldirectly while staying backend-independent.Explicit Non-Goals
channeldb.channeldb.OpenChanneltochanstate.OpenChannel; compatibility aliases intentionally keep that as afollow-up.
channeldbaliases. They are kept sothe package boundary can be introduced without unrelated caller churn.
Current Package Boundary
After this PR:
chanstateowns the backend-neutral channel-state model and the storeinterface.
channeldb.ChannelStateDBsatisfieschanstate.Store.remain in
channeldb/channel.go.chanstatedoes not importchanneldborkvdb.Follow-up PRs can then migrate consumers to direct
chanstateimports and,later, move KV implementation code into a
chanstate/kvstore.go-style layout.Steps to Test
Run:
Local verification performed:
go test ./chanstate ./channeldbpassed.make release-installpassed.git diff --checkpassed.rgcheck forchanneldb/kvdbimports inchanstatereturned nomatches.
make lint-nativereached the source lint step and was killed locally withKilled: 9before producing diagnostics. This appears to be a local resourceissue rather than an actionable lint finding.
Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]in the commit message for small changes.No release notes are included because this is an internal refactor with no user
visible behavior change.