channeldb: move channel tests to owner packages#10951
Draft
ziggie1984 wants to merge 136 commits into
Draft
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 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.
Move the remaining channel-state tests and helpers into the chanstate package so the store owns the KV-specific behavior directly. This also narrows the channel-state store interface by removing cross-domain link-node lifecycle methods. Those workflows will be owned by a separate channel coordinator in the next commits.
Move link-node persistence into its own package while keeping channeldb aliases and wrappers for existing callers. This creates an explicit linknode.Store boundary that can be wired independently from channel-state persistence in later commits.
Add the channel coordination contract and a KV-backed implementation for workflows that span channel-state and link-node storage. The channeldb.DB type now constructs and exposes the coordinator while remaining the legacy KV composition and migration root. ChannelStateDB keeps compatibility wrappers that delegate to the coordinator.
Pass channel-state persistence and lifecycle coordination into the wallet separately instead of routing both through ChannelStateDB. Update the funding manager config and tests with the same split because the funding flow constructs wallets and uses the same channel-state surface.
Use channel-state store interfaces for ordinary channel reads and closed-channel queries in the arbitrators. Route lifecycle operations that also maintain link-node state through channelcoord.ChannelLifecycle instead of the ChannelStateDB facade.
Accept only the open and closed channel store methods needed to hydrate channel notification payloads. This keeps the notifier on channel-state persistence without depending on the full store surface.
Rename the peer channel persistence dependency to ChannelStateStore and describe the exact channel-state surfaces it needs. Test fixtures now write channels through the coordinator while channel records keep only the channel-state store.
Update htlcswitch test channels to persist through channelcoord while keeping channel records bound to chanstate.Store. The tests retain access to the owning channeldb.DB separately for switch construction and restart scenarios.
Keep channeldb.DB as the server's legacy KV and migration root, but store the channel-state, coordinator, and link-node dependencies separately on server. Route RPC, restore, autopilot, and subserver setup through those explicit fields so consumers no longer reach through ChannelStateDB for mixed responsibilities.
Move the close-channel tombstone and synchronous close tests from channeldb into chanstate because they exercise KVStore internals. The tests remain KV-specific and therefore live in kv_close_channel_test.go rather than a backend-independent conformance suite.
Move pending-channel, confirmation-height, and waiting-close query tests from channeldb into chanstate. These tests still use the KV store fixture directly, so they live under the kv_ test prefix rather than a backend-independent suite.
Move RepairLinkNodes coverage out of channeldb and into channelcoord because the behavior coordinates channel-state and link-node persistence. Keep the test in a KV-prefixed file for now because it still builds the coordinator through the KV-backed channeldb fixture.
Move FetchChannel, FetchHistoricalChannel, and FetchClosedChannelForID coverage out of channeldb because these paths now belong to the KV channel-state store. The moved tests use chanstate fixtures directly and avoid depending on the ChannelStateDB compatibility facade.
Move channel collection filtering and FetchPermAndTempPeers coverage from channeldb into chanstate because these behaviors are owned by the KV channel-state store. Add small test fixture options so the moved tests can construct the required channel shapes without depending on ChannelStateDB.
Move RestoreChannelShells coverage from channeldb into channelcoord because the operation coordinates channel-state restoration with link-node creation. The moved test builds the KV coordinator fixture directly and checks the persisted link-node address by its normalized string form.
Move AbandonChannel coverage from channeldb into chanstate because the implementation now lives on the KV channel-state store. The moved test uses the chanstate fixture directly and keeps abandoned-channel behavior with the other close-channel tests.
Move FetchClosedChannels coverage that exercises MarkChanFullyClosed into channelcoord, since the test spans channel-state summaries and coordinator lifecycle mutation. The moved test uses the KV coordinator fixture directly and leaves pure closed-summary storage coverage in chanstate.
Move OpenChannel put, fetch, revocation update, and close coverage from channeldb into chanstate because these paths belong to the KV channel-state store. Reuse the existing chanstate test fixtures and add small HTLC option helpers rather than keeping the ChannelStateDB fixture around.
Move ChannelStateTransition coverage from channeldb into chanstate because the test exercises commitment, revocation-log, and forwarding-package persistence owned by the KV channel-state store. The moved test uses KVStore directly and replaces the ChannelStateDB revocation-log helper with the chanstate store method.
Rename the former channel_test.go file now that the channel-state tests live in their owner packages. Keep only shared channeldb test fixtures that are still used by unrelated package tests.
github-actions
Bot
added
the
severity-critical
🔴 PR Severity: CRITICAL
🔴 Critical (56 non-test files)
🟠 High (2 non-test files)
🟡 Medium (9 non-test files)
🟢 Low / excluded from counts (33 test files)
AnalysisDespite the title ("move channel tests to owner packages") suggesting a test-only reshuffle, this PR moves substantial production code, not just tests. It decomposes The diff also directly touches Bump criteria are independently satisfied as well (>20 non-test files changed - 67 here; >500 non-test lines changed; multiple distinct critical packages touched), though the base classification is already at the ceiling (CRITICAL). Given the scope — a structural refactor of core channel-state storage touching persistence, funding, contract resolution, and HTLC forwarding code paths simultaneously — this warrants careful review by an engineer deeply familiar with To override, add a |
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
1 participant
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.
This PR follows up on the channel store/channel coordinator decomposition by moving channel-related tests into the packages that now own the behavior.
The moved tests are grouped by owner:
chanstatenow owns KV channel-state store tests.channelcoordnow owns KV coordination tests that span channel state and link-node state.channeldbkeeps only the remaining shared fixtures used by unrelated package tests.This keeps the test layout aligned with the package boundaries before later work removes the remaining
ChannelStateDBcompatibility facade from theDBtype.Validation:
go test ./chanstate ./channelcoord ./channeldbmake lint