💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711453934)
Addressed in https://github.com/hebasto/bitcoin/pull/316.

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2277938747)
This has been open for a few years, but no recent activity: https://issues.guix.gnu.org/56137

💬 sipa commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2277939198)
@paplorinc Huh, this PR shouldn't affect IBD speed at all.

💬 maflcko commented on pull request "test: remove `ExtractDestination` false assertion for `ANCHOR` script":
(https://github.com/bitcoin/bitcoin/pull/30616#issuecomment-2277962247)
(Could wait a day before merging this, to check if OSS-Fuzz also found it, because it should and so far has not)

📝 paplorinc opened a pull request: "test: TrySanitizeHexNumber fizz and unit testing coverage"
(https://github.com/bitcoin/bitcoin/pull/30618)
WIP: depends on https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706637780

Split out the test related recommendations from https://github.com/bitcoin/bitcoin/pull/30569

--------

* https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706637780

Use BOOST_CHECK_EQUAL for std::optional, arith_uint256, uint256, uint160

Example error before:
> unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_o
...

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2277974891)
Once openssl-1 stuff is gone, we can drop the GnuTLS workaround instructions along with it.

It had a similar problem in 3.6.12, but has been updated to 3.7.2. That was included in the [Guix 1.4.0 release](https://guix.gnu.org/en/blog/2022/gnu-guix-1.4.0-released) in late 2022.

(or I could reword the GnuTLS workaround text to refer to openssl 1)

💬 maflcko commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2277975393)
I presume this will be fixed in guix 1.5, but someone claimed if it isn't already fixed, then it is a bug, see https://mail.gnu.org/archive/html/bug-guix/2023-09/msg00112.html

💬 paplorinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1711493167)
Split out to pr [`aec1c90` (#30618)](https://github.com/bitcoin/bitcoin/pull/30618/commits/aec1c901118311eec5dc24104a7439377c8fd6de)

💬 paplorinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1711494297)
Split out to other pr: [`fd6d110` (#30618)](https://github.com/bitcoin/bitcoin/pull/30618/commits/fd6d1106c07d009a9f9517d905feee6f4136d8d7)

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2277984815)
Interesting, I haven't tried deleting `.cache/guix/checkouts` as suggested in that thread. I'm going to wait for guix pull to finish first, then take a look at `guix graph --type=reverse-package openssl-1.1.1l` (`reverse-bag`?)

💬 instagibbs commented on pull request "test: remove `ExtractDestination` false assertion for `ANCHOR` script":
(https://github.com/bitcoin/bitcoin/pull/30616#issuecomment-2277986584)
ACK a4f2b185732649eeea4a042cebd90d0e0e12cc92

this was leftover from when anchor outputs were bare OP_TRUE, which don't have an address

💬 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
...