💬 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

💬 maflcko commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2072669835)
The CI should be deterministic and trivially reproducible, as explained in https://github.com/bitcoin/bitcoin/blob/master/ci/README.md. If you can't reproduce it, please share the exact steps you tried, starting from a fresh system. For example, you can start with a fresh Hetzner Cloud VM (or any other fresh VM of your choice).

The integer sanitizer should equally be deterministic and reproducible.

💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2072673025)
I meant I couldn't reproduce any of the supposed failures that suppressions were meant to avoid - but neither is the CI, so I'm not sure what to explain about the removals: they don't seem to be needed for CI to pass.
Unless there's a secret CI which shows these failures - or any other way that you can reproduce them, please let me know how to do it.

👍 hebasto approved a pull request: "doc: Explain that .gitignore is not for IDE-specific excludes"
(https://github.com/bitcoin/bitcoin/pull/32417#pullrequestreview-2813527707)
ACK fada115cbeaa8c0ca3d19178499079b66ee5f499.

💬 maflcko commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2072676770)
In https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2067406415 I was saying it would be good to known since when they are unused. To test an earlier commit, you can just switch to it in git.

👍 l0rinc approved a pull request: "validation: periodically flush dbcache during reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32414#pullrequestreview-2813522966)
Concept ACK

Thanks for fixing it!
I need a bit mor explanation to be able to fully ack

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072670546)
moving `DATABASE_WRITE_INTERVAL_MAX` to the header might be overkill for this, but now that we're repeating it here in multiple tests, consider adding a copy to this file as well.

Nit: I've renamed it to `DB_WRITE_INTERVAL_MAX` in https://github.com/bitcoin/bitcoin/pull/32404/commits/d415f242a6f45229543fabe81d5a6b8b4234c9a6#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R98

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072675975)
👍 I can confirm that this passes after the PR and fails without the PERIODIC flushing moved inside the loop

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072674865)
After we've initialized the mock time we might as well use that as the basis (it also gets rid of the type hint):
```suggestion
SetMockTime(GetMockTime() + 70min);
```

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072677126)
Can you please explain where the 3 separate locks were that we're replacing (I only see 2, but don't yet have experience with how we're locking exactly)?

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072674552)
```suggestion
BOOST_CHECK_EQUAL(tip, chainstate.m_chain.Tip());
```

(and a few others, though displaying the pointer values isn't very useful, unless one of the is a `nullptr`)

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072676586)
My understanding this is equivalent to:
```suggestion
LOCK2(m_node.chainman->GetMutex(), chainstate.MempoolMutex());
```