💬 yuvicc commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#issuecomment-3586940490)
- Addressed some clang-format nits
- Updated the comments to use struct name instead of raw names as suggested by @stickies-v.

💬 maflcko commented on pull request "ubsan: add another suppression for InsecureRandomContext":
(https://github.com/bitcoin/bitcoin/pull/33949#issuecomment-3586968767)
To explain my comment: On this pull and current master the compile flags are: `C++ compiler flags .................... -ftrivial-auto-var-init=pattern -O2 -g -std=c++20 ...`

On the failing push, they are: `C++ compiler flags .................... -ftrivial-auto-var-init=pattern -O2 -std=c++20 ...` (with `-g` removed)

So I fail to see why the correct fix is to add a suppression. See also the docs:

https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#runtime-suppressions

> Someti
...

💬 cobratbq commented on pull request "docs: sync external-signer.md with current external signer flow":
(https://github.com/bitcoin/bitcoin/pull/33947#issuecomment-3587011977)
I tried to include some of your work in #33765. Feel free to comment or collaborate.

📝 l0rinc opened a pull request: "test: tighten `V2TransportTester::SendMessage` bounds for GCC 15.2"
(https://github.com/bitcoin/bitcoin/pull/33963)
### Summary
#33840 adjusted V2TransportTester::SendMessage to avoid a `-Warray-bounds` false positive with **GCC 13**, but **GCC 15.2** still warns.

### Fix

Implemented the suggestion in https://github.com/bitcoin/bitcoin/pull/33840#pullrequestreview-3447477940:
- take `mtype` as `std::string_view`;
- assert `mtype.size() <= CMessageHeader::MESSAGE_TYPE_SIZE`;
- build `[0x00][12-byte type][payload]` via `std::ranges::copy`.

### Reproducer

Before this change, using GCC 15.2 on a M
...

⚠️ l0rinc opened an issue: "Startup crash on macOS with GCC 15.2: `std::source_location::file_name()` is `nullptr`"
(https://github.com/bitcoin/bitcoin/issues/33964)
### Summary

`bitcoind` crashes on startup when built with Homebrew `GCC 15.2.0 `on macOS (arm64).
The crash occurs inside `SourceLocationHasher::operator()` when constructing a `std::basic_string_view` from `std::source_location::file_name()`.

The same commit runs successfully when built with the default `AppleClang` toolchain.

### Versions

* Bitcoin Core: `master` at `808f1d972be35f4c66bdc30ab0f4060dab0c43c0`
* Platform: macOS 26.1 (25B78), Apple M4 Pro (arm64)
* CMake: `cmake version 4.2.0
...

👍 sedited approved a pull request: "kernel: Add block header support and validation"
(https://github.com/bitcoin/bitcoin/pull/33822#pullrequestreview-3516825286)
re-ACK 30367c1b3e7e94ed59d0a3ee816da14a3ee0424f

💬 maflcko commented on pull request "test: tighten `V2TransportTester::SendMessage` bounds for GCC 15.2":
(https://github.com/bitcoin/bitcoin/pull/33963#issuecomment-3587196960)
I don't understand this change:

* The tighter bounds check does not fix the false positive warning: https://godbolt.org/z/bn4sn5ehx
* Using ranges fixes it, but likely only because they break the array-bounds analysis.
* There is no CI task that runs into this warning
* I can't reproduce this on GCC 15.2 on Linux: https://github.com/maflcko/bitcoin-core-nightly/actions/runs/19735442483/job/56546112570#step:5:8
* It reproduces with GCC 16, but there are many more warnings: https://github.c
...

💬 hodlinator commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569912274)
I'm fine with including the leaf construction, but that's not my concern in this case.

`mutation`/`mutated` is used here:
https://github.com/bitcoin/bitcoin/blob/14a4dca89adaab6f1ad42af6716f9065f1abe2e1/src/validation.cpp#L3944-L3961

Which leads to this being run on every iteration through the outer `while`:
https://github.com/bitcoin/bitcoin/blob/14a4dca89adaab6f1ad42af6716f9065f1abe2e1/src/consensus/merkle.cpp#L46-L53

✅ l0rinc closed a pull request: "test: tighten `V2TransportTester::SendMessage` bounds for GCC 15.2"
(https://github.com/bitcoin/bitcoin/pull/33963)

💬 l0rinc commented on pull request "test: tighten `V2TransportTester::SendMessage` bounds for GCC 15.2":
(https://github.com/bitcoin/bitcoin/pull/33963#issuecomment-3587209651)
Thanks for checking, it seems we have to be systematic about this.

💬 maflcko commented on issue "Startup crash on macOS with GCC 15.2: `std::source_location::file_name()` is `nullptr`":
(https://github.com/bitcoin/bitcoin/issues/33964#issuecomment-3587213137)
Your toolchain clearly violates the C++ standard (https://eel.is/c++draft/support.srcloc#class.general-2), so you'll have to report this upstream via a minimal reproducer.

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569947721)
`ComputeMerkleRoot` is usually called without mutation checks:
https://github.com/bitcoin/bitcoin/blob/fa283d28e261416f0785450ab687af199103776a/src/validation.cpp#L3929-L3932

Most existing mutation checks are noops, see: https://github.com/bitcoin/bitcoin/pull/33805.
The benchmark was modeling pre-segwit `BlockMerkleRoot` before, and now it's closer to the behavior of `BlockWitnessMerkleRoot`.

I haven't measured it explicitly, but the latter should be more representative of modern usage - plea
...

💬 joshdoman commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2569950802)
An explicit test makes sense. Just added one.

💬 joshdoman commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2569952140)
Good suggestion, made that change.

💬 joshdoman commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2569953795)
No problem.

💬 joshdoman commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2569960162)
To try to mitigate this concern, I renamed the parameter `precomputed_txdata`, to make it more clear that it pertains to the actual transaction being verified (previously the parameter was more vaguely named `precomputed_data`).

I also updated the parameter description in the documentation for `btck_script_pubkey_verify`, to make it explicit that the parameter is derived from `tx_to` and the spent outputs.

Hopefully these changes help, but open to other suggestions!

💬 joshdoman commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2569960497)
Done!

💬 hodlinator commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2569972760)
Explored that now, but we still have the ci-test-each-commit-exec.py script...

https://github.com/bitcoin/bitcoin/blob/d14d1b52b51c4e8e2388af7e54c854e8a133e088/.github/workflows/ci.yml#L113

...and unlike the `name`, this job "id" is not externally facing, so I'd rather not touch it.

💬 hodlinator commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2569986901)
I've spent more time than I care to admit together with an underpowered LLM and StackOverflow to come up with ways to avoid duplicating the number 6 in both the job's `name` and the `MAX_COUNT` constant.

Haven't found a way to access `env.*` when setting `name`, or to access `name` when setting `env.*` (`github.job` returns "test-each-commit"). I've also explored doing it through YAML node aliases, but that doesn't seem to work either.

I want to specify the 6 in the name in order to make i
...

💬 l0rinc commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2570013937)
Why is it better to duplicate than to abstract the above to what we should pay attention to? It's not the 6 that matters, but that not all of them, so why not focus on that instead?