🤔 cedwies reviewed a pull request: "test: refactor mempool_accept_wtxid"
(https://github.com/bitcoin/bitcoin/pull/33067#pullrequestreview-3082223310)
Changes look great.

Nice to see the valid_witness_malleate_tx helper instead of the ValidWitnessMalleatedTx class. Makes the test's logic much clearer IMO.

ACK 0cbcdca

💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-3148513933)
It will be nice if this get in to v30, cc all previous reviewers @maflcko perhaps add the v30.0 milestone

💬 Ataraxia009 commented on issue "Crash when chain tip is missing from candidate set":
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3148515467)
The issue I was addressing has nothing to do with `FindMostWorkChain` or a crash in it.

The root issue that I witnessed was a crash on launch in `PruneBlockIndexCandidates` which is caused by the data dir reaching a bad state.

In this state, the block validity of the block indexes in the `setBlockIndexCandidates` was correct, i.e.: `BLOCK_VALID_TRANSACTIONS`,
but the block validity for the blocks that were used to to build the `CoinsView` and therefore the `M_Chain` did not have their require
...

💬 furszy commented on issue "Crash when chain tip is missing from candidate set":
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3148522739)
Which version of Bitcoin-Core are you running?

🤔 mzumsande reviewed a pull request: "validation: Keep chain tip in candidate set"
(https://github.com/bitcoin/bitcoin/pull/33130#pullrequestreview-3082372722)
> After PruneBlockIndexCandidates() is called, the candidate set must include the current chain tip or a successor of it.

`setBlockIndexCandidates` must at all times include all connectable blocks that have at least as much work as the tip, so the tip itself must at all times be in the set.

> However, because the tip itself is never added to setBlockIndexCandidates via TryAddBlockIndexCandidate(), it may be absent from the set.

This seems wrong / makes no sense to me. No block can ever
...

💬 mzumsande commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3148775143)
Does this mean that `feature_reindex.py` still succeeds, so that reindex only fails if we delete the `index/` dir like in `feature_reindex_init.py`, but not if we just reindex without corrupting anything?!

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2250201595)
Ok, I am working on a PR for this and I should have it ready by tomorrow but I think it's better to keep it separate from this one.

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3148815473)
> which I think could be enabled for coinstatsindex relatively easily, FYI @furszy

Not sure if I maybe misunderstand that part but do you mean parallelization could be enabled easily for coinstatsindex? I haven't reviewed https://github.com/bitcoin/bitcoin/pull/26966 in depth yet but I don't think that's the case because the blocks have to be processed in order.

✅ nervana21 closed a pull request: "validation: Keep chain tip in candidate set"
(https://github.com/bitcoin/bitcoin/pull/33130)

💬 nervana21 commented on pull request "validation: Keep chain tip in candidate set":
(https://github.com/bitcoin/bitcoin/pull/33130#issuecomment-3148820941)
Hi mzumsande,
Thanks for the thorough feedback.

You're right, it is preferable to crash a node that is out of consensus. I believe that I did have a chain sequence construction that led to `setBlockIndexCandidates` without chain tip (now I'm less confident), but even so, I'm certain the chain construction would be out of consensus and therefore not worth salvaging. I wish I had considered before and I will try to consider in the future.

Thanks again!

📝 theStack opened a pull request: "fuzz: txgraph: fix `real_is_optimal` flag propagation in `CommitStaging`"
(https://github.com/bitcoin/bitcoin/pull/33132)
In the `txgraph` fuzz test, the `CommitStaging` step updates the `SimTxGraph` levels simply by erasing the front (=main) one in the `sims` vector, i.e. the staging level instance takes the place of the main level instance:

https://github.com/bitcoin/bitcoin/blob/83a2216f5278c1123ba740f1c8a055652e033ddd/src/test/fuzz/txgraph.cpp#L668-L672

This also includes the `real_is_optimal` flag (reflecting whether the corresponding real graph is known to be optimally linearized), without taking into a
...

💬 theStack commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3148839416)
> [@theStack](https://github.com/theStack)
>
> Can you confirm the bug?

Will test in 1-2 weeks, as I unfortunately don't have access to an OpenBSD machine right now.

💬 Crypt-iQ commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#issuecomment-3148912384)
I'm a little out of my depth and it is getting late, but reviewing this PR I think it's a slight behavior change in what we see as witness-stripped? P2SH-P2WSH are not caught by `IsWitnessProgram` in this PR, but they are caught as witness-stripped in master. I checked by modifying the test_p2sh_witness subtest in p2p_segwit.py. I didn't test P2SH-P2WPKH Not sure if it matters, figured I'd point it out.

🤔 furszy reviewed a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3082638040)
> > which I think could be enabled for coinstatsindex relatively easily, FYI @furszy
>
> Not sure if I maybe misunderstand that part but do you mean parallelization could be enabled easily for coinstatsindex? I haven't reviewed #26966 in depth yet but I don't think that's the case because the blocks have to be processed in order.

It shouldn't take me much to parallelize it. Most fields here are essentially accumulators. Blocks can be read from disk and processed in parallel. Only the dump
...

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#issuecomment-3149108432)
Good catch.

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2250635349)
Typo in added parenthesis: beggining

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2250618511)
Better to not add it in 784459ac79a89f591eb52a4c6c266c421ca8baec?

💬 maflcko commented on issue "Crash when chain tip is missing from candidate set":
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3149612107)
> I cannot figure out how the data got to that state because it seems like this state should never happen.

Bitcoin Core makes heavy use of CPU, RAM and storage IO. Hardware defects might only become visible when running Bitcoin Core. You might want to check your hardware for defects.

* Use software such as memtest86 to check your RAM.
* Use software such as linpack, or Prime95 to check the CPU behaviour under load.
* Use software such as smartctl, fsck, badblocks, or CrystalDiskInfo to test yo
...

💬 maflcko commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2250784148)
can be resolved?

💬 maflcko commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2250792650)
could just remove it?