💬 Sjors commented on issue "TSAN/MSAN fails with vm.mmap_rnd_bits=32 even with llvm 18.1.3":
(https://github.com/bitcoin/bitcoin/issues/30674#issuecomment-2493384001)
When I install the ARM version of Ubuntu in VM on an M4 macOS machine using UTM, the default of `vm.mmap_rnd_bits` is 33. Though I haven't checked if the native ARM job cares.

💬 hebasto commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#discussion_r1853671315)
[Reworked](https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490932956).

💬 hebasto commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2493416095)
> > which inadvertently broke coverage builds for both [Clang's source-based code coverage](https://issues.oss-fuzz.com/issues/379122777)
>
> Can you clarify that this is only for oss-fuzz, due to its use of other options and a third party script, otherwise, source based code coverage using Clang currently still works fine with master (see: [#31337 (comment)](https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490620693)). Similarly, in the inline comment you've added, can you be a bi
...

💬 hebasto commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#discussion_r1853686757)
The comment has been added.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2493428765)
Added a commit to enable the `native_asan` and `native_valgrind` jobs, to get a more update to date log.

💬 willcl-ark commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2493431874)
I would also support slightly reducing the value in this PR, my preference though would be 32MB, for these reasons:

- The benchmark data shows the biggest gains come from the initial increases (2MB → 4MB → 8MB)
- There are diminishing returns after 8MB, with 128MB actually performing slightly worse than 64MB in total time in some of the benchmarks above
- Most of the performance gains are captured by the 32MB size, esp. when including HDDs
- Smaller files will also:
- Be more manageable
...

💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2493435481)
The false positives keep coming in, so it would be good to make progress here:

https://cirrus-ci.com/task/4830737755537408?logs=ci#L2355

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853700315)
given that checking the args is not for free (triggers locks and merges from multiple sources), maybe we could introduce a `IsArgEnabled` which gets rid of this confusing combination (where `-noconf=0` would be set and not negated, I think)

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853683566)
What's the reason for not wanting an `about` window regardless of the `-version` flag's value? Doesn't `gArgs.IsArgSet("-version")` make more sense here?

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853679146)
why is this false and not an empty optional? As mentioned before, I don't really see the line

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853676537)
as mentioned before, I find this tri-state-boolean quite confusing - if 3 states are indeed needed, maybe it's hiding a different structure such as a variant or enum or `optional<Error>`

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853672421)
will the `node0_cookie_file_path_tmp` file be kept when assertion fails on purpose? If not, you could wrap it in a try/finally (deleting regardless of the assertion's outcome)

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853701435)
Yes, I was confused by indentation, resolve the comment por favor

💬 hebasto commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2493443094)
I'm a bit hesitant about this change.

> Conceptually there are many other nightly tasks, which rarely find issues and are not run by default, like the valgrind or s390x tasks. So putting the Wine unit tests in the same bucket should be fine.

I disagree. Conceptually, this is a different case because the cross-compiled `test_bitcoin.exe` is part of release bundle for Windows. And leaving the repo without a CI job to run it seems like a drawback.

> A follow-up could run them on real Windo
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2493443688)
Updated a9b71eadb8eff5530500cdb7d7227b8575948df6 -> fc67047b7e1fb7031285f790ea3a7ea349474f31 ([kernelApi_3](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_3) -> [kernelApi_4](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_3..kernelApi_4))

* Made the `user_data` argument passed in with the callbacks `const` to better convey that the library doesn't do anything with it besides passing it back to the user whe
...

👍 rkrux approved a pull request: "test: Rework wallet_migration.py to use previous releases"
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2454103454)
reACK 55347a5

Reviewed the range diff - only minor comment, commit message, renaming changes. Ran `cmake` again.

```
git range-diff a76ad56...55347a5
```

⚠️ Sjors opened an issue: "ci: how to run native arm job on Apple silicon?"
(https://github.com/bitcoin/bitcoin/issues/31344)
I'm trying to run the `arm64` worker job (ARM, unit tests, no functional tests) on my M4 mac, because that should be lot faster than emulating it in Qemu on my x86_64 machine.

Unfortunately so far it fails:

```
[10:34:36.739] Test project /ci_container_base/ci/scratch/build-arm-linux-gnueabihf
[10:34:36.742] Start 2: util_rpcauth_test
[10:34:36.744] Start 4: univalue_object_test
[10:34:36.747] Start 6: secp256k1_tests
[10:34:36.751] Start 8: tes
...

💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2493503739)
> the cross-compiled `test_bitcoin.exe` is part of release bundle for Windows. And leaving the repo without a CI job to run it seems like a drawback.

How is this different from the whole macos-cross release, or even the real binary `bitcoind.exe`, which is never tested? Moreover, if you think that running the windows unit tests on Wine is useful, it would be good to give at least one example where it caught an issue that was found by none of the other tasks. (For example, they can't even dete
...

👍 laanwj approved a pull request: "wallet: Remove IsMine from migration code"
(https://github.com/bitcoin/bitcoin/pull/30328#pullrequestreview-2451750295)
Must admit that i'm not versed enough in the subtle specifics with regard to validity and spendability of scripts, to be sure all of this is correct. But where i've checked, the behavior matches the IsMine-based implementation. Overall changes LGTM.

💬 laanwj commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1852300265)
comment: -there