💬 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?

💬 maflcko commented on pull request "tests: cover abortrescan() in-flight True path with dynamic-tail retry":
(https://github.com/bitcoin/bitcoin/pull/33131#discussion_r2250797712)
why not just use the maximum in the first try?

💬 maflcko commented on pull request "ci: Use mlc `v1` and ignore `depends/work`":
(https://github.com/bitcoin/bitcoin/pull/33125#discussion_r2250809067)
This was already reported in https://github.com/bitcoin/bitcoin/issues/31044 (and fixed?) So it looks like a regression and this is the wrong fix?

💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2250835852)
Yes

💬 hebasto commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3149692620)
> 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?!

Correct.

💬 dergoegge commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3149703053)
> to better support forked clients

Concept NACK

This is not something Bitcoin Core needs to support.

💬 fanquake commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2250868861)
Are we going to pull in the updates, or leave this as-is?

🤔 janb84 reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3083347788)
ACK 53461dc5cedf5f7080c09246ef76ba8a6c38a103

This PR refactors the code to use the more modern contains() method. In my opinion this PR increased the readability of the code and removes the ambiguity of the intention of the count() methods used. With this change, the intent to enforce that exactly one item is/ is not present or just a presence check will be more obvious. (count() vs contains()).

The argument NOT to change the code because of the risk losing the original behaviour is an
...

💬 maflcko commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2250943593)
> Are we going to pull in the updates, or leave this as-is?

I've also created a preliminary review for all languages: https://github.com/maflcko/b-c-gui-translations-review/tree/main/reviews

There is some obvious erroneous translations, like https://github.com/maflcko/b-c-gui-translations-review/blob/05100fec918fc44787d43004ea3b02749f5a16ee/reviews/te.md#L160-L166, but I am not familiar with those languages, so I can't evaluate if the LLM generated translation is correct.

💬 janb84 commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3149866089)
Personally I like the intent to structure the code more (the introduction of functions).

Paused reviewing this PR:
- I paused reviewing this PR because the way the code is currently structured is unfinished to me. The introduction of a function for the new functionality is great but the rest of the code needs to be restructured to better incorporate the use of functions imho (e.g. use a main function).
- Have not looked at the intent of the code yet.

(P.S. This is not meant to be har
...

💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251017723)
Agree. I removed this "optimisation" in eb68bb48ab2bb163f20dc7430d0401d88ca43f4a

We now just check both directories to avoid missing any as you describe (and if they are renamed as per https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2171706368 )

💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251019297)
Going to leave this nit, as I don't think it's worth exatra logic here