Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👋 davidgumberg's pull request is ready for review: "build: Make config warnings fatal if -DWERROR=ON"
(https://github.com/bitcoin/bitcoin/pull/31665)
💬 luke-jr commented on pull request "doc: improve NODE_NETWORK_LIMITED documentation per BIP159":
(https://github.com/bitcoin/bitcoin/pull/31805#discussion_r1947409010)
That's not guaranteed. Adding this is technically incorrect, and doesn't provide any real information, just makes it more complex.
💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2644456448)
> Given that CMake lacks any native functionality to acheive the same thing, using `-DWERROR` seems ok, especially if the alternative is implement `N` more build options (now/in future) with varying defaults for everything that may warn and needs to be caught.

> I don't like the idea of overloading the `WERROR` build option with additional functionality, as this is not what users would expect.

> As pointed in [#31724 (comment)](https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-261
...
👍 luke-jr approved a pull request: "build: consistently use `CLIENT_NAME` in libbitcoinkernel.pc.in"
(https://github.com/bitcoin/bitcoin/pull/31820#pullrequestreview-2603255184)
utACK
💬 jonatack commented on pull request "doc: improve NODE_NETWORK_LIMITED documentation per BIP159":
(https://github.com/bitcoin/bitcoin/pull/31805#discussion_r1947457946)
Can you please clarify your comment -- what isn't correct?
luke-jr closed a pull request: "Translate unit names & fix external signer error"
(https://github.com/bitcoin-core/gui/pull/599)
💬 hebasto commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#discussion_r1947496645)
Thanks! Amended.
💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2644560045)
Could the `CAPNP_EXECUTABLE`, `CAPNPC_CXX_EXECUTABLE` and all `CapnProto_*` CMake variables be marked as advanced to avoid cluttering the CMake cache view when inspected by the user?
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2644725633)
> > Did anyone test this on Windows?
>
> Of course would be nice to test this on windows but it should work there according to cmake release notes: https://cmake.org/cmake/help/v3.13/release/3.13.html?highlight=release%20notes#command-line

The success of the `cmake -E create_symlink` command on Windows depends on the the "Create symbolic links" privilege ([`SeCreateSymbolicLinkPrivilege`](https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-10/security/threat-protecti
...
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2644734444)
Additionally, 433aa99d238740bbdcb07b53b17f639170bd326d does not work for any multi-config generators.

For example:
```
$ cmake -B build -G "Ninja Multi-Config"
$ cmake --build build
$ file build/src/bitcoind
build/src/bitcoind: broken symbolic link to /home/hebasto/git/bitcoin/build/bin/bitcoind
📝 bhandariarun opened a pull request: "Bitcoin"
(https://github.com/bitcoin/bitcoin/pull/31825)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
hebasto closed a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31825)
📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31825)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 hebasto commented on pull request "cmake: Copy `cov_tool_wrapper.sh.in` to the build tree":
(https://github.com/bitcoin/bitcoin/pull/31722#discussion_r1947553413)
With the current design, the required `gcov` or `llvm-cov` tools are searched during the `Coverage{Fuzz}.cmake` script invocation. This two-step processing could potentially be avoided by moving these tool checks into the main buildsystem script. However, there is no reliable way to gate them depending on whether the build configuration is 'Coverage' or not.
💬 hebasto commented on pull request "guix: remove test-security/symbol-check scripts":
(https://github.com/bitcoin/bitcoin/pull/31818#issuecomment-2644785548)
> In the pre-Guix days, these were valuable to sanity-check the environment, because we were pulling that pre-built from Ubuntu, with little control. At this point, it's less clear what these scripts are (sanity) checking.

Concept ACK.
📝 eval-exec opened a pull request: "Limit retries in GetRNDRRS to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826)
This PR want to fix #31817 by added a maximum retry limit (`max_retries`) to the `GetRNDRRS` function to prevent it from entering an infinite loop if the hardware random number generator fails to return a valid random number. This change improves stability and ensures that the function terminates after a predefined number of retries.
⚠️ Sjors opened an issue: "Rename bitcoin-wallet?"
(https://github.com/bitcoin/bitcoin/issues/31827)
Some chatter from IRC:

```
17:21:41 < darosior> It might be confusing to release both a bitcoin-wallet utility and a bitcoin-wallet binary as part of multiprocess?
17:24:08 < darosior> We could rename the utility, but then it would be nice to at least have one deprecation cycle. Given recent momentum i estimate it's possible we might release multiprocess in
30.0, which means if we want to deprecate the bitcoin-wallet utility name we should do it.. now?
17:33:31 < sipa> bit
...
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2644928621)
@sipa I opened #31827
💬 MrSuddenJoy commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2645031354)
Just bear in mind that bundled version means slower installation/operation time as well as higher bandwitch usage.
💬 hebasto commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1947662256)
> This does affect us wrt debug info.

I don't think it affects us too much because:https://github.com/bitcoin/bitcoin/blob/fb0ada982a73687520c43b8fde480fa5d456f3e1/CMakeLists.txt#L456-L458