💬 laanwj commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1816413934)
thanks!
factoring it out was more fo a readablity thing, i don't really mind if it's in netutil
i'm fine with leaving it where it is, if some other code needs it it can always be moved to a more general place

👍 marcofleon approved a pull request: "util: Treat Assume as Assert when evaluating at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31150#pullrequestreview-2394880609)
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd

💬 fanquake commented on pull request "depends: sqlite 3.46.1":
(https://github.com/bitcoin/bitcoin/pull/29991#issuecomment-2437444272)
Looks like as of `3.48.0` (current release is `3.47.0`), SQLite is going to migrate it's build system from Autotools to [autosetup](https://msteveb.github.io/autosetup/): https://sqlite.org/src/timeline?r=autosetup.

🚀 fanquake merged a pull request: "depends: sqlite 3.46.1"
(https://github.com/bitcoin/bitcoin/pull/29991)

💬 brunoerg commented on pull request "[tests] New fuzz target wallet_rpc":
(https://github.com/bitcoin/bitcoin/pull/30570#discussion_r1816460307)
In 26591de46c93a0c73df8c9133af69dcc7f4db569: `g_limit_to_rpc_command_wallet` is not being set, so `LIMIT_TO_RPC_COMMAND` has no effect in this target. When using `LIMIT_TO_RPC_COMMAND` for this target, it's probably setting `g_limit_to_rpc_command` in `fuzz/rpc.cpp`.

💬 brunoerg commented on pull request "[tests] New fuzz target wallet_rpc":
(https://github.com/bitcoin/bitcoin/pull/30570#issuecomment-2437465419)
> @brunoerg I think this should be ready for review now, I think I just need to currate the list of WALLET_RPC_COMMANDS_SAFE_FOR_FUZZING and WALLET_RPC_COMMANDS_NOT_SAFE_FOR_FUZZING

Did you advance on curating the list? Is this the final list?

💬 brunoerg commented on pull request "[tests] New fuzz target wallet_rpc":
(https://github.com/bitcoin/bitcoin/pull/30570#discussion_r1816464048)
In 26591de46c93a0c73df8c9133af69dcc7f4db569: nit: If something got wrong with `wallet/test/fuzz/rpc.cpp`, this will ask to update the `test/fuzz/rpc.cpp` file.

🤔 fanquake reviewed a pull request: "Drop miniupnp dependency"
(https://github.com/bitcoin/bitcoin/pull/31130#pullrequestreview-2394931860)
Not sure how much you want to do here, but I think this code can be cleaned up further, given it was written to cater for using two protocols, but after this change, we've got 1 protocol (with a fallback).

💬 fanquake commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1816467880)
I think you can just drop this comment entirely, as there is no-longer a "next protocol" in the loop.

💬 fanquake commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1816475493)
Above here:
```cpp
if (g_mapport_enabled_protos & g_mapport_current_proto) {
// Enabling another protocol does not cause switching from the currently used one.
return;
}
```
This code is either dead, or the comment is outdated, as we can no-longer "enable another protocol".

💬 fanquake commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1816468353)
Just up from here, could drop `// High priority protocol.`.

👍 instagibbs approved a pull request: "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped"
(https://github.com/bitcoin/bitcoin/pull/31040#pullrequestreview-2394949723)
ACK 5c299ecafe6f336cffa145d28036b04b87e26712

feel free to ignore nit

💬 instagibbs commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1816479232)
nit: logging here seems excessive considering a failure of the assert makes it obvious what went wrong

👍 tdb3 approved a pull request: "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped"
(https://github.com/bitcoin/bitcoin/pull/31040#pullrequestreview-2394975338)
ACK 5c299ecafe6f336cffa145d28036b04b87e26712

I would also re-ACK quickly if the changes recommended by @rkrux and @instagibbs were implemented, and `DEFAULT_MAX_ORPHAN_TRANSACTIONS` moved to `test_framework/mempool_util.py`

💬 laanwj commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#discussion_r1816495172)
Added an upper limit here to avoid hanging forever or consuming absurd amounts of resources in case of a windows bug. i would imagine 4MB of adapter addresses ought to be enough for anyone.

💬 stickies-v commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816516919)
Good idea, thanks - added you as co-author to 99b12b5cfcf438a59e9f54ad6d2228be86a9bfeb

🤔 stickies-v reviewed a pull request: "tinyformat: enforce compile-time checks for format string literals"
(https://github.com/bitcoin/bitcoin/pull/31149#pullrequestreview-2395013005)
Rebased to address merge-conflict from #29936. Also added commit 99b12b5cfcf438a59e9f54ad6d2228be86a9bfeb to minimize `c_str()` usage (an [earlier review concern](https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2389355374) of @ryanofsky) and minimize code churn, as [suggested](https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816298165) by @maflcko.

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64/32 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1816522021)
> It would be good to share the number you go

The `reindex-chainstate` until 600k, 2 runs just finished - comparing master against the 64/32 bit packing (current state) on Linux (with GCC, showing the above inconsistency).

<details>
<summary>Details</summary>

```bash
hyperfine \
--runs 2 \
--show-output \
--export-json /mnt/my_storage/reindex-xor.json \
--parameter-list COMMIT dea7e2faf1bc48f96741ef
84e25e6f47cefd5a92,353915bae14b9704a209bc09b021d3dd2ee11cf2 \
--prepare 'cmake
...

💬 maflcko commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#issuecomment-2437561736)
re-ACK 23560b468c474bbc6a3abd6c0778da62766ac8f7 🎛

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 23560b468c474bbc6a3a
...

👍 vasild approved a pull request: "optimization: Preallocate addresses in GetAddr based on nNodes"
(https://github.com/bitcoin/bitcoin/pull/29608#pullrequestreview-2395071050)
ACK 66082ca3488e7ad78149e05631dccd09be03c961