💬 maflcko commented on pull request "ci: Split out native fuzz jobs for macOS and windows":
(https://github.com/bitcoin/bitcoin/pull/31073#issuecomment-2441328135)
Can this be closed?
(https://github.com/bitcoin/bitcoin/pull/31073#issuecomment-2441328135)
Can this be closed?
✅ maflcko closed an issue: "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`"
(https://github.com/bitcoin/bitcoin/issues/31057)
(https://github.com/bitcoin/bitcoin/issues/31057)
✅ carnhofdaki closed an issue: "Stop at header"
(https://github.com/bitcoin/bitcoin/issues/31162)
(https://github.com/bitcoin/bitcoin/issues/31162)
💬 carnhofdaki commented on issue "Stop at header":
(https://github.com/bitcoin/bitcoin/issues/31162#issuecomment-2441340067)
Thank you @maflcko ! The `minimumchainwork=0` does exactly what I was looking for.
Closing as solved.
(https://github.com/bitcoin/bitcoin/issues/31162#issuecomment-2441340067)
Thank you @maflcko ! The `minimumchainwork=0` does exactly what I was looking for.
Closing as solved.
💬 l0rinc commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818907871)
it's in `bech32` and is called `CHECKSUM_SIZE` - what does the comment add to that?
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818907871)
it's in `bech32` and is called `CHECKSUM_SIZE` - what does the comment add to that?
💬 brunoerg commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818911326)
Maybe that this value works for both bech32 and bech32m?
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818911326)
Maybe that this value works for both bech32 and bech32m?
💬 l0rinc commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#issuecomment-2441347505)
> I don't see anything new in these reports compared to [maflcko.github.io/b-c-cov/fuzz.coverage/src/index.html](https://maflcko.github.io/b-c-cov/fuzz.coverage/src/index.html).
Thanks for checking!
The human readable part is variable now since in Silent Payments it's not just `bc` anymore, but the covered lines should be the same (it's not a perfect metric).
(https://github.com/bitcoin/bitcoin/pull/30623#issuecomment-2441347505)
> I don't see anything new in these reports compared to [maflcko.github.io/b-c-cov/fuzz.coverage/src/index.html](https://maflcko.github.io/b-c-cov/fuzz.coverage/src/index.html).
Thanks for checking!
The human readable part is variable now since in Silent Payments it's not just `bc` anymore, but the covered lines should be the same (it's not a perfect metric).
💬 l0rinc commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818914169)
The exceptional case is when they're different (see https://github.com/bitcoin/bitcoin/pull/30623/files#diff-9607135f3b89a188b92ae678f50685a3f65f4e8ef7832e653cc344ec623100dfR39), we assume otherwise that these are common.
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1818914169)
The exceptional case is when they're different (see https://github.com/bitcoin/bitcoin/pull/30623/files#diff-9607135f3b89a188b92ae678f50685a3f65f4e8ef7832e653cc344ec623100dfR39), we assume otherwise that these are common.
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2441355825)
re-ACK 303661871debadee5f67bd7e4cd0cccc85344ef2, thanks for adding the test.
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2441355825)
re-ACK 303661871debadee5f67bd7e4cd0cccc85344ef2, thanks for adding the test.
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818920353)
> Would avoid stripping const for these slightly longer lived variables, just for peace of mind.
Done, thanks.
> Not super happy about the memcmp though
agree, `memcmp` is ugly, but `std::ranges::equal` should work here.
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818920353)
> Would avoid stripping const for these slightly longer lived variables, just for peace of mind.
Done, thanks.
> Not super happy about the memcmp though
agree, `memcmp` is ugly, but `std::ranges::equal` should work here.
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818920957)
Perfection! ✨👌✨
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818920957)
Perfection! ✨👌✨
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818921051)
astute observation ;)
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818921051)
astute observation ;)
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818921173)
Good idea, this will simplify the string generation as well since it requires a `FuzzedDataProvider` anyway
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818921173)
Good idea, this will simplify the string generation as well since it requires a `FuzzedDataProvider` anyway
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818921361)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818921361)
Done, thanks
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818923703)
Is this a blocker for anyone? If anyone has strong feelings about this, please let me know, we can either restructure here or provide it in another PR. Resolving for now.
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1818923703)
Is this a blocker for anyone? If anyone has strong feelings about this, please let me know, we can either restructure here or provide it in another PR. Resolving for now.
💬 maflcko commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2441368688)
I switched https://maflcko.github.io/b-c-cov/ to GCC, just to get something working again. But the config is open source and pull requests with changes are welcome: https://github.com/maflcko/b-c-cov
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2441368688)
I switched https://maflcko.github.io/b-c-cov/ to GCC, just to get something working again. But the config is open source and pull requests with changes are welcome: https://github.com/maflcko/b-c-cov
👍 brunoerg approved a pull request: "test: Fuzz the human-readable part of bech32 as well"
(https://github.com/bitcoin/bitcoin/pull/30623#pullrequestreview-2398850907)
code review ACK 9b7023d31a3ec95f66b45f0ecb47e79762d74854
(https://github.com/bitcoin/bitcoin/pull/30623#pullrequestreview-2398850907)
code review ACK 9b7023d31a3ec95f66b45f0ecb47e79762d74854
✅ dergoegge closed a pull request: "ci: Split out native fuzz jobs for macOS and windows"
(https://github.com/bitcoin/bitcoin/pull/31073)
(https://github.com/bitcoin/bitcoin/pull/31073)
💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows":
(https://github.com/bitcoin/bitcoin/pull/31073#issuecomment-2441373851)
I still think this would make sense but there seems to be no interest. Also #30950 has been resolved.
(https://github.com/bitcoin/bitcoin/pull/31073#issuecomment-2441373851)
I still think this would make sense but there seems to be no interest. Also #30950 has been resolved.
💬 fanquake commented on pull request "depends: Specify CMake generator explicitly":
(https://github.com/bitcoin/bitcoin/pull/31171#issuecomment-2441405718)
> However, this assumption can be wrong in environments where the [CMAKE_GENERATOR](https://cmake.org/cmake/help/latest/envvar/CMAKE_GENERATOR.html) variable is set.
Generally, this holds for many CMake variables. Wouldn't a better approach be facilitating it working, rather than guaranteeing these environments never get the expected bahaviour.
(https://github.com/bitcoin/bitcoin/pull/31171#issuecomment-2441405718)
> However, this assumption can be wrong in environments where the [CMAKE_GENERATOR](https://cmake.org/cmake/help/latest/envvar/CMAKE_GENERATOR.html) variable is set.
Generally, this holds for many CMake variables. Wouldn't a better approach be facilitating it working, rather than guaranteeing these environments never get the expected bahaviour.