💬 hodlinator commented on pull request "validation: fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3520773269)
Thanks for trying this out @Eunovo!

> Also, attempting to reindex while offline wouldn't work anymore if we are below minchainwork.

Would it be acceptable to emit an error and halt, saying that `-reindex` for cases where nodes don't have sufficient block data also requires decreasing `-minimumchainwork=` to whatever level we have in the block data?
Could also explain that this is in order to keep logic consistent regardless of `-reindex`, and also say that running with lowered `-minimumch
...

💬 Sjors commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3520877571)
ACK 866bbb98fd365962840ee99df0d9f7ad557cc025

On macOS 26.0.1 I ran the full test suite with `-DENABLE_IPC` set to `OFF` and `ON`.

💬 janb84 commented on pull request "build: Bump g++ minimum supported version to 12":
(https://github.com/bitcoin/bitcoin/pull/33842#issuecomment-3520878892)
re-ACK fa9f29a4a79944f6ffbb58eab0ac41e243fbeb97

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3520921384)
Here is my Guix build on RISC-V machine:
```
riscv64
451630ecff800ab320a9d5ad8062758df9d700310690110e764c916ae4c8121e guix-build-527acc5ee497/output/dist-archive/bitcoin-527acc5ee497.tar.gz
bf19a3b8e9e9cf609102c38cd6c00dca4d2645f66ae3af591f29fdeceef7b6cf guix-build-527acc5ee497/output/x86_64-w64-mingw32/SHA256SUMS.part
b69dc956f6fd6bb9b4e20e5f4d29eb4ef74e450d250346bf0450cc051b5cfa95 guix-build-527acc5ee497/output/x86_64-w64-mingw32/bitcoin-527acc5ee497-win64-codesigning.tar.gz
9e44c7635
...

🤔 frankomosh reviewed a pull request: "transaction: Adding script witness to ToString for CTxIn"
(https://github.com/bitcoin/bitcoin/pull/33711#pullrequestreview-3452460486)
I think the goal for this is reasonable, but doesn't it remove witness data from `CTransaction::ToString()` and break existing debug output?

💬 frankomosh commented on pull request "transaction: Adding script witness to ToString for CTxIn":
(https://github.com/bitcoin/bitcoin/pull/33711#discussion_r2517605640)
Though I have not done a thorough test, I think that completely removing this might affect debug workflow. Before removing it, `Ctransaction::ToString()` is able to print all inputs, then a separate block with all witnesses, then all outputs. I don’t think this is possible anymore.

💬 frankomosh commented on pull request "transaction: Adding script witness to ToString for CTxIn":
(https://github.com/bitcoin/bitcoin/pull/33711#discussion_r2517569031)
I think there might be an inconsistency in formatting here. `scriptWitness.ToString()` is called directly, while `scriptSig` uses `HexStr().substr(0, 24)`. Shouldn't there be some symmetry ?

💬 quapka commented on issue "Fuzz: compare our AES implementation to AES-NI ":
(https://github.com/bitcoin/bitcoin/issues/27548#issuecomment-3521038218)
> Since cryptofuzz is not alive anymore

There is a bit of activity in [Mozilla's fork](https://github.com/MozillaSecurity/cryptofuzz). It's also still present in [OSS-Fuzz](https://github.com/google/oss-fuzz/tree/master/projects/cryptofuzz), but references the non-existent original repository, so probably is not ran anymore.

👍 hebasto approved a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3452644296)
ACK fa9f29a4a79944f6ffbb58eab0ac41e243fbeb97.

💬 willcl-ark commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3521153141)
I get a guix build failure @ 758da5a5732:

```
g++ -pie -Wl,-z,now -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -DHAVE_CONFIG_H -pie -Wl,-z,now -static-libstdc++ -static-libgcc -o Tcollect2 \
collect2.o collect2-aix.o vec.o ggc
...

💬 maflcko commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3521179236)
I am not using GCC, and I am not sure how many devs are using GCC-15. Is there even a ci task using gcc-15? Maybe Alpine 3.23 later this month? I haven't looked at the build system changes, otherwise this seems fine to do.

💬 hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3521187261)
> and I am not sure how many devs are using GCC-15.

I am :)

💬 l0rinc commented on pull request "validation: fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3521247943)
> Developers doing benchmarks/other work who either have sufficient block data or are okay with lowering -minimumchainwork temporarily

I don't know anyone who does `-reindex` benchmarks anymore - `-reindex-chainstate` is usually enough, `-reindex` isn't representative of IBD so we have stopped relying on it.

> reindex while offline wouldn't work anymore

I haven't checked the code yet (am planning on doing that later), but would it be possible to just issue a warning if we're offline and
...

⚠️ maflcko opened an issue: "depends: native_capnp picks system gcc, not CC/CXX"
(https://github.com/bitcoin/bitcoin/issues/33859)
`native_capnp` seems to ignore the CC/CXX settings. This leads to compile failures on distros that come with an older GCC, such as RHEL-based Linux 8, or the latest OpenSuse Leap:

```
# ( cd depends && make CC=clang CXX=clang++ -j $(nproc) )

Configuring native_capnp...
-- The CXX compiler identification is GNU 7.5.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - failed
-- Check for working CXX compiler: /usr/bin/g++
-- Check for working CXX compiler: /usr/bin/g++ -
...

💬 l0rinc commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3521252813)
> how many devs are using GCC-15

lately I'm also doing most of my benchmarks with 15

🚀 fanquake merged a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842)

💬 hebasto commented on issue "depends: native_capnp picks system gcc, not CC/CXX":
(https://github.com/bitcoin/bitcoin/issues/33859#issuecomment-3521322346)
Native compilers are managed by `build_CC` and `build_CXX` variables, no?

🚀 fanquake merged a pull request: "build: add `-W*-whitespace`"
(https://github.com/bitcoin/bitcoin/pull/32482)

💬 maflcko commented on issue "depends: native_capnp picks system gcc, not CC/CXX":
(https://github.com/bitcoin/bitcoin/issues/33859#issuecomment-3521392898)
thx, i guess some docs about those settings could help. Otherwise, users will have to read the source code.

💬 frankomosh commented on pull request "test: add unit test coverage for the empty leaves path in MerkleComputation":
(https://github.com/bitcoin/bitcoin/pull/33858#issuecomment-3521419961)
> I don't think those links work.
>
I'm not sure why they didn't render correctly here.

> Also, this is redundant to the corecheck result anyway?
>
I included them mainly to visually show % increase, but you’re right that they may be redundant.
(also, I think you have been against adding tests without showing any proof of coverage)