💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2485715952)
`c4f51c7f61...2b0103705f`: rebase and make the virtual methods of `SockMan` private since they are only used by `SockMan`.

About storing the `CNode` in `SockMan` (which would be templated like `SockMan<CNode>`) and making the communication between `SockMan` and the higher class (e.g. `CConnman`) based on `CNode` instead of node id: this is an excellent idea that will make the code more straight-forward and the higher classes simpler. However it would make this PR larger. I will leave it off f
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2485727139)
Marking this draft, let's review the early return and m_tip_block block changes in #31297 first.

📝 Sjors converted_to_draft a pull request: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)
Alternative approach to #31003, suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362.

This PR introduces `waitNext()`. It waits for either the tip to update or for fees at the top of the mempool to rise sufficiently. It then returns a new template, with which the caller can rinse and repeat.

👍 TheCharlatan approved a pull request: "Prune mining interface"
(https://github.com/bitcoin/bitcoin/pull/31196#pullrequestreview-2445511412)
ACK a67ba64f8f63a0f9f2c86d72cf7240bee7c5388c

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1848395907)
I also have a goal of eliminating the need to invoke `CMPA` on a changeset anyway. In #28676, I believe the only place that `CMPA` is used in a changeset is for determining whether an RBF might be replacing something it depends on. I think it should be straightforward to have the changeset logic directly test for that condition and return failure if a dependency is being added to something that is slated for removal, which would then allow us to remove the method altogether.

🤔 hodlinator requested changes to a pull request: "test: Revert to random path element"
(https://github.com/bitcoin/bitcoin/pull/31317#pullrequestreview-2445541192)
Reviewed fa80b08fef0eaa600339caa678fdf80a8aec3ce3

💬 hodlinator commented on pull request "test: Revert to random path element":
(https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1848404959)
Tested cherrypicking 8e5f8a4f775b93d30b86d28405886eea232cf875 from #30737 on top of this PR to forward `RANDOM_CTX_SEED` on inside *test/fuzz/test_runner.py*.

Ran this ~10 times:
```
RANDOM_CTX_SEED=21 build_fuzz/test/fuzz/test_runner.py ../qa-assets/
```
Many of the failures come down to:
```
Done 2 runs in 0 second(s)
terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
what(): filesystem error: cannot remove all: No such file or directory [
...

💬 ryanofsky commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848409060)
In commit "mining: add early return to waitTipChanged()" (71a539c5c3ae165be71a5c8d7f5b1860f8261bd1)

Does this change have any effect? It looks like this is basically changing `cv.wait_for(condition)` to `if (!condition) cv.wait_for(condition)` which I would expect to be equivalent.

✅ fanquake closed a pull request: "refactor: spanify DecodeBits, use constexpr std::array instead of vector"
(https://github.com/bitcoin/bitcoin/pull/31302)

💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485792072)
Afaik the only way external users make blocks today is the `getblocktemplate` RPC, which drops the coinbase transaction from the template it returns.

Similarly in `submitSolution`, used by Stratum v2, we expect to receive the full coinbase transaction and replace whatever was in the `CBlockTemplate`.

Unless you're solo mining, the pool is going to add its own outputs, manipulate the extra nonce, add its own name, etc. Even with a solo pool (a weird concept) you would configure the payout a
...

💬 fanquake commented on pull request "depends: Update minimum required CMake version":
(https://github.com/bitcoin/bitcoin/pull/31300#issuecomment-2485792150)
> The selected versions in the patches seem random, why not pick the same value for all of them?

Yea. If anything, just consolidate to the same minimum required version as Core.

💬 GIgako19929 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485804884)
This pull request introduces a new RPC method, `getdescriptoractivity`, and includes several related changes. The most important changes are the addition of the new RPC method, updates to documentation, and the introduction of a new utility function.

### New RPC Method:
* Added the `getdescriptoractivity` RPC method to retrieve spend and receive activity for a set of descriptors within specified blocks. This method works in conjunction with the `scanblocks` RPC to reduce the need for additio
...

💬 GIgako19929 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485806660)
This pull request introduces a new RPC method, `getdescriptoractivity`, which provides detailed information on spend and receive activities for specified descriptors and blocks. Additionally, it includes several related updates to the codebase and documentation.

### New RPC Method:
* `getdescriptoractivity` RPC method: This new method allows users to find all spend/receive activity relevant to a given set of descriptors within specified blocks. It pairs well with the `scanblocks` method to r
...

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1848424503)
yep feel free to not do this one for now (I just ran into it writing my own test case)

💬 GIgako19929 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485817184)
This pull request introduces a new RPC method, `getdescriptoractivity`, and includes various related updates to the documentation and codebase. The most important changes include the addition of the new RPC method, updates to the documentation to reflect this new method, and code improvements to support the new feature.

### New RPC Method:

* Added `getdescriptoractivity` RPC method to retrieve spend and receive activity for a set of descriptors within specified blocks. This method pairs wi
...

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848440029)
I'm assuming `wait_for` won't be triggered until `m_tip_block` is set or modified, or the timeout passes. So it doesn't return early.

✅ fanquake closed a pull request: "Typo Update bitcoin-conf.md"
(https://github.com/bitcoin/bitcoin/pull/31322)

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2485835136)
Rebased on top of the https://github.com/bitcoin/bitcoin/pull/31307 to workaround MSVC compiler issue.

💬 fanquake commented on pull request "build: Enable -Wbidi-chars=any":
(https://github.com/bitcoin/bitcoin/pull/31315#issuecomment-2485840546)
Concept ACK - looks like GH also tries to warn about this.

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485845484)
light review Ack 878b6c85466366c4ae5f454ec49b5a5f561e0ed2

Reviewed tests and interface mostly