💬 l0rinc commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2389738877)
nit: most logs don't need a trailing newline anymore

💬 l0rinc commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2389762328)
I understand that we need different sizes for ipv4 and ipv6, hence these ugly polimorphism-mimicking structures, but I'm wondering if there's a way to hide this ugliness in the C++ code with either decicated structures or something lke `std::variant<sockaddr_in, sockaddr_in6>`

💬 l0rinc commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2389740440)
We don't actually need the value here, just that it exists, right?

```suggestion
Assume(std::ranges::any_of(m_listen, [&](const auto& sock) { return sock.get() == &listen_sock; }));
```

💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3353488242)
#33511 contains the patch above and should fix this issue

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392615787)
Agree it doesn't really fit. Implemented there because of similarity with `sendrawtransaction`.
Any suggestions? Maybe `rpc/rawtransaction.cpp` or `rpc/net.cpp`?

💬 m3dwards commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3353508042)
ACK 93a70a42d30fa2f9404b76d5bbdb5ea316fc1032

Also checked archive contents are the same as OP:
<details>
<summary>diff -r $dir1/libqrencode-4.1.1 $dir2/qrencode-4.1.1</summary>

```shell
Only in /var/folders/q9/65c9dvyj3lbdsv5zrhp_8qk80000gn/T/tmp.g0dIABqWhw/libqrencode-4.1.1: .github
Only in /var/folders/q9/65c9dvyj3lbdsv5zrhp_8qk80000gn/T/tmp.g0dIABqWhw/libqrencode-4.1.1: .gitignore
Only in /var/folders/q9/65c9dvyj3lbdsv5zrhp_8qk80000gn/T/tmp.g0dIABqWhw/libqrencode-4.1.1: .travis.yml
...

💬 davidgumberg commented on pull request "test: sock: Enable socket pair tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/33506#issuecomment-3353532500)
Rebased to split into two commits, a mostly refactor commit https://github.com/bitcoin/bitcoin/pull/33506/commits/06a5140e96a75948cd2e490b414419a5485f2cfd and the commit that adds the new `CreateSocketPair()` for windows and enables Windows socket tests https://github.com/bitcoin/bitcoin/pull/33506/commits/7586205c4a5e94c7e2357d8928c68dfab95dc47a

💬 achow101 commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#issuecomment-3353541552)
Can you also do `gen-bitcoin-conf.sh` since that will need to update for the new defaults as well.

💬 polespinasa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353606181)
How about we make the expiration time dynamic based on the position (computer by fee rate) in the mempool?

Txs more likely to be mined have a bigger expiry time. If the mempool clears a bit, txs with low feerate will be on top of the mempool and their expiry time can be re-calculated to increase and avoid evicting txs that will be mined.

This way transactions more likely to be mined will not be evicted while the txs less likely to be mined will be evicted and only kept if the mempool clears a
...

💬 mzumsande commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#discussion_r2392697933)
done with the latest push. I also adjusted the third commit with the new test in a similar way.

💬 mzumsande commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3353612781)
[a52c148](https://github.com/bitcoin/bitcoin/commit/a52c148ece2296f22e2fe1b7e9584d49e23f03f2) to [14ae71f](https://github.com/bitcoin/bitcoin/commit/14ae71f323dd011c6d51470ea15cf00750970f65):
- Used mocktime everywhere to speed up the test.
- corrected `on_inv` override variable name in 1st commit (my IDE complained about that)

💬 achow101 commented on pull request "test: Use extra_port() helper in feature_bind_extra.py":
(https://github.com/bitcoin/bitcoin/pull/33260#issuecomment-3353680023)
ACK fabc2615af26c61a503f23ae4fd0353f90602bbe

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392760028)
You're right! Done :)

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392760668)
You're right! Done :)

✅ achow101 closed an issue: "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added"
(https://github.com/bitcoin/bitcoin/issues/33250)

🚀 achow101 merged a pull request: "test: Use extra_port() helper in feature_bind_extra.py"
(https://github.com/bitcoin/bitcoin/pull/33260)

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392767537)
I might be wrong but I don't think there's a way to know if the peer accepted it or not.

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392777914)
I would rather keep the same format as `sendrawtransaction`.

💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3353787691)
Code Review ACK [`24391ed`](https://github.com/bitcoin/bitcoin/pull/32821/commits/24391ed7804a724a62034b21150c89e45ac9b625)

Just a few non-blocking nits

🤔 Prabhat1308 reviewed a pull request: "rpc: Handle -named argument parsing where '=' character is used"
(https://github.com/bitcoin/bitcoin/pull/32821#pullrequestreview-3286685012)
Code Review ACK [`24391ed`](https://github.com/bitcoin/bitcoin/pull/32821/commits/24391ed7804a724a62034b21150c89e45ac9b625)

Just left a few non-blocking nits