💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2711068992)
> Oh this code path isn't used at all when building natively?

Correct.

💬 dergoegge commented on pull request "test: get rid of redundant TODO tag in fuzz tests":
(https://github.com/bitcoin/bitcoin/pull/32024#issuecomment-2711071334)
Thanks for your contribution but `FuzzedDataProvider.h` is not maintained by us. This should be opened against llvm: https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/FuzzedDataProvider.h.

💬 laanwj commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2711075412)
Tested windows path without NSIS
```
$ ninja -j4
[224/224] Linking CXX executable src/test/test_bitcoin.exe (successful build)
$ ninja deploy
[1/1] cd /home/user/src/bitcoin-windows/build && /usr/bin/cmake -E echo && /usr/bin/cm... -E echo "Then re-run cmake to regenerate the build system." && /usr/bin/cmake -E echo

Error: NSIS not found.
Please install NSIS and/or ensure that its executable is accessible to the find_program() command—
for example, by setting the MAKENSIS_EXECUTABLE va
...

✅ hebasto closed a pull request: "test: get rid of redundant TODO tag in fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/32024)

📝 hebasto locked a pull request: "test: get rid of redundant TODO tag in fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/32024)
`list.size()` is determined at runtime, so using `static_assert` on it as suggested by the TODO comment is not feasible and produces the following error when done:

`error: static assertion expression is not an integral constant expression`

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1987604281)
Indeed, done, thanks!

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2711102281)
`36d932cebb...af622d00ba`: address https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1985793960

💬 fjahr commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#issuecomment-2711162276)
re-ACK 3301d2cbe8c3b76c97285d75fa59637cb6952d0b

Only change was to make the helper internals a bit nicer.

📝 hebasto opened a pull request: "cmake: Add `NO_CACHE_IF_FAILED` option for checking linker flags"
(https://github.com/bitcoin/bitcoin/pull/32027)
Use it for checking `-fsanitize`.

This change improves the user experience when the configuration step fails due to a missing library. Now, there is no need to manually clean the CMake cache after installing the required library.

Addresses [this](https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2703801270) comment from https://github.com/bitcoin/bitcoin/issues/31942.

💬 hebasto commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2711249969)
> > reliable steps to reproduce those issues.
>
> For example, configuring with GCC + thread sanitizer (where libtsan is missing). After installing the missing dependency, and re-running CMake, even though the check is being re-run, CMake doesn't "fix" itself, and requires removing the build directory entirely.

Fixed in https://github.com/bitcoin/bitcoin/pull/32027.

📝 hebasto opened a pull request: "Update `secp256k1` subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/32028)
This PR updates the `secp256k1` subtree to https://github.com/bitcoin-core/secp256k1/commit/4ba1ba2af953b7d124db9b80b34568e5c4a2d48a, which includes the following changes:
- https://github.com/bitcoin-core/secp256k1/pull/1633
- https://github.com/bitcoin-core/secp256k1/pull/1634
- https://github.com/bitcoin-core/secp256k1/pull/1641
- https://github.com/bitcoin-core/secp256k1/pull/1650
- https://github.com/bitcoin-core/secp256k1/pull/1646
- https://github.com/bitcoin-core/secp256k1/pull/165
...

💬 hebasto commented on pull request "[POC] build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2711376386)
Rebased on https://github.com/bitcoin/bitcoin/pull/32028.

🤔 mzumsande reviewed a pull request: "qa: Fix TxIndex race conditions"
(https://github.com/bitcoin/bitcoin/pull/32010#pullrequestreview-2671833519)
Code Review ACK 3301d2cbe8c3b76c97285d75fa59637cb6952d0b

💬 purpleKarrot commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2711463453)
Instead of passing around shared pointers to mutable data (aka global variables in disguise), would it be possible to increase the level of abstraction and pass around values with regular semantics? Every C++ programmer should rewatch https://youtu.be/W2tWOdzgXHA from time to time.

💬 yancyribbens commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1987820088)
Is this documented anywhere? This is a good question and I would imagine others will have the same in the future.

💬 yancyribbens commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1987822504)
This method name looks funky. SingleTxTXO is short for single transaction transaction output?

💬 yancyribbens commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1987847360)
nit - this should be indented. Right now it's mis-aligned.

💬 darosior commented on pull request "rpc: add target to getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1987859999)
This documentation is incomplete, as following the instructions one would run into the check for connections performed by `getblocktemplate`:
https://github.com/bitcoin/bitcoin/blob/45719390a1434ad7377a5ed05dcd73028130cf2d/src/rpc/mining.cpp#L764-L773

To actually follow these instructions on mainnet it is necessary to comment out these lines and recompile.

💬 Sjors commented on pull request "rpc: add target to getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1987869351)
Indeed I left out a few details because it would get tedious.

💬 Sjors commented on pull request "rpc: add target to getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1987870592)
@darosior depending on what operating system you're using, you may need one or more of these patches: https://github.com/pooler/cpuminer/pulls