💬 fanquake commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1985295727)
Shouldn't we be using CMake options now that we are using CMake?

🤔 fjahr reviewed a pull request: "qa: Fix TxIndex race conditions"
(https://github.com/bitcoin/bitcoin/pull/32010#pullrequestreview-2667572104)
Concept ACK

💬 fjahr commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985242659)
Did you check if this has an performance impact and maybe that's why they were added here?

💬 fjahr commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985310423)
Can this be a bit more clear what this means? I guess "Removing these files does not result in a startup error"? Would be great if you could spell that out because I needed a moment to understand. You could also put the commented out line below and this one together and above the variable declaration. I am not sure if having them in line and split up helps with understanding. Same below.

💬 fjahr commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#discussion_r1985243645)
Same as above, may check for performance impact on this test.

💬 tnndbtc commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-2706831776)
> That sounds very plausible, [@darosior](https://github.com/darosior).
>
> If so, I think there is an approach that doesn't lose optimality: keep track of the optimal satisfaction size, and once reached, don't bother evaluation further keys?

@sipa I agree with you and @darosior that this suggestion would gain performance on many cases. However, in proposed unit test in https://github.com/bitcoin/bitcoin/pull/28212

`self.do_test_k_of_n(999, 999)`

This test would still timeout in most of mac
...

💬 jonatack commented on pull request "Add mainnet assumeutxo param at height 880,000":
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2706832777)
> Concept ACK, shasum of the torrent file matches up and will re-test soon after further reindexing. For now am seeing this:
>
> ```
> error code: -32603
> error message:
> Unable to load UTXO snapshot: The base block header (000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again. (utxo-880000.dat)
> ```

Successfully activated the snapshot, once the node had the headers.

``
2
...

💬 tnndbtc commented on pull request "miniscript: fixes #29098 by only use first k valid signatures":
(https://github.com/bitcoin/bitcoin/pull/31719#issuecomment-2706836700)
@sipa Yes, I considered your proposal and it would still make the proposed test to timeout.
`self.do_test_k_of_n(999, 999)`

I just responded your comment in the original thread [here](https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-2706831776)

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

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

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

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

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

💬 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