👍 hodlinator approved a pull request: "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets"
(https://github.com/bitcoin/bitcoin/pull/32019#pullrequestreview-2670874014)
re-ACK 0ecd2e0748bdbfc12af1fc100862a1a6f046c2c9

Good that nsis and zip could be made optional unless deploying.

Only re-tested Windows-side, but Mac side diff looks equivalent.

Without nsis:
```shell
/mnt/c/Users/hodlinator/bitcoin$ cmake --build build --target deploy
Error: NSIS not found
Built target deploy
```

I needed to re-run `cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake` for the build to pick up that makensis was available. It would be good if we
...

💬 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-2710603528)
> re-ACK [0ecd2e0](https://github.com/bitcoin/bitcoin/commit/0ecd2e0748bdbfc12af1fc100862a1a6f046c2c9)
>
> Good that nsis and zip could be made optional unless deploying.
>
> Only re-tested Windows-side, but Mac side diff looks equivalent.
>
> Without nsis:
>
> ```shell
> /mnt/c/Users/hodlinator/bitcoin$ cmake --build build --target deploy
> Error: NSIS not found
> Built target deploy
> ```
>
> I needed to re-run `cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain
...

💬 hodlinator 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-2710629594)
> > or indicate in the error message that one has to regenerate the build config.
>
> Agree. Mind suggesting a wording for the error message?

Felt I was going out on a limb using the term "build config", was thinking something like:
```
Error: NSIS not found. Install it (make it available to find_program()) and regenerate the build config.
```

💬 furszy commented on pull request "removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#discussion_r1987312628)
This introduces a cyclic dependency. Please do not add RPC dependencies to the wallet. These are two separate components, and the wallet operates at a lower level.

💬 furszy commented on pull request "removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#discussion_r1987318572)
No need to log the error message. It will be retrieved to the user via the RPC response.

💬 hodlinator commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1987418788)
Looked around for `wait_for_`-functions, but a bunch of those are used to check for `last_message` in `P2PInterface`. Staring a bit at `sync_txindex` I realized the wait condition could be simplified, decided to do that to pre-empt other reviewers. Excuse me for invalidating your ACK.

📝 Chand-ra opened 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`

💬 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-2710893296)
Addressed the feedback from @hodlinator. Thank you!

👍 laanwj approved a pull request: "wallet: Replace "non-0" with "non-zero" in translatable error message"
(https://github.com/bitcoin/bitcoin/pull/31987#pullrequestreview-2671328596)
Code review ACK 18e83534ace7aa2d26bc7dfa521b1d591b66edfa
Aside from the issue raised, this makes the message more readable.

💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1987507317)
Removed on Transifex.

💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1987507697)
Fixed on Transifex.

💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1987508337)
Removed on Transifex.

💬 Sjors 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-2710949958)
Tested on macOS 15.3.1 that `cmake --build build --target deploy` still works. It's impossible (?) to test the missing condition, because /usr/bin/zip comes with the OS.

📝 marcofleon opened a pull request: "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`"
(https://github.com/bitcoin/bitcoin/pull/32025)
This PR addresses a small bug in [`AcceptMultipleTransactions`](https://github.com/bitcoin/bitcoin/blob/45719390a1434ad7377a5ed05dcd73028130cf2d/src/validation.cpp#L1598) where a txid was being inserted into a map that should only hold wtxids. `CheckEphemeralSpends` has an out parameter on failure that records that the child transaction did not spend the parent's dust. Instead of using the txid of this child, use its wtxid.

The second commit in this PR is a refactor of the `PackageMempoolAcc
...

💬 yancyribbens commented on pull request "tests: Improve stderr validation in test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/31966#issuecomment-2710957626)
Is there any reason the `TODO` was not removed on L159 with this commit?

💬 marcofleon commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#issuecomment-2710962639)
note: I found this while working on the uint256 to txid/wtxid full refactor. I figured I would include a tiny part of that in this PR because it relates to the bug. If it's preferred to only have the fix be merged for branch off, then I can remove the second commit and include it as part of the txid type safety project later on.

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#issuecomment-2710976385)
I've started reviewing this PR and will leave a review soon in the coming days.

💬 Sjors 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-2710977444)
Oh this code path isn't used at all when building natively? E.g. on macOS I can do `ZIP_EXECUTABLE=$HOME/temp/zip2 cmake --build build --target deploy` which doesn't fail.

💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1987530875)
No, it is not expected. Removed on Transifex.

💬 Sjors commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2711000319)
I'm not too worried about the size increase. A typical guix build eats 30GB on my Ubuntu VM (3 GB for the outputs).

> it makes it possible to check if mistakes have been made in the process at earlier steps.

Agreed.

It seems simple enough to backport, especially because we're not bumping the version.

I'll see if I can reproduce the new hashes.