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

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

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2010257849)
I've replaced "maintains" with "defines". As far as the interface is concerned, what the actual implementation does in this regard is irrelevant.

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

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2010260015)
I believe the plan is to basically run it at every "stable" point in time, so after any transaction or block processing.

💬 l0rinc commented on pull request "doc: clarify the documentation of `Assume` assertion":
(https://github.com/bitcoin/bitcoin/pull/32100#discussion_r2010256634)
According to https://www.reddit.com/r/cpp_questions/comments/plg8ij/expression_vs_statement/
> Statements do not have a return type

So by definition, unless they're empty, they cannot be `side-effect-free` - if my interpretation is correct.
```suggestion
expression is always evaluated. However, if the compiler can prove that
an expression inside `Assume` is side-effect-free, it may optimize the call away,
```

💬 instagibbs commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#issuecomment-2748280390)
@rkrux pushed an update with another related direction. Ideally a new contributor can add a set of spenders under a new function, aggregate to the list, without having to touch the current walls of current spender making functions?

Let me know if I'm getting warmer.

🤔 rkrux reviewed a pull request: "rpc: Support v3 raw transactions creation"
(https://github.com/bitcoin/bitcoin/pull/31936#pullrequestreview-2710562125)
Answering open comments.

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2010279314)
Interesting, I like these points quite a lot. Summarising below, please correct me if I misunderstand:

**We can avoid using defaults here completely by letting the corresponding `version` default trickle down from the RPC help into these functions here.**

As I checked in the call sites of `CreateTransaction`, this^ requires other transaction creation RPCs to accept a version param as well such as `createpsbt`, `walletcreatefundedpsbt`, `send`, `sendall`, which seems consistent with an earl
...