🤔 mzumsande reviewed a pull request: "Ensure assumevalid is always used during reindex"
(https://github.com/bitcoin/bitcoin/pull/31615#pullrequestreview-2668456532)
Concept ACK

I think the current approach would work.
Even though it means that if the user provided a bad / non-existing assumeutxo hash they could skip script validation during a reindex when they wouldn't have in the original run, I'd say that's not a major problem because a reindex should only be necessary in case of a a db corruption, which should not be remotely triggerable.

So it will hopefully make the reindex a bit faster in case of a database corruption, while not changing the
...

💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1985755797)
This is just clearing those members because they don't make sense in the LOCKED_IN state. The earlier line `result.current_state = StateName(current_state)` is what's reporting the state. Make sense?

👍 hodlinator approved a pull request: "doc: add note to Windows build about stripping bins"
(https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2668463615)
ACK c94195c077ff227e5e2d80e803e1400d7f60812b

Tested on WSL

Without `--strip`:
```
27 998 527 /mnt/c/workspace/bitcoin/bin/bitcoin-cli.exe
587 584 175 /mnt/c/workspace/bitcoin/bin/bitcoin-qt.exe
59 799 144 /mnt/c/workspace/bitcoin/bin/bitcoin-tx.exe
27 408 020 /mnt/c/workspace/bitcoin/bin/bitcoin-util.exe
226 141 145 /mnt/c/workspace/bitcoin/bin/bitcoin-wallet.exe
467 648 497 /mnt/c/workspace/bitcoin/bin/bitcoind.exe
642 186 589 /mnt/c/workspace/bitcoin/bin/test_bitcoin-qt.exe
9
...

💬 hodlinator commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985759190)
nit: Phantom newline.

💬 achow101 commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2707537453)
ACK ac6cde731314d981391bc313c98d671c68211d33

👍 hodlinator approved a pull request: "cmake: Check for `makensis` tool before using it"
(https://github.com/bitcoin/bitcoin/pull/32019#pullrequestreview-2668492229)
tACK deea0988720aab65b3cc6d80b50bbf0d537adb24

Tested on WSL.

Without:
```shell
$ cmake --build build --target deploy -j16
...
[100%] Generating bitcoin-win64-setup.exe
gmake[3]: makensis: No such file or directory
gmake[3]: *** [CMakeFiles/deploy.dir/build.make:81: bitcoin-win64-setup.exe] Error 127
gmake[2]: *** [CMakeFiles/Makefile2:530: CMakeFiles/deploy.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:537: CMakeFiles/deploy.dir/rule] Error 2
gmake: *** [Makefile:244: deplo
...

🤔 jonatack reviewed a pull request: "doc: update fuzz instructions when on macOS"
(https://github.com/bitcoin/bitcoin/pull/31954#pullrequestreview-2668496484)
Post-merge tested ACK using the following incantation with arm64 macos 15.3.1:

```bash
$ rm -rf ./build_fuzz/ && cmake -B ./build_fuzz -DAPPEND_CXXFLAGS="-std=c++23" --preset=libfuzzer -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" -DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" -DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld" && cmake --build ./build_fuzz --parallel 11 --

$ FUZZ=process_message ./build_fuzz/src/test/fuzz/fuzz
```

🤔 furszy reviewed a pull request: "Require sqlite when building the wallet"
(https://github.com/bitcoin/bitcoin/pull/31961#pullrequestreview-2668500087)
utACK ac6cde731314d981391bc313c98d671c68211d33

💬 jonatack commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2707596335)
Plan to review soon.

💬 jonatack commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-2707597628)
I plan to review this soon.

🤔 hodlinator reviewed a pull request: "net: replace manual reference counting of CNode with shared_ptr"
(https://github.com/bitcoin/bitcoin/pull/32015#pullrequestreview-2668522266)
Concept ACK 36d932cebbb235a79e90575960646e86a23d39a8

💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1985826873)
In thread https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1984875189:

Maybe one could have an additional `vector<shared_ptr<CNode>>` member in `CConnman` beyond `m_nodes` that always keeps the `shared_ptr::use_count()` above 0. Then use removal from that vector (when `use_count() == 1`) for controlled deletion with enforced lock order, more like how the code was before?

💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1985793960)
nit: Might as well use `std::ranges::count_if` here and in `GetFullOutboundConnCount`.

💬 1440000bytes commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2707765737)
This pull request is last attempt to get some "technical reviews" and find bugs.

It will obviously get merged when community wants to use it and looking for activation.

💬 achow101 commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2707778237)
ACK f0b659716bd455dca02053df573d888b5a115aa4

💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985888295)
This looks like spam.

💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985900417)
Extra newline

💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985904822)
I noticed a couple of translations have these pronunciations, is that expected according to our translation guidelines (do we have any)?

💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985900345)
AI generated?

💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1985897639)
Extra newlines, although google translate thinks the translation is correct. It seems like the newlines do render in the gui.

There are a number of these in this file.