💬 maflcko commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2397035722)
You'll also have to remove `doc/build-osx.md:54:For macOS 11 (Big Sur) and 12 (Monterey) you need to install a more recent version of llvm.`, no?

💬 hebasto commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2397047130)
> You'll also have to remove `doc/build-osx.md:54:For macOS 11 (Big Sur) and 12 (Monterey) you need to install a more recent version of llvm.`, no?

Thanks! Updated.

💬 maflcko commented on pull request "Fix unsigned integer overflows in interpreter":
(https://github.com/bitcoin/bitcoin/pull/24214#issuecomment-2397050476)
I am happy to change code, if there is a reason. However, the dev notes recommend this style, so for now I will not address style nits one way or another. If you want to change or discuss the dev notes, a separate issue or pull request may be better.

💬 maflcko commented on pull request "fuzz: Add fuzz-only build mode option for targets":
(https://github.com/bitcoin/bitcoin/pull/31028#discussion_r1790324492)
style nit: Seems fine, but I preferred the previous approach where all fuzz targets are registered with the same macro. This should be possible by guarding the `FuzzFrameworkRegisterTarget` behind a `if constexpr (opts.require_build_for_fuzzing and not build_for_fuzzing)`.


This should also make https://github.com/bitcoin/bitcoin/pull/30882 easier to implement, because it doesn't have to deal with two macros.

💬 maflcko commented on issue "build: macOS fuzz instructions broken using latest macOS linker":
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2397086472)
Does it also happen with `llvm@16`, instead of the current version?

📝 marcofleon converted_to_draft a pull request: "fuzz: Add fuzz-only build mode option for targets"
(https://github.com/bitcoin/bitcoin/pull/31028)
Addresses https://github.com/bitcoin/bitcoin/issues/30950.

Any targets that require BUILD_ON_FUZZING=ON (currently only `p2p_headers_presync`) to work properly can now set `require_build_for_fuzzing` as an option. If BUILD_FOR_FUZZING is not on and you try to run a target that has this option, then there's a message printed upon exit.

With this change, the CI will be able to run the fuzz test runner without any timeouts/failures.

💬 maflcko commented on issue "Crash upon RPC v1 connection in v28.0.0":
(https://github.com/bitcoin/bitcoin/issues/31041#issuecomment-2397104680)
I see. It would be good to know what the batch request is looking like. Also, it would be good to confirm that this works on 27.x and is broken on 28.x. Also, it would be good to confirm that the system has enough memory to fit the whole batch request response (as json) into memory.

👍 theuni approved a pull request: "ci: Add missing -DWERROR=ON to test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/31045#pullrequestreview-2352144845)
utACK fa1cffacae61264ac89e30a069ba093495cb3216.

Nice catch on the Werror.

A quick search confirms that nproc and hw.logicalcpu should be the same value.

💬 dergoegge commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2397117284)
`-v2transport=0` should be added to the bitcoind args used for netdriver fuzzing. Otherwise, it doesn't seem like honggfuzz ever fuzzes anything besides the v2 handshake.

Also playing around with this and reading the [netdriver post](https://blog.swiecki.net/2018/01/fuzzing-tcp-servers.html) (i couldn't find any other docs on it) I'm not sure if this will ever even get past the version handshake? So I'm wondering how useful this tool really is for us.

💬 fanquake commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#issuecomment-2397119816)
Looks like this is working: https://cirrus-ci.com/task/5848920612405248?logs=ci#L2234
```bash
[14:18:46.371] 126/135 Test #130: spend_tests .......................... Passed 19.83 sec
[14:19:14.504] 127/135 Test #110: txvalidationcache_tests .............. Passed 122.32 sec
[14:19:15.437] 128/135 Test #8: bench_sanity_check_high_priority ..... Passed 318.61 sec
[14:19:24.864] 129/135 Test #76: random_tests ......................... Passed 208.03 sec
[14:19:25.959] 130/135 T
...

💬 fanquake commented on issue "build: macOS fuzz instructions broken using latest macOS linker":
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2397123023)
It works with `llvm@18`, so it looks like this could be an incompatibility with LLVM 19 (19.1.1) and Apples `ld`.

💬 maflcko commented on pull request "ci: Add missing -DWERROR=ON to test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2397143969)
The CI logs:

* Before https://github.com/bitcoin/bitcoin/actions/runs/11091169274/job/30814507260#step:6:353:

```
Treat compiler warnings as errors ..... OFF
```

* This pull https://github.com/bitcoin/bitcoin/actions/runs/11213662779/job/31167158663?pr=31045#step:6:350:

```
Treat compiler warnings as errors ..... ON

💬 theuni commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2397158180)
Concept ACK.

💬 maflcko commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#issuecomment-2397181620)
> Looks like this is working: https://cirrus-ci.com/task/5848920612405248?logs=ci#L2234

Though, this is msan, which seems just like a slow machine, because out of 10 runs, it sometimes takes longer: https://cirrus-ci.com/task/6180976303276032?logs=ci#L2223

Once https://github.com/bitcoin/bitcoin/issues/30852 is fixed, the results should be hopefully both faster and more consistent.

⚠️ Sjors opened an issue: "macOS 13.7 depends build can't find qt"
(https://github.com/bitcoin/bitcoin/issues/31050)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
cd depends
make NO_BDB=1
...
copying packages: boost libevent qt qrencode sqlite miniupnpc zeromq
to: /Volumes/SSD/Dev/bitcoin/depends/x86_64-apple-darwin22.6.0

cd ..
cmake -B build --toolchain depends/x86_64-apple-darwin22.6.0/toolchain.cmake
```

This fails, see below

(building BDB hasn't work for me for a while, but that's going away anyway)

### Expected behaviour

T
...

💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2397189214)
Will investigate and provide more details later.

🤔 instagibbs reviewed a pull request: "rpc: getorphantxs follow-up"
(https://github.com/bitcoin/bitcoin/pull/31043#pullrequestreview-2352223628)
> Pushed to https://github.com/bitcoin/bitcoin/commit/ae17f9050e0c2a6fd31cd9f5d2f3ad8066f02cc9 to replace assert_approx with assert_equal (using setmocktime)

good call, approx always sets off alarm bells as a test reader :)

we have a chance to outright reject verbosity>2 for extensibility before a release, but I can't think of what other things you'd possible want that can't be given in the 3 modes already included.

concept ACK

💬 instagibbs commented on pull request "rpc: getorphantxs follow-up":
(https://github.com/bitcoin/bitcoin/pull/31043#discussion_r1790399506)
let's name it the same as in the CPP code? `ORPHAN_TX_EXPIRE_TIME`

💬 theuni commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2397218483)
> Ninja is required to build the `qt` package in depends. It is mentioned in `depends/README.md`. However, `doc/build-osx.md` does not cover building with depends, so I don't think it is necessary to mention Ninja there.

Could you please explain why this is the case? Ninja seems like an odd requirement here.

💬 l0rinc commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2397229720)
Finished benchmarking with the default 2 mb file size vs 4, 8, 64 and 128 mb.
This time it's full IBD with real peers until 800k blocks on a HDD.

<img src="https://github.com/user-attachments/assets/17d1bb75-c85c-4f82-82f7-d4028bbcb88a">

<details>
<summary>Benchmarks</summary>

```bash
hyperfine --runs 1 --export-json /mnt/ibd_DBFILESIZE.json --parameter-list DBFILESIZE 2,4,8,64,128 --prepare 'rm -rf /mnt/BitcoinData/*' './build/src/bitcoind -datadir=/mnt/BitcoinData -stopat
...

💬 maflcko commented on pull request "fuzz: Add fuzz-only build mode option for targets":
(https://github.com/bitcoin/bitcoin/pull/31028#discussion_r1790444004)
This may also work around the CI false positive