👍 tdb3 approved a pull request: "test: Remove dead code from interface_zmq test"
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335986876)
CR re ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6

💬 beage666 commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381599058)
Ok

👋 l0rinc's pull request is ready for review: "Cover remaining tinyformat usages in CheckFormatSpecifiers"
(https://github.com/bitcoin/bitcoin/pull/30999)

📝 furszy opened a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000)
Expands the benchmark framework with the existing `-testdatadir` arg,
enabling the ability to change the benchmark data directory.

This is useful for running benchmarks on different storage devices, and
not just under the OS `/tmp/` directory.

A good use case is #28574, where we are benchmarking the wallet
migration process on an HDD.

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172239)
Yeah, looking at it again, this doesn't make sense because the test may fail if someone actually scales the wait time down. I changed it back and will leave the mocktime approach for a follow-up.

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172255)
Changed to use `**kwargs` in both `ensure_for` impls

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1780172263)
I agree and couldn't find any good use cases for it. It can be re-added when there is a use case for it so I have removed it for now and simplified the code.

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2381605700)
Addressed feedback from @maflcko , thanks!

👍 tdb3 approved a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2336023567)
re ACK 352b8209aa5327f7d369e2acc4d87f9767389a6b

Also sanity checked testing timing (https://github.com/bitcoin/bitcoin/pull/30893?notification_referrer_id=NT_kwDOBljilbUxMjM4MjY5Nzc4OToxMDY0ODg0Njk#pullrequestreview-2316167311)

🤔 tdb3 reviewed a pull request: "Split CConnman"
(https://github.com/bitcoin/bitcoin/pull/30988#pullrequestreview-2336026243)
Concept ACK

💬 glozow commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#issuecomment-2381649791)
reACK 940edd6 via range-diff

💬 mzumsande commented on issue "assumeutxo: not syncing the snapshot chainstate":
(https://github.com/bitcoin/bitcoin/issues/30971#issuecomment-2382050086)
> It also seems like the cache resizing has not worked correctly?

Just wanted to mention that I think that the cache resize worked as expected here:
After the snaphot is loaded, the cache of the background chainstate is set to a lower value:
`resized coinstip cache to 22.0 MiB`
because loading from the snapshot chainstate is prioritized.

Then, the cache for the background chainstate is frequently flushed because blocks are downloaded to it and it runs full quickly, resulting in a flush.
...

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2382159543)
Just as a note, it should already be possible to pick the device via an env var, see https://en.cppreference.com/w/cpp/filesystem/temp_directory_path#Notes.

🤔 maflcko reviewed a pull request: "Cover remaining tinyformat usages in CheckFormatSpecifiers"
(https://github.com/bitcoin/bitcoin/pull/30999#pullrequestreview-2336508940)
d49dfafa9aea349ae6741f49ed992b78a88b6839 looks correct, but I am not sure the others.

💬 maflcko commented on pull request "Cover remaining tinyformat usages in CheckFormatSpecifiers":
(https://github.com/bitcoin/bitcoin/pull/30999#discussion_r1780515759)
nit: not sure about changing this back again to accommodate test-only code. Instead of using `-1` for test code, you could set `constexpr INVALID=-1;` and use that.

💬 maflcko commented on pull request "Cover remaining tinyformat usages in CheckFormatSpecifiers":
(https://github.com/bitcoin/bitcoin/pull/30999#discussion_r1780527702)
Why remove this comment? `format("'%1$*3$s %2$-*3$s'", "hi", "w", 12)` is still unsupported and parsed incorrectly at compile time.

💬 maflcko commented on pull request "Cover remaining tinyformat usages in CheckFormatSpecifiers":
(https://github.com/bitcoin/bitcoin/pull/30999#discussion_r1780512926)
in cba51f2e380f0f89f799369b489c2e25e7215221: Not sure about turning the passing one into a runtime check from a compile-time check. Previously it was trivial to compile a single unit (this test) to sanity check the parser, as well as check the compile failures for various errors, by simply looking at the compile output. Also, the code was as close as possible to the real code, serving as documentation of how to use it.

Now, checking the passing cases requires not only compiling, but also link
...

💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2382352367)
macOS Monterey 12 hardware support: https://support.apple.com/en-us/103260
macOS Big Sur 11 hardware support: https://support.apple.com/en-us/111980

Those differ by about a year. By time this is released, Monterey will work on ten year old machines. So I think it's fine to drop Big Sur (for Qt).

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2382371880)
The bump for Ubuntu seems fine as well. There's already the requirement from https://github.com/bitcoin/bitcoin/pull/29987 for all binaries, so going one LTS further for the GUI seems fine.

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2382378949)
> Don't forget to update `build-osx.md`.

I don't think the gui requirements are documented anywhere, because for now they were always equal to the daemon requirements? Possibly just saying globally "qt6" is required implies the additional requirement on the OS?