👍 TheCharlatan approved a pull request: "doc: build: Fix instructions for msvc gui builds"
(https://github.com/bitcoin/bitcoin/pull/31851#pullrequestreview-2617147568)
ACK c3fa043ae560289759b6f836ac62736d97ba91bf

Am I misremembering or was this, or a similar workaround, part of the previous build instructions at some point?

📝 eval-exec opened a pull request: "random: Initialize variables in hardware RNG functions"
(https://github.com/bitcoin/bitcoin/pull/31863)
See: https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1955045279 , So this PR want to prevent potential uninitialized value issues and improve code clarity.

👍 rkrux approved a pull request: "wallet: abandon orphan coinbase txs, and their descendants, during startup"
(https://github.com/bitcoin/bitcoin/pull/31794#pullrequestreview-2617045748)
ACK 409241db5dca0b23f5c7714f99be52411fc5541e

Build and tests successful.

Seems to be an edge case that's fixed but I'm in favour because it ensures data correctness.

Re: this comment https://github.com/bitcoin/bitcoin/pull/31794#pullrequestreview-2605174936
I agree with @furszy that this should be in a separate PR because of the separate context.

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955732785)
Nit: The third node isn't really used in the test. I ran the test without it and it worked.

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955746499)
Would it be helpful to assert for `'category': 'orphan'` as well?

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955742226)
These comments are very helpful!

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955765173)
Trying to understand why the `AbandonTransaction` was overloaded. IIUC, the older one `AbandonTransaction(const uint256& hashTx)` can be used as well here with the `id` passed as the argument.

Am I missing something? I do notice `EXCLUSIVE_LOCKS_REQUIRED` in the new function signature but I also see it being used already in the older function before this change.

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955795282)
Is the following scenario handled by the usual handling or via this new one?

> User has their node running and receive a coinbase transaction, and then the node is stopped. User restarts the node after a long time and meanwhile that transaction was invalidated (block invalidated), and at the time of loading the wallet the correct balance is shown.

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955733317)
Was this statement supposed to be inside the `for` loop? Otherwise only `blocks` seem to be copied to `node0` while the `chainstate` as well is deleted in `node0`.

💬 rkrux commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#discussion_r1955766889)
Does an external wallet here mean when a wallet is loaded from wallet file that was manually added by the user in the path?

🤔 hebasto reviewed a pull request: "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain"
(https://github.com/bitcoin/bitcoin/pull/31849#pullrequestreview-2617167523)
Concept ACK on using as many CMake's abstractions as possible.

We already use them in depends:https://github.com/bitcoin/bitcoin/blob/2549fc6fd1cc958a0e2d59838907c8808fd129b3/depends/funcs.mk#L194-L198

> ... we'd end up with a duplicated `--target=$ARCH-apple-darwin` on the compiler line

This will become a persistent source of confusion and complains, which could, of course, just be ignored. That was the only reason why I initially refrained from using `CMAKE_<LANG>_COMPILER_TARGET` whi
...

💬 hebasto commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#discussion_r1955802997)
Add `set(CMAKE_OBJCXX_COMPILER_TARGET @host@)`?

💬 TheCharlatan commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1955803986)
Ah, I was irritated by the reference to the expiry, but it is only a couple of lines above, so I guess that is fine as is.

💬 hebasto commented on pull request "cmake: Improve safety and robustness during building `crc32c` subtree":
(https://github.com/bitcoin/bitcoin/pull/31779#issuecomment-2658718992)
> Another thought, isn't this going to break git bisect, or, result in having to do unclean subtree pulls? As the commit where you update the subtree will no-longer compile unless it also simultaneously changes the hash hardcoded into our build system.

Fair enough.

✅ hebasto closed a pull request: "cmake: Improve safety and robustness during building `crc32c` subtree"
(https://github.com/bitcoin/bitcoin/pull/31779)

💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2658745672)
@goodthebest this pull request is not about pruning blocks. A better place to ask is https://bitcoin.stackexchange.com

But yes, it doesn't matter if you prune your node for mining and any value is fine (the minimum is `-prune=550` MB).

👍 TheCharlatan approved a pull request: "Cleanups to port mapping module post UPnP drop"
(https://github.com/bitcoin/bitcoin/pull/31157#pullrequestreview-2617235585)
ACK 70398ae05bc36a2788e87f67ae06962f43fe35a6

Just a nit: Maybe cleanup the dangling includes?

🚀 fanquake merged a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758)

🚀 fanquake merged a pull request: "depends: avoid an unset `CMAKE_OBJDUMP`"
(https://github.com/bitcoin/bitcoin/pull/31857)

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2658779105)
Force pushed to add support for executables linked with libFuzzer