💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686230)
Yeah, but isn't the locking kept in `FlushStateToDisk`? So we're reducing 3 locks to 2 locks, right? That's not what I understand from the commit message

💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686728)
Done in c1e554d3e5834a140f2a53854018499a3bfe6822. I had to move `FlushStateToDisk` above possibly calling `m_chainman.snapshot_download_completed()` and breaking or breaking on `m_chainman.m_interrupt`, but that order should not make a difference.

💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072687088)
If we kept the code as is, and `exited_ibd` is `true` and `m_disabled` is `false`, then we would be releasing and acquiring the lock 3 times after we release it for the inner loop.

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686596)
now that we have the constants these comments don't really add anything

👍 l0rinc approved a pull request: "validation: periodically flush dbcache during reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32414#pullrequestreview-2813539202)
Code review ACK - I'm still running the reindex / IBD benchmarks to see if this changed anything unexpected.

Do you think I should stop and restart after the locking changes?

<details>
<summary>nits</summary>

```patch
diff --git a/src/test/chainstate_write_tests.cpp b/src/test/chainstate_write_tests.cpp
index e1b82ebc12..44257d5591 100644
--- a/src/test/chainstate_write_tests.cpp
+++ b/src/test/chainstate_write_tests.cpp
@@ -16,7 +16,8 @@ BOOST_AUTO_TEST_SUITE(chainstate_write_tes
...

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686734)
same, can be removed now I think (and 70 minutes isn't inclusive anymore, right?)

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686476)
nit: if you edit again please reformat `TestSubscriber` as well

🤔 naiyoma reviewed a pull request: "rpc: combinerawtransaction now rejects unmergeable transactions"
(https://github.com/bitcoin/bitcoin/pull/31298#pullrequestreview-2813548569)
Tested ACK https://github.com/bitcoin/bitcoin/pull/31298/commits/9d31e94bf584e12269683b74b1c96c7ac883a760

I have tested the changes on regtest and got expected output

```
bitcoin-cli -regtest combinerawtransaction "[\"$RAW_TX1\", \"$RAW_TX2\"]"
error code: -8
error message:
Transaction 1 not compatible (different transactions)
```

🤔 janb84 reviewed a pull request: "doc: Explain that .gitignore is not for IDE-specific excludes"
(https://github.com/bitcoin/bitcoin/pull/32417#pullrequestreview-2813549591)
ACK [fada115](https://github.com/bitcoin/bitcoin/commit/fada115cbeaa8c0ca3d19178499079b66ee5f499)

Will reduce issues & PR's , good to include the extra comment.

💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2849384794)
Friendly ping @laanwj @Sjors :)

💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2072702883)
Whenever I tried going back to pre-cmake times, I couldn't get it to compile and run again (on Mac or on Linux). Not the first time I try to do a git bisect, but it always turns bad really quickly.

I tried pushing a PR to my fork with the [original PR](https://github.com/bitcoin/bitcoin/pull/28865) that introduced the suppressions + removing `DynamicMemoryUsage`, `SpendCoin` and `Uncache` suppressionss from ubsan.

The build is [failing with similar problems](https://github.com/l0rinc/bitco
...

💬 naiyoma commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2072704239)
nit: IMO, this could be a bit more descriptive

```diff
// Can't merge < 2 items
if (txs.size() < 2) {
- throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Missing transactions");
+ throw JSONRPCError(
+ RPC_INVALID_PARAMETER,
+ txs.size() == 0
+ ? "Missing transactions. At least two transactions required."
+ : "Insufficient transactions. At least two transactions required."
+ );
}
```

✅ furszy closed a pull request: "[WIP] wallet: standardize change output detection process"
(https://github.com/bitcoin/bitcoin/pull/25979)

💬 furszy commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-2849481894)
> The dependency was closed more than a year ago: [#27601 (comment)](https://github.com/bitcoin/bitcoin/pull/27601#issuecomment-1989555740)

Time flies. Closing for now as it is not in my priorities.

📝 cmdruid opened a pull request: "[WIP] rpc: add `clearmempool` command for regtest mode"
(https://github.com/bitcoin/bitcoin/pull/32418)
## Description

This commit adds a new RPC command called `clearmempool`. It is designed to remove all tranasctions from the mempool (including those from invalidated blocks), and return their TXIDs.

The purpose of this command is to clear the mempool, then use the returned TXIDs to perform additional cleanup.

When used in combination with `invalidateblock`, it allows me to roll-back the chain to a particular height, clear the mempool, then properly clean up wallets with `abandontransact
...

💬 andrewtoth commented on pull request "[WIP] rpc: add `clearmempool` command for regtest mode":
(https://github.com/bitcoin/bitcoin/pull/32418#issuecomment-2849691671)
> I need the ability to cleanly rollback the chain and wallets during testing

What about invalidating the block, recording all mempool txs via `getrawmempool`, stopping the node, deleting `mempool.dat` and restarting?

🤔 shahsb reviewed a pull request: "validation: periodically flush dbcache during reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32414#pullrequestreview-2813733894)
Concept ACK

💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2849942977)
I think I discovered a bug in existing test `test/functional/feature_signet.py`.
I found that it generates a block. I added the code to make sure that the block is distributed between the two nodes (node 0 and node 1) using the same signet challenge:
```diff
diff --git a/test/functional/feature_signet.py b/test/functional/feature_signet.py
index 02e37f0fdd..86ab3202b8 100755
--- a/test/functional/feature_signet.py
+++ b/test/functional/feature_signet.py
@@ -87,7 +87,10 @@ class SignetBasi
...

🚀 hebasto merged a pull request: "doc: Explain that .gitignore is not for IDE-specific excludes"
(https://github.com/bitcoin/bitcoin/pull/32417)

✅ maflcko closed a pull request: "Catch harmful arbitrary data. (missing code for #32359)"
(https://github.com/bitcoin/bitcoin/pull/32368)