🚀 fanquake merged a pull request: "test: Remove legacy wallet RPC overloads"
(https://github.com/bitcoin/bitcoin/pull/32452)
(https://github.com/bitcoin/bitcoin/pull/32452)
💬 TheCharlatan commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2888253440)
Concept ACK
Can you add the schema of the index and the expected arguments for the REST API to the pull request description? I was a bit confused at first if this now exposes the file position, but if I read it correctly now, this just allows querying a transaction by its index in the block.
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2888253440)
Concept ACK
Can you add the schema of the index and the expected arguments for the REST API to the pull request description? I was a bit confused at first if this now exposes the file position, but if I read it correctly now, this just allows querying a transaction by its index in the block.
💬 TheCharlatan commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2094072165)
Is there really a scenario where we could be reading a block, but not have its block index entry? Maybe this should just be an `Assume` instead.
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2094072165)
Is there really a scenario where we could be reading a block, but not have its block index entry? Maybe this should just be an `Assume` instead.
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2888263760)
> Concept ACK
Thanks!
> Can you add the schema of the index and the expected arguments for the REST API to the pull request description?
Sure - updated in https://github.com/bitcoin/bitcoin/pull/32541#issue-3070502385.
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2888263760)
> Concept ACK
Thanks!
> Can you add the schema of the index and the expected arguments for the REST API to the pull request description?
Sure - updated in https://github.com/bitcoin/bitcoin/pull/32541#issue-3070502385.
💬 hebasto commented on issue "Unit tests incompatible with `qt5ct` or `qt5-style-plugins`":
(https://github.com/bitcoin-core/gui/issues/630#issuecomment-2888264275)
Closing due to recent upgrade to Qt 6.
Feel free to reopen with updated details.
(https://github.com/bitcoin-core/gui/issues/630#issuecomment-2888264275)
Closing due to recent upgrade to Qt 6.
Feel free to reopen with updated details.
✅ hebasto closed an issue: "Unit tests incompatible with `qt5ct` or `qt5-style-plugins`"
(https://github.com/bitcoin-core/gui/issues/630)
(https://github.com/bitcoin-core/gui/issues/630)
💬 hebasto commented on issue "Option to use dark theme for Windows":
(https://github.com/bitcoin-core/gui/issues/378#issuecomment-2888271232)
The issue has been resolved in https://github.com/bitcoin/bitcoin/pull/30997, as Qt 6.7.3 allows an app to use the dark system palette on Windows.
Compare screenshots:
- v29.0:

- the master branch @ 7710a31f0cb69a04529f39840196826d0b9770ab:

(https://github.com/bitcoin-core/gui/issues/378#issuecomment-2888271232)
The issue has been resolved in https://github.com/bitcoin/bitcoin/pull/30997, as Qt 6.7.3 allows an app to use the dark system palette on Windows.
Compare screenshots:
- v29.0:

- the master branch @ 7710a31f0cb69a04529f39840196826d0b9770ab:

✅ hebasto closed an issue: "Option to use dark theme for Windows"
(https://github.com/bitcoin-core/gui/issues/378)
(https://github.com/bitcoin-core/gui/issues/378)
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2888272325)
We might want to add to the release notes that this change effectively makes [dark mode](https://github.com/bitcoin-core/gui/issues/378) available on Windows.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2888272325)
We might want to add to the release notes that this change effectively makes [dark mode](https://github.com/bitcoin-core/gui/issues/378) available on Windows.
⚠️ fanquake opened an issue: "test: Running thread * was not suspended. False leaks are possible."
(https://github.com/bitcoin/bitcoin/issues/32542)
Related to the discussion in #32538. Our `test_bitcoin` binary will output this warning when compiled with `address` sanitzer:
```bash
# clang version 20.1.3 (Fedora 20.1.3-1.fc43)
cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DSANITIZERS=address
cmake --build build
./build/bin/test_bitcoin
Running 655 test cases...
==72451==Running thread 72090 was not suspended. False leaks are possible.
*** No errors detected
```
However I couldn't get`ctest` to output the same warn
...
(https://github.com/bitcoin/bitcoin/issues/32542)
Related to the discussion in #32538. Our `test_bitcoin` binary will output this warning when compiled with `address` sanitzer:
```bash
# clang version 20.1.3 (Fedora 20.1.3-1.fc43)
cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DSANITIZERS=address
cmake --build build
./build/bin/test_bitcoin
Running 655 test cases...
==72451==Running thread 72090 was not suspended. False leaks are possible.
*** No errors detected
```
However I couldn't get`ctest` to output the same warn
...
💬 fanquake commented on pull request "restrict std::cerr to errors; use std::cout for warnings and info":
(https://github.com/bitcoin/bitcoin/pull/32538#issuecomment-2888272927)
> We want the tests to fail when there is any warning or error (from Bitcoin Core or any other tool/sanitizer/lib) that is not accounted for
Opened #32542 in relation to this.
(https://github.com/bitcoin/bitcoin/pull/32538#issuecomment-2888272927)
> We want the tests to fail when there is any warning or error (from Bitcoin Core or any other tool/sanitizer/lib) that is not accounted for
Opened #32542 in relation to this.
💬 fanquake commented on issue "cmake: Cannot find Qt 6 on SunOS / illumos (OpenIndiana Distribution)":
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2888278841)
> If we want to provide binary packages for download on a website, we take the role of package maintainers. In packaging scripts, we need to setup a build environment so that all dependencies can be found. There are countless ways to do that.
The only way we will produce anything binary, that would end up being distributed on our website, is via our Guix build; so any approach taken needs to happen inside Guix (all blobs/binaries must be reproducible).
> Such build scripts ideally should be m
...
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2888278841)
> If we want to provide binary packages for download on a website, we take the role of package maintainers. In packaging scripts, we need to setup a build environment so that all dependencies can be found. There are countless ways to do that.
The only way we will produce anything binary, that would end up being distributed on our website, is via our Guix build; so any approach taken needs to happen inside Guix (all blobs/binaries must be reproducible).
> Such build scripts ideally should be m
...
👍 TheCharlatan approved a pull request: "refactor: validation: mark CheckBlockIndex as const"
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2848233855)
ACK 3e6ac5bf772751c66cdcd015dcb7e6ce4ea2cc77
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2848233855)
ACK 3e6ac5bf772751c66cdcd015dcb7e6ce4ea2cc77
🤔 luke-jr reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2848222887)
Probably an improvement as-is, but might want to think through the name proxy logic
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2848222887)
Probably an improvement as-is, but might want to think through the name proxy logic
💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094078095)
I'm not sure this logic is ideal. If you set an all-network proxy, but then disable it for IPv6, you might be expecting name resolution to use the proxy. Or maybe not.
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094078095)
I'm not sure this logic is ideal. If you set an all-network proxy, but then disable it for IPv6, you might be expecting name resolution to use the proxy. Or maybe not.
💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094077060)
These conditions aren't needed. `SetProxy` itself just returns false if it's not valid.
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094077060)
These conditions aren't needed. `SetProxy` itself just returns false if it's not valid.
💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094087242)
string_view?
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094087242)
string_view?
💬 maflcko commented on issue "cmake: Cannot find Qt 6 on SunOS / illumos (OpenIndiana Distribution)":
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2888283003)
Are there any known users on this platform? Maybe this can be closed for now, until there are a few users. Then, a `build-*.md` can be added with any needed details. Whether or not to do a guix build can be decided then as well.
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2888283003)
Are there any known users on this platform? Maybe this can be closed for now, until there are a few users. Then, a `build-*.md` can be added with any needed details. Whether or not to do a guix build can be decided then as well.
💬 maflcko commented on pull request "[DONOTMERGE] subprocess: replace `fs::directory_iterator` with `readdir`":
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2888285136)
I haven't looked at the code, but the CI passed 50 times: https://cirrus-ci.com/task/6662743735926784
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2888285136)
I haven't looked at the code, but the CI passed 50 times: https://cirrus-ci.com/task/6662743735926784
💬 luke-jr commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094091911)
Did you want to indent this?
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2094091911)
Did you want to indent this?