🤔 maflcko reviewed a pull request: "[POC] ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2408594619)
Thanks for the POC!
Without caching I wonder how useful this is. However, caching may be the hardest part to get right.
Taking a step back, I wonder what was the last real issue found by Wine, which wasn't found by the native Windows task? If there are none, or not too many, I think this could also be made a nightly CI task, only run on master after a merge (without caching), or in a completely separate repo. The same is done for other stuff, like the valgrind tasks, or the s390x tasks.
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2408594619)
Thanks for the POC!
Without caching I wonder how useful this is. However, caching may be the hardest part to get right.
Taking a step back, I wonder what was the last real issue found by Wine, which wasn't found by the native Windows task? If there are none, or not too many, I think this could also be made a nightly CI task, only run on master after a merge (without caching), or in a completely separate repo. The same is done for other stuff, like the valgrind tasks, or the s390x tasks.
💬 maflcko commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1824875135)
At least with QT, this may be too slow without caching.
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1824875135)
At least with QT, this may be too slow without caching.
💬 maflcko commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1824859575)
Forgot to copy the comment?
```suggestion
container: debian:bookworm # Check that https://packages.debian.org/bookworm/g++-mingw-w64-x86-64-posix (version 12.2, similar to guix) can cross-compile
```
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1824859575)
Forgot to copy the comment?
```suggestion
container: debian:bookworm # Check that https://packages.debian.org/bookworm/g++-mingw-w64-x86-64-posix (version 12.2, similar to guix) can cross-compile
```
💬 maflcko commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1824861385)
Also, would be good to remove (in the same commit) `wine` from `ci/test/00_setup_env_win64.sh`, or at least set `RUN_UNIT_TESTS=false`.
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1824861385)
Also, would be good to remove (in the same commit) `wine` from `ci/test/00_setup_env_win64.sh`, or at least set `RUN_UNIT_TESTS=false`.
💬 Urpferd commented on issue "Bitcoin Core 28.0 (Flatpak Linux Mint) pointing or saving blockchain to external drives does not work":
(https://github.com/bitcoin/bitcoin/issues/31188#issuecomment-2450434016)
Thank you so much, good catch, it is my bad!
Although I don't know when this change into .config might have happened, weird. I don't remember creating this file but copy pasting.
Blocks get now written on external drive but not the other data. Seems some command-line arg switches path back to default datadir, is there any way to change this?
It would be so much easier for less tech savvy people and normies being able to adjust paths trough GUI like other software does.
```
...
Defaul
...
(https://github.com/bitcoin/bitcoin/issues/31188#issuecomment-2450434016)
Thank you so much, good catch, it is my bad!
Although I don't know when this change into .config might have happened, weird. I don't remember creating this file but copy pasting.
Blocks get now written on external drive but not the other data. Seems some command-line arg switches path back to default datadir, is there any way to change this?
It would be so much easier for less tech savvy people and normies being able to adjust paths trough GUI like other software does.
```
...
Defaul
...
💬 davidgumberg commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450443551)
Even if a bigger dbcache mitigates the issue because of fewer flushes, using a low dbcache setting should not result in leveldb block checksum mismatches, I will experiment with syncing a node with `-dbache=32`.
With such a low dbcache you are definitely doing way more writing than a node with default dbcache, so I imagine the rate of wear on the disk is somewhat high
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450443551)
Even if a bigger dbcache mitigates the issue because of fewer flushes, using a low dbcache setting should not result in leveldb block checksum mismatches, I will experiment with syncing a node with `-dbache=32`.
With such a low dbcache you are definitely doing way more writing than a node with default dbcache, so I imagine the rate of wear on the disk is somewhat high
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450446603)
> I think 32MB might be low enough that a flush gets triggered every block?
I didn't see that in the logs
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450446603)
> I think 32MB might be low enough that a flush gets triggered every block?
I didn't see that in the logs
💬 maflcko commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2450447158)
> We could have used `ReadLE64` to unify the byte order for keys and writable values, but that shouldn't be necessary, since both have the same endianness locally that shouldn't be affected by a byte-by-byte xor.
The s390x unit tests fail:
```
./src/test/streams_tests.cpp(40): error: in "streams_tests/xor_bytes": check { expected.begin(), expected.end() } == { actual.begin(), actual.end() } has failed.
Mismatch at position 0: � != |
Mismatch at position 1: Y != �
Mismatch at positio
...
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2450447158)
> We could have used `ReadLE64` to unify the byte order for keys and writable values, but that shouldn't be necessary, since both have the same endianness locally that shouldn't be affected by a byte-by-byte xor.
The s390x unit tests fail:
```
./src/test/streams_tests.cpp(40): error: in "streams_tests/xor_bytes": check { expected.begin(), expected.end() } == { actual.begin(), actual.end() } has failed.
Mismatch at position 0: � != |
Mismatch at position 1: Y != �
Mismatch at positio
...
💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2450465825)
> The s390x unit tests fail:
I don't know how to access that, is it part of CI? Does the test suite pass on it otherwise or was it just curiosity? Do you know the reason it fails, is my assumption incorrect that endianness should apply to both parts (key and value) the same way?
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2450465825)
> The s390x unit tests fail:
I don't know how to access that, is it part of CI? Does the test suite pass on it otherwise or was it just curiosity? Do you know the reason it fails, is my assumption incorrect that endianness should apply to both parts (key and value) the same way?
💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824904589)
I prefer the new `key_missing` part, it did confuse me once during development as well
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824904589)
I prefer the new `key_missing` part, it did confuse me once during development as well
💬 davidgumberg commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450469057)
> I didn't see that in the logs
You can see flushes with `-debug=coindb`
Every block was ambitious, sorry, maybe every ~15 blocks or so.
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450469057)
> I didn't see that in the logs
You can see flushes with `-debug=coindb`
Every block was ambitious, sorry, maybe every ~15 blocks or so.
👍 instagibbs approved a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2408693031)
reACK https://github.com/bitcoin/bitcoin/pull/31139/commits/d7fd766feb2f579bdba0e778bacdeb13103e8282
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2408693031)
reACK https://github.com/bitcoin/bitcoin/pull/31139/commits/d7fd766feb2f579bdba0e778bacdeb13103e8282
💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450507672)
maybe ECC ram would be smarter too... could just be getting hit by cosmic rays or something
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2450507672)
maybe ECC ram would be smarter too... could just be getting hit by cosmic rays or something
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1824943979)
I think this is resolved now with the rewrite of `ChangeSet::CalculateMemPoolAncestors()` -- let me know if you agree.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1824943979)
I think this is resolved now with the rewrite of `ChangeSet::CalculateMemPoolAncestors()` -- let me know if you agree.
💬 instagibbs commented on pull request "include verbose "reject-details" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#discussion_r1824946074)
@pinheadmz could pop that field just like the check_mempool_result pops fee field?
(https://github.com/bitcoin/bitcoin/pull/28121#discussion_r1824946074)
@pinheadmz could pop that field just like the check_mempool_result pops fee field?
📝 Sjors opened a pull request: "refactor: mining interface 30955 followups"
(https://github.com/bitcoin/bitcoin/pull/31197)
This PR implements the refactors suggested in https://github.com/bitcoin/bitcoin/pull/30955#pullrequestreview-2354931253.
(https://github.com/bitcoin/bitcoin/pull/31197)
This PR implements the refactors suggested in https://github.com/bitcoin/bitcoin/pull/30955#pullrequestreview-2354931253.
💬 Sjors commented on pull request "Mining interface: getCoinbaseMerklePath() and submitSolution()":
(https://github.com/bitcoin/bitcoin/pull/30955#discussion_r1824953742)
Done in #31197, though I used `leaf_pos`.
(https://github.com/bitcoin/bitcoin/pull/30955#discussion_r1824953742)
Done in #31197, though I used `leaf_pos`.
💬 Sjors commented on pull request "Mining interface: getCoinbaseMerklePath() and submitSolution()":
(https://github.com/bitcoin/bitcoin/pull/30955#discussion_r1824954017)
Done in #31197
(https://github.com/bitcoin/bitcoin/pull/30955#discussion_r1824954017)
Done in #31197
💬 Sjors commented on pull request "Mining interface: getCoinbaseMerklePath() and submitSolution()":
(https://github.com/bitcoin/bitcoin/pull/30955#discussion_r1824954557)
Done in #31197
(https://github.com/bitcoin/bitcoin/pull/30955#discussion_r1824954557)
Done in #31197
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1824955448)
I think I prefer letting the fuzzer pick it, so that I don't have to worry about whether there are other aspects of the transaction I'm overfitting (and thus getting worse coverage than if I let the fuzzer do all the work).
On the other hand, if there are certain transaction templates that we want to be sure are covered, then perhaps I can flip a bool and determine whether to use a pre-constructed transaction versus use one from the fuzzer.
I can't think of what that might be though, so
...
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1824955448)
I think I prefer letting the fuzzer pick it, so that I don't have to worry about whether there are other aspects of the transaction I'm overfitting (and thus getting worse coverage than if I let the fuzzer do all the work).
On the other hand, if there are certain transaction templates that we want to be sure are covered, then perhaps I can flip a bool and determine whether to use a pre-constructed transaction versus use one from the fuzzer.
I can't think of what that might be though, so
...