💬 l0rinc commented on pull request "ci: Use latest Xcode that the minimum macOS version allows":
(https://github.com/bitcoin/bitcoin/pull/33932#issuecomment-3571747488)
ACK fa9537cde10120b12c96061cbc3f79a7680f9d64

💬 psam21 commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#discussion_r2557032045)
Right to point out that the _cmake macro already passes CC and CXX on the cmake command line at funcs.mk:215-217.

The issue is that CMake's initial compiler detection happens before it processes those command-line variables. When CMake runs its configure phase, it looks for CC/CXX in the environment first. The command-line arguments passed by _cmake aren't set as environment variables, so CMake falls back to system defaults (gcc), which is what issue #33859 reported.

The config_env variabl
...

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3571793549)
I've been tracking the non-mempool transaction memory footprint for an hour now on mainnet, using fairly aggressive template update criteria (minimum fee delta 1 sat and no more than once per second). So far the footprint is minuscule, but of course this depends on the mempool weather:
<img width="1344" height="685" alt="getmemoryload-scatter" src="https://github.com/user-
attachments/assets/f46f2ca6-84c2-4571-9026-64428f1531ff" />

💬 brunoerg commented on pull request "test: add `-alertnotify` test for large work invalid chain warning":
(https://github.com/bitcoin/bitcoin/pull/33893#issuecomment-3571972044)
reACK 8343a9ffcc752f77eb2248315d10b6dff4a5c98b

From my last review, it addresses https://github.com/bitcoin/bitcoin/pull/33893#discussion_r2543670655.

💬 plebhash commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3572028327)
any chance this could get backported?

💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2557221072)
I guess that might be more informative but the comment is just removed now

💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2557221500)
Fixed

💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2557221903)
Done

💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2557226776)
Yeah, that looks very good to me, I took this almost exactly as is, squashed the `NUM_WALLETS` in there because these lines are now touched as well and made you co-author of that commit. Thanks a lot!

💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#discussion_r2557228453)
It's gone, probably left-over from an earlier version of the code where this was still the case

💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#issuecomment-3572052555)
Addressed the feedback from @rkrux , thanks for the review!

💬 Ataraxia009 commented on pull request "init: Changing the rpcbind argument being ignored to a pop up warning":
(https://github.com/bitcoin/bitcoin/pull/33813#discussion_r2557263164)
right but is there an actual case where people do that? The only reason you would need to `-rpcallowip` is if you are exposing the rpc server with `-rpcbind`?

💬 Sjors commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3572110958)
No strong opinion on whether this should be backported. In `sv2-tp` I found it easy to work around by calling `waitNext()` in a loop with a reasonable timeout: https://github.com/stratum-mining/sv2-tp/blob/fbe0984ca21e2b6ae0f554bbc5b2819332e59afc/src/sv2/template_provider.cpp#L131-L144

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3572122250)
reACK 17cf9ff7efdbab07644fc2f9017fcac1b0757c38

💬 monlovesmango commented on pull request "wallet: don't consider unconfirmed TRUC coins with ancestors":
(https://github.com/bitcoin/bitcoin/pull/33528#discussion_r2557293653)
As far as I can tell `descendants` from `getTransactionAncestry` actually finds the parent(/ancestor) with the most descendants and returns that count, not the count of children for `txid` in question.

💬 sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2557315230)
Thanks, fixed in https://github.com/bitcoin/bitcoin/commit/38f2f53963e833d25bc71df0713fb28cff17cc43

💬 ryanofsky commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#discussion_r2557363728)
Thanks for the pointers. Just looking at the current code I'm confused about why it isn't doing the right thing. It looks like `native_capnp_type` should be getting set to "build" here:

https://github.com/bitcoin/bitcoin/blob/238c1c8933b1f7479a9bca2b7cb207d26151c39d/depends/funcs.mk#L318

And then `native_capnp_cxx` should be getting set to `build_CXX` here:

https://github.com/bitcoin/bitcoin/blob/238c1c8933b1f7479a9bca2b7cb207d26151c39d/depends/funcs.mk#L4

And then the `CXX` environm
...

💬 plebhash commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3572260427)
we also have workarounds on https://github.com/stratum-mining/sv2-apps/pull/59 but they're substantially increasing code complexity, which will likely have a negative impact on maintainability on the mid-long term

we have https://github.com/stratum-mining/sv2-apps/issues/81 to keep track of this but we'd really appreciate if this got backported instead of landing on v31+

💬 furszy commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2557392174)
Cool. It would be good to add a test case for it.

💬 Christewart commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3572313467)
> > Hi AJ, are these the test vectors you had in mind?
>
> I was thinking functional tests, to also make sure there isn't some obscure policy rule that the unit tests miss that nevertheless ends up blocking successful spends of things that should be spendable. Unit tests are good too though.

:+1:

> The things that (per this PR's arguments) should be spendable are `1 <pubkey> <invalid> 2 CMS` and `1 <invalid> <pubkey> 2 CMS`.

I took a closer look through `script_tests.json` and it see
...