💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719367069)
> It's because part of my goal with this PR was to etch these bytes into the runtime binary. Things that can be done at compile time without too much hassle should be done at compile time IMO.
>
> If we go the `Vec(HexLiteral` route at least it becomes shorter.

Makes sense, thanks for looking into it!

💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#discussion_r1719382540)
Sure, fixed the style

💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#discussion_r1719384643)
Sure, removed the ranges include.

(Maybe another reason to enforce iwyu in the CI)

💬 Sjors commented on pull request "Move maximum timewarp attack threshold back to 600s from 7200s":
(https://github.com/bitcoin/bitcoin/pull/30647#issuecomment-2292976404)
@maflcko for this test I set `enforce_BIP94 = true` for regtest. I just updated the branch to set `nPowTargetTimespan` to 144.

@TheBlueMatt feel free to take all or some of that code. Otherwise I'll open a followup.

💬 maflcko commented on pull request "Move maximum timewarp attack threshold back to 600s from 7200s":
(https://github.com/bitcoin/bitcoin/pull/30647#issuecomment-2293002597)
> `enforce_BIP94 = true` for regtest

I guess it is fine to do, but if this change is taken, it would be good to add a short release note snippet with a `Tests` section about this change. Otherwise people may wonder why their fancy regtest chain isn't working anymore due to a testnet4 rule (`time-timewarp-attack`). (Also, the code comment could be updated to say testnet4/regtest)

💬 Sjors commented on pull request "Move maximum timewarp attack threshold back to 600s from 7200s":
(https://github.com/bitcoin/bitcoin/pull/30647#issuecomment-2293028876)
@maflcko added

💬 glozow commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1719562102)
```
Filesystem Size Used Avail Use% Mounted on
/dev/nvme0n1p2 468G 110G 335G 25% /
```
It worked with gcc. `./configure` didn't like libc++-17. After reboot I can't get it to fail anymore (also I don't mean to hold up this PR)

💬 glozow commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#issuecomment-2293114610)
ACK faa1b9b0e6d

💬 maflcko commented on pull request "test: [refactor] Use g_rng/m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1719563783)
q in f1c1b37d2247eed49156c0467daa68aa38497bb8: Can you explain why this change is correct? `SeedRandomForTest` is doing two things:

* Set the internal global RNGState
* Reseed the passed `g_insecure_rand_ctx`

Removing `g_insecure_rand_ctx` does not remove the need to make the fuzz tests deterministic and stable.

I'll drop this change as well.

💬 maflcko commented on pull request "test: [refactor] Use g_rng/m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1719563882)
> so it could still be used conveniently to seed m_rng.

Not sure. The "feature" is only used in three places, two of which will be removed in the last commit.

So it just seems easier to drop this change and instead move the call to `Reseed` from this function to the only place that needs it.

📝 fanquake locked a pull request: "3 more security enhancements"
(https://github.com/bitcoin/bitcoin/pull/30663)
Please see the detailed description in the commits below.

📝 fanquake locked a pull request: "Adding security"
(https://github.com/bitcoin/bitcoin/pull/30662)
Adding two security enhancements. Please see the detailed description in the two commits below.
Thanks.

💬 maflcko commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1719575839)
> After reboot I can't get it to fail anymore (also I don't mean to hold up this PR)

Thanks for checking. This is really helpful to make sure that the feature isn't shipped to users with bugs included.

My random guess would be that there was a process running on your machine which accidentally or intentionally put files into all folders in `/tmp/`. However, based on your check with `ls --all` on the temp folder, which didn't reveal any hidden files, this may be something else, or a race (a
...

✅ maflcko closed an issue: "ci: failure in `wallet_multiwallet.py --legacy-wallet` - (`void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion 'it.second' failed.`)"
(https://github.com/bitcoin/bitcoin/issues/29073)

💬 maflcko commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet` - (`void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion 'it.second' failed.`)":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2293145694)
Closing for now. I think the test may still fail intermittently, however the bitcoind *crash* should be fixed.

I'd say it is the best to open a new issue for the test failure, once it happens again.

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1719620550)

Here's what I get on 255571339e7:
```
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 724,714.00 | 1,379.85 | 2.4% | 0.01 | `Linearize32TxWorstCase15000Iters`
| 245,712.75 | 4,069.79 | 0.3% | 0.01 | `Linearize32TxWorstCase5000Iters`
| 688,589.50 | 1,452.24 | 5.2% | 0.01 | :wavy_dash: `Linearize48
...

💬 maflcko commented on pull request "test: [refactor] Use g_rng/m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1719621319)
Done: Force pushed to remove this presumed bug in the last commit

🤔 vasild reviewed a pull request: "test: Do not write Python bytecode to source directory"
(https://github.com/bitcoin/bitcoin/pull/30533#pullrequestreview-2242364341)
ACK c91a10b6843b9c48cb200592e025bd2a0fcb8a7e in case this is reconsidered.

IMO this approach is better than leaving it as it is.

👍 danielabrozzoni approved a pull request: "contrib/signet/miner updates"
(https://github.com/bitcoin/bitcoin/pull/28417#pullrequestreview-2242377679)
Code review ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1719646767)
CI is failing because it halts at 128 iters, it seems.
Original iters given was 2^(16/2-1) = 128.
However, sqrt(2^(n-1)) = sqrt(2^15) > 150