🤔 ajtowns requested changes to a pull request: "RPC/txoutproof: Support including (and verifying) proofs of wtxid"
(https://github.com/bitcoin/bitcoin/pull/32844#pullrequestreview-3400680165)
Transaction proofs are an interoperability feature, I don't think it makes much sense to implement this without also documenting it and providing test vectors via a BIP.

💬 ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478870174)
I think this should probably be automatic, with the current "array of txids" result being stuck behind a `-deprecatedrpc` option.

💬 ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478893275)
Being able to easily prove/verify that a tx was included in a reorged block seems fine?

💬 ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478855640)
There is no such subclass?

I think all this logic should entirely be in a separate class though. For efficiency probably two paths:

* if the generation tx has no witness commitment; provde all (w)txids and the generation tx via the block partial merkle tree.
* if the generation tx does have a witness commitment; provide just the generation tx's merkle path back to the block header, and prove all wtxids via a partial merkle tree back to the witness commitment

💬 ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478889737)
I would have expected this logic to be in merkleblock.cpp as a standard part of verifying a proof (eg so that it could be included in kernel), not in the RPC code.

💬 ajtowns commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2478880286)
This information (height, confirmations, assumeutxo info) seems better obtained via `getblockheader` on the blockhash to me.

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478901830)
fixed those.

✅ ismaelsadeeq closed a pull request: "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity"
(https://github.com/bitcoin/bitcoin/pull/32395)

💬 ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-3469165154)
Closing this due to lack of interest.

💬 ismaelsadeeq commented on issue "Add a script for Signet or Regtest to unlock fee estimator":
(https://github.com/bitcoin/bitcoin/issues/32105#issuecomment-3469218336)
Reviewers of #32395 can also share weigh input here so we don’t keep this open unnecessarily.
What @yellowred suggested can be handled by the client that actually needs this functionality, rather than us having to write and maintain this script in our repository.

I took a pragmatic approach, but it received little to mixed support (~0 and +1), so it seems the general sentiment is that we don’t want to enable this. Based on the feedback from my previous (now closed) PR, this can be closed as w
...

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2478967921)
I took a stab at it and it's a bit tedious (I managed to move it up earlier in the commit history, but stopped when I got to a conflict with a commit that also touches truc_policy in different ways).

My overall take is it's not such a big deal, as the concern is not around correctness of the code but a potential CPU DoS if exposed over the network, which doesn't strike me as all that concerning for an intermediate commit.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3469378663)
Latest push ed813c48f826d083becf93c741b483774c850c86 -> f2ce362:
- implements `GetStrongRandBytes` per [comment](https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2410849029)
- modifies `create_tx` to choose a mempool UTXO only sometimes instead of most of the time
- modifies `create_block` to generate more than 2 non-coinbase transactions per feedback from review club

📝 hulxv opened a pull request: "feat: shell completions for bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/33747)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 frankomosh commented on pull request "doc: reference fuzz coverage steps in quick-start":
(https://github.com/bitcoin/bitcoin/pull/33536#issuecomment-3469389865)




> @frankomosh the PR description needs updating to reflect the current change?
>
> cc @dergoegge @marcofleon

yeah indeed

🤔 fjahr reviewed a pull request: "RPC/txoutproof: Support including (and verifying) proofs of wtxid"
(https://github.com/bitcoin/bitcoin/pull/32844#pullrequestreview-3400931461)
Concept ACK

Is there something special about the naming "generation tx" here? It seems to be just an older alias for the coinbase transaction without any other special meaning. I can not recall seeing it used anywhere in the code base or online discourse. If I am not overlooking something it would probably be better to just call it coinbase tx instead which should be less confusing to newer contributors that haven't heard this term before.

💬 fjahr commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2479023056)
Should also cover an invalid proof here

💬 fjahr commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2479054592)
I think that makes sense and we could even do the same for `gettxoutproof` I think (if that wasn't already intended with the comment as well).

💬 davidgumberg commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2479106793)
The comment here could at least be changed to something like:

```suggestion
* @returns if the block was processed, does not necessarily indicate validity.
```

💬 pinheadmz commented on issue "Add a script for Signet or Regtest to unlock fee estimator":
(https://github.com/bitcoin/bitcoin/issues/32105#issuecomment-3469611314)
Agree (to close the issue), I have several scripts like this for different use cases anyway. Thanks for putting in the work! This question will definitely come up again and the closed PR can justify a "wont fix"

💬 marcofleon commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3469642268)
I've been playing around with the fuzz test and wanted to update here, even if it's still a bit inconclusive.

I'm running into non-reproducible timeouts when using `fork` with libFuzzer. I'm overusing my cores by a lot (`fork=25` plus the 3 workers each), and I'm not yet sure if these timeouts would happen eventually when using less cores.

I have "fixed" this by changing `notify_one` in `Submit` to `notify_all`, although I can't say I'm exactly sure why this works. It almost seems like so
...