💬 yuvicc commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#discussion_r2545063204)
I think we also expose `_copy` API here:

```suggestion

/**
* @brief Copies the block validation state.
*
* @param[in] block_validation_state Non-null.
* @return The copied block validation state.
*/
BITCOINKERNEL_API btck_BlockValidationState* BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_validation_state_copy(
const btck_BlockValidationState* block_validation_state) BITCOINKERNEL_ARG_NONNULL(1);

```

💬 yuvicc commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#discussion_r2545075148)
Also here:

```suggestion
explicit BlockValidationState() : Handle{btck_block_validation_state_create()} {}
```

💬 fanquake commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#discussion_r2545130316)
I think the issue was updating the packages in lockstep. I'll cherry-pick the two commits, with some adjustments, i.e to remove `--without-launchd`.

💬 fanquake commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#discussion_r2545150228)
There's a related report to libxcb here: https://gitlab.freedesktop.org/xorg/lib/libxcb/-/merge_requests/25.

👍 vasild approved a pull request: "refactor: remove incorrect lifetimebounds"
(https://github.com/bitcoin/bitcoin/pull/33870#pullrequestreview-3486800310)
ACK 99d012ec80a4415e1a37218fb4933550276b9a0a

Reviewing is learning, I love it! Thanks!

📝 maflcko opened a pull request: "clang-format: Set PackConstructorInitializers: CurrentLine"
(https://github.com/bitcoin/bitcoin/pull/33912)
Now that the minimum supported clang version is 17, the `PackConstructorInitializers` setting can be set to `CurrentLine` in the clang-format file. (https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#packconstructorinitializers)

The `CurrentLine` option will either put all constructor initializers on the current line if they fit. Otherwise, it will put each one on its own line.

The `CurrentLine` option is desirable over the current `BinPack` option, because:


...

💬 maflcko commented on pull request "refactor: remove incorrect lifetimebounds":
(https://github.com/bitcoin/bitcoin/pull/33870#discussion_r2545299017)
I don't think the tip can be nullptr in rpc code. This can just be a `CHECK_NONFATAL`, but this seems unrelated to lifetimebound stuff?

💬 l0rinc commented on pull request "refactor: remove incorrect lifetimebounds":
(https://github.com/bitcoin/bitcoin/pull/33870#discussion_r2545337568)
> this seems unrelated to lifetimebound stuff

Yes, he started with:

> An off-topic nit

💬 maflcko commented on pull request "doc: Improve CI docs on env and qemu-user-static":
(https://github.com/bitcoin/bitcoin/pull/33887#discussion_r2545357404)
reminds me that the `bash -c` isn't needed here, now that `env` is used. Feel free to remove, or ignore the unrelated nit.

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3557161480)
ACK de4242f47476769d0a7f3e79e8297ed2dd60d9a4

Only a [const removal](https://github.com/bitcoin/bitcoin/compare/9a387e9175d34d1eaf83b9008f5e857d1eb04b8e..de4242f47476769d0a7f3e79e8297ed2dd60d9a4?diff=unified&w) since my last ACK

💬 maflcko commented on pull request "doc: Improve CI docs on env and qemu-user-static":
(https://github.com/bitcoin/bitcoin/pull/33887#discussion_r2545381694)
maybe unrelated, because I think it was working at some point, but using `podman` on various distros (Debian/Ubuntu/OpenSuse) to try to run the s390x task fails with a segfault in the depends compilation:

```
...
[58/817] Building CXX object qtbase/src/tools/bootstrap/CMakeFiles/Bootstrap.dir/__/__/corelib/io/qfsfileengine_iterator.cpp.o
[59/817] Building CXX object qtbase/src/tools/bootstrap/CMakeFiles/Bootstrap.dir/__/__/corelib/io/qloggingcategory.cpp.o
FAILED: qtbase/src/tools/bootstrap/C
...

👍 l0rinc approved a pull request: "clang-format: Set PackConstructorInitializers: CurrentLine"
(https://github.com/bitcoin/bitcoin/pull/33912#pullrequestreview-3487068101)
ACK fad0c76d0a109ab7b063f0d405588cf1e6c15e4d

I agree that this is our best option now, mostly because it will minimize diffs

💬 l0rinc commented on pull request "clang-format: Set PackConstructorInitializers: CurrentLine":
(https://github.com/bitcoin/bitcoin/pull/33912#discussion_r2545431725)
> Now that the minimum supported clang version is 17, the PackConstructorInitializers setting can be set to CurrentLine in the clang-format file.

Nit: `CurrentLine` was already available in [16](https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#packconstructorinitializers)
<img width="967" height="703" alt="Image" src="https://github.com/user-attachments/assets/4e1fb719-9f19-496d-b184-9d869cc27a33" />

The new addition in [17](https://releases.llvm.org/17.0.1/tools/
...

💬 fanquake commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3557317662)
> This [branch](https://github.com/hebasto/bitcoin/tree/251119-qt6.9) fails to cross-compile on Debian Bookworm with GCC 12.2.0:

That's the same issue we work around in CMake, which is fixed by just turning off stack-clash-protection: https://github.com/fanquake/bitcoin/tree/qt_6_9_no_stack_clash_win. However it looks like there are other issues, possibly due to https://github.com/qt/qtbase/commit/d25d9f2c2673bc287590d9a83bd7ef1357d7021a#diff-4ae1a340d7e79a6402009cb635708b4950b1633f4bc850aba4
...

💬 maflcko commented on pull request "clang-format: Set PackConstructorInitializers: NextLineOnly":
(https://github.com/bitcoin/bitcoin/pull/33912#discussion_r2545495948)
Thx, nice find. `NextLineOnly` is actually even closer to what this project is using. I've switched to that

⚠️ fanquake opened an issue: "ci: failure to download 0.14.3-win64.zip in Win test cross-built"
(https://github.com/bitcoin/bitcoin/issues/33913)
This is happening semi-frequently:

https://github.com/bitcoin/bitcoin/actions/runs/19520873587/job/55885060054:
```bash
Run ./test/get_previous_releases.py --target-dir $PREVIOUS_RELEASES_DIR

Download failed: Remote end closed connection without response
Releases directory: D:\a\_temp/previous_releases
Fetching: https://bitcoincore.org/bin/bitcoin-core-0.14.3/bitcoin-0.14.3-win64.zip
Error: Process completed with exit code 1.
```

https://github.com/bitcoin/bitcoin/actions/runs/19239483669/job
...

💬 l0rinc commented on pull request "clang-format: Set PackConstructorInitializers: CurrentLine":
(https://github.com/bitcoin/bitcoin/pull/33912#discussion_r2545533671)
I'm fine with both, just want, agree that `BinPack` is not the best one - can you please update the PR description to reflect that the current one wasn't necessarily motivated by the 16 -> 17 upgrade.

📝 Sjors opened a pull request: "Add string literal convenience overload to Parse()"
(https://github.com/bitcoin/bitcoin/pull/33914)
While investigating a silent merge conflict in #33135 I noticed that #32983 changed the descriptor `Parse` function signature from `const std::string& descriptor` to `std::span<const char> descriptor`.

Calling that new version of `Parse` with a string literal will trigger a confusing "Invalid characters in payload" due to the trailing "\0".

It can be worked around by having (the test) wrap string literals in `std::string()`, but it seems better to either disallow literal strings or have a
...

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3557518664)
#33914 fixes this. I based this PR on it.

💬 Sjors commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#issuecomment-3557529213)
See #33914 for a followup.