📝 TheCharlatan opened a pull request: "test: Add functional test for bitcoin-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32145)
While the `bitcoin-chainstate` utility is not shipped in a release, it is the only current utility directly using the bitcoin kernel library. Adding a simple test for it is useful for checking that the library is actually usable. The test is also useful in future to demonstrate that the `bitcoin-chainstate` binary using the API for the kernel library introduced in #30595 actually works and offers similar features.
(https://github.com/bitcoin/bitcoin/pull/32145)
While the `bitcoin-chainstate` utility is not shipped in a release, it is the only current utility directly using the bitcoin kernel library. Adding a simple test for it is useful for checking that the library is actually usable. The test is also useful in future to demonstrate that the `bitcoin-chainstate` binary using the API for the kernel library introduced in #30595 actually works and offers similar features.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2013750590)
> Which standard library are you trying to build with?
libc++. Given the goal is to not have anything GCC related installed, only LLVM/Clang.
> Unsetting *PATH variables caused Ccache to stop working: #21552.
This doesn't seem relevant to Guix, where ccache isn't used? I think we are talking past each other, I'm asking about, what failure happens when this commit isn't included here?
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2013750590)
> Which standard library are you trying to build with?
libc++. Given the goal is to not have anything GCC related installed, only LLVM/Clang.
> Unsetting *PATH variables caused Ccache to stop working: #21552.
This doesn't seem relevant to Guix, where ccache isn't used? I think we are talking past each other, I'm asking about, what failure happens when this commit isn't included here?
💬 fanquake commented on pull request "doc: Add Clang/LLVM based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r2013754170)
We don't do this in any other docs, and as far as I'm aware, this being needed would be a bug, not sure it should be here.
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r2013754170)
We don't do this in any other docs, and as far as I'm aware, this being needed would be a bug, not sure it should be here.
💬 maflcko commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013761919)
e1862a0b5d3ff46d507ff8cd3426292ff587328d: Why are you changing this? Have you tested this?
My understanding is that this is already the rpc port, so re-using it should fail.
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013761919)
e1862a0b5d3ff46d507ff8cd3426292ff587328d: Why are you changing this? Have you tested this?
My understanding is that this is already the rpc port, so re-using it should fail.
💬 maflcko commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013763660)
same. doc/zmq does not even mention any test network, so the changes here seem wrong and unrelated at best.
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013763660)
same. doc/zmq does not even mention any test network, so the changes here seem wrong and unrelated at best.
💬 maflcko commented on pull request "test: Add functional test for bitcoin-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32145#issuecomment-2753817926)
Needs rebase, as the base is too old?
(https://github.com/bitcoin/bitcoin/pull/32145#issuecomment-2753817926)
Needs rebase, as the base is too old?
💬 maflcko commented on pull request "doc: Add Clang/LLVM based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r2013782146)
> We don't do this in any other docs, and as far as I'm aware, this being needed would be a bug, not sure it should be here.
Maybe I am misunderstanding cmake, but all docs refer to a single build directory as `build` and I don't think it is supported to use a single build directory without cleaning it and while using different configure flags on it on the fly. I am sure that not cleaning it will lead to more issues down the line (https://github.com/bitcoin/bitcoin/issues/31942). Maybe the `r
...
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r2013782146)
> We don't do this in any other docs, and as far as I'm aware, this being needed would be a bug, not sure it should be here.
Maybe I am misunderstanding cmake, but all docs refer to a single build directory as `build` and I don't think it is supported to use a single build directory without cleaning it and while using different configure flags on it on the fly. I am sure that not cleaning it will lead to more issues down the line (https://github.com/bitcoin/bitcoin/issues/31942). Maybe the `r
...
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2013832506)
On an `aarch64` build machine:
```
$ env HOSTS="x86_64-apple-darwin" ./contrib/guix/guix-build
<snip>
Building qt...
<snip>
[185/1062] Building CXX object qtbase/src/corelib/CMakeFiles/Core.dir/compat/removed_api.cpp.o
FAILED: qtbase/src/corelib/CMakeFiles/Core.dir/compat/removed_api.cpp.o
/home/hebasto/.guix-profile/bin/clang++ --target=x86_64-apple-darwin -isysroot/root/SDKs/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers -nostdlibinc -iwithsysroot/usr/include/c++/v1 -iwithsysro
...
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2013832506)
On an `aarch64` build machine:
```
$ env HOSTS="x86_64-apple-darwin" ./contrib/guix/guix-build
<snip>
Building qt...
<snip>
[185/1062] Building CXX object qtbase/src/corelib/CMakeFiles/Core.dir/compat/removed_api.cpp.o
FAILED: qtbase/src/corelib/CMakeFiles/Core.dir/compat/removed_api.cpp.o
/home/hebasto/.guix-profile/bin/clang++ --target=x86_64-apple-darwin -isysroot/root/SDKs/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers -nostdlibinc -iwithsysroot/usr/include/c++/v1 -iwithsysro
...
📝 Hopium21 opened a pull request: "Fix broken link"
(https://github.com/bitcoin/bitcoin/pull/32146)
Fix broken link
(https://github.com/bitcoin/bitcoin/pull/32146)
Fix broken link
✅ fanquake closed a pull request: "Fix broken link"
(https://github.com/bitcoin/bitcoin/pull/32146)
(https://github.com/bitcoin/bitcoin/pull/32146)
💬 TheCharlatan commented on pull request "test: Add functional test for bitcoin-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32145#issuecomment-2754009184)
> Needs rebase, as the base is too old?
Thanks, wrote this some time ago and forgot to rebase.
(https://github.com/bitcoin/bitcoin/pull/32145#issuecomment-2754009184)
> Needs rebase, as the base is too old?
Thanks, wrote this some time ago and forgot to rebase.
💬 Prabhat1308 commented on pull request "doc: Add Clang/LLVM based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2754014778)
Don't know how the runs got cancelled.
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2754014778)
Don't know how the runs got cancelled.
👍 hebasto approved a pull request: "cmake: Avoid fuzzer "multiple definition of `main'" errors"
(https://github.com/bitcoin/bitcoin/pull/31992#pullrequestreview-2716746018)
ACK 57d8b1f1b3375452213c48ce80e8207e2fe53ebc, tested on Ubuntu 24.04.
(https://github.com/bitcoin/bitcoin/pull/31992#pullrequestreview-2716746018)
ACK 57d8b1f1b3375452213c48ce80e8207e2fe53ebc, tested on Ubuntu 24.04.
💬 EniRox001 commented on pull request "contrib: Add external RPC code generator with unit tests":
(https://github.com/bitcoin/bitcoin/pull/32140#issuecomment-2754028392)
> `/schema.json` probably wasn't intended to be in this PR.
Yes, redundant now that latest schema is retrieved each time, pushed the change
(https://github.com/bitcoin/bitcoin/pull/32140#issuecomment-2754028392)
> `/schema.json` probably wasn't intended to be in this PR.
Yes, redundant now that latest schema is retrieved each time, pushed the change
💬 TheCharlatan commented on pull request "cmake: Avoid fuzzer "multiple definition of `main'" errors":
(https://github.com/bitcoin/bitcoin/pull/31992#issuecomment-2754036766)
Concept ACK, but I'll defer to the opinions of people actually dealing with the fuzzing infrastructure on a regular basis.
(https://github.com/bitcoin/bitcoin/pull/31992#issuecomment-2754036766)
Concept ACK, but I'll defer to the opinions of people actually dealing with the fuzzing infrastructure on a regular basis.
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2754051147)
Pushed 29f05b91cf8a479e403b0322afeb5ff1133da221 -> 97d1edcdafe074e910ed647dcb6beedd24744b17 ([kernelApi_32](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_32) -> [kernelApi_33](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_33), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_32..kernelApi_33))
* Added a commit introducing a small purpose section in the header documentation. It briefly mentions the features, that the header is unversioned, might just break
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2754051147)
Pushed 29f05b91cf8a479e403b0322afeb5ff1133da221 -> 97d1edcdafe074e910ed647dcb6beedd24744b17 ([kernelApi_32](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_32) -> [kernelApi_33](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_33), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_32..kernelApi_33))
* Added a commit introducing a small purpose section in the header documentation. It briefly mentions the features, that the header is unversioned, might just break
...
💬 laanwj commented on pull request "Fix 11-year-old mis-categorized error code in OP_IF evaluation":
(https://github.com/bitcoin/bitcoin/pull/32143#issuecomment-2754060888)
This is failing the "test each commit" check. Please squash test commits where needed to make sure that the intermediate commits pass the tests, too.
<details>
```
test/script_tests.cpp(908): Entering test case "script_json_test"
2025-03-26T09:15:59.630554Z [test] [test/util/random.cpp:48] [SeedRandomStateForTest] Setting random seed for current tests to RANDOM_CTX_SEED=be1d635f1a8e48c6048a40b25f2bd5e1d235e5ec1571241fe7157b541468c337
2025-03-26T09:15:59.630977Z [test] [init/common.cpp:151
...
(https://github.com/bitcoin/bitcoin/pull/32143#issuecomment-2754060888)
This is failing the "test each commit" check. Please squash test commits where needed to make sure that the intermediate commits pass the tests, too.
<details>
```
test/script_tests.cpp(908): Entering test case "script_json_test"
2025-03-26T09:15:59.630554Z [test] [test/util/random.cpp:48] [SeedRandomStateForTest] Setting random seed for current tests to RANDOM_CTX_SEED=be1d635f1a8e48c6048a40b25f2bd5e1d235e5ec1571241fe7157b541468c337
2025-03-26T09:15:59.630977Z [test] [init/common.cpp:151
...
🤔 BrandonOdiwuor reviewed a pull request: "test: Add test coverage for rpcwhitelistdefault when unset"
(https://github.com/bitcoin/bitcoin/pull/32079#pullrequestreview-2716798478)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32079#pullrequestreview-2716798478)
Concept ACK
💬 Sjors commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013934249)
Because testnet4 uses 4xxxx ports. But I guess that doesn't really matter here, because testnet3 uses 1xxxx ports and not 2xxxx. I'll just revert the port numbers and only change `-testnet` to ` -testnet4`.
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013934249)
Because testnet4 uses 4xxxx ports. But I guess that doesn't really matter here, because testnet3 uses 1xxxx ports and not 2xxxx. I'll just revert the port numbers and only change `-testnet` to ` -testnet4`.
💬 Sjors commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#issuecomment-2754092067)
e1862a0b5d3ff46d507ff8cd3426292ff587328d -> 4281e3603a2eadefc8535b863128a05cf3a5a75f: reduced the scope of ZMQ changes, no longer touching the ports, see https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013761919
(https://github.com/bitcoin/bitcoin/pull/32096#issuecomment-2754092067)
e1862a0b5d3ff46d507ff8cd3426292ff587328d -> 4281e3603a2eadefc8535b863128a05cf3a5a75f: reduced the scope of ZMQ changes, no longer touching the ports, see https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013761919