💬 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

💬 fanquake commented on pull request "util: Treat Assume as Assert when evaluating at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31150#issuecomment-2437612378)
Guix Build:
```bash
7a44582701c541810939dcc31608f04f52fdde4d60281d3432c246df1df2b903 guix-build-fa69a5f4b76a/output/aarch64-linux-gnu/SHA256SUMS.part
688a63d66f5d194f84403dd193a69db76a44c767abdcb2a1ad1ffb20a5e8ecbc guix-build-fa69a5f4b76a/output/aarch64-linux-gnu/bitcoin-fa69a5f4b76a-aarch64-linux-gnu-debug.tar.gz
5357d07cb97dd44e870c83863cb1bf5db963a32d25dd87add96474f48cb3d74d guix-build-fa69a5f4b76a/output/aarch64-linux-gnu/bitcoin-fa69a5f4b76a-aarch64-linux-gnu.tar.gz
41318dc317d5aeb2
...

🚀 fanquake merged a pull request: "util: Treat Assume as Assert when evaluating at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31150)

👋 dergoegge's pull request is ready for review: "Introduce `g_fuzzing` global for fuzzing checks"
(https://github.com/bitcoin/bitcoin/pull/31093)

💬 rkrux commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1816583968)
`- set(big_parent_txids)`

I suppose this can be dropped based on the assertion above on lines 167/8?`big_parent_txids` is already in `resulting_mempool_txids `.

💬 jonatack commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816599432)
Is all the indentation churn really necessary?

Does changing strprintf to tfm_format_raw here catch any potential issues that wouldn't be seen in manual testing?

💬 dergoegge commented on pull request "[tests] New fuzz target wallet_rpc":
(https://github.com/bitcoin/bitcoin/pull/30570#discussion_r1816611878)
There are many more wallet rpcs that go to disk (e.g. `createwallet`). I would assume a lot of the rpcs that modify a wallet also flush the changes to disk. And I don't think it is possible to even have a wallet that doesn't exist on disk, so trying to avoid disk access can't work.

To work around this I would suggest to create (and delete at the end) a new wallet (e.g. `/tmp/fuzz-wallet-xyz`) each iteration and call wallet rpcs on that specific wallet (could be more than one).

💬 stickies-v commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816615151)
> Is all the indentation churn really necessary?

It's `clang-format` output which I believe is what we're trying to adhere to.

> Does changing strprintf to tfm_format_raw here catch any potential issues that wouldn't be seen in manual testing?

The code does not compile with `strprintf` because this PR enforces compile-time checks, which fail because of the width specifiers in the format string (there's quite some discussion on these limitations in https://github.com/bitcoin/bitcoin/pull
...

💬 jonatack commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816628934)
Clang-format can be helpful for new code, but I don't think it's necessary to re-indent code for it that isn't otherwise touched -- that's just noisy churn.

💬 jonatack commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816630949)
Please provide an example of how this code could crash currently without this change?

👍 vasild approved a pull request: "netinfo: add peer services column and outbound-only option"
(https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2395186250)
ACK 17f8f03ec69b6dd3d61137a39b8f88201ec500dc

💬 vasild commented on pull request "netinfo: add peer services column and outbound-only option":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1816619674)
nit: since `args.at(1)` is repeated also in the `throw` statements, maybe:

```cpp
std::string_view arg1{args.at(1)};
if (n && (arg1 == "o" || arg1 == "outonly")) {
} else {
throw ... arg1 ...
```

💬 vasild commented on pull request "netinfo: add peer services column and outbound-only option":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1816633325)
nit: `level|outonly|help` reads as "exactly one of `level`, `outonly` or `help`". I guess it should be

```
-netinfo (level [outonly]) | help
```
or two lines:

```
"-netinfo \"help\"\n"
"-netinfo level [outonly]\n\n"
```

💬 stickies-v commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816643611)
> Clang-format can be helpful for new code, but I don't think it's necessary to re-indent code for it that isn't otherwise touched -- that's just noisy churn.

I'm just going with our `clang-format` settings as defined in `src/.clang-format` - if you feel like those settings are suboptimal opening a separate pull/issue for that is probably more appropriate?

> Please provide an example of how this code could crash currently without this change?

The PR states that there is no behaviour cha
...

💬 stickies-v commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816650254)
> Please provide an example of how this code could crash currently without this change?

If you're asking about how such an error could sneak in without this PR, a change as simple adding a single extra `*` would lead to a run-time exception if no extra parameter is added:

```cpp
result += tfm::format_raw("<-> type net v mping ping send recv txn blk hb %**s%*s%*s ",
```

👍 dergoegge approved a pull request: "fuzz: fuzz connman with non-empty addrman + ASMap"
(https://github.com/bitcoin/bitcoin/pull/29536#pullrequestreview-2395241979)
Code review ACK 552cae243a1bf26bfec03eccd1458f3bf33e01dc

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only option":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1816658776)
done

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only option":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1816659057)
Thanks! Done.