📝 hebasto opened a pull request: "POC: Replace Boost.Process with cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/28981)
Closes https://github.com/bitcoin/bitcoin/issues/24907.

This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049).

I've fixed Windows-related bugs and issues and commenced submitting patches to [upstream](https://github.com/arun11299/cpp-subprocess).

For more details, please refer to commit messages.

👍 hebasto approved a pull request: "build: disable external-signer for Windows"
(https://github.com/bitcoin/bitcoin/pull/28967#pullrequestreview-1758892319)
> It's come to light that Boost ASIO (a Boost Process sub dep) has in some instances, been quietly initialising our network stack on Windows (see PR #28486 and discussion in #28940).
>
> This has been shielding a bug in our own code, but the larger issue is that Boost Process/ASIO is running code before main, and doing things like setting up networking. This undermines our own assumptions about how our binary works, happens before we run any sanity checks, and before we call our own code to s
...

👍 vasild approved a pull request: "validation: log which peer sent us a header"
(https://github.com/bitcoin/bitcoin/pull/27826#pullrequestreview-1759207756)
ACK 708600f3b7df4cf166a2fcdfd3c0dc70b70812f0

💬 maflcko commented on pull request "wallet: Fix migration of blank wallets":
(https://github.com/bitcoin/bitcoin/pull/28976#discussion_r1411751970)
If this truly should never happen, and it seems like an internal bug when it happens, then I wonder if the translation is needed and if `STR_INTERNAL_BUG` can be used?

```suggestion
error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing"));
```

💬 maflcko commented on pull request "wallet: Fix migration of blank wallets":
(https://github.com/bitcoin/bitcoin/pull/28976#discussion_r1411752078)
(same below)

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1411756962)
Say we have 5 inbound Erlay peers and 0 inbound legacy peers.
`inbound_targets = (5+0) * 0.1 = 0.5`
`destinations = 0.5 - 0 = 0.5`

It just means that we will take 1 inbound peer for fanout with a 50% chance.

I guess it will work just fine if i drop the `0.01` (1% or less chance) check. It's kinda unfortunate we have to do the compute if chance is that small. Fine with me either way, let me know what you think.

💬 maflcko commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1411756653)
```suggestion
[#19160](https://github.com/bitcoin/bitcoin/pull/19160) added a new `bitcoin-node` `-ipcbind` option and an `bitcoind-wallet` `-ipcconnect` option to allow new wallet processes to connect to an existing node process.
```

The pull is merged?

💬 maflcko commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1411756818)
```suggestion
[#19161](https://github.com/bitcoin/bitcoin/pull/???) adds a new `bitcoin-gui` `-ipcconnect` option to allow new GUI processes to connect to an existing node process.
```

wrong number?

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1411776086)
done

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1411780306)
Yes. My primary concern is the complexity though. Cascade removal is more difficult to reason about. I just thought it was not worth it.

💬 Sjors commented on pull request "net: additional disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-1835692974)
I added `peeraddr=` to each of them. This is indeed quite useful, because we can't look up the IP after disconnection. Adding `net=` also makes sense, but let's do that everywhere in another PR (probably using a helper function).

💬 maflcko commented on pull request "net: additional disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-1835740082)
> because we can't look up the IP after disconnection.

Is the IP not logged on connection, when IP logging is enabled?

💬 TheCharlatan commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1835740522)
Concept ACK

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1411821098)
122658f95b36b7f940351bd34a89e13831931f18

should we make this addition overflow safe?

🤔 naumenkogs reviewed a pull request: "Nuke adjusted time (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1759382310)
Concept ACK

💬 maflcko commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1835807820)
lgtm ACK 05aca093819be276ac7d648472c6ed5c7d235cc5

💬 Sjors commented on pull request "net: additional disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-1835817607)
@maflcko indeed it is, though for inbound it depends on the exact log settings. I think right now you won't see these disconnect messages without `-debug=net` and with it you'll see all the connect messages. But if we increase the relative priority of disconnect messages then you might not. So it still seems useful to add.

💬 dergoegge commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1835850051)
```
$ echo "jtHRO///YED/AAD/AAABCQAAgQAN/wAAAQAALAEB/QAAAAAJAAAAAAAAAAAAAAAA+GwAAPAV/v8BkAEicG9vcyYANwAAAAEBADsBAQHo/v7+bv4RAQEBkQABAQE3CTsBIQEBAQEBAAEBATcAACYKAAQBAf8BAQEBAQEBCAABAQEBfgEAAQEBCgAAADsBAQEBAQEB/v8BJgABQQE3AAAAAAFBATcAAAAA8AAAAQEBAQEBAQEBCAABLgEBAX4BAAE=" | base64 --decode > tx_package_eval-81b8ec0e06b86811790d61f69fe27701ca0c8305.crash
$ FUZZ=tx_package_eval ./src/test/fuzz/fuzz tx_package_eval-81b8ec0e06b86811790d61f69fe27701ca0c8305.crash
test/fuzz/package_eval.cpp:320 tx_pac
...

⚠️ DMUNEY45 opened an issue: "> In file included from ../dist/./../cxx/cxx_db.cpp:13:"
(https://github.com/bitcoin/bitcoin/issues/28982)
> In file included from ../dist/./../cxx/cxx_db.cpp:13:
> ./db_cxx.h:59:10: fatal error: 'iostream.h' file not found

The issue is some code like this in bdb:
```cpp
#ifdef HAVE_CXX_STDHEADERS
#include <iostream>
#include <exception>
#define __DB_STD(x) std::x
#else
#include <iostream.h>
#include <exception.h>
#define __DB_STD(x) x
#endif
```

For some reason, configure fails to detect C++ header availability, and `HAVE_CXX_STDHEADERS` is not defined. Is `g++` and a
...

✅ fanquake closed an issue: "> In file included from ../dist/./../cxx/cxx_db.cpp:13:"
(https://github.com/bitcoin/bitcoin/issues/28982)