From 446ce51c21cd2466cb12fa0166fd069d42b603bf Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sat, 2 Mar 2024 16:37:00 +0100
Subject: [PATCH 1/8] RPC: Extract InvalidateBlock helper

---
 src/rpc/blockchain.cpp | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index b449444aff4..4da26e84bde 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1577,6 +1577,27 @@ static RPCHelpMan preciousblock()
     };
 }
 
+void InvalidateBlock(ChainstateManager& chainman, const uint256 block_hash) {
+    BlockValidationState state;
+    CBlockIndex* pblockindex;
+    {
+        LOCK(chainman.GetMutex());
+        pblockindex = chainman.m_blockman.LookupBlockIndex(block_hash);
+        if (!pblockindex) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
+        }
+    }
+    chainman.ActiveChainstate().InvalidateBlock(state, pblockindex);
+
+    if (state.IsValid()) {
+        chainman.ActiveChainstate().ActivateBestChain(state);
+    }
+
+    if (!state.IsValid()) {
+        throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
+    }
+}
+
 static RPCHelpMan invalidateblock()
 {
     return RPCHelpMan{"invalidateblock",
@@ -1591,27 +1612,10 @@ static RPCHelpMan invalidateblock()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    uint256 hash(ParseHashV(request.params[0], "blockhash"));
-    BlockValidationState state;
-
     ChainstateManager& chainman = EnsureAnyChainman(request.context);
-    CBlockIndex* pblockindex;
-    {
-        LOCK(cs_main);
-        pblockindex = chainman.m_blockman.LookupBlockIndex(hash);
-        if (!pblockindex) {
-            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
-        }
-    }
-    chainman.ActiveChainstate().InvalidateBlock(state, pblockindex);
+    uint256 hash(ParseHashV(request.params[0], "blockhash"));
 
-    if (state.IsValid()) {
-        chainman.ActiveChainstate().ActivateBestChain(state);
-    }
-
-    if (!state.IsValid()) {
-        throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
-    }
+    InvalidateBlock(chainman, hash);
 
     return UniValue::VNULL;
 },

From fccf4f91d21c351d742943d35476f53d40963b8b Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sat, 2 Mar 2024 16:42:27 +0100
Subject: [PATCH 2/8] RPC: Extract ReconsiderBlock helper

---
 src/rpc/blockchain.cpp | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 4da26e84bde..3f6def27d6d 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1622,6 +1622,25 @@ static RPCHelpMan invalidateblock()
     };
 }
 
+void ReconsiderBlock(ChainstateManager& chainman, uint256 block_hash) {
+    {
+        LOCK(chainman.GetMutex());
+        CBlockIndex* pblockindex = chainman.m_blockman.LookupBlockIndex(block_hash);
+        if (!pblockindex) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
+        }
+
+        chainman.ActiveChainstate().ResetBlockFailureFlags(pblockindex);
+    }
+
+    BlockValidationState state;
+    chainman.ActiveChainstate().ActivateBestChain(state);
+
+    if (!state.IsValid()) {
+        throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
+    }
+}
+
 static RPCHelpMan reconsiderblock()
 {
     return RPCHelpMan{"reconsiderblock",
@@ -1640,22 +1659,7 @@ static RPCHelpMan reconsiderblock()
     ChainstateManager& chainman = EnsureAnyChainman(request.context);
     uint256 hash(ParseHashV(request.params[0], "blockhash"));
 
-    {
-        LOCK(cs_main);
-        CBlockIndex* pblockindex = chainman.m_blockman.LookupBlockIndex(hash);
-        if (!pblockindex) {
-            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
-        }
-
-        chainman.ActiveChainstate().ResetBlockFailureFlags(pblockindex);
-    }
-
-    BlockValidationState state;
-    chainman.ActiveChainstate().ActivateBestChain(state);
-
-    if (!state.IsValid()) {
-        throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
-    }
+    ReconsiderBlock(chainman, hash);
 
     return UniValue::VNULL;
 },

From 993cafe7e45ab0af1e862c7def3de688f47c0443 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sat, 2 Mar 2024 16:43:58 +0100
Subject: [PATCH 3/8] RPC: Add type parameter to dumptxoutset

---
 src/rpc/blockchain.cpp                | 98 ++++++++++++++++++++++++++-
 src/rpc/client.cpp                    |  2 +
 test/functional/feature_assumeutxo.py |  2 +-
 test/functional/rpc_dumptxoutset.py   |  6 +-
 test/functional/wallet_assumeutxo.py  |  2 +-
 5 files changed, 103 insertions(+), 7 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 3f6def27d6d..4dc7dbfa939 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2648,6 +2648,25 @@ static RPCHelpMan getblockfilter()
     };
 }
 
