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

💬 cobratbq commented on pull request "doc: update interface, --stdin flag, IPC-command signtx (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2515960152)
This is content that is written to stdout, thus piped to external-signer. This is not part of the commandline, such that a shell might interfere or otherwise preprocess. Thus quotes are passed on as any other byte. Consequently, probably better to mention this.

💬 cobratbq commented on pull request "doc: update interface, --stdin flag, IPC-command signtx (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2515963270)
Is your comment that the doc is wrong, or that you moved away from that solution long ago? I only recently got involved with this side of Bitcoin, so I am really only familiar with v27+.

I can drop the `signtransaction` shell command for the sake obsolescence regardless, but it would be nice to know.

💬 cobratbq commented on pull request "doc: update interface, --stdin flag, IPC-command signtx (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#issuecomment-3518901563)
> Good idea to update these docs.

Yes ... I was inspired by a certain https://github.com/bitcoin/bitcoin/issues/31005#issuecomment-3144377397

💬 plebhash commented on issue "RFC: Cancelling waitNext calls in the IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3518915748)
hey @ryanofsky sorry for the delay

I compiled Bitcoin Core with the branch from https://github.com/bitcoin/bitcoin/pull/33676

then I removed the workarounds, saving the code here: https://github.com/plebhash/sv2-bitcoin-core/tree/2025-11-11-try-interrupt-await

I did some testing and everything seems to be working as expected... especially the usage of the new `interruptWait` interface, introduced by https://github.com/bitcoin/bitcoin/pull/33676

💬 rahmatshahfarooqee1122-dev commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-3519083364)
Send me many from jalalabad Afghanistan all this account 257427052021105841

💬 rahmatshahfarooqee1122-dev commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-3519084572)
257427052021105841

💬 kevkevinpal commented on issue "`test_kernel` fails to build on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33846#issuecomment-3519093351)
> An alternative could be to use an lvalue. However, I am not sure if this is worth it:

I think it might make most sense to continue with https://github.com/bitcoin/bitcoin/pull/33842 and to bump the min version of g++ to 12 and above as you mentioned

💬 ajtowns commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2516214890)
> I don't think a virtual move assignment would actually help

Yeah. What about dropping it entirely? I think if the fuzz test is changed something like this:

```diff
--- a/src/test/fuzz/txgraph.cpp
+++ b/src/test/fuzz/txgraph.cpp
@@ -138,14 +138,14 @@ struct SimTxGraph
}

/** Add a new transaction to the simulation. */
- TxGraph::Ref* AddTransaction(const FeePerWeight& feerate)
+ TxGraph::Ref* AddTransaction(TxGraph::Ref&& ref, const FeePerWeight& feerate)
{

...

📝 frankomosh opened a pull request: "test: add unit test coverage for the empty leaves path in MerkleComputation"
(https://github.com/bitcoin/bitcoin/pull/33858)
As noted in [#32243 (comment)](https://github.com/bitcoin/bitcoin/pull/32243#issuecomment-2988854482), the early return inside `MerkleComputation` when `leaves.size() == 0` was only exercised by fuzz tests.

The existing `merkle_test_empty_block` calls `BlockMerkleRoot`, which uses `ComputeMerkleRoot`, but does not exercise the `TransactionMerklePath` → `ComputeMerklePath` → `MerkleComputation` code path.

Coverage before adding test:
<img width="2459" height="66" alt="before" src="https:
...

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2516439828)
Fixed in 607b61c8b186

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2516441074)
Done.

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2516441785)
Done in da5ddd2002bfe46f56c45b260ebda226c2fd45ee

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2516442549)
Done in da5ddd2002bf

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2516444966)
Incorporated in 017b21286a30374fc8ed21c3b8b92239c6cff55a