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

💬 l0rinc commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#issuecomment-3521439654)
> Since when is this "unused"?

Did some digging, https://github.com/bitcoin/bitcoin/commit/1f0e7ca09c9d7c5787c218156fa5096a1bdf2ea8#diff-706988c23877f8a557484053887f932b2cafb3b5998b50497ce7ff8118ac85a3L135-L165 inlined `ComputeMerkleRoot` in one case, leaving the `proot` and `pmutated` values unused in `MerkleComputation` (cc: @sipa, @Sjors).
And `BlockWitnessMerkleRoot` was introduced in https://github.com/bitcoin/bitcoin/commit/8b49040854be2e26b66366aeae1cba4716f93d93 where it was already
...

💬 yancyribbens commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#issuecomment-3521446053)
@murchandamus ; would appreciate your input on this.

💬 TheCharlatan commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2517943925)
Might we be able to keep this `ON` too, if the windows job also copies over the dll?
```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index d433646292..e2bd3a6940 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -369,0 +370 @@ jobs:
+ ${{ env.BASE_BUILD_DIR }}/bin/*.dll
```

💬 stickies-v commented on pull request "kernel: add btck_block_tree_entry_equals":
(https://github.com/bitcoin/bitcoin/pull/33855#discussion_r2517974921)
The fact that `CBlockIndex` equality is currently defined as pointer equality seems like an implementation detail, tightly coupled with our block tree implementation. Exposing that as part of our public interface seems brittle and unergonomic?

Logically, I think two `BlockTreeEntry` objects are equal if they point to the same `Block`. I've force-pushed a slight doc update to reflect that.