💬 Sjors commented on pull request "Move maximum timewarp attack threshold back to 600s from 7200s":
(https://github.com/bitcoin/bitcoin/pull/30647#issuecomment-2291734952)
Here's a branch on top of this PR that add a test for the timewarp attack, and also fixes the miner.

Maybe for this PR we should only take the fix and leave the test for later, because it's quite slow.

https://github.com/bitcoin/bitcoin/compare/master...Sjors:bitcoin:2024/08/conervative-timewarp

💬 achow101 commented on pull request "wallet: fix UnloadWallet thread safety assumptions":
(https://github.com/bitcoin/bitcoin/pull/30659#issuecomment-2291745717)
ACK f550a8e035b4603787273ea250f403f6f0be453f

💬 sipa commented on pull request "3 more security enhancements":
(https://github.com/bitcoin/bitcoin/pull/30663#discussion_r1718742563)
This is true, but in this case actually irrelevant. Since every constructor of the class initializes all the fields, the change here literally has no effect; the initializers for fields are just defaults that get used when a constructor doesn't contain an explicit initialization.

💬 glozow commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1718748117)
[logs_30657_2.txt](https://github.com/user-attachments/files/16627939/logs_30657_2.txt)

Ah interesting, `fs::is_empty(opts.blocks_dir)` is 0

🚀 achow101 merged a pull request: "wallet: fix UnloadWallet thread safety assumptions"
(https://github.com/bitcoin/bitcoin/pull/30659)

💬 maflcko commented on pull request "Move maximum timewarp attack threshold back to 600s from 7200s":
(https://github.com/bitcoin/bitcoin/pull/30647#issuecomment-2291808999)
> because it's quite slow

Possible ideas to speed it up could be to use the "real" testnet4 blocks for the first interval, similar to `./test/functional/data/blockheader_testnet3.hex`. If they are too large, one could "pre-mine" the first interval and recreate it deterministically in the test on each run, which should also be fast.

🤔 murchandamus reviewed a pull request: "Move maximum timewarp attack threshold back to 600s from 7200s"
(https://github.com/bitcoin/bitcoin/pull/30647#pullrequestreview-2240965068)
crACK 16e95bda86302af20cfb314a2c0252256d01f750

💬 maflcko commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1718764031)
Fascinating. What is your compiler, stdlib version, OS and filesystem for `/tmp/` and `./`?

Does the issue happen when you create a non-`/tmp/` folder? (I presume `/tmp/` is a ramdisk?)

```
mkdir no_ramdisk_temp_prefix
./test/functional/test_runner.py --tmpdirprefix ./no_ramdisk_temp_prefix/ feature_blocksxor

💬 maflcko commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1718767576)
Also, what is `ls --all /tmp/bitcoin_func_test_fj812gkv/node0/regtest/blocks`

💬 maflcko commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1718769752)
Is it Apple https://en.wikipedia.org/wiki/.DS_Store ?

💬 glozow commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1718786049)
It's jammy with clang 17, indeed it works with a non-`/tmp` folder.

💬 maflcko commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1718816229)
Jammy doesn't have clang 17, so I guess it is installed from the nightly llvm apt and then `CC=clang-17 CXX=clang++-17 ../configure` (using the libstdc++-11, not libc++)?

🤔 maflcko reviewed a pull request: "test: [refactor] Use g_rng/m_rng directly"
(https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2241150302)
Sure, pushed your branch. A skim looks good. I left a nit and will do a real review later.

💬 maflcko commented on pull request "test: [refactor] Use g_rng/m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1718879706)
nit in bb67ca09c88a46b99bf83e937fa761f0e25cba37: Not sure about this change. It seems overly restrictive to require a `FastRandomContext&` here, when a caller may not want to pass anything at all, or a different rng.

I think it would be better to drop this change.

💬 TheCharlatan commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#discussion_r1718552423)
Nit: I find these callbacks a bit ugly, how about:
```diff
diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp
index 0b4e0d183a..a30ac53f97 100644
--- a/src/test/fuzz/utxo_snapshot.cpp
+++ b/src/test/fuzz/utxo_snapshot.cpp
@@ -54,0 +55 @@ void initialize_chain()
+ .min_validation_cache = true,
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index cf47d16faf..62ff61b227 100644
--- a/src
...

👍 TheCharlatan approved a pull request: "fuzz: Faster utxo_snapshot fuzz target"
(https://github.com/bitcoin/bitcoin/pull/30644#pullrequestreview-2240627537)
ACK fa69afd51879f8441df7a51144318a49e7dd27b9

The logic is a bit convoluted, but I don't have many suggestions for improving it and I think it is important to get this performance improvement deployed quickly.

💬 TheCharlatan commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#discussion_r1718893373)
Nit: The includes need to be corrected again.

💬 mzumsande commented on pull request "kernel: pre-28.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1718921534)
I see similar size to @Sjors, as of height=2873716:
```
du -sh ~/.bitcoin/testnet3/chainstate/ ~/.bitcoin/testnet3/blocks/
13G /home/martin/.bitcoin/testnet3/chainstate/
82G /home/martin/.bitcoin/testnet3/blocks/
```
(I wasn't continuously online during the fork shenanigans, so having fewer blocks makes sense).

📝 hebasto opened a pull request: "CMake replaces Autotools"
(https://github.com/bitcoin/bitcoin/pull/30664)
This PR is based on https://github.com/bitcoin/bitcoin/pull/30454 with additional commits that delete both the Autotools-based build system and the legacy MSVC build system.

It aims reveal conflicts with other PRs.

For more details, please refer to today's IRC meeting discussion.

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1718993291)
fixed