💬 ldionne commented on pull request "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*":
(https://github.com/bitcoin/bitcoin/pull/31612#issuecomment-2575746681)
> Hmm, once libc++ itself is compiled with `_LIBCPP_ABI_BOUNDED_*` does the user program have to be compiled with those as well, e.g. by passing `-D_LIBCPP_ABI_BOUNDED_ITERATORS` to the compiler? Or will those extra checks be enabled regardless of how the user program is compiled?

Users don't have anything to do, and in fact they shouldn't try to do anything. The idea is that ABI macros are not knobs that can be changed by "end users" (as in people using libc++), they are things that must be
...

💬 brunoerg commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2575750096)
> No objection, but this will just create a conflict with the bdb removal, at which point the test will be deleted as well?

The check will still exist in `importdescriptors`. But you're right, the legacy wallet iteration in this functional test will be removed, so it's better to move this check to `wallet_migration` test.

⚠️ hebasto opened an issue: "multiprocess: `ipc_tests` fail on NetBSD"
(https://github.com/bitcoin/bitcoin/issues/31618)
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/12655211444/job/35264969094:
```
...
./test/ipc_tests.cpp(11): Entering test suite "ipc_tests"
./test/ipc_tests.cpp(12): Entering test case "ipc_tests"
terminate called after throwing an instance of 'std::system_error'
what(): Invalid argument
2025-01-07T16:33:06.682484Z [unknown] [test/util/random.cpp:46] [SeedRandomStateForTest] Setting random seed for current tests to RANDOM_CTX_SEED=fa0a6578ac6467296e537ce4d0824c43c31c96f
...

💬 hebasto commented on issue "multiprocess: `ipc_tests` fail on NetBSD":
(https://github.com/bitcoin/bitcoin/issues/31618#issuecomment-2575763378)
cc @ryanofsky

🤔 jonatack reviewed a pull request: "RPC: Fix invalid txid in `gettransaction` example"
(https://github.com/bitcoin/bitcoin/pull/31610#pullrequestreview-2534911339)
A similar discussion came up a few years ago to prefer invalid addresses in RPC examples; see https://github.com/bitcoin/bitcoin/pull/17819. Do we have RPC examples with valid txids?

💬 vasild commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1905783548)
Is `bpf.perf_buffer_poll(timeout=400)` needed now that there is `self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 0)`? If not needed, then better drop it.

💬 hebasto commented on pull request "depends: Avoid hardcoding `host_prefix` in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31358#issuecomment-2575795693)
> It does issue a warning though (at least when you don't build the GUI):
>
> ```
> CMake Warning:
> Manually-specified variables were not used by the project:
>
> CMAKE_TOOLCHAIN_FILE
> ```

Are you sure that you build directory is clean before the first `cmake` invocation?

💬 vasild commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1905788722)
Non blocker comment: this cast from `uint64_t` to `(void*)` looks odd, maybe it will produce compiler warning in some cases? Also, on 32 bit systems that should be `uint32_t`. Can these variables be declared as `void*` instead? Like:

```suggestion
void* conn_type_pointer;
void* address_pointer;
bpf_usdt_readarg(1, ctx, &inbound.conn.id);
bpf_usdt_readarg(2, ctx, &address_pointer);
bpf_usdt_readarg(3, ctx, &conn_type_pointer);
bpf_usdt_readarg(4, ctx, &inbound.con
...

👍 vasild approved a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2534935820)
ACK 15dfd72f8ff47736a13c80743d5d132fd2763ecb (modulo to lint failure)

💬 vasild commented on pull request "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*":
(https://github.com/bitcoin/bitcoin/pull/31612#issuecomment-2575812488)
Excellent, that will simplify https://github.com/bitcoin/bitcoin/pull/31424

Thank you!

👍 hebasto approved a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608#pullrequestreview-2534950260)
re-ACK fa029a78780fd00e1d3ce1bebb81a95857bfbb94.

💬 fanquake commented on pull request "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree":
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2575819149)
> I guess I'm a little confused about how this should work. It seems it's not very consistent.

> So.. how should this all work ideally? Do we really need to forward the flags at all?

I agree. It'd be good to know what the goals are here, if some amount of this code is redundant, and what the status of this change is.

💬 vasild commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1905810616)
All `_LIBCPP_ABI_BOUNDED*` additions have been removed from this PR.

💬 vasild commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#issuecomment-2575844233)
`39a1326488...1ef9f7ca4a`: drop the additions of `_LIBCPP_ABI_BOUNDED*` when compiling Bitcoin Core. See https://github.com/bitcoin/bitcoin/pull/31612#issuecomment-2575746681 for reasoning.

💬 l0rinc commented on pull request "init,log: Unify block index log line":
(https://github.com/bitcoin/bitcoin/pull/31616#discussion_r1905813574)
Good idea, [done](https://github.com/bitcoin/bitcoin/compare/2ee82dfdeb3777c1d1648daa7e009c420caf5973..60b7bbd3709519c6f2f8023a23ee0a194a5f0eb4)

📝 vasild opened a pull request: "ci: change the build to be verbose by default"
(https://github.com/bitcoin/bitcoin/pull/31619)
Previously CI would use a non-verbose build (where exact compilation and linking commands are not visible) and only if it fails, would rerun the build in verbose mode.

Change this to use verbose in the initial build as well for two reasons:
1. It is useful to be able to see the exact compilation and linking commands even for successful builds, to e.g. confirm some particular flags are (not) used as expected.
2. In failed builds, if the failure is during linking, the repeated verbose build m
...

💬 vasild commented on pull request "ci: change the build to be verbose by default":
(https://github.com/bitcoin/bitcoin/pull/31619#issuecomment-2575850927)
I actually needed this while working on https://github.com/bitcoin/bitcoin/pull/31424

💬 l0rinc commented on pull request "RPC: Fix invalid txid in `gettransaction` example":
(https://github.com/bitcoin/bitcoin/pull/31610#issuecomment-2575861575)
I understand the rationale for using placeholders instead of hard-coded values—especially for dangerous commands—and perhaps even correctly formatted ones that clearly don’t point to anything real. However, using slightly incorrect examples as guidance for harmless queries seems really counterproductive. Why would we want to deliberately mislead users? What’s the harm in providing a real example so they can see how a valid transaction should behave?

💬 Sjors commented on pull request "depends: Avoid hardcoding `host_prefix` in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31358#issuecomment-2575864267)
Mmm, not seeing this anymore, so maybe I did something wrong the first time.

After the depends are built:

```sh
rm -rf build
cmake -B build -DBUILD_GUI=ON --toolchain /Volumes/SSD/Dev/bitcoin/depends/x86_64-apple-darwin22.6.0/toolchain.cmake
```

Logs with and without GUI:

https://gist.github.com/Sjors/f4a493c57eb3ab5b62754c1d4e54a9c6

💬 maflcko commented on pull request "ci: change the build to be verbose by default":
(https://github.com/bitcoin/bitcoin/pull/31619#issuecomment-2575871852)
How is this different from just looking at the `C++ compiler flags ....................` output?