💬 maflcko commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2277991593)
There is a good chance that a `guix pull` will build the *current* guix first, and then the pulled one, and the reply I linked to is just a wrong guess, but I am not a guix dev, and I don't know if that'd be a bug.

However, if the build isn't fixed with 1.5, then it should be a bug, last time I checked.

👍 tdb3 approved a pull request: "doc, chainparams: 29775 release notes and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/30604#pullrequestreview-2230262209)
re ACK 92c1d7d1f8664fe2d259cc609c87f603609b2b60
The new constant adds clarity.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2277997375)
> (The drahtbot guix build failed due to a silent merge conflict, I presume)

Should be fixed after porting https://github.com/bitcoin/bitcoin/pull/30051.

💬 davidgumberg commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#issuecomment-2278018116)
reACK https://github.com/bitcoin/bitcoin/commit/fac7b7ff7f453a9ba14053a99ce0a113509bf600

👍 BrandonOdiwuor approved a pull request: "test: remove `ExtractDestination` false assertion for `ANCHOR` script"
(https://github.com/bitcoin/bitcoin/pull/30616#pullrequestreview-2230303455)
Code Review ACK a4f2b185732649eeea4a042cebd90d0e0e12cc92

💬 hebasto commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2278035414)
The buildsystem related stuff has been ported to the CMake-based buildsystem in https://github.com/hebasto/bitcoin/pull/317.

💬 Sjors commented on pull request "doc, chainparams: 29775 release notes and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/30604#issuecomment-2278041546)
ACK 92c1d7d1f8664fe2d259cc609c87f603609b2b60

💬 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