🤔 djs19901 reviewed a pull request: "depends: add missing Darwin objcopy"
(https://github.com/bitcoin/bitcoin/pull/31840#pullrequestreview-3035634094)
bc1q5vef369tqlwztc08rwjjn3y6986ds56z3hfgg7

📝 l0rinc opened a pull request: "test: revive test verifying that `GetCoinsCacheSizeState` switches from OK→LARGE→CRITICAL"
(https://github.com/bitcoin/bitcoin/pull/33021)
After the changes in https://github.com/bitcoin/bitcoin/pull/25325 `getcoinscachesizestate` always end the test early, see:
https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/test/validation_flush_tests.cpp.gcov.html

The test revival was extracted from a related PR where it was discovered, see: https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109417797

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2217521200)
I have extracted the suggested test to a separate PR: https://github.com/bitcoin/bitcoin/pull/33021

👍 l0rinc approved a pull request: "log: [refactor] Use info level for init logs"
(https://github.com/bitcoin/bitcoin/pull/32967#pullrequestreview-3035642619)
Concept ACK, thanks for the cleanup.

I personally would prefer doing every single `LogPrintf` -> `LogInfo` inlining here in a scripted diff (so we can get rid of the alias), with follow-up commits fixing the line endings and formatting and stuff.

And I agree that the warn/error split should be done in a separate PR where we can focus on the context (we could probably do a scripted diff for most of the lines that contain warning/error to cover most of those as well)

💬 l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217522134)
Would it be possible to separate the `LogPrintf` inlining from the `__func__` and rewording changes into focused commits?

💬 l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217527057)
I personally would find it simpler if we inlined every single `LogPrintf` call here, removed the [alias](https://github.com/bitcoin/bitcoin/blob/master/src/logging.h#L369-L370) and the [dedicated test](https://github.com/bitcoin/bitcoin/blob/master/src/test/logging_tests.cpp#L131), and do the info/warn/error differentiation in a separate PR.
I know that means we'd be touching the same lines multiple times, but in that case this PR could simply be a scripted diff with alias removal (+ line endin
...

💬 l0rinc commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3092689738)
We're making progress, https://github.com/bitcoin/bitcoin/pull/31144 was just merged! 🎉
The next ones that need some love are:
- https://github.com/bitcoin/bitcoin/pull/32827
- https://github.com/bitcoin/bitcoin/pull/32497
- https://github.com/bitcoin/bitcoin/pull/32279

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217563506)
Without `sp_keys`, the wallet can't identify which keys are scan and spend keys from the signing provider. Do you have an alternative approach to this?

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217567798)
Done

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217567956)
Fixed.

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568207)
Done

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568262)
Done

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568307)
I've dropped this commit from this branch.

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568482)
Done

💬 optout21 commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2217569237)
Very low relevance, but "updating the fee logic" is a nit confusing to me:
- why "updating" and not "update"? "remove ..., updating ..." suggests it's a side effect in the same step as opposed to a separate step
- is it `(updating the fee) logic` or `updating the (fee logic)`? What logic is updated?

My suggestion: `also update fee stats` or just `also update fees`.
Otherwise LGTM.

💬 ajtowns commented on pull request "tests: speed up coins_tests by parallelizing":
(https://github.com/bitcoin/bitcoin/pull/32945#issuecomment-3093204312)
> This branch (with a small patch):

Nice, that seems to have fixed my cmake errors.

💬 l0rinc commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2217569835)
"Remove... while also update" -> "Remove..., updating".

If you agree with the change please comment an uppercase ACK with the latest commit hash to your message.

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217570563)
Maybe they deserve their own PR?

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217570887)
Don't we need to derive the scan and spend keys?

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217571215)
In the `OutputType::SILENT_PAYMENTS` case, I set the `desc_str` not the `desc_prefix`; I have to modify the `assert` to prevent a failure.