💬 luke-jr commented on pull request "rpc, cli: add getbalances#total, and use it for -getinfo":
(https://github.com/bitcoin/bitcoin/pull/31353#issuecomment-2558263107)
Seems like it would be better to just add them in `-getinfo`?

💬 kallewoof commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#issuecomment-2558266241)
Concept ACK

💬 luke-jr commented on issue "rfc: DATUM mining interface requirements":
(https://github.com/bitcoin/bitcoin/issues/31002#issuecomment-2558270819)
> No need to tell users to manually set -blockmaxsize and -blockmaxweight, because createNewBlock() can pass a custom coinbase weight reservation through its BlockCreateOptions argument.

Seems like it would be trivial to add this to GBT. The current approach in Knots isn't ideal (since it would override blockmaxsize/weight if the miner intentionally sets them lower).

> No need to use -blocknotify (which launches datum_gateway and presumable then calls getblocktemplate)

It only sends a s
...

🤔 ariard reviewed a pull request: "p2p: Fill reconciliation sets (Erlay) attempt 2"
(https://github.com/bitcoin/bitcoin/pull/30116#pullrequestreview-2519443536)
Reviewed up to commit 478137aa (“p2p: Add PeerManager method to count…”).
Just minor comments so far and I still have to answer back comments on #28765.

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1894709738)
nit: s/Get the amount on/Get the amount of/g.
And can say inbound is given first.

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1894709326)
I think the variable naming could be enhanced, e.g `m_collision`, to dissociate from mempool conflicts. Here, it’s a collision from SipHash outputs that can arise from 2 distinct transactions, not even spending the same set of UTXOs.

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1894707254)
I think this snippet of code could be documented in the following fashion

```
// If we receive many `SENDTXRCNCL` from the same peer, before to receive a
// single `VERACK`, re-register the peer id state with potentially different local
// and remote salts.
```

As far as I can see, at `SENDTXRCNCL` reception we do not check for duplicate `SENDTXRCNCL` message (we check if we have txreconciliation enabled, then that it’s happening before verack reception and then that transaction-relay
...

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1894707917)
Logging that a given peer’s local reconciliation set max size has been reached can be useuful info for future debug.

```
diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp
index d4d6bdf1ea..b99bb41681 100644
--- a/src/node/txreconciliation.cpp
+++ b/src/node/txreconciliation.cpp
@@ -150,7 +150,11 @@ public:
if (!peer_state) return false;

// Transactions which don't make it to the set due to the limit are announced via fan-out.
- if (
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1894728411)
I simulated this to see if there was any difference between doing the decision-making before or after trickle.

I didn't do it for dependant transactions, just for a single one, trying to answer whether choosing fanout peers at scheduling time or at relay time made any difference. The answer is no, given the number of peers that know about the transaction at both times is almost the same.

The approach, simulation and results can be found here: https://srgi.notion.site/Select-fanout-candidates-
...

💬 1440000bytes commented on pull request "coins: warn on shutdown for big UTXO set flushes":
(https://github.com/bitcoin/bitcoin/pull/31534#discussion_r1894742730)
```suggestion
if (coins_mem_usage >= WARN_FLUSH_COINS_SIZE) LogWarning("Flushing %d GiB UTXO set to disk, it may take several minutes.", coins_mem_usage >> 30);
```

💬 l0rinc commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2558426619)
The PR has been split into 3 to simplify review, please check those out first:
* refactor: cache block[undo] serialized size for consecutive calls - https://github.com/bitcoin/bitcoin/pull/31490
* optimization: bulk reads(27%)/writes(290%) in [undo]block [de]serialization, 6% faster IBD - https://github.com/bitcoin/bitcoin/pull/31551

The 3 changes together achieve a 12.3% speedup in raw IBD (real nodes, multiple runs, 870k blocks, 1k dbcache, ssd)

<details>
<summary>Details</summary>

...

💬 l0rinc commented on pull request "coins: warn on shutdown for big UTXO set flushes":
(https://github.com/bitcoin/bitcoin/pull/31534#discussion_r1894920990)
The *large* part is important, we're not always displaying this, only when flushing is expected to take a long time

💬 1440000bytes commented on pull request "coins: warn on shutdown for big UTXO set flushes":
(https://github.com/bitcoin/bitcoin/pull/31534#discussion_r1894921918)
It is grammatically incorrect to write "large N GiB".

💬 l0rinc commented on pull request "coins: warn on shutdown for big UTXO set flushes":
(https://github.com/bitcoin/bitcoin/pull/31534#discussion_r1894926926)
K, rephrased it to `Flushing large (1 GiB) UTXO set to disk, it may take several minutes`

👍 tdb3 approved a pull request: "coins: warn on shutdown for big UTXO set flushes"
(https://github.com/bitcoin/bitcoin/pull/31534#pullrequestreview-2519642270)
re ACK 5709718b830161b7c2ba0db545ef0cfa98423597

👍 1440000bytes approved a pull request: "coins: warn on shutdown for big UTXO set flushes"
(https://github.com/bitcoin/bitcoin/pull/31534#pullrequestreview-2519647341)
ACK https://github.com/bitcoin/bitcoin/pull/31534/commits/5709718b830161b7c2ba0db545ef0cfa98423597

🤔 hebasto reviewed a pull request: "guix: disable gcov in base-linux-gcc"
(https://github.com/bitcoin/bitcoin/pull/31450#pullrequestreview-2519657730)
Post-merge ACK f6496a838828aa6d5a2a40380c2338fda2e0d812.

👍 tdb3 approved a pull request: "qa: Limit `-maxconnections` in tests"
(https://github.com/bitcoin/bitcoin/pull/31537#pullrequestreview-2519666841)
tested ACK d9d5bc2e7466033d989432f53a112325fa3d6d4a

Tested using NetBSD 10 (master fails with `Warning: Reducing -maxconnections from 125 to 96, because of system limitations`, while the PR branch does not).

Also simulated the fd constraint on Debian 12.8 with `ulimit -n256`. Master saw the warning (and test failure). The PR branch did not.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2558527144)
I have pushed a substantial change to the `TxGraphImpl::GroupClusters` function. In a benchmark with 64000 transactions being combined into 1000 clusters, this makes it go from ~140ms to ~16ms. I will add the benchmark to this PR when I clean it up.

📝 sipa opened a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553)
Part of cluster mempool (#30289). Builds on top of #31444.

During reorganisations, it is possible that dependencies get add which result in clusters that violate policy limits (cluster count, cluster weight), when linking the new from-block transactions to the old from-mempool transactions. Unlike RBF scenarios, we cannot simply reject these policy violations when they are due to received blocks. To accommodate this, add a `TxGraph::Trim()`, which removes some subset of transactions (includin
...