💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1790246276)
Good catch! Just to make sure I ran it 100 times and on the 90th attempt it failed, I removed the assert in [33e4dc8](https://github.com/bitcoin/bitcoin/pull/31040/commits/33e4dc834f433a51d5a4a8ffa7ce81a98ac35b7a)

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

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2396973132)
`54f4f3b05e...f9b2eaf96c`: rebase due to conflicts

💬 ismaelsadeeq commented on pull request "Fix unsigned integer overflows in interpreter":
(https://github.com/bitcoin/bitcoin/pull/24214#issuecomment-2397020325)
> Starting with C++11, `int64_t{something}` is actually preferred, because it documents (and checks at compile time) that no narrowing happens. `int64_t(something)` documents that the reviewer will have to check for possible narrowing.

Agreed I am talking about changing `size_t(int64_t(stack.size())` to `static_cast<size_t>(static_cast<int64_t>(stack.size())` same for `altstack` ?

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