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

💬 fanquake commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1611341497)
Not sure what is happening here, but when testing this commit (rebased on master), on aarch64, `make check` just "dies" in the wallet tests? https://gist.github.com/fanquake/087fdff84b0d5379d94859de47a0e797

💬 dergoegge commented on pull request "refactor: Continue moving application data from CNode to Peer":
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1611345224)
> Needs rebase if still relevant

Rebased

💬 TheCharlatan commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1611386623)
ACK a51d7abf1e13c532c7acf437c3a65a9511b44987

Guix builds:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
6edd213a90a043d09da144b99332ab16a320774098d35de28464262b25f260b2 guix-build-a51d7abf1e13/output/aarch64-linux-gnu/SHA256SUMS.part
1ececa8121d51a5c358e25e1a4b529413faadef5721387005db2b928c05cad3d guix-build-a51d7abf1e13/output/aarch64-linux-gnu/bitcoin-a51d7abf1e13-aarch64-linux-gnu-debug.tar.gz
fbc1724995b2d9
...

💬 MarcoFalke commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1611401222)
Ah, sorry that's a bug. Can you check `log_path=${BASE_SCRATCH_DIR}/sanitizer-output/tsan`?

💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1611401634)
Guix builds:
```
6edd213a90a043d09da144b99332ab16a320774098d35de28464262b25f260b2 guix-build-a51d7abf1e13/output/aarch64-linux-gnu/SHA256SUMS.part
1ececa8121d51a5c358e25e1a4b529413faadef5721387005db2b928c05cad3d guix-build-a51d7abf1e13/output/aarch64-linux-gnu/bitcoin-a51d7abf1e13-aarch64-linux-gnu-debug.tar.gz
fbc1724995b2d9a43c0bccbb3c20e02d115d9aeaecce0792bde46d9fedc0dbe0 guix-build-a51d7abf1e13/output/aarch64-linux-gnu/bitcoin-a51d7abf1e13-aarch64-linux-gnu.tar.gz
8a9edb553d6e6db1d63
...

💬 MarcoFalke commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1611403242)
Anyway, I'd guess it's the bdb upstream data race, so maybe for now try without bdb?

💬 CocolinoFan commented on issue "Finding peers to connect to after -onlynet changes may be problematic":
(https://github.com/bitcoin/bitcoin/issues/26035#issuecomment-1611404822)
100% agree my node is running for 5 days now on IPv4, i2p, cjdns and Tor.
I have 75 in peers for IPv4 but 0 for the rest.
I can manually add peers for i2p, cjdns and Tor but in my mind it should work like [vasild] said, maintain at least one connection to every available network.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1245203016)
Yeah. Added 6a0262a4 improving this docs in a separated commit.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1244419674)
It refers to the `NodeContext` struct. Same as the line that is right above it.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1245203265)
Done

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1244446404)
I thought about that initially too, but we have #24230 going into the opposite direction (removing the `CBlockIndex` usages from the `index/` directory).

I think that the best would be to place it inside the chain interface. But let me check how far that goes. If it requires more changes, might be good to leave it for a follow-up to not keep expanding this PR.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1611418821)
Tackled naming and docs suggestions, thanks mzumsande.