💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3497856893)
_Apologies for the force pushes, I am attempting to fix a C.I failure which happens only remotely due to `SetMockTime` not able to nudge the node clock in ms, I adjusted the time advances to be in seconds which a large buffer_

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499419184)
when can this be true?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499427113)
nit: trailing newline shouldn't be needed anymore

```suggestion
LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Warning, "InputFetcher failed to fetch input: %s.", e.what());
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499420838)
```suggestion
if (std::ranges::binary_search(m_txids, input.outpoint.hash.ToUint256().GetUint64(0))) {
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499430732)
```suggestion
std::ranges::sort(m_txids);
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499446404)
not yet sure I fully understand why this is needed - it's more complex, but does it result in at least a theoretic speedup?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499464047)
should we document here what happens in case of a cache miss or collision?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499449442)
`continue` skipping a single line is a bit confusing an dangerous - it should only skip the next line:
```suggestion
if (!input.coin.IsSpent()) {
temp_cache.EmplaceCoinInternalDANGER(COutPoint{input.outpoint}, std::move(input.coin));
}
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499442182)
based on https://en.cppreference.com/w/cpp/atomic/atomic_flag/wait.html
```suggestion
input.ready.wait(/*old*/false, std::memory_order_acquire);
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499486476)
this will fix the linter failure as well (whitespace after `*`):
```suggestion
*
* @return whether there are more inputs in the queue to fetch
```

💬 jurraca commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3497961898)
ACK 44717da29b626bfdcb20a32318689d74c4113b7b

I think it's a better pattern to force users to specify the filename, but I can't tell whether users currently depend on the default.

💬 sipa commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3498015913)
I'm neutral on this PR.

I do use the `asmap.dat` loading by default, but (a) don't care about changing that to specify it explicitly and (b) I hope #28794 happens soonish so it becomes a non-issue (as I will switch to built-in asmap files rather than explicit ones in the datadir).

💬 maflcko commented on pull request "script: remove dead code in `CountWitnessSigOps`":
(https://github.com/bitcoin/bitcoin/pull/33786#issuecomment-3498072962)
review ACK 24bcad3d4df59690f30c9df8ebb62f0bddd0f1c7 🐏

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 24bcad3d4df5
...

💬 maflcko commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#issuecomment-3498077725)
Since when is this "unused"?

💬 l0rinc commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#issuecomment-3498102849)
> Since when is this "unused"?

Can you be more specific please?
These methods were always called with the same values (`nullptr`), always disabling that functionality.

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3498178885)
> CBlockIndexWorkComparator seems unrelated to the issue

The original issue won't be fixed since it's not exactly a bug, but we could still speed up the regression by optimizing another bottleneck.
Since you found the etymology confusing, I have removed the details from the description, only mentioned https://github.com/bitcoin/bitcoin/blob/master/src/validation.h#L637 for the flame graph that show this being one of the bottlenecks. Not the biggest one, but the one we can easily optimize and
...

💬 Pttn commented on issue "qt: Amount field too narrow on Windows in Send Coins dialog":
(https://github.com/bitcoin-core/gui/issues/906#issuecomment-3498286137)
Can confirm on several Windows 11 24H2/25H2 machines with Bitcoin Core Master 5c5704e730796c6f31e2d7891bf6334674a04219.
Built with Guix on Debian 12. For some reason, the field is fine if running the .Exe via Wine...

💬 vasild commented on pull request "Run feature_bind_port_(discover|externalip).py in CI":
(https://github.com/bitcoin/bitcoin/pull/33362#issuecomment-3498341743)
`9b30b0bea8...e245da32dc`: rebase due to conflicts

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2499916845)
can we call it something else to not coincide with `CCoinsViewCache::FetchCoin`

💬 vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2499928634)
@mzumsande ?