💬 alexanderwiederin commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3479398323)
re-ACK https://github.com/bitcoin/bitcoin/commit/e95efc00842d5d0df96ee9294cdf818741be539e

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485697378)
There are other ones that I didn't mention, reformatted the change, please take the ones that make sense:

<details>
<summary>Details</summary>

```patch
diff --git a/src/coins.cpp b/src/coins.cpp
index 2ef2e36ccc..baac1a32b5 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -173,7 +173,8 @@ bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const {
return (it != cacheCoins.end() && !it->second.coin.IsSpent());
}

-std::optional<Coin> CCoinsViewCache::GetPossib
...

👍 hodlinator approved a pull request: "refactor: Header sync optimisations & simplifications"
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3409949311)
re-ACK 8a4ab45c13dd7ca4474bc4621110da82e0a634f8

Changes since previous review (https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3128785887):

* More helpful commit messages.
* Rebased, especially to handle #32579.

💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3479517366)
@hodlinator thanks! Was about to comment with the update haha :)

In my last push:

- Rebase after #32579
- Address https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289739420, https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289729638, https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289725133
- Specify in 8a4ab45c13dd7ca4474bc4621110da82e0a634f8 commit message that I'm also slightly reformatting the code

💬 Sjors commented on pull request "miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC":
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-3479528530)
I switched to the approach of adding an `include_dummy_extranonce`, having the tests set it to `true` while IPC calls use the default `false`. Expanded `test/functional/interface_ipc.py` to demonstrate it.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485772199)
Hmm, I'm not sure this is correct.

> Obviously we don't want to use the sorted vector

That's not what I'm getting. You were inserting to the same collection over and over, I'm not sure what we were measuring there.
Adding the collection creation and an assertion to not optimize it away reveals something completely different.

<details>
<summary>updated benchmarking code</summary>

```C++
#include <bench/bench.h>
#include <bench/data/block413567.raw.h>
#include <coins.h>
#include
...

👍 TheCharlatan approved a pull request: "mining: check witness commitment in submitBlock"
(https://github.com/bitcoin/bitcoin/pull/33745#pullrequestreview-3410154831)
Re-ACK 6eaa00fe20206baedc0d8ade5bb8d066ea615704

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3479640421)
I have the IBD numbers for the i7-hdd and i9-ssd server. They're not as glorious as our `reindex-chainstate` measurements, most likely since I don't yet have a way to test IBD from extremely fast peers. But as a sanity-check I think it's fine, we're still bandwidth bound - which is a good problem to have.

<details>
<summary>7% faster IBD | 921129 blocks | dbcache 4500 | i7-hdd | x86_64 | Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz | 8 cores | 62Gi RAM | ext4 | HDD</summary>

```
COMMITS="bf07
...

💬 maflcko commented on pull request "rest: Query predecessor headers using negative count param":
(https://github.com/bitcoin/bitcoin/pull/33752#issuecomment-3479685859)
Though, I wonder what the use case is. If someone knows a header, it seems likely they also know about the previous headers and a feature to support asking for them would have no users?

💬 maflcko commented on issue "test: break `feature_block` into subtests?":
(https://github.com/bitcoin/bitcoin/issues/32877#issuecomment-3479692292)
Some functional tests are defined as "extended":

```
--extended run the extended test suite in addition to the basic tests

💬 maflcko commented on pull request "ci: gha: Set debug_pull_request_number_str annotation":
(https://github.com/bitcoin/bitcoin/pull/33754#issuecomment-3479741328)
> lgtm ack [fa9d0f9](https://github.com/bitcoin/bitcoin/commit/fa9d0f994b45a94e3f26c01e395c58ff59f47f43)

The ack needs to be in all-upper case for the scripts to pick it up :sweat_smile:

🤔 frankomosh reviewed a pull request: "ci: Call docker exec from Python script to fix word splitting"
(https://github.com/bitcoin/bitcoin/pull/33732#pullrequestreview-3410289108)
Concept ACK

💬 frankomosh commented on pull request "ci: Call docker exec from Python script to fix word splitting":
(https://github.com/bitcoin/bitcoin/pull/33732#discussion_r2485943158)
nit: Should all non-zero exits be treated as "nothing to copy from"? am making the assumption that rsync could fail for other reasons

💬 l0rinc commented on pull request "ci: gha: Set debug_pull_request_number_str annotation":
(https://github.com/bitcoin/bitcoin/pull/33754#issuecomment-3479767050)
code review ACK fa9d0f994b45a94e3f26c01e395c58ff59f47f43

💬 Sjors commented on pull request "miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC":
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-3479773495)
Looks like `fuzz/process_message.cpp` and `fuzz/process_messages.cpp` were missing `include_dummy_extranonce = true`.

💬 stringintech commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3479798341)
re-ACK 01cc20f

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2485972867)
I see you've reverted this after the rebase - was it on purpose?

💬 A-Manning commented on pull request "rest: Query predecessor headers using negative count param":
(https://github.com/bitcoin/bitcoin/pull/33752#issuecomment-3479817272)
>Though, I wonder what the use case is. If someone knows a header, it seems likely they also know about the previous headers and a feature to support asking for them would have no users?

Knowing a header only means that you know the previous block hash. If you want to request headers from best tip to genesis, There is no existing mechanism other than requesting headers one-at-a-time. This makes it possible to *batch* request headers from tip to genesis, which is much more efficient than reque
...

💬 maflcko commented on pull request "ci: Call docker exec from Python script to fix word splitting":
(https://github.com/bitcoin/bitcoin/pull/33732#discussion_r2485987246)
This is just moving the code one-to-one, but possibly `rsync`, along with the rsync command passing, could be required here. (IIRC the error message may have been better worded to reflect the intention: `rsync not found, no need to copy from ...`)

At least on macOS, it seems to pass: https://github.com/bitcoin/bitcoin/actions/runs/18913390006/job/53990076461?pr=33732#step:7:134

```
+ rsync --recursive --perms --stats --human-readable /Users/runner/work/bitcoin/bitcoin/ /Users/runner/work/
...

💬 maflcko commented on pull request "ci: Call docker exec from Python script to fix word splitting":
(https://github.com/bitcoin/bitcoin/pull/33732#discussion_r2485988430)
(added a commit to require rsync)