#34400 wallet: parallel fast rescan (approx 8x speed up with 8 threads)
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #34400 wallet: parallel fast rescan (approx 8x speed up with 8 threads) | Ready | 0 | Strong | 561 + 177 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Parallelizes BIP 157/158 compact block filter evaluation during wallet fast rescans using a bounded worker thread pool. Adds the -walletpar configuration option (defaulting to auto-detected CPU cores up to 8 threads) to control wallet concurrency. Block reading and wallet state modifications remain serialized on the main thread, while filter checks are batched across threads and keypool top-ups re-check in-flight negative verdicts against newly added descriptor scripts.
Problem: Fast wallet rescans check compact block filters sequentially on a single thread. When rescanning large block ranges or restoring wallets on multi-core systems, available CPU cores sit idle and rescans take minutes.
Evidence of importance:
- Author's mainnet benchmark using hyperfine across 200,000 blocks (heights 700,000 to 900,000): master completed in 272.2 seconds, 1 thread completed in 275.0 seconds, and 8 threads completed in 34.8 seconds (~7.8x speedup).
Scope: Touches wallet scanning pipelines extensively, introducing thread pool scheduling and handling incremental script set updates concurrently with filter evaluations.
Risk of a mistake: high (wallet). Flaws in filter evaluation, race conditions during keypool top-up script patching, or dropped blocks in the pipeline could cause wallets to miss incoming transactions and report incorrect balances.
Changed since the previous assessment: Added periodic progress logging and disk persistence at interval boundaries during parallel block checks, and updated functional and unit tests to explicitly test both serial and parallel paths.
Reviewability
Ready
Nothing known that would invalidate a review now.
Agreement
Strong
substantive support, no open objection (ismaelsadeeq, l0rinc, ismaelsadeeq, rkrux)
Thread notes: Initial assessment populated. All concerns regarding non-SSD performance, prerequisite refactoring, and thread pool sharing have been addressed and resolved.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| luke-jr (contributor) | objection, approach | parallelization could degrade rescan performance on non-SSD disks due to random I/O seeking | answered · v1, not yet re-checked | answered | yes | 2026-01-29: 'Have you benchmarked this on a non-SSD?' Status from: 2026-04-20: l0rinc: 'I have measured it on an Intel(R) Core(TM) i7-7700 CPU with 8 cores, 64Gi RAM and a HDD' |
| bvbfan (contributor) | objection, approach | scanning thread competing for mutexes and tasks could starve workers and slow down rescanning | fixed · fix pushed · v1, not yet re-checked | answered | yes | 2026-02-08: 'All workers already process submit task in its own WorkThread just randomly trying to acquire mutex from scanning thread is non sense to me.' Status from: 2026-04-14: Eunovo: 'I have changed the implementation so that the main thread only joins task processing when it needs to clear the work queue' |
| furszy (member) | objection, maintenance | mixing scanning restructuring with parallelization overcomplicates review and combines distinct responsibilities | fixed · fix pushed · v1, not yet re-checked | answered | yes | 2026-02-11: 'I think it would be nice to first improve the current scanning code structure, then land the parallelization feature.' Status from: 2026-02-26: Eunovo: 'I moved the test change into https://github.com/bitcoin/bitcoin/pull/34667 and the ScanForWalletTransactions refactor into https://github.com/bitcoin/bitcoin/pull/34681.' |
| rkrux (member) | objection, correctness | sharing a global threadpool across wallets could let a rescan controller thread process tasks from another wallet's RPC, distorting latency | fixed · fix pushed · v1, not yet re-checked | answered | yes | 2026-03-16: 'Can't it happen that this controller thread picks up the task of another RPC of another wallet, thereby distorting results (from a RPC latency point of view) of this one?' Status from: 2026-04-14: Eunovo: 'I have moved away from a shared ThreadPool to one that is initialized before scanning and discarded after.' |
Support:
- w0xlt (contributor) (Concept ACK, 2026-02-21): Concept ACK [not substantive]
- ismaelsadeeq (member) (Concept ACK, 2026-03-13): Speedup scales well up to 16 threads, going from ~283s down to ~63s — a 4.5x improvement.
- l0rinc (member) (2026-04-20): verified benchmarks on HDD setup demonstrating a 3x speedup
- ismaelsadeeq (member) (2026-07-22): re-ran benchmarks with updated changes showing 6x speedup relative to master
- rkrux (member) (Concept ACK, 2026-08-26): Strong Concept ACK ae888e0, forgot to share it in the prior review.
Participants:
- luke-jr (contributor, objection, 1 statements 2026-01-29 to 2026-01-29): asked whether parallelization harms non-SSD performance due to random seeking
- bvbfan (contributor, objection, 2 statements 2026-02-08 to 2026-02-09): concerned that the main thread processing tasks causes contention and starves workers
- furszy (member, objection, 1 statements 2026-02-11 to 2026-02-11): conceptually likes the change but requested separating scanning refactoring into prerequisite PRs
- w0xlt (contributor, support, 1 statements 2026-02-21 to 2026-02-21): Concept ACK
- ismaelsadeeq (member, support, 2 statements 2026-03-13 to 2026-07-22): Concept ACK with reproduction benchmarks demonstrating up to 6.5x speedup
- rkrux (member, objection, 7 statements 2026-03-16 to 2026-08-26): concerned about cross-wallet task contention with a shared threadpool; later Strong Concept ACK
- l0rinc (member, support, 1 statements 2026-04-20 to 2026-04-20): benchmarked on HDD demonstrating a 3x speedup
Files
225 lines under test/bench/ci.
- src/wallet/scan.cpp +482/-84
- src/wallet/test/rescan_tests.cpp +159/-47
- src/wallet/scan.h +63/-7
- test/functional/wallet_fast_rescan.py +18/-1
- src/wallet/wallet.cpp +8/-0
- src/wallet/wallet.h +6/-0
- src/dummywallet.cpp +1/-0
- src/wallet/init.cpp +1/-0
Card
The short text the ranking pass compares across PRs.
Parallelizes BIP 157/158 compact block filter evaluation during wallet fast rescans using a worker thread pool. Introduces the -walletpar configuration option to control wallet concurrency, defaulting to auto-detecting core count up to 8 threads. Fast wallet rescans currently evaluate block filters sequentially on a single core, leaving other cores idle when scanning long chain intervals. On mainnet benchmarks scanning 200,000 blocks, rescan duration dropped from 272 seconds to 34.8 seconds with 8 threads (an ~7.8x speedup). Block reading, matching, and keypool top-ups remain serialized on the main thread, while in-flight negative filter verdicts are re-checked against newly derived descriptor scripts.
Data
thread read through 2026-08-26 (28 statements), head 3089c6fc3f
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Wallet
Categories
Wallet
P2 · speedup (ranking pass: Moved down: a real user-visible speedup, but ranked below the correctness and privacy fixes above it.)
Parallelizes BIP 157/158 compact filter evaluations during wallet rescans, delivering a substantial and reviewer-verified 3x to 8x speedup on multi-core systems. The wallet priority rubric explicitly highlights user-visible reliability and slow rescans, and multiple reviewers confirmed benchmarks dropping rescan times from nearly 5 minutes down to under 1 minute.
Membership: Landed entirely in src/wallet/ with the Wallet area label, modifying wallet rescan logic and adding the -walletpar option.
Factors: security/stability 0, bug 0, performance 3, user value 2, leverage 0