💬 l0rinc commented on pull request "[CI] remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3592432635)
The release notes is kinda' important to check for spelling errors - could we maybe add the exceptional contributor names to https://github.com/bitcoin/bitcoin/blob/master/test/lint/spelling.ignore-words.txt instead?
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3592432635)
The release notes is kinda' important to check for spelling errors - could we maybe add the exceptional contributor names to https://github.com/bitcoin/bitcoin/blob/master/test/lint/spelling.ignore-words.txt instead?
💬 hebasto commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3592479046)
The feedback from @m3dwards has been addressed.
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3592479046)
The feedback from @m3dwards has been addressed.
💬 hebasto commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2573603087)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3592479046).
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2573603087)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3592479046).
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3592480696)
> What is the plan for adding CI, as that blocks everything here?
The UCRT CI job is up [now](https://github.com/bitcoin/bitcoin/pull/33764).
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3592480696)
> What is the plan for adding CI, as that blocks everything here?
The UCRT CI job is up [now](https://github.com/bitcoin/bitcoin/pull/33764).
💬 l0rinc 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-3592487964)
Tried getting to the bottom of it, something's really off here, two consecutive calls to `std::source_location::current()` return very different and suspicious values:
<img width="902" height="584" alt="Image" src="https://github.com/user-attachments/assets/cbd0798c-c86a-44f2-8874-618a2676dff8" />
What does `BTC` file name and `sat` function name refer to and why do we have millions of lines :/?
They seem to come from (modifying these strings updates the file and method names):
https://github.
...
(https://github.com/bitcoin/bitcoin/issues/33964#issuecomment-3592487964)
Tried getting to the bottom of it, something's really off here, two consecutive calls to `std::source_location::current()` return very different and suspicious values:
<img width="902" height="584" alt="Image" src="https://github.com/user-attachments/assets/cbd0798c-c86a-44f2-8874-618a2676dff8" />
What does `BTC` file name and `sat` function name refer to and why do we have millions of lines :/?
They seem to come from (modifying these strings updates the file and method names):
https://github.
...
📝 hebasto opened a pull request: "cmake: Set `WITH_ZMQ` to `ON` in Windows presets"
(https://github.com/bitcoin/bitcoin/pull/33971)
The "zeromq" feature is already enabled by default in `vcpkg.json`, and there appears to be no reason to omit this configuration option when building on Windows.
(https://github.com/bitcoin/bitcoin/pull/33971)
The "zeromq" feature is already enabled by default in `vcpkg.json`, and there appears to be no reason to omit this configuration option when building on Windows.
📝 hebasto opened a pull request: "cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB`"
(https://github.com/bitcoin/bitcoin/pull/33972)
The CMake script in the `test/kernel` subdirectory is already gated by `BUILD_KERNEL_LIB`:https://github.com/bitcoin/bitcoin/blob/f6acbef1084e34f126bf530df99e4ef6a11c38e8/src/CMakeLists.txt#L405-L409
As a result, the following configuration summary is misleading:
```
$ cmake -B build -DBUILD_KERNEL_LIB=OFF -DBUILD_KERNEL_TEST=ON
<snip>
bitcoin-chainstate (experimental) ... OFF
libbitcoinkernel (experimental) ..... OFF
kernel-test (experimental) .......... ON
<snip>
```
This P
...
(https://github.com/bitcoin/bitcoin/pull/33972)
The CMake script in the `test/kernel` subdirectory is already gated by `BUILD_KERNEL_LIB`:https://github.com/bitcoin/bitcoin/blob/f6acbef1084e34f126bf530df99e4ef6a11c38e8/src/CMakeLists.txt#L405-L409
As a result, the following configuration summary is misleading:
```
$ cmake -B build -DBUILD_KERNEL_LIB=OFF -DBUILD_KERNEL_TEST=ON
<snip>
bitcoin-chainstate (experimental) ... OFF
libbitcoinkernel (experimental) ..... OFF
kernel-test (experimental) .......... ON
<snip>
```
This P
...
💬 hebasto commented on pull request "cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB`":
(https://github.com/bitcoin/bitcoin/pull/33972#issuecomment-3592564348)
cc @sedited
(https://github.com/bitcoin/bitcoin/pull/33972#issuecomment-3592564348)
cc @sedited
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573674644)
In commit "Remove unused argument to RemoveStaged"
Nit: strange and unrelated style change.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573674644)
In commit "Remove unused argument to RemoveStaged"
Nit: strange and unrelated style change.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573683712)
In commit "Rewrite removeForReorg to avoid using sets"
Nit: it seems this scope level is unnecessary.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573683712)
In commit "Rewrite removeForReorg to avoid using sets"
Nit: it seems this scope level is unnecessary.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573742109)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"
The word "maximize" seems out of place here.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573742109)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"
The word "maximize" seems out of place here.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573685509)
In commit "Rewrite removeForReorg to avoid using sets"
Nit: given that you're rewriting most of this function anyway, make this variable name match style?
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573685509)
In commit "Rewrite removeForReorg to avoid using sets"
Nit: given that you're rewriting most of this function anyway, make this variable name match style?
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573704140)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"
Writing nit: `ie,` -> `i.e.,`
Also in two other places below.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573704140)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"
Writing nit: `ie,` -> `i.e.,`
Also in two other places below.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573747087)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"
Nice write-up, enough to understand the terminology used in the codebase, but not going to detailed into the rationale.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573747087)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"
Nice write-up, enough to understand the terminology used in the codebase, but not going to detailed into the rationale.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573755332)
In commit "doc: add release notes snippet for cluster mempool"
Add "any combination of"? It's noted elsewhere, but the release notes are likely read by far more people.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573755332)
In commit "doc: add release notes snippet for cluster mempool"
Add "any combination of"? It's noted elsewhere, but the release notes are likely read by far more people.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573752182)
In commit "doc: add release notes snippet for cluster mempool"
I wouldn't say "better performance". While it's true many things are more performant now, that's not the motivation, and in many cases, not really critical anyway. How about "better decision-making"?
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573752182)
In commit "doc: add release notes snippet for cluster mempool"
I wouldn't say "better performance". While it's true many things are more performant now, that's not the motivation, and in many cases, not really critical anyway. How about "better decision-making"?
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573756538)
In commit "doc: add release notes snippet for cluster mempool"
Writing nit: `eg` -> `e.g.,`.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573756538)
In commit "doc: add release notes snippet for cluster mempool"
Writing nit: `eg` -> `e.g.,`.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573692555)
In commit " Warn user if using -limitancestorsize/-limitdescendantsize that the options have no effect"
Perhaps mention the relevant corresponding option names (`-limitclustercount` and `-limitclustersize`).
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573692555)
In commit " Warn user if using -limitancestorsize/-limitdescendantsize that the options have no effect"
Perhaps mention the relevant corresponding option names (`-limitclustercount` and `-limitclustersize`).
📝 billymcbip opened a pull request: "test: Add DERSIG unit tests to script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973)
I'd like to add a few `DERSIG` variants of existing `STRICTENC` tests, since `DERSIG` is a consensus flag.
I ran:
```
cmake -B build -DENABLE_WALLET=OFF
cmake --build build --parallel 8
ctest --test-dir build --parallel 8
```
```
6/133 Test #93: script_tests ......................... Passed 1.57 sec
```
(https://github.com/bitcoin/bitcoin/pull/33973)
I'd like to add a few `DERSIG` variants of existing `STRICTENC` tests, since `DERSIG` is a consensus flag.
I ran:
```
cmake -B build -DENABLE_WALLET=OFF
cmake --build build --parallel 8
ctest --test-dir build --parallel 8
```
```
6/133 Test #93: script_tests ......................... Passed 1.57 sec
```
💬 JeremyRubin commented on pull request "[CI] remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3592667408)
I think it's a separate issue -- since it seems that the release (e.g., non code doc changes pre-release) is maybe not getting run through CI anyways, or else this would have been caught?
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3592667408)
I think it's a separate issue -- since it seems that the release (e.g., non code doc changes pre-release) is maybe not getting run through CI anyways, or else this would have been caught?