💬 jonatack commented on pull request "rpc, cli: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31135#issuecomment-2432156438)
@laanwj I looked at doing it in `GuessVerificationProgress` but worried that reviewers might prefer not to touch it or have the logging output change, and so initially proposed the smaller-scoped, user-facing-only approach. It looked like the GUI only would call it from the console via the RPCs updated here (but may have overlooked something). Will try the approach @sipa suggested in https://github.com/bitcoin/bitcoin/pull/31135#discussion_r1812643857.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812779405)
This is blocking so we can access the queue of shared outpoints that we need to fetch from. It is not blocking for LevelDB, we access the db once we are out of the critical section.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812781136)
Yes, I can add these but I am waiting for some more conceptual support.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812785573)
It won't process twice, but it could pass in an empty vector, which is ignored if you look at `Add` implementation.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812794214)
I think it would be similar in complexity, we would still need all the locking mechanisms to prevent multithreaded access.

What would really be great is if we had a similar construction to Rust's `std::sync::mpsc`.

💬 fjahr commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2432238813)
Concept ACK but shouldn't this go through a deprecation cycle?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812797796)
I'm not sure it would warrant the complexity I think this batch size is "good enough" for now. In a follow up we could maybe add ways to set this with configs to experiment if there really is more optimal settings.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812800076)
Adding more threads will require more memory, which is one reason to not use many more.

I did a benchmark using 64 threads on the same 16 vcore machine, and it was slightly slower :/

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812807883)
I don't think there is any lock contention here if we are doing multithreaded reading?

I also think what you're suggesting would add a lot more complexity to this PR, when this is "good enough".

💬 maflcko commented on pull request "rpc, cli: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31135#discussion_r1812811243)
Not sure about this. Hard-coding the value to `1` when `height == headers` means that the value may inconsistently jump from 1 to 0.xxxx (in a loop), when headers pre-sync is disabled and the node is fed blocks one-by-one (header before block).

Also, if the node is eclipsed (intentionally, or just accidentally due to a network config error), this may also return `1`, when the node is far behind the real network.

🤔 maflcko reviewed a pull request: "rpc, cli: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31135#pullrequestreview-2388797020)
Not sure about the current fix. It seems to "fix" one style issue in a value that is meant as an estimate, but it may be adding two new issues at the same time?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812815888)
Unsure, copied from `CScriptCheck`. If the state of the art of thread naming has advanced since that was written, please let me know!

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1812825825)
Seems erroneous, indeed.

💬 sipa commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812826580)
The C++ standard library does as far as I know have no way of renaming threads at all. `src/util/threadnames.{h,cpp}` is our wrapper around the various platform-dependent ways of doing so on supported systems.

💬 maflcko commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#issuecomment-2432289339)
CI failure is unrelated, see https://github.com/bitcoin/bitcoin/issues/30969

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1812829296)
~0 on keeping it, we're reducing loc and don't really need the coverage anymore imo

Willing to be over-ridden by other reviewers.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812838378)
Can you tell me why we need to prevent multithreaded access exactly? We could collect the values to different vectors, each one accessed only by a single thread and merge them into the cache at the end on a single thread, right?

How would `mpsc` solve this better? Do you think we need work stealing to make it perfectly parallel? Wouldn't coroutines already achieve the same?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812842053)
4x may be too much to begin with, but 1.5-2x sounds plausible, I'll help with benchmarking this once my current batches finish.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812845859)
This might be as simple as sorting by tx before we create the buckets.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1812849403)
Thank you, please resolve the comment.