Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1904193756)
nit: Could use more modern `LogInfo()` instead.
📝 vasild opened a pull request: "ci: build msan's libc with _LIBCPP_ABI_BOUNDED_*"
(https://github.com/bitcoin/bitcoin/pull/31612)
For the task `MSan, depends (Cirrus CI)` we build a custom libc++ for which we already use `-DLIBCXX_HARDENING_MODE=debug`. Compile it also with `_LIBCPP_ABI_BOUNDED_*` to enable further checks.

Docs at: https://libcxx.llvm.org/Hardening.html#abi-options

<!--
*** 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
...
💬 vasild commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#issuecomment-2573250107)
`7daae2cf96...39a1326488`: drop some changes and move them into a separate PR: https://github.com/bitcoin/bitcoin/pull/31612, suggested by @maflcko
💬 fanquake commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2573295321)
I tried testing the asan job, via `time MAKEJOBS="-j17" FILE_ENV="./ci/test/00_setup_env_native_asan.sh" ./ci/test_run_all.sh`, on a Fedora dev box, and it fails with the following output:
```bash
+ chown root:root /tmp/tcpdump_unitparallel_eth0
++ tcpdump -n -r /tmp/tcpdump_unitparallel_eth0 --direction=out tcp or udp
reading from file /tmp/tcpdump_unitparallel_eth0, link-type EN10MB (Ethernet), snapshot length 262144
+ out='14:47:32.023429 IP6 1111:1111::4.33262 > 1111:1111::1.53: 9493+ A
...
💬 sipa commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1904274621)
Ah, by recursing through transactions in the orphanage I meant.

Imagine a sequence A->B->C (A spends on output of B, B spends an output of C). We have received A and B already (in orphanage), but C is still missing. An announcement from a new peer for transaction A can reasonably be treated as an announcement for C from that peer.
👍 ryanofsky approved a pull request: "cmake: Set top-level target output locations"
(https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2532359577)
Code review ACK 06ff9429d32943aa4debec38ea105c8af4283aec. It seems like a improvement to me overall if binaries in the build directory have the same layout as binaries in releases. I'm not sure current implementation of this is ideal but it seems like a a step in the right direction.

Having binaries built in consistent locations that match install locations should make it is easier to find them, make scripts and wrappers simpler, and avoid bugs that only happen in developer builds but not fin
...
🤔 jarolrod reviewed a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608#pullrequestreview-2532365008)
ACK fa029a78780fd00e1d3ce1bebb81a95857bfbb94

> Does macOS auto-update or provide pop-ups for minor releases?

Yeah, depending on the users preference for auto-updates. See ["Automatic Updates"](https://support.apple.com/guide/mac-help/software-update-settings-on-mac-mchla7037245/15.0/mac/15.0)
🚀 fanquake merged a pull request: "util: Add missing types in make_secure_unique"
(https://github.com/bitcoin/bitcoin/pull/31464)
💬 i-am-yuvi commented on pull request "#31403 follow-up":
(https://github.com/bitcoin/bitcoin/pull/31599#issuecomment-2573352968)
> ACK [a3f5573](https://github.com/bitcoin/bitcoin/commit/a3f5573ae3aa5edbe92d0388f36586ad02214de6)
>
> First commit follows recommendations by myself with improvements by maflcko in already merged PR.
>
> Passed subset of functional tests locally.
>
> Might squeeze a little bit more information into the title?
>
> "#31403 follow-up" -> "qa: Improve framework.generate* enforcement (#31403 follow-up)"
>
> See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#creating-t
...
💬 marcofleon commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2573398883)
Thanks for the review @i-am-yuvi!

> I haven't tested on Mac due to storage issue, it would be interesting to see if that would crash on macos!!

So I learned that it's likely this
https://github.com/bitcoin/bitcoin/blob/41a2ce9b7d7354c633c104bab5653f1f60eb2068/src/test/fuzz/util.cpp#L278-L289
that is preventing the `load_external_block_file` target from crashing on a mac.
📝 hebasto opened a pull request: "depends, NetBSD: Fix `bdb` package compilation with GCC-14"
(https://github.com/bitcoin/bitcoin/pull/31613)
On NetBSD 10, compilation of the `bdb` package with GCC-14 fails with the following error:
```
$ gmake -C depends bdb CC=/usr/pkg/gcc14/bin/gcc
<snip>
./libtool --mode=compile /usr/pkg/gcc14/bin/gcc -c -I. -I../dist/./.. -I/home/hebasto/dev/bitcoin/depends/x86_64-unknown-netbsd10.0/include -D_XOPEN_SOURCE=600 -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security -Wno-error=implicit-int ../dist/./../env/env_config.c
libtool: compile: /usr/pkg/gcc14/bin/g
...
💬 marcofleon commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2573406944)
> I think you forgot to set it in the init function? Otherwise, the global state may be non-deterministic