+/**
+ * RAII class that disables the network in its constructor and enables it in its
+ * destructor.
+ */
+class NetworkDisable
+{
+    CConnman& m_connman;
+public:
+    NetworkDisable(CConnman& connman) : m_connman(connman) {
+        m_connman.SetNetworkActive(false);
+        if (m_connman.GetNetworkActive()) {
+            throw JSONRPCError(RPC_MISC_ERROR, "Network activity could not be suspended.");
+        }
+    };
+    ~NetworkDisable() {
+        m_connman.SetNetworkActive(true);
+    };
+};
+
 /**
  * Serialize the UTXO set to a file for loading elsewhere.
  *
@@ -2660,6 +2679,14 @@ static RPCHelpMan dumptxoutset()
         "Write the serialized UTXO set to a file.",
         {
             {"path", RPCArg::Type::STR, RPCArg::Optional::NO, "Path to the output file. If relative, will be prefixed by datadir."},
+            {"type", RPCArg::Type::STR, RPCArg::Default(""), "The type of snapshot to create. Can be \"latest\" to create a snapshot of the current UTXO set or \"rollback\" to temporarily roll back the state of the node to a historical block before creating the snapshot of a historical UTXO set. This parameter can be omitted if a separate \"rollback\" named parameter is specified indicating the height or hash of a specific historical block. If \"rollback\" is specified and separate \"rollback\" named parameter is not specified, this will roll back to the latest valid snapshot block that currently be loaded with loadtxoutset."},
+            {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "",
+                {
+                    {"rollback", RPCArg::Type::NUM, RPCArg::Optional::OMITTED,
+                        "Height or hash of the block to roll back to before creating the snapshot. Note: The further this number is from the tip, the longer this process will take. Consider setting a higher -rpcclienttimeout value in this case.",
+                    RPCArgOptions{.skip_type_check = true, .type_str = {"", "string or numeric"}}},
+                },
+            },
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",
@@ -2673,10 +2700,33 @@ static RPCHelpMan dumptxoutset()
                 }
         },
         RPCExamples{
-            HelpExampleCli("dumptxoutset", "utxo.dat")
+            HelpExampleCli("-rpcclienttimeout=0 dumptxoutset", "utxo.dat latest") +
+            HelpExampleCli("-rpcclienttimeout=0 dumptxoutset", "utxo.dat rollback") +
+            HelpExampleCli("-rpcclienttimeout=0 -named dumptxoutset", R"(utxo.dat rollback=853456)")
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
+    NodeContext& node = EnsureAnyNodeContext(request.context);
+    const CBlockIndex* tip{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
+    const CBlockIndex* target_index{nullptr};
+    const std::string snapshot_type{self.Arg<std::string>("type")};
+    const UniValue options{request.params[2].isNull() ? UniValue::VOBJ : request.params[2]};
+    if (options.exists("rollback")) {
+        if (!snapshot_type.empty() && snapshot_type != "rollback") {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid snapshot type \"%s\" specified with rollback option", snapshot_type));
+        }
+        target_index = ParseHashOrHeight(options["rollback"], *node.chainman);
+    } else if (snapshot_type == "rollback") {
+        auto snapshot_heights = node.chainman->GetParams().GetAvailableSnapshotHeights();
+        CHECK_NONFATAL(snapshot_heights.size() > 0);
+        auto max_height = std::max_element(snapshot_heights.begin(), snapshot_heights.end());
+        target_index = ParseHashOrHeight(*max_height, *node.chainman);
+    } else if (snapshot_type == "latest") {
+        target_index = tip;
+    } else {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid snapshot type \"%s\" specified. Please specify \"rollback\" or \"latest\"", snapshot_type));
+    }
+
     const ArgsManager& args{EnsureAnyArgsman(request.context)};
     const fs::path path = fsbridge::AbsPathJoin(args.GetDataDirNet(), fs::u8path(request.params[0].get_str()));
     // Write to a temporary path and then move into `path` on completion
@@ -2698,11 +2748,55 @@ static RPCHelpMan dumptxoutset()
             "Couldn't open file " + temppath.utf8string() + " for writing.");
     }
 
-    NodeContext& node = EnsureAnyNodeContext(request.context);
+    CConnman& connman = EnsureConnman(node);
+    const CBlockIndex* invalidate_index{nullptr};
+    std::unique_ptr<NetworkDisable> disable_network;
+
+    // If the user wants to dump the txoutset of the current tip, we don't have
+    // to roll back at all
+    if (target_index != tip) {
+        // If the node is running in pruned mode we ensure all necessary block
+        // data is available before starting to roll back.
+        if (node.chainman->m_blockman.IsPruneMode()) {
+            LOCK(node.chainman->GetMutex());
+            const CBlockIndex* current_tip{node.chainman->ActiveChain().Tip()};
+            const CBlockIndex* first_block{node.chainman->m_blockman.GetFirstBlock(*current_tip, /*status_mask=*/BLOCK_HAVE_MASK)};
+            if (first_block->nHeight > target_index->nHeight) {
+                throw JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height since necessary block data is already pruned.");
+            }
+        }
+
+        // Suspend network activity for the duration of the process when we are
+        // rolling back the chain to get a utxo set from a past height. We do
+        // this so we don't punish peers that send us that send us data that
+        // seems wrong in this temporary state. For example a normal new block
+        // would be classified as a block connecting an invalid block.
+        disable_network = std::make_unique<NetworkDisable>(connman);
+
+        invalidate_index = WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Next(target_index));
+        InvalidateBlock(*node.chainman, invalidate_index->GetBlockHash());
+        const CBlockIndex* new_tip_index{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
+
+        // In case there is any issue with a block being read from disk we need
+        // to stop here, otherwise the dump could still be created for the wrong
+        // height.
+        // The new tip could also not be the target block if we have a stale
+        // sister block of invalidate_index. This block (or a descendant) would
+        // be activated as the new tip and we would not get to new_tip_index.
+        if (new_tip_index != target_index) {
+            ReconsiderBlock(*node.chainman, invalidate_index->GetBlockHash());
+            throw JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height, reverting to tip.");
+        }
+    }
+
     UniValue result = CreateUTXOSnapshot(
         node, node.chainman->ActiveChainstate(), afile, path, temppath);
     fs::rename(temppath, path);
 
+    if (invalidate_index) {
+        ReconsiderBlock(*node.chainman, invalidate_index->GetBlockHash());
+    }
+
     result.pushKV("path", path.utf8string());
     return result;
 },
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index b866fa484bf..0112a261ce7 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -187,6 +187,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "gettxoutproof", 0, "txids" },
     { "gettxoutsetinfo", 1, "hash_or_height" },
     { "gettxoutsetinfo", 2, "use_index"},
+    { "dumptxoutset", 2, "options" },
+    { "dumptxoutset", 2, "rollback" },
     { "lockunspent", 0, "unlock" },
     { "lockunspent", 1, "transactions" },
     { "lockunspent", 2, "persistent" },
diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index a212704311d..7f4cfb9eab5 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -295,7 +295,7 @@ class AssumeutxoTest(BitcoinTestFramework):
         assert_equal(n1.getblockcount(), START_HEIGHT)
 
         self.log.info(f"Creating a UTXO snapshot at height {SNAPSHOT_BASE_HEIGHT}")
-        dump_output = n0.dumptxoutset('utxos.dat')
+        dump_output = n0.dumptxoutset('utxos.dat', "latest")
 
         self.log.info("Test loading snapshot when the node tip is on the same block as the snapshot")
         assert_equal(n0.getblockcount(), SNAPSHOT_BASE_HEIGHT)
diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py
index aa12da6ceb2..6c98b4ee661 100755
--- a/test/functional/rpc_dumptxoutset.py
+++ b/test/functional/rpc_dumptxoutset.py
@@ -27,7 +27,7 @@ class DumptxoutsetTest(BitcoinTestFramework):
         self.generate(node, COINBASE_MATURITY)
 
         FILENAME = 'txoutset.dat'
-        out = node.dumptxoutset(FILENAME)
+        out = node.dumptxoutset(FILENAME, "latest")
         expected_path = node.datadir_path / self.chain / FILENAME
 
         assert expected_path.is_file()
@@ -51,10 +51,10 @@ class DumptxoutsetTest(BitcoinTestFramework):
 
         # Specifying a path to an existing or invalid file will fail.
         assert_raises_rpc_error(
-            -8, '{} already exists'.format(FILENAME),  node.dumptxoutset, FILENAME)
+            -8, '{} already exists'.format(FILENAME),  node.dumptxoutset, FILENAME, "latest")
         invalid_path = node.datadir_path / "invalid" / "path"
         assert_raises_rpc_error(
-            -8, "Couldn't open file {}.incomplete for writing".format(invalid_path), node.dumptxoutset, invalid_path)
+            -8, "Couldn't open file {}.incomplete for writing".format(invalid_path), node.dumptxoutset, invalid_path, "latest")
 
 
 if __name__ == '__main__':
