🤔 w0xlt reviewed a pull request: "test: sock: Enable socket pair tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/33506#pullrequestreview-3287055021)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33506#pullrequestreview-3287055021)
Concept ACK
🤔 pablomartin4btc reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3287139328)
post-merge ACK https://github.com/bitcoin/bitcoin/commit/fc861332b351c9390400054ff74193ce26eb0713
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3287139328)
post-merge ACK https://github.com/bitcoin/bitcoin/commit/fc861332b351c9390400054ff74193ce26eb0713
📝 l0rinc opened a pull request: "Use number of dirty cache entries in flush warnings/logs"
(https://github.com/bitcoin/bitcoin/pull/33512)
Revival of https://github.com/bitcoin/bitcoin/pull/31703 with all outstanding review feedback addressed.
<details>
<summary>Changes compared to the original</summary>
* `EmplaceCoinInternalDANGER` is tested now and memory accounting is fixed
* Warning and logging moved from `FlushStateToDisk` to `CCoinsViewDB::BatchWrite`, ensuring AssumeUTXO snapshot writes also display progress warnings
* Removed the redundant changed counter in `BatchWrite` since we now track `dirty_count` directly
...
(https://github.com/bitcoin/bitcoin/pull/33512)
Revival of https://github.com/bitcoin/bitcoin/pull/31703 with all outstanding review feedback addressed.
<details>
<summary>Changes compared to the original</summary>
* `EmplaceCoinInternalDANGER` is tested now and memory accounting is fixed
* Warning and logging moved from `FlushStateToDisk` to `CCoinsViewDB::BatchWrite`, ensuring AssumeUTXO snapshot writes also display progress warnings
* Removed the redundant changed counter in `BatchWrite` since we now track `dirty_count` directly
...
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#issuecomment-3354466151)
Addressed all outstanding feedback in https://github.com/bitcoin/bitcoin/pull/33512
(https://github.com/bitcoin/bitcoin/pull/31703#issuecomment-3354466151)
Addressed all outstanding feedback in https://github.com/bitcoin/bitcoin/pull/33512
💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3354492388)
Thanks @sipa for the review, @achow101 for the comment.
I have pushed an updated version, extended the optimization from just `SipHashUint256Extra` to cover all `SipHash` operations on `uint256`:
* Renamed and unified: `Uint256ExtraSipHasher` → `PresaltedSipHasher`
* All `Salted*Hasher` classes now use `PresaltedSipHasher` internally, so the compact block short ID computations should also be slightly faster after this.
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3354492388)
Thanks @sipa for the review, @achow101 for the comment.
I have pushed an updated version, extended the optimization from just `SipHashUint256Extra` to cover all `SipHash` operations on `uint256`:
* Renamed and unified: `Uint256ExtraSipHasher` → `PresaltedSipHasher`
* All `Salted*Hasher` classes now use `PresaltedSipHasher` internally, so the compact block short ID computations should also be slightly faster after this.
💬 davidgumberg commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3354496291)
Tested ACK https://github.com/bitcoin/bitcoin/pull/33434/commits/eca50854e1cb04e20478bd3df4762e18520a3611
I tested doing a guix build of this branch on another x86_64 device running Fedora 41, and on an x86 Fedora 41 virtual machine and an x86 Fedora 42 virtual machine (both hosted on the troublesome machine mentioned above).
The flickering issue described in #33432 occurs on both Fedora 41 and Fedora 42 on both master and the distro-packaged 29.1, but a guix build of this branch fixes the
...
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3354496291)
Tested ACK https://github.com/bitcoin/bitcoin/pull/33434/commits/eca50854e1cb04e20478bd3df4762e18520a3611
I tested doing a guix build of this branch on another x86_64 device running Fedora 41, and on an x86 Fedora 41 virtual machine and an x86 Fedora 42 virtual machine (both hosted on the troublesome machine mentioned above).
The flickering issue described in #33432 occurs on both Fedora 41 and Fedora 42 on both master and the distro-packaged 29.1, but a guix build of this branch fixes the
...
💬 fanquake commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354509363)
https://github.com/bitcoin/bitcoin/actions/runs/18149289661/job/51656823259?pr=33512#step:9:2059:
```bash
Run coins_view_db with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1436700023
INFO: Loaded 1 modules (611994 inline 8-bit counters): 611994 [0x6082fc8a27f8, 0x6082fc937e92),
INFO: Loaded 1
...
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354509363)
https://github.com/bitcoin/bitcoin/actions/runs/18149289661/job/51656823259?pr=33512#step:9:2059:
```bash
Run coins_view_db with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1436700023
INFO: Loaded 1 modules (611994 inline 8-bit counters): 611994 [0x6082fc8a27f8, 0x6082fc937e92),
INFO: Loaded 1
...
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354512593)
@fanquake the fuzz test is fixed in a [separate PR](https://github.com/bitcoin/bitcoin/pull/32313), I have reverted a similar attempt here
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354512593)
@fanquake the fuzz test is fixed in a [separate PR](https://github.com/bitcoin/bitcoin/pull/32313), I have reverted a similar attempt here
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3354814334)
Updated 21b0503c2f19f5e4662cea1ceecb425b8460967b -> f3ca1d65b0444097fd0e2f059a01eb5b29c950c1 ([kernelApi_67](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_67) -> [kernelApi_68](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_68), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_67..kernelApi_68))
* Improved docstrings for processing blocks.
* Added three more validation interface callbacks, which should make it a bit easier to use when implementing a blocks
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3354814334)
Updated 21b0503c2f19f5e4662cea1ceecb425b8460967b -> f3ca1d65b0444097fd0e2f059a01eb5b29c950c1 ([kernelApi_67](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_67) -> [kernelApi_68](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_68), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_67..kernelApi_68))
* Improved docstrings for processing blocks.
* Added three more validation interface callbacks, which should make it a bit easier to use when implementing a blocks
...
💬 maflcko commented on pull request "ci: Remove bash -c from cmake invocation using eval":
(https://github.com/bitcoin/bitcoin/pull/33401#issuecomment-3354869230)
lgtm ACK 50194029e7c2581b751931080f5999785a39929f
(https://github.com/bitcoin/bitcoin/pull/33401#issuecomment-3354869230)
lgtm ACK 50194029e7c2581b751931080f5999785a39929f
💬 brainx commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2393557892)
Bitcoin ❤️
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2393557892)
Bitcoin ❤️
💬 BrandonOdiwuor commented on pull request "ci: Remove bash -c from cmake invocation using eval":
(https://github.com/bitcoin/bitcoin/pull/33401#discussion_r2393578871)
fixed
(https://github.com/bitcoin/bitcoin/pull/33401#discussion_r2393578871)
fixed
👍 hodlinator approved a pull request: "build: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3287946924)
re-ACK 1aaaaa078bb2efed126e3f41ecf7c81ccf005818
Bumped version in 3 more places and dropped fuzz-workaround.
```
a9cdca5dc929d3c6c937e0a9069f2607552283087c51c4fac7921531a4b97bbf guix-build-1aaaaa078bb2/output/arm64-apple-darwin/SHA256SUMS.part
673dcca25defb6d09d5ffb9aa9bec86884d445bc15c94d191bb81b06bc13b0a1 guix-build-1aaaaa078bb2/output/arm64-apple-darwin/bitcoin-1aaaaa078bb2-arm64-apple-darwin-codesigning.tar.gz
674cfe724d57aeb2afaff18195347c90b347565d800ede2842d42ca04b1a9023 guix-
...
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3287946924)
re-ACK 1aaaaa078bb2efed126e3f41ecf7c81ccf005818
Bumped version in 3 more places and dropped fuzz-workaround.
```
a9cdca5dc929d3c6c937e0a9069f2607552283087c51c4fac7921531a4b97bbf guix-build-1aaaaa078bb2/output/arm64-apple-darwin/SHA256SUMS.part
673dcca25defb6d09d5ffb9aa9bec86884d445bc15c94d191bb81b06bc13b0a1 guix-build-1aaaaa078bb2/output/arm64-apple-darwin/bitcoin-1aaaaa078bb2-arm64-apple-darwin-codesigning.tar.gz
674cfe724d57aeb2afaff18195347c90b347565d800ede2842d42ca04b1a9023 guix-
...
💬 hodlinator commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2393728354)
nit: This change to ci.yml in this PR should probably be broken out to a separate commit or mentioned in the commit message as now it's just "Drop support for EOL macOS 13".
Cheers, will check out 33509.
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2393728354)
nit: This change to ci.yml in this PR should probably be broken out to a separate commit or mentioned in the commit message as now it's just "Drop support for EOL macOS 13".
Cheers, will check out 33509.
💬 Sjors commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355157444)
> @Sjors Is there something else I should check to try to reproduce the issue you had?
I have no idea. One factor might have been my guix version:
```
guix (GNU Guix) 20dbf225f332ccc707578263ed710dcf2a8fb78e
Copyright (C) 2024 the Guix authors
```
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355157444)
> @Sjors Is there something else I should check to try to reproduce the issue you had?
I have no idea. One factor might have been my guix version:
```
guix (GNU Guix) 20dbf225f332ccc707578263ed710dcf2a8fb78e
Copyright (C) 2024 the Guix authors
```
🤔 hodlinator reviewed a pull request: "node: Add --debug_runs/-waitfordebugger + --debug_cmd"
(https://github.com/bitcoin/bitcoin/pull/31723#pullrequestreview-3282841954)
Latest push adds a functional test and breaks apart commits somewhat.
(https://github.com/bitcoin/bitcoin/pull/31723#pullrequestreview-3282841954)
Latest push adds a functional test and breaks apart commits somewhat.
💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392681962)
Added a test for Linux (the only OS where we explicitly *enable* attaching) in the latest push.
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392681962)
Added a test for Linux (the only OS where we explicitly *enable* attaching) in the latest push.
💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392687192)
New test includes `assert_start_raises_init_error()` at the end for this precisely.
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392687192)
New test includes `assert_start_raises_init_error()` at the end for this precisely.
💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2390087376)
That's a clever way of doing it, did something very close to it now (hopefully it's small enough that source code license issues don't apply).
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2390087376)
That's a clever way of doing it, did something very close to it now (hopefully it's small enough that source code license issues don't apply).
💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2390038435)
Did something similar to https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389662377
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2390038435)
Did something similar to https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389662377