📝 m3dwards converted_to_draft a pull request: "ci: parse TEST_RUNNER_EXTRA into an array"
(https://github.com/bitcoin/bitcoin/pull/30244)
While working on CI I wanted to disable some functional tests so I used the `TEST_RUNNER_EXTRA` var. The problem I had was tests that have flags such as `rpc_bind.py --ipv6` must be passed in quotes otherwise the `--ipv6` portion will be considered an argument to `test_runner.py` rather than a test name.

This change allows proper parsing of quotes and complex values such as:

```shell
TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6,feature_proxy.py"'
```

💬 m3dwards commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2156093176)
Converting back to draft to see if I can fix the bug @hebasto found.

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r1622933175)
typo: instead

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r1623132784)
Why call `PruneBlockIndexCandidates` twice instead of moving it here?

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r1622932918)
typo: replace

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632134959)
If I am understanding this logic correctly, if `m_deterministic_prng.has_value()` is true then MixExtract in SeedStrengthen will never use m_counter or m_state to do MixExtract?

Strengthen is going to make the output of this call non-deterministic anyways, so what value does `allow_deterministic=true` as an argument provide in MixExtract? Shouldn't this be false?

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632132631)
As far as I can tell GetOSRand is unchanged in this PR. Why move below the `class RNGState` declaration?

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632131052)
Nit: Why not rand_exp_duration instead? It is one character shorter and `exp` reads more clearly to me as exponential than `expo`?

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632133956)
```suggestion
* If allow_deterministic is true, and MakeDeterministic has been called before, output
* from the deterministic PRNG.
```

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632130266)
Is the change from milliseconds to microseconds intentional?

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632133792)
This confused me because MixExtract is a deterministic function based on hasher, m_counter, and m_state. Unless I am mistaken the non-determinism comes from the calling function determining on the value of ret. Why not address strengthen ret value in the calling function rather than here?

💬 EthanHeilman commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632135242)
Does `allow_deterministic=false` and `m_deterministic_prng.has_value()=true` imply that something has gone wrong? Should we log an error or take some action or are there cases where this is expected behavior?

💬 BenWestgate commented on issue "Pruning keeps getting reenabled":
(https://github.com/bitcoin-core/gui/issues/709#issuecomment-2156327310)
Hi hebasto, a work around would be to put a bitcoin.conf file in `/Volumes/Drive/Bitcoin` with `prune=0` BEFORE mounting your external drive, that way if it hasn't been mounted Bitcoin Core will not try to prune. This file will be buried and ignored after your external drive mounts.

Another work around would be adding `-prune=0` to whatever method you are launching Bitcoin, be that command line or a .desktop file.

The file that remembers where your custom datadir is `$HOME/.config/Bitcoin/
...

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#discussion_r1632201184)
This is still needed. You didn't change the fuzz test runner invocation, only the functional test runner invocation.

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#discussion_r1632203401)
You will have to remove this diff hunk, or apply the same approach to the fuzz test runner, as you did to the functional test runner.

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2156377203)
> Passing `--exclude "rpc_bind.py --ipv6"` excludes not only the specified test but also `rpc_bind.py --ipv4` and `rpc_bind.py --nonloopback`.

This is the existing behavior on current master and this pull request. I don't think it was ever supported. Though, I guess this means this pull request isn't needed, then.

💬 maflcko commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1632204359)
Not sure about exposing this. This is an expert-only setting. Anyone changing it needs to understand exactly what they are doing, so they might as well modify the `ci/test/00_setup_env_native_asan.sh` config file manually.

I'd prefer to keep the `sed` approach as-is. If you want to change it, it may be better to do in a separate pull request. This way, the focus here is kept on only moving the task from one infra to the other.

💬 maflcko commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1632204875)
In theory, only the fuzz binary compilation could be excluded, because there is a separate task for the fuzz tests under asan. But either seems fine.

💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1632205551)
Looks like this imported a broken wine 9 release candidate from `debian:trixie`?

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1632207682)
https://github.com/bitcoin/bitcoin/blob/5cc2ae4ff287c6fef10b2e634e35353588f9ea3d/contrib/verify-binaries/verify.py#L511-L515