✅ pinheadmz closed a pull request: "doc: fix typo in MAX_MONEY comment"
(https://github.com/bitcoin/bitcoin/pull/33970)
(https://github.com/bitcoin/bitcoin/pull/33970)
💬 pinheadmz commented on pull request "doc: fix typo in MAX_MONEY comment":
(https://github.com/bitcoin/bitcoin/pull/33970#issuecomment-3591960468)
Please don't open new pull requests just for tiny typo fixes, they are a drag on our integration testing infrastructure, maintainer time and reviewer time. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring:
> Trivial pull requests or pull requests that refactor the code with no clear benefits may be immediately closed by the maintainers to reduce unnecessary workload on reviewing.
There are many more significant ways to contribute to Bitcoin -- for example, loo
...
(https://github.com/bitcoin/bitcoin/pull/33970#issuecomment-3591960468)
Please don't open new pull requests just for tiny typo fixes, they are a drag on our integration testing infrastructure, maintainer time and reviewer time. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring:
> Trivial pull requests or pull requests that refactor the code with no clear benefits may be immediately closed by the maintainers to reduce unnecessary workload on reviewing.
There are many more significant ways to contribute to Bitcoin -- for example, loo
...
💬 151henry151 commented on pull request "Defer transaction signing until user clicks Send":
(https://github.com/bitcoin-core/gui/pull/915#issuecomment-3591963820)
@achow101 would you like to check this out and let me know if it looks OK?
(https://github.com/bitcoin-core/gui/pull/915#issuecomment-3591963820)
@achow101 would you like to check this out and let me know if it looks OK?
💬 151henry151 commented on pull request "Check required interfaces before generating manpages":
(https://github.com/bitcoin/bitcoin/pull/33828#issuecomment-3591967154)
> This duplicates #33085. If the author doesn't follow up there shortly, we could reopen here.
It's been a few weeks, would it be appropriate to reopen here? I did try to reach out to @BrandonOdiwuor on the comments of #33085 and haven't heard back from him yet.
(https://github.com/bitcoin/bitcoin/pull/33828#issuecomment-3591967154)
> This duplicates #33085. If the author doesn't follow up there shortly, we could reopen here.
It's been a few weeks, would it be appropriate to reopen here? I did try to reach out to @BrandonOdiwuor on the comments of #33085 and haven't heard back from him yet.
💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#issuecomment-3592108964)
I believe I've responded to all the previous review comments, so this should be ready for re-review.
Also fyi, I do have some additional changes to propose beyond what's included here already, but I think I'll stop adding additional scope so that this PR can make forward progress with the improvements already suggested.
(https://github.com/bitcoin/bitcoin/pull/33591#issuecomment-3592108964)
I believe I've responded to all the previous review comments, so this should be ready for re-review.
Also fyi, I do have some additional changes to propose beyond what's included here already, but I think I'll stop adding additional scope so that this PR can make forward progress with the improvements already suggested.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348088)
Hmm the thread is just an input fetcher though. That's all it does. I like this name for it.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348088)
Hmm the thread is just an input fetcher though. That's all it does. I like this name for it.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348285)
Renamed to `m_connect_block_view`.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348285)
Renamed to `m_connect_block_view`.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348456)
I prefer the current version.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348456)
I prefer the current version.
💬 wiz commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3592429159)
ACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e
> A DNS seed operating organization or person is expected to follow good host security practices
I have nothing against Luke personally, but if his server was known to have been compromised and not immediately wiped/rebuilt, he is clearly not following good host security practices... let alone running it for years like that.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3592429159)
ACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e
> A DNS seed operating organization or person is expected to follow good host security practices
I have nothing against Luke personally, but if his server was known to have been compromised and not immediately wiped/rebuilt, he is clearly not following good host security practices... let alone running it for years like that.
💬 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.