💬 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

📝 mzumsande opened a pull request: "test: fix intermittent failure in wallet_resendwallettransactions.py"
(https://github.com/bitcoin/bitcoin/pull/28108)
Fixes #28094

The test bumps the mocktime for ~2 weeks and then triggers eviction from the mempool. But this bump will also cause a new resubmit, and if the timing is such that this resubmit happens right after the eviction and before the check that the tx was evicted, the test can fail as in #28094:

```
node0 2023-07-17T21:31:23.809483Z (mocktime: 2023-08-02T09:46:27Z) [httpworker.1] [validation.cpp:267] [LimitMempoolSize] [mempool] Expired 2 transactions from the memory pool
node0 2023-
...

👍 stickies-v approved a pull request: "net processing, refactor: Decouple PeerManager from gArgs"
(https://github.com/bitcoin/bitcoin/pull/27499#pullrequestreview-1537528603)
ACK 064ac73d0715fb2c371e68f8d4d234fee002299b

💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268348590)
nit: no longer needed

💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268347744)
nit: with this change, `#include <node/txreconciliation.h>` can be removed from `init.cpp`

💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268400386)
nit: probably best to use `DEFAULT_BLOCKSONLY` to make it explicit what `false` refers to, and keep it in sync?

```suggestion
bool ignore_incoming_txs{DEFAULT_BLOCKSONLY};
```

Alternatively (but I don't think using `peerman_opts` like this is elegant), could deduplicate it entirely:
<details>
<summary>git diff on e3ddc5cc96cf9f6339e7828c570fa5ad5130918a</summary>

```diff
diff --git a/src/init.cpp b/src/init.cpp
index dd7a359a2..8b62690f7 100644
--- a/src/init.cpp
+++ b/s
...

🤔 jonatack reviewed a pull request: "test: fix intermittent failure in wallet_resendwallettransactions.py"
(https://github.com/bitcoin/bitcoin/pull/28108#pullrequestreview-1537849103)
Light "this looks like the other tests in this file" ACK e667bd68a10512ddc784df44bdcb63ee441e5551