💬 maflcko commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1836124767)
nit in b2ac698ce102edaf8039f42b8793ca10deaa0a53: Can use f-strings?

💬 maflcko commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1836135115)
8362ec6ebbe2410c551c4d6550438426c425ce3c: I am not sure this is accurate. The time can now be scaled, but in reality it is really a fixed time.

🤔 maflcko reviewed a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2426388517)
almost ack dde84e2b2b6a64f5f67d4dff6e3fb2fc7d2c5a77 🚿

<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: almost ack dde84e2b2b6a
...

💬 maflcko commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1836124512)
nit in b2ac698ce102edaf8039f42b8793ca10deaa0a53: Can remove unused kwargs?

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1836226771)
Time to return to C89? ;)
```suggestion
while (strchr("#0- +", *it)) ++it;
```

📝 maflcko opened a pull request: "refactor: Drop deprecated space in operator""_mst"
(https://github.com/bitcoin/bitcoin/pull/31267)
The space is deprecated according to https://en.cppreference.com/w/cpp/language/user_literal#Literal_operators and compilers will start to warn about this. For example, GCC-15 should warn when compiling under C++23, according to https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#index-Wdeprecated-literal-operator and Clang-20 will do so by default, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-literal-operator.

Fix it by removing the unused an
...

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836348991)
I don't think it is right to use mockable time here. Also, `GetTime` is deprecated, so what about `TicksSinceEpoch<std::chrono::nanoseconds>(SystemClock::now())`?

Also, I think it would be better to switch `test_name / rand_str` to `rand_str / test_name`, so that all unit tests from the same time are bundled together. This would also be identical to the functional test runner.

💬 maflcko commented on pull request "doc: Fix missing comma in JSON example in REST-interface.md":
(https://github.com/bitcoin/bitcoin/pull/31259#issuecomment-2467792806)
lgtm ACK 5e3b444022c354ad4d69e3142f7bc98d723c9e29

💬 TheCharlatan commented on pull request "refactor: Drop deprecated space in operator""_mst":
(https://github.com/bitcoin/bitcoin/pull/31267#issuecomment-2467799611)
```
git grep -i 'operator"" '
src/test/fuzz/miniscript.cpp:using miniscript::operator"" _mst;
src/test/miniscript_tests.cpp:using miniscript::operator"" _mst;
```
Might want to correct those two imports as well?

💬 maflcko commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1836375257)
Looks like this coverage should be preserved?

See also https://corecheck.dev/bitcoin/bitcoin/pulls/31248

🚀 fanquake merged a pull request: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593)

💬 l0rinc commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1836449767)
I thought of that, but not sure how to make it work, I'm getting:
> note: non-constexpr function 'strchr' cannot be used in a constant expression

🚀 fanquake merged a pull request: "ci: make ctest stop on failure"
(https://github.com/bitcoin/bitcoin/pull/31257)

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2467876464)
Thanks for the comments @vasild. I plan to address your comments soon, when I rebase on https://github.com/bitcoin/bitcoin/pull/26593.

🚀 fanquake merged a pull request: "scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}"
(https://github.com/bitcoin/bitcoin/pull/31163)

🚀 fanquake merged a pull request: "doc: Fixup bitcoin-wallet manpage chain selection args"
(https://github.com/bitcoin/bitcoin/pull/31264)

💬 willcl-ark commented on pull request "doc: Fixup bitcoin-wallet manpage chain selection args":
(https://github.com/bitcoin/bitcoin/pull/31264#discussion_r1836491398)
We generally address these as and when files are touched (and folks remember).

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2467888419)
I ran a `reindex-chainstate` until 860k blocks, before and after this change, 2 runs per commit:

<details>
<summary>benchmark</summary>

```bash
hyperfine \
--runs 2 \
--export-json /mnt/my_storage/reindex-chainstate-obfuscation-overhead.json \
--parameter-list COMMIT 866f4fa521f6932162570d6531055cc007e3d0cd,3efc72ff7cbdfb788b23bf4346e29ba99362c120,08db1794647c37f966c525411f931a4a0f6b6119 \
--prepare 'git checkout {COMMIT} && git clean -fxd && git reset --hard && cmake -B build -
...

🤔 Abdulkbk reviewed a pull request: "test: Add combinerawtransaction test to rpc_createmultisig"
(https://github.com/bitcoin/bitcoin/pull/31249#pullrequestreview-2427016825)
ACK 83fab3212c91d91fc5502f940c901a07772ff747

📝 purpleKarrot opened a pull request: "cmake: add optional source files to bitcoin_crypto directly"
(https://github.com/bitcoin/bitcoin/pull/31268)
Avoid having many static libraries by adding the optional sources to the target `bitcoin_crypto` directly.
Set the necessary compile options at the source file level, rather than the target level.