💬 Crypt-iQ commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268291011)
yeah I didn't see allocations come up at all in perf

💬 MarcoFalke commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268305587)
Maybe I'll pick up https://github.com/bitcoin/bitcoin/pull/28006 again and remove `Get`?

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642386101)
> needs rebase if still relevant.

For the `previous releases` CI? (I don't find any merge conflicts with current master.)

💬 MarcoFalke commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#issuecomment-1642398008)
Pushed a small change to call `std::ftell` less often if a large data span is written. In a follow-up, changes can be made to call it less often when small data spans are written.

💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642400630)
> I don't find any merge conflicts with current master.

Are you sure you ran a full build after the merge?

💬 Crypt-iQ commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268324286)
happy to review if you do

💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1268324570)
Thanks, just fixed & pushed

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1268326494)
ah of course thanks, fixed and pushed

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642405661)
Ah, the second try did it :)

<details><summary>build log</summary><p>

```
test/fuzz/process_message_e2e.cpp:59:1: error: a type specifier is required for all declarations
FUZZ_TARGET_INIT(process_message_e2e, initialize_process_message_e2e)
^
test/fuzz/process_message_e2e.cpp:59:18: error: use of undeclared identifier 'process_message_e2e'
FUZZ_TARGET_INIT(process_message_e2e, initialize_process_message_e2e)
^
test/fuzz/process_message_e2e.cpp:59:70: error: expected
...

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1268330073)
nice catch!

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1642413131)
> This assumes that a connection can never have more than 1 request, and I'm not sure if that's the case.

I'm also not sure. This PR body leads me to believe that it is possible to have more than 1 request per connection: https://github.com/libevent/libevent/pull/592#issue-292981989 , however I'm not sure whether it's possible for them to be interleaved and cause problems with your connection-based patch. Something I'll try to investigate.

💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1268398896)
🤦 ty sir. fixed & pushed

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268442591)
Good idea! Done.

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268442671)
Done, changed from implicit in the declarations to explicit in the definitions.

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268446564)
> nit: no need to specify where it's used for a public method imo

Done.

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268446978)
Dropped these changes.

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268447455)
Dropped the snakecase updates.

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#issuecomment-1642518571)
Updated to take all feedback by @stickies-v and @vasild -- thank you!

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268486082)
> Done, changed from implicit in the declarations to explicit in the definitions.

The CI is unhappy with that, so reverting the last commit to c69a74229da514228d3388e9652d2ea2e89224d7.

💬 jonatack commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1642547623)
ACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba