💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218040983)
note: the cursor contains dirty entries now, so `if (!it->second.IsDirty()) {` on line 186 is also a noop - could we add a TODO there to signal that we're aware of it but want to tackle it in a follow-up?
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218040983)
note: the cursor contains dirty entries now, so `if (!it->second.IsDirty()) {` on line 186 is also a noop - could we add a TODO there to signal that we're aware of it but want to tackle it in a follow-up?
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218050169)
Done.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218050169)
Done.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218050233)
Done.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218050233)
Done.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218050997)
I don't see how that is different. `m_flags` being set implies `IsDirty()` now, so it is equivalent. I prefer not to change this.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218050997)
I don't see how that is different. `m_flags` being set implies `IsDirty()` now, so it is equivalent. I prefer not to change this.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218053411)
`Assume(m_flags);` would be true for fresh only - `Assume(IsDirty());` is stricter
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218053411)
`Assume(m_flags);` would be true for fresh only - `Assume(IsDirty());` is stricter
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218054579)
Isn't that the same as the parent in `CCoinsView`?
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218054579)
Isn't that the same as the parent in `CCoinsView`?
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218061776)
It's not possible to set fresh only now.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218061776)
It's not possible to set fresh only now.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218064237)
Exactly, hence my recommendation
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218064237)
Exactly, hence my recommendation
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218067867)
Done.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218067867)
Done.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218068273)
We can't since we are still calling this with spent, dirty, and fresh entries. These would be deleted in production code. That can also be tackled in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218068273)
We can't since we are still calling this with spent, dirty, and fresh entries. These would be deleted in production code. That can also be tackled in a follow-up.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218068575)
There are quite a few places where we can clean up production and test logic to not have to deal with non-dirty entries in BatchWrite. I think there would be too many TODOs.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218068575)
There are quite a few places where we can clean up production and test logic to not have to deal with non-dirty entries in BatchWrite. I think there would be too many TODOs.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218068664)
Done.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218068664)
Done.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218069073)
You did enable it for a few additional cases, that's already a win 👍
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218069073)
You did enable it for a few additional cases, that's already a win 👍
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218069128)
Done.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218069128)
Done.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218070113)
Err, I did but didn't test locally before pushing. I will have to revert. Tests are failing.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218070113)
Err, I did but didn't test locally before pushing. I will have to revert. Tests are failing.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218071116)
I don't see what can be explained more than the code? This seems obvious to me.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218071116)
I don't see what can be explained more than the code? This seems obvious to me.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218071185)
Can we fix those case of instead? Having a sanitycheck which isn't run just gives us a false sense of security otherwise
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218071185)
Can we fix those case of instead? Having a sanitycheck which isn't run just gives us a false sense of security otherwise
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218071890)
Ok, agree, please resolve the comment
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218071890)
Ok, agree, please resolve the comment
💬 Lalaweazel commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#issuecomment-3095031799)
' DQ at need f cc 2iooo9 to be in work iiiiiiii iroieeieeieowwiereo W e
On Mon, 21 July 2025, 12:02 pm Andrew Toth, ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/coins.h
> <https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218067867>:
>
> > * - spent, not FRESH, DIRTY (e.g. a coin is spent and spentness needs to be flushed to the parent)
> */
> struct CCoinsCacheEntry
> {
> private:
> /**
>
...
(https://github.com/bitcoin/bitcoin/pull/33018#issuecomment-3095031799)
' DQ at need f cc 2iooo9 to be in work iiiiiiii iroieeieeieowwiereo W e
On Mon, 21 July 2025, 12:02 pm Andrew Toth, ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/coins.h
> <https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218067867>:
>
> > * - spent, not FRESH, DIRTY (e.g. a coin is spent and spentness needs to be flushed to the parent)
> */
> struct CCoinsCacheEntry
> {
> private:
> /**
>
...
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218079735)
Ah right, removed.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218079735)
Ah right, removed.