💬 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
...
(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?
(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?
(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?
(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?
(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
(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.
(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.
(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?
(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
...
(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.
(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
...
(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 )
(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
(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
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251021307)
FWIW it is now working for GUI startup according to https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2829171034 in eb68bb48ab2bb163f20dc7430d0401d88ca43f4a
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251021307)
FWIW it is now working for GUI startup according to https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2829171034 in eb68bb48ab2bb163f20dc7430d0401d88ca43f4a
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251022089)
MacOS corrected to macOS in according to #31453 (comment)
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251022089)
MacOS corrected to macOS in according to #31453 (comment)
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251024649)
I've avoided explicit recommendation of a FS on purpose here; as far as I know exFAT is the only known-problematic FS on macOS?
Can be convinced otherwise, but leaving as-is for now
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251024649)
I've avoided explicit recommendation of a FS on purpose here; as far as I know exFAT is the only known-problematic FS on macOS?
Can be convinced otherwise, but leaving as-is for now
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-3149949745)
While I was re-touching this for the outstanding comments I took the chance to inline the check for a smaller diff and generally a little cleaner/better code (IMO).
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-3149949745)
While I was re-touching this for the outstanding comments I took the chance to inline the check for a smaller diff and generally a little cleaner/better code (IMO).
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2251119754)
Removed in dc1613169db
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2251119754)
Removed in dc1613169db
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3150096441)
Pushed another run with caches saving to re-test cache hit/restore later.
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3150096441)
Pushed another run with caches saving to re-test cache hit/restore later.