Rollback for dumptxoutset without invalidating blocks #33477

pull fjahr wants to merge 5 commits into bitcoin:master from fjahr:202509-better-rollback changing 6 files +251 −111
  1. fjahr commented at 9:15 pm on September 24, 2025: contributor

    This is an alternative approach to implement dumptxoutset with rollback that was discussed a few times. It does not rely on invalidateblock and reconsiderblock and instead creates a temporary copy of the coins DB, modifies this copy by rolling back as many blocks as necessary and then creating the dump from this temp copy DB. See also #29553 (comment), #32817 (comment) and #29565 discussions.

    The nice side-effects of this are that forks can not interfere with the rollback and network activity does not have to be suspended. But there are also some downsides when comparing to the current approach: this does require some additional disk space for the copied coins DB and performance is slower (master took 3m 17s vs 9m 16s in my last test with the code here, rolling back ~1500 blocks). However, there is also not much code being added here, network can stay active throughout and performance would stay constant with this approach while it would impact master if there were forks that needed to be invalidated as well (see #33444 for the alternative approach), so this could still be considered a good trade-off.

  2. DrahtBot commented at 9:15 pm on September 24, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33477.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK luke-jr, kevkevinpal, theStack, mzumsande
    Stale ACK enirox001

    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:

    • #34654 ([RFC] BlockMap and CChain Concurrency Improvement by alexanderwiederin)
    • #34440 (Refactor CChain methods to use references, tests by optout21)

    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.

  3. fjahr force-pushed on Sep 24, 2025
  4. fjahr commented at 9:22 pm on September 24, 2025: contributor
    cc @Sjors since you were asking for this approach a few times :)
  5. fjahr force-pushed on Sep 24, 2025
  6. fjahr force-pushed on Sep 24, 2025
  7. luke-jr commented at 10:40 am on September 25, 2025: member

    Concept ACK, this seems cleaner.

    master took 3m 17s vs 9m 16s in my last test with the code here

    I suspect if you go back further, this approach will end up performing better because we no longer need to roll back forward at the end

  8. in src/rpc/blockchain.cpp:2995 in f046286c0c outdated
    2991@@ -3020,9 +2992,8 @@ static RPCHelpMan dumptxoutset()
    2992     return RPCHelpMan{
    2993         "dumptxoutset",
    2994         "Write the serialized UTXO set to a file. This can be used in loadtxoutset afterwards if this snapshot height is supported in the chainparams as well.\n\n"
    2995-        "Unless the \"latest\" type is requested, the node will roll back to the requested height and network activity will be suspended during this process. "
    2996-        "Because of this it is discouraged to interact with the node in any other way during the execution of this call to avoid inconsistent results and race conditions, particularly RPCs that interact with blockstorage.\n\n"
    2997-        "This call may take several minutes. Make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)",
    2998+        "This creates a temporary UTXO database when rolling back, keeping the main chain intact. Should the node experience an unclean shutdown the temporary database may need to be removed from the datadir manually.\n\n"
    


    kevkevinpal commented at 12:54 pm on September 27, 2025:
    It may be worth noting that “network activity will not be suspended during this process.”

    fjahr commented at 9:48 pm on November 26, 2025:
    I don’t think this is necessary, I don’t think a user would naturally assume that there is a reason to suspend network activity for this. Previously we had to do this as basically a hack. When we don’t do this anymore it would seem odd to me to mention it.
  9. in src/rpc/blockchain.cpp:2996 in f046286c0c outdated
    2991@@ -3020,9 +2992,8 @@ static RPCHelpMan dumptxoutset()
    2992     return RPCHelpMan{
    2993         "dumptxoutset",
    2994         "Write the serialized UTXO set to a file. This can be used in loadtxoutset afterwards if this snapshot height is supported in the chainparams as well.\n\n"
    2995-        "Unless the \"latest\" type is requested, the node will roll back to the requested height and network activity will be suspended during this process. "
    2996-        "Because of this it is discouraged to interact with the node in any other way during the execution of this call to avoid inconsistent results and race conditions, particularly RPCs that interact with blockstorage.\n\n"
    2997-        "This call may take several minutes. Make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)",
    2998+        "This creates a temporary UTXO database when rolling back, keeping the main chain intact. Should the node experience an unclean shutdown the temporary database may need to be removed from the datadir manually.\n\n"
    2999+        "This call may take several minutes for deep rollbacks. Make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)",
    


    kevkevinpal commented at 12:56 pm on September 27, 2025:

    Maybe this text would make more sense

    “For deep rollbacks, make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0) as it may take several minutes.”


    fjahr commented at 9:48 pm on November 26, 2025:
    Taken
  10. in src/rpc/blockchain.cpp:3199 in 6d409d5970 outdated
    3194+        node.rpc_interruption_point();
    3195+
    3196+        CBlock block;
    3197+        if (!node.chainman->m_blockman.ReadBlock(block, *block_index)) {
    3198+            throw JSONRPCError(RPC_INTERNAL_ERROR,
    3199+                strprintf("Failed to read block at height %d", block_index->nHeight));
    


    kevkevinpal commented at 1:46 pm on September 27, 2025:
    Might be able to add a functional test for this rpc error

    fjahr commented at 9:48 pm on November 26, 2025:

    Hm, this one and the other similar comments about test coverage are all cases that are pretty hard to hit because they should only be possible in a case of db corruption or a similarly unlikely event. Not saying that this wouldn’t be valuable coverage, but afaik we hardly ever go through the hassle to cover such cases and this RPC is far from being a critical path in the comparison to the rest of the code base. So, unless you have a specific suggestion for how the can be hit in practical way I would suggest we keep these for a follow-up. The current changes should be a robustness improvement by themselves.

    (Marking the other comments as resolved for now but feel free to correct me if you think different about one of them specifically)

  11. in src/rpc/blockchain.cpp:3205 in 6d409d5970 outdated
    3200+        }
    3201+
    3202+        WITH_LOCK(::cs_main, res = chainstate.DisconnectBlock(block, block_index, rollback_cache));
    3203+        if (res == DISCONNECT_FAILED) {
    3204+            throw JSONRPCError(RPC_INTERNAL_ERROR,
    3205+                strprintf("Failed to roll back block at height %d", block_index->nHeight));
    


    kevkevinpal commented at 1:47 pm on September 27, 2025:
    Might be able to add a functional test for this rpc error
  12. in src/rpc/blockchain.cpp:3227 in 6d409d5970 outdated
    3222+                                                          chainstate.m_blockman,
    3223+                                                          CoinStatsHashType::HASH_SERIALIZED,
    3224+                                                          node.rpc_interruption_point);
    3225+
    3226+    if (!maybe_stats) {
    3227+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to compute UTXO statistics");
    


    kevkevinpal commented at 1:47 pm on September 27, 2025:
    Might be able to add a functional test for this rpc error
  13. in src/rpc/blockchain.cpp:3232 in 6d409d5970 outdated
    3227+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to compute UTXO statistics");
    3228+    }
    3229+
    3230+    std::unique_ptr<CCoinsViewCursor> pcursor{temp_db->Cursor()};
    3231+    if (!pcursor) {
    3232+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to create UTXO cursor");
    


    kevkevinpal commented at 1:47 pm on September 27, 2025:
    Might be able to add a functional test for this rpc error
  14. in src/rpc/blockchain.cpp:3113 in 6d409d5970 outdated
    3108+    TemporaryUTXODatabase(const fs::path& path) : m_path(path) {
    3109+        fs::create_directories(m_path);
    3110+    }
    3111+    ~TemporaryUTXODatabase() {
    3112+        try {
    3113+            fs::remove_all(m_path);
    


    kevkevinpal commented at 2:01 pm on September 27, 2025:
    It might be useful to add a log here to inform the user the temp UTXO db was cleaned up since we mentioned in logs that we are creating a temp UTXO db

    fjahr commented at 9:47 pm on November 26, 2025:
    Hm, I don’t think we log anything on the creation of the DB so I think I would keep it the same on the destruction. It should only be a debug level log if we would add anything like that since it seems a bit low level for the general user.
  15. kevkevinpal commented at 2:04 pm on September 27, 2025: contributor

    Concept ACK 6d409d5

    This approach makes more sense. I reviewed the code a bit and made some comments, but nothing blocking

    I also added comments on possible functional tests for the new JSONRPCError but these can be done in a followup

  16. ajtowns commented at 3:27 am on September 29, 2025: contributor

    Nice!

    performance is slower (master took 3m 17s vs 9m 16s in my last test with the code here,

    That probably makes sense. It might be possible to do it faster and with less disk usage for relatively short rollbacks via a two step process:

    • create a read-only snapshot of the db
    • create an empty “coins-delta” db
    • iterate through the rev data to rollback, update the coins-delta db:
      • when you rollback past a coin’s creation:
        • if the coin was in the snapshot db, add “[coin] deleted”
        • otherwise, if it was in the coins-delta db, remove “[coin]”
        • (otherwise, there’s a bug)
      • when you rollback past a coin’s spend, add “[coin]”
    • when you’ve finished the rollback,
      • iterate through the snapshot coins, skipping any where there is a “[coin] deleted” entry, reporting them
      • iterate through the non-deleted coins-delta coins, reporting them
    • delete the coins-delta db, delete the snapshot

    Rather than being direct RPC functionality, maybe it would be better to have an RPC function to export a copy of the utxo set at the current height, and have a separate bitcoin-kernel binary that performs the rollback and utxoset stats calculation itself?

  17. theStack commented at 4:57 pm on October 9, 2025: contributor
    Concept ACK
  18. enirox001 commented at 6:38 pm on October 27, 2025: contributor

    ACK 6d409d5

    This is a good change. Using a temporary coins DB for the rollback is a much cleaner and safer solution than the invalidateblock method. It correctly solves fork-related bugs by isolating the process and avoids the need for network suspension, making it a superior approach to what I proposed in #33444.

    The code is well-contained, and the new TemporaryUTXODatabase class handles the DB lifecycle cleanly.

    I’ve pulled the branch, compiled, and the full functional test suite passes, including the rpc_dumptxoutset test

  19. DrahtBot requested review from kevkevinpal on Oct 27, 2025
  20. DrahtBot requested review from theStack on Oct 27, 2025
  21. DrahtBot requested review from luke-jr on Oct 27, 2025
  22. in src/rpc/blockchain.cpp:3159 in f046286c0c outdated
    3154+
    3155+        std::unique_ptr<CCoinsViewCursor> cursor;
    3156+        WITH_LOCK(::cs_main, cursor = chainstate.CoinsDB().Cursor());
    3157+
    3158+        size_t coins_count = 0;
    3159+        while (cursor->Valid()) {
    


    mzumsande commented at 8:14 pm on October 27, 2025:
    Don’t we need to hold cs_main throughout the copying phase? What if the utxo set changes while we are copying coins?

    fjahr commented at 9:47 pm on November 26, 2025:
    I don’t think so, my understanding is that the cursor/LevelDB iterator works on a snapshot of the DB itself which doesn’t get mutated. You can see the same pattern in WriteUTXOSnapshot where we are working with a cursor without holding cs_main as well.
  23. in src/rpc/blockchain.cpp:3217 in f046286c0c outdated
    3212+        }
    3213+
    3214+        block_index = block_index->pprev;
    3215+    }
    3216+
    3217+    rollback_cache.SetBestBlock(target->GetBlockHash());
    


    mzumsande commented at 8:31 pm on October 27, 2025:
    This seems unnecessary. As far as I can see the final DisconnectBlock call should have set the best block already, so that could we instead Assume here that rollback_cache.GetBestBlock() == target->GetBlockHash().

    fjahr commented at 9:47 pm on November 26, 2025:
    Changed to an Assume instead.
  24. mzumsande commented at 8:53 pm on October 27, 2025: contributor

    Concept ACK

    I suspect if you go back further, this approach will end up performing better because we no longer need to roll back forward at the end

    Would be interesting if someone could try out by going back 25k blocks or more, as is usually done for creating snapshots (I can’t right now).

  25. fjahr force-pushed on Nov 26, 2025
  26. fjahr commented at 9:47 pm on November 26, 2025: contributor

    Thanks for all the feedback so far and sorry for the slow response!

    Rather than being direct RPC functionality, maybe it would be better to have an RPC function to export a copy of the utxo set at the current height, and have a separate bitcoin-kernel binary that performs the rollback and utxoset stats calculation itself?

    Hm, feels like a bit overengineered for this functionality, considering the overhead for test coverage and build changes for this. Maybe I am overcomplicating it in my head, I have not done much with kernel yet. But if this is considerably more complex I would rather first go ahead with this and then I would rather keep it for consideration of a future change.

    Would be interesting if someone could try out by going back 25k blocks or more, as is usually done for creating snapshots (I can’t right now).

    I tried with 880,000, our v29 param, so 45,000 blocks rollback. It took just under 20min in total and it returned the correct hash.

    0$ build/bin/bitcoin-cli -rpcclienttimeout=0 -named dumptxoutset ~/Downloads/utxo880k.dat rollback=880000
    1{
    2  "coins_written": 184821030,
    3  "base_hash": "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880",
    4  "base_height": 880000,
    5  "path": "/Users/FJ/Downloads/utxo880k.dat",
    6  "txoutset_hash": "dbd190983eaf433ef7c15f78a278ae42c00ef52e0fd2a54953782175fbadcea9",
    7  "nchaintx": 1145604538
    8}
    

    It might be possible to do it faster and with less disk usage for relatively short rollbacks via a two step process:

    I actually had a similar idea early on but then stayed with the simpler approach. I will try this out with a POC and check the performance impact.

  27. fjahr force-pushed on Nov 26, 2025
  28. DrahtBot added the label CI failed on Nov 26, 2025
  29. DrahtBot commented at 9:52 pm on November 26, 2025: contributor

    🚧 At least one of the CI tasks failed. Task Windows-cross to x86_64: https://github.com/bitcoin/bitcoin/actions/runs/19718249847/job/56495362448 LLM reason (✨ experimental): Compile errors in rpc/blockchain.cpp due to RPCHelpMan constructor signature mismatch (brace-initializer/API change).

    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.

  30. fjahr force-pushed on Nov 26, 2025
  31. DrahtBot removed the label CI failed on Nov 26, 2025
  32. fjahr commented at 0:21 am on November 27, 2025: contributor

    It might be possible to do it faster and with less disk usage for relatively short rollbacks via a two step process:

    I actually had a similar idea early on but then stayed with the simpler approach. I will try this out with a POC and check the performance impact.

    I tried to a few different takes on the delta-based idea including some vibe coding tests. Here is the latest one, something is definitely still broken there but the dump it generates is correct. All tests I did had in common that the processing time actually took longer than the current approach, almost 28 min with the latest code version. I am now thinking that longer rollbacks may not be quicker, only shorter ones will be because the big copy overhead in the beginning is saved. But then we also don’t have that much to gain. Even if the performance can be improved, now that I have played around with it, it doesn’t seem worth the additional code complexity it introduces. It would need to be significantly faster to justify that and I am currently not seeing that.

  33. sedited commented at 4:15 pm on March 4, 2026: contributor

    Even if the performance can be improved, now that I have played around with it, it doesn’t seem worth the additional code complexity it introduces. It would need to be significantly faster to justify that and I am currently not seeing that.

    Are you saying this PR might not be worth it, or just the other approaches you tried out? I’m also curious if this runs counter to the purpose of #31560 , which would reduce the amount of interim data written to disk for the purpose of collecting the data into a db.

  34. fjahr commented at 11:14 am on March 6, 2026: contributor

    Are you saying this PR might not be worth it, or just the other approaches you tried out?

    Wording is a bit confusing, sorry. I meant the other approach that I tried out. That approach had a negative impact on performance but even if that could be improved I would still doubt it’s our best choice.

    I’m also curious if this runs counter to the purpose of #31560 , which would reduce the amount of interim data written to disk for the purpose of collecting the data into a db.

    I haven’t reviewed that one but from a quick glance I don’t see why these would conflict. This PR here deals with how the UTXO set data is collected and #31560 deals with writing that data somewhere else. What #31560 does is just overwriting the temppath variable for this special case, but the call to WriteUTXOSnapshot stays the same as in master and here.

    It also seems like #31560 is basically RFM. I would say this can move forward and I will figure the integration in the rebase :)

  35. ajtowns commented at 4:21 pm on March 7, 2026: contributor
    Oh, huh, I kind-of thought this was merged already. I guess ping for review after 31560 is merged and this is rebased on top?
  36. fjahr commented at 7:44 pm on March 7, 2026: contributor

    Oh, huh, I kind-of thought this was merged already. I guess ping for review after 31560 is merged and this is rebased on top?

    Sure, I’m just waiting for 31560 myself now :) FWIW, I’ve been using this branch for all my txoutset dump needs since I opened it and never saw any issues so it seems to work well (anecdotally).

  37. DrahtBot added the label Needs rebase on Mar 12, 2026
  38. fjahr force-pushed on Mar 12, 2026
  39. fjahr commented at 2:01 pm on March 12, 2026: contributor

    Rebased on top of the changes from #31560, the conflict was very minor and things seem to just work (TM). I even added a commit which extends the sqlite test for named pipe usage to do a rollback so both functionalities are tested together.

    I’m also curious if this runs counter to the purpose of #31560 , which would reduce the amount of interim data written to disk for the purpose of collecting the data into a db. @sedited I have thought about your comment again and I probably didn’t address it correctly. Your point was not about if things can be integrated mechanically but rather that this offsets the gain someone might aim for when using a pipe, right? Indeed the negative impact is that there is more disk space used temporarily while the temp utxo set exists. However this happens only in the rollback case, with the latest utxo set the gain is still there.

  40. DrahtBot removed the label Needs rebase on Mar 12, 2026
  41. in src/rpc/blockchain.cpp:3174 in fde5f96777
    3169+    TemporaryUTXODatabase temp_db_cleaner{temp_db_path};
    3170+
    3171+    // Create temporary database
    3172+    DBParams db_params{
    3173+        .path = temp_db_path,
    3174+        .cache_bytes = size_t(1) << 24,  // 16MB cache
    


    theStack commented at 6:07 pm on March 12, 2026:

    nit (using operator""_MiB)

    0        .cache_bytes = 16_MiB,
    

    fjahr commented at 11:49 am on March 13, 2026:
    Nice, done!
  42. in src/rpc/blockchain.cpp:3207 in fde5f96777 outdated
    3202+                temp_cache.AddCoin(key, std::move(coin), false);
    3203+                coins_count++;
    3204+
    3205+                // Log every 10M coins (optimized for mainnet)
    3206+                if (coins_count % 10'000'000 == 0) {
    3207+                    LogInfo("Copying UTXO set: %uM coins copied.", coins_count / 1'000'000);
    


    theStack commented at 6:17 pm on March 12, 2026:
    logging nit: would be nice to also show the number of total coins, to provide some information about the copying progress. I guess that’s not easily possible though as we would need to a call GetUTXOStats before (adding a significant delay and therefore not worth it)?

    fjahr commented at 11:49 am on March 13, 2026:

    Yeah, I don’t like it either but to my knowledge there isn’t an easy enough way that would be worth it, IMO, unless the user runs coinstatsindex. Doing the logging different with coinstatsindex is certainly doable but also a bit ugly so not sure if it’s worth it. Running the stats before getting started also doesn’t seem worth it to take the performance hit. One creative idea I had was to take the db size on disk and calculate an estimate of progress based on bytes processed. But that is probably even worse than the other approaches because it takes more code, we would need to deal with all the stuff leveldb does like at least trigger a compaction manually before getting the size on disk etc. And it would likely be inaccurate often so we might still get user complaints. One more dirty estimate idea: getting the latest assumeutxo coins count and then extrapolate some coins count based on the blocks delta. That would have worked pretty well until 2023 but now it would be off to an annoying degree as well: https://mainnet.observer/charts/utxoset-size/

    Happy to consider more ideas but so far I haven’t had one where I felt it was worth it for nicer logging.


    theStack commented at 7:05 pm on March 13, 2026:
    I agree that none of the mentioned ideas are worth it the additional code complexity / performance penalty just for nicer log messages (though it’s interesting to reason about them, even if they are dirty :-)). Forgot to mention that in my review yesterday, but for the rollback a progress could be added though, as in that case the necessary information is available, e.g. “Rolled back 500/52000 blocks” (maybe even show the percentage if we want to be extra fancy). But even that I would consider a non-blocking nit.

    fjahr commented at 10:49 pm on March 14, 2026:
    Good idea. Added the rollback progress, the extra fancy way :)
  43. theStack commented at 6:42 pm on March 12, 2026: contributor

    Tested manually on signet so far with an arbitrary snapshot height (deep rollback=50000) and it seems to work fine, both with a regular file and a named pipe. Out of curiosity I tried to place the temporary db in /tmp (tmpfs)

     0diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
     1index e2a15d52d8..8453f8fe0d 100644
     2--- a/src/rpc/blockchain.cpp
     3+++ b/src/rpc/blockchain.cpp
     4@@ -3121,7 +3121,7 @@ static RPCHelpMan dumptxoutset()
     5                                               target_index,
     6                                               std::move(afile),
     7                                               path,
     8-                                              temppath);
     9+                                              "/tmp/");
    10     }
    11
    12     if (!fs::is_fifo(path_info)) {
    

    with the hope that it could be a bit faster, and also experimented with higher .cache_bytes values for the temporary db (256_MiB, 1024_MiB), but none of these changes made a difference in performance. Will re-test on mainnet within the next days.

  44. fjahr force-pushed on Mar 13, 2026
  45. in src/rpc/blockchain.cpp:3174 in f9378c1862
    3169+    TemporaryUTXODatabase temp_db_cleaner{temp_db_path};
    3170+
    3171+    // Create temporary database
    3172+    DBParams db_params{
    3173+        .path = temp_db_path,
    3174+        .cache_bytes = 16_MiB,
    


    sedited commented at 9:14 pm on March 13, 2026:
    Does this make at a difference at all? We are just reading once, so I would not expect db-level caching to make a difference.

    fjahr commented at 10:49 pm on March 14, 2026:

    You’re right, technically we are reading twice but the cache doesn’t really help here. During the rollback phase each coin is looked up only once, so the cache has no impact. And at the end everything is read once for the final dump, there 16 MB cache is negligible.

    Good catch, setting it to 0.

  46. in src/rpc/blockchain.cpp:3175 in f9378c1862
    3170+
    3171+    // Create temporary database
    3172+    DBParams db_params{
    3173+        .path = temp_db_path,
    3174+        .cache_bytes = 16_MiB,
    3175+        .memory_only = false,
    


    sedited commented at 9:15 pm on March 13, 2026:
    Might it be interesting to make this a named argument? On systems with heaps of memory this might reduce the disk load a bit, and for me at least it seems to cut the dump time in half.

    fjahr commented at 10:50 pm on March 14, 2026:
    Sure, that seems a small enough change to be worth it. I have added is as a separate commit so it can be considered as an optional addition to the change here.
  47. in src/rpc/blockchain.cpp:3239 in f9378c1862 outdated
    3234+        if (!node.chainman->m_blockman.ReadBlock(block, *block_index)) {
    3235+            throw JSONRPCError(RPC_INTERNAL_ERROR,
    3236+                strprintf("Failed to read block at height %d", block_index->nHeight));
    3237+        }
    3238+
    3239+        WITH_LOCK(::cs_main, res = chainstate.DisconnectBlock(block, block_index, rollback_cache));
    


    sedited commented at 9:52 pm on March 13, 2026:
    Might there be a TOCTOU issue in case pruning happens in the meantime here?

    fjahr commented at 10:50 pm on March 14, 2026:
    Hm, yeah, I think it is. The simple way would have been to just throw an error in this case but it’s a bit annoying when this happened to the user because maybe it would have worked but now they have to resync because they lost the blocks. I rather implemented what I think is a nicer solution using a prune lock.
  48. rpc: Don't invalidate blocks in dumptxoutset
    Instead this new approach uses a temporary coins db to roll back the
    UTXO set.
    8a2d68dd22
  49. test: Add dumptxoutset fork test b865017145
  50. test: Extend named pipe sqlite tool test to use rollback e3b366bd6b
  51. fjahr force-pushed on Mar 14, 2026
  52. fjahr commented at 10:51 pm on March 14, 2026: contributor

    Addressed comments from @sedited and @theStack , thanks for the review!

    There are now two new commits and a bit of extra lines of code. As mentioned in some of the comments there would have probably been simpler solutions available for each of the addressed issues and I am open to consider rolling back (pun intended) what I have added in this push in favor of something simpler if that’s what reviewers prefer.

    EDIT: FWIW, the newly added DeletePruneLock would also be a part of #34534.

  53. rpc: Add in_memory option to dumptxoutset with rollback ae03a7ec8b
  54. rpc: Prevent pruning necessary blocks during dumptxoutset execution e3410e6856
  55. fjahr force-pushed on Mar 14, 2026
  56. DrahtBot added the label CI failed on Mar 14, 2026
  57. DrahtBot removed the label CI failed on Mar 14, 2026

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-03-15 09:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me