💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2515659830)
I think @glozow's approach is definitely the right direction to go, but let's defer making that change to a future PR so that it can get more focused review, since it's not trivial to reason about how all this currently works. I'm taking @sipa's suggestion for now and will re-push.

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2515672555)
I took this change in #33591.

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3518690143)
I really appreciate the feedback and reviews so far from @ryanofsky, @Sjors, and @brunoerg.
Given that I’ve opened #33758 with the overall plan and we’ve had some recent discussions https://github.com/bitcoin/bitcoin/issues/33756#issuecomment-3502079264, I’ll work on fixing the C. I issue here hence marking as draft.

I will also work on the e2e implementation separately.

📝 ismaelsadeeq converted_to_draft a pull request: "node: add `BlockTemplateCache`"
(https://github.com/bitcoin/bitcoin/pull/33421)
This PR implements the first step of #33758.

This PR Implement a cache layer. All newly created block templates should be stored along with their respective configuration options. Client requests for block templates will specify the maximum age of the template in seconds; that is, how fresh they want the template to be:

If a template with matching options exists in the cache and the interval has not elapsed, a cached template is returned.
If no template exists or the interval has elapsed,
...

🤔 glozow reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3449740145)
Small batch of RPC comments

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2515530268)
in ca18549ee72: Perhaps set the expectation that there could be interface changes.
```suggestion
{"hidden", &getmempoolfeeratediagram},
```

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2515774000)
in ca18549ee7288f16f7c93ef9f9cac932d29a0eba

(Happy to make this a followup, because there are other RPCs that need this change too)

```suggestion
RPCResult{RPCResult::Type::NUM, "vsize", "Sigops-adjusted virtual transaction size: the maximum of BIP 141 virtual size and the transaction's sigops multiplied by -bytespersigop."
```

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2515779737)
nit, also happy to put in followup

```suggestion
{RPCResult::Type::NUM, "vsize", "cumulative sigops-adjusted vsize"},
{RPCResult::Type::NUM, "fee", "cumulative modified fee"}
```

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2515703456)
in ca18549ee72: Maybe creeped in during a rebase? This string is already on another line.

💬 ajtowns commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3518709899)
> Why doesn't `tx.get_vsize()` return the correct value in the code above. Did I make a stupid mistake somewhere?

The vsize that `check_mempool_result()` (aka `testmempoolaccept`) returns is adjusted for the sigop count (which is treated as 20 due to `fAccurate=false` via `GetTransactionSigOpCost()`/`GetLegacySigOpCount()`), while `get_vsize` is not. See also #32800.

💬 TheCharlatan commented on pull request "kernel: add btck_block_tree_entry_equals":
(https://github.com/bitcoin/bitcoin/pull/33855#discussion_r2515827417)
This just checks pointer equality. Do we really need a separate function for that? Maybe we can just handle this on the C++ wrapper layer?

💬 tobtoht commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3518761170)
Guix Build:

```
x86_64
451630ecff800ab320a9d5ad8062758df9d700310690110e764c916ae4c8121e guix-build-527acc5ee497/output/dist-archive/bitcoin-527acc5ee497.tar.gz
bf19a3b8e9e9cf609102c38cd6c00dca4d2645f66ae3af591f29fdeceef7b6cf guix-build-527acc5ee497/output/x86_64-w64-mingw32/SHA256SUMS.part
b69dc956f6fd6bb9b4e20e5f4d29eb4ef74e450d250346bf0450cc051b5cfa95 guix-build-527acc5ee497/output/x86_64-w64-mingw32/bitcoin-527acc5ee497-win64-codesigning.tar.gz
9e44c7635597430ee32fb26dff8fcd3261fe7
...

💬 ajtowns commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3518785282)
utACK 40dcbf580d8eb31a067b62bf9676099919b9841e

🤔 alvroble reviewed a pull request: "test: add case where `TOTAL_TRIES` is exceeded yet solution remains"
(https://github.com/bitcoin/bitcoin/pull/33701#pullrequestreview-3450241034)
re-ACK

🤔 ajtowns reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3446637980)
Have run this live for a little while now without encountering problems which is a good sign. Some comments on the interface follow.

💬 ajtowns commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2513151134)
Could also report txid? Seems strange to report the mempool entry by wtxid only, but its parents and children by txid only (prevout.hash for parents because it's cheap, GetTx().GetHash() for children because it's consistent?). Likewise looking up the cluster requires (and results in) txids.

💬 ajtowns commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2513247414)
When should someone use this RPC? (Otherwise, should it be hidden and/or marked as EXPERIMENTAL?)

💬 ajtowns commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2513201796)
Would extracting the txgraph for a cluster a reasonable thing for the RPC interface to support? Having all the info for a cluster available in one place, without having to find each tx's entry in getrawmempool's output seems like a nice-to-have. Can obviously be a followup.

💬 ajtowns commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2513179750)
Wouldn't it be better to return the results by chunk instead of tx? eg:

```json
{
"vsize": 3500,
"txcount": 25,
"chunks": [
{
"chunk_fee": 0.00000143,
"chunk_vsize": 140,
"txs": [
"c2b9bd0493e4b919ee34710ed411144e5b296d2004434fd0f932b8f3bff93f74"
]
},
{
"chunk_fee": 0.00001001,
"chunk_vsize": 976,
"txs": [
"0ec3b21902ac9091311eb8e0db3533fb54990c6f9987adbc4015a0fc79136910",
"0dea2da2eaff995f546
...

💬 cobratbq commented on pull request "doc: update interface, --stdin flag, IPC-command signtx (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2515957358)
Makes sense. Will do.