🤔 maflcko reviewed a pull request: "depends: suggest GNU patch for macOS <= 13"
(https://github.com/bitcoin/bitcoin/pull/29814#pullrequestreview-1999542526)
Not sure. Anyone else on any version of macOS can build fine. So far only a single person ran into this issue, without steps to reproduce, so I am not sure about adjusting the docs.

💬 maflcko commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#discussion_r1564613112)
Installing it would be insufficient anyway, because you'd still need to overwrite the global default?

💬 maflcko commented on pull request "ci: use Clang 16 for Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29848#issuecomment-2053999845)
lgtm ACK ad21f2294821d7c436e58a8f199fb555b11a56ad

💬 maflcko commented on pull request "build: Fix false positive `CHECK_ATOMIC` test for clang-15":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2054000372)
> > The same error happens with clang-18 locally, but why does CI pass?
> > [fad23a0](https://github.com/bitcoin/bitcoin/commit/fad23a06469607689c4f637bb407c96af4902a27)
>
> Depends on standard C++ library version?

I can re-try, but my findings didn't indicate that IIRC.

In any case, the title needs to be adjusted to remove the `clang-15`, no?

💬 hebasto commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2054000744)
> In any case, the title needs to be adjusted to remove the `clang-15`, no?

Updated.

💬 maflcko commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2054000903)
Is this related to bbe82c116e72ca0638751e063bf564cd1fe5c4d5?

💬 furszy commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2054064278)
> Is this related to [bbe82c1](https://github.com/bitcoin/bitcoin/commit/bbe82c116e72ca0638751e063bf564cd1fe5c4d5)?

No, it is related to 0faafb57f8298547949cbc0044ee9e925ed887ba. This change is partially reverting it and documenting the flow so the regression does not happen again.

💬 laanwj commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2054078250)
Concept ACK, but could it be that we could need some of these?
Eg `close_fds` sounds like what's described in #22417

✅ furszy closed a pull request: "Introduce 'getblockfileinfo' RPC command"
(https://github.com/bitcoin/bitcoin/pull/27770)

💬 furszy commented on pull request "Introduce 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-2054101285)
> I think this information is too low-level. In the past we've rejected changes that expose low-level information about our block storage because block files are not an external API, and exposing this kind of info limits our flexibility in regard to changing the block storage model.

Conceptually, I agree. But I don't think that the block storage model flexibility argument is the best for not doing this. We do have procedures tightly coupled to the current block storage model (e.g. the best ch
...

💬 michaelwklein commented on issue "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows":
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2054102603)
Hi @hebasto, @achow101 thanks for moderating the repository. @joinfuel is a project that assigns rewards to GitHub issues, developers simply to connect their GitHub via joinfuel.io to obtain the rewards. Is there someone we can talk to in Developer relations to have these comments approved? The community and the project itself and assign rewards to GitHub issues to promote open source projects.

💬 laanwj commented on pull request "Introduce 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-2054104846)
> Removing this new debug-only RPC command would be the least of our problems if we ever seek to change the storage model.

Sure, that's true. In that sense it's different than say, adding block file/pos to `getblock` info, as it doesn't affect the output of a common RPC.

📝 hebasto opened a pull request: "Reintroduce external signer support for Windows"
(https://github.com/bitcoin/bitcoin/pull/29868)
Based on upstream https://github.com/arun11299/cpp-subprocess/pull/99.

Partially reverts:
- https://github.com/bitcoin/bitcoin/pull/28967
- https://github.com/bitcoin/bitcoin/pull/29489

After this PR, we can proceed to actually remove the [unused code](https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1991272752) from `src/util/subprocess.hpp`.

💬 fanquake commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1564859345)
> ci: remove --enable-external-signer from win64 job

Why this is being reverted? This is already applied globally to the CI (and I'd say pointlessly so).

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1564860366)
Thanks! Dropped.

💬 fanquake commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1564861016)
Great, can you remove it from the global config here as well. Given it's not required or actually testing anything.

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1564897342)
> Great, can you remove it from the global config here as well. Given it's not required or actually testing anything.

I agree. Implemented.

💬 laanwj commented on issue "contrib: add symbol-check test for non-existence of `vmova` instructions in Windows build":
(https://github.com/bitcoin/bitcoin/issues/28413#issuecomment-2054181503)
Is introducing a dependency on the python `capstone` binding acceptable here? It's not possible to do this check without a disassembler of some kind, we don't want to rely on calling out to `objdump`, and i'm not sure we want our own x86 mini-disassembler. Capstone is great for this and might help in future instruction security checks as well. But it's another build-time dep.

🤔 stickies-v reviewed a pull request: "[27.x] More backports and finalize"
(https://github.com/bitcoin/bitcoin/pull/29780#pullrequestreview-1999739645)
LGTM 4681f554bc6eb6f603512575de19bc52d08813e9 with a few release note nits

- backports mostly clean, 753c68dc0f02644f223d407b33000dce7a763056 being the exception because of LLVM version bump
- release notes look good and correspond with devwiki, quickly went through merges since v26.0 and couldn't see any major omissions
- i'm getting the same manpages output

💬 stickies-v commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#discussion_r1564914593)
Incorrect pull number

```suggestion
- Improved handling of empty settings.json files (#29144)
```