🤔 monlovesmango reviewed a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-3006458828)
ACK d7fca5c171f450621112457ea6f7c99b2e21d354
The separation of concerns in the cluster linearization fuzz tests makes sense, namely separating testing of production of code from testing of test code. The small added efficiencies and comparisons are nice and the diagram documenting all the relationships is extremely helpful in clarifying how all the parts tie together.
Unrelated to this specific PR, but in the `clusterlin_postlinearize_tree` fuzz target, `post_post_linearization` isn't actu
...
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-3006458828)
ACK d7fca5c171f450621112457ea6f7c99b2e21d354
The separation of concerns in the cluster linearization fuzz tests makes sense, namely separating testing of production of code from testing of test code. The small added efficiencies and comparisons are nice and the diagram documenting all the relationships is extremely helpful in clarifying how all the parts tie together.
Unrelated to this specific PR, but in the `clusterlin_postlinearize_tree` fuzz target, `post_post_linearization` isn't actu
...
💬 monlovesmango commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2198250370)
I think this comment can be resolved because `clusterlin_simple_linearize` now has a check for `simple_chunking` against `read_chunking` in optimal scenario?
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2198250370)
I think this comment can be resolved because `clusterlin_simple_linearize` now has a check for `simple_chunking` against `read_chunking` in optimal scenario?
💬 monlovesmango commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2198192109)
Should `SearchCandidateFinder` also point to `AncestorCandidateFinder` (with single arrow ->) since it is also used in `clusterlin_search_finder` test? Similar to how `SimpleCandidateFinder` implementation is tested with both `ExhaustiveCandidateFinder` and `AncestorCandidateFinder` in `clusterlin_simple_finder`?
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r2198192109)
Should `SearchCandidateFinder` also point to `AncestorCandidateFinder` (with single arrow ->) since it is also used in `clusterlin_search_finder` test? Similar to how `SimpleCandidateFinder` implementation is tested with both `ExhaustiveCandidateFinder` and `AncestorCandidateFinder` in `clusterlin_simple_finder`?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198327882)
I still think we should be LimitOphan-ing for each AddAnnouncer?
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198327882)
I still think we should be LimitOphan-ing for each AddAnnouncer?
💬 maflcko commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3058387306)
> Likely the stdin is missing in this particular run and caused `test/functional/mocks/signer.py` to hang.
What does "stdin is missing" mean? It should always be present and point to some file descriptor.
I think the only way this could hang is when the process spawning the signer is also waiting on it, without first closing the pipes. However this seems to happen correctly, looking at `RunCommandParseJSON`:
```cpp
auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE},
...
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3058387306)
> Likely the stdin is missing in this particular run and caused `test/functional/mocks/signer.py` to hang.
What does "stdin is missing" mean? It should always be present and point to some file descriptor.
I think the only way this could hang is when the process spawning the signer is also waiting on it, without first closing the pipes. However this seems to happen correctly, looking at `RunCommandParseJSON`:
```cpp
auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE},
...
💬 andrewtoth commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r2198342677)
I removed `SetFresh` in the last commit. This lets us remove these tests.
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r2198342677)
I removed `SetFresh` in the last commit. This lets us remove these tests.
🚀 glozow merged a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605)
(https://github.com/bitcoin/bitcoin/pull/30605)
💬 maflcko commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3058393000)
Maybe this is https://github.com/bitcoin/bitcoin/issues/32524, but with N=1 and no way to reproduce, this seems difficult to debug.
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3058393000)
Maybe this is https://github.com/bitcoin/bitcoin/issues/32524, but with N=1 and no way to reproduce, this seems difficult to debug.
💬 johnnyasantoss commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-3058443028)
> what version were you running?
We upgraded since then, but if I recall correctly it was version `26` (upgraded because of p2pv2)
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-3058443028)
> what version were you running?
We upgraded since then, but if I recall correctly it was version `26` (upgraded because of p2pv2)
💬 murchandamus commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#issuecomment-3058605059)
Thanks @maflcko, I fixed the sentence.
(https://github.com/bitcoin/bitcoin/pull/32894#issuecomment-3058605059)
Thanks @maflcko, I fixed the sentence.
📝 hebasto opened a pull request: "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`"
(https://github.com/bitcoin/bitcoin/pull/32943)
When using CMake policies 3.14 and below, the `export(PACKAGE)` command by default populates the user package registry, which is stored outside the build tree. Setting the `CMAKE_EXPORT_NO_PACKAGE_REGISTRY` variable disables this side effect.
In CMake 3.15 and later, this behavior is disabled by default, and the variable has no effect.
This PR forces `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE` globally, rather than managing it for each dependency package individually rather. It may be reverted
...
(https://github.com/bitcoin/bitcoin/pull/32943)
When using CMake policies 3.14 and below, the `export(PACKAGE)` command by default populates the user package registry, which is stored outside the build tree. Setting the `CMAKE_EXPORT_NO_PACKAGE_REGISTRY` variable disables this side effect.
In CMake 3.15 and later, this behavior is disabled by default, and the variable has no effect.
This PR forces `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE` globally, rather than managing it for each dependency package individually rather. It may be reverted
...
🤔 instagibbs reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3006927847)
reviewed through 76e65b8d2982764c9f47be925c26bef23ad0ab97
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3006927847)
reviewed through 76e65b8d2982764c9f47be925c26bef23ad0ab97
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198516588)
fits in a block, just not very typical? How was this value chosen?
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198516588)
fits in a block, just not very typical? How was this value chosen?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198479997)
Maybe leave a comment that discrepencies between the simulation and real Limiting will be revealed below in full comparison
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198479997)
Maybe leave a comment that discrepencies between the simulation and real Limiting will be revealed below in full comparison
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198505837)
```Suggestion
// Constructs a transaction using a subset of inputs[start_input : start_input + num_inputs] up to the weight_limit.
```
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198505837)
```Suggestion
// Constructs a transaction using a subset of inputs[start_input : start_input + num_inputs] up to the weight_limit.
```
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198465090)
ones not present?
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198465090)
ones not present?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198517643)
```Suggestion
// Transactions with 9 inputs maximize the computation / LatencyScore ratio.
```
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198517643)
```Suggestion
// Transactions with 9 inputs maximize the computation / LatencyScore ratio.
```
💬 achow101 commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3058896316)
x86_64 guix build of f43571010e38
```
$ find guix-build-f43571010e38/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
bb9e6435df82cfe01c78e046ef39cea62a3b0ad0deafa66e1d2a15257bc21eeb guix-build-f43571010e38/output/dist-archive/bitcoin-f43571010e38.tar.gz
7d3bc9459a51abcb2d9d36b69fc135cbb4940c092a9f8df63032c6e9f659a958 guix-build-f43571010e38/output/x86_64-w64-mingw32/SHA256SUMS.part
34af63c893a315745364d1f4dd36bbc1521eef0cad42ea964ba1d0dc5cbcf738 guix-build-f4357101
...
(https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3058896316)
x86_64 guix build of f43571010e38
```
$ find guix-build-f43571010e38/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
bb9e6435df82cfe01c78e046ef39cea62a3b0ad0deafa66e1d2a15257bc21eeb guix-build-f43571010e38/output/dist-archive/bitcoin-f43571010e38.tar.gz
7d3bc9459a51abcb2d9d36b69fc135cbb4940c092a9f8df63032c6e9f659a958 guix-build-f43571010e38/output/x86_64-w64-mingw32/SHA256SUMS.part
34af63c893a315745364d1f4dd36bbc1521eef0cad42ea964ba1d0dc5cbcf738 guix-build-f4357101
...
💬 l0rinc commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2198649602)
do we need to create a temp copy or would this also work?
```suggestion
vecSend.emplace_back(DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount);
```
(https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2198649602)
do we need to create a temp copy or would this also work?
```suggestion
vecSend.emplace_back(DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount);
```
💬 achow101 commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2198651313)
https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3056019241 and https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2196837764
(https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2198651313)
https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3056019241 and https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2196837764