diff --git a/test/functional/wallet_assumeutxo.py b/test/functional/wallet_assumeutxo.py
index 0bce2f137c5..a5025a64e7d 100755
--- a/test/functional/wallet_assumeutxo.py
+++ b/test/functional/wallet_assumeutxo.py
@@ -93,7 +93,7 @@ class AssumeutxoTest(BitcoinTestFramework):
 
         self.log.info(
             f"Creating a UTXO snapshot at height {SNAPSHOT_BASE_HEIGHT}")
-        dump_output = n0.dumptxoutset('utxos.dat')
+        dump_output = n0.dumptxoutset('utxos.dat', "latest")
 
         assert_equal(
             dump_output['txoutset_hash'],

From 842685035244e151f4a10019af2dfe0563f11a82 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sat, 2 Mar 2024 18:26:27 +0100
Subject: [PATCH 4/8] test: Test for dumptxoutset at specific height

---
 test/functional/feature_assumeutxo.py | 46 ++++++++++++++++++++++++---
 test/functional/rpc_dumptxoutset.py   |  4 +++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index 7f4cfb9eab5..5f28d07f55d 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -22,6 +22,7 @@ from test_framework.util import (
     assert_approx,
     assert_equal,
     assert_raises_rpc_error,
+    sha256sum_file,
 )
 from test_framework.wallet import (
     getnewdestination,
@@ -320,12 +321,16 @@ class AssumeutxoTest(BitcoinTestFramework):
         for n in self.nodes:
             assert_equal(n.getblockchaininfo()["headers"], SNAPSHOT_BASE_HEIGHT)
 
-        assert_equal(
-            dump_output['txoutset_hash'],
-            "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")
-        assert_equal(dump_output["nchaintx"], blocks[SNAPSHOT_BASE_HEIGHT].chain_tx)
         assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT)
 
+        def check_dump_output(output):
+            assert_equal(
+                output['txoutset_hash'],
+                "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")
+            assert_equal(output["nchaintx"], blocks[SNAPSHOT_BASE_HEIGHT].chain_tx)
+
+        check_dump_output(dump_output)
+
         # Mine more blocks on top of the snapshot that n1 hasn't yet seen. This
         # will allow us to test n1's sync-to-tip on top of a snapshot.
         self.generate(n0, nblocks=100, sync_fun=self.no_op)
@@ -335,6 +340,39 @@ class AssumeutxoTest(BitcoinTestFramework):
 
         assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT)
 
+        self.log.info(f"Check that dumptxoutset works for past block heights")
+        # rollback defaults to the snapshot base height
+        dump_output2 = n0.dumptxoutset('utxos2.dat', "rollback")
+        check_dump_output(dump_output2)
+        assert_equal(sha256sum_file(dump_output['path']), sha256sum_file(dump_output2['path']))
+
+        # Rollback with specific height
+        dump_output3 = n0.dumptxoutset('utxos3.dat', rollback=SNAPSHOT_BASE_HEIGHT)
+        check_dump_output(dump_output3)
+        assert_equal(sha256sum_file(dump_output['path']), sha256sum_file(dump_output3['path']))
+
+        # Specified height that is not a snapshot height
+        prev_snap_height = SNAPSHOT_BASE_HEIGHT - 1
+        dump_output4 = n0.dumptxoutset(path='utxos4.dat', rollback=prev_snap_height)
+        assert_equal(
+            dump_output4['txoutset_hash'],
+            "8a1db0d6e958ce0d7c963bc6fc91ead596c027129bacec68acc40351037b09d7")
+        assert sha256sum_file(dump_output['path']) != sha256sum_file(dump_output4['path'])
+
+        # Use a hash instead of a height
+        prev_snap_hash = n0.getblockhash(prev_snap_height)
+        dump_output5 = n0.dumptxoutset('utxos5.dat', rollback=prev_snap_hash)
+        assert_equal(sha256sum_file(dump_output4['path']), sha256sum_file(dump_output5['path']))
+
+        # TODO: This is a hack to set m_best_header to the correct value after
+        # dumptxoutset/reconsiderblock. Otherwise the wrong error messages are
+        # returned in following tests. It can be removed once this bug is
+        # fixed. See also https://github.com/bitcoin/bitcoin/issues/26245
+        self.restart_node(0, ["-reindex"])
+
+        # Ensure n0 is back at the tip
+        assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT)
+
         self.test_snapshot_with_less_work(dump_output['path'])
         self.test_invalid_mempool_state(dump_output['path'])
         self.test_invalid_snapshot_scenarios(dump_output['path'])
diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py
index 6c98b4ee661..2542ddd220b 100755
--- a/test/functional/rpc_dumptxoutset.py
+++ b/test/functional/rpc_dumptxoutset.py
@@ -56,6 +56,10 @@ class DumptxoutsetTest(BitcoinTestFramework):
         assert_raises_rpc_error(
             -8, "Couldn't open file {}.incomplete for writing".format(invalid_path), node.dumptxoutset, invalid_path, "latest")
 
+        self.log.info(f"Test that dumptxoutset with unknown dump type fails")
+        assert_raises_rpc_error(
+            -8, 'Invalid snapshot type "bogus" specified. Please specify "rollback" or "latest"', node.dumptxoutset, 'utxos.dat', "bogus")
+
 
 if __name__ == '__main__':
     DumptxoutsetTest(__file__).main()

From 20a1c77aa7dec2449071187a439d17f7aeaee648 Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sat, 2 Mar 2024 18:28:07 +0100
Subject: [PATCH 5/8] contrib: Remove test_utxo_snapshots.sh

---
 contrib/devtools/test_utxo_snapshots.sh | 209 ------------------------
 1 file changed, 209 deletions(-)
 delete mode 100755 contrib/devtools/test_utxo_snapshots.sh

