💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3556680990)
`6e0f3a4a58...47f4f65d0c`: rebase due to conflicts
Is the local failure you observe reproducible or sporadic? If it is reproducible maybe you can nail down which test is making the traffic? Previously to find the offending test I bisected the list of tests (used to nail down the traffic from `node_init_tests/init_test` which is fixed in this PR). Here is a write-only script to list all tests (681 currently) and run only e.g. from 1 to 340:
```sh
BUILD/bin/test_bitcoin $(BUILD/bin/test_bit
...
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3556680990)
`6e0f3a4a58...47f4f65d0c`: rebase due to conflicts
Is the local failure you observe reproducible or sporadic? If it is reproducible maybe you can nail down which test is making the traffic? Previously to find the offending test I bisected the list of tests (used to nail down the traffic from `node_init_tests/init_test` which is fixed in this PR). Here is a write-only script to list all tests (681 currently) and run only e.g. from 1 to 340:
```sh
BUILD/bin/test_bitcoin $(BUILD/bin/test_bit
...
🤔 yuvicc reviewed a 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#pullrequestreview-3486594405)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/33908#pullrequestreview-3486594405)
Approach ACK
💬 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_r2545072096)
And then use `Handle` instead of `UniqueHandle` here:
```suggestion
class BlockValidationState : public Handle<btck_BlockValidationState, btck_block_validation_state_copy, btck_block_validation_state_destroy>, public BlockValidationStateApi<BlockValidationState>
```
(https://github.com/bitcoin/bitcoin/pull/33908#discussion_r2545072096)
And then use `Handle` instead of `UniqueHandle` here:
```suggestion
class BlockValidationState : public Handle<btck_BlockValidationState, btck_block_validation_state_copy, btck_block_validation_state_destroy>, public BlockValidationStateApi<BlockValidationState>
```
💬 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);
```
(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()} {}
```
(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`.
(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.
(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!
(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:
...
(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?
(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
(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.
(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
(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
...
(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
(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/
...
(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
...
(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
(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
...
(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.
(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.