💬 fanquake commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2842608674)
> To clarify, now it is a configure failure, as opposed to a build (make) failure?
It's still a build failure, with no way to override the offending flag, as the APPEND options don't currently work properly.
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2842608674)
> To clarify, now it is a configure failure, as opposed to a build (make) failure?
It's still a build failure, with no way to override the offending flag, as the APPEND options don't currently work properly.
💬 hebasto commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2842616077)
> > To clarify, now it is a configure failure, as opposed to a build (make) failure?
>
> It's still a build failure, with no way to override the offending flag, as the APPEND options don't currently work properly.
Could someone provide a configuration options and a line from the verbose build to show what exactly is broken?
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2842616077)
> > To clarify, now it is a configure failure, as opposed to a build (make) failure?
>
> It's still a build failure, with no way to override the offending flag, as the APPEND options don't currently work properly.
Could someone provide a configuration options and a line from the verbose build to show what exactly is broken?
💬 davidgumberg commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2069106692)
```suggestion
CCoinsViewCache view_dummy(&chainstate.CoinsTip());
```
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2069106692)
```suggestion
CCoinsViewCache view_dummy(&chainstate.CoinsTip());
```
💬 maflcko commented on issue "ctest -C Debug fails on vs2022 (miniscript_tests (SEGFAULT))":
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2842716175)
> A third question would be, why the fuzz output is https://github.com/hebasto/bitcoin/actions/runs/14650416249/job/41114604643#step:14:60: `subprocess timed out: Currently only libFuzzer is supported`. A timeout should not happen for this process call, and the expected outcome should be an immediate stderr of `test/fuzz/fuzz.cpp:269 main: Assertion 'read_file(input_path, buffer)' failed. Error processing input "-help=1"` and an ignored exit code.
Looks like the assertions fails, but the progra
...
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2842716175)
> A third question would be, why the fuzz output is https://github.com/hebasto/bitcoin/actions/runs/14650416249/job/41114604643#step:14:60: `subprocess timed out: Currently only libFuzzer is supported`. A timeout should not happen for this process call, and the expected outcome should be an immediate stderr of `test/fuzz/fuzz.cpp:269 main: Assertion 'read_file(input_path, buffer)' failed. Error processing input "-help=1"` and an ignored exit code.
Looks like the assertions fails, but the progra
...
💬 maflcko commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2842751761)
I checked the original non-afl asan dockerfile and the existing workaround should still apply there (Don't use `-O0`, or disable secp256k1 asm).
Thus, my guess is the new issue is about afl only and the new dockerfile is https://github.com/dergoegge/fuzzamoto/blob/master/Dockerfile?
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2842751761)
I checked the original non-afl asan dockerfile and the existing workaround should still apply there (Don't use `-O0`, or disable secp256k1 asm).
Thus, my guess is the new issue is about afl only and the new dockerfile is https://github.com/dergoegge/fuzzamoto/blob/master/Dockerfile?
💬 achow101 commented on pull request "util: Remove `fsbridge::get_filesystem_error_message()`":
(https://github.com/bitcoin/bitcoin/pull/32383#issuecomment-2842817151)
ACK 97eaadc3bf9f621ba397e29bb1c0cd99af55f2e3
(https://github.com/bitcoin/bitcoin/pull/32383#issuecomment-2842817151)
ACK 97eaadc3bf9f621ba397e29bb1c0cd99af55f2e3
🚀 achow101 merged a pull request: "util: Remove `fsbridge::get_filesystem_error_message()`"
(https://github.com/bitcoin/bitcoin/pull/32383)
(https://github.com/bitcoin/bitcoin/pull/32383)
📝 maflcko opened a pull request: "fuzz: Remove unused TimeoutExpired catch in fuzz runner"
(https://github.com/bitcoin/bitcoin/pull/32388)
Currently, the way to check for libFuzzer is to search the stderr of the fuzz executable when passed `-help=1` for the string `libFuzzer`. See also https://github.com/bitcoin/bitcoin/blob/14b8dfb2bd5e2ca2b7c0c9a7f7d50e1e60adf75c/contrib/devtools/deterministic-fuzz-coverage/src/main.rs#L90-L101
The python test runner additionally includes a timeout catch, which was needed before the plain `read_file` fallback was implemented, see https://github.com/bitcoin/bitcoin/blob/14b8dfb2bd5e2ca2b7c0c9a7
...
(https://github.com/bitcoin/bitcoin/pull/32388)
Currently, the way to check for libFuzzer is to search the stderr of the fuzz executable when passed `-help=1` for the string `libFuzzer`. See also https://github.com/bitcoin/bitcoin/blob/14b8dfb2bd5e2ca2b7c0c9a7f7d50e1e60adf75c/contrib/devtools/deterministic-fuzz-coverage/src/main.rs#L90-L101
The python test runner additionally includes a timeout catch, which was needed before the plain `read_file` fallback was implemented, see https://github.com/bitcoin/bitcoin/blob/14b8dfb2bd5e2ca2b7c0c9a7
...
✅ maflcko closed a pull request: "ci: Merge fuzz task for macOS"
(https://github.com/bitcoin/bitcoin/pull/32339)
(https://github.com/bitcoin/bitcoin/pull/32339)
💬 maflcko commented on pull request "ci: Merge fuzz task for macOS":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2842856892)
Closing for now, but happy to re-open if there is any interest.
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2842856892)
Closing for now, but happy to re-open if there is any interest.
💬 stringintech commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2069214844)
Since `TestBlockValidity` is typically called with a fresh block instance, I'm not seeing how the race condition with `fChecked` could occur. Could you elaborate on this please?
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2069214844)
Since `TestBlockValidity` is typically called with a fresh block instance, I'm not seeing how the race condition with `fChecked` could occur. Could you elaborate on this please?
💬 achow101 commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r2069273906)
In 7998eda03402a8dd8227a506c022f8bef81f38ef "validation: write chainstate to disk every hour"
I think it makes more sense to update the timer after the chainstate write, in the same place that `m_last_flush` was. Even after the change in semantics in the later commits.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r2069273906)
In 7998eda03402a8dd8227a506c022f8bef81f38ef "validation: write chainstate to disk every hour"
I think it makes more sense to update the timer after the chainstate write, in the same place that `m_last_flush` was. Even after the change in semantics in the later commits.
✅ darosior closed a pull request: "policy: allow more than one OP_RETURN outputs per tx"
(https://github.com/bitcoin/bitcoin/pull/32381)
(https://github.com/bitcoin/bitcoin/pull/32381)
💬 darosior commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2843023281)
My intention with this PR was to have a first step that is consensual among regular contributors, seeing some expressed doubts with https://github.com/bitcoin/bitcoin/pull/32359 (i counted @danielabrozzoni @polespinasa and @andrewtoth but i may have missed some in the middle of all the spam).
Seeing that it is not, i don't think there is any use in keeping this PR open. We should address the concerns of those contributors and do https://github.com/bitcoin/bitcoin/pull/32359 instead.
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2843023281)
My intention with this PR was to have a first step that is consensual among regular contributors, seeing some expressed doubts with https://github.com/bitcoin/bitcoin/pull/32359 (i counted @danielabrozzoni @polespinasa and @andrewtoth but i may have missed some in the middle of all the spam).
Seeing that it is not, i don't think there is any use in keeping this PR open. We should address the concerns of those contributors and do https://github.com/bitcoin/bitcoin/pull/32359 instead.
💬 Sjors commented on pull request "[DRAFT] ipc: add windows support":
(https://github.com/bitcoin/bitcoin/pull/32387#issuecomment-2843024844)
I guess the easiest way to test this would be to combine it with:
1. #31756 so I can do a guix build instead of learning to compile on Windows; and
2. #30437 to have something to test against
(https://github.com/bitcoin/bitcoin/pull/32387#issuecomment-2843024844)
I guess the easiest way to test this would be to combine it with:
1. #31756 so I can do a guix build instead of learning to compile on Windows; and
2. #30437 to have something to test against
💬 Sjors commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2843043721)
I already [suspected](https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841239327) that even a variant of #32359 that retains the `-datacarrier` configuration option would meet just as much resistance. This PR does far less than that. So the response here confirms, at least to me, that there's no point in having _any_ reduced scope alternative.
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2843043721)
I already [suspected](https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841239327) that even a variant of #32359 that retains the `-datacarrier` configuration option would meet just as much resistance. This PR does far less than that. So the response here confirms, at least to me, that there's no point in having _any_ reduced scope alternative.
💬 Sjors commented on pull request "Catch harmful arbitrary data. (missing code for #32359)":
(https://github.com/bitcoin/bitcoin/pull/32368#issuecomment-2843050092)
NACK
This is a duplicate of #29520 by the same author.
(https://github.com/bitcoin/bitcoin/pull/32368#issuecomment-2843050092)
NACK
This is a duplicate of #29520 by the same author.
💬 kevkevinpal commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2843149921)
Updated to [7ca9b5e](https://github.com/bitcoin/bitcoin/pull/32354/commits/7ca9b5eb6f1f375d25a2e5731ec5a2279db46e5e)
This is now what it looks like, it will print
`<targets_finished>/<total_targets>) - <target_name>`
then when there are less than 10 fuzz targets it will print the name of the remaining targets
Example
```
./build_fuzz/test/fuzz/test_runner.py ../qa-assets/fuzz_corpora asmap_direct base64_encode_decode addition_overflow asmap txgraph base58_encode_decode autofile bech
...
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2843149921)
Updated to [7ca9b5e](https://github.com/bitcoin/bitcoin/pull/32354/commits/7ca9b5eb6f1f375d25a2e5731ec5a2279db46e5e)
This is now what it looks like, it will print
`<targets_finished>/<total_targets>) - <target_name>`
then when there are less than 10 fuzz targets it will print the name of the remaining targets
Example
```
./build_fuzz/test/fuzz/test_runner.py ../qa-assets/fuzz_corpora asmap_direct base64_encode_decode addition_overflow asmap txgraph base58_encode_decode autofile bech
...
💬 achow101 commented on pull request "mining: rename gbt_force and gbt_force_name":
(https://github.com/bitcoin/bitcoin/pull/32386#issuecomment-2843168779)
ACK 0750249289c092fc8e2e29669fec73a58b873767
This was a little bit confusing to me when reviewing #29039
(https://github.com/bitcoin/bitcoin/pull/32386#issuecomment-2843168779)
ACK 0750249289c092fc8e2e29669fec73a58b873767
This was a little bit confusing to me when reviewing #29039
💬 kevkevinpal commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2843234359)
Pushed [9d08d71](https://github.com/bitcoin/bitcoin/pull/32354/commits/9d08d7140fc91f60d6d09497633b207a18aa5d92)
It adds the duration it took to run the fuzz target
Example
```
1/219 - addition_overflow, Duration: 0.44 s
2/219 - address_deserialize, Duration: 0.56 s
3/219 - addr_info_deserialize, Duration: 0.72 s
4/219 - asmap, Duration: 0.36 s
5/219 - asmap_direct, Duration: 1.41 s
6/219 - autofile, Duration: 1.41 s
7/219 - base32_encode_decode, Duration: 0.28 s
8/219 - base58_en
...
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2843234359)
Pushed [9d08d71](https://github.com/bitcoin/bitcoin/pull/32354/commits/9d08d7140fc91f60d6d09497633b207a18aa5d92)
It adds the duration it took to run the fuzz target
Example
```
1/219 - addition_overflow, Duration: 0.44 s
2/219 - address_deserialize, Duration: 0.56 s
3/219 - addr_info_deserialize, Duration: 0.72 s
4/219 - asmap, Duration: 0.36 s
5/219 - asmap_direct, Duration: 1.41 s
6/219 - autofile, Duration: 1.41 s
7/219 - base32_encode_decode, Duration: 0.28 s
8/219 - base58_en
...
💬 rebroad commented on pull request "Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence":
(https://github.com/bitcoin-core/gui/pull/866#discussion_r2069423549)
> The GUI should not be changing low-level internals.
>
> If for some reason the clientModel is changed, its data should be taken as authoritative, even if it contradicts the history so far (if that's an issue, wipe the history).
>
> Alternatively, maybe the history should be stored with the node, and loaded into the GUI (or RPC, in another hypothetical PR).
@luke-jr I've removed the setTotalBytes functions as I agree with your reasoning. The GUI keeps a local track of the totals from t
...
(https://github.com/bitcoin-core/gui/pull/866#discussion_r2069423549)
> The GUI should not be changing low-level internals.
>
> If for some reason the clientModel is changed, its data should be taken as authoritative, even if it contradicts the history so far (if that's an issue, wipe the history).
>
> Alternatively, maybe the history should be stored with the node, and loaded into the GUI (or RPC, in another hypothetical PR).
@luke-jr I've removed the setTotalBytes functions as I agree with your reasoning. The GUI keeps a local track of the totals from t
...