👍 hebasto approved a pull request: "threading: drop CSemaphore in favor of c++20 std::counting_semaphore"
(https://github.com/bitcoin/bitcoin/pull/32466#pullrequestreview-2854910068)
ACK 6f7052a7b96f058568af9aed2f014ae7a25e0f68, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/32466#pullrequestreview-2854910068)
ACK 6f7052a7b96f058568af9aed2f014ae7a25e0f68, I have reviewed the code and it looks OK.
⚠️ brunoerg opened an issue: "build: error when building for test coverage"
(https://github.com/bitcoin/bitcoin/issues/32571)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When trying to compile it for test coverage, I'm getting the following error:
```
genhtml: ERROR: cannot read /Users/brunogarcia/projects/bitcoin-core-dev/build_cov/src/CMakeFiles/bitcoin-cli.dir/span.h
Processing file CMakeFiles/bitcoin-cli.dir/span.h
CMake Error at build_cov/Coverage.cmake:46 (execute_process):
execute_process failed command indexes:
1: "Child return code: 2"
```
...
(https://github.com/bitcoin/bitcoin/issues/32571)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When trying to compile it for test coverage, I'm getting the following error:
```
genhtml: ERROR: cannot read /Users/brunogarcia/projects/bitcoin-core-dev/build_cov/src/CMakeFiles/bitcoin-cli.dir/span.h
Processing file CMakeFiles/bitcoin-cli.dir/span.h
CMake Error at build_cov/Coverage.cmake:46 (execute_process):
execute_process failed command indexes:
1: "Child return code: 2"
```
...
💬 fanquake commented on pull request "depends: use "mkdir -p" when installing xproto":
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2895159711)
Guix Build:
```bash
c07d19f1279ac1255fff06fff094eea425aa2d228b2cf4fef8ffa366fac344dc guix-build-df9ebbf659d5/output/aarch64-linux-gnu/SHA256SUMS.part
7a20ba73e966048d991d3d274c2a97575d47473befefe17e850760bc48cbbd9b guix-build-df9ebbf659d5/output/aarch64-linux-gnu/bitcoin-df9ebbf659d5-aarch64-linux-gnu-debug.tar.gz
bf70710e4308ec852471510d9f6ddc301c0c083c74d49c208e935d8b72e26e1b guix-build-df9ebbf659d5/output/aarch64-linux-gnu/bitcoin-df9ebbf659d5-aarch64-linux-gnu.tar.gz
00fc3a52592fe07a
...
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2895159711)
Guix Build:
```bash
c07d19f1279ac1255fff06fff094eea425aa2d228b2cf4fef8ffa366fac344dc guix-build-df9ebbf659d5/output/aarch64-linux-gnu/SHA256SUMS.part
7a20ba73e966048d991d3d274c2a97575d47473befefe17e850760bc48cbbd9b guix-build-df9ebbf659d5/output/aarch64-linux-gnu/bitcoin-df9ebbf659d5-aarch64-linux-gnu-debug.tar.gz
bf70710e4308ec852471510d9f6ddc301c0c083c74d49c208e935d8b72e26e1b guix-build-df9ebbf659d5/output/aarch64-linux-gnu/bitcoin-df9ebbf659d5-aarch64-linux-gnu.tar.gz
00fc3a52592fe07a
...
💬 hebasto commented on pull request "threading: drop CSemaphore in favor of c++20 std::counting_semaphore":
(https://github.com/bitcoin/bitcoin/pull/32466#discussion_r2098433690)
nit: It doesn't appear to be used anywhere at the moment.
(https://github.com/bitcoin/bitcoin/pull/32466#discussion_r2098433690)
nit: It doesn't appear to be used anywhere at the moment.
📝 maflcko opened a pull request: "doc: Remove stale sections in dev notes"
(https://github.com/bitcoin/bitcoin/pull/32572)
This removes sections that I've been collecting as stale or overly redundant over the years. The rationale for each removal is in the commit message.
(https://github.com/bitcoin/bitcoin/pull/32572)
This removes sections that I've been collecting as stale or overly redundant over the years. The rationale for each removal is in the commit message.
💬 hebasto commented on issue "build: error when building for test coverage":
(https://github.com/bitcoin/bitcoin/issues/32571#issuecomment-2895198821)
What if you were to use the instructions from https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-llvmclang-toolchain?
(https://github.com/bitcoin/bitcoin/issues/32571#issuecomment-2895198821)
What if you were to use the instructions from https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-llvmclang-toolchain?
✅ brunoerg closed an issue: "build: error when building for test coverage"
(https://github.com/bitcoin/bitcoin/issues/32571)
(https://github.com/bitcoin/bitcoin/issues/32571)
💬 brunoerg commented on issue "build: error when building for test coverage":
(https://github.com/bitcoin/bitcoin/issues/32571#issuecomment-2895208969)
It worked, sorry, missed that. Thank you.
(https://github.com/bitcoin/bitcoin/issues/32571#issuecomment-2895208969)
It worked, sorry, missed that. Thank you.
💬 hebasto commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2895214684)
My Guix build:
```
aarch64
d899fe5104a77d8c86d40518cb872e8200142e5711689a7bec1e116fcb1807af guix-build-800b7cc42ca6/output/aarch64-linux-gnu/SHA256SUMS.part
8f81b2cd236c0116937b950964f1a4a88c0fdf1baf0d859c055153c4fceddcdc guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu-debug.tar.gz
43812573c4d833df3cae58332dfd632afedd428fc0c46a403b872bdfb344bfb7 guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu.tar.gz
1e93c16f
...
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2895214684)
My Guix build:
```
aarch64
d899fe5104a77d8c86d40518cb872e8200142e5711689a7bec1e116fcb1807af guix-build-800b7cc42ca6/output/aarch64-linux-gnu/SHA256SUMS.part
8f81b2cd236c0116937b950964f1a4a88c0fdf1baf0d859c055153c4fceddcdc guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu-debug.tar.gz
43812573c4d833df3cae58332dfd632afedd428fc0c46a403b872bdfb344bfb7 guix-build-800b7cc42ca6/output/aarch64-linux-gnu/bitcoin-800b7cc42ca6-aarch64-linux-gnu.tar.gz
1e93c16f
...
👍 ryanofsky approved a pull request: "init: Configure reachable networks before we start the RPC server"
(https://github.com/bitcoin/bitcoin/pull/32539#pullrequestreview-2854992208)
Code review ACK 12ff4be9c724c752fe67835419be3ff4d0e65990
Did not look closely how the RPC binding code works, but the only code change here is moving g_reachable_nets initialization earlier in the InitAppMain function, which makes sense and is safe. There is also a new test and documentation.
Since the g_reachable_nets code has to move anyway, it could be make sense to move it into an `InitReachableNets(args)` or similar helper function so `AppInitMain` could be simpler. But not important,
...
(https://github.com/bitcoin/bitcoin/pull/32539#pullrequestreview-2854992208)
Code review ACK 12ff4be9c724c752fe67835419be3ff4d0e65990
Did not look closely how the RPC binding code works, but the only code change here is moving g_reachable_nets initialization earlier in the InitAppMain function, which makes sense and is safe. There is also a new test and documentation.
Since the g_reachable_nets code has to move anyway, it could be make sense to move it into an `InitReachableNets(args)` or similar helper function so `AppInitMain` could be simpler. But not important,
...
🤔 furszy reviewed a pull request: "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors"
(https://github.com/bitcoin/bitcoin/pull/32475#pullrequestreview-2855022471)
Code review ACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
(https://github.com/bitcoin/bitcoin/pull/32475#pullrequestreview-2855022471)
Code review ACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
✅ portlandhodl closed an issue: "[RPC] deriveaddresses pk(descriptor) returns p2pkh address"
(https://github.com/bitcoin/bitcoin/issues/32570)
(https://github.com/bitcoin/bitcoin/issues/32570)
💬 portlandhodl commented on issue "[RPC] deriveaddresses pk(descriptor) returns p2pkh address":
(https://github.com/bitcoin/bitcoin/issues/32570#issuecomment-2895272087)
Closing, incorrect version
(https://github.com/bitcoin/bitcoin/issues/32570#issuecomment-2895272087)
Closing, incorrect version
💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2098527513)
Will leave for a followup
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2098527513)
Will leave for a followup
💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2098527629)
Will leave for a followup
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2098527629)
Will leave for a followup
🤔 1440000bytes reviewed a pull request: "doc: Remove stale sections in dev notes"
(https://github.com/bitcoin/bitcoin/pull/32572#pullrequestreview-2855148437)
ACK https://github.com/bitcoin/bitcoin/pull/32572/commits/fa2482adf5d2df2192e882dc9ca817056ea52087
<details>
<summary>Signature</summary>
<pre>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK fa2482adf5d2df2192e882dc9ca817056ea52087
-----BEGIN PGP SIGNATURE-----
iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaCzEGQAKCRAtIwUgISpZ
AbhcAP9ygFAp7qD4fcUKOAg6ZSm16COjkWKxafquCHUkmqn8MQD/Ve6fkUMINSW4
sodJPMcTbr55vrJoR7G4KLtRoLs+ogg=
=9NNA
-----END PGP SIGNATURE-----
</pre>
</detai
...
(https://github.com/bitcoin/bitcoin/pull/32572#pullrequestreview-2855148437)
ACK https://github.com/bitcoin/bitcoin/pull/32572/commits/fa2482adf5d2df2192e882dc9ca817056ea52087
<details>
<summary>Signature</summary>
<pre>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK fa2482adf5d2df2192e882dc9ca817056ea52087
-----BEGIN PGP SIGNATURE-----
iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaCzEGQAKCRAtIwUgISpZ
AbhcAP9ygFAp7qD4fcUKOAg6ZSm16COjkWKxafquCHUkmqn8MQD/Ve6fkUMINSW4
sodJPMcTbr55vrJoR7G4KLtRoLs+ogg=
=9NNA
-----END PGP SIGNATURE-----
</pre>
</detai
...
💬 luke-jr commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2895395162)
How does this compare to `getrawtransaction <txid> 0 <blockhash>` without a txindex?
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-2895395162)
How does this compare to `getrawtransaction <txid> 0 <blockhash>` without a txindex?
💬 murchandamus commented on issue "Coin Selection tracepoint overreports use of APS":
(https://github.com/bitcoin/bitcoin/issues/25150#issuecomment-2895404858)
Maybe this issue would be more relevant if we renamed it to "Only run APS if the original solution would spend a subset of UTXOs sharing the same output script". cc: @achow101
(https://github.com/bitcoin/bitcoin/issues/25150#issuecomment-2895404858)
Maybe this issue would be more relevant if we renamed it to "Only run APS if the original solution would spend a subset of UTXOs sharing the same output script". cc: @achow101
💬 luke-jr commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2098610613)
Why?
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2098610613)
Why?
💬 fanquake commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2098613366)
Can be dropped after #32568.
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2098613366)
Can be dropped after #32568.