Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions btcwallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,25 +228,10 @@ func rpcClientConnectLoop(legacyRPCServer *legacyrpc.Server, loader *wallet.Load

loadedWallet, ok := loader.LoadedWallet()
if ok {
// Do not attempt a reconnect when the wallet was
// explicitly stopped.
if loadedWallet.ShuttingDown() {
err := loader.HandleChainClientDisconnect(loadedWallet)
if err != nil {
return
}

loadedWallet.SetChainSynced(false)

// TODO: Rework the wallet so changing the RPC client
//nolint:staticcheck // This should be fixed once
// the interface refactor is finished, and new wallet
// RPC is built.
loadedWallet.StopDeprecated()
loadedWallet.WaitForShutdown()

//nolint:staticcheck // This should be fixed once
// the interface refactor is finished, and new wallet
// RPC is built.
loadedWallet.StartDeprecated()
}
}
}
Expand Down
96 changes: 81 additions & 15 deletions bwtest/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ type HarnessTest struct {
// subtests. This includes the wallet registry and idempotent shutdown.
mu sync.Mutex

// wallets is the set of wallets created by a test case.
wallets []*wallet.Wallet
// wallets is the set of Manager-owned runtimes created by a test case.
wallets []managedWallet

// stopped prevents stopping shared infrastructure more than once.
stopped bool
Expand All @@ -86,6 +86,12 @@ type HarnessTest struct {
cleaned bool
}

// managedWallet binds a registered runtime to its lifecycle-owning Manager.
type managedWallet struct {
manager *wallet.Manager
wallet *wallet.Wallet
}

// SetupHarness creates a new HarnessTest.
func SetupHarness(t *testing.T, chainBackendType, dbType string) *HarnessTest {
t.Helper()
Expand Down Expand Up @@ -330,19 +336,28 @@ func validateFileBackendArtifact(dbType, dbPath string) error {
return nil
}

// RegisterWallet registers a wallet with the harness.
// RegisterManagedWallet registers a Manager-owned wallet with the harness.
//
// Registered wallets are automatically included in harness-level assertions,
// such as MineBlocks.
func (h *HarnessTest) RegisterWallet(w *wallet.Wallet) {
func (h *HarnessTest) RegisterManagedWallet(manager *wallet.Manager,
w *wallet.Wallet) {

h.Helper()

if manager == nil {
h.Fatalf("cannot register wallet without manager")
}

if w == nil {
h.Fatalf("cannot register nil wallet")
}

h.mu.Lock()
h.wallets = append(h.wallets, w)
h.wallets = append(h.wallets, managedWallet{
manager: manager,
wallet: w,
})
h.mu.Unlock()
}

Expand All @@ -361,7 +376,7 @@ func (h *HarnessTest) DeregisterWallet(w *wallet.Wallet) bool {
defer h.mu.Unlock()

for i, registered := range h.wallets {
if registered != w {
if registered.wallet != w {
continue
}

Expand Down Expand Up @@ -402,7 +417,63 @@ func (h *HarnessTest) ActiveWallets() []*wallet.Wallet {
h.Helper()

h.mu.Lock()
wallets := append([]*wallet.Wallet(nil), h.wallets...)

wallets := make([]*wallet.Wallet, 0, len(h.wallets))
for _, managed := range h.wallets {
wallets = append(wallets, managed.wallet)
}

h.mu.Unlock()

return wallets
}

// StartWallet starts a registered runtime through its owning Manager.
func (h *HarnessTest) StartWallet(w *wallet.Wallet) error {
h.Helper()

manager, ok := h.walletManager(w)
if !ok {
return errors.New("wallet is not registered")
}

return manager.StartWallet(h.Context(), w)
}

// StopWallet stops a registered runtime through its owning Manager.
func (h *HarnessTest) StopWallet(w *wallet.Wallet) error {
h.Helper()

manager, ok := h.walletManager(w)
if !ok {
return errors.New("wallet is not registered")
}

return manager.StopWallet(h.Context(), w)
}

// walletManager returns the lifecycle owner registered for w.
func (h *HarnessTest) walletManager(w *wallet.Wallet) (*wallet.Manager, bool) {
h.Helper()

h.mu.Lock()
defer h.mu.Unlock()

for _, managed := range h.wallets {
if managed.wallet == w {
return managed.manager, true
}
}

return nil, false
}

// activeManagedWallets returns a snapshot of lifecycle registrations.
func (h *HarnessTest) activeManagedWallets() []managedWallet {
h.Helper()

h.mu.Lock()
wallets := append([]managedWallet(nil), h.wallets...)
h.mu.Unlock()

return wallets
Expand Down Expand Up @@ -495,20 +566,15 @@ func (h *HarnessTest) stopActiveWallets(ctx context.Context) error {

var stopErr error

for i, w := range h.ActiveWallets() {
if w == nil {
for i, managed := range h.activeManagedWallets() {
if managed.wallet == nil || managed.manager == nil {
// Keep cleanup robust against partially initialized test state. A
// caller could register a wallet reference and fail before the
// assignment completes.
continue
}

// The modern Wallet controller's Stop method is idempotent.
//
// NOTE: We intentionally don't call the deprecated WaitForShutdown/
// ShuttingDown methods here, as modern wallets might not have the
// legacy fields initialized.
err := w.Stop(ctx)
err := managed.manager.StopWallet(ctx, managed.wallet)
if err != nil {
stopErr = errors.Join(stopErr, fmt.Errorf(
"stop wallet %d: %w", i, err,
Expand Down
7 changes: 4 additions & 3 deletions bwtest/harness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ func TestDeregisterWalletRemovesRegisteredWallet(t *testing.T) {

// Arrange: a harness with three registered wallet identities.
h := &HarnessTest{T: t}
manager := &wallet.Manager{}
first := &wallet.Wallet{}
removed := &wallet.Wallet{}
last := &wallet.Wallet{}

h.RegisterWallet(first)
h.RegisterWallet(removed)
h.RegisterWallet(last)
h.RegisterManagedWallet(manager, first)
h.RegisterManagedWallet(manager, removed)
h.RegisterManagedWallet(manager, last)

// Act: the middle wallet is deregistered.
require.True(t, h.DeregisterWallet(removed))
Expand Down
4 changes: 2 additions & 2 deletions bwtest/harness_wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (h *HarnessTest) NewWallet(fixture WalletFixture) (*wallet.Wallet,
// cleanup owner. Registering after Start would leave a wallet whose Start
// failed unregistered, and a second direct Stop callback here would stop a
// successful one twice, out of order with the Manager close.
h.RegisterWallet(w)
h.RegisterManagedWallet(manager, w)

if fixture.Unstarted {
require.Empty(
Expand All @@ -125,7 +125,7 @@ func (h *HarnessTest) NewWallet(fixture WalletFixture) (*wallet.Wallet,
return w, WalletFunding{}
}

err = w.Start(h.Context())
err = manager.StartWallet(h.Context(), w)
require.NoError(h, err, "failed to start wallet")

if fixture.Unlocked {
Expand Down
16 changes: 12 additions & 4 deletions docs/developer/adr/0002-controller-syncer-architecture.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# ADR 0002: Controller-Syncer-State Architecture

## Status

- **Status:** Accepted
- **Date:** 2026-07-26

## Relationships

- **Amends:** None.
- **Supersedes:** None.
- **Amended by:** [ADR 0015](./0015-manager-owned-wallet-lifecycle.md).
- **Superseded by:** None.

## 1. Context

The legacy `btcwallet` architecture tightly coupled lifecycle management, synchronization logic, and state tracking within a single `Wallet` struct. This monolithic design led to several issues:
Expand Down Expand Up @@ -45,7 +57,3 @@ Instead of a single status enum, we track three separate dimensions:
### Cons
* **Complexity:** Increases the number of distinct types and files.
* **Indirection:** Calls to sync functionality now go through a channel-based request mechanism rather than direct method calls.

## 4. Status

Accepted and Implemented.
146 changes: 146 additions & 0 deletions docs/developer/adr/0015-manager-owned-wallet-lifecycle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# ADR 0015: Manager-Owned Wallet Lifecycle

## Status

- **Status:** Accepted
- **Date:** 2026-08-20

## Relationships

- **Amends:** [ADR 0002](./0002-controller-syncer-architecture.md).
- **Supersedes:** None.
- **Amended by:** None.
- **Superseded by:** None.

## 1. Problem

ADR 0002 assigned lifecycle ownership to the Wallet Controller. Manager also
owns the backend and publishes the runtime Wallet pointer, so lifecycle
ownership was split across two components. Concurrent Start and Stop calls
could disagree about whether setup had published workers, and restarting one
pointer required reusing cancellation and completion state across generations.

## 2. Context

Manager synchronously assembles and caches one exact Wallet pointer. Wallet
state remains atomically observable, but a lifecycle transition spans setup,
worker publication, worker joining, Vault locking, result publication, and
terminal completion. Those steps require one owner and one ordering boundary.

### Constraints

- Start and Stop results must correspond to an exact managed Wallet pointer.
- Blocking setup and worker joining must not run under Manager admission.
- Successful worker publication, `Started`, and the nil Start result form one
event that cannot be contradicted by cancellation or Stop.
- Stop teardown continues when an individual Stop caller cancels.
- Manager owns backend resources; stopping a runtime must not close them.
- Manager Close admission and Store closure remain separate decisions.

## 3. Decision

Manager is the sole public Wallet lifecycle owner. It exposes `StartWallet`
and `StopWallet`; Wallet, Controller, and deprecated compatibility interfaces
do not expose lifecycle aliases.

Each Manager cache entry binds one exact Wallet pointer to one lazily created
lifecycle coordinator and one terminal completion record. The coordinator
owns the one-shot state sequence `Created -> Starting -> Started -> Stopping ->
Stopped`. A stopped pointer is terminal. A later `Manager.Load` joins terminal
completion and publishes one fresh pointer for the same durable wallet.

Setup and worker joining run in bounded helper goroutines. The coordinator
serializes its final startup decision with Manager admission: an already
admitted Stop or observed caller cancellation prevents worker publication;
otherwise all workers, the `Started` transition, and the nil Start result
publish together. Stop joins setup or workers, locks the Vault, records the
teardown result, closes completion, and exits. A Stop caller's context bounds
only that caller's wait.

Wallet state remains the atomic gate for ordinary operations. The Controller
continues to own non-lifecycle Wallet operations and the Syncer continues to
own chain synchronization. This decision amends only ADR 0002's lifecycle
ownership and lifecycle state sequence.

### Legacy Loader compatibility boundary

The legacy `Loader` retains private, restartable compatibility goroutines for
daemon and setup paths that have not migrated to Manager. These goroutines use
the legacy quit channels and `started` flag; they are separate from Manager's
one-shot runtime state machine. Their `startDeprecated` and `stopDeprecated`
entry points are unexported, so Loader ownership does not create a second
public Wallet lifecycle surface.

Create and open start the compatibility goroutines before `onLoaded` publishes
the Wallet, ensuring Loader callbacks never observe an inactive legacy
runtime. On chain disconnect, Loader holds its own lock, verifies the exact
currently loaded pointer, rejects foreign or shutting-down wallets, marks the
wallet unsynced, and performs stop, join, and restart before a replacement
client attaches. Unload performs the final stop and join, then closes only a
database that the Loader itself opened. These steps keep legacy resource
ownership local without weakening Manager's terminal runtime contract.

## 4. Rationale

Manager already owns runtime identity and durable assembly. Binding lifecycle
to that same identity removes competing public owners and makes stale-pointer
rejection explicit. Terminal pointers avoid reusing cancellation functions,
completion latches, and WaitGroups across generations. One coordinator keeps
multi-step ordering local while bounded helpers prevent blocking work from
stalling its decisions.

## 5. Alternatives Considered

### Wallet-owned restartable lifecycle

A Wallet mutex or per-generation records could serialize Start and Stop, but
Manager would still own cache replacement and backend lifetime. The split
would preserve two authorities and require careful reuse of runtime fields.

### Public compatibility wrappers

Forwarding Wallet or Controller Start and Stop methods to Manager would need a
stable Manager back-reference and would retain a second apparent lifecycle
surface. The side branch permits the breaking API change, so wrappers add
ambiguity without a compatibility requirement.

### Manager Close owns runtime shutdown

Close could stop every runtime and order Store closure, but that also requires
admission fencing and resource ordering. Those concerns remain outside this
decision.

## 6. Consequences

### Positive

- Every lifecycle call names one exact Manager-owned runtime identity.
- Start publishes one result at one cutoff and Stop has one shared completion.
- A stopped Wallet pointer cannot accidentally restart or receive a request
after its coordinator exits.
- Runtime teardown does not close Manager-owned backend resources.

### Negative and Risks

- Callers must retain the owning Manager to start or stop a Wallet.
- Existing Wallet and Controller lifecycle callers require migration.
- Restart means loading a fresh runtime pointer and replacing stale references.
- Until Manager Close owns admission, maintained callers must stop every
current runtime before closing Manager resources.

## 7. Implementation Overview

Manager cache values become runtime entries containing an exact Wallet pointer,
lazy coordinator, admission flags, and terminal result. Wallet startup is split
into setup and all-worker publication; teardown joins workers and locks the
Vault. Load replaces a terminal entry under the existing Manager assembly
boundary. Unit and integration contracts cover kvdb, SQLite, and PostgreSQL,
publication arbitration, exact pointer identity, shared teardown, and terminal
replacement.

## 8. References

- [ADR 0002](./0002-controller-syncer-architecture.md)
- [Wallet synchronization architecture](../scanning_sync_architecture.md)
- Roadmap Task 368: make Manager own Wallet lifecycle
- Roadmap Task 453: serialize Manager Wallet assembly
3 changes: 3 additions & 0 deletions docs/developer/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,6 @@ relationship metadata, but do not rewrite its historical decision body.
- [ADR 0014: Durable SQL Database Identity](./0014-sql-database-identity.md) -
Defines the durable role-wallet identity and identity-first initialization
order for SQLite and PostgreSQL.
- [ADR 0015: Manager-Owned Wallet Lifecycle](./0015-manager-owned-wallet-lifecycle.md) -
Makes Manager the sole public lifecycle owner and makes stopped runtime
Wallet pointers terminal.
Loading
Loading