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

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1790442628)
correct_tx appears in both for the txid-indexed version

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2352323652)
li

💬 ismaelsadeeq commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790464732)
nit: can go further with the explanation the comments with something like

```suggestion
* i's ancestors (unless i is part of a cycle of dependencies). Note that DepGraph does not
* store set of parents; it has to be reduced from ancestor set.
```

for both the parent and child reducing methods.

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2397283288)
> threadripper

Nice. Any machine with at least 96 CPU threads, or two machines with at least 48 cores should be a good start. (Less cores would be fine as well, but require the remote ccache, which could break. Generally, if there is something that can break, it will break, given enough time)

> (which should have local-ish mirrors for apt and etc)

`apt` should be cached in the docker image and doesn't take more than 30 seconds on a cache miss, so should be fine. Also, inside docker, any
...