💬 sipa commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-2706846092)
@tnndbtc I don't understand why there is a difference at all in performance between the two approaches for 999-of-999; it needs 999 signing attempts regardless.
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-2706846092)
@tnndbtc I don't understand why there is a difference at all in performance between the two approaches for 999-of-999; it needs 999 signing attempts regardless.
💬 achow101 commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985347658)
This pattern is specifically not generalized because we want to restrict to at least post-segwit versions, hence beginning at 0.14.0.
Furthermore, while the tag is similar to the user agent, they do not match exactly. Specifically, since the version style change in 22.0, the tag is typically MAJOR.MINOR, but the user agent is always MAJOR.MINOR.PATCH. So 22.0 is 22.0.0 in the user agent. Release candidates additionally produce user agents without the rc suffix. Hence your test script is not r
...
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985347658)
This pattern is specifically not generalized because we want to restrict to at least post-segwit versions, hence beginning at 0.14.0.
Furthermore, while the tag is similar to the user agent, they do not match exactly. Specifically, since the version style change in 22.0, the tag is typically MAJOR.MINOR, but the user agent is always MAJOR.MINOR.PATCH. So 22.0 is 22.0.0 in the user agent. Release candidates additionally produce user agents without the rc suffix. Hence your test script is not r
...
📝 hebasto opened a pull request: "cmake: Check for `makensis` tool before using it"
(https://github.com/bitcoin/bitcoin/pull/32019)
Fixes https://github.com/bitcoin/bitcoin/issues/32018.
(https://github.com/bitcoin/bitcoin/pull/32019)
Fixes https://github.com/bitcoin/bitcoin/issues/32018.
💬 achow101 commented on issue "Check if the wallet already contains the descriptor GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/issues/32013#issuecomment-2706880541)
If you believe you have a useful change, please just open a PR. There's no need to ask permission to do so, and doing code review in an issue is painful.
(https://github.com/bitcoin/bitcoin/issues/32013#issuecomment-2706880541)
If you believe you have a useful change, please just open a PR. There's no need to ask permission to do so, and doing code review in an issue is painful.
💬 hebasto commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985358929)
As I mentioned [above](https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2662658024), the `cmake --install` command does support the `--prefix` option.
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985358929)
As I mentioned [above](https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2662658024), the `cmake --install` command does support the `--prefix` option.
💬 l0rinc commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985359617)
Thank you for the correction!
Would it make sense to use the generalized, simple regex for extracting the version only and match that against a pre-specified whitelist of valid versions instead?
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985359617)
Thank you for the correction!
Would it make sense to use the generalized, simple regex for extracting the version only and match that against a pre-specified whitelist of valid versions instead?
💬 achow101 commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985372517)
> Would it make sense to use the generalized, simple regex for extracting the version only and match that against a pre-specified whitelist of valid versions instead?
I don't see why that would be better than this approach.
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985372517)
> Would it make sense to use the generalized, simple regex for extracting the version only and match that against a pre-specified whitelist of valid versions instead?
I don't see why that would be better than this approach.
🤔 darosior reviewed a pull request: "kernel: pre-29.x chainparams and headerssync update"
(https://github.com/bitcoin/bitcoin/pull/31978#pullrequestreview-2667820843)
post-merge ACK 11a2d3a63e90cdc1920ede3c67d52a9c72860e6b
Checked all the parameters but the headersync. Performed an `-assumevalid=0` sync on mainnet.
(https://github.com/bitcoin/bitcoin/pull/31978#pullrequestreview-2667820843)
post-merge ACK 11a2d3a63e90cdc1920ede3c67d52a9c72860e6b
Checked all the parameters but the headersync. Performed an `-assumevalid=0` sync on mainnet.
💬 jonatack commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985403649)
> This pattern is specifically not generalized because we want to restrict to at least post-segwit versions, hence beginning at 0.14.0. We might even want to go further and restrict to post-taproot versions, although the benefit there is much less significant.
>
> Furthermore, while the tag is similar to the user agent, they do not match exactly. Specifically, since the version style change in 22.0, the tag is typically MAJOR.MINOR, but the user agent is always MAJOR.MINOR.PATCH. So 22.0 is 2
...
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985403649)
> This pattern is specifically not generalized because we want to restrict to at least post-segwit versions, hence beginning at 0.14.0. We might even want to go further and restrict to post-taproot versions, although the benefit there is much less significant.
>
> Furthermore, while the tag is similar to the user agent, they do not match exactly. Specifically, since the version style change in 22.0, the tag is typically MAJOR.MINOR, but the user agent is always MAJOR.MINOR.PATCH. So 22.0 is 2
...
💬 fanquake commented on pull request "leveldb: pull upstream C++23 changes":
(https://github.com/bitcoin/bitcoin/pull/31766#issuecomment-2706979978)
Updated now that the changes have landed upstream in https://github.com/bitcoin-core/leveldb-subtree/pull/47.
(https://github.com/bitcoin/bitcoin/pull/31766#issuecomment-2706979978)
Updated now that the changes have landed upstream in https://github.com/bitcoin-core/leveldb-subtree/pull/47.
👋 fanquake's pull request is ready for review: "leveldb: pull upstream C++23 changes"
(https://github.com/bitcoin/bitcoin/pull/31766)
(https://github.com/bitcoin/bitcoin/pull/31766)
💬 hebasto commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2706990912)
> This seems like something that needs to be solved in CMake itself.
Asked here: https://discourse.cmake.org/t/static-pie-is-incompatible-with-checkpiesupported/13696
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2706990912)
> This seems like something that needs to be solved in CMake itself.
Asked here: https://discourse.cmake.org/t/static-pie-is-incompatible-with-checkpiesupported/13696
💬 fanquake commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985423823)
Ok, will just back this up.
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985423823)
Ok, will just back this up.
🔓 fanquake unlocked an issue: ""missing operand" assembler warnings on Windows"
(https://github.com/bitcoin/bitcoin/issues/28111)
(https://github.com/bitcoin/bitcoin/issues/28111)
💬 purpleKarrot commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2707270259)
As @hebasto pointed out, the `find_package()` command creates cache variables **when it succeeds**. I think this is the behaviour we want for `TryAppendLinkerFlag.cmake`too, which means the cache variable should be dropped on failure.
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2707270259)
As @hebasto pointed out, the `find_package()` command creates cache variables **when it succeeds**. I think this is the behaviour we want for `TryAppendLinkerFlag.cmake`too, which means the cache variable should be dropped on failure.
💬 jonatack commented on pull request "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985638247)
Here is a python script that sanity checks the regex in this PR. Only the last entry "24.1rc1" doesn't match. Changing entries to, say, 24.3.0 also don't match.
<details><summary>regex-test.py</summary><p>
```python3
#!/usr/bin/env python3
import re
TAGS = """
0.14.0
0.14.1
0.14.2
0.14.3
0.15.0
0.15.0.1
0.15.1
0.16.0
0.16.1
0.16.2
0.16.3
0.15.2
0.14.3
0.17.0
0.17.0.1
0.17.1
0.18.0
0.18.1
0.17.2
0.19.0
0.19.0.1
0.19.1
0.20.0
0.20.1
0.19.2
0.21.0
0.21.1
22.0.
...
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1985638247)
Here is a python script that sanity checks the regex in this PR. Only the last entry "24.1rc1" doesn't match. Changing entries to, say, 24.3.0 also don't match.
<details><summary>regex-test.py</summary><p>
```python3
#!/usr/bin/env python3
import re
TAGS = """
0.14.0
0.14.1
0.14.2
0.14.3
0.15.0
0.15.0.1
0.15.1
0.16.0
0.16.1
0.16.2
0.16.3
0.15.2
0.14.3
0.17.0
0.17.0.1
0.17.1
0.18.0
0.18.1
0.17.2
0.19.0
0.19.0.1
0.19.1
0.20.0
0.20.1
0.19.2
0.21.0
0.21.1
22.0.
...
💬 hodlinator commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985735508)
Done in latest push.
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985735508)
Done in latest push.
💬 hodlinator commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985710679)
At 081ae231ff542c254924a1770c9db666aff9f880^:
```shell
₿ hyperfine -w 1 "build/test/functional/rpc_rawtransaction.py --legacy-wallet"
Benchmark 1: build/test/functional/rpc_rawtransaction.py --legacy-wallet
Time (mean ± σ): 4.776 s ± 0.025 s [User: 1.420 s, System: 0.267 s]
Range (min … max): 4.717 s … 4.811 s 10 runs
```
At 081ae231ff542c254924a1770c9db666aff9f880:
```shell
₿ hyperfine -w 1 "build/test/functional/rpc_rawtransaction.py --legacy-wallet"
Benchmark 1:
...
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985710679)
At 081ae231ff542c254924a1770c9db666aff9f880^:
```shell
₿ hyperfine -w 1 "build/test/functional/rpc_rawtransaction.py --legacy-wallet"
Benchmark 1: build/test/functional/rpc_rawtransaction.py --legacy-wallet
Time (mean ± σ): 4.776 s ± 0.025 s [User: 1.420 s, System: 0.267 s]
Range (min … max): 4.717 s … 4.811 s 10 runs
```
At 081ae231ff542c254924a1770c9db666aff9f880:
```shell
₿ hyperfine -w 1 "build/test/functional/rpc_rawtransaction.py --legacy-wallet"
Benchmark 1:
...
💬 hodlinator commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985711620)
At 081ae231ff542c254924a1770c9db666aff9f880^:
```shell
₿ hyperfine -w 1 "build/test/functional/wallet_avoid_mixing_output_types.py --descriptors"
Benchmark 1: build/test/functional/wallet_avoid_mixing_output_types.py --descriptors
Time (mean ± σ): 1.581 s ± 0.588 s [User: 0.610 s, System: 0.113 s]
Range (min … max): 0.927 s … 2.288 s 10 runs
```
At 081ae231ff542c254924a1770c9db666aff9f880:
```shell
₿ hyperfine -w 1 "build/test/functional/wallet_avoid_mixing_output_
...
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985711620)
At 081ae231ff542c254924a1770c9db666aff9f880^:
```shell
₿ hyperfine -w 1 "build/test/functional/wallet_avoid_mixing_output_types.py --descriptors"
Benchmark 1: build/test/functional/wallet_avoid_mixing_output_types.py --descriptors
Time (mean ± σ): 1.581 s ± 0.588 s [User: 0.610 s, System: 0.113 s]
Range (min … max): 0.927 s … 2.288 s 10 runs
```
At 081ae231ff542c254924a1770c9db666aff9f880:
```shell
₿ hyperfine -w 1 "build/test/functional/wallet_avoid_mixing_output_
...
💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1985753579)
For non-testnets, the 2016 and 1815 figures WarningBitsConditionChecker in versionbits.cpp (by the end of this PR) are constrained by what we might expect to see in future from consensus change coordination signalling; while that currently matches DifficultyAdjustmentInterval, it doesn't need to -- for instance BIP 91 had a window of 336 blocks, and I think in practice would not have (did not) trigger this warning code. So I think it makes sense to leave them as constants, though perhaps some in
...
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1985753579)
For non-testnets, the 2016 and 1815 figures WarningBitsConditionChecker in versionbits.cpp (by the end of this PR) are constrained by what we might expect to see in future from consensus change coordination signalling; while that currently matches DifficultyAdjustmentInterval, it doesn't need to -- for instance BIP 91 had a window of 336 blocks, and I think in practice would not have (did not) trigger this warning code. So I think it makes sense to leave them as constants, though perhaps some in
...