💬 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 ?

💬 dergoegge commented on pull request "fuzz: avoid returning non-conforming results from FuzzedSock::GetSockName()":
(https://github.com/bitcoin/bitcoin/pull/32109#issuecomment-3498443216)
Also leaning towards concept NACK on this

1. It does not seem like fuzzers struggle to produce canonical `getsockname` results using our current approach
2. Even if super unlikely, this would catch incorrect handling of `getsockname` results (in case of OS bug) on our side, which seems like a nice thing if it's basically free (given 1.)

👍 stickies-v approved a pull request: "util: Allow `Assert` (et al.) in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3429500574)
ACK fad6efd3bef1d123f806d492f019e29530b03a5e

💬 stickies-v commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2500004322)
There are 2 places in `validation.cpp` where `STR_INTERNAL_BUG` could be used too, but I suppose you didn't because it (afaik) requires us to abandon the compile time format string checking?

📝 waketraindev converted_to_draft a pull request: "Comment out sensitive console commands in history to prevent re-execution"
(https://github.com/bitcoin-core/gui/pull/909)
Prefix filtered commands with "#" before adding them to the RPC console history.

Console lines starting with "#" are ignored by the executor, preventing accidental re-execution of filtered sensitive commands when recalled from history.

Also adds a noop signal to handle ignored commands cleanly and documents comment behavior in the console help text (`help-console`).

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500031838)
This is true when all inputs have been fetched from the block.

💬 Crypt-iQ commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3498504204)
crACK fa6c0bedd33ac7ad27454adaf9522fd27bef6ea3

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500047237)
The work done on the other threads is much slower than what the main thread is doing. Fetching inputs possibly from disk vs inserting coins into the temporary cache. So, in cases where there are fewer worker threads the main thread will likely be waiting on the work to be done. In these cases we can just start fetching as well. I think this would have a large impact in cases where there are few worker threads (like rpis), or if using a low but > 1 -par value. For instance, using `-par=2` this wo
...

🤔 vasild reviewed a pull request: "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency"
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3429510882)
Reviewed as of 016ab85a13

💬 vasild commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2500012759)
nit:
```suggestion
UniValue ProcessReply(const UniValue& batch_in) override
```