💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485569647)
I don't think we should abort, it's not the fetcher's job to validate. If we didn't, we could even use short ids for the intra-block spends (64 bit likely won't even result in a single duplicate, and when they do xollide we would just do a db check)

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485572792)
atomics work via CAS, they need to retry in case of high contention. the previous solution didn't have contention, the threads all knew beforehand what to work on.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485575807)
that's my point, I don't think we should validate at all, it would simplify the code if we didn't

✅ maflcko closed an issue: "Decouple datacarrier size and count limits (Draft PR)"
(https://github.com/bitcoin/bitcoin/issues/33595)

💬 maflcko commented on issue "Decouple datacarrier size and count limits (Draft PR)":
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3479289768)
The feature request didn't seem to attract much positive attention in the past. Thus, closing due to lack of interest, progress and direction. It is possible to re-open this issue or create a new issue referencing it, if there is fresh interest.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485579371)
how so? Can we assert the behavior of the main cache as well so that the previous version doesn't pass?

💬 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