💬 vasild commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2326175963)
`5956668ecd...f93fab4c58`: rebase to pick CMake (even though this PR only contains changes to `.py` files)

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2326176023)
> 2\. but `BOOST_CHECK` is deprecated

I don't see the deprecation https://live.boost.org/doc/libs/1_86_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level.html, I think it's still useful for truthy checks (e.g. empty optionals).

> one can compare optional against a value without .value() (C++17 addition).

Yes, you're right here, I wrongly assumed some BOOST magic was done in this case instead.

💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2326186779)
`3769f89a78...e59097a0a5`: rebase to pick CMake

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2326190100)
@TheCharlatan oops, I meant #30750.

💬 fanquake commented on pull request "depends: build libevent with `-D_GNU_SOURCE`":
(https://github.com/bitcoin/bitcoin/pull/30743#issuecomment-2326193470)
Guix Build (aarch64):
```bash
27bc371752bd40a5f634cc8769c860dae2faec5f71976dba4fef0b0b22c69dc4 guix-build-556775408797/output/aarch64-linux-gnu/SHA256SUMS.part
a0e74a766c104f2e64196c6711e9eb895786847ff08a92a2c3b253f8b2611725 guix-build-556775408797/output/aarch64-linux-gnu/bitcoin-556775408797-aarch64-linux-gnu-debug.tar.gz
0c2099f20a86aff4766e6762b3a69780d72af0f5b26a8579e614085035461a90 guix-build-556775408797/output/aarch64-linux-gnu/bitcoin-556775408797-aarch64-linux-gnu.tar.gz
17c466
...

👍 fanquake approved a pull request: "build: Fix linking for `fuzz` target when building with MSan"
(https://github.com/bitcoin/bitcoin/pull/30778#pullrequestreview-2277100490)
ACK 787dfaf084a3952319778da9cbcda9d7d619e4ee - as a follow up it would be good to:
* Actually figure out why this fix works, and why the other msan build wasn't broken: https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1741597310.
* Update oss-fuzz to match what is being done here? Now that they've diverged.
* Open an issue to figure out expectations around depends-with-cmake behaviour.

💬 maflcko commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326233559)
For reference, before (https://cirrus-ci.com/task/6668539271053312?logs=ci#L1064):

```
Cross compiling ....................... FALSE
C++ compiler .......................... Clang 18.1.3, /usr/bin/clang++
CMAKE_BUILD_TYPE ...................... RelWithDebInfo
Preprocessor defined macros ...........
C++ compiler flags .................... -fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls -nostdinc++ -nostdlib++ -isystem /msan/cx
...

✅ fanquake closed an issue: "ci: fuzz_msan failed with ==4201==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x55f0c9bdeffb in SetArgs"
(https://github.com/bitcoin/bitcoin/issues/30760)

🚀 fanquake merged a pull request: "build: Fix linking for `fuzz` target when building with MSan"
(https://github.com/bitcoin/bitcoin/pull/30778)

💬 TheCharlatan commented on pull request "depends: build libevent with `-D_GNU_SOURCE`":
(https://github.com/bitcoin/bitcoin/pull/30743#issuecomment-2326285628)
Guix build (aarch64):
```
27bc371752bd40a5f634cc8769c860dae2faec5f71976dba4fef0b0b22c69dc4 guix-build-556775408797/output/aarch64-linux-gnu/SHA256SUMS.part
a0e74a766c104f2e64196c6711e9eb895786847ff08a92a2c3b253f8b2611725 guix-build-556775408797/output/aarch64-linux-gnu/bitcoin-556775408797-aarch64-linux-gnu-debug.tar.gz
0c2099f20a86aff4766e6762b3a69780d72af0f5b26a8579e614085035461a90 guix-build-556775408797/output/aarch64-linux-gnu/bitcoin-556775408797-aarch64-linux-gnu.tar.gz
17c4667a2b
...

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#issuecomment-2326310300)
Force-pushed to address all review comments by @l0rinc and @hodlinator, thanks for the review!
Mostly small style nits, and also added 9ef049db5ea30f60d0b72714c42c74e2d816c820 to move uint256_tests/conversion to arith_uint256_tests [since that better reflects how the code is organized](https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741633513).

🤔 stickies-v reviewed a pull request: "Remove unsafe uint256S() and test-only uint160S()"
(https://github.com/bitcoin/bitcoin/pull/30773#pullrequestreview-2277180999)
Force-pushed to address all review comments by @l0rinc and @hodlinator, thanks for the review!
Mostly small style nits, and also added 9ef049db5ea30f60d0b72714c42c74e2d816c820 to move uint256_tests/conversion to arith_uint256_tests [since that better reflects how the code is organized](https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741633513).

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741905568)
> Thanks, I'll patch this if I have to force-push.

Done!

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741908187)
Nice, didn't quite love the docstring and this is much clearer, thanks.

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741909541)
I was considering moving it but then thought it was quite arbitrary, but you're right - tests mirroring how code is structured is good, so I've added a commit to move the `conversion` tests. Thanks!

👋 0xB10C's pull request is ready for review: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593)

💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2326369142)
rebased after #30741 and fixed the compilation with `-DWITH_USDT=ON` (by also setting `SDT_USE_VARIADIC` in `cmake/module/FindUSDT.cmake`)

💬 jadijadi commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2326382391)
> Also, the commit message is wrong `echo success` is never translated, because `echo` is just a tool to echo something, not to translate a passed string.

you are right and thanks for mentioning it. fixed the message

👍 danielabrozzoni approved a pull request: "test: add check that too large txs aren't put into orphanage"
(https://github.com/bitcoin/bitcoin/pull/30784#pullrequestreview-2277309899)
tACK 30aab71063d039d4386f580a031cb5e5e742c363

💬 maflcko commented on pull request "tracing: explicitly cast block_connected duration to nanoseconds":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2326417585)
re-lgtm ACK cd0edf26c07c8c615f3ae3ac040c4774dcc8e650