💬 maflcko commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#discussion_r2309807425)

"Aborting generating manpages..." -> "Aborting generation of manpages..." [corrects ungrammatical gerund usage]

💬 maflcko commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#discussion_r2309816260)
in theory, `--skip-missing-binaries` can now be removed and the list of binaries derived from the build options.

💬 maflcko commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#discussion_r2309824605)
what is the point of listing those, which are unused?

💬 maflcko commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2309866024)
```suggestion
/** Guess background verification progress in case assume-utxo was used (as a fraction between 0.0=genesis and 1.0=snapshot blocks). */
double GetBackgroundVerificationProgress(const CBlockIndex& block) const EXCLUSIVE_LOCKS_REQUIRED(GetMutex());
```

can remove the unused nullptr handling here, as all call-sites already dereferenced the pointer

💬 maflcko commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2309861761)

"the heigh of the snapshot block" -> "the height of the snapshot block" [spelling error: "heigh" should be "height"]

💬 frankomosh commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3236657195)
Updated `fuzz/difference_formatter.cpp` to directly use `BlockTransactionsRequest`

👍 hodlinator approved a pull request: "rpc: Distinguish between vsize and sigop adjusted mempool vsize"
(https://github.com/bitcoin/bitcoin/pull/32800#pullrequestreview-3168171370)
ACK 74530ac9e05aafa91aaa00cc4f53bb4f26a55153

Clarifies the distinction between sigop-adjusted and non-sigop-adjusted vsize for RPC users.

Minor fixups and corrections since my last review https://github.com/bitcoin/bitcoin/pull/32800#pullrequestreview-3081014156

💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2309759951)
info: Verified that Bitcoin Core seems to use sigop-adjusted size for block template construction as it sounds like that in this part. IIUC, we do, going by:

1. Checking the block is full with `packageSize`...
https://github.com/bitcoin/bitcoin/blob/ca9c5219a4029901db6f49678620d1935bf1634f/src/node/miner.cpp#L209-L214
...which comes from `GetSizeWithAncestors()`...
https://github.com/bitcoin/bitcoin/blob/ca9c5219a4029901db6f49678620d1935bf1634f/src/node/miner.cpp#L376-L390

...

💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2309887787)
nit: I think this part is repetitive and somewhat arbitrary, currently the PR ensures sigop-adjusted and non-sigop-adjusted sizes are available for all mentioned RPCs. If I was using RPCs for each transaction for this calculation I would just pick the fastest one. Maybe I'm overlooking something though.
```suggestion
```

💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2309870327)
nit: The adjusted beginning is nicer, but would rather bring back the end from 1523e8d6c981efff1394bd1669fbbbea5ff7ac97:
> Additionally, `getrawtransaction` now includes
+a `sigopsize` field and continues to report the unadjusted BIP-141 virtual size in its `vsize` field.

💬 jsarenik commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3236825770)
After it gets merged into `master` branch, I would recommend backporting it to current stable release branch which already contains the ephemeral outputs support ([29.x](https://github.com/bitcoin/bitcoin/blob/7cc9a087069bfcdb79a08ce77eb3a60adf9483af/doc/release-notes/release-notes-29.0.md?plain=1#L64}).

💬 jsarenik commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3236886747)
Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?

💬 Crypt-iQ commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3236937041)
tACK af4156ab75565acc5a71b68e70da5e2cf407df80

I ran into build failures while trying to measure fuzz coverage.

💬 instagibbs commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2309954963)
> which is necessary for marking anchor outputs as spent.

To be pedantic, it's not about PayToAnchor(P2A), which can be any value, but any 0-value outputs, even if they weren't ephemeral (say a miner mined it to dust your wallet).

There are other legitimate 0-value output use-cases such as connector outputs, so let's just call it 0-value dust?

💬 instagibbs commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2310110919)
this subtest is covering 0-value outputs, not anchors per se. I'd rather keep the tests logically separated to not confused the concepts more than they already are :)

💬 instagibbs commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3236975180)
@jsarenik technically this issue could happen if miners mine 0-value outputs of any kind, but given that it's now relay-possible, seems like a good idea to backport

💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3236980424)
Rebased over #33253 but not optimised to iterate over template txs efficiently.

🤔 stickies-v reviewed a pull request: "rpc: refactor: use string_view in Arg/MaybeArg"
(https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3168699729)
Force-pushed to address review comments from @maflcko:
- fixed docstring
- removed usage of std::string_view::data() to avoid null termination issues in the future
- added one more usage of Arg<string_view> helper

💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2310136874)
DELETE

💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2310142429)
Done!