⚠️ portlandhodl opened an issue: "[RPC] deriveaddresses pk(descriptor) returns p2pkh address"
(https://github.com/bitcoin/bitcoin/issues/32570)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
While trying to extract the public key from a descriptor I might have walked into this bug. My thought was passing a pk(xpub) descriptor would return either an error or a pubkey to push instead it returned the p2pkh address. This is IMO inconsistent and frankly not very useful because the PK is behind the hash.
Note there are both pk and pkh types.
```
[bitcoin@qrsnaps0 ~]$ bitcoin-cli d
...
(https://github.com/bitcoin/bitcoin/issues/32570)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
While trying to extract the public key from a descriptor I might have walked into this bug. My thought was passing a pk(xpub) descriptor would return either an error or a pubkey to push instead it returned the p2pkh address. This is IMO inconsistent and frankly not very useful because the PK is behind the hash.
Note there are both pk and pkh types.
```
[bitcoin@qrsnaps0 ~]$ bitcoin-cli d
...
💬 fanquake commented on pull request "deps: Bump lief to 0.16.4":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2895075154)
I think it'd be worth investigating any issues with `0.16.5`, rather than deferring. Seeing some build output with this branch, which is unexpected:
```bash
[100%] Built target bitcoin-qt
Checking binary security...
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_r
...
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2895075154)
I think it'd be worth investigating any issues with `0.16.5`, rather than deferring. Seeing some build output with this branch, which is unexpected:
```bash
[100%] Built target bitcoin-qt
Checking binary security...
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_relro:#True and have_bindnow: True
have_gnu_r
...
💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2098378254)
> Shall I just put it back?
Perhaps updating only the `Path` variable will be sufficient?
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2098378254)
> Shall I just put it back?
Perhaps updating only the `Path` variable will be sufficient?
💬 ryanofsky commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2098410495)
> That way if someone ever tries to add a `return false` in the batched case, we'd catch it right away in testing.
Catch what though? Catch CheckInputScripts detecting an invalid input and returning false like someone would reasonably expect it to? There aren't warnings or any indication inside CheckInputScripts saying that returning false may crash the program. So I don't think it's hard to imagine someone optimizing CheckInputScripts or refactoring or just adding a new check that returns fa
...
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2098410495)
> That way if someone ever tries to add a `return false` in the batched case, we'd catch it right away in testing.
Catch what though? Catch CheckInputScripts detecting an invalid input and returning false like someone would reasonably expect it to? There aren't warnings or any indication inside CheckInputScripts saying that returning false may crash the program. So I don't think it's hard to imagine someone optimizing CheckInputScripts or refactoring or just adding a new check that returns fa
...
👍 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
...