Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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());
```
💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072675369)
Nit - [developer notes suggests](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md?plain=1#L70-L71) to put the braces to new lines:
```md
- Braces on new lines for classes, functions, methods.
```
💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072678126)
What about structs?
💬 l0rinc commented on pull request "doc: Explain that .gitignore is not for IDE-specific excludes":
(https://github.com/bitcoin/bitcoin/pull/32417#issuecomment-2849351198)
ACK fada115cbeaa8c0ca3d19178499079b66ee5f499

PR title and commit message still state IDE, but it's not important.
💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072683803)
`AfterStruct` seems to fall back to `AfterClass` if unspecified https://github.com/bitcoin/bitcoin/blob/master/src/.clang-format#L19
💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072685989)
Done.
💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686009)
Done.
💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686020)
Done.
💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686031)
Done.
💬 andrewtoth commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#discussion_r2072686068)
Done.