👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2260895880)
Code review ACK 33f5ab32b2651a18c9d56bef1e5b3c69dd26e199. Only changes since last review were include changes and reformatting changes.

I have to say I am not a fan of running clang-format-diff in same commits that introduce real code changes and making unrelated changes to surrounding lines of code. It particularly adds a lot of noise to this PR in the ThresholdState switch statement and makes it harder to review. IMO it is preferable to use clang-format-diff only to format code that is actu
...

💬 MarnixCroes commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2310426838)
> > [4105129](https://github.com/bitcoin/bitcoin/commit/41051290ab3b6c36312cec26a27f787cea9961b4) succesfully built on debian12 and fedora40
> >
> > * disable-wallet mode is not working without sqlite dependency installed? Using `cmake -B build -DENABLE_WALLET=OFF`, as per the instructions, will not work without sqlite installed. (error that sqlite is missing)
>
> I cannot reproduce the issue on Fedora 40:
>
> ```
> $ dnf list installed sqlite-devel
> Error: No matching Packages to l
...

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2310483256)
@ryanofsky I used the incantation suggested here: https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#clang-format-diffpy

I assumed this would only edit lines impacted by the commit. But I see it touched a few other places in `rpc/mining.cpp`. I removed unrelated changes from the commit.

🤔 ryanofsky reviewed a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2260994997)
Code review 93176016322687f3ad9ade8f32c1ff1392ee6470, but I'm concerned code is failing to signal m_tip_block_cv on shutdown so it could make shutdown slower. Suggested a fix below. It also seems like it would be easy to eliminate g_best_block completely, so suggested a way to do that too.

💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1731440200)
If we are going to add a new `m_tip_block` variable to replace `g_best_block` seems like it would be good to make replacement complete and eliminate `g_best_block`. I think following could work (compiles but untested):

```diff
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -739,7 +739,6 @@ static RPCHelpMan getblocktemplate()
{
// Wait to respond until either the best block changes, OR a minute has passed and there are more transactions
uint256 hashWatchedCh
...

💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1731432419)
In commit "Add waitTipChanged to Mining interface" (078fc60f198eb7a3bcccb557416fe1d4498f26e7)

One problem with switching from g_best_block_cv to m_tip_block_cv is that this m_tip_block_cv is not signaled when StopRPC is called, which could mean shutdown hangs for a second instead of being instantaneous. I believe this could be fixed in earlier commit 8400d49ffec47add2dc27267ae13ede5d2fabeea with:

```diff
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -282,7 +282,7 @@ void Shutdown(NodeContext
...

📝 maflcko opened a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716)
Missing includes are problematic, because:

* Upcoming releases of the C++ standard library often minimize their internal header dependencies. For example, `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` (https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html). This can lead to compile failures, which are easy to fix for developers, but may not for users. For example, https://github.com/bitcoin/bitcoin/pull/30612/commits/39d1ca4bc9d29f683108c800bdfe59d2dc90f3c7 had to add missing includes to accommo
...

👍 ryanofsky approved a pull request: "test: [refactor] Use m_rng directly"
(https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2261031461)
Code review ACK 948238a683b6c99f4e91114aa75680c6c2d73714. Only changes since last review were changing a comments a little bit.

💬 hebasto commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2310537762)
Concept ACK.

Maybe check `src/bench` in the CI separately and treat warnings as errors?

💬 maflcko commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2310541118)
> Maybe check `src/bench` in the CI separately and treat warnings as errors?

Yes, this will be a follow-up, because it requires other changes as well.

👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2261045238)
Code review ACK 23374a22f442939802c0296d6447e0db02d513da. Just reined in clang-tidy since last review so there are fewer changes

💬 stratospher commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2310604676)
reACK c8e6771.

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1731519643)
Nice! I split the changes to `rpc/mining.cpp` into its own commit, that looks correct.

Running tests before pushing...

🤔 ismaelsadeeq reviewed a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2261138334)
Concept ACK

💬 ismaelsadeeq commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2310653344)
post merge ACK 647fa37cdbadbeebba147ca6b24e138559cffaaf

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: optimized candidate search"
(https://github.com/bitcoin/bitcoin/pull/30286#pullrequestreview-2260608958)
Concept ACK

💬 ismaelsadeeq commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1731199919)
While reading through the bench I noticed that docstring explanation of `BenchLinearizeNoItersWorstCaseAnc` still refer to `BenchLinearizePerIterWorstCase`

🤔 ismaelsadeeq reviewed a pull request: "Use MiniWallet in functional test rpc_signrawtransactionwithkey."
(https://github.com/bitcoin/bitcoin/pull/30701#pullrequestreview-2261157933)
Concept ACK

I'll prefer if you split this commit intro three as you highlight in commit message

1- Replace custom funding tx creation with MiniWallet
2- Remove unnecessary clean chain setup and second node
2- Simplify transaction handling

💬 luke-jr commented on issue "[build] cannot build tests using v26.0":
(https://github.com/bitcoin/bitcoin/issues/29051#issuecomment-2310689306)
This should be reopened. It's a bug if `make clean` fixes things. The build system is supposed to automatically detect when a file needs to be rebuilt. (Maybe CMake will fix it?)

👍 ismaelsadeeq approved a pull request: "test: fix `TestShell` initialization (late follow-up for #30463)"
(https://github.com/bitcoin/bitcoin/pull/30714#pullrequestreview-2261197556)
I verified that this behavior occurs locally:

```python
Python 3.12.4 (main, Jun 6 2024, 18:26:44) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path.insert(0, "/Users/abubakarismail/Desktop/bitcoin-dev/bitcoin/test/functional")
>>> from test_framework.test_shell import TestShell
>>> test = TestShell().setup()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/U
...