💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1762668345)
02c6a1ed58867fa14b1e0c42f476ef45e64ea7ea `-> (result: BlockTemplate);`

🚀 fanquake merged a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438)

💬 maflcko commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2354922939)
> ... though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.

I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.

💬 hebasto commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1762679548)
A note for reviewers. The Guix builds are using CMake 3.24.2 (the `cmake-minimal` package).

🤔 hebasto reviewed a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308880257)
Concept ACK.

Perhaps explicitly mention the switch from the `x86_64` architecture to `arm64`?

👋 fanquake's pull request is ready for review: "depends: Qt 5.15.15"
(https://github.com/bitcoin/bitcoin/pull/30774)

💬 fanquake commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2355041535)
> but wasn't the idea to use qt6 once cmake was done? IIRC there was a tracking/meta issue, but I can't find it?

Maybe #24798? It's not really clear what the status of any migration is.

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2355045583)
> So I think the best way forward would be to have all workers of the same type on the same machine. (I am still running some performance tests to get an estimate if this make sense at all)

Did a quick sanity check with a 100G shared ccache on a single machine `cpx51` and all tasks finished in under a hour. Some tasks were faster than the AX52 run above, some were slower:

![Screenshot 2024-09-17 at 11-11-05 Merge commit 'fa99e4521b6fc0e7f6636d40bc0d6a7325227374' into main-with-ci - Cirrus
...

💬 maflcko commented on pull request "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)":
(https://github.com/bitcoin/bitcoin/pull/30913#issuecomment-2355068096)
> Perhaps explicitly mention the switch from the `x86_64` architecture to `arm64`?

Sure, done

💬 dergoegge commented on pull request "doc: update Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2355075262)
I think we could even remove eclipser from the docs entirely. As you noted as well, it looks unmaintained and I don't think any of us are actively using it.

That being said it would be nice to have an example of a maintained concolic testing tool in the docs (e.g. https://github.com/eurecom-s3/symqemu) but that could be done as a follow up.

💬 laanwj commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#discussion_r1762896709)
> Is there any reason why AutoFile::ignore(nSize) couldn't be removed and AutoFile::seek(nSize, SEEK_CURR) used instead?

Keep in mind not all streams support seeking, so it would at the least be a conditional optimization. Agree it's not worth it as long as the amounts skipped are small (generally smaller or equal to the page size).

💬 laanwj commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2355119437)
Code review ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35

💬 maflcko commented on pull request "doc: update Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2355137275)
> That being said it would be nice to have an example of a maintained concolic testing tool in the docs (e.g. https://github.com/eurecom-s3/symqemu) but that could be done as a follow up.

Agree. Also, it would be nice to (at the same time) add a synthetic bug (or prior real bug) that shows how the tool works and shows that (in this specific instance at least) it provides real value over just afl+libFuzzer+honggfuzz, using a saturated or at least well-developed corpus based on the existing dev
...

💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2355149293)
Ran `txdownloadman_impl` and it crashed on this assertion:
```
operator(): Assertion `!txdownload_impl.RecentRejectsFilter().contains(package.front()->GetWitnessHash().ToUint256())' failed.
```
Base64 of the input: 4d3Myytlcf9a3czLK2UxoFrjzMtlMQBSWt3Myyv//w==

Input file if it's any easier:
[txdownload_impl_crash.txt](https://github.com/user-attachments/files/17026573/txdownload_impl_crash.txt)

🤔 vasild reviewed a pull request: "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)"
(https://github.com/bitcoin-core/gui/pull/836#pullrequestreview-2309063950)
The changes to `src/qt/optionsmodel.cpp` look ok to me.

💬 vasild commented on pull request "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)":
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1762813848)
I think that we are not supposed to access the global state from here, @hebasto?

Also, this change is mixing the "reachable nets" with the "which network is accessed via which proxy" which are two different things. Maybe drop this change altogether?

Since we are here, I have been thinking that those text descriptions are confusing:
"Used for reaching peers via [ ] IPv4 [ ] IPv6 [ ] Tor"
"Use separte SOCKS5 proxy to reach peers via Tor onion services".
It is not that we reach peers **vi
...

👍 theStack approved a pull request: "scripted-diff: Modernize nLocalServices naming"
(https://github.com/bitcoin/bitcoin/pull/30885#pullrequestreview-2309344094)
ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca

💬 pablomartin4btc commented on pull request "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)":
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1763010650)
> Also, this change is mixing the "reachable nets" with the "which network is accessed via which proxy" which are two different things. Maybe drop this change altogether?

I get it, that's correct, but also when `-onlynet` is specified, g_reachable_nets contains only the nets that are specified by each `-onlynet` option (ie so in this case `-onlynet=ipv6`, ipv4 peers won't be reached by the proxy):

https://github.com/bitcoin-core/gui/blob/225718eda89d65a7041c33e1994d3bf7bd71bbdd/src/init.c
...

💬 nanlour commented on issue "test: WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=32090)":
(https://github.com/bitcoin/bitcoin/issues/30764#issuecomment-2355330277)
This looks like a false positive. ThreadSanitizer incorrectly counts cs_wallet and cs_args as the same lock because they are mapped to the same memory address. I tried adding padding to distribute the memory mapping, as done in commit c551ea17c93f59abb2df2f8a66e381973291a5e3, and it successfully silenced the warning.

👍 hebasto approved a pull request: "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2309467718)
ACK fab932b4211ea0a4c10e484c08042552f9c58c15.

We were already using an `arm64` CI task before https://github.com/bitcoin/bitcoin/pull/28187.