Rollback for dumptxoutset without invalidating blocks

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.

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

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33444 (rpc: Fix dumptxoutset rollback with competing forks by enirox001)
#31560 (rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script by theStack)

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 force-pushed on Sep 24, 2025

fjahr commented at 9:22 pm on September 24, 2025: contributor

cc @Sjors since you were asking for this approach a few times :)

fjahr force-pushed on Sep 24, 2025

rpc: Don't invalidate blocks in dumptxoutset

Instead this new approach uses a temporary coins db to roll back the
UTXO set.

f046286c0c

test: Add dumptxoutset fork test 6d409d5970

fjahr force-pushed on Sep 24, 2025

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

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.”

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.”

in src/rpc/blockchain.cpp:3199 in 6d409d5970

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

in src/rpc/blockchain.cpp:3205 in 6d409d5970

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

in src/rpc/blockchain.cpp:3227 in 6d409d5970

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

in src/rpc/blockchain.cpp:3232 in 6d409d5970

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

in src/rpc/blockchain.cpp:3113 in 6d409d5970

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

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

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?

theStack commented at 4:57 pm on October 9, 2025: contributor

Concept ACK

Rollback for dumptxoutset without invalidating blocks #33477

Code Coverage & Benchmarks

Reviews

Conflicts