#34534 rpc: Manual prune lock management (Take 2)
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #34534 rpc: Manual prune lock management (Take 2) | Ready | 0 (+1) | Strong | 171 + 238 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
Adds `setprunelock` and `listprunelocks` RPCs alongside a `-prunelockheight` startup option to allow users and external applications to manually manage block prune locks. These in-memory locks prevent the block storage manager from pruning block files at or above a specified height, using a `user:` prefix to isolate them from internal subsystem locks.
Problem: On pruned nodes, blocks needed for rescanning can be pruned if a wallet is unloaded while the node continues syncing or processing blocks. Furthermore, external wallet software using Bitcoin Core has had no mechanism to hold blocks on disk during offline periods or temporary maintenance without disabling pruning entirely.
Evidence of importance:
- PR description: 'On pruned nodes, if a wallet is not loaded while the node is running, blocks necessary for rescanning when the wallet is loaded may be pruned. This PR allows users and external wallet software to manually manage prune locks via RPC, so they can protect blocks needed for future rescans and also clean up locks that are no longer relevant.'
- Revives closed PR #19463 with updated UX patterns matching `setban`/`listbanned`.
Risk of a mistake: medium (rpc_api). A flaw in prune lock logic could either prune block files required by wallets or indexes, causing rescan failures, or prevent block pruning indefinitely and exhaust disk space.
Changed since the previous assessment: Added functional test coverage verifying that an unloaded wallet remains loadable on a pruned node when protected by a manual prune lock.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- author: address andrewtoth's review comments and question on -prunelockheight=0 behavior 2026-11-01
Agreement
Strong
substantive support, no open objection (achow101, stickies-v)
Thread notes: andrewtoth reviewed and gave a Concept ACK while raising suggestions on RPC help, test helpers, and questioning why -prunelockheight=0 uses the chain tip instead of genesis.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| luke-jr (contributor) | objection, interface | returning an array instead of an object prevents future extensibility of the listprunelocks RPC response | fixed · fix pushed · author updated listprunelocks to return an object | changed | yes | 2026-02-10: 'This was an Object for future extensibility. No reason to lose that.' Status from: 2026-02-19: fjahr: 'fair enough, I have changed it to return an object.' |
| bvbfan (contributor) | suggestion, approach | copy prune locks instead of holding cs_main lock while iterating | fixed · fix pushed · author implemented copy | answered | no | 2026-02-10: 'You could get it as copy and don't keep lock while iterating, performance wise.' Status from: 2026-02-18: fjahr: 'Done' |
| sedited (member) | question, usefulness | asks who mentioned the specific users or use cases for the feature | answered · achow101 detailed the wallet rescan use cases | answered | by others | 2026-03-18: 'Do you remember at least who mentioned these users/use cases?' Status from: 2026-03-19: achow101: 'This is generally useful in the wallet and for any wallets that leverage Bitcoin Core.' |
| stickies-v (member) | suggestion, usefulness | questioned whether there is real demand or known users to justify adding the RPCs | accepted · achow101 explained wallet rescanning on pruned nodes and stickies-v Concept ACKed | answered | by others | 2026-03-19: 'This looks potentially interesting, but I don't think we should be implementing RPCs when we don't know anyone that would actually be using them?' Status from: 2026-03-20: stickies-v: 'Thanks for the explanation @achow101 . Concept ACK.' |
| stickies-v (member) | suggestion, maintenance | include the wallet rescan motivation in the PR description | fixed · author updated the PR description | answered | yes | 2026-03-20: 'I think this might be useful to include in PR description to give it more motivation?' Status from: 2026-03-20: fjahr: 'Done, replace that vague part of the description with a better motivation paragraph.' |
| sedited (member) | question, approach | asks how prune locks are useful to external applications if they are not persisted across restarts | accepted · fix pushed · author discussed and added the -prunelockheight startup option which reviewer agreed with and approved | answered | yes | 2026-03-21: 'One thing that is still not clear to me is how this is actually useful to external applications if the locks are not persisted.' Status from: 2026-04-23: sedited: 'ACK 9465be6fd01138246337ec91745d08a65f7bea66' |
| sedited (member) | objection, safety | iterating over prune locks container without holding lock or copying risks data race | fixed · fix pushed · author implemented explicit copy before lock scope ends; reviewer later approved | changed | yes | 2026-03-21: 'Don't we either have to hold this lock while iterating through the prune locks, or copy them? I think my preference would be to just take a copy here.' Status from: 2026-04-23: sedited: 'ACK 9465be6fd01138246337ec91745d08a65f7bea66' |
| sedited (member) | question, approach | asks why a helper getter is introduced rather than iterating directly | answered · author explained m_prune_locks is private | answered | yes | 2026-03-21: 'Is there a reason for introducing this helper? Existing validation code that iterates through the locks directly.' Status from: 2026-03-22: fjahr: 'm_prune_locks is private and the direct access to it in validation.cpp is only possible because ChainState is a friend c' |
| rkrux (member) | question, approach | asks if the startup option can cause -prune to not be respected | answered · author confirmed that is the intended behavior of prune locks and pointed to PR 21726 | answered | yes | 2026-05-25: 'Can't there be a case wherein this new argument causes the -prune=<n> option to not be respected?' Status from: 2026-05-25: fjahr: 'Yes, unless I am misunderstanding your question that is the whole idea behind prune locks' |
| rkrux (member) | suggestion, maintenance | add functional test for wallet exercising prune locks | fixed · fix pushed · author added a functional test covering wallet prune locks | answered | yes | 2026-06-12: 'Also, a functional test for wallet exercising prune locks that solves the above scenario would be nice.' Status from: 2026-07-15: fjahr: 'I added this in a separate commit. I folded it into the existing test file since generating all those blocks is pretty e' |
| rkrux (member) | question, approach | asks if INITIAL_BLOCKS in functional test can be halved | answered · author noted the diff fails locally and kept the minimum reliable value | answered | yes | 2026-06-12: 'Is this correct? The following diff passes where this value is halved along with the corresponding heights in the pruneblockchain RPCs.' Status from: 2026-07-15: fjahr: 'for me the proposed diff fails because the blocks are not enough to get actual pruning.' |
| rkrux (member) | suggestion, correctness | suggested improving test assertion since prune_height_locked is -1 | fixed · fix pushed · author changed test to check explicitly for -1 and >0 | answered | yes | 2026-06-12: 'prune_height_locked is -1 because the lock prevents the pruning from happening. I don't see much point in asserting that prune_height_unlocked is greater than prune_height_locked.' Status from: 2026-07-15: fjahr: 'Changed this to check for -1 and >0 more explicitly now.' |
| rkrux (member) | suggestion, maintenance | add release notes for the new RPCs and startup option | fixed · fix pushed · author added release notes | changed | yes | 2026-06-12: 'The changes in this file require a release note because of new RPCs being added. Along with the new startup option.' Status from: 2026-07-15: fjahr: 'Right, added the necessary release notes!' |
| rkrux (member) | question, interface | asks whether allowing prune locks on future block heights is intentional | answered · fix pushed · author explained it can be useful and added a documentation note | answered | yes | 2026-06-12: 'There isn't a check that forbids locking blocks that'd be generated in the future. Is it intentional?' Status from: 2026-07-15: fjahr: 'Right, that's a good observation, I didn't think about this explicitly so far but I don't see harm in allowing this and ' |
| rkrux (member) | suggestion, maintenance | explicitly mention in init.cpp help that -prunelockheight causes -prune to not be respected | fixed · fix pushed · author added the note to help text | answered | yes | 2026-06-12: 'Ok, can add a sentence here that this option will cause the -prune option to not be respected - just to be explicit.' Status from: 2026-07-15: fjahr: 'Good idea, done!' |
| sedited (member) | suggestion, maintenance | mention in release notes that the init lock can be viewed and removed by RPC | fixed · fix pushed · author updated release notes and adjusted prefix to user: to allow removing it via RPC | answered | yes | 2026-07-23: 'Maybe mention here that the lock may be viewed by `listprunelock`and removed by `setprunelock`.' Status from: 2026-07-28: fjahr: 'I changed this and renamed the prefix to `user:` since it's not strictly used by RPCs anymore.' |
| andrewtoth (member) | suggestion, usefulness | questioned whether manual prune locks are needed over using -prune=1 and pruneblockchain RPC | answered · author explained that managing manual pruning from an external wallet requires reimplementing automatic pruning logic | answered | yes | 2026-09-09: 'I'm not sure I find the motivation compelling. Would the same functionality be achieved via the `pruneblockchain` RPC and a `-prune=1` setting?' Status from: 2026-09-13: fjahr: 'Partly yes, but I think it would be much more complex and difficult to implement managing that behavior on the side of a' |
| shuv-amp (contributor) | objection, correctness | restarting with -prunelockheight can delete blocks that should be locked because startup prune runs in Step 10 before the lock is registered | fixed · fix pushed · author moved the lock addition before initial prune and added a test | changed | yes | 2026-10-30: 'restarting with `-prune=550 -prunelockheight=H` while block-file usage is above the target can delete the blocks the lock should keep, then sets the lock.' Status from: 2026-10-31: fjahr: 'Moved the lock add before the initial prune and added a minimal test that checks the debug log for it.' |
| andrewtoth (member) | suggestion, interface | strike the second note from the RPC help describing implementation details | open | answered | no | 2026-11-01: 'I think this second note can be struck. It describes an implementation detail that should have no effect on usage.' |
| andrewtoth (member) | suggestion, maintenance | use has_blockfile from the test framework instead of custom file check | open | answered | no | 2026-11-01: 'The test framework has `has_blockfile` which could be used instead of this.' |
| andrewtoth (member) | question, interface | asks what the rationale is for setting 0 to mean tip rather than genesis block in -prunelockheight | open | answered | no | 2026-11-01: 'Hmm I think setting an explicit 0 value should set to genesis block, same as the RPC. What is the rationale for making 0 use tip?' |
| andrewtoth (member) | suggestion, maintenance | mention in release notes that setting 0 means tip for -prunelockheight | open | answered | no | 2026-11-01: 'The release notes also don't mention the 0 case meaning tip here.' |
Support:
- sedited (member) (Concept ACK, 2026-02-08): Concept ACK [not substantive]
- achow101 (member) (2026-03-19): This is generally useful in the wallet and for any wallets that leverage Bitcoin Core. The fundamental issue is that if the wallet is not loaded while the node is running, blocks necessary for rescann
- stickies-v (member) (Concept ACK, 2026-03-20): Enabling wallets to build on top of a pruned node via RPC seems enough of a use case to move this forward.
- sedited (member) (ACK, 2026-04-23): ACK 9465be6fd01138246337ec91745d08a65f7bea66 [not substantive]
- sedited (member) (ACK, 2026-07-23): Re-ACK 9fa4953f01cab3ab8669b316224b4eeb8433a8d8 [not substantive]
- sedited (member) (ACK, 2026-10-07): Re-ACK 67960317cda01b102c5cbec7efb8ab56611b21a7 [not substantive]
- andrewtoth (member) (Concept ACK, 2026-11-01): Concept ACK [not substantive]
Participants:
- sedited (member, support, 11 statements 2026-02-08 to 2026-10-07): reviewed code, suggested copying prune locks for safety, approved multiple revisions
- bvbfan (contributor, neutral, 1 statements 2026-02-10 to 2026-02-10): suggested taking a copy of prune locks without holding lock while iterating
- luke-jr (contributor, objection, 3 statements 2026-02-10 to 2026-02-10): requested changes to retain returning an object rather than an array for RPC extensibility
- maflcko (member, neutral, 1 statements 2026-02-18 to 2026-02-18): illustrated potential future extensibility with an object response
- achow101 (member, support, 1 statements 2026-03-19 to 2026-03-19): explained motivation and use cases for wallet rescan on pruned nodes
- stickies-v (member, support, 2 statements 2026-03-19 to 2026-03-20): questioned initial demand, then gave Concept ACK after use cases were clarified
- rkrux (member, neutral, 9 statements 2026-05-25 to 2026-06-12): reviewed, suggested release notes, documentation, test refinements, and wallet functional test
- andrewtoth (member, support, 9 statements 2026-09-09 to 2026-11-01): Concept ACK; questioned use cases and handling of 0 value, suggested test and documentation improvements
- shuv-amp (contributor, objection, 1 statements 2026-10-30 to 2026-10-30): reported bug where startup prune deletes blocks before -prunelockheight is registered
Files
238 lines under test/bench/ci.
- test/functional/rpc_prunelock.py +235/-0
- src/rpc/blockchain.cpp +118/-0
- src/init.cpp +26/-0
- doc/release-notes-34534.md +17/-0
- src/node/blockstorage.h +7/-0
- src/node/blockstorage.cpp +2/-1
- src/test/fuzz/rpc.cpp +2/-0
- src/rpc/client.cpp +1/-0
- test/functional/test_runner.py +1/-0
Card
The short text the ranking pass compares across PRs.
This pull request adds the setprunelock and listprunelocks RPCs, as well as a -prunelockheight startup argument, allowing manual management of block pruning limits. It resolves the problem on pruned nodes where unloading a wallet or taking an external indexer offline allows historical blocks needed for future rescans to be pruned away. The change exposes the existing internal BlockManager prune lock system with a user prefix, holding locks in memory across the node's runtime without persistence across restarts.
Data
dossier JSON · extract JSON · model ledger, generated 2026-11-02T01:11, confidence high, input hash d1379455480de7a7
thread read through 2026-11-01 (64 statements), head d08600255d
labels: RPC/REST/ZMQ
thread read through 2026-11-01 (64 statements), head d08600255d
labels: RPC/REST/ZMQ
Categories
RPC / REST / ZMQ
P2 · new feature
This PR adds missing RPC methods that external wallets require to prevent needed blocks from being pruned during offline periods or rescans. Reviewers confirmed strong ecosystem value, noting it is 'generally useful in the wallet and for any wallets that leverage Bitcoin Core' (achow101) and enables 'wallets to build on top of a pruned node via RPC' (stickies-v).
Membership: Labeled RPC/REST/ZMQ and implements setprunelock and listprunelocks in src/rpc/blockchain.cpp.
Factors: security/stability 0, bug 0, performance 0, user value 2, leverage 1