💬 paplorinc commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2278056328)
> I'd suggest a reindex test

Thanks for the hint, running the following, please let me know if you think this isn't representative:
```bash
hyperfine \
--runs 5 \
--parameter-list COMMIT 27a770b34b8f1dbb84760f442edb3e23a0c2420b,4471aafb25b37fa8f780c87a4067b2bb52aef347 \
--prepare 'git checkout {COMMIT} && git clean -fxd && git reset --hard && ./autogen.sh && ./configure && make -j8' \
'COMMIT={COMMIT} ./src/bitcoind -datadir=/mnt/sda1/BitcoinData -dbcache=16384 -reindex -stopatheight=50
...

💬 furszy commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet` - (`void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion 'it.second' failed.`)":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2278066850)
> > but how do you link it to this failure?
>
> It seems pretty clear to me the suggested fix in [#29073 (comment)](https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2274237242) will prevent this failure because it removes the assert and handles the case where UnloadWalllet is called by more than one thread, instead of aborting. I agree it would be nice to understand more about this failure, though, because maybe there are more problems and this fix isn't sufficient.

Your sugges
...

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2278079028)
That makes sense. I might test that by rebuilding `/usr/local/bin/guix` from source on a recent master commit, and then try `guix pull` again.

💬 andrewtoth commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2278082792)
@paplorinc I don't see anything in this PR that would affect reindexing. This PR seems like a refactor only.

💬 ryanofsky commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet` - (`void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion 'it.second' failed.`)":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2278146211)
> Give me your thumbs up and will push the PR. And will probably also rework the shutdown flow too.

Definitely feel free! I think the fix you suggested https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2278066850 will prevent the crash, but it has drawbacks compared to the change suggested in https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2274237242:

- The main drawback is that allows the UnloadWallets call to return early before all wallets have actually unloaded.
...

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711661191)
I noticed that there are still a few leftover `make` reference in this file, and in other documentation: are we planning on updating them here or in a separate pr, e.g. when removing Autotools?

I searched for other possible `make` references revealed a few:
* https://github.com/bitcoin/bitcoin/blob/5ebb4063571aabd89c2d7ed6c2e70d27636efdeb/src/test/README.md?plain=1#L18-L24
* https://github.com/bitcoin/bitcoin/blob/183e2fd6b55d84829faf70ba74d3f8865807772a/src/secp256k1/doc/release-process.md
...

🤔 glozow reviewed a pull request: "test: remove `ExtractDestination` false assertion for `ANCHOR` script"
(https://github.com/bitcoin/bitcoin/pull/30616#pullrequestreview-2230467362)
ACK a4f2b185732649eeea4a042cebd90d0e0e12cc92

Reproduced crash and fix, looks correct. Will plan to merge later.

💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-2278201819)
> Would be nice to extend the motivation (pull request description) a bit about the benefits and risks.

Thanks. Drafted for the moment, but going to come back here soon with much more information.

📝 fanquake converted_to_draft a pull request: "guix: Use LTO to build releases"
(https://github.com/bitcoin/bitcoin/pull/25391)
Changes required to use LTO in Guix (release) builds.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711674603)
> are we planning on updating them here or in a separate pr, e.g. when removing Autotools?

Here. Feel free to open a PR in https://github.com/hebasto/bitcoin/pulls against the `cmake-staging` branch. After merging, the changes will be pulled in here during the next update.

💬 fanquake commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2278208327)
@theuni reminder that you might want to followup here at some point, re recent discussion/debugging.

💬 paplorinc commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1711689242)
Pulled these to another PR: [`d1e3f0e` (#30618)](https://github.com/bitcoin/bitcoin/pull/30618/commits/d1e3f0e8a9df410317a120b81087fa4a0cddc480)

note1: nit: `// Enable BOOST_CHECK_EQUAL for enum class types` could be put inside the `std` now that we have multiple
note2: we might want to move the remaining ones in the cpp file here as well (inline-ing them all, of course)

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2278235877)
Rebased cf9aaf4b7f27b38bf201aa39c56c9967f3b2a904 -> 4137419b0170fa1e9ff9daacde57933f7c995b76 ([`pr/ipc-bind.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-bind.3) -> [`pr/ipc-bind.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-bind.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-bind.3-rebase..pr/ipc-bind.4)) with fix for new test that was failing when run in the multiprocess CI container https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2276739868

🤔 mzumsande reviewed a pull request: "validation: write chainstate to disk every hour"
(https://github.com/bitcoin/bitcoin/pull/30611#pullrequestreview-2230536256)
Did you check if this would slow down IBD noticeably when running with a large dbcache?
If i understand it right, coins that are created and destroyed more than 1 hour of syncing apart would not have been written to disk before, but will be written and deleted after this PR - though presumably this is not a frequent event, so there wouldn't be much of an impact?!

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1711719710)
Oh of course, I was confusing myself :+1:

📝 paplorinc opened a pull request: "Change `nproc` in docs to `getconf _NPROCESSORS_ONLN`"
(https://github.com/bitcoin/bitcoin/pull/30619)
Split out of https://github.com/hebasto/bitcoin/pull/316/files#r1711537463

nproc is linux only, getconf _NPROCESSORS_ONLN is used in the Linux kernel for the same task: https://github.com/torvalds/linux/blob/master/tools/testing/selftests/rcutorture/bin/kvm-build.sh#L44

On linux:
```
# nproc
8
root@rescue ~ # getconf _NPROCESSORS_ONLN
8
```

Note that in the productivity doc the recommended core count is one less than before, seems simpler to me this way.

👍 itornaza approved a pull request: "Stratum v2 Noise Protocol"
(https://github.com/bitcoin/bitcoin/pull/29346#pullrequestreview-2230564992)
Code review ACK e73740fde107f79eeee8e55c6bed156713abe8a5

This pr closely follows the intersection of the original [noise protocol paper](https://noiseprotocol.org/noise.pdf) and the [Protocol Security section](https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md) of the Stratum V2 Specification to provide an implementation of the Noise Protocol on Bitcoin Core. In that way we can now step by step integrate the Stratum protocol Template Provider and Job Declaration Clie
...

💬 ajtowns commented on pull request "contrib/signet/miner updates":
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1711764521)
Added a commit to use mutually exclusive groups here and in a few other places.

💬 achow101 commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#issuecomment-2278320928)
> `-assumevalid` will raise an error when input contains invalid hex characters (including `0x` prefix), or when it is not exactly 64 characters long

Why not allow `0x` prefixes as `-minimumchainwork` does? It seems plausible to me that someone might have put a `0x` prefixed block hash in their conf file and this change breaks that.

👍 pablomartin4btc approved a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2230620169)
> > Tested [d45ac03](https://github.com/bitcoin-core/gui/commit/d45ac03a89cc500cd461f878f092cf8ec99e7760). The "Migrate Wallet" menu item is still disabled when no wallet is loaded.
>
> Should be fixed now.
>

re-ACK bcf47b9f34e4f21e5ac75521ddd4f25ff3812412