💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087197)
Done.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087251)
Removed it.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087341)
Done.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087547)
Done.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087694)
Fixed. But, it wouldn't affect the correctness of the test.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087978)
Rewrote this part to make it more clear. I was trying to be clever by avoiding some extra checks, but they are probably meaningless and clarity is better here.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485157101)
I wrote benchmarks to check how fast it was to construct, since this is the work that is done not in parallel:

```C++
static void SortedVectorBenchmark(benchmark::Bench& bench)
{
CBlock block;
DataStream{benchmark::data::block413567} >> TX_WITH_WITNESS(block);
std::vector<Txid> v{};
v.reserve(block.vtx.size());

bench.run([&] {
for (const auto& tx : block.vtx) {
v.emplace_back(tx->GetHash());
}
std::sort(v.begin(), v.end());
...

🤔 danielabrozzoni reviewed a pull request: "doc: document fingerprinting risk when operating node on multiple networks"
(https://github.com/bitcoin/bitcoin/pull/33750#pullrequestreview-3409743891)
ACK e346ecae830e10310979e5f64de63e043a383ff1

I really like how it's written, it's really fair and balanced, without sounding scary. Thanks for working on this! :)

Just a small nit, you included the suggestions from https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2481496048 and https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482405261 in the second commit (e346ecae830e10310979e5f64de63e043a383ff1). This makes the first commit modify only tor.md, while the second one cha
...

💬 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