Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 maflcko commented on pull request "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`":
(https://github.com/bitcoin/bitcoin/pull/33624#issuecomment-3405047054)
re-lgtm ACK 3a10d700bc1889b3690097efc935c5a4ba5966bb

Only change seems to be a comment/doc.
🤔 janb84 reviewed a pull request: "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`"
(https://github.com/bitcoin/bitcoin/pull/33624#pullrequestreview-3338930586)
tested ACK 3a10d700bc1889b3690097efc935c5a4ba5966bb

PR introduces new assert in test that prohibits the `if ( 1 == 1)` mutation / `if(true)`, because the assert will fail in that condition.

- code review
- build & tested
- changed code to true to test assert
🤔 maflcko reviewed a pull request: "init: Improve -asmap option behavior and documentation"
(https://github.com/bitcoin/bitcoin/pull/33632#pullrequestreview-3338942228)
(nit from the llm bot)
💬 maflcko commented on pull request "init: Improve -asmap option behavior and documentation":
(https://github.com/bitcoin/bitcoin/pull/33632#discussion_r2431523908)

Specify asn mapping used for bucketing of the peers -> Specify the ASN mapping used for bucketing of peers [adds missing article and capitalizes the ASN acronym to improve clarity]
💬 hodlinator commented on pull request "http: Make server shutdown more robust":
(https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-3405114414)
Was worried that libevent would clean up the memory leak I thought was fixed by aa11ee5ccfcedf3fb44e7722ecdeb3cdaa92fd51 later during shutdown, so verified that that libevent does no such thing using Valgrind: https://github.com/bitcoin/bitcoin/compare/master...hodlinator:bitcoin:2025/02/stop_http_robust.valgrind
💬 willcl-ark commented on issue "`test_bitcoin-qt`: segfault under LTO (CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON)":
(https://github.com/bitcoin/bitcoin/issues/33548#issuecomment-3405201314)
FWIW this doesn't reproduce for me using Clang 20 via [this nix flake](https://github.com/bitcoin-dev-tools/bix/blob/wip-updates/flake.nix), where I need to force `QT_QPA_PLATFORM=xcb` to run Qt:

<details>
<summary>Details</summary>

```
Configure summary
=================
Executables:
bitcoin ............................. ON
bitcoind ............................ ON
bitcoin-node (multiprocess) ......... OFF
bitcoin-qt (GUI) .................... ON
bitcoin-gui (GUI, multiprocess) .....
...
🚀 fanquake merged a pull request: "ci: add libcpp hardening flags to macOS fuzz job"
(https://github.com/bitcoin/bitcoin/pull/33462)
🚀 fanquake merged a pull request: "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`"
(https://github.com/bitcoin/bitcoin/pull/33624)
💬 fanquake commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3405473825)
> my worry is that the error is not exactly trivial to understand and actionable:

I agree that it's odd, and I'm wondering why it's not happening (seemingly at all?) in the qa-assets repo?
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2431873023)
Thanks for pointing this out. I will update the comment.
💬 maflcko commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3405514021)
> I agree that it's odd, and I'm wondering why it's not happening (seemingly at all?) in the qa-assets repo?

I can see it happening here: https://github.com/bitcoin-core/qa-assets/actions/runs/18426954137/job/52509387674#step:7:5257:

```
==11740==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x55e39ba5db80 in SetArgs(int, char**) /home/runner/work/_temp/build/src/test/fuzz/util/./test/fuzz/fuzz.cpp:52:5
#1 0x55e39ba5db80 in LLVMFuzzerInitialize /home/runner/work/_temp/
...
💬 vasild commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#discussion_r2431914452)
A boolean `need_broadcast` does not fit the need for "don't broadcast", "broadcast to everybody" and "private broadcast".
fanquake closed an issue: "ci: add an Alpine (musl libc) job"
(https://github.com/bitcoin/bitcoin/issues/33437)
🚀 fanquake merged a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480)
💬 fjahr commented on pull request "init: Improve -asmap option behavior and documentation":
(https://github.com/bitcoin/bitcoin/pull/33632#discussion_r2431971241)
Done
🚀 fanquake merged a pull request: "ci: Add macOS cross task for arm64-apple-darwin"
(https://github.com/bitcoin/bitcoin/pull/33549)
💬 fanquake commented on pull request "[wip] A more static bitcoin-qt":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3405681891)
> Or is it to simplify deployment, like being able to provide a single binary that can be run on any Linux machine?

We currently provide a single binary that can be run on (most) Linux machines (along with anything in https://github.com/bitcoin-core/packaging); however I'd like to remove runtime requirements further. See #33434 for a recent motivation (users shouldn't need to install additional dependencies/install other software to run our software).

> If the latter is the case, [AppImage
...
👋 fanquake's pull request is ready for review: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33625)
💬 fanquake commented on pull request "Update secp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33625#issuecomment-3405765634)
Guix Build (aarch64):
```bash
22b8a342b9e47069d05d1fca40437ce2b19820e3f3c6b7ef17f2132a462500f9 guix-build-879c21045eba/output/aarch64-linux-gnu/SHA256SUMS.part
8f156c6e1cea62efb25007ab62ce02292eac014fe81f526249e4ff6724292a0c guix-build-879c21045eba/output/aarch64-linux-gnu/bitcoin-879c21045eba-aarch64-linux-gnu-debug.tar.gz
9afd57b78623c0ca553a6e8bf794baf8e31ee55b3dcbe9e265b02810cfaebb82 guix-build-879c21045eba/output/aarch64-linux-gnu/bitcoin-879c21045eba-aarch64-linux-gnu.tar.gz
8819e7
...
💬 stringintech commented on pull request "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation":
(https://github.com/bitcoin/bitcoin/pull/33604#issuecomment-3405795725)
Thanks for the links @stratospher!
FYI, calculating `pindexLastCommonBlock` is undergoing some improvements in #32180, and I will probably have to rebase onto that change. But either way, as you mentioned, it shouldn't cause any issues.