💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072622314)
Is this only needed when we're using different durations for the parameters such as `ctx.randrange<std::chrono::seconds>(2s, 2min)`?

The result is a bit ugly now, but I've changed it.

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072622329)
I'm still not sure why we're adding them, but made sure the IDE adds newlines to the end now.

💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2849231289)
I will work on this more in the upcoming weeks, drafted it for now since I don't like the current single-byte specializations, have to find a better way. Suggestions are welcome.

📝 andrewtoth reopened a pull request: "validation: periodically flush dbcache during reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32414)
After #30611 we periodically do a non-erasing flush of the dbcache to disk roughly every hour during IBD.
The intention was to also do this periodic flush during reindex-chainstate, so we would not risk losing progress during a system failure when reindexing with a high dbcache value.

It was discovered that reindex-chainstate does not perform a PERIODIC flush until it has already reached the tip. Since reindexing to tip usually happens within 24 hours, this behaviour was unnoticed with the p
...

💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2849258766)
My guess is that the concept specialization here at most could have an effect on xor'd IO? This is because anything not xor'd won't need a buffer to (un)do the xor before reading or writing.

Thus, it just seems easier to use the existing BufferedReader/Writer in places that could benefit from it?

However, for block serialization they are already used.

Again, no objection here, but it would be good to explain:

* What is the goal of the changes here, and what real-world usage do they a
...

💬 maflcko commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072639000)
> I'm still not sure why we're adding them, but made sure the IDE adds newlines to the end now.

* They are already present, so removing them is just pointless churn
* It is documented in `.editorconfig` and most editors add them by default, if missing. Which will cause churn in the future if they are removed on some files.
* This repo is using git as content management system, which complains about this. Also GitHub web and GitHub diff warn about this. E.g. see the tail of https://github.co
...

💬 maflcko commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072639006)
> Is this only needed when we're using different durations for the parameters such as

No. The function above only takes one parameter. The reason is explained in the comment of the function immediately above. (Line 338)

🤔 furszy reviewed a pull request: "crypto: Use secure_allocator for `AES256_ctx`"
(https://github.com/bitcoin/bitcoin/pull/31774#pullrequestreview-2813492635)
I don't see why the first commit is necessary. Couldn't you just mock the time so that it's fixed and the number of derivation rounds always stays at the default?

💬 hebasto commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2849289055)
@maflcko

> No objection, but assert/Assert seems to be used widely in the codebase, so shouldn't this be done for all test binaries, or none? Otherwise the same assert could lead to inconsistent behavior, depending on which test binary ran into it.

Thanks! Reworked.

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072654348)
Yeah, I know tgese, but it still doesn't explain why - metaphorically speaking - we need a comma after the last element in a list. But I've configured my IDE now to add it, I don't have to agree to accept it :)

💬 maflcko commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072657473)
> Yeah, I know these, but it still doesn't explain why - metaphorically speaking - we need a comma after the last element in a list.

The reason is the same: If there weren't a comma (newline) after the last element (line) in a list (file), a diff would be verbose when a new element (line) is added after the last. Also, the verbose diff breaks `git blame`. Finally, tools may misbehave when the tool or the user assumes a newline is present.

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072657701)
Thanks for explaining

💬 maflcko commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072657734)
I guess it could make sense to add a comment to say that the same rationale as above applies. Otherwise, someone could remove it with the rationale that it isn't needed. But no strong opinion.

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072658130)
I have added a test which uses different durations, it would fail if reverted

📝 maflcko opened a pull request: "doc: Explain that .gitignore is not for IDE-specific excludes"
(https://github.com/bitcoin/bitcoin/pull/32417)
This comes up frequently, so document it.

Examples:

* https://github.com/bitcoin/bitcoin/pull/27275
* https://github.com/bitcoin/bitcoin/pull/21894
* https://github.com/bitcoin/bitcoin/pull/32392
* https://github.com/bitcoin/bitcoin/pull/17868

...

💬 l0rinc commented on pull request "doc: Explain that .gitignore is not for IDE-specific excludes":
(https://github.com/bitcoin/bitcoin/pull/32417#issuecomment-2849316243)
ACK fa9c8bc05e3bb72e2c3c726b643997cea4874158

🤔 hebasto reviewed a pull request: "doc: Explain that .gitignore is not for IDE-specific excludes"
(https://github.com/bitcoin/bitcoin/pull/32417#pullrequestreview-2813515366)
Concept ACK.

I would suggest expanding this list to include desktop environment–specific files, such as `.DS_Store` on macOS.

💬 maflcko commented on issue "`keypoolrefill` doesn't fill keypool to specified parameter":
(https://github.com/bitcoin/bitcoin/issues/29924#issuecomment-2849321859)
I guess it could make sense to clarify the RPC docs to properly document the behavior for descriptor wallets with several ranged descriptors (the default).

💬 maflcko commented on pull request "fees: document non-monotonic estimation edge case":
(https://github.com/bitcoin/bitcoin/pull/31080#issuecomment-2849323269)
@willcl-ark I guess you'll have to push the requested change, or reply to it, or close this pull?

💬 maflcko commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-2849324249)
The dependency was closed more than a year ago: https://github.com/bitcoin/bitcoin/pull/27601#issuecomment-1989555740