💬 fanquake commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-1611216402)
> though there are others popping up as well ...

I think at least for the (windows) ones coming out of leveldb, we could filter them out, as we do with other warnings there.

💬 MarcoFalke commented on pull request "util: Don't derive secure_allocator from std::allocator":
(https://github.com/bitcoin/bitcoin/pull/27930#issuecomment-1611219016)
> > I'm guessing we should consider doing the same for [`zero_after_free_allocator`](https://github.com/bitcoin/bitcoin/blob/0c84a0e4841f00d931aa7339e9aa8f26eb2f3a61/src/support/allocators/zeroafterfree.h#L15)? Even though it doesn't seem to be affected by this particular issue.
>
> That's probably a good idea, yes.

Looks like this wasn't addressed yet?

🚀 fanquake merged a pull request: "http: update libevent workaround to correct version"
(https://github.com/bitcoin/bitcoin/pull/27949)

💬 fanquake commented on pull request "test: various USDT functional test cleanups (27831 follow-ups)":
(https://github.com/bitcoin/bitcoin/pull/27944#issuecomment-1611232273)
cc @virtu

💬 fanquake commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1611235566)
```bash
7851f08737a0133002c0224d4bd5e52aed7eaee91adac46f1247ef164775d037 guix-build-23d39c5079cf/output/arm64-apple-darwin/SHA256SUMS.part
172e378d18cd9c624e4cdce51228bfbb43d25098871c18911c70eeb1c2390f34 guix-build-23d39c5079cf/output/arm64-apple-darwin/bitcoin-23d39c5079cf-arm64-apple-darwin-unsigned.tar.gz
3de4d2cec142a8464788da7571b2f09a47893c7f8d75fe52912ecfcefac72242 guix-build-23d39c5079cf/output/arm64-apple-darwin/bitcoin-23d39c5079cf-arm64-apple-darwin-unsigned.zip
33da433dbd3c3d6
...

💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1611244263)
Rebased due to the conflict with #27813.

💬 MarcoFalke commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1245084467)
style-nit: Seems odd to have three asserts to check two things. Can be written shorter and easier to read:

```suggestion
p = [p for p in self.getpeerinfo() if p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}"]
assert_equal(len(p), 1)
assert_equal(p[0]["subver"], P2P_SUBVERSION)
```

💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1611246591)
> ... unsigned integer overflow ...

Maybe collecting the number of "full outbound or manual" connections on the fly was not so bad? I am not objecting pre-collecting and maintaining that number in `CConnman`, just thinking aloud. That looks a bit more tricky and maybe more prone to bugs (like the above). It is also not generic - the "full outbound or manual" number is used only in this specific case and unlikely to be used elsewhere in the future. A more generic approach, that could possibly
...

💬 carnhofdaki commented on issue "Add maxrelaytxfee":
(https://github.com/bitcoin/bitcoin/issues/27983#issuecomment-1611258937)
OK, thanks.

✅ carnhofdaki closed an issue: "Add maxrelaytxfee"
(https://github.com/bitcoin/bitcoin/issues/27983)

💬 kristapsk commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1611260025)
Concept ACK

💬 hebasto commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1611271297)
Guix build on `x86_64`:
```
7851f08737a0133002c0224d4bd5e52aed7eaee91adac46f1247ef164775d037 guix-build-23d39c5079cf/output/arm64-apple-darwin/SHA256SUMS.part
172e378d18cd9c624e4cdce51228bfbb43d25098871c18911c70eeb1c2390f34 guix-build-23d39c5079cf/output/arm64-apple-darwin/bitcoin-23d39c5079cf-arm64-apple-darwin-unsigned.tar.gz
3de4d2cec142a8464788da7571b2f09a47893c7f8d75fe52912ecfcefac72242 guix-build-23d39c5079cf/output/arm64-apple-darwin/bitcoin-23d39c5079cf-arm64-apple-darwin-unsigned
...

💬 Sjors commented on pull request "Add support for RFC8439 variant of ChaCha20":
(https://github.com/bitcoin/bitcoin/pull/27985#issuecomment-1611297395)
#19225 :-)

🚀 fanquake merged a pull request: "ci: remove duplicate bsdmainutils from CI configs"
(https://github.com/bitcoin/bitcoin/pull/27987)

🤔 stickies-v reviewed a pull request: "http: add evhttp_connection_set_closecb to avoid g_requests hang"
(https://github.com/bitcoin/bitcoin/pull/27909#pullrequestreview-1502978676)
Approach ACK 710985d6953eb73b5edddaa7d4d54b58c90d1d34

I think improving remote client disconnect detection is best left for a separate PR; this is a worthwhile improvement on its own and I think uncontroversial.

💬 stickies-v commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1245140984)
nit
```suggestion
auto conn{evhttp_request_get_connection(req)};
```

💬 stickies-v commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1245143903)
I'm curious why we make `notify_all()` dependent on `g_requests.empty()` here, when in all other places we notify for all added and removed events?

```suggestion
if (g_requests.erase(req)) g_requests_cv.notify_all();
```

💬 stickies-v commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1245141287)
nit
```suggestion
auto req{static_cast<evhttp_request*>(arg)};
```

💬 sipa commented on pull request "Add support for RFC8439 variant of ChaCha20":
(https://github.com/bitcoin/bitcoin/pull/27985#issuecomment-1611334433)
@Sjors Thanks, I had forgotten about that. Added a reference.

⚠️ dergoegge opened an issue: "libsecp256k1 not instrumented when building with sanitizers"
(https://github.com/bitcoin/bitcoin/issues/27990)
Our libsecp256k1 builds are not instrumented when building with sanitizers (using `--with-sanitizers`).

For example building with:
```
./configure --enable-fuzz --with-sanitizers=fuzzer
make
```
will not instrument secp code paths for fuzzing. Can be checked with `objdump`, e.g. `objdump --disassemble-symbols=secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz`.

As a workaround it is possible to set `CFLAGS` to use the desired sanitizers (e.g. CFLAGS=`-fsanitize=fuzzer-no-link`) but i
...