💬 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?

💬 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.

🤔 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.

💬 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
...

💬 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.

👋 fanquake's pull request is ready for review: "leveldb: pull upstream C++23 changes"
(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

💬 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.

🔓 fanquake unlocked an issue: ""missing operand" assembler warnings on Windows"
(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.

💬 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.
...

💬 hodlinator commented on pull request "qa: Fix TxIndex race conditions":
(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:
...

💬 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_
...

💬 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
...

🤔 mzumsande reviewed a pull request: "Ensure assumevalid is always used during reindex"
(https://github.com/bitcoin/bitcoin/pull/31615#pullrequestreview-2668456532)
Concept ACK

I think the current approach would work.
Even though it means that if the user provided a bad / non-existing assumeutxo hash they could skip script validation during a reindex when they wouldn't have in the original run, I'd say that's not a major problem because a reindex should only be necessary in case of a a db corruption, which should not be remotely triggerable.

So it will hopefully make the reindex a bit faster in case of a database corruption, while not changing the
...

💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1985755797)
This is just clearing those members because they don't make sense in the LOCKED_IN state. The earlier line `result.current_state = StateName(current_state)` is what's reporting the state. Make sense?

👍 hodlinator approved a pull request: "doc: add note to Windows build about stripping bins"
(https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2668463615)
ACK c94195c077ff227e5e2d80e803e1400d7f60812b

Tested on WSL

Without `--strip`:
```
27 998 527 /mnt/c/workspace/bitcoin/bin/bitcoin-cli.exe
587 584 175 /mnt/c/workspace/bitcoin/bin/bitcoin-qt.exe
59 799 144 /mnt/c/workspace/bitcoin/bin/bitcoin-tx.exe
27 408 020 /mnt/c/workspace/bitcoin/bin/bitcoin-util.exe
226 141 145 /mnt/c/workspace/bitcoin/bin/bitcoin-wallet.exe
467 648 497 /mnt/c/workspace/bitcoin/bin/bitcoind.exe
642 186 589 /mnt/c/workspace/bitcoin/bin/test_bitcoin-qt.exe
9
...

💬 hodlinator commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985759190)
nit: Phantom newline.

💬 achow101 commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2707537453)
ACK ac6cde731314d981391bc313c98d671c68211d33