💬 instagibbs commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#discussion_r2010150220)
I find that particular code block really tucked away and hard to find fwiw.

💬 Sjors commented on pull request "OP_CHECKCONTRACTVERIFY":
(https://github.com/bitcoin/bitcoin/pull/32080#discussion_r2010152687)
```diff
diff --git a/test/functional/feature_checkcontractverify_vaults.py b/test/functional/feature_checkcontractverify_vaults.py
index 37e0bbd39f..60871850c1 100755
--- a/test/functional/feature_checkcontractverify_vaults.py
+++ b/test/functional/feature_checkcontractverify_vaults.py
@@ -47,19 +47,16 @@ class Vault(P2TR):
- with the "trigger" clause, sending the entire amount to an Unvaulting output, after providing a 'withdrawal_pk'
- with the "trigger_and_revault" clause, se
...

👍 hodlinator approved a pull request: "doc: clarify the documentation of `Assume` assertion"
(https://github.com/bitcoin/bitcoin/pull/32100#pullrequestreview-2710323053)
ACK a7c65edc884b0e22aaabd7e725f5f39e60b6e76b

Adds more information on the strengths of `Assume()`, being able to be used as a kind of low-cost documentation of constraints that in the best case can be optimized away in production.

Latest version of the change also keeps with the pattern of only having one `- For example ...`-subsection.

💬 hodlinator commented on pull request "doc: clarify the documentation of `Assume` assertion":
(https://github.com/bitcoin/bitcoin/pull/32100#discussion_r2010147577)
Thank you for taking my suggestion!

💬 instagibbs commented on pull request "doc: Update comments for AreInputsStandard to match code":
(https://github.com/bitcoin/bitcoin/pull/32129#discussion_r2010168332)
It's still filtering other types of non-standard scripts, which also protects against the same issue of lots of operations (in addition to offering upgrade hooks)

💬 sipa commented on pull request "doc: clarify the documentation of `Assume` assertion":
(https://github.com/bitcoin/bitcoin/pull/32100#issuecomment-2748126948)
ACK a7c65edc884b0e22aaabd7e725f5f39e60b6e76b

💬 rkrux commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#discussion_r2010180903)
True. I mentioned it because one would end up looking at it once they see `make_spender`. This place would be more useful if the `Spender` type was also used more in the test such as while initialising the `spenders` list.

I find right at the top a suitable alternative as well though I missed this section entirely the first time I read the test: https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_taproot.py#L121

Fine either way.

💬 brunoerg commented on pull request "fuzz: wallet: fix crypter target":
(https://github.com/bitcoin/bitcoin/pull/32118#issuecomment-2748136281)
> Would be nice to add a short line on how to test/observe this fix

I would say that simply checking the coverage would be enough. Note that https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/crypter.cpp.gcov.html L143 and L144 are unreachable and probably never be reach with the current harness.

💬 sipa commented on pull request "fuzz: Fix off-by-one in package_rbf target":
(https://github.com/bitcoin/bitcoin/pull/32122#discussion_r2010185289)
Nit: I find it a bit unintuitive to use a loop variable inside the condition here, and would prefer:

```c++
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS) {
if (iter >= NUM_ITERS) break;
...
```

💬 fjahr commented on pull request "[EXPERIMENTAL] Schnorr batch verification for blocks":
(https://github.com/bitcoin/bitcoin/pull/29491#issuecomment-2748140699)
I've also rebased to get #31689 in here by the way.

👍 rkrux approved a pull request: "doc: clarify the documentation of `Assume` assertion"
(https://github.com/bitcoin/bitcoin/pull/32100#pullrequestreview-2710393754)
ACK a7c65edc884b0e22aaabd7e725f5f39e60b6e76b

💬 rkrux commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r2010188842)
Hmm interesting, `Assume` is growing on me.

👍 hebasto approved a pull request: "build: Drop option to disable hardening."
(https://github.com/bitcoin/bitcoin/pull/32071#pullrequestreview-2710390665)
ACK 0e98ec819b65cfc1728a6aecf5e43a7c73756a66, I have reviewed the code and it looks OK.

Here are NetBSD CI jobs for this branch: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14036068740.

💬 hebasto commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#discussion_r2010184304)
style nit: two-spaces indent.

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2748190847)
My Guix build:
```
aarch64
27ee7eaf88b360cf203947bb4d2f61029e01ed2836545b877314f2d029a998dd guix-build-3bf637c693a5/output/aarch64-linux-gnu/SHA256SUMS.part
dd4541b9fa668f3ca739e25d8d696125f850307ef9d97e7ca24aa38b9de1503d guix-build-3bf637c693a5/output/aarch64-linux-gnu/bitcoin-3bf637c693a5-aarch64-linux-gnu-debug.tar.gz
20be09dc09e0ae929874cc45ae2222a75f3c256236b2c03f08ee6722df988bff guix-build-3bf637c693a5/output/aarch64-linux-gnu/bitcoin-3bf637c693a5-aarch64-linux-gnu.tar.gz
effdb607
...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2748194105)
Please let me know if I haven’t addressed any of your feedback.

💬 darosior commented on pull request "doc: Update comments for AreInputsStandard to match code":
(https://github.com/bitcoin/bitcoin/pull/32129#issuecomment-2748214422)
Concept ACK

💬 maflcko commented on pull request "fuzz: Fix off-by-one in package_rbf target":
(https://github.com/bitcoin/bitcoin/pull/32122#discussion_r2010234399)
If the limit check is split up into a manual break, I'd prefer to switch to a plain while loop, but no strong opinion.

💬 rkrux commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#discussion_r2010241999)
> Open for suggestions how we can make it more welcoming beyond this.

One point I want to mention is that initially it was not evident to me that a spender is a test case in itself until I saw `err_msg` being a part of it. Technically this is mentioned in the file albeit in a buried location: https://github.com/bitcoin/bitcoin/blob/770d39a37652d40885533fecce37e9f71cc0d051/test/functional/feature_taproot.py#L1330

A more elaborate name like `spender_test_case` or `spender_tc` would be useful
...

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2010256485)
Done.