Skip to content

wallet: serialize lifecycle transitions - #1328

Draft
yyforyongyu wants to merge 21 commits into
btcsuite:sql-walletfrom
yyforyongyu:task-wallet-lifecycle
Draft

wallet: serialize lifecycle transitions#1328
yyforyongyu wants to merge 21 commits into
btcsuite:sql-walletfrom
yyforyongyu:task-wallet-lifecycle

Conversation

@yyforyongyu

@yyforyongyu yyforyongyu commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Change Description

Implements roadmap Task 368

Depends on #1336.

Make Manager the sole public owner of each Wallet runtime lifecycle. Startup
is serialized with shutdown, concurrent callers share one result, and teardown
continues independently of the initiating caller. Once stopped, a runtime stays
terminal and a later Manager.Load publishes a fresh instance.

The legacy Loader-owned chain reconnect remains internal to Loader, so this
change does not restore public Wallet or Controller lifecycle methods.
Deterministic tests cover lifecycle state transitions, startup arbitration,
terminal reload, concurrent stop, and legacy chain reconnect.

Steps to Test

  1. Run: make unit-race pkg=wallet case=TestManagerWalletLifecycle
  2. Run: go test -race ./wallet -run '^(TestLoaderHandleChainClientDisconnect|TestManagerWalletLifecycle)$' -count=10
  3. Run: make unit pkg=wallet
  4. Run the controller integration case for each backend:
    • make itest chain=btcd db=kvdb icase=controller
    • make itest chain=btcd db=sqlite icase=controller
    • make itest chain=btcd db=postgres icase=controller
  5. Run: go test ./...
  6. Run: go vet ./...
  7. Run: make lint-check

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

Comment thread wallet/controller.go Outdated
// This is part of the Controller interface.
func (w *Wallet) Start(startCtx context.Context) error {
// 1. Attempt to transition from Stopped to Starting.
w.lifecycleMu.Lock()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This design is ugly and hard to follow - instead, we should take a step back and think about how each component handles its own internal state and boundary. For the wallet, there is no need to support restart, instead, its lifecycle should be managed by a higher component like the manager.

Hold the Manager lock from durable Create or the Load cache
decision through runtime assembly and cache installation.

Keep only fully assembled Wallet pointers in the cache.
Prove concurrent cold Loads install one exact Wallet pointer.
Require Create to exclude Load from durable-write completion through
runtime cache installation.
Prove failures release Manager admission for retry. Require
different wallet names to assemble sequentially through the same
Manager-wide lock.
Create durable state with a setup Manager, then contend cold Loads
through a fresh Manager. Require every backend to return one exact
runtime pointer.
Distinguish a newly created runtime from a terminally stopped one.

Allow shutdown admission from every non-terminal lifecycle state so the
Manager coordinator can settle each Wallet incarnation exactly once.
Attach terminal completion and lazy coordinator admission to each exact
Manager-published runtime.
Provide coordinator-only helpers that publish all runtime workers before
returning and join them before terminal Vault locking.
Settle stopped, canceled, and failed runtime startups through one
caller-independent terminal path.
Arbitrate accepted Start cancellation, Stop, and setup completion at one
coordinator-owned publication point.
Resolve lifecycle requests against the exact current Wallet pointer.
Join the recorded terminal result with caller-bounded waits.
Bind StartWallet and StopWallet to the exact Manager-published Wallet
pointer and admit one lazy coordinator per runtime.

Make Stop teardown caller-independent while preserving caller context as
only a bound on each wait.
Join terminal shutdown before deleting the exact stopped cache entry and
reuse the per-name publication gate for one fresh Wallet pointer.
Cover pre-accept cancellation, terminal Stop results, exact pointer
validation, and failed startup teardown through the Manager API.
Use setup, worker, and Vault barriers to prove Stop and cancellation
publish one Start result and every Stop joins the same teardown.
Verify that an admitted Stop or canceled Start caller prevents worker
publication at the final Manager-serialized lifecycle cutoff.
Race concurrent Load calls against blocked terminal teardown. Prove the
publication gate returns one fresh runtime without reviving the old
pointer.
Register each integration runtime with its owning Manager so startup and
teardown use StartWallet and StopWallet.
Drive maintained integration lifecycle calls through the exact owning
Manager.

Cover terminal pointers and same-Manager replacement through Load.
Keep compatibility workers private to Loader, including creation,
unload, RPC shutdown, and chain-client reconnect handoff.
Remove public lifecycle entry points from Wallet and Controller.
Route maintained modern callers through the exact owning Manager.

Keep startup setup and worker operations private to the lifecycle
coordinator.
Run worker joining and terminal Vault locking in one helper goroutine.
Report completion for the exact runtime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants