👍 hodlinator approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2394616462)
re-ACK 80d35bc20797e59571e82fe6919705b33dcc40cd

* Commit message improvements.
* `test_path` -> `test_name` (improved naming).
* Made `test_name` be included as part of randomized test directory when *not* setting `-testdatadir`. Including it is helpful for locating test data of a specific test when multiple tests fail. :+1:

Verified correct unit test directory behavior using `build/src/test/test_bitcoin -- -testdatadir=/tmp/foo/`, then not passing the `-testdatadir` and simultaneously r
...

💬 hodlinator commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1816263171)
(Since with the latest non-rebase push you are now touching 2/4 lines mentioning `TEST_DIR_PATH_ELEMENT`, iff you were to re-touch again, it would be nice if you also renamed it to `TESTS_DIR_NAME` as per @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/30748#pullrequestreview-2283763614)).

✅ fanquake closed a pull request: "depends: bump miniupnpc to 2.2.8"
(https://github.com/bitcoin/bitcoin/pull/30301)

💬 fanquake commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2437321903)
I think #31130 is going to go in for `v29.0`. If that doesn't happen, we can reopen here.

💬 rkrux 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_r1816346116)
Was this https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815889280 done in 5c299ecafe6f336cffa145d28036b04b87e26712? I could not find it.

👍 rkrux 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-2394740828)
ACK 5c299ecafe6f336cffa145d28036b04b87e26712

Thanks for adding this test, it's easy to follow through.

💬 rkrux 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_r1816349294)








```suggestion
assert_equal(len(node.getorphantxs()), DEFAULT_MAX_ORPHAN_TRANSACTIONS)
```
Storing in another variable is not necessary only for one time usage.

You don't need to invalidate ACKs only for this though.

💬 laanwj commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2437336876)
> I think https://github.com/bitcoin/bitcoin/pull/31130 is going to go in for v29.0. If that doesn't happen, we can reopen here.

yes... i dont think i've seen any PR that popular in my time here 😄

💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2437349340)
Waiting for #31150 to go in

👍 fanquake approved a pull request: "ci: Temporary workaround for old CCACHE_DIR cirrus env"
(https://github.com/bitcoin/bitcoin/pull/31146#pullrequestreview-2394783289)
ACK fa9747a896188f4dd70f275aec2469dba5cd435e - have seen this now.

🚀 fanquake merged a pull request: "ci: Temporary workaround for old CCACHE_DIR cirrus env"
(https://github.com/bitcoin/bitcoin/pull/31146)

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)
Not sure about the CI failure. IIRC it happened after this push: https://github.com/bitcoin/bitcoin/compare/15cfeebb47587af6ce7be72fd52c57a0483d86d2..5757fdf0dc74ec9d6bbefba937d6e23b09652605

The logs don't say anything except: `validation_chainstatemanager_tests ...Exit code 0xc0000409
2024-10-24T23:23:18.2978457Z ***Exception: 9.33 sec` and the debug log is truncated (presumably when the exception happens).

It would be good if ctest/Windows actually printed the exception and error mess
...

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

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