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

💬 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)
{

...