💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1958078713)
There is no need to remove it, will put back. Probably missed it during the rework. Thanks.

💬 laanwj commented on pull request "correct wrong assumptions in the contrib linearize data script":
(https://github.com/bitcoin/bitcoin/pull/31888#issuecomment-2662871105)
Lint is failing on the formatting of the commit message:
```
[10:12:47.126] The subject line of commit hash 0cf4a6245d2bc8d57f91dd2b4f5bd5a0eed18ed4 is followed by a non-empty line. Subject lines should always be followed by a blank line.
[10:12:47.126] ^^^
[10:12:47.126]
[10:12:47.126] ^---- ⚠️ Failure generated from lint check 'commit_msg' (Check that commit messages have a new line before the body or no body at all.)!
```

It also looks like the changes somehow break the `feature_loa
...

💬 dergoegge commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2662894878)
Concept ACK

💬 hebasto commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#discussion_r1958113978)
> shouldn't this be an error now?

Why? It precisely mirrors the scenario “I don't have python and I don't care about the tests” and simply serves as a reminder to the user of the consequences:

```
$ cmake -B build -DWITH_PYTHON=OFF
...
Use ccache for compiling .............. ON


******

CMake Warning at CMakeLists.txt:680 (message):
Utils and rpcauth tests are disabled.

Use "-DWITH_PYTHON=ON" to enable them.


******

-- Configuring done (12.8s)
-- Generating don
...

💬 maflcko commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#discussion_r1958149211)
I see. I guess I wanted to say the opposite: Can't this be removed, because it seems a bit odd to print a warning that something is disabled, when the users explicitly asked it to be disabled? There is also no warning when a user sets `BUILD_TESTS=OFF`.

Another alternative would be to just require python3, without a way to disable it. Thus, the option `WITH_PYTHON` can be removed.

I am just wondering: Was there ever a single user (compiling from source) to ask for the python dependency to
...

✅ hebasto closed a pull request: "cmake: Introduce `WITH_PYTHON` build option"
(https://github.com/bitcoin/bitcoin/pull/31669)

💬 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.

💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2663128872)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1957893638, https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1957883443 and https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1957883107. Thanks, @maflcko.

🤔 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
...