👍 vasild approved a pull request: "optimization: Preallocate addresses in GetAddr based on nNodes"
(https://github.com/bitcoin/bitcoin/pull/29608#pullrequestreview-2395071050)
ACK 66082ca3488e7ad78149e05631dccd09be03c961
(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
...
(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)
(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)
(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 `.
(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?
(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).
(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
...
(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.
(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?
(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
(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 ...
```
(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"
```
(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
...
(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 ",
```
(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
(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
(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.
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1816659057)
Thanks! Done.
💬 fanquake commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2437744416)
Guix Build (x86_64 & aarch64):
```bash
c27a601dbc89ed3abccb6f742178f15f0eb256cd5086d8360dd6727c9c866cab guix-build-a4e17239ca54/output/aarch64-linux-gnu/SHA256SUMS.part
0fdb1ff5320398ec5cf6378f72c826c619909e1079a9e8ab871bac938db41c0e guix-build-a4e17239ca54/output/aarch64-linux-gnu/bitcoin-a4e17239ca54-aarch64-linux-gnu-debug.tar.gz
60f6d4f34f3d5af5aeb6cf678a6f2ca60035d52cf8c48b3dbea7a54a8ce89776 guix-build-a4e17239ca54/output/aarch64-linux-gnu/bitcoin-a4e17239ca54-aarch64-linux-gnu.tar.g
...
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2437744416)
Guix Build (x86_64 & aarch64):
```bash
c27a601dbc89ed3abccb6f742178f15f0eb256cd5086d8360dd6727c9c866cab guix-build-a4e17239ca54/output/aarch64-linux-gnu/SHA256SUMS.part
0fdb1ff5320398ec5cf6378f72c826c619909e1079a9e8ab871bac938db41c0e guix-build-a4e17239ca54/output/aarch64-linux-gnu/bitcoin-a4e17239ca54-aarch64-linux-gnu-debug.tar.gz
60f6d4f34f3d5af5aeb6cf678a6f2ca60035d52cf8c48b3dbea7a54a8ce89776 guix-build-a4e17239ca54/output/aarch64-linux-gnu/bitcoin-a4e17239ca54-aarch64-linux-gnu.tar.g
...
👍 brunoerg approved a pull request: "Introduce `g_fuzzing` global for fuzzing checks"
(https://github.com/bitcoin/bitcoin/pull/31093#pullrequestreview-2395260296)
crACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53
(https://github.com/bitcoin/bitcoin/pull/31093#pullrequestreview-2395260296)
crACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53