💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2650310815)
`752fbab26a...53d2c8e0e2`: rebase due to conflicts

This PR is now smaller because part of it was merged via https://github.com/bitcoin/bitcoin/pull/30205, thanks!

💬 laanwj commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1950557551)
Yes that seems something to consider when and if we cross that bridge. Currently an `.ots` is already generated for the final SHA256SUMS at release time, not sure to move that to another phase in the process.

🤔 hebasto reviewed a pull request: "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries"
(https://github.com/bitcoin/bitcoin/pull/31407#pullrequestreview-2608297032)
Concept ACK 46e44a35b85830a60cf622e039db19ccf1989008.

I have reviewed the code and it looks OK. However, I am not entirely confident in 8400ada306063f1412ef3ace57e255783db879ef due to my lack of familiarity with the signapple tool.

Additionally, I did not review the changes to the signapple tool itself.

🤔 Prabhat1308 reviewed a pull request: "fuzz: coinselection: cover `SetBumpFeeDiscount`"
(https://github.com/bitcoin/bitcoin/pull/31806#pullrequestreview-2608300048)
ACK [0289f03](https://github.com/bitcoin/bitcoin/pull/31806/commits/0289f03790151135afbd5281a45a9f6256f0a235)

`result_{selection-algo-type}->SetBumpFeeDiscount()` sets discount amount before Recalculating the Waste . This will catch any `underflow` scenarios if occuring while subtracting the discount from waste.
Also adding it increased fuzz coverage for earlier untouched part.

💬 laanwj commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1950608126)
Not sure if it's a difference in the tool or the magic files used by default, but `file` output seems noticibly different between linux and mac:

mac (`file-5.41`):
```
/bin/ls: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit executable x86_64
- Mach-O 64-bit executable x86_64] [arm64e:Mach-O 64-bit executable arm64e
- Mach-O 64-bit executable arm64e]
/bin/ls (for architecture x86_64): Mach-O 64-bit executable x86_64
/bin/ls (for architecture arm64e): Mach-O 64-bit ex
...

👍 hebasto approved a pull request: "build: set build type and per-build-type flags as early as possible"
(https://github.com/bitcoin/bitcoin/pull/31711#pullrequestreview-2608325691)
ACK 56a9b847bba2b8deb6a9c3f3a7eb95b4c71c2d14.

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2650448405)
> > IMO, there is isn't a good reason to break compatibility here at all, if a shim that just adds one new line and one new file to the build [#31161 (comment)](https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2623350369) can provide it.
>
> Thanks! Applied.

Reverted back due to the GenerateSymlinks module was not robust enough.


> In its current form, this change seems likely to cause confusion and wasted time because when upgrading or downgrading around the change, **builds
...

👍 laanwj approved a pull request: "build: disable bitcoin-node if daemon is not built"
(https://github.com/bitcoin/bitcoin/pull/31834#pullrequestreview-2608355209)
Code review ACK 2ffea09820e66e25ab639c9fc14810fd96ad6213

💬 laanwj commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#discussion_r1950628441)
Would it be possible to use `bitcoin_daemon_status` here somehow, instead of repeating the expression ? (just a small maintainability thing to keep the reporting and usage consistent if it changes in the future)

📝 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)