👍 instagibbs approved a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3404624528)
reACK ed23b4537ae8a8a814738909fe9a84c548f2855d

`git range-diff master 1ae9fd3f7a4fddc786e9921e7fc2af41ab96bf9a ed23b4537ae8a8a814738909fe9a84c548f2855d`

💬 ryanofsky commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3473445529)
> Yes, I think it’s a good idea to prevent multiple calls to waitNext if there is no explicit use case, or have interruptWait identify and cancel a targeted wait?

I think I like the current implementation more than either of these two ideas, because they both seem like they would bring unneeded complexity. It seems ok to allow multiple wait calls, since they should work perfectly well, even if there is not a very convenient way to cancel them. If it ever becomes important for interruptWait to
...

💬 Sjors commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3473456047)
I successfully tested this PR against my Template Provider implementation, see https://github.com/Sjors/sv2-tp/pull/57.

💬 A-Manning commented on pull request "rest: Query predecessor headers using negative count param":
(https://github.com/bitcoin/bitcoin/pull/33752#discussion_r2481717919)
Updated to make this a bit more succinct. `ptrdiff_t` used here because it is the signed equivalent to `size_t`, and is suitable for casting to `size_t` to compare against `headers.size()`.

🤔 stringintech reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3395010361)
Comments on commits 8 (ffd4ae23) through 15 (0e973b20) and a couple of follow-ups.

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2474390216)
Seems unnecessary since `Block` is extending a public `Handle` (also `friend` statements in `BlockTreeEntry`). Seems only the ones currently extending non-public `UniqueHandle`s would need the `friend` statements. Any reason why extending `View` and `Handle` is done publicly but not for `UniqueHandle`?

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2474403542)
Unused variable. Also line 661.

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2474982074)
Maybe not important but we could use `::ref()` to avoid the copy you [mentioned](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2468790150)? (`CChainParams` copy ctor is called in `::create()` right now)
```cpp
btck_ChainParameters::ref(const_cast<CChainParams*>(CChainParams::Main().release()));
```

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2474401085)
In commit bc6788ab description:
"Adds options for wiping the chainstate and block tree indexes to the chainstate load options" -> "... chainstate manager options" (since only one is a load option)

Also for efcfe2df commit title and 2ecd926c commit description.

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2478084857)
```suggestion
/**
* @brief Triggers the start of a reindex if the wipe options were previously set for
* the chainstate manager. Can also import an array of existing block
* files selected by the user.
*
* @param[in] chainstate_manager Non-null.
* @param[in] block_file_paths_data Nullable, array of block files described by their full filesystem paths.
* @param[in] block_file_paths_lens Nullable, array containing the lengths of each of the paths.
* @param[in] block
...

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2474412320)
It seems [this](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2459840316) was not applied (force push [message](https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3387750815) suggests otherwise)?

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2477740569)
In commit 2ecd926c description:
"import blocks from a single specified file path" -> "import blocks from specified file paths"

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2479513352)
Could this be also declared as final? (like `KernelValidationInterface`)

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2481021568)
Is this line necessary?

💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2481753366)
Also related to the discussion [here](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2347348366).

💬 l0rinc commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2481763116)
> As far as I can tell, CTransaction's hashes are computed and cached on construction

No, I mean `uint256 hash = header.GetHash()` is only needed for the debug log - let me try separating all calculations that are only needed for debugging. Given that we're clearing `header` and `txn_available` I understand why the split wasn't done before. But we can still use `block` for the hash and `vtx_missing` for the missing, so I think it should be possible to split the two.
That's a more invasive ch
...

💬 A-Manning commented on pull request "zmq: Log bind error at Error level, abort startup on init error":
(https://github.com/bitcoin/bitcoin/pull/33727#discussion_r2481787433)
I agree that `zmqDebug`/`zmqError` would be ideal, however `zmqError` is currently used to log at the `Debug` level - changing it to log at `Error` level while keeping the type signature the same seems likely to cause misuse.

💬 maflcko commented on pull request "rest: Query predecessor headers using negative count param":
(https://github.com/bitcoin/bitcoin/pull/33752#discussion_r2481808137)
MAX_REST_HEADERS_RESULTS is a small number, so I don't think we need any architecture-specific and special PTRDIFF handling and can just use fixed size integral values.

Also, to walk backwards, I am sure you can just use `pprev` and not need to create a new method.

💬 instagibbs commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3473633999)
If anyone has documentation showing the format (pre-OP_RETURN) this would be useful.

🚀 fanquake merged a pull request: "test: Format strings in `test_runner`"
(https://github.com/bitcoin/bitcoin/pull/33753)