Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1958228951)
Done
💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1958229143)
Done
💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1958232618)
I just removed the `ALL` option. I don't see anything that needs all of them combined.
🤔 BrandonOdiwuor reviewed a pull request: "cmake: Add `libbitcoinkernel` target"
(https://github.com/bitcoin/bitcoin/pull/31869#pullrequestreview-2621071470)
Tested ACK 3a914ab96bdeb70cdf848dd74deda5a80d0a7f78
💬 maflcko commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1958240211)
```suggestion
FuzzCoinSelectionAlgorithm<CoinSelectionAlgorithm::KNAPSACK>(buffer);
```

nit: it is already a span,no?
💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1958248096)
yes, done.
🤔 marcofleon reviewed a pull request: "contrib: Add deterministic-fuzz-coverage"
(https://github.com/bitcoin/bitcoin/pull/31836#pullrequestreview-2621090220)
Tested ACK fa3e409c9a084112fc2644a2bba9aa196bdb229d
💬 hodlinator commented on pull request "qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/31874#discussion_r1958267479)
Thanks, added type annotations to `assert_true()` in latest push. `mypy --check-untyped-defs` catches type mismatches for it, but not for built-in `assert`. It also triggers a deluge of other errors though.

The point of #23119 is for assertion failures to output the values that made them fail. While it also entails replacing a subset of instances of built-in `assert` with our own, the purpose is different.
💬 l0rinc commented on pull request "qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/31874#discussion_r1958276728)
It's also similar to https://github.com/bitcoin/bitcoin/pull/29500 from a year ago.
Btw, [type hints](https://github.com/bitcoin/bitcoin/compare/84485368c4234596519aace2b851402e3b53e624..790c509a4796ec47be2275d86b35370ff25a599a#diff-e20e1f68486e5c096fdc11bca1cda063aacef524411f011d9c3497eb650c5eafR73) in Python are just hints, adding `assert_true(1)` will still pass the test.

@hodlinator, could we separate fixing dead code from introducing another assertion?
👍 brunoerg approved a pull request: "contrib: Add deterministic-fuzz-coverage"
(https://github.com/bitcoin/bitcoin/pull/31836#pullrequestreview-2621138925)
tested ACK fa3e409c9a084112fc2644a2bba9aa196bdb229d

```sh
...
--- /Users/brunogarcia/projects/bitcoin-core-dev/build_fuzz/fuzz_det_cov.show.0.txt 2025-02-17 10:47:20
+++ /Users/brunogarcia/projects/bitcoin-core-dev/build_fuzz/fuzz_det_cov.show.1.txt 2025-02-17 10:47:20
@@ -19737,7 +19737,7 @@
24| 4.48k|# define bitcoin_builtin_bswap32(x) __builtin_bswap32(x)
25| |# endif
26| |# if __has_builtin(__builtin_bswap64)
- 27| 8.37M|# de
...
💬 brunoerg commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#discussion_r1958279670)
nit: maybe could mention `fuzz_corpora` directly
```suggestion
RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_dir $PWD/qa-assets/fuzz_corpora fuzz_target_name
```
💬 brunoerg commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2663217465)
Also, worths adding a .gitignore for the tool (especially for the `target` folder)?
⚠️ maflcko opened an issue: "ci: Intermittent failure "Could not resolve host: github.com""
(https://github.com/bitcoin/bitcoin/issues/31889)
Looks like this happens for some workers/users on the same machine. For example, it happens for `ci_worker_1738693519_026122670`, but none of the others. Also, it seems to *almost* persist for several hours. That is, it happens on almost all runs, once it happened at least once.

Example failures:

* https://cirrus-ci.com/task/6482591089426432?logs=ci#L603
* https://cirrus-ci.com/task/6102859474796544?logs=ci#L624
* https://cirrus-ci.com/task/6278234062454784?logs=ci#L614

However, the same work
...
💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2663218525)
> > [96d3b2a](https://github.com/bitcoin/bitcoin/commit/96d3b2a0bb0c267569162b055ca306f709ec0b4e) "cmake: Fix fuzzer "multiple definition of `main'" errors"
>
> It may be good to explain this commit, or add steps to reproduce.

@maflcko guessing you maybe did not notice the comments or commit message? All the points you raised should be covered there, but let me know if anything should be updated or clarified.

Steps to reproduce are to check out the branch, revert the commit, and try to
...
maflcko closed an issue: "ci: Intermittent failure "Could not resolve host: github.com""
(https://github.com/bitcoin/bitcoin/issues/31889)
💬 maflcko commented on issue "ci: Intermittent failure "Could not resolve host: github.com"":
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2663219347)
Closing for now, because it seems out-of-scope for this repo. (Not sure where to file otherwise, so feel free to continue discussion here for now)
🤔 sipa reviewed a pull request: "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826#pullrequestreview-2621167189)
utACK 09b150bb8adae00854f02ece69fc6ef222fb07d9
💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#discussion_r1958297299)
thanks, will do in the follow-up
💬 hodlinator commented on pull request "qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/31874#discussion_r1958297307)
I'd say #29500 is more related to #23119 than this PR.
> Btw, [type hints](https://github.com/bitcoin/bitcoin/compare/84485368c4234596519aace2b851402e3b53e624..790c509a4796ec47be2275d86b35370ff25a599a#diff-e20e1f68486e5c096fdc11bca1cda063aacef524411f011d9c3497eb650c5eafR73) in Python are just hints, adding assert_true(1) will still pass the test.

Yes, that's why I commented about the linter situation above.

> @hodlinator, could we separate fixing dead code from introducing another asserti
...