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

💬 optout21 commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3093209323)
ACK 249889bee6b88eb9814eb969e6fb108f86a4bf98

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217572750)
The derived spend private key must be saved to the DB by the wallet.

We can avoid this by doing what other descriptors do; the sp descriptor will then be in this form `sp(xpriv/352h/0h/0h/1h/0,xpub/352h/0h/0h/0h/0)` . The `Parse` function will then derive the scan key and save it in the descriptor. The spend key can be derived later when needed from the master key.

With this alternative method, the wallet only saves the master key to DB as it has always done.