💬 achow101 commented on issue "Is not demanding low-R grinding still relevant for external signers?":
(https://github.com/bitcoin/bitcoin/issues/33761#issuecomment-3478202041)
> Any ref of high-S / low-S?

https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#user-content-Low_S_values_in_signatures (the BIP itself is withdrawn, but this section on low-S is relevant in this dicsussion)

💬 andrewtoth commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2484961831)
I believe this case should have been removed in https://github.com/bitcoin/bitcoin/pull/25325, which was merged just after this was introduced in https://github.com/bitcoin/bitcoin/pull/27011. So it likely got missed that `ReallocateCache` is always called.

💬 andrewtoth commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2484965939)
```suggestion
* If will_reuse_cache is false, the cache will retain the same memory footprint after flushing and should be destroyed to deallocate.
```

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3478211267)
@romanz do you have a proof-of-concept for the tx indexer that will utilize this? It would be good to make sure this will be sufficient and performant enough for your requirements.

🤔 l0rinc requested changes to a pull request: "validation: fetch block inputs on parallel threads >20% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-3389779624)
I like how we're progressing here! I think we need a few more things and have to try out a few alternatives (I haven't given up on sorting yet, especially now with bigger dbcache) and want to see how this combines with the other optimizations (threadpool, SipHash13, map hash caching, etc), but I'm definitely getting closer and closer to an ACK :D

I'm testing full IBD locally on my servers, but those are always slower than `reindex-chainstes` since the nodes can't send the blocks fast enough - I
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470286368)
Nit: structs are now formatted consistently on the same line

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470285353)
nit: we could throw now to indicate we don't have unrepeatable side-effects

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484846327)
Nit: can you please reformat the change with latest clang-format?
Nit2: this seems to belong closer to `GetCacheSize`, one reserves, the other returns the actual size

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484854164)
we're also just adding stuff to the `m_inputs` without any reservation, the first few times we will have some needless copying - we could likely alleviate that by doing a rough reservation
```C++
m_txids.reserve(block.vtx.size());
m_inputs.reserve(2 * block.vtx.size()); // rough guess
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484858152)
I understand if you don't want to bother with this, but how many of these do we expect per block?
I wonder if we want to incur the hashing cost here or if using a sorted set or a sorted vector with `std::binary_search` (or even (unsorted?) SIMD-enabled linear scan) would be faster or simpler here.

I also thought of whether we could just add these to `temp_cache` directly, but that would likely pollute the up-coming validation (unless we can differentiate these from existing inputs).

----
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484851139)
I like that we're doing this, we've usually avoided preallocating our maps - we should do it more often!

-----

I know we're providing empty caches via the parameters, but for completeness either assert that they're empty or:

```suggestion
temp_cache.Reserve(temp_cache.GetCacheSize() + m_inputs.size() + outputs_count);
```

----

I have added
```C++
Assert(temp_cache.GetCacheSize() <= temp_cache.GetCacheSize() + m_inputs.size() + outputs_count);
```
after the `f
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484905167)
nit:
```suggestion
m_worker_threads.emplace_back([this, n] {
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484907043)
nit:
```suggestion
explicit Input(const COutPoint& o LIFETIMEBOUND) noexcept : outpoint{o} {}
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2484898679)
This seems like a code smell that we should pay attention to. I have already brought this up a year ago in https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1722286606, seems it keep biting us.
Instead of adding a new method that basically does the same, can we keep `GetCoin` to simply return the coin and move the spentness checks to the call sites?
I understand that it would likely require another PR, but it would make this one cleaner - I don't like that we need a workaround for a sc
...

💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2485001950)
[Done](https://github.com/bitcoin/bitcoin/compare/e4e4f05f18b1791a4625175c22a154cbcb94c371..0ac969cddfdba52f7947e9b140ef36e2b19c2c41)

💬 cobratbq commented on issue "Is not demanding low-R grinding still relevant for external signers?":
(https://github.com/bitcoin/bitcoin/issues/33761#issuecomment-3478379562)
This seems to do the job. Right, so high-`s` was the problem. Given that it produces signatures of both 222 bytes and 223 bytes, high `R` must indeed (still) be accepted.

✅ cobratbq closed an issue: "Is not demanding low-R grinding still relevant for external signers?"
(https://github.com/bitcoin/bitcoin/issues/33761)

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485033847)
I did try and use a sorted vector and do binary search in the workers, but it was not a measurable performance difference. In theory it should be faster since txid comparison is much faster than siphash. I can do this if you want, but I think it makes the code clearer using the `unordered_set`.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485039220)
This is a consequence of skipping the main cache and writing directly to the temp cache.

I don't think we can modify `GetCoin` to not return spent coins, it's part of the definition [Retrieve the Coin (unspent transaction output) for a given outpoint.](https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L310). This also goes backwards from your change https://github.com/bitcoin/bitcoin/pull/30849 where you added TODOs to not return spent coins where our test `GetCoin`s do. I don't thin
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485040051)
It was for false sharing. I don't think it has a measurable effect, but it might on some systems? I think it's harmless to keep, since there's only one InputFetcher.