Good catch, thank you. Added in ff21870e20b2391b684cc50fdd6879805055d6a1
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1904334692)
Ugh, must have switched to the old branch by mistake when I was testing this. I think it would be better to clamp the negative values to zero, which preserves the current behaviour.
💬 fanquake commented on pull request "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*":
(https://github.com/bitcoin/bitcoin/pull/31612#issuecomment-2573424858)
https://cirrus-ci.com/task/5516452708483072:
```bash
[16:01:15.125] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src && /usr/bin/ccache /usr/bin/clang++ -DABORT_ON_FAILED_ASSUME -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DDEBUG -DDEBUG_LOCKCONTENTION -DDEBUG_LOCKORDER -DRPC_DOC_CHECK -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -I/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src -I/ci_container_base/src -isystem /ci_co
...
💬 hebasto commented on issue "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/27242#issuecomment-2573441771)
I believe this issue is no longer relevant after migrating to CMake.

On OpenBSD 7.6, the build completes without any issues:
```
$ cmake -B build -DBUILD_KERNEL_LIB=ON
$ cmake --build build
$ file build/src/kernel/libbitcoinkernel.so
build/src/kernel/libbitcoinkernel.so: ELF 64-bit LSB shared object, x86-64, version 1
```
💬 maflcko commented on pull request "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*":
(https://github.com/bitcoin/bitcoin/pull/31612#issuecomment-2573448664)
Does that mean it needs to be passed again at build time? This seems a bit confusing, given that they are supposed to be vendor settings.
🤔 mzumsande reviewed a pull request: "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors"
(https://github.com/bitcoin/bitcoin/pull/30909#pullrequestreview-2532471105)
Code Review ACK 9d2d9f7ce29636f08322df70cf6abec8e0ca3727
💬 mzumsande commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1904342279)
Makes sense, I got confused there!
It just seemed a bit unusual, plus we generally use `CHECK_NONFATAL` instead of `assert` in rpc code, which may not be the ideal choice for possible future interface users (though it is currently just used in another rpc call, so it's not an actual problem).
But I'm not an expert on the design of our interfaces, so I don't know if moving it somewhere else is necessary.
👍 BrandonOdiwuor approved a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2532501177)
LGTM ACK 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40
👍 ryanofsky approved a pull request: "rpc: add `revelant_blocks` to `scanblocks status`"
(https://github.com/bitcoin/bitcoin/pull/30713#pullrequestreview-2532399834)
Code review ACK 5b2d0216d871bd481f733506a2e8f80b8a34eca0. Seems like a nice feature, and the implementation is pretty simple. I think there is a minor regression if scanblocks start is called from multiple threads, and suggested a fix below.

re: https://github.com/bitcoin/bitcoin/pull/30713#issuecomment-2344018591

> Any ideas on how to slow down `scanblocks()` enough to allow a `status` call to consistently return status of a scan in progress?

Will suggest is a very general solution th
...