👍 vasild approved a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2605030796)
ACK e7743aa5df6387964c23f0629debf8c3cbb8ed4c

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1948609163)
nit: `$(1)_file_name)` -> `$(1)_file_name`

Thanks for the comment. That line is still ~!@*&~(!@*&~&_ in my eyes. I wonder if it can be split into shorter and more easily understandable `define foo` functions...

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1948608064)
nit: ` | head -n1` is unnecessary

💬 willcl-ark commented on issue "Wallets not automatically loaded":
(https://github.com/bitcoin/bitcoin/issues/31832#issuecomment-2647327456)
In addition to @maflcko's suggestion, another way to explicitly load wallets at startup is to use the `-wallet=<path>` startup option in your configuration (multiple times in your case, for multiple wallets).

💬 maflcko commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2647372194)
lgtm ACK 459683bcfc70b77b3dae2e70c88cf5c6a007442e

Only change is a small mktemp cleanup.

I still haven't tested this (I am using a modified script locally anyway), but the code changes look harmless and reasonable. As for the scope, I wonder how much effort should be spent on this script, given that:

* No one(?) is using it right now unmodified, or has been in the past years?
* It is written in bash, causing compat issues with the ancient version of bash shipped on macOS, as well as th
...

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647374260)
It is better to post such suggestions not in the main thread of the PR but as a comment to some line of code, even if that is a random, unrelated line. That way replies will be grouped together with the questions instead of being scattered around in the main PR thread. In the main thread it is easier to forget to reply to some questions. As comments to some line of code, they can be tracked and eventually "resolved" to collapse them and reduce the main PR thread noise.

> #### WinSock
>
> W
...

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647374943)
It is better to post such suggestions not in the main thread of the PR but as a comment to some line of code, even if that is a random, unrelated line. That way replies will be grouped together with the questions instead of being scattered around in the main PR thread. In the main thread it is easier to forget to reply to some questions. As comments to some line of code, they can be tracked and eventually "resolved" to collapse them and reduce the main PR thread noise.

> #### WinSock
>
> W
...

💬 Sjors commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#issuecomment-2647388325)
Concept ACK. I'll need to rebase some things, but I'm favor of getting that over with.

💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1948687873)
1b3613505deb4baeacc3c4c2fc91ccca1a8ecec5

Usually, external dependencies follow the internal ones.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948692321)
Done, will be in next push.

💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2647421895)
> > Could the `CAPNP_EXECUTABLE`, `CAPNPC_CXX_EXECUTABLE` and all `CapnProto_*` CMake variables be marked as advanced to avoid cluttering the CMake cache view when inspected by the user?
>
> Sure, done in latest push.

The `CapnProto_*` CMake variables are still lingering:
```
$ cmake -B build -DENABLE_IPC=ON -L 2>&1 | grep CapnProto_
CapnProto_capnp-json_IMPORTED_LOCATION:FILEPATH=/usr/lib/x86_64-linux-gnu/libcapnp-json-1.0.1.so
CapnProto_capnp-rpc_IMPORTED_LOCATION:FILEPATH=/usr/lib/x
...

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948716391)
By default trying to pass `std::span<std::byte>` to a function that takes `std::span<uint8_t>` gives:

```
no known conversion from 'span<std::byte>' to 'span<uint8_t>'
```

If I really insist then I can convert it like:

```cpp
void f(std::span<uint8_t> s);
...
std::span<std::byte> a;
f(std::span<uint8_t>{reinterpret_cast<uint8_t*>(a.data()), a.size()});
```

I find it cleaner to use `uint8_t` because it is already used in the existent code (`ReceiveMsgBytes()`) and avoid such fo
...

💬 hebasto commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2647448250)
@theuni

> > However, switching the CC variable context from Makefile to the shell environment breaks expectations
>
> Arguably that's because this wasn't intended to be supported :)
>
> I suppose this fix is reasonable, though supporting env vars like this seems quite brittle.

There are testing environments, such as OSS-Fuzz, that rely on a project's build system to support environment variables.

💬 hebasto commented on pull request "depends: Make default `host` and `build` comparable":
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2647452355)
Friendly ping @laanwj @josibake @theuni ;)

💬 hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2647463681)
Friendly ping @theuni :)

⚠️ Sjors opened an issue: "ci: intermittent p2p_tx_download.py timeout (test_preferred_tiebreaker_inv)"
(https://github.com/bitcoin/bitcoin/issues/31833)
Seen in: https://github.com/bitcoin/bitcoin/actions/runs/13236532677/job/36942441812?pr=30975 (#30975)

```
test 2025-02-10T09:21:22.592000Z TestFramework (INFO): Test that preferred peers are always selected over non-preferred when ready
test 2025-02-10T09:21:22.592000Z TestFramework.node0 (DEBUG): Stopping node
node0 2025-02-10T09:21:22.593102Z [http] [httpserver.cpp:306] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:57336
node0 2025-02-10T09:21:22.593212Z [http
...

💬 Sjors commented on issue "qa: Functional tests are intrinsic vulnerable to timeouts":
(https://github.com/bitcoin/bitcoin/issues/19732#issuecomment-2647473725)
Is #31833 a symptom of this?

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2647477699)
Reported spurious CI failure in #31833. Otherwise it looks good.

💬 hebasto commented on pull request "test: Move `script_assets_tests` into its own suite":
(https://github.com/bitcoin/bitcoin/pull/31576#issuecomment-2647481085)
Friendly ping @maflcko ;)

🚀 fanquake merged a pull request: "build: consistently use `CLIENT_NAME` in libbitcoinkernel.pc.in"
(https://github.com/bitcoin/bitcoin/pull/31820)