Repository: bitcoin/bitcoin (language: cpp)

Issue:
28.0rc1 synchronizes much slower on Windows
Bitcoin Core 28.0rc1 (both pre-built and self-built binary) on Windows 10 synchronizes much slower than 27.0 after around block 120,000 when blocks stop being almost empty. I'm running with default settings (dbcache=450, pruning disabled).

Time to synchronize to block 300,000: (measured around 10 runs each, alternating between the versions)
27.0: **695 seconds**
28.0rc1: **2760 seconds**

![28.0rc1](https://github.com/user-attachments/assets/060a1e18-7f97-4e86-9ba8-5bf867b3510e)

Running the Linux pre-built binary in WSL doesn't show the slowdown, so it's probably limited to Windows.

~~I'm going to bisect this, but that will definitely take me a while. If anyone suspects a commit/PR that I should look at first that might help.~~ I've bisected this down to #28052, for more details see https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335184072.


Discussion / hints:
Try whether #28280 had any impact.
I actually benchmarked #28280 right before it got merged: https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2268695679

From the results it definitely seems that PR is not to blame and the regression happened later.
https://github.com/bitcoin/bitcoin/pull/30326?
Reason I suspect #30326 is because `try_emplace` must be doing more work than just `find` and is only called after blocks stop being empty because previous coins are now being looked up to be spent.

Relevant stackoverflow https://stackoverflow.com/questions/24209592/performance-of-emplace-is-worse-than-check-followed-by-emplace
@andrewtoth I cannot imagine that causing a 5x slowdown, though?
Only did one run of each but I have basically the opposite result: 4406 seconds for 27.1, and 2949 for 28.0rc1.
@vostrnad when you say synchronizes, do you mean running with `-reindex-chainstate` or are you starting with a fresh data directory and downloading blocks from peers? If the latter, are you downloading blocks from a node in your local network?
@andrewtoth I updated the OP. I'm synchronizing from a local node with a fresh datadir each time.
@vostrnad  I'm trying to get a Windows box setup to reproduce this myself, but if you get a chance, could you do a run of both again with `-debug=bench` set, and post the final set of `[bench]` logs that get written to debug.log? Those might help narrow where the slowdown is coming from.
I've bisected this down to #28052. From the benchmark results it appears to be solely responsible for the slowdown between 27.0 and 28.0rc1 (so #30326 should be fine @andrewtoth).

Benchmark results: (fresh sync to block 300,000 from a local node, using default settings, all binaries self-built using Mingw-w64, around 6 runs each)

d82283950f5ff3b2116e705f931c6e89e5fdd0be (v27.0): **738s ± 16s**
fa7f7ac040a9467c307b20e77dc47c87d7377ded (first commit of #28052): **733s ± 21s**
fa895c72832f9555b52d5bb1dba1093f73de3136 (top commit of #28052): **2827s ± 9s**
88f0419c1ab1e5f3ccf425435e530d8ceb72c7f1 (v28.0rc1): **2815 ± 13s**

![blocksxor](https://github.com/user-attachments/assets/180f6600-fe2f-4cef-8bcd-6c5fa6c405fc)

@vostrnad Does the performance on Windows recover if you run with the `-blocksxor=0` option?
@sipa No, it's exactly the same with `-blocksxor=0`.
> @sipa No, it's exactly the same with `-blocksxor=0`.

This has uncovered a bug in #28052, `InitBlocksdirXorKey` does not return an empty `std::vector<std::byte>` when the `-blocksxor=0` argument is used, it instead [returns](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1185) a [vector with 8 bytes equal to zero](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1152):

```cpp
static auto InitBlocksdirXorKey(const BlockManager::Options& opts)
{
    std::array<std::byte, 8> xor_key{};
    
    if (opts.use_xor && fs::is_empty(opts.blocks_dir)) {
        // Only use random fresh key when the boolean option is set and on the
        // very first start of the program.
        FastRandomContext{}.fillrand(xor_key);
    }
    
    // [...]
    return std::vector<std::byte>{xor_key.begin(), xor_key.end()};
}
```

In turn, this causes us to go through the XOR logic of [`AutoFile::detail_fread()`](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/streams.cpp#L10-L22) and [`AutoFile::write()`](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/streams.cpp#L66-L88) since the block autofiles' `std::vector<std::byte> m_xor` are [initialized](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1190) to the return value of `InitBlocksdirXorKey`.

E.g.: https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/streams.cpp#L10-L23

I have a branch off of `28.x` (https://github.com/davidgumberg/bitcoin/commit/aa0be4031cbe5bed407bc7a1753875cc1c4ddccb) that should fix this issue when `-blocksxor=0` but I haven't tested yet

I also managed to reproduce (n=1) the issue on Windows 10 LTSC Build 17763:

```bash
bitcoind.exe -dbcache=450 -stopatheight=300000 -debug=bench -connect=localnode:8333
```

28.0rc1 took 7,368 seconds
27.1 took 3,594 seconds
> This has uncovered a bug in https://github.com/bitcoin/bitcoin/pull/28052, InitBlocksdirXorKey does not return an empty std::vector<std::byte> when the -blocksxor=0 argument is used, it instead [returns](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1185) a [vector with 8 bytes equal to zero](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1152):

I may be missing something because I am looking at the blocksxor stuff the first time but that doesn't seem to be a bug. When XOR is disabled we still write the xor file with zeros. There is even a functional test for that behavior: https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_blocksxor.py#L64 Your branch seems to change that expected behavior.
Seems worth experimenting with:

https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src%2Fstreams.cpp#L76-L80

I wonder if the array creation, the copy, or the xoring here are the long pole in the tent? Experimenting with the all 0s xor key can help isolate.

I'll have to take a look, but do we have any IBD tests/benches mimicking a significant IBD (e.g. first 400k blocks)? It would be good to catch performence regressions automatically.

Something on [bitcoinperf.com](https://codespeed.bitcoinperf.com/comparison/)? (looks like maybe the data there is stale)

Or something like a functional test that has two nodes, one node creating a chain of similar complexity/size (if we don't want to store that part of mainnet), then the second node does IBD from first? Ci could run the test and be set to timeout for an excessively long IBD (which albeit is subjective so not ideal). Maybe it's better to leave CI out of this?
> > This has uncovered a bug in #28052, InitBlocksdirXorKey does not return an empty std::vectorstd::byte when the -blocksxor=0 argument is used, it instead [returns](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1185) a [vector with 8 bytes equal to zero](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1152):
> 
> I may be missing something because I am looking at the blocksxor stuff the first time but that doesn't seem to be a bug. When XOR is disabled we still write the xor file with zeros. There is even a functional test for that behavior: https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_blocksxor.py#L64 Your branch seems to change that expected behavior.

You're right, creating an xor.dat with all zeroes when `-blocksxor=0` was intended, so not a bug in that PR. Still, the fact that when a user disables xor'ing blocks we still perform all the usual XOR logic, but with an XOR pattern of zeroes doesn't make sense to me, and it seems like an unintended side effect of https://github.com/bitcoin/bitcoin/pull/28052 .

I tested https://github.com/davidgumberg/bitcoin/commit/aa0be4031cbe5bed407bc7a1753875cc1c4ddccb with `-blocksxor=0` and the same Windows setup and the performance regression goes away.

I'm still investigating, but my bench logs indicate that the regression is coming from writing block undo data, the difference between the (reported) total block connection time between v27.1 and v28.0rc1 is +1,337.85 seconds for v28, and the difference in (reported) time spent writing undo block data is +1,579.31s for v28:

<details>

<summary>Version 28.0 rc1 bench logs</summary>

```diff
 [bench]   - Using cached block
 [bench]   - Load block from disk: 0.57ms
 [bench]     - Sanity checks: 0.02ms [5.37s (0.02ms/blk)]
 [bench]     - Fork checks: 0.40ms [159.15s (0.53ms/blk)]
 [bench]       - Connect 512 transactions: 2.95ms (0.006ms/tx, 0.002ms/txin) [303.09s (1.01ms/blk)]
 [bench]     - Verify 1180 txins: 3.45ms (0.003ms/txin) [425.49s (1.42ms/blk)]
+[bench]     - Write undo data: 26.38ms [2296.37s (7.65ms/blk)]
 [bench]     - Index writing: 0.46ms [148.03s (0.49ms/blk)]
 [bench]   - Connect total: 31.98ms [3281.81s (10.94ms/blk)]
 [bench]   - Flush: 2.44ms [246.56s (0.82ms/blk)]
 [bench]   - Writing chainstate: 0.89ms [107.89s (0.36ms/blk)]
 UpdateTip: new best=000000000000000049a0914d83df36982c77ac1f65ade6a52bdced2ce312aba9 height=300001 version=0x00000002 log2_work=78.499665 tx=38464301 date='2014-05-10T06:51:23Z' progress=0.035833 cache=641.2MiB(5186028txo)
 [bench]   - Connect postprocess: 0.64ms [261.25s (0.87ms/blk)]
+[bench] - Connect block: 36.53ms [4041.91s (13.47ms/blk)]
```

</details>

<details>

<summary>Version 27.1 bench logs</summary>

```diff
 [bench]   - Using cached block
 [bench]   - Load block from disk: 1.03ms
 [bench]     - Sanity checks: 0.02ms [5.58s (0.02ms/blk)]
 [bench]     - Fork checks: 0.36ms [154.95s (0.52ms/blk)]
 [bench]       - Connect 173 transactions: 1.39ms (0.008ms/tx, 0.003ms/txin) [330.82s (1.10ms/blk)]
 [bench]     - Verify 435 txins: 2.02ms (0.005ms/txin) [478.57s (1.60ms/blk)]
+[bench]     - Write undo data: 1.24ms [717.06s (2.39ms/blk)]
 [bench]     - Index writing: 0.33ms [152.89s (0.51ms/blk)]
 [bench]   - Connect total: 5.11ms [1754.52s (5.85ms/blk)]
 [bench]   - Flush: 1.35ms [310.98s (1.04ms/blk)]
 [bench]   - Writing chainstate: 0.33ms [149.58s (0.50ms/blk)]
 UpdateTip: new best=000000000000000082aee4ff546c1db5e1aa5f9bfbaa0c76300a792b3e91fce7 height=300003 version=0x00000002 log2_work=78.499897 tx=38464563 date='2014-05-10T06:58:47Z' progress=0.035342 cache=510.6MiB(4668409txo)
 [bench]   - Connect postprocess: 0.86ms [356.88s (1.19ms/blk)]
+[bench] - Connect block: 8.68ms [2704.06s (9.01ms/blk)]
```

</details>

<details>

<summary>

My modified branch of 28.x with `-blocksxor=0` bench logs

</summary>

```diff
 [bench]   - Using cached block
 [bench]   - Load block from disk: 0.23ms
 [bench]     - Sanity checks: 0.01ms [5.35s (0.02ms/blk)]
 [bench]     - Fork checks: 0.29ms [150.64s (0.50ms/blk)]
 [bench]       - Connect 512 transactions: 2.18ms (0.004ms/tx, 0.002ms/txin) [308.30s (1.03ms/blk)]
 [bench]     - Verify 1180 txins: 2.58ms (0.002ms/txin) [448.44s (1.49ms/blk)]
+[bench]     - Write undo data: 1.62ms [758.14s (2.53ms/blk)]
 [bench]     - Index writing: 0.30ms [127.39s (0.42ms/blk)]
 [bench]   - Connect total: 5.72ms [1710.90s (5.70ms/blk)]
 [bench]   - Flush: 2.31ms [258.95s (0.86ms/blk)]
 [bench]   - Writing chainstate: 0.49ms [142.43s (0.47ms/blk)]
 UpdateTip: new best=000000000000000049a0914d83df36982c77ac1f65ade6a52bdced2ce312aba9 height=300001 version=0x00000002 log2_work=78.499665 tx=38464301 date='2014-05-10T06:51:23Z' progress=0.035817 cache=641.2MiB(5186028txo)
 [bench]   - Connect postprocess: 0.89ms [305.06s (1.02ms/blk)]
+[bench] - Connect block: 9.64ms [2531.72s (8.44ms/blk)]
```

</details>
@davidgumberg Well from the (apparently mistaken) assumption that the XOR logic comes with no material performance cost, it made sense to always want to use it (even with a pattern of zero), just from a perspective of minimizing different code branch combinations.

If the XOR logic is too slow (on some systems), as it appears to be, we have two options:
* Disabling it when (nonzero) XORing is not enabled, but that leaves us with the uncomfortable choice to make of (perhaps selectively) disabling (nonzero) XORing by default, as Windows users will otherwise still suffer with 28.0.
* Fixing the performance issue.
> My bench logs indicate that the regression is coming from writing block undo data, the difference between the (reported) total block connection time between v27.1 and v28.0rc1 is +1,337.85 seconds for v28, and the difference in (reported) time spent writing undo block data is +1,579.31s for v28:

This is saying that the total time for writing undo block data exceeds total block connection time. But, writing undo block data is a subsection of block connection time, so it should always be less no?

Also, the logs show `Using cached block`, meaning it is not counting saving the block itself to disk in the `Connect block` benchmark. So, actually saving the blocks in `AcceptBlock` might be taking even more time than writing the undo blocks.
@andrewtoth 
>> My bench logs indicate that the regression is coming from writing block undo data, the difference between the (reported) total block connection time between v27.1 and v28.0rc1 is +1,337.85 seconds for v28, and the difference in (reported) time spent writing undo block data is +1,579.31s for v28:
>
> This is saying that the total time for writing undo block data exceeds total block connection time. But, writing undo block data is a subsection of block connection time, so it should always be less no?

Sorry, I did not express that well, the two numbers I gave are the delta between v27.1 and v28.0rc1, total time spent writing undo block data is 2,296.37s in v28 and 717.06s in v27.1. Total time spent doing block connection is 4041.91s in v28.0rc1 and 2704.06s in v27.1.
 
Other parts of the v28 sync are faster (at least in my n=1 run~~s~~) , e.g. flushing the temporary `CCoinsViewCache` takes ~60 fewer seconds in v28 compensating for the increased time spent writing undo data.

> Also, the logs show Using cached block, meaning it is not counting saving the block itself to disk in the Connect block benchmark. So, actually saving the blocks in AcceptBlock might be taking even more time than writing the undo blocks.

I see, I guess that explains why the difference in wall clock time taken (v28 took 3,774 more seconds) is more than double the size of the slowdown reported by the ConnectBlock bench logs (v28 took 1,337.85 seconds longer doing block connection).

---

@tdb3
> Seems worth experimenting with:
> 
> https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src%2Fstreams.cpp#L76-L80
> 
> I wonder if the array creation, the copy, or the xoring here are the long pole in the tent? Experimenting with the all 0s xor key can help isolate.

It seems there was some prior discussion around this in #28060 which may be of interest:
https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1260260590

---

I wonder if this issue could be caused by number of calls we make to `fwrite` because of the write buffering in xor path of `AutoFile::write()`, maybe platforms other than windows are optimizing our writes into batches behind the scenes? I don't know much about how I/O works behind the scenes, just a guess.
> I wonder if this issue could be caused by number of calls we make to `fwrite` because of the write buffering in xor path of `AutoFile::write()`, maybe platforms other than windows are optimizing our writes into batches behind the scenes? I don't know much about how I/O works behind the scenes, just a guess.

I have not checked, but this would be my guess.
The xor patch didn't change buffering. Yes, in theory there is a 4096 byte max buffer, however, this is never hit in practice. Undo data consists of small integers (4 bytes or 1 byte) and short strings (like output scripts).

I expect that if you print the number of writes before and after, they are exactly the same. If you also print the size of the writes they should show a histogram where 1-byte writes are the most common (probably all varint), followed by various writes all shorter than 33 bytes (or whatever the largest output script is).

I don't have Windows, so I can't say what is happening there and why something is observable there, while the same isn't observable on Linux.

It may be useful to run the Xor benchmark on the same machine in Windows and on Linux, to see if the performance differs there: `./bench_bitcoin -filter=Xor -min-time=100`.

Also, it is possible to write a new benchmark, to see if the xor overhead is unexpectely large inside AutoFile:

```diff
diff --git a/src/bench/xor.cpp b/src/bench/xor.cpp
index fc9dc5d172..96cb952c3b 100644
--- a/src/bench/xor.cpp
+++ b/src/bench/xor.cpp
@@ -21,4 +21,19 @@ static void Xor(benchmark::Bench& bench)
     });
 }
 
+static void AutoFileXor(benchmark::Bench& bench)
+{
+    FastRandomContext frc{/*fDeterministic=*/true};
+    auto data{frc.randbytes<std::byte>(4'000'000)};
+    auto key{frc.randbytes<std::byte>(8)};
+
+    auto path{path_to_a_temp_file_on_the_storage_device_under_test};
+
+    bench.batch(data.size()).unit("byte").run([&] {
+        AutoFile f{fsbridge::fopen(path, "wb+"), key};
+        f << data;
+    });
+}
+
 BENCHMARK(Xor, benchmark::PriorityLevel::HIGH);
+BENCHMARK(AutoFileXor, benchmark::PriorityLevel::HIGH);
```

<!---

Generally, when providing an xor pattern to an `AutoFile` before using its `write` method, it is expected that the `write` is slower. This should hold on any platform and the relative slowdown should depend on the write speed of the underlying storage, as well as the CPU speed.
Apart from the benchmark, it may also be useful to create a flamegraph/heatmap, so that it is easy to see where the time is spent.
I managed to get bitcoind compiled on my windows installation (Windows 10 Pro 22H2, MSVC 2022), didn't observe a notable difference in the benchmarks.
I'm still in the process of trying to test the sync speed, but if it's due to the undo data, then a reindex-chainstate should be sufficient to see the difference?! Did anyone try that already?
> I managed to get bitcoind compiled on my windows installation (Windows 10 Pro 22H2, MSVC 2022), didn't observe a notable difference in the benchmarks.

I hope everyone measuring performance on Windows has disabled all antivirus software, at least for the data directory. Right?
@hebasto Yes, I have disabled antivirus for both the data directory and the directory where the binaries are located, I checked that the antivirus CPU utilization stays low during the benchmarks, and I'm using identical directory locations for both versions so they wouldn't be treated differently by the antivirus anyway.
Discussion from today's IRC meeting is that we should move forward with @davidgumberg's patch and default to false on Windows for this release.

Fix patch:
diff --git a/src/addrdb.cpp b/src/addrdb.cpp
index e9838d722291e..b89141c88e324 100644
--- a/src/addrdb.cpp
+++ b/src/addrdb.cpp
@@ -73,7 +73,7 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data
         remove(pathTmp);
         return false;
     }
-    if (!FileCommit(fileout.Get())) {
+    if (!fileout.Commit()) {
         fileout.fclose();
         remove(pathTmp);
         LogError("%s: Failed to flush file %s\n", __func__, fs::PathToString(pathTmp));
diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp
index 5098262e9afa6..004bf8ffc9dc7 100644
--- a/src/bench/streams_findbyte.cpp
+++ b/src/bench/streams_findbyte.cpp
@@ -19,7 +19,7 @@ static void FindByte(benchmark::Bench& bench)
     uint8_t data[file_size] = {0};
     data[file_size-1] = 1;
     file << data;
-    std::rewind(file.Get());
+    file.seek(0, SEEK_SET);
     BufferedFile bf{file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size};
 
     bench.run([&] {
diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp
index 41bdca9df562a..26de7eee32fc0 100644
--- a/src/index/blockfilterindex.cpp
+++ b/src/index/blockfilterindex.cpp
@@ -151,7 +151,7 @@ bool BlockFilterIndex::CustomCommit(CDBBatch& batch)
         LogError("%s: Failed to open filter file %d\n", __func__, pos.nFile);
         return false;
     }
-    if (!FileCommit(file.Get())) {
+    if (!file.Commit()) {
         LogError("%s: Failed to commit filter file %d\n", __func__, pos.nFile);
         return false;
     }
@@ -201,11 +201,11 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter&
             LogPrintf("%s: Failed to open filter file %d\n", __func__, pos.nFile);
             return 0;
         }
-        if (!TruncateFile(last_file.Get(), pos.nPos)) {
+        if (!last_file.Truncate(pos.nPos)) {
             LogPrintf("%s: Failed to truncate filter file %d\n", __func__, pos.nFile);
             return 0;
         }
-        if (!FileCommit(last_file.Get())) {
+        if (!last_file.Commit()) {
             LogPrintf("%s: Failed to commit filter file %d\n", __func__, pos.nFile);
             return 0;
         }
diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp
index 80f615ed0e77b..425a7f00a0d08 100644
--- a/src/index/txindex.cpp
+++ b/src/index/txindex.cpp
@@ -87,10 +87,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe
     CBlockHeader header;
     try {
         file >> header;
-        if (fseek(file.Get(), postx.nTxOffset, SEEK_CUR)) {
-            LogError("%s: fseek(...) failed\n", __func__);
-            return false;
-        }
+        file.seek(postx.nTxOffset, SEEK_CUR);
         file >> TX_WITH_WITNESS(tx);
     } catch (const std::exception& e) {
         LogError("%s: Deserialize or I/O error - %s\n", __func__, e.what());
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index b40ca0260beee..44c2808c3bd46 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -683,7 +683,7 @@ bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos
     fileout << GetParams().MessageStart() << nSize;
 
     // Write undo data
-    long fileOutPos = ftell(fileout.Get());
+    long fileOutPos = fileout.tell();
     if (fileOutPos < 0) {
         LogError("%s: ftell failed\n", __func__);
         return false;
@@ -981,7 +981,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const
     fileout << GetParams().MessageStart() << nSize;
 
     // Write block
-    long fileOutPos = ftell(fileout.Get());
+    long fileOutPos = fileout.tell();
     if (fileOutPos < 0) {
         LogError("%s: ftell failed\n", __func__);
         return false;
diff --git a/src/node/mempool_persist.cpp b/src/node/mempool_persist.cpp
index a265c2e12d15a..ff7de8c64a259 100644
--- a/src/node/mempool_persist.cpp
+++ b/src/node/mempool_persist.cpp
@@ -199,8 +199,8 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock
         LogInfo("Writing %d unbroadcast transactions to file.\n", unbroadcast_txids.size());
         file << unbroadcast_txids;
 
-        if (!skip_file_commit && !FileCommit(file.Get()))
-            throw std::runtime_error("FileCommit failed");
+        if (!skip_file_commit && !file.Commit())
+            throw std::runtime_error("Commit failed");
         file.fclose();
         if (!RenameOver(dump_path + ".new", dump_path)) {
             throw std::runtime_error("Rename failed");
diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp
index 976421e455391..7d589c886b824 100644
--- a/src/node/utxo_snapshot.cpp
+++ b/src/node/utxo_snapshot.cpp
@@ -73,9 +73,11 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir)
     }
     afile >> base_blockhash;
 
-    if (std::fgetc(afile.Get()) != EOF) {
+    int64_t position = afile.tell();
+    afile.seek(0, SEEK_END);
+    if (position != afile.tell()) {
         LogPrintf("[snapshot] warning: unexpected trailing data in %s\n", read_from_str);
-    } else if (std::ferror(afile.Get())) {
+    } else if (afile.IsError()) {
         LogPrintf("[snapshot] warning: i/o error reading %s\n", read_from_str);
     }
     return base_blockhash;
diff --git a/src/streams.cpp b/src/streams.cpp
index cdd36a86feb17..5f7baf92b9e8e 100644
--- a/src/streams.cpp
+++ b/src/streams.cpp
@@ -4,21 +4,29 @@
 
 #include <span.h>
 #include <streams.h>
+#include <util/fs_helpers.h>
 
 #include <array>
 
+AutoFile::AutoFile(std::FILE* file, std::vector<std::byte> data_xor)
+    : m_file{file}, m_xor{std::move(data_xor)}
+{
+    if (!IsNull()) {
+        auto pos{std::ftell(m_file)};
+        if (pos >= 0) m_position = pos;
+    }
+}
+
 std::size_t AutoFile::detail_fread(Span<std::byte> dst)
 {
     if (!m_file) throw std::ios_base::failure("AutoFile::read: file handle is nullptr");
-    if (m_xor.empty()) {
-        return std::fread(dst.data(), 1, dst.size(), m_file);
-    } else {
-        const auto init_pos{std::ftell(m_file)};
-        if (init_pos < 0) throw std::ios_base::failure("AutoFile::read: ftell failed");
-        std::size_t ret{std::fread(dst.data(), 1, dst.size(), m_file)};
-        util::Xor(dst.subspan(0, ret), m_xor, init_pos);
-        return ret;
+    size_t ret = std::fread(dst.data(), 1, dst.size(), m_file);
+    if (!m_xor.empty()) {
+        if (!m_position.has_value()) throw std::ios_base::failure("AutoFile::read: position unknown");
+        util::Xor(dst.subspan(0, ret), m_xor, *m_position);
     }
+    if (m_position.has_value()) *m_position += ret;
+    return ret;
 }
 
 void AutoFile::seek(int64_t offset, int origin)
@@ -29,18 +37,23 @@ void AutoFile::seek(int64_t offset, int origin)
     if (std::fseek(m_file, offset, origin) != 0) {
         throw std::ios_base::failure(feof() ? "AutoFile::seek: end of file" : "AutoFile::seek: fseek failed");
     }
+    if (origin == SEEK_SET) {
+        m_position = offset;
+    } else if (origin == SEEK_CUR && m_position.has_value()) {
+        *m_position += offset;
+    } else {
+        int64_t r{std::ftell(m_file)};
+        if (r < 0) {
+            throw std::ios_base::failure("AutoFile::seek: ftell failed");
+        }
+        m_position = r;
+    }
 }
 
 int64_t AutoFile::tell()
 {
-    if (IsNull()) {
-        throw std::ios_base::failure("AutoFile::tell: file handle is nullptr");
-    }
-    int64_t r{std::ftell(m_file)};
-    if (r < 0) {
-        throw std::ios_base::failure("AutoFile::tell: ftell failed");
-    }
-    return r;
+    if (!m_position.has_value()) throw std::ios_base::failure("AutoFile::tell: position unknown");
+    return *m_position;
 }
 
 void AutoFile::read(Span<std::byte> dst)
@@ -60,6 +73,7 @@ void AutoFile::ignore(size_t nSize)
             throw std::ios_base::failure(feof() ? "AutoFile::ignore: end of file" : "AutoFile::ignore: fread failed");
         }
         nSize -= nNow;
+        if (m_position.has_value()) *m_position += nNow;
     }
 }
 
@@ -70,19 +84,34 @@ void AutoFile::write(Span<const std::byte> src)
         if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) {
             throw std::ios_base::failure("AutoFile::write: write failed");
         }
+        if (m_position.has_value()) *m_position += src.size();
     } else {
-        auto current_pos{std::ftell(m_file)};
-        if (current_pos < 0) throw std::ios_base::failure("AutoFile::write: ftell failed");
+        if (!m_position.has_value()) throw std::ios_base::failure("AutoFile::write: position unknown");
         std::array<std::byte, 4096> buf;
         while (src.size() > 0) {
             auto buf_now{Span{buf}.first(std::min<size_t>(src.size(), buf.size()))};
             std::copy(src.begin(), src.begin() + buf_now.size(), buf_now.begin());
-            util::Xor(buf_now, m_xor, current_pos);
+            util::Xor(buf_now, m_xor, *m_position);
             if (std::fwrite(buf_now.data(), 1, buf_now.size(), m_file) != buf_now.size()) {
                 throw std::ios_base::failure{"XorFile::write: failed"};
             }
             src = src.subspan(buf_now.size());
-            current_pos += buf_now.size();
+            *m_position += buf_now.size();
         }
     }
 }
+
+bool AutoFile::Commit()
+{
+    return ::FileCommit(m_file);
+}
+
+bool AutoFile::IsError()
+{
+    return ferror(m_file);
+}
+
+bool AutoFile::Truncate(unsigned size)
+{
+    return ::TruncateFile(m_file, size);
+}
diff --git a/src/streams.h b/src/streams.h
index c2a9dea2878d1..431a4d77c6ced 100644
--- a/src/streams.h
+++ b/src/streams.h
@@ -390,9 +390,10 @@ class AutoFile
 protected:
     std::FILE* m_file;
     std::vector<std::byte> m_xor;
+    std::optional<int64_t> m_position;
 
 public:
-    explicit AutoFile(std::FILE* file, std::vector<std::byte> data_xor={}) : m_file{file}, m_xor{std::move(data_xor)} {}
+    explicit AutoFile(std::FILE* file, std::vector<std::byte> data_xor={});
 
     ~AutoFile() { fclose(); }
 
@@ -419,12 +420,6 @@ class AutoFile
         return ret;
     }
 
-    /** Get wrapped FILE* without transfer of ownership.
-     * @note Ownership of the FILE* will remain with this class. Use this only if the scope of the
-     * AutoFile outlives use of the passed pointer.
-     */
-    std::FILE* Get() const { return m_file; }
-
     /** Return true if the wrapped FILE* is nullptr, false otherwise.
      */
     bool IsNull() const { return m_file == nullptr; }
@@ -458,6 +453,10 @@ class AutoFile
         ::Unserialize(*this, obj);
         return *this;
     }
+
+    bool Commit();
+    bool IsError();
+    bool Truncate(unsigned size);
 };
 
 /** Wrapper around an AutoFile& that implements a ring buffer to
diff --git a/src/util/asmap.cpp b/src/util/asmap.cpp
index f50cd8a28c12d..04b0673c49b42 100644
--- a/src/util/asmap.cpp
+++ b/src/util/asmap.cpp
@@ -203,10 +203,10 @@ std::vector<bool> DecodeAsmap(fs::path path)
         LogPrintf("Failed to open asmap file from disk\n");
         return bits;
     }
-    fseek(filestr, 0, SEEK_END);
-    int length = ftell(filestr);
+    file.seek(0, SEEK_END);
+    int length = file.tell();
     LogPrintf("Opened asmap file %s (%d bytes) from disk\n", fs::quoted(fs::PathToString(path)), length);
-    fseek(filestr, 0, SEEK_SET);
+    file.seek(0, SEEK_SET);
     uint8_t cur_byte;
     for (int i = 0; i < length; ++i) {
         file >> cur_byte;


Test patch:
diff --git a/src/test/fuzz/autofile.cpp b/src/test/fuzz/autofile.cpp
index 45316b6b218db..81761c7bf9694 100644
--- a/src/test/fuzz/autofile.cpp
+++ b/src/test/fuzz/autofile.cpp
@@ -56,7 +56,6 @@ FUZZ_TARGET(autofile)
                 WriteToStream(fuzzed_data_provider, auto_file);
             });
     }
-    (void)auto_file.Get();
     (void)auto_file.IsNull();
     if (fuzzed_data_provider.ConsumeBool()) {
         FILE* f = auto_file.release();
diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp
index 9217f05945f0c..777122df6d03a 100644
--- a/src/test/streams_tests.cpp
+++ b/src/test/streams_tests.cpp
@@ -261,7 +261,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
     for (uint8_t j = 0; j < 40; ++j) {
         file << j;
     }
-    std::rewind(file.Get());
+    file.seek(0, SEEK_SET);
 
     // The buffer size (second arg) must be greater than the rewind
     // amount (third arg).
@@ -391,7 +391,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
     for (uint8_t j = 0; j < 40; ++j) {
         file << j;
     }
-    std::rewind(file.Get());
+    file.seek(0, SEEK_SET);
 
     // The buffer is 25 bytes, allow rewinding 10 bytes.
     BufferedFile bf{file, 25, 10};
@@ -444,7 +444,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
         for (uint8_t i = 0; i < fileSize; ++i) {
             file << i;
         }
-        std::rewind(file.Get());
+        file.seek(0, SEEK_SET);
 
         size_t bufSize = m_rng.randrange(300) + 1;
         size_t rewindSize = m_rng.randrange(bufSize);