diff --git a/contrib/devtools/test_utxo_snapshots.sh b/contrib/devtools/test_utxo_snapshots.sh
deleted file mode 100755
index ad948d4a142..00000000000
--- a/contrib/devtools/test_utxo_snapshots.sh
+++ /dev/null
@@ -1,209 +0,0 @@
-#!/usr/bin/env bash
-# Demonstrate the creation and usage of UTXO snapshots.
-#
-# A server node starts up, IBDs up to a certain height, then generates a UTXO
-# snapshot at that point.
-#
-# The server then downloads more blocks (to create a diff from the snapshot).
-#
-# We bring a client up, load the UTXO snapshot, and we show the client sync to
-# the "network tip" and then start a background validation of the snapshot it
-# loaded. We see the background validation chainstate removed after validation
-# completes.
-#
-# The shellcheck rule SC2086 (quoted variables) disablements are necessary
-# since this rule needs to be violated in order to get bitcoind to pick up on
-# $EARLY_IBD_FLAGS for the script to work.
-
-export LC_ALL=C
-set -e
-
-BASE_HEIGHT=${1:-30000}
-INCREMENTAL_HEIGHT=20000
-FINAL_HEIGHT=$((BASE_HEIGHT + INCREMENTAL_HEIGHT))
-
-SERVER_DATADIR="$(pwd)/utxodemo-data-server-$BASE_HEIGHT"
-CLIENT_DATADIR="$(pwd)/utxodemo-data-client-$BASE_HEIGHT"
-UTXO_DAT_FILE="$(pwd)/utxo.$BASE_HEIGHT.dat"
-
-# Chosen to try to not interfere with any running bitcoind processes.
-SERVER_PORT=8633
-SERVER_RPC_PORT=8632
-
-CLIENT_PORT=8733
-CLIENT_RPC_PORT=8732
-
-SERVER_PORTS="-port=${SERVER_PORT} -rpcport=${SERVER_RPC_PORT}"
-CLIENT_PORTS="-port=${CLIENT_PORT} -rpcport=${CLIENT_RPC_PORT}"
-
-# Ensure the client exercises all indexes to test that snapshot use works
-# properly with indexes.
-ALL_INDEXES="-txindex -coinstatsindex -blockfilterindex=1"
-
-if ! command -v jq >/dev/null ; then
-  echo "This script requires jq to parse JSON RPC output. Please install it."
-  echo "(e.g. sudo apt install jq)"
-  exit 1
-fi
-
-DUMP_OUTPUT="dumptxoutset-output-$BASE_HEIGHT.json"
-
-finish() {
-  echo
-  echo "Killing server and client PIDs ($SERVER_PID, $CLIENT_PID) and cleaning up datadirs"
-  echo
-  rm -f "$UTXO_DAT_FILE" "$DUMP_OUTPUT"
-  rm -rf "$SERVER_DATADIR" "$CLIENT_DATADIR"
-  kill -9 "$SERVER_PID" "$CLIENT_PID"
-}
-
-trap finish EXIT
-
-# Need to specify these to trick client into accepting server as a peer
-# it can IBD from, otherwise the default values prevent IBD from the server node.
-EARLY_IBD_FLAGS="-maxtipage=9223372036854775207 -minimumchainwork=0x00"
-
-server_rpc() {
-  ./src/bitcoin-cli -rpcport=$SERVER_RPC_PORT -datadir="$SERVER_DATADIR" "$@"
-}
-client_rpc() {
-  ./src/bitcoin-cli -rpcport=$CLIENT_RPC_PORT -datadir="$CLIENT_DATADIR" "$@"
-}
-server_sleep_til_boot() {
-  while ! server_rpc ping >/dev/null 2>&1; do sleep 0.1; done
-}
-client_sleep_til_boot() {
-  while ! client_rpc ping >/dev/null 2>&1; do sleep 0.1; done
-}
-server_sleep_til_shutdown() {
-  while server_rpc ping >/dev/null 2>&1; do sleep 0.1; done
-}
-
-mkdir -p "$SERVER_DATADIR" "$CLIENT_DATADIR"
-
-echo "Hi, welcome to the assumeutxo demo/test"
-echo
-echo "We're going to"
-echo
-echo "  - start up a 'server' node, sync it via mainnet IBD to height ${BASE_HEIGHT}"
-echo "  - create a UTXO snapshot at that height"
-echo "  - IBD ${INCREMENTAL_HEIGHT} more blocks on top of that"
-echo
-echo "then we'll demonstrate assumeutxo by "
-echo
-echo "  - starting another node (the 'client') and loading the snapshot in"
-echo "    * first you'll have to modify the code slightly (chainparams) and recompile"
-echo "    * don't worry, we'll make it easy"
-echo "  - observing the client sync ${INCREMENTAL_HEIGHT} blocks on top of the snapshot from the server"
-echo "  - observing the client validate the snapshot chain via background IBD"
-echo
-read -p "Press [enter] to continue" _
-
-echo
-echo "-- Starting the demo. You might want to run the two following commands in"
-echo "   separate terminal windows:"
-echo
-echo "   watch -n0.1 tail -n 30 $SERVER_DATADIR/debug.log"
-echo "   watch -n0.1 tail -n 30 $CLIENT_DATADIR/debug.log"
-echo
-read -p "Press [enter] to continue" _
-
-echo
-echo "-- IBDing the blocks (height=$BASE_HEIGHT) required to the server node..."
-# shellcheck disable=SC2086
-./src/bitcoind -logthreadnames=1 $SERVER_PORTS \
-    -datadir="$SERVER_DATADIR" $EARLY_IBD_FLAGS -stopatheight="$BASE_HEIGHT" >/dev/null
-
-echo
-echo "-- Creating snapshot at ~ height $BASE_HEIGHT ($UTXO_DAT_FILE)..."
-server_sleep_til_shutdown # wait for stopatheight to be hit
-# shellcheck disable=SC2086
-./src/bitcoind -logthreadnames=1 $SERVER_PORTS \
-    -datadir="$SERVER_DATADIR" $EARLY_IBD_FLAGS -connect=0 -listen=0 >/dev/null &
-SERVER_PID="$!"
-
-server_sleep_til_boot
-server_rpc dumptxoutset "$UTXO_DAT_FILE" > "$DUMP_OUTPUT"
-cat "$DUMP_OUTPUT"
-kill -9 "$SERVER_PID"
-
-RPC_BASE_HEIGHT=$(jq -r .base_height < "$DUMP_OUTPUT")
-RPC_AU=$(jq -r .txoutset_hash < "$DUMP_OUTPUT")
-RPC_NCHAINTX=$(jq -r .nchaintx < "$DUMP_OUTPUT")
-RPC_BLOCKHASH=$(jq -r .base_hash < "$DUMP_OUTPUT")
-
-server_sleep_til_shutdown
-
-echo
-echo "-- Now: add the following to CMainParams::m_assumeutxo_data"
-echo "   in src/kernel/chainparams.cpp, and recompile:"
-echo
-echo "   {.height = ${RPC_BASE_HEIGHT}, .hash_serialized = AssumeutxoHash{uint256{\"${RPC_AU}\"}}, .m_chain_tx_count = ${RPC_NCHAINTX}, .blockhash = consteval_ctor(uint256{\"${RPC_BLOCKHASH}\"})},"
-echo
-echo
-echo "-- IBDing more blocks to the server node (height=$FINAL_HEIGHT) so there is a diff between snapshot and tip..."
-# shellcheck disable=SC2086
-./src/bitcoind $SERVER_PORTS -logthreadnames=1 -datadir="$SERVER_DATADIR" \
-    $EARLY_IBD_FLAGS -stopatheight="$FINAL_HEIGHT" >/dev/null
-
-echo
-echo "-- Starting the server node to provide blocks to the client node..."
-# shellcheck disable=SC2086
-./src/bitcoind $SERVER_PORTS -logthreadnames=1 -debug=net -datadir="$SERVER_DATADIR" \
-    $EARLY_IBD_FLAGS -connect=0 -listen=1 >/dev/null &
-SERVER_PID="$!"
-server_sleep_til_boot
-
-echo
-echo "-- Okay, what you're about to see is the client starting up and activating the snapshot."
-echo "   I'm going to display the top 14 log lines from the client on top of an RPC called"
-echo "   getchainstates, which is like getblockchaininfo but for both the snapshot and "
-echo "   background validation chainstates."
-echo
-echo "   You're going to first see the snapshot chainstate sync to the server's tip, then"
-echo "   the background IBD chain kicks in to validate up to the base of the snapshot."
-echo
-echo "   Once validation of the snapshot is done, you should see log lines indicating"
-echo "   that we've deleted the background validation chainstate."
-echo
-echo "   Once everything completes, exit the watch command with CTRL+C."
-echo
-read -p "When you're ready for all this, hit [enter]" _
-
-echo
-echo "-- Starting the client node to get headers from the server, then load the snapshot..."
-# shellcheck disable=SC2086
-./src/bitcoind $CLIENT_PORTS $ALL_INDEXES -logthreadnames=1 -datadir="$CLIENT_DATADIR" \
-    -connect=0 -addnode=127.0.0.1:$SERVER_PORT -debug=net $EARLY_IBD_FLAGS >/dev/null &
-CLIENT_PID="$!"
-client_sleep_til_boot
-
-echo
-echo "-- Initial state of the client:"
-client_rpc getchainstates
-
-echo
-echo "-- Loading UTXO snapshot into client. Calling RPC in a loop..."
-while ! client_rpc loadtxoutset "$UTXO_DAT_FILE" ; do sleep 10; done
-
-watch -n 0.3 "( tail -n 14 $CLIENT_DATADIR/debug.log ; echo ; ./src/bitcoin-cli -rpcport=$CLIENT_RPC_PORT -datadir=$CLIENT_DATADIR getchainstates) | cat"
-
-echo
-echo "-- Okay, now I'm going to restart the client to make sure that the snapshot chain reloads "
-echo "   as the main chain properly..."
-echo
-echo "   Press CTRL+C after you're satisfied to exit the demo"
-echo
-read -p "Press [enter] to continue"
-
-client_sleep_til_boot
-# shellcheck disable=SC2086
-./src/bitcoind $CLIENT_PORTS $ALL_INDEXES -logthreadnames=1 -datadir="$CLIENT_DATADIR" -connect=0 \
-    -addnode=127.0.0.1:$SERVER_PORT "$EARLY_IBD_FLAGS" >/dev/null &
-CLIENT_PID="$!"
-client_sleep_til_boot
-
-watch -n 0.3 "( tail -n 14 $CLIENT_DATADIR/debug.log ; echo ; ./src/bitcoin-cli -rpcport=$CLIENT_RPC_PORT -datadir=$CLIENT_DATADIR getchainstates) | cat"
-
-echo
-echo "-- Done!"

From b29c21fc92dcc3da95bd032ba41675a8b9a0a24b Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Mon, 4 Mar 2024 16:14:29 +0100
Subject: [PATCH 6/8] assumeutxo: Remove devtools/utxo_snapshot.sh

---
 contrib/devtools/utxo_snapshot.sh | 104 ------------------------------
 doc/design/assumeutxo.md          |   6 +-
 2 files changed, 2 insertions(+), 108 deletions(-)
 delete mode 100755 contrib/devtools/utxo_snapshot.sh

diff --git a/contrib/devtools/utxo_snapshot.sh b/contrib/devtools/utxo_snapshot.sh
deleted file mode 100755
index e8781d94d92..00000000000
--- a/contrib/devtools/utxo_snapshot.sh
+++ /dev/null
@@ -1,104 +0,0 @@
-#!/usr/bin/env bash
-#
-# Copyright (c) 2019-2023 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-export LC_ALL=C
-
-set -ueo pipefail
-
-NETWORK_DISABLED=false
-
-if (( $# < 3 )); then
-  echo 'Usage: utxo_snapshot.sh <generate-at-height> <snapshot-out-path> <bitcoin-cli-call ...>'
-  echo
-  echo "  if <snapshot-out-path> is '-', don't produce a snapshot file but instead print the "
-  echo "  expected assumeutxo hash"
-  echo
-  echo 'Examples:'
-  echo
-  echo "  ./contrib/devtools/utxo_snapshot.sh 570000 utxo.dat ./src/bitcoin-cli -datadir=\$(pwd)/testdata"
-  echo '  ./contrib/devtools/utxo_snapshot.sh 570000 - ./src/bitcoin-cli'
-  exit 1
-fi
-
-GENERATE_AT_HEIGHT="${1}"; shift;
-OUTPUT_PATH="${1}"; shift;
-# Most of the calls we make take a while to run, so pad with a lengthy timeout.
-BITCOIN_CLI_CALL="${*} -rpcclienttimeout=9999999"
-
-# Check if the node is pruned and get the pruned block height
-PRUNED=$( ${BITCOIN_CLI_CALL} getblockchaininfo | awk '/pruneheight/ {print $2}' | tr -d ',' )
-
-if (( GENERATE_AT_HEIGHT < PRUNED )); then
-  echo "Error: The requested snapshot height (${GENERATE_AT_HEIGHT}) should be greater than the pruned block height (${PRUNED})."
-  exit 1
-fi
-
-# Check current block height to ensure the node has synchronized past the required block
-CURRENT_BLOCK_HEIGHT=$(${BITCOIN_CLI_CALL} getblockcount)
-PIVOT_BLOCK_HEIGHT=$(( GENERATE_AT_HEIGHT + 1 ))
-
-if (( PIVOT_BLOCK_HEIGHT > CURRENT_BLOCK_HEIGHT )); then
-  (>&2 echo "Error: The node has not yet synchronized to block height ${PIVOT_BLOCK_HEIGHT}.")
-  (>&2 echo "Please wait until the node has synchronized past this block height and try again.")
-  exit 1
-fi
-
-# Early exit if file at OUTPUT_PATH already exists
-if [[ -e "$OUTPUT_PATH" ]]; then
-  (>&2 echo "Error: $OUTPUT_PATH already exists or is not a valid path.")
-  exit 1
-fi
-
-# Validate that the path is correct
-if [[ "${OUTPUT_PATH}" != "-" && ! -d "$(dirname "${OUTPUT_PATH}")" ]]; then
-  (>&2 echo "Error: The directory $(dirname "${OUTPUT_PATH}") does not exist.")
-  exit 1
-fi
-
-function cleanup {
-  (>&2 echo "Restoring chain to original height; this may take a while")
-  ${BITCOIN_CLI_CALL} reconsiderblock "${PIVOT_BLOCKHASH}"
-
-  if $NETWORK_DISABLED; then
-    (>&2 echo "Restoring network activity")
-    ${BITCOIN_CLI_CALL} setnetworkactive true
-  fi
-}
-
-function early_exit {
-  (>&2 echo "Exiting due to Ctrl-C")
-  cleanup
-  exit 1
-}
-
-# Prompt the user to disable network activity
-read -p "Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n): " -r
-if [[ "$REPLY" =~ ^[Yy]*$ || -z "$REPLY" ]]; then
-  # User input is "Y", "y", or Enter key, proceed with the action
-  NETWORK_DISABLED=true
-  (>&2 echo "Disabling network activity")
-  ${BITCOIN_CLI_CALL} setnetworkactive false
-else
-  (>&2 echo "Network activity remains enabled")
-fi
-
-# Block we'll invalidate/reconsider to rewind/fast-forward the chain.
-PIVOT_BLOCKHASH=$($BITCOIN_CLI_CALL getblockhash $(( GENERATE_AT_HEIGHT + 1 )) )
-
-# Trap for normal exit and Ctrl-C
-trap cleanup EXIT
-trap early_exit INT
-
-(>&2 echo "Rewinding chain back to height ${GENERATE_AT_HEIGHT} (by invalidating ${PIVOT_BLOCKHASH}); this may take a while")
-${BITCOIN_CLI_CALL} invalidateblock "${PIVOT_BLOCKHASH}"
-
-if [[ "${OUTPUT_PATH}" = "-" ]]; then
-  (>&2 echo "Generating txoutset info...")
-  ${BITCOIN_CLI_CALL} gettxoutsetinfo | grep hash_serialized_3 | sed 's/^.*: "\(.\+\)\+",/\1/g'
-else
-  (>&2 echo "Generating UTXO snapshot...")
-  ${BITCOIN_CLI_CALL} dumptxoutset "${OUTPUT_PATH}"
-fi
diff --git a/doc/design/assumeutxo.md b/doc/design/assumeutxo.md
index a4980729d0c..91bc96b57f6 100644
--- a/doc/design/assumeutxo.md
+++ b/doc/design/assumeutxo.md
@@ -36,13 +36,11 @@ use a significant amount of disk space.
 
 ## Generating a snapshot
 
-The RPC command `dumptxoutset` can be used to generate a snapshot. This can be used
+The RPC command `dumptxoutset` can be used to generate a snapshot for the current
+tip or a recent height. This can be used
 to create a snapshot on one node that you wish to load on another node.
 It can also be used to verify the hardcoded snapshot hash in the source code.
 
-The utility script
-`./contrib/devtools/utxo_snapshot.sh` may be of use.
-
 ## General background
 
 - [assumeutxo proposal](https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal)

From e868a6e070a91c00555e72181f9b14bbf0373fdc Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Sat, 16 Mar 2024 21:36:28 +0100
Subject: [PATCH 7/8] doc: Improve assumeutxo guide and add more docs/comments

Also fixes some outdated information in the remaining design doc.
---
 doc/assumeutxo.md        | 85 ++++++++++++++++++++++++++++++++++++++++
 doc/design/assumeutxo.md | 45 ++-------------------
 src/rpc/blockchain.cpp   |  7 +++-
 3 files changed, 94 insertions(+), 43 deletions(-)
 create mode 100644 doc/assumeutxo.md

diff --git a/doc/assumeutxo.md b/doc/assumeutxo.md
new file mode 100644
index 00000000000..4584eebf436
--- /dev/null
+++ b/doc/assumeutxo.md
@@ -0,0 +1,85 @@
+# Assumeutxo Usage
+
+Assumeutxo is a feature that allows fast bootstrapping of a validating bitcoind
+instance.
+
+For notes on the design of Assumeutxo, please refer to [the design doc](/doc/assumeutxo.md).
+
+## Loading a snapshot
+
+There is currently no canonical source for snapshots, but any downloaded snapshot
+will be checked against a hash that's been hardcoded in source code. If there is
+no source for the snapshot you need, you can generate it yourself using
+`dumptxoutset` on another node that is already synced (see
+[Generating a snapshot](#generating-a-snapshot)).
+
+Once you've obtained the snapshot, you can use the RPC command `loadtxoutset` to
+load it.
+
+```
+$ bitcoin-cli loadtxoutset /path/to/input
+```
+
+After the snapshot has loaded, the syncing process of both the snapshot chain
+and the background IBD chain can be monitored with the `getchainstates` RPC.
+
+### Pruning
+
+A pruned node can load a snapshot. To save space, it's possible to delete the
+snapshot file as soon as `loadtxoutset` finishes.
+
+The minimum `-prune` setting is 550 MiB, but this functionality ignores that
+minimum and uses at least 1100 MiB.
+
+As the background sync continues there will be temporarily two chainstate
+directories, each multiple gigabytes in size (likely growing larger than the
+downloaded snapshot).
+
+### Indexes
+
+Indexes work but don't take advantage of this feature. They always start building
+from the genesis block and can only apply blocks in order. Once the background
+validation reaches the snapshot block, indexes will continue to build all the
+way to the tip.
+
+
+For indexes that support pruning, note that these indexes only allow blocks that
+were already indexed to be pruned. Blocks that are not indexed yet will also
+not be pruned.
+
+This means that, if the snapshot is old, then a lot of blocks after the snapshot
+block will need to be downloaded, and these blocks can't be pruned until they
+are indexed, so they could consume a lot of disk space until indexing catches up
+to the snapshot block.
+
+## Generating a snapshot
+
+The RPC command `dumptxoutset` can be used to generate a snapshot for the current
+tip (using type "latest") or a recent height (using type "rollback"). A generated
+snapshot from one node can then be loaded
+on any other node. However, keep in mind that the snapshot hash needs to be
+listed in the chainparams to make it usable. If there is no snapshot hash for
+the height you have chosen already, you will need to change the code there and
+re-compile.
+
+Using the type parameter "rollback", `dumptxoutset` can also be used to verify the
+hardcoded snapshot hash in the source code by regenerating the snapshot and
+comparing the hash.
+
+Example usage:
+
+```
+$ bitcoin-cli -rpcclienttimeout=0 dumptxoutset /path/to/output rollback
+```
+
+For most of the duration of `dumptxoutset` running the node is in a temporary
+state that does not actually reflect reality, i.e. blocks are marked invalid
+although we know they are not invalid. Because of this it is discouraged to
+interact with the node in any other way during this time to avoid inconsistent
+results and race conditions, particularly RPCs that interact with blockstorage.
+This inconsistent state is also why network activity is temporarily disabled,
+causing us to disconnect from all peers.
+
+`dumptxoutset` takes some time to complete, independent of hardware and
+what parameter is chosen. Because of that it is recommended to increase the RPC
+client timeout value (use `-rpcclienttimeout=0` for no timeout).
diff --git a/doc/design/assumeutxo.md b/doc/design/assumeutxo.md
index 91bc96b57f6..123c02ac138 100644
--- a/doc/design/assumeutxo.md
+++ b/doc/design/assumeutxo.md
@@ -1,45 +1,6 @@
-# assumeutxo
+# Assumeutxo Design
 
-Assumeutxo is a feature that allows fast bootstrapping of a validating bitcoind
-instance.
-
-## Loading a snapshot
-
-There is currently no canonical source for snapshots, but any downloaded snapshot
-will be checked against a hash that's been hardcoded in source code.
-
-Once you've obtained the snapshot, you can use the RPC command `loadtxoutset` to
-load it.
-
-### Pruning
-
-A pruned node can load a snapshot. To save space, it's possible to delete the
-snapshot file as soon as `loadtxoutset` finishes.
-
-The minimum `-prune` setting is 550 MiB, but this functionality ignores that
-minimum and uses at least 1100 MiB.
-
-As the background sync continues there will be temporarily two chainstate
-directories, each multiple gigabytes in size (likely growing larger than the
-downloaded snapshot).
-
-### Indexes
-
-Indexes work but don't take advantage of this feature. They always start building
-from the genesis block. Once the background validation reaches the snapshot block,
-indexes will continue to build all the way to the tip.
-
-For indexes that support pruning, note that no pruning will take place between
-the snapshot and the tip, until the background sync has completed - after which
-everything is pruned. Depending on how old the snapshot is, this may temporarily
-use a significant amount of disk space.
-
-## Generating a snapshot
-
-The RPC command `dumptxoutset` can be used to generate a snapshot for the current
-tip or a recent height. This can be used
-to create a snapshot on one node that you wish to load on another node.
-It can also be used to verify the hardcoded snapshot hash in the source code.
+For notes on the usage of Assumeutxo, please refer to [the usage doc](/doc/assumeutxo.md).
 
 ## General background
 
@@ -77,7 +38,7 @@ data.
 ### "Normal" operation via initial block download
 
 `ChainstateManager` manages a single Chainstate object, for which
-`m_snapshot_blockhash` is null. This chainstate is (maybe obviously)
+`m_from_snapshot_blockhash` is `std::nullopt`. This chainstate is (maybe obviously)
 considered active. This is the "traditional" mode of operation for bitcoind.
 
 |    |    |
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 4dc7dbfa939..3a2bbeecf35 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2676,7 +2676,10 @@ static RPCHelpMan dumptxoutset()
 {
     return RPCHelpMan{
         "dumptxoutset",
-        "Write the serialized UTXO set to a file.",
+        "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"
+        "Unless the the \"latest\" type is requested, the node will roll back to the requested height and network activity will be suspended during this process. "
+        "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"
+        "This call may take several minutes. Make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)",
         {
             {"path", RPCArg::Type::STR, RPCArg::Optional::NO, "Path to the output file. If relative, will be prefixed by datadir."},
             {"type", RPCArg::Type::STR, RPCArg::Default(""), "The type of snapshot to create. Can be \"latest\" to create a snapshot of the current UTXO set or \"rollback\" to temporarily roll back the state of the node to a historical block before creating the snapshot of a historical UTXO set. This parameter can be omitted if a separate \"rollback\" named parameter is specified indicating the height or hash of a specific historical block. If \"rollback\" is specified and separate \"rollback\" named parameter is not specified, this will roll back to the latest valid snapshot block that currently be loaded with loadtxoutset."},
@@ -2773,6 +2776,8 @@ static RPCHelpMan dumptxoutset()
         // would be classified as a block connecting an invalid block.
         disable_network = std::make_unique<NetworkDisable>(connman);
 
+        // Note: Unlocking cs_main before CreateUTXOSnapshot might be racy
+        // if the user interacts with any other *block RPCs.
         invalidate_index = WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Next(target_index));
         InvalidateBlock(*node.chainman, invalidate_index->GetBlockHash());
         const CBlockIndex* new_tip_index{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};

From 94b0adcc371540732453d70309c4083d4bd9cd6b Mon Sep 17 00:00:00 2001
From: Fabian Jahr <fjahr@protonmail.com>
Date: Thu, 15 Aug 2024 22:50:09 +0200
Subject: [PATCH 8/8] rpc, refactor: Prevent potential race conditions in
 dumptxoutset

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/rpc/blockchain.cpp | 93 +++++++++++++++++++++++++++++++++---------
 src/rpc/blockchain.h   |  2 +-
 src/validation.h       |  2 +-
 3 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 3a2bbeecf35..2824b5e8ef4 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -75,6 +75,22 @@ static GlobalMutex cs_blockchange;
 static std::condition_variable cond_blockchange;
 static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange);
 
+std::tuple<std::unique_ptr<CCoinsViewCursor>, CCoinsStats, const CBlockIndex*>
+PrepareUTXOSnapshot(
+    Chainstate& chainstate,
+    const std::function<void()>& interruption_point = {})
+    EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
+UniValue WriteUTXOSnapshot(
+    Chainstate& chainstate,
+    CCoinsViewCursor* pcursor,
+    CCoinsStats* maybe_stats,
+    const CBlockIndex* tip,
+    AutoFile& afile,
+    const fs::path& path,
+    const fs::path& temppath,
+    const std::function<void()>& interruption_point = {});
+
 /* Calculate the difficulty for a given block index.
  */
 double GetDifficulty(const CBlockIndex& blockindex)
@@ -2776,31 +2792,48 @@ static RPCHelpMan dumptxoutset()
         // would be classified as a block connecting an invalid block.
         disable_network = std::make_unique<NetworkDisable>(connman);
 
-        // Note: Unlocking cs_main before CreateUTXOSnapshot might be racy
-        // if the user interacts with any other *block RPCs.
         invalidate_index = WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Next(target_index));
         InvalidateBlock(*node.chainman, invalidate_index->GetBlockHash());
-        const CBlockIndex* new_tip_index{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
+    }
 
+    Chainstate* chainstate;
+    std::unique_ptr<CCoinsViewCursor> cursor;
+    CCoinsStats stats;
+    UniValue result;
+    UniValue error;
+    {
+        // Lock the chainstate before calling PrepareUtxoSnapshot, to be able
+        // to get a UTXO database cursor while the chain is pointing at the
+        // target block. After that, release the lock while calling
+        // WriteUTXOSnapshot. The cursor will remain valid and be used by
+        // WriteUTXOSnapshot to write a consistent snapshot even if the
+        // chainstate changes.
+        LOCK(node.chainman->GetMutex());
+        chainstate = &node.chainman->ActiveChainstate();
         // In case there is any issue with a block being read from disk we need
         // to stop here, otherwise the dump could still be created for the wrong
         // height.
         // The new tip could also not be the target block if we have a stale
         // sister block of invalidate_index. This block (or a descendant) would
         // be activated as the new tip and we would not get to new_tip_index.
-        if (new_tip_index != target_index) {
-            ReconsiderBlock(*node.chainman, invalidate_index->GetBlockHash());
-            throw JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height, reverting to tip.");
+        if (target_index != chainstate->m_chain.Tip()) {
+            LogInfo("Failed to roll back to requested height, reverting to tip.\n");
+            error = JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height.");
+        } else {
+            std::tie(cursor, stats, tip) = PrepareUTXOSnapshot(*chainstate, node.rpc_interruption_point);
         }
     }
 
-    UniValue result = CreateUTXOSnapshot(
-        node, node.chainman->ActiveChainstate(), afile, path, temppath);
-    fs::rename(temppath, path);
-
+    if (error.isNull()) {
+        result = WriteUTXOSnapshot(*chainstate, cursor.get(), &stats, tip, afile, path, temppath, node.rpc_interruption_point);
+        fs::rename(temppath, path);
+    }
     if (invalidate_index) {
         ReconsiderBlock(*node.chainman, invalidate_index->GetBlockHash());
     }
+    if (!error.isNull()) {
+        throw error;
+    }
 
     result.pushKV("path", path.utf8string());
     return result;
@@ -2808,12 +2841,10 @@ static RPCHelpMan dumptxoutset()
     };
 }
 
-UniValue CreateUTXOSnapshot(
-    NodeContext& node,
+std::tuple<std::unique_ptr<CCoinsViewCursor>, CCoinsStats, const CBlockIndex*>
+PrepareUTXOSnapshot(
     Chainstate& chainstate,
-    AutoFile& afile,
-    const fs::path& path,
-    const fs::path& temppath)
+    const std::function<void()>& interruption_point)
 {
     std::unique_ptr<CCoinsViewCursor> pcursor;
     std::optional<CCoinsStats> maybe_stats;
@@ -2823,7 +2854,7 @@ UniValue CreateUTXOSnapshot(
         // We need to lock cs_main to ensure that the coinsdb isn't written to
         // between (i) flushing coins cache to disk (coinsdb), (ii) getting stats
         // based upon the coinsdb, and (iii) constructing a cursor to the
-        // coinsdb for use below this block.
+        // coinsdb for use in WriteUTXOSnapshot.
         //
         // Cursors returned by leveldb iterate over snapshots, so the contents
         // of the pcursor will not be affected by simultaneous writes during
@@ -2832,11 +2863,11 @@ UniValue CreateUTXOSnapshot(
         // See discussion here:
         //   https://github.com/bitcoin/bitcoin/pull/15606#discussion_r274479369
         //
-        LOCK(::cs_main);
+        AssertLockHeld(::cs_main);
 
         chainstate.ForceFlushStateToDisk();
 
-        maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, node.rpc_interruption_point);
+        maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, interruption_point);
         if (!maybe_stats) {
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set");
         }
@@ -2845,6 +2876,19 @@ UniValue CreateUTXOSnapshot(
         tip = CHECK_NONFATAL(chainstate.m_blockman.LookupBlockIndex(maybe_stats->hashBlock));
     }
 
+    return {std::move(pcursor), *CHECK_NONFATAL(maybe_stats), tip};
+}
+
+UniValue WriteUTXOSnapshot(
+    Chainstate& chainstate,
+    CCoinsViewCursor* pcursor,
+    CCoinsStats* maybe_stats,
+    const CBlockIndex* tip,
+    AutoFile& afile,
+    const fs::path& path,
+    const fs::path& temppath,
+    const std::function<void()>& interruption_point)
+{
     LOG_TIME_SECONDS(strprintf("writing UTXO snapshot at height %s (%s) to file %s (via %s)",
         tip->nHeight, tip->GetBlockHash().ToString(),
         fs::PathToString(path), fs::PathToString(temppath)));
@@ -2880,7 +2924,7 @@ UniValue CreateUTXOSnapshot(
     pcursor->GetKey(key);
     last_hash = key.hash;
     while (pcursor->Valid()) {
-        if (iter % 5000 == 0) node.rpc_interruption_point();
+        if (iter % 5000 == 0) interruption_point();
         ++iter;
         if (pcursor->GetKey(key) && pcursor->GetValue(coin)) {
             if (key.hash != last_hash) {
@@ -2911,6 +2955,17 @@ UniValue CreateUTXOSnapshot(
     return result;
 }
 
+UniValue CreateUTXOSnapshot(
+    node::NodeContext& node,
+    Chainstate& chainstate,
+    AutoFile& afile,
+    const fs::path& path,
+    const fs::path& tmppath)
+{
+    auto [cursor, stats, tip]{WITH_LOCK(::cs_main, return PrepareUTXOSnapshot(chainstate, node.rpc_interruption_point))};
+    return WriteUTXOSnapshot(chainstate, cursor.get(), &stats, tip, afile, path, tmppath, node.rpc_interruption_point);
+}
+
 static RPCHelpMan loadtxoutset()
 {
     return RPCHelpMan{
diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h
index f6a7fe236c9..b5a0382da16 100644
--- a/src/rpc/blockchain.h
+++ b/src/rpc/blockchain.h
@@ -48,7 +48,7 @@ UniValue blockheaderToJSON(const CBlockIndex& tip, const CBlockIndex& blockindex
 void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_weight);
 
 /**
- * Helper to create UTXO snapshots given a chainstate and a file handle.
+ * Test-only helper to create UTXO snapshots given a chainstate and a file handle.
  * @return a UniValue map containing metadata about the snapshot.
  */
 UniValue CreateUTXOSnapshot(
diff --git a/src/validation.h b/src/validation.h
index f905d6e6240..cfaa4d04d3e 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -914,7 +914,7 @@ private:
     //! Internal helper for ActivateSnapshot().
     //!
     //! De-serialization of a snapshot that is created with
-    //! CreateUTXOSnapshot() in rpc/blockchain.cpp.
+    //! the dumptxoutset RPC.
     //! To reduce space the serialization format of the snapshot avoids
     //! duplication of tx hashes. The code takes advantage of the guarantee by
     //! leveldb that keys are lexicographically sorted.