👍 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

💬 theuni commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2397251484)
I could also possibly supply a beefy dedicated threadripper machine in an MIT datacenter (which should have local-ish mirrors for apt and etc). When comparing costs, I think it makes sense to think of buying hardware vs renting as well.

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2351684992)
first pass light code review ACK 2b21aebd9754c503bac12d1dbf566b3aa27451e8

Mostly trying to understand what the code is doing!


comments are just nits

💬 ismaelsadeeq commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790355464)
nit maybe calling `mapping` a container will be better.
```suggestion
*@param mapping A container, such that mapping[i] gives the position in the new DepGraph
```

💬 ismaelsadeeq commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790049084)
nit: This loop can be modified to be more readable by using using descriptive variable names and some helpful comment on what the inner loops are doing.
```suggestion
for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) {
// Initialize the set of ancestors with just the current transaction itself.
SetType ancestor_union = SetType::Singleton(i);
// Iteratively add parents and parents of parents to the ancestor set until there is no more ancestor t
...

📝 fanquake opened a pull request: "test: remove unused code from `script_tests`"
(https://github.com/bitcoin/bitcoin/pull/31051)
This has been unused since #29648. Noticed while running a newer version of clang-tidy (19.1.1):
```bash
[127/391][6.2s] /opt/homebrew/opt/llvm/bin/clang-tidy -p=build -quiet --config-file=/bitcoin/src/.clang-tidy /bitcoin/src/test/script_tests.cpp
bitcoin/src/test/script_tests.cpp:126:25: error: local copy 'tx2' of the variable 'tx' is never modified and never used; consider removing the statement [performance-unnecessary-copy-initialization,-warnings-as-errors]
126 | CMutableTransact
...

🤔 instagibbs reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2352291145)
Remaining (non nitty) things I think to be addressed:
1) https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391768053
2) https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1787010637

`git range-diff master ce205abe10ebccfdbea60adf4c568a8ba61390c3 ff1f2dc055efe92b3202387c66a12948134eced2`