Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 hebasto commented on pull request "cmake: Fix `IF_CHECK_PASSED` option handling":
(https://github.com/bitcoin/bitcoin/pull/31231#issuecomment-2515497249)
> @hebasto Is there a way to get CMake to warn us about these? It seems like it should know that we were attempting a string operation on a list.

I'm not aware of any such embedded functionality. CMake's internally represents both "strings" and "lists" as strings. A list can safely be treated as a string when it contains fewer than two elements.
🤔 ryanofsky reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2476650402)
Thanks for the reviews!

Updated da108a6e5be220654a65b6613ee7eb2c4ddc8677 -> 02567bf2bef5997ea5f0765780d196f36d3053e8 ([`pr/wrap.5`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.5) -> [`pr/wrap.6`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.5..pr/wrap.6)) to fix windows build warning and making a change to avoid a potentially confusing behavior https://github.com/bitcoin/bitcoin/pull/31375#discussion_r18618148
...
💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1868303275)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1861814807

> [b06c7ad](https://github.com/bitcoin/bitcoin/commit/b06c7ad0ae91102fe8cdcac6f86d627aace2219b): this is potentially confusing. If I build without gui, I expect `build/src/bitcoin gui` to fail. Right now it would launch any `bitcoin-qt` in my `$PATH`.

Agree this behavior is potentially confusing, and prevented it for now, but I'm not totally I sure see it as a downside. I like the simplicity of `bitcoin daemon` being
...
💬 ryanofsky commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2515536756)
The `bitcoin` wrapper discussed above is added in PR #31375
💬 hebasto commented on pull request "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree":
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2515540501)
> 🤦🤦
>
> Could you split this into 2 commits? The fixes you describe sound simple enough, but the removal of the helpers makes this hard to review.

Sure thing! Done.
💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2515543821)
Updated 27072547bb22cdd2080d1014eba9c30bc9d47650 -> 99f2f335b6a8c2b5fe518a0c0056eb2a42c0496b ([`pr/mine.14`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.14) -> [`pr/mine.15`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.15), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.14..pr/mine.15)) just moving a line to avoid a merge conflict.
💬 darosior commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2515554249)
Not sure if it's worth sharing here but hey what's an 1300th comment.

Running `bitcoin-node` from https://github.com/bitcoin/bitcoin/pull/29409 with commits `1045047a73a0b3ae15647ab5bf1829d27141b7cd`..`9ca113f326fc9ba2f661595283f4aef390df984d` from this PR cherry-picked on top.

Simply compiled with `cmake -B multiprocbuild/ -DWITH_MULTIPROCESS=ON && cmake --build multiprocbuild/ -j20`. (Although i did have to apply the following diff to be able to compile on Debian stable.)
<details>
<su
...
💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1868374227)
I'm not sure if testing the scenario with the modified ACL on Windows is worth the added complexity of the test script.
💬 hebasto commented on pull request "cmake: use python from venv if available":
(https://github.com/bitcoin/bitcoin/pull/31411#issuecomment-2515600619)
> ... it can be tricky to configure cmake to use a python venv.

I've read https://github.com/astral-sh/uv/blob/main/README.md#python-management and used the following script for simplicity:
```cmake
cmake_minimum_required(VERSION 3.22)
project(test_py LANGUAGES NONE)
find_package(Python3 COMPONENTS Interpreter)
message("Python3_EXECUTABLE=${Python3_EXECUTABLE}")
```

I have no problems with finding `uv`'s Python on my Ubuntu 24.04:
```
$ python3 --version # system's Python
Python
...
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1868406931)
I added a comment for this instead of making a full set part of the sorting criteria. This should not happen under normal circumstances provided the set size is defined to account for way over the expected traffic between reconciliations. A peer hitting the limit is likely to be either broken or an attacker, and I don't think we should be catering to them (nor making the logic more complex based on that)
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1868409396)
Done in the last force push
💬 achow101 commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2515669856)
ACK 73db95c65c1d372822166045ca8b9f173d5fd883
🚀 achow101 merged a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175)
💬 achow101 commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2515696899)
ACK 32fc59796f74a2941772b5ec2755b1319132cd9c
💬 achow101 commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#issuecomment-2515702156)
ACK 8f85d36d68ab33ba237407a2ed16667eb149d61f
achow101 closed an issue: "Why does `submitpackage` require at least two transactions"
(https://github.com/bitcoin/bitcoin/issues/31085)
🚀 achow101 merged a pull request: "Package validation: accept packages of size 1"
(https://github.com/bitcoin/bitcoin/pull/31096)
🚀 achow101 merged a pull request: "refactor: Clamp worker threads in ChainstateManager constructor"
(https://github.com/bitcoin/bitcoin/pull/31313)