π¬ dergoegge commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341484545)
```suggestion
CBlock block = *info[index].block;
block.vtx.clear();
```
The headers message is a vector of blocks without transactions, so I think you should clear the copy of the block here? Although, technically this doesn't matter because there's only one header in the vector being sent, so the following txs are just ignored.
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341484545)
```suggestion
CBlock block = *info[index].block;
block.vtx.clear();
```
The headers message is a vector of blocks without transactions, so I think you should clear the copy of the block here? Although, technically this doesn't matter because there's only one header in the vector being sent, so the following txs are just ignored.
π¬ dergoegge commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341516465)
```suggestion
CBlockHeaderAndShortTxIDs baseCmpctBlock = cmpctBlock;
```
nit: there are few instances of camel case usage here. Our convention is to use snake case (see dev notes).
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341516465)
```suggestion
CBlockHeaderAndShortTxIDs baseCmpctBlock = cmpctBlock;
```
nit: there are few instances of camel case usage here. Our convention is to use snake case (see dev notes).
π€ dergoegge requested changes to a pull request: "fuzz: compact block harness"
(https://github.com/bitcoin/bitcoin/pull/33300#pullrequestreview-3212403908)
Did a first pass, overall approach looks good to me
(https://github.com/bitcoin/bitcoin/pull/33300#pullrequestreview-3212403908)
Did a first pass, overall approach looks good to me
π¬ Crypt-iQ commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33344#issuecomment-3285838336)
Checked that #32646 and #33296 backports are correct and ran `p2p_compactblocks.py` just to be sure.
(https://github.com/bitcoin/bitcoin/pull/33344#issuecomment-3285838336)
Checked that #32646 and #33296 backports are correct and ran `p2p_compactblocks.py` just to be sure.
π¬ Raimo33 commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344704754)
you mean as it was before?
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344704754)
you mean as it was before?
π theStack approved a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3217716655)
re-ACK 75e6984ec8c6fa196ad78c11f454da506d7c8ff1
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3217716655)
re-ACK 75e6984ec8c6fa196ad78c11f454da506d7c8ff1
π pinheadmz opened a pull request: "Remove unnecessary casts when calling socket operations"
(https://github.com/bitcoin/bitcoin/pull/33378)
During review of https://github.com/bitcoin/bitcoin/pull/32747 several casting operations were questioned in existing code that had been copied or moved. That lead me to find a few other similar casts in the codebase.
It turns out that since the `Sock` class wraps syscalls with its own internal casting (see https://github.com/bitcoin/bitcoin/pull/24357 and https://github.com/bitcoin/bitcoin/pull/20788 written in 2020-2022) we no longer need to cast the arguments when calling these functions.
...
(https://github.com/bitcoin/bitcoin/pull/33378)
During review of https://github.com/bitcoin/bitcoin/pull/32747 several casting operations were questioned in existing code that had been copied or moved. That lead me to find a few other similar casts in the codebase.
It turns out that since the `Sock` class wraps syscalls with its own internal casting (see https://github.com/bitcoin/bitcoin/pull/24357 and https://github.com/bitcoin/bitcoin/pull/20788 written in 2020-2022) we no longer need to cast the arguments when calling these functions.
...
π hebasto opened a pull request: "cmake: Fix regression in `secp256k1.cmake`"
(https://github.com/bitcoin/bitcoin/pull/33379)
This PR fixes a regression introduced in https://github.com/bitcoin/bitcoin/pull/33101 (mea culpa).
From the CMake [docs](https://cmake.org/cmake/help/latest/command/enable_language.html):
> The following restrictions apply to where `enable_language()` may be called:
>
> - It must be called in file scope, not in a function call.
Fixes https://github.com/bitcoin/bitcoin/issues/33153.
(https://github.com/bitcoin/bitcoin/pull/33379)
This PR fixes a regression introduced in https://github.com/bitcoin/bitcoin/pull/33101 (mea culpa).
From the CMake [docs](https://cmake.org/cmake/help/latest/command/enable_language.html):
> The following restrictions apply to where `enable_language()` may be called:
>
> - It must be called in file scope, not in a function call.
Fixes https://github.com/bitcoin/bitcoin/issues/33153.
π¬ Sjors commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#issuecomment-3286123576)
Fixed typo.
(https://github.com/bitcoin/bitcoin/pull/33374#issuecomment-3286123576)
Fixed typo.
β
Sjors closed a pull request: "mining: log failed blocks in submitSolution()"
(https://github.com/bitcoin/bitcoin/pull/33372)
(https://github.com/bitcoin/bitcoin/pull/33372)
π¬ Sjors commented on pull request "mining: log failed blocks in submitSolution()":
(https://github.com/bitcoin/bitcoin/pull/33372#issuecomment-3286128162)
The other PR has the additional benefit that the client can broadcast the block in additional ways, and/or inspect it with `checkBlock()`.
(https://github.com/bitcoin/bitcoin/pull/33372#issuecomment-3286128162)
The other PR has the additional benefit that the client can broadcast the block in additional ways, and/or inspect it with `checkBlock()`.
π¬ Crypt-iQ commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3286270887)
tACK 65a10fc3c52ea09a4794345bcf607dff908c783a
I modified the fuzz test and checked that the added `Assume` in net_processing can't be hit.
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3286270887)
tACK 65a10fc3c52ea09a4794345bcf607dff908c783a
I modified the fuzz test and checked that the added `Assume` in net_processing can't be hit.
π¬ l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2345036887)
sorry, woke up in the middle of the night and replied from my phone :))
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2345036887)
sorry, woke up in the middle of the night and replied from my phone :))
π¬ vasild commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2345042637)
I see. Commit `test: don't throw from the destructor of DebugLogHelper` from https://github.com/bitcoin/bitcoin/pull/26812 changes the destructor to not throw exceptions. It has been staying in that PR for a looong time. Maybe it deserves its own PR?
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2345042637)
I see. Commit `test: don't throw from the destructor of DebugLogHelper` from https://github.com/bitcoin/bitcoin/pull/26812 changes the destructor to not throw exceptions. It has been staying in that PR for a looong time. Maybe it deserves its own PR?
π¬ l0rinc commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286334324)
Multiple people have pushed this already, e.g. [`723c49b` (#32128)](https://github.com/bitcoin/bitcoin/pull/32128/commits/723c49b63bb10da843fbb6efc6928dca415cc47f) or [l0rinc/bitcoin@`b23c6a1` (#34)](https://github.com/l0rinc/bitcoin/pull/34/commits/b23c6a1fc8f9a44561add79427d2f3fff4c3718f).
I was about to push my change from my fork after I finished all IBD measurements.
(https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286334324)
Multiple people have pushed this already, e.g. [`723c49b` (#32128)](https://github.com/bitcoin/bitcoin/pull/32128/commits/723c49b63bb10da843fbb6efc6928dca415cc47f) or [l0rinc/bitcoin@`b23c6a1` (#34)](https://github.com/l0rinc/bitcoin/pull/34/commits/b23c6a1fc8f9a44561add79427d2f3fff4c3718f).
I was about to push my change from my fork after I finished all IBD measurements.
π TheCharlatan opened a pull request: "test: Add submitblock test in interface_ipc"
(https://github.com/bitcoin/bitcoin/pull/33380)
Expands the ipc mining test a bit with submitting a solved block and checking its validity.
(https://github.com/bitcoin/bitcoin/pull/33380)
Expands the ipc mining test a bit with submitting a solved block and checking its validity.
π¬ BaStiaNQeu commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286386567)
_:-!.
pt., 12 wrz 2025, 20:02 uΕΌytkownik l0rinc ***@***.***>
napisaΕ:
> *l0rinc* left a comment (bitcoin/bitcoin#33376)
> <https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286334324>
>
> Multiple people have pushed this already, e.g. 723c49b (#32128)
> <https://github.com/bitcoin/bitcoin/pull/32128/commits/723c49b63bb10da843fbb6efc6928dca415cc47f>
> or ***@***.*** (#34)
> <https://github.com/l0rinc/bitcoin/pull/34/commits/b23c6a1fc8f9a44561add79427d2f3fff4c3718f>
> .
>
...
(https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286386567)
_:-!.
pt., 12 wrz 2025, 20:02 uΕΌytkownik l0rinc ***@***.***>
napisaΕ:
> *l0rinc* left a comment (bitcoin/bitcoin#33376)
> <https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286334324>
>
> Multiple people have pushed this already, e.g. 723c49b (#32128)
> <https://github.com/bitcoin/bitcoin/pull/32128/commits/723c49b63bb10da843fbb6efc6928dca415cc47f>
> or ***@***.*** (#34)
> <https://github.com/l0rinc/bitcoin/pull/34/commits/b23c6a1fc8f9a44561add79427d2f3fff4c3718f>
> .
>
...
π¬ Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2345091587)
Oh, I didn't see #26812 before. Do you want to open a PR with that commit? I am more than happy to review the change.
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2345091587)
Oh, I didn't see #26812 before. Do you want to open a PR with that commit? I am more than happy to review the change.
π TheCharlatan approved a pull request: "cmake: Fix regression in `secp256k1.cmake`"
(https://github.com/bitcoin/bitcoin/pull/33379#pullrequestreview-3218326877)
ACK 9193c3e4340bb5b49af2ab04bce335876e7b1076
(https://github.com/bitcoin/bitcoin/pull/33379#pullrequestreview-3218326877)
ACK 9193c3e4340bb5b49af2ab04bce335876e7b1076
π€ furszy reviewed a pull request: "cmake: Fix regression in `secp256k1.cmake`"
(https://github.com/bitcoin/bitcoin/pull/33379#pullrequestreview-3218377417)
ACK 9193c3e4340bb5b49af2ab04bce335876e7b1076
(https://github.com/bitcoin/bitcoin/pull/33379#pullrequestreview-3218377417)
ACK 9193c3e4340bb5b49af2ab04bce335876e7b1076