rpc: Let dumptxoutset fail early on stale targets and concurrent runs #36266

pull fjahr wants to merge 2 commits into bitcoin:master from fjahr:2026-09-dump-fail-early changing 2 files +57 −12
  1. fjahr commented at 7:25 AM on September 16, 2026: contributor

    This improves handling of two edge cases where dumptxoutset currently only fails after doing unnecessary and possibly incorrect work:

    Stale rollback target (first commit): ParseHashOrHeight accepts any known block hash, and the rollback path never checks the target is on the active chain. The full UTXO set is copied and blocks are disconnected back from the tip only to fail when the target block height is reached. This is now rejected before all that as an invalid param, matching the check in getchaintxstats and scanblocks. This is a pretty unlikely scenario but the fix is also very small.

    Concurrent calls (second commit): Nothing prevented triggering two runs at once and since this is a relatively long running operation I don't think it is that unlikely that this might happen by accident or that users think this might actually be a possible thing to do if you want two different dumps. On master currently both runs would write to <path>.incomplete, so the second dump to the same path corrupts the file the first one reports as successfully written. And also the prune lock has a fixed name, so the second run overwrites the first's height and the first to finish deletes the others lock mid-rollback. A second call now fails immediately with "dumptxoutset is already running". TemporaryPruneLock is extended and renamed to DumptxoutsetGuard and handles both the general running of multiple instances and the lock management. The alternative to this approach would be to allow for concurrent runs and fix that behavior but it seemed to me like that would be overengineering considering that the audience for this functionality should be very small.

  2. rpc: Exit dumptxoutset early if target is stale 35a9b9193f
  3. rpc: Reject concurrent dumptxoutset calls 31000db0ea
  4. DrahtBot added the label RPC/REST/ZMQ on Sep 16, 2026
  5. DrahtBot commented at 7:25 AM on September 16, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    External sites

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • // The prune lock must be gone before the next run can register its own -> // The prune lock must be gone before the next run can register its own lock [the sentence is incomplete as written; the missing noun makes the comment unclear]

    <sup>2026-09-16 07:25:53</sup>

  6. cyb3ralbert commented at 7:35 AM on October 22, 2026: contributor

    dumptxoutset checks ActiveChain().Contains(*target_index) under cs_main at blockchain.cpp:3161, but CreateRolledBackUTXOSnapshot pins the tip under a separate lock at :3296.

    Is it intentional that if the target leaves the active chain between these two points, the operation can still reach CHECK_NONFATAL(rollback_cache.GetBestBlock() == target->GetBlockHash()) at :3365?

    I reproduced this on regtest by blocking the first rollback call on a FIFO, invalidating the target block, generating 8 new blocks, and then allowing the dump to proceed. Both master and the PR branch hit the check, although the node remains up.

    Would it make sense to revalidate the target against the pinned tip immediately after tip = chainstate.m_chain.Tip()?

    if (tip->GetAncestor(target->nHeight) != target) {
        throw JSONRPCError(RPC_INVALID_PARAMETER, "Block is not in main chain");
    }
    

    With this check, the same reproduction returns -8 Block is not in main chain, and the subsequent latest dump succeeds. The relevant tests (rpc_dumptxoutset.py and feature_assumeutxo.py) also pass.

    Tested on regtest, head 31000db0ea, Linux aarch64 (Debian 13), GCC 14.2.0, Release.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-30 09:40 UTC