💬 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)

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

💬 fanquake commented on pull request "build: disable external-signer for Windows":
(https://github.com/bitcoin/bitcoin/pull/28967#issuecomment-1835873829)
> and drop boost-process from vcpkg.json.

Added.

👍 hebasto approved a pull request: "build: disable external-signer for Windows"
(https://github.com/bitcoin/bitcoin/pull/28967#pullrequestreview-1759532758)
re-ACK 308aec3e5655327d98e0428d8205d246f24d6af5.

💬 willcl-ark commented on pull request "rpc: keep `.cookie` file if it was not generated":
(https://github.com/bitcoin/bitcoin/pull/28784#issuecomment-1835934522)
re-ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d

💬 maflcko commented on pull request "bugfix: Make `CCheckQueue` RAII-styled (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1411992509)
This is a change in behavior for the test in 9cf89f7a5b81197e38f58b24be0793b28fe41477?

💬 maflcko commented on pull request "bugfix: Make `CCheckQueue` RAII-styled (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1411998596)
5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd: Comment is now wrong, no?

🤔 maflcko reviewed a pull request: "bugfix: Make `CCheckQueue` RAII-styled (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/26762#pullrequestreview-1759638398)
lgtm

📝 Sjors opened a pull request: "Stratum v2 Template Provider (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28983)
Based on on @Fi3's https://github.com/bitcoin/bitcoin/compare/master...Fi3:bitcoin:PatchTemplates which is based on @ccdle12's #27854. I rebased it and re-wrote the commit history. See the original branch for the evolution of the spec.

The Stratum v2 [spec](https://github.com/stratum-mining/sv2-spec) describes various roles, but for Bitcoin Core the role of the [Template Provider](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md) is the most interestin
...

💬 Sjors commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1835979687)
Happy to leave this open for a bit, but here's my PR: #28983