💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2344933415)
7341a51a02adbac1246f72c6643ee0d878b6aa46

nit: could we add some napkin math how we're getting to 9 here

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2345084353)
7f44c6d9893df97bb413e01aa737028cbe52ce75

Think you can delete `CompareTxMemPoolEntryByScore`

commit message is also a bit weird since it's replaced by something related

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2345144877)
5de7f1378a3146368975c10511dfec086d522ff4

This needs to be checking number of ancestors of the (only) child to make sure it doesn't have multiple in-mempool parents. Here it's re-checking the same logic as the line above?

This logic seems to persist to the end of the PR.

I think this could only happen in a reorg, resulting in something like a 2p1c TRUC sibling. Looks like we only test for 1p2c et al.

Using txgraph I feel like we can make these checks extremely compact anyways, maybe c
...

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2345163699)
20d6bcb2e7f2c3016280321ef31421ca4039b017

nit: child isn't large

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2345196854)
cbcfdf4cb0ef05773bf430ebf6beafc59c1c7bc6

grammar: you're not `avoid`ing disabling sibling eviction here

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2345024271)
7f44c6d9893df97bb413e01aa737028cbe52ce75

I was unable to break any existing tests by messing with this. Added a few lines to ::check which would at least catch asymmetry in orderings:

```
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index f44113ed8b..64e8de251d 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -442,4 +442,6 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCach
...

📝 PiRK opened a pull request: "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage"
(https://github.com/bitcoin/bitcoin/pull/33381)
If the random coin in this test has a non-null scriptpubkey, `cache->SanityCheck();` fails an assertion because the `BOOST_CHECK_THROW` line leaves `CCoinsViewCache::cachedCoinsUsage` in a corrupted state: the erroring `AddCoin` call decrements the value to 0 even though the coin is still in the cache, then the next `SpendCoin` call decrements the value again causing a `size_t` underflow.

Move the `BOOST_CHECK_THROW` line to the very end of the test so it does not break following tests.

💬 BenWestgate commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2345324305)
Okay, I've updated the tutorial to use `bitcoin rpc` everywhere possible (getcoins.py won't accept `bitcoin rpc -signet` yet).

I also fixed two bugs where we forgot to specify `-signet`.
https://github.com/bitcoin/bitcoin/pull/33286/commits/3d42607fb5966d8e3b79fdb878d59483432625d9

https://github.com/bitcoin/bitcoin/pull/33286/commits/3d42607fb5966d8e3b79fdb878d59483432625d9

💬 PiRK commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#issuecomment-3286670785)
I force pushed to fix a use-after-move bug

✅ Raimo33 closed a pull request: "[WIP] cache: remove redundant find() call"
(https://github.com/bitcoin/bitcoin/pull/33376)

💬 Raimo33 commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#issuecomment-3286747889)
Following up in https://github.com/l0rinc/bitcoin/pull/34/commits/b23c6a1fc8f9a44561add79427d2f3fff4c3718f as reviewer/coauthor

💬 hodlinator commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3286779725)
(I'd be in slight favor of changing raw arrays to `std::array` as done in https://github.com/davidgumberg/bitcoin/commit/eda422cfa56245a9b78b9a4f499a490d44e58eb5 since some debug builds in CI run with extra bounds-checking for std-types. Although maybe MSAN builds already catch out of bounds access for raw arrays, and I don't suspect we would detect any new issues for `base_uint`/`arith_uint256`).

💬 Raimo33 commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3286813683)
same. i think https://github.com/davidgumberg/bitcoin/commit/eda422cfa56245a9b78b9a4f499a490d44e58eb5 is worth a PR even if it's a minor change. Maybe you can pack it with other commits where `std::array` is needed.

💬 stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2345431507)
I think docs should be updated for these two about "not being thread safe" now that we have `LOCK(cs_main)` calls in the implementations?

💬 stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2345384878)
Is there a reason `cs_main` isn't acquired for the first two functions?

💬 stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2345483298)
Also noticed in this part that the two functions alter the same global value (`m_categories`) which happens to be atomic. We have acquired `cs_main` for one of them (but not the other?). Since the global value is already updated atomically, I assume locking `cs_main` serves a different purpose? In general, the `cs_main` locks for logging functions are a bit unclear to me. I would appreciate it if you could elaborate a bit on what you had in mind when adding them.

💬 mzumsande commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369)
Maybe covered by the previous comments, but running `test_bitcoin` on master now prints out a warning, caused by `node_init_tests`:

```
Running 671 test cases...
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
```

🤔 hodlinator reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3218940692)
Concept ACK

Confirmed to work on Windows:
```
$ ./build/bin/Release/bitcoind.exe -regtest -dbcache=64534
2025-09-12T22:14:09Z Bitcoin Core version v29.99.0-080a21cb127f-dirty (release build)
...
2025-09-12T22:14:09Z [warning] A 64524 MiB dbcache may be too large for a system memory of only 65413 MiB.
```

📝 caesrcd opened a pull request: "Bash completion for bitcoin(1) wrapper"
(https://github.com/bitcoin/bitcoin/pull/33382)
Uses `bitcoin --help` / `bitcoin help` to list available commands and options for the `bitcoin` wrapper.
Integrates completions for `bitcoind`, `bitcoin-cli`, and `bitcoin-tx`.

Placed under `contrib/` so it can be installed or used easily by distros/users who want completion.

💬 davidgumberg commented on issue "bench: unrealistic ConnectBlock benchmarks":
(https://github.com/bitcoin/bitcoin/issues/33375#issuecomment-3287201170)
I am [skeptical](https://github.com/bitcoin/bitcoin/pull/31689#issuecomment-2655685417) of trying to measure end-to-end performance of Bitcoin Core systems like block connection including LevelDB performance using a microbenchmark suite. In my opinion, these benchmarks work best at measuring the performance of specific components and functions a-la unit tests, and I would not object to adding more scenarios to `ConnectBlock()` that are meant to exercise the performance of certain segments of blo
...