💬 fjahr commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1632036240)
nit: I guess this is needed so the node can be stopped fast enough. It's still a race and could turn out to be flakey in the CI, right? I don't have a better idea to fix this right now but a comment might be good to make this explicit and make future debugging easier if this turns out to be the case.

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1632042991)
I just picked the one which furszy picked and I'm guessing he just picked one too. I'll check if there is any signifcant effect when picking a different one.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632043452)
Seems I did not. Done now.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1632043992)
It was added in #26153, with a commit originally taken from #25325. I assume it originally had an explicit `copy()` member function, which was removed before merge. I've added a commit to remove it.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2156037857)
I've rebased on top of the now-merged #30161, and believe I've addressed outstanding comments.

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