Bitcoin Core Github
44 subscribers
120K links
Download Telegram
:lock: fanquake locked an issue: "Bug Report RescanWallet backup.cpp"
(https://github.com/bitcoin/bitcoin/issues/31791)
💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940677521)
> Is the (possible) running of 01_base_install.sh more than once deliberate? If so, then I'm curious why we ever do that? Is it just for the `DANGER_RUN_CI_ON_HOST` path or is there another use?

It should only install once (otherwise there will be issues). Yes, it is for `DANGER_RUN_CI_ON_HOST`.

> If not then it seems like we could just mark install as done when building the image, something like [this](https://github.com/bitcoin/bitcoin/commit/410b01403346553a0aab9ac3d8347fa69398257c)...
...
💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940680325)
Would be good to explain why this is needed (with an example error log) and why this is the correct fix. My preference would be to fix it in the lint test_runner itself, so that everyone benefits from the fix, not just people running this ci bash script in a workdir.
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633222863)
Rebased on the latest #31741, replaced `-DBUILD_MULTIPROCESS` with `-DENABLE_IPC`.

I dropped 188cae8df38a83b08b104992c5c84c910b01f4b9 "ci: build multiprocess with Tsan" since this is a non-depends job, so IIUC it should also build multiprocess with the same flags as the rest of the code.

I also dropped 743ae9b4e2746f48d707d49fc2c8ba908995034d "build: add bitcoin-{node,gui} to Maintenance.cmake" which can go in a followup that turns it on by default.

On my own macOS 15.3 with Xcode 16.2
...
💬 hebasto commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2633228974)
> Without that commit, the headers must be generated late in the build, after a bunch of other libs have been built:
>
> `base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake || libcrc32c.a libcrc32c_sse42.a libleveldb.a libminisketch.a minisketch_clmul src/bitcoin_clientversion src/crypto/libbitcoin_crypto.a src/crypto/libbitcoin_crypto_avx2.a src/crypto/libbitcoin_crypto_sse41.a src/crypto/libbitcoin_crypto_x86_shani.a src/generate_build_info src/libbit
...
👍 hebasto approved a pull request: "build: simplify by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911#pullrequestreview-2592139055)
ACK 12fa9511b5fba18e83f88b7b831906595bcf2116.
💬 hebasto commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633281600)
> Rebased on the latest #31741, replaced `-DBUILD_MULTIPROCESS` with `-DENABLE_IPC`.
>
> I dropped [8a7bcdb](https://github.com/bitcoin/bitcoin/commit/8a7bcdbaa5a45b40ac451439106e78ade7194e37) "ci: build multiprocess with Tsan" since this is a non-depends job, so IIUC it should also build multiprocess with the same flags as the rest of the code.
>
> I also dropped [743ae9b](https://github.com/bitcoin/bitcoin/commit/743ae9b4e2746f48d707d49fc2c8ba908995034d) "build: add bitcoin-{node,gui} to
...
💬 maflcko commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1940777811)
Won't this race with `importing` and intermittently fail?
💬 maflcko commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1940798789)
I'd be fine with a hacky check that only runs on windows and checks for a magic msvc binary string in `open(exe_path, "rb").read()`, but anything is fine and this is not a blocker from my side.
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2633332880)
Rebased and addressed https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1925424138
💬 laanwj commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940822375)
Unnecessary empty line removal
💬 laanwj commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940823734)
Why delete this comment?
💬 laanwj commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940835808)
Please add a descriptive comment what this code fragment does and why; we know, but the code isn't super easy to follow if you don't know the context.
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633380099)
The ASan build fails with a long and cryptic error: https://github.com/bitcoin/bitcoin/actions/runs/13131433348/job/36637228850?pr=30975

```
/usr/bin/ld: CMakeFiles/mputil.dir/src/mp/util.cpp.o: in function `asan.module_ctor':
util.cpp:(.text.asan.module_ctor[asan.module_ctor]+0x6): undefined reference to `__asan_init'
/usr/bin/ld: util.cpp:(.text.asan.module_ctor[asan.module_ctor]+0xb): undefined reference to `__asan_version_mismatch_check_v8'
/usr/bin/ld: util.cpp:(.text.asan.module_cto
...
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633384108)
macOS cross compile fails (in a different way): https://cirrus-ci.com/task/5052270577975296
👍 dergoegge approved a pull request: "consensus: Remove checkpoints (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31649#pullrequestreview-2592286198)
ACK 301017c6217378ca868e17c7cb8ffe3447abb11d

Only a comment changed since my last review (https://github.com/bitcoin/bitcoin/pull/31649#pullrequestreview-2550177263).
💬 fanquake commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2633390450)
> If there are any easier steps to reproduce this maybe using docker,

Running `time env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_i686_multiprocess.sh" ./ci/test_run_all.sh'` with the branch from #29796.

> or just a CI run I can look at that would be helpful.

https://github.com/bitcoin/bitcoin/pull/29796/checks?check_run_id=36485685971
🚀 fanquake merged a pull request: "lint: Call more checks from test_runner"
(https://github.com/bitcoin/bitcoin/pull/31653)
👍 dergoegge approved a pull request: "multi-peer orphan resolution followups"
(https://github.com/bitcoin/bitcoin/pull/31666#pullrequestreview-2592314173)
Code review ACK 7426afbe62414fa575f91b4f8d3ea63bcc653e8b
💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2633426678)
I'm unable to make a Guix build. I'm using https://github.com/Sjors/bitcoin/commits/2025/02/ipc-guix/ which enables multiprocess in the simplest possible way, without modifying the depends system, by setting `MULTIPROCESS=1` in the pre-download and depends make step.

It downloads and builds capnp as expected, but in the download phase it says:

```
Checksum missing or mismatched for native_libmultiprocess source. Forcing re-download.
```

Which seems strange.

And then during the (off
...