⚠️ jimhashhq opened an issue: "cmake: Compiling for test coverage (low-priority workaround exists)"
(https://github.com/bitcoin/bitcoin/issues/31638)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When following Compiling for Test Coverage instructions:
[doc/developer-notes-CompilingForTestCoverage](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage)
I encounter error:
```
CMake Error: File /home/alicebob/wkspc1/presets/bitcoin/cmake/cov_tool_wrapper.sh.in does not exist.
CMake Error at /home/alicebob/wkspc1/build/bitcoin/Covera
...
(https://github.com/bitcoin/bitcoin/issues/31638)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When following Compiling for Test Coverage instructions:
[doc/developer-notes-CompilingForTestCoverage](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage)
I encounter error:
```
CMake Error: File /home/alicebob/wkspc1/presets/bitcoin/cmake/cov_tool_wrapper.sh.in does not exist.
CMake Error at /home/alicebob/wkspc1/build/bitcoin/Covera
...
💬 jimhashhq commented on issue "cmake: Compiling for test coverage (low-priority workaround exists)":
(https://github.com/bitcoin/bitcoin/issues/31638#issuecomment-2584643838)
Assuming this is not an execution or environment issue on my side, something like the following one-line addition to the top level project CMakeLists.txt resolves this (though maybe this can be simplified):
$ git diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4b21646ca1..6753211ee5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -433,6 +433,7 @@ endif()
configure_file(cmake/script/Coverage.cmake Coverage.cmake USE_SOURCE_PERMISSIONS COPYONLY)
...
(https://github.com/bitcoin/bitcoin/issues/31638#issuecomment-2584643838)
Assuming this is not an execution or environment issue on my side, something like the following one-line addition to the top level project CMakeLists.txt resolves this (though maybe this can be simplified):
$ git diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4b21646ca1..6753211ee5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -433,6 +433,7 @@ endif()
configure_file(cmake/script/Coverage.cmake Coverage.cmake USE_SOURCE_PERMISSIONS COPYONLY)
...
💬 triptrapz2 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/723c7526368badda15df8ac1ffc047a0ab2e384a#r151201565)
613btc
(https://github.com/bitcoin/bitcoin/commit/723c7526368badda15df8ac1ffc047a0ab2e384a#r151201565)
613btc
⚠️ triptrapz2 opened an issue: "bc1qp89r98q2d45gfgu0428p780ljea47ny2vm2yu3"
(https://github.com/bitcoin/bitcoin/issues/31639)
> 613btc send to wallet listed above
_Originally posted by @triptrapz2 in [723c752](https://github.com/bitcoin/bitcoin/commit/723c7526368badda15df8ac1ffc047a0ab2e384a#r151201565)_
(https://github.com/bitcoin/bitcoin/issues/31639)
> 613btc send to wallet listed above
_Originally posted by @triptrapz2 in [723c752](https://github.com/bitcoin/bitcoin/commit/723c7526368badda15df8ac1ffc047a0ab2e384a#r151201565)_
✅ pinheadmz closed an issue: "bc1qp89r98q2d45gfgu0428p780ljea47ny2vm2yu3"
(https://github.com/bitcoin/bitcoin/issues/31639)
(https://github.com/bitcoin/bitcoin/issues/31639)
📝 EthanHeilman opened a pull request: "tests: improves tapscript unit tests"
(https://github.com/bitcoin/bitcoin/pull/31640)
This commit creates new test utilities for future Taproot script tests within script_tests.json. The key features of this commit are the addition of three new tags: `#SCRIPT#`, `#CONTROLBLOCK#`, and `#TAPROOTOUTPUT#`. These tags streamline the test creation process by eliminating the need to manually generate these components outside the test suite.
* `#SCRIPT#`: Parses Tapscript and outputs a byte string of opcodes.
* `#CONTROLBLOCK#`: Automatically generates the control block for a given T
...
(https://github.com/bitcoin/bitcoin/pull/31640)
This commit creates new test utilities for future Taproot script tests within script_tests.json. The key features of this commit are the addition of three new tags: `#SCRIPT#`, `#CONTROLBLOCK#`, and `#TAPROOTOUTPUT#`. These tags streamline the test creation process by eliminating the need to manually generate these components outside the test suite.
* `#SCRIPT#`: Parses Tapscript and outputs a byte string of opcodes.
* `#CONTROLBLOCK#`: Automatically generates the control block for a given T
...
💬 1440000bytes 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#issuecomment-2584992376)
> Currently, we do not add the sighash field to PSBTs at all, even when we have signed with a non-default sighash. **This PR changes the behavior such that when we (attempt to) sign with a sighash other than DEFAULT or ALL, the sighash type field will be added to the PSBT to inform the later signers that a different sighash type was used by a signer.** Notably, this is necessary for MuSig2 support as all signers must sign using the same sighash type, but the sighash is not provided in partial si
...
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2584992376)
> Currently, we do not add the sighash field to PSBTs at all, even when we have signed with a non-default sighash. **This PR changes the behavior such that when we (attempt to) sign with a sighash other than DEFAULT or ALL, the sighash type field will be added to the PSBT to inform the later signers that a different sighash type was used by a signer.** Notably, this is necessary for MuSig2 support as all signers must sign using the same sighash type, but the sighash is not provided in partial si
...
💬 scgbckbone 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_r1911868423)
is there any specific reason to allow SIGHASH_DEFAULT on non-taproot utxo? Shouldn't it be error instead of rewriting sighash in `sign.cpp`
```
// BASE/WITNESS_V0 signatures don't support explicit SIGHASH_DEFAULT, use SIGHASH_ALL instead.
const int hashtype = nHashType == SIGHASH_DEFAULT ? SIGHASH_ALL : nHashType;
```
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1911868423)
is there any specific reason to allow SIGHASH_DEFAULT on non-taproot utxo? Shouldn't it be error instead of rewriting sighash in `sign.cpp`
```
// BASE/WITNESS_V0 signatures don't support explicit SIGHASH_DEFAULT, use SIGHASH_ALL instead.
const int hashtype = nHashType == SIGHASH_DEFAULT ? SIGHASH_ALL : nHashType;
```
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31639)
(https://github.com/bitcoin/bitcoin/issues/31639)
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2585196535)
re https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583073982
> @ryanofsky wrote:
>
> > Possibly, it might make sense to build bitcoin-chainstate.exe and bitcoinkernel.dll to libexec/ instead of bin/.
>
> I like that as well. If we add them to the release later, they won't clutter `bin/`.
Moved to `libexec/`. cc @TheCharlatan
> Now is also a good time to move `bitcoin-util` there, because it's only used by our test code (and for calibrating a new signet).
Done.
r
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2585196535)
re https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583073982
> @ryanofsky wrote:
>
> > Possibly, it might make sense to build bitcoin-chainstate.exe and bitcoinkernel.dll to libexec/ instead of bin/.
>
> I like that as well. If we add them to the release later, they won't clutter `bin/`.
Moved to `libexec/`. cc @TheCharlatan
> Now is also a good time to move `bitcoin-util` there, because it's only used by our test code (and for calibrating a new signet).
Done.
r
...
💬 hodlinator commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2585199785)
> bitcoind -dbcache=30000 -stopatheight=878000 -blocksdir=/magnetic/.bitcoin -addnode=local-network
@Sjors, what kind of drive is `/magnetic/`?
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2585199785)
> bitcoind -dbcache=30000 -stopatheight=878000 -blocksdir=/magnetic/.bitcoin -addnode=local-network
@Sjors, what kind of drive is `/magnetic/`?
💬 hebasto commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2585200022)
The new code in `test/functional/wallet_encryption.py` [fails](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/12720574600/job/35462426268) on NetBSD:
```
230/313 - wallet_encryption.py --legacy-wallet failed, Duration: 8 s
stdout:
2025-01-11T03:37:44.955000Z TestFramework (INFO): PRNG seed is: 6983165365525497692
2025-01-11T03:37:44.956000Z TestFramework (INFO): Initializing test directory /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/test_runner_₿_🏃_20250111_
...
(https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2585200022)
The new code in `test/functional/wallet_encryption.py` [fails](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/12720574600/job/35462426268) on NetBSD:
```
230/313 - wallet_encryption.py --legacy-wallet failed, Duration: 8 s
stdout:
2025-01-11T03:37:44.955000Z TestFramework (INFO): PRNG seed is: 6983165365525497692
2025-01-11T03:37:44.956000Z TestFramework (INFO): Initializing test directory /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/test_runner_₿_🏃_20250111_
...
🤔 naiyoma reviewed a pull request: "rpc: print P2WSH witScript in getrawtransaction"
(https://github.com/bitcoin/bitcoin/pull/31252#pullrequestreview-2544716718)
I've tested https://github.com/bitcoin/bitcoin/pull/31252/commits/43dbc696b9e9bb5472b01708d33e022042b5ce3b on regtest, and both (P2SH redeemScript and P2WSH witnessScript )are only being decompiled when calling getblock … 2. I think you should consider editing the PR description to indicate exactly what is working so far.
(https://github.com/bitcoin/bitcoin/pull/31252#pullrequestreview-2544716718)
I've tested https://github.com/bitcoin/bitcoin/pull/31252/commits/43dbc696b9e9bb5472b01708d33e022042b5ce3b on regtest, and both (P2SH redeemScript and P2WSH witnessScript )are only being decompiled when calling getblock … 2. I think you should consider editing the PR description to indicate exactly what is working so far.
💬 l0rinc commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2585263747)
I think I managed to reproduce the ~2% difference - by not doing an IBD but a `-reindex-chainstate`.
@Sjors, was your datadir completely empty for the runs? My 12% comes from having nothing locally (e.g. no blocks) to being fully synced (i.e. has to include the final flush as well) - to be as close to the user's experience as possible.
<details>
<summary>Details</summary>
```
hyperfine \
--runs 2 \
--parameter-list COMMIT d73f37dda221835b5109ede1b84db2dc7c4b74a1,fe7365584bb3703e5691c
...
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2585263747)
I think I managed to reproduce the ~2% difference - by not doing an IBD but a `-reindex-chainstate`.
@Sjors, was your datadir completely empty for the runs? My 12% comes from having nothing locally (e.g. no blocks) to being fully synced (i.e. has to include the final flush as well) - to be as close to the user's experience as possible.
<details>
<summary>Details</summary>
```
hyperfine \
--runs 2 \
--parameter-list COMMIT d73f37dda221835b5109ede1b84db2dc7c4b74a1,fe7365584bb3703e5691c
...
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2585284602)
Thank you for the re-newed reviews and your suggestions @ryanofsky and @stickies-v. I hope we can settle on something this time round:
Updated 82706e217f34d1f09cbd30dde6b8ae5ac0385f0a -> 45fe603f8fd94c171f5ce1a38bc20d273baa9b1b ([kernel_cache_sizes_11](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_11) -> [kernel_cache_sizes_12](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_11.
...
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2585284602)
Thank you for the re-newed reviews and your suggestions @ryanofsky and @stickies-v. I hope we can settle on something this time round:
Updated 82706e217f34d1f09cbd30dde6b8ae5ac0385f0a -> 45fe603f8fd94c171f5ce1a38bc20d273baa9b1b ([kernel_cache_sizes_11](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_11) -> [kernel_cache_sizes_12](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_11.
...
📝 satscoffee opened a pull request: "Comment coypright date update"
(https://github.com/bitcoin/bitcoin/pull/31642)
doc: updated comments in line 1 from:
// Copyright (c) 2012-2022 The Bitcoin Core developers
to
// Copyright (c) 2012-2025 The Bitcoin Core developers
PR was for comment update only so no testing was performed.
(https://github.com/bitcoin/bitcoin/pull/31642)
doc: updated comments in line 1 from:
// Copyright (c) 2012-2022 The Bitcoin Core developers
to
// Copyright (c) 2012-2025 The Bitcoin Core developers
PR was for comment update only so no testing was performed.
💬 instagibbs commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2585300460)
is this in draft for a reason?
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2585300460)
is this in draft for a reason?
💬 NicolaLS commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2585301823)
I agree that the previous version or this PR did not make sense for clearing up the confusion. I force pushed a new approach and amended the commit message / PR description.
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2585301823)
I agree that the previous version or this PR did not make sense for clearing up the confusion. I force pushed a new approach and amended the commit message / PR description.
✅ fanquake closed a pull request: "Comment coypright date update"
(https://github.com/bitcoin/bitcoin/pull/31642)
(https://github.com/bitcoin/bitcoin/pull/31642)
👍 tdb3 approved a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2545082647)
code review re ACK 58b828f7f7ad216c7d7bf2e2ff431a66591e9d5c
The data file is down to 71K, which is much nicer!
`mining_mainnet.py` ran locally without issue.
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2545082647)
code review re ACK 58b828f7f7ad216c7d7bf2e2ff431a66591e9d5c
The data file is down to 71K, which is much nicer!
`mining_mainnet.py` ran locally without issue.