📝 fanquake opened a pull request: "depends: add missing Darwin objcopy"
(https://github.com/bitcoin/bitcoin/pull/31840)
Our CMake toolchain for a Darwin cross build currently contains:
```bash
set(CMAKE_AR "/usr/bin/llvm-ar")
set(CMAKE_RANLIB "/usr/bin/llvm-ranlib")
set(CMAKE_STRIP "/usr/bin/llvm-strip")
set(CMAKE_OBJCOPY "arm64-apple-darwin-objcopy")
set(CMAKE_OBJDUMP "/usr/bin/llvm-objdump")
```

`objcopy` isn't currently used for the Darwin build (only for Linux and splitting the debug symbols), but we shouldn't be producing a toolchain file that refers to nonexistent tools.

💬 danielabrozzoni commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1950645609)
Of course! Thank you, updated, and made last_negated and categories_to_process const

💬 Sjors commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#discussion_r1950652514)
It turns out yes, as long as you define `bitcoin_daemon_status` before `add_subdirectory(src)`. Pushing that.

💬 hebasto commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#discussion_r1950655717)
TBH, I'd refrain from making `bitcoin_daemon_status` a "global" variable.

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2650508714)
> This PR's script is saying the harness passes the coverage test while the one I linked is showing differences in hit count.

Thanks, good catch! Fixed off-by-one

💬 Sjors commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#discussion_r1950668773)
In that case I'm inclined to not use the variable at all.

💬 Sjors commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#discussion_r1950674530)
Oh but `message()` can't handle that. Alright, let's go back to the previous version.

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2650541771)
> Nice. We do have something similar, although it could likely be refined more. You can see it [here](https://github.com/marcofleon/coverage/tree/master/deterministic-coverage). It should show all the places where hit count between two coverage reports differs.

Nice. Happy to pull it in here, but I think it is similar to `diff --unified`, so I used that here for now. Not sure if your script handles sub-line regions, but in this pull they can trivally be enabled by adding the `-show-line-count
...

💬 laanwj commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#discussion_r1950689515)
> In that case I'm inclined to not use the variable at all.

Ok, yes, that would have been my other proposal. But a variable seems to be needed then, let's leave it as-is.

💬 fanquake commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/31422#issuecomment-2650590978)
> probably is not necessary for this PR?

I agree, I'm not planning on backporting that here.

🚀 fanquake merged a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/31422)

💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2650680523)
That would be `DynSock::Pipe::GetBytes()` and `DynSock::Pipe::PushBytes()`. `Sock::Send()` and `Sock::Recv()` also use raw pointer + length, but they were not introduced in this PR and, more importantly, are supposed to mimic 1:1 the `send(2)` and `recv(2)` syscalls, in return value, arguments and semantic.

For `GetBytes()` and `PushBytes()` the change would be:

```diff
diff --git i/src/test/util/net.h w/src/test/util/net.h
index 3e717341d8..a28c442035 100644
--- i/src/test/util/net.h

...

🚀 ryanofsky merged a pull request: "build: disable bitcoin-node if daemon is not built"
(https://github.com/bitcoin/bitcoin/pull/31834)

💬 hodlinator commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2650784726)
> > Got a big diff, indicating a lot of test non-determinism:
>
> I haven't looked in detail, but I presume much is due to randomness. On one hand, one could go with #31486 for the unit tests as well. On the other hand, some unit tests are fuzz tests (drawing inputs from a random seed, which is recorded), so they'd be a bit more limited if the randomness was made deterministic. But I think either way is fine.

This gives me an 8.4K line diff instead of 39k, so there is some hope:
```
RAND
...

💬 ryanofsky commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2650797960)
re: What do you think? PR? Leave it alone?

Whichever you prefer, I'd happily review it. I wanted to make suggestion more as an idea to keep in mind when updating this code or writing new code, because of benefits described in https://github.com/bitcoin/bitcoin/issues/31272

Also ideally the change would go further IMO: replace `std::vector<uint8_t>` with `std::vector<std::byte>`, replace `memcpy` with `std::vector::assign`, replace Recv/Send arguments with `span` instead of `void*`. I don't
...

👍 hodlinator approved a pull request: "logging: Ensure -debug=0/none behaves consistently with -nodebug"
(https://github.com/bitcoin/bitcoin/pull/31767#pullrequestreview-2608732876)
re-ACK 7afeaa24693039730c9ae00c325c2b9d0e2deda1

Changes since [previous ACK](https://github.com/bitcoin/bitcoin/pull/31767#pullrequestreview-2599809742):
- Broke out initial commit which tests prior behavior (with minor improvements).
- Terser test for last occurrence of negated arg suggested by self.

💬 0xB10C commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650860541)
> > I want to check if this helps [#30852 (comment)](https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2558198430) when used with #31545 by not having to keep the base images around anymore.
>
> @0xB10C Are you waiting for me to rebase this once more? I've held back, because there is already one review, but I am happy to rebase.

Thanks for the reminder. I played around with this a bit and it does indeed help:

When using https://github.com/bitcoin/bitcoin/pull/31545, the base
...

📝 maflcko opened a pull request: "test: Clear scheduler after init to increase fuzz stability"
(https://github.com/bitcoin/bitcoin/pull/31841)
Leaking a scheduler with a non-empty queue from the fuzz initialization phase into the fuzz target execution phase is problematic, because it messes with coverage data. This in turn is problematic, because it leads to:

* Decrease in fuzz target execution stability (non-determinism when running the fuzz target).
* Decrease in fuzz input merge stability (non-determinism when selecting a minimum set of fuzz input to reach maximum coverage), which leads to qa-assets bloat.

Fix one such issue.
...

💬 0xB10C commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650867558)
My understanding is that https://github.com/bitcoin/bitcoin/pull/28138 was only needed for `DANGER_RUN_CI_ON_HOST`. That's why removing it here isn't a regression to pre 28138, correct?

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2650868943)
I've created https://github.com/bitcoin/bitcoin/pull/31841 to make it easier to test this on a real patch.