wallet: modernize architecture with interfaces, syncer, SQL store, and integration tests

[yyforyongyu]

Summary

• Refactors the wallet into modular interfaces (Controller, Signer, AddressManager, AccountManager, UtxoManager, TxCreator, TxPublisher, TxReader, TxWriter, PsbtManager) with comprehensive implementations and tests
• Introduces a new chain syncer with CFilter-based scanning, reorg handling, and targeted rescan support
• Adds a SQL database layer (wallet/internal/db) with PostgreSQL and SQLite backends, migrations, and integration tests
• Adds bwtest integration test harness with bitcoind/neutrino backends and itest test cases
• Adds btcunit package for type-safe fee rate and transaction size calculations
• Extends chain.Interface with batch RPCs (GetBlockHashes, GetCFilter) across btcd, bitcoind, and neutrino backends
• Includes extensive benchmarks, CI improvements, flaky test fixes, and ADR documentation

Test plan

• Verify unit tests pass: make unit
• Verify integration tests pass: make itest
• Verify linting passes: make lint
• Review SQL migrations for correctness
• Review new interface contracts and implementations

🤖 Generated with Claude Code

[Roasbeef]

@claude review this

[claude]

Claude finished @Roasbeef's task in 1m 23s —— View job

---

Review of PR #1181

• Read repository guidelines
• Gather context and read the changed files
• Analyze the diff against base branch
• Post review feedback

---

Change Summary

This PR contains a single commit (87609b1) that fixes a flaky TestPrunedBlockDispatcherQuerySameBlock test by changing how the stop() method handles leftover queriedPeer channel signals.

Before: The stop() method used a select/default pattern that would fail the test if any unconsumed queriedPeer signals remained:

select {
case <-h.queriedPeer:
    h.t.Fatal("did not consume all queriedPeer signals")
default:
}

After: It drains any remaining signals without failing:

for len(h.queriedPeer) > 0 {
    <-h.queriedPeer
}

---

Review

The fix is correct and well-reasoned. Here's why:

1. Root cause is sound. The queriedPeer channel is signaled from a goroutine inside OnGetData (pruned_block_dispatcher_test.go:234-236). The work manager may internally retry or redistribute work across peers, producing a non-deterministic number of query signals. The old strict assertion was inherently racy — a test could pass the assertPeerQueried() checks for the expected number of queries, but the work manager could fire additional queries before stop() runs.

2. Asymmetry with dialedPeer is justified. The dialedPeer check remains strict (fail if unconsumed signals exist) because dial operations are deterministic — each peer is dialed exactly once during start(), and assertPeerDialed() consumes them. In contrast, peer queries can be retried/redistributed by the work manager, making the count non-deterministic.

3. Tests retain meaningful assertions. The comment correctly notes that tests which care about specific query counts still call assertPeerQueried() explicitly (e.g., TestPrunedBlockDispatcherQuerySameBlock at line 519 asserts exactly one query). The stop() cleanup is purely a teardown guard, not a behavioral assertion.

4. The drain pattern is safe. Since stop() is called after h.dispatcher.Stop() (line 176), no new signals will be produced, so the for len(ch) > 0 loop will terminate. There's no race with producers here.

One minor note: The for len(h.queriedPeer) > 0 pattern has a subtle theoretical race — if a goroutine were still writing to the channel, len() could return 0 before the write completes, then the write happens after the loop exits. However, since h.dispatcher.Stop() is called first on line 176 and the OnGetData goroutines are spawned by peers associated with the dispatcher, the dispatcher's shutdown should ensure no more OnGetData callbacks fire. This is fine as-is.

Verdict: LGTM — this is a clean, minimal fix for a legitimate flaky test. The comment explaining the rationale is helpful.

---