Skip to content

rpc: Manual prune lock management (Take 2)#34534

Open
fjahr wants to merge 3 commits intobitcoin:masterfrom
fjahr:2026-02-prunelocks-rpc-v2
Open

rpc: Manual prune lock management (Take 2)#34534
fjahr wants to merge 3 commits intobitcoin:masterfrom
fjahr:2026-02-prunelocks-rpc-v2

Conversation

@fjahr
Copy link
Contributor

@fjahr fjahr commented Feb 8, 2026

This is a refreshed version of #19463, which was closed due to inactivity.

Motivation: 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 (e.g. after permanently unloading a wallet from a node). This is useful both for Bitcoin Core's own wallet and for external wallets that use Bitcoin Core as a backend. Usage of temporary prune locking may also be helpful in other functional test scenarios but I didn't spend time to check where might apply.

This PR leverages the existing prune locks system we already have. The interface of the RPCs of the old PR has been slightly updated, it now matches the pattern used by setban/listbanned exactly. The locks are prefixed to distinguish them from the systems locks and prevent naming colissions. They are not persisted across restarts.

These are helpful for RPC methods to manage prune locks manually.
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 8, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK sedited, stickies-v

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #34049 (rpc: Disallow captures in RPCMethodImpl by ajtowns)
  • #33477 (Rollback for dumptxoutset without invalidating blocks by fjahr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fjahr fjahr mentioned this pull request Feb 8, 2026
@sedited
Copy link
Contributor

sedited commented Feb 8, 2026

Concept ACK

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 8, 2026

🚧 At least one of the CI tasks failed.
Task Windows native, fuzz, VS 2022: https://github.com/bitcoin/bitcoin/actions/runs/21799390662/job/62892243983
LLM reason (✨ experimental): Fuzz test failed because the RPC command "listprunelocks" is not registered as safe or unsafe for fuzzing in rpc.cpp.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@fjahr fjahr force-pushed the 2026-02-prunelocks-rpc-v2 branch from df7a8c4 to f192b06 Compare February 8, 2026 15:28
@fjahr
Copy link
Contributor Author

fjahr commented Feb 8, 2026

Fixing CI failure: Forgot to set RPC_COMMANDS_[NOT_]SAFE_FOR_FUZZING for the new RPCs.

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete review. Basically, the original PR interface was way better.

"These locks are used internally by indexes and can also be set manually via the setprunelock RPC.\n",
{},
RPCResult{
RPCResult::Type::ARR, "", "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an Object for future extensibility. No reason to lose that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RPC is called listprunelocks, I don't see how it could be extended to return anything else aside from a list of locks without that other thing feeling out of place. Can you give some specific ideas that you have? At least I would want to give the RPC a different name then (getpruneinfo or so) but potentially these ideas should rather be in a separate RPC. I prefer consistency and so I really like that we could be consistenty with the banning RPC.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listprunelocks could return a "blockman epoch" in the future, if there was need to further distinguish rpc results. (Yes, this is made up, but with an array, this is impossible, with an object, this is trivial and harmless)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "blockman epoch"? I don't think I have heard of that before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess TIL per this comment that the JSON object thing is in the dev notes, fair enough, I have changed it to return an object.

@fjahr fjahr force-pushed the 2026-02-prunelocks-rpc-v2 branch from f192b06 to 0032cee Compare February 18, 2026 13:54
@fjahr
Copy link
Contributor Author

fjahr commented Feb 18, 2026

@luke-jr

Basically, the original PR interface was way better.

Is this just about #34534 (comment) or did you mean something else?

@fjahr
Copy link
Contributor Author

fjahr commented Feb 18, 2026

Addressed feedback from @luke-jr and @bvbfan , thanks for the review!

fjahr and others added 2 commits February 19, 2026 16:37
These allow for manual management of prune locks by users and follow the UX pattern of setban/listbanned.

Co-authored-by: Luke Dashjr <[email protected]>
@sedited
Copy link
Contributor

sedited commented Mar 18, 2026

I can not recall who was mentioned as a potential user but I would appreciate if someone could ping them to take a look if this is still relevant to them.

Do you remember at least who mentioned these users/use cases?

@sedited sedited requested a review from stickies-v March 19, 2026 17:13
@stickies-v
Copy link
Contributor

Unfortunately, I can not recall who was mentioned as a potential user but I would appreciate if someone could ping them to take a look if this is still relevant to them.

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? Since the previous PR has been open since 2020 already without users chiming in, it seems like there's no real demand for this?

@achow101
Copy link
Member

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 rescanning when the wallet is loaded may be pruned away. The wallet definitely should be using prune locks, but it's also necessary to have a way to for users to remove prune locks that are no longer relevant, e.g. if they unload a wallet that they don't plan to load on that node again. External wallets that use Bitcoin Core should also find this useful as they can then set prune locks that match the scan state of the external wallet.

@stickies-v
Copy link
Contributor

Thanks for the explanation @achow101 . Concept ACK. Enabling wallets to build on top of a pruned node via RPC seems enough of a use case to move this forward. @fjahr I think this might be useful to include in PR description to give it more motivation?

@fjahr
Copy link
Contributor Author

fjahr commented Mar 20, 2026

Thanks for the explanation @achow101 . Concept ACK. Enabling wallets to build on top of a pruned node via RPC seems enough of a use case to move this forward. @fjahr I think this might be useful to include in PR description to give it more motivation?

Done, replace that vague part of the description with a better motivation paragraph.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants