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
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ itest-db-race:
env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(ITEST_DB_RACE)

#? itest: Run integration tests
#? itest (vars): chain=btcd|bitcoind|neutrino backend=btcd|bitcoind|neutrino db=kvdb|sqlite|postgres
#? itest (vars): chain=btcd|bitcoind|neutrino backend=btcd|bitcoind|neutrino
#? itest (vars): db=sqlite|kvdb
#? itest (vars): icase=<regex> (filter itest cases)
#? itest (vars): timeout=<duration> verbose=1 nocache=1
#? itest (ex): make itest icase=manager
Expand All @@ -175,7 +176,7 @@ itest:
$(filter-out -test.run=%,$(TEST_FLAGS)) \
-args \
-chain="$(if $(backend),$(backend),$(if $(chain),$(chain),btcd))" \
-db="$(if $(db),$(db),kvdb)"
-db="$(if $(db),$(db),sqlite)"

# =========
# UTILITIES
Expand Down
14 changes: 12 additions & 2 deletions bwtest/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bwtest

import (
"context"
"errors"
"fmt"
"path/filepath"
"runtime/debug"
Expand Down Expand Up @@ -262,6 +263,8 @@ func (h *HarnessTest) Stop() {
func (h *HarnessTest) stopActiveWallets(ctx context.Context) error {
h.Helper()

var stopErrs []error

for _, w := range h.ActiveWallets() {
if w == nil {
// Keep cleanup robust against partially initialized test state. A
Expand All @@ -275,13 +278,20 @@ func (h *HarnessTest) stopActiveWallets(ctx context.Context) error {
// NOTE: We intentionally don't call the deprecated WaitForShutdown/
// ShuttingDown methods here, as modern wallets might not have the
// legacy fields initialized.
//
// Every wallet is stopped even after a failure: this is the
// sole teardown owner, so returning on the first error would
// leave every later-registered wallet running with its stores
// open, leaking them into the rest of the run.
err := w.Stop(ctx)
if err != nil {
return fmt.Errorf("stop wallet: %w", err)
stopErrs = append(
stopErrs, fmt.Errorf("stop wallet: %w", err),
)
}
}

return nil
return errors.Join(stopErrs...)
Comment thread
yyforyongyu marked this conversation as resolved.
}

// setUpChainClient creates and starts a chain client for the active harness
Expand Down
69 changes: 55 additions & 14 deletions bwtest/harness_wallet.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package bwtest

import (
"context"
"strings"
"time"

Expand All @@ -14,21 +13,52 @@ import (
)

const (
// defaultPubPass is the standard public passphrase used by test wallets.
// defaultPubPass is the standard public passphrase used by test
// wallets.
defaultPubPass = "public"

// defaultPrivPass is the standard private passphrase used by test wallets.
// defaultPrivPass is the standard private passphrase used by test
// wallets.
defaultPrivPass = "private"

// defaultWalletRecoveryWindow keeps enough look-ahead addresses for test
// cases that derive multiple addresses while scanning historical blocks.
// defaultWalletRecoveryWindow keeps enough look-ahead addresses for
// test cases that derive multiple addresses while scanning historical
// blocks.
defaultWalletRecoveryWindow = 20

// defaultWalletSyncRetryInterval controls how often wallet sync retries
// when the chain backend is temporarily unavailable during startup.
defaultWalletSyncRetryInterval = 500 * time.Millisecond
)

// DBBackend resolves the harness-configured wallet database backend, rejecting
// any db value the harness cannot wire. kvdb runs directly off the subtest DB
// path; sqlite derives its path from that same kvdb path via the wallet's
// config defaults. postgres needs a DSN the harness does not provision, so it
// is rejected here with a clear message rather than failing deep inside
// Manager.Create.
//
// Manager-focused tests use this to build a wallet config directly while still
// honoring the -db flag.
func (h *HarnessTest) DBBackend() wallet.DBBackend {
h.Helper()

backend := wallet.DBBackend(h.dbType)
switch backend {
// kvdb and sqlite are both wireable from the subtest DB path.
case wallet.DBBackendKVDB, wallet.DBBackendSQLite:

case wallet.DBBackendPostgres:
h.Fatalf("db=%s is not supported by the itest harness: no "+
"Postgres DSN is provisioned", h.dbType)

default:
h.Fatalf("unknown db backend %q: use kvdb or sqlite", h.dbType)
}

return backend
}

// CreateEmptyWallet creates, starts, and registers a new wallet instance.
//
// This is intended for non-manager integration tests that want a ready-to-use
Expand All @@ -38,10 +68,18 @@ func (h *HarnessTest) CreateEmptyWallet() *wallet.Wallet {

name := "itest-" + strings.ReplaceAll(h.Name(), "/", "_")

// Resolve the harness-configured backend.
backend := h.DBBackend()

cfg := wallet.Config{
// Use the subtest-scoped DB path and chain client prepared by
// the harness.
DB: wallet.DBConfig{
// Honor the harness-configured backend so db=kvdb
// parity runs exercise kvdb rather than silently
// defaulting to SQLite. When sqlite is selected the
// SQLite path is derived from the kvdb path below.
Backend: backend,
KVDB: wallet.KVDBConfig{
DBPath: h.WalletDBPath,
},
Expand All @@ -53,7 +91,8 @@ func (h *HarnessTest) CreateEmptyWallet() *wallet.Wallet {
RecoveryWindow: defaultWalletRecoveryWindow,
WalletSyncRetryInterval: defaultWalletSyncRetryInterval,

// Use a unique wallet name per test to avoid collisions in logs.
// Use a unique wallet name per test to avoid collisions in
// logs.
Name: name,
PubPassphrase: []byte(defaultPubPass),
}
Expand All @@ -64,8 +103,9 @@ func (h *HarnessTest) CreateEmptyWallet() *wallet.Wallet {
PubPassphrase: []byte(defaultPubPass),
PrivatePassphrase: []byte(defaultPrivPass),

// Use an old birthday to ensure the wallet can discover historical
// blocks when used in tests that pre-mine chain state.
// Use an old birthday to ensure the wallet can discover
// historical blocks when used in tests that pre-mine chain
// state.
Birthday: time.Now().Add(-1 * time.Hour),
}

Expand All @@ -76,13 +116,14 @@ func (h *HarnessTest) CreateEmptyWallet() *wallet.Wallet {
err = w.Start(h.Context())
require.NoError(h, err, "failed to start wallet")

h.Cleanup(func() {
// We use a background context here because the test context might be
// canceled by the time cleanup runs.
_ = w.Stop(context.Background())
})

// Register the wallet so harness helpers can assert global invariants.
//
// Registration also hands teardown to the harness: the subtest cleanup
// stops every registered wallet via stopActiveWallets and asserts the
// error. We deliberately don't register a second Stop cleanup here. A
// duplicate cleanup would run first (LIFO) and discard the owning
// Stop's error, and the harness's later Stop would return nil
// because Stop is idempotent, masking a real teardown failure.
h.RegisterWallet(w)

return w
Expand Down
19 changes: 13 additions & 6 deletions docs/developer/adr/0008-integration-test-framework.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ graph TD
* For `bitcoind` tests: A separate `bitcoind` process connects (P2P) to the Miner to sync.
* For `neutrino` tests: There is no separate "Chain Node" process; Neutrino connects directly to the Miner.
* **`ChainBackend`**: The interface wrapper (`rpcclient` or `neutrino.ChainService`) used by the Wallet to communicate with the Chain Node.
* **`Database Backend`**: The storage layer (`kvdb`, `postgres`, `sqlite`).
* **`Database Backend`**: The storage layer. The wallet supports `sqlite`
(the default), `kvdb`, and `postgres`; the itest harness currently wires
only `sqlite` and `kvdb`, and rejects `postgres` because it provisions no
DSN.
* **`HarnessTest`**: The specific test instance. It manages unique ports, temp directories, and process lifecycles.

### 2.3. Package Structure
Expand All @@ -82,14 +85,17 @@ graph TD
Configuration is handled via `go test` flags.

```bash
# Default (btcd + kvdb)
# Default (btcd + sqlite)
make itest

# Explicit configuration
make itest chain=bitcoind db=postgres
# Explicit configuration. The wallet defaults to sqlite; pass db=kvdb to
# run the legacy walletdb parity suite. postgres is not supported by the
# itest harness (no Postgres DSN is provisioned), so db=postgres is
# rejected.
make itest chain=bitcoind db=kvdb

# Run specific test case
make itest case=TestNewWallet
make itest icase=manager
```

* **Sequential Execution**: Tests run sequentially to avoid resource exhaustion and port conflicts, given the heavy overhead of spinning up multiple full nodes per test.
Expand All @@ -113,7 +119,8 @@ make itest case=TestNewWallet
### Pros
- **Consistency**: Clear separation between Network, Infrastructure, and Application.
- **Isolation**: Per-test harnesses prevent state leaks.
- **Coverage**: capable of validating the entire support matrix.
- **Coverage**: capable of validating the chain-backend matrix; the database
matrix covers `sqlite` and `kvdb` until a PostgreSQL DSN is provisioned.

### Cons
- **Resource Intensity**: Running a separate `bitcoind`/`btcd` process per test is CPU/RAM intensive.
Expand Down
16 changes: 9 additions & 7 deletions itest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ var (

// dbBackend defines the database backend to be used for the wallet
// storage.
// Options: "kvdb" (default), "sqlite", "postgres".
// Options: "sqlite" (default), "kvdb".
//
// This flag allows verifying that the wallet functions correctly across all
// supported database drivers.
// This flag allows verifying that the wallet functions correctly
// across the supported database drivers. It defaults to sqlite to
// match the wallet's default backend; pass -db=kvdb to run the legacy
// walletdb parity suite.
dbBackend = flag.String(
"db", "kvdb",
"database backend to use (kvdb, sqlite, postgres)",
"db", "sqlite",
"database backend to use (sqlite, kvdb)",
)

// shuffleSeedFlag is the source of randomness used to shuffle the test
Expand All @@ -46,8 +48,8 @@ var (

// init configures rpctest to allocate unique listener ports.
func init() {
// Use system-unique ports for rpctest harnesses so multiple local test runs
// don't collide.
// Use system-unique ports for rpctest harnesses so multiple local
// test runs don't collide.
rpctest.ListenAddressGenerator = func() (string, string) {
p2p := fmt.Sprintf(rpctest.ListenerFormat, port.NextAvailablePort())
rpc := fmt.Sprintf(rpctest.ListenerFormat, port.NextAvailablePort())
Expand Down
21 changes: 10 additions & 11 deletions itest/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package itest

import (
"context"
"time"

"github.com/btcsuite/btcwallet/bwtest"
Expand All @@ -15,10 +14,14 @@ import (
func testCreateWallet(h *bwtest.HarnessTest) {
h.Helper()

// Create a wallet using the Manager API.
// This is a manager-focused test, so drive the Manager API directly
// rather than the harness's CreateEmptyWallet convenience helper. Honor
// the harness-configured backend via h.DBBackend() so db=sqlite and
// db=kvdb genuinely exercise their respective backends instead of
// hard-coding one here.
cfg := wallet.Config{
DB: wallet.DBConfig{
Backend: wallet.DBBackendKVDB,
Backend: h.DBBackend(),
KVDB: wallet.KVDBConfig{
DBPath: h.WalletDBPath,
},
Expand All @@ -31,28 +34,24 @@ func testCreateWallet(h *bwtest.HarnessTest) {
PubPassphrase: []byte("public"),
}

manager := wallet.NewManager()
params := wallet.CreateWalletParams{
Mode: wallet.ModeGenSeed,
PubPassphrase: []byte("public"),
PrivatePassphrase: []byte("private"),
Birthday: time.Now().Add(-1 * time.Hour),
}

manager := wallet.NewManager()
w, err := manager.Create(cfg, params)
require.NoError(h, err, "failed to create wallet")

err = w.Start(h.Context())
require.NoError(h, err, "failed to start wallet")
h.Cleanup(func() {
// We use a background context here because h.Context() might be
// cancelled already.
require.NoError(
h, w.Stop(context.Background()), "failed to stop wallet",
)
})

// Register the wallet so harness helpers can assert global invariants.
// Registration also hands teardown to the harness, which stops every
// registered wallet during subtest cleanup, so we don't add a Stop
// cleanup here.
h.RegisterWallet(w)

// Mine a few blocks and require the wallet catches up.
Expand Down
9 changes: 4 additions & 5 deletions wallet/deprecated.go
Original file line number Diff line number Diff line change
Expand Up @@ -7272,11 +7272,10 @@ func OpenWithRetry(db walletdb.DB, pubPass []byte, cbs *waddrmgr.OpenCallbacks,
walletDeprecated: deprecated,

// This deprecated constructor wires the kvdb store directly, so
// pin the kvdb backend explicitly. An unset Backend already
// resolves to kvdb in this PR, but the backend-dependent legacy
// paths (e.g. storeSyncedTo, import mirroring) rely on an
// explicit kvdb backend, so set it rather than lean on the
// default.
// pin the kvdb backend explicitly. An unset Backend now
// defaults to sqlite, and the backend-dependent legacy paths
// (e.g. storeSyncedTo, import mirroring) rely on an explicit
// kvdb backend, so set it rather than lean on the default.
cfg: Config{
DB: DBConfig{Backend: DBBackendKVDB},
},
Expand Down
15 changes: 11 additions & 4 deletions wallet/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2181,10 +2181,14 @@ func TestManager_genRootKey(t *testing.T) {
t.Parallel()

m := NewManager()
cfg := Config{ChainParams: &chainParams}
cfg := Config{
ChainParams: &chainParams,
DB: DBConfig{Backend: DBBackendKVDB},
}

// With no configured kvdb path there is no legacy wallet to recover, so
// genRootKey takes the fresh-generation branch.
// With explicit kvdb and no configured path there is no legacy
// wallet to recover, so genRootKey takes the fresh-generation
// branch.
key, err := m.genRootKey(cfg, CreateWalletParams{Mode: ModeGenSeed})

// Verify we got a valid private extended key.
Expand All @@ -2200,7 +2204,10 @@ func TestManager_deriveRootKey(t *testing.T) {
t.Parallel()

m := NewManager()
cfg := Config{ChainParams: &chainParams}
cfg := Config{
ChainParams: &chainParams,
DB: DBConfig{Backend: DBBackendKVDB},
}

// 1. ModeShell: Should return nil/nil (no root key for shell).
t.Run("ModeShell", func(t *testing.T) {
Expand Down
14 changes: 7 additions & 7 deletions wallet/runtime_store_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ type PostgresDBConfig struct {
}

// DBConfig selects and configures the wallet runtime store backend. A
// zero-valued Backend selects DBBackendKVDB, so wallets default to the
// legacy walletdb store. Set Backend to DBBackendSQLite or
// DBBackendPostgres explicitly to use the SQL runtime store.
// zero-valued Backend selects DBBackendSQLite, so new library-created
// wallets default to the SQLite runtime store. Existing kvdb-only wallets
// must set Backend to DBBackendKVDB explicitly to keep using walletdb.
type DBConfig struct {
// Backend identifies the selected runtime store backend.
Backend DBBackend
Expand Down Expand Up @@ -157,15 +157,15 @@ const defaultSQLiteDBName = "wallet.sqlite"

// withDefaults returns c with the implicit runtime store defaults applied.
//
// An unset Backend selects DBBackendKVDB so wallets default to the legacy
// walletdb store; callers must set Backend to DBBackendSQLite or
// DBBackendPostgres explicitly to use the SQL runtime store. When the resolved
// An unset Backend selects DBBackendSQLite so new library-created wallets
// default to the SQLite runtime store; callers opening an existing kvdb-only
// wallet must set Backend to DBBackendKVDB explicitly. When the resolved
// backend is SQLite and no SQLite path was given, a deterministic default is
// derived next to the legacy kvdb database (DB.KVDB.DBPath) so the runtime
// store has a stable on-disk location.
func (c DBConfig) withDefaults() DBConfig {
Comment thread
yyforyongyu marked this conversation as resolved.
if c.Backend == "" {
Comment thread
yyforyongyu marked this conversation as resolved.
Comment thread
yyforyongyu marked this conversation as resolved.
c.Backend = DBBackendKVDB
c.Backend = DBBackendSQLite
Comment thread
yyforyongyu marked this conversation as resolved.
Comment thread
yyforyongyu marked this conversation as resolved.
Comment thread
yyforyongyu marked this conversation as resolved.
}

if c.Backend == DBBackendSQLite && c.SQLite.DBPath == "" &&
Expand Down
Loading
Loading