💬 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.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218080120)
I suppose its legal to call SetDirty on a coin that is already DIRTY-and-FRESH. I'm not sure we need to do anything here in that case.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218080120)
I suppose its legal to call SetDirty on a coin that is already DIRTY-and-FRESH. I'm not sure we need to do anything here in that case.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218081257)
Should we remove its freshness in that case?
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218081257)
Should we remove its freshness in that case?
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218084376)
No, that would make this a behavior change rather than a refactor. We only added flags before, so we must only do that now as well.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218084376)
No, that would make this a behavior change rather than a refactor. We only added flags before, so we must only do that now as well.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218086462)
But we have prohibited one of the states here, we can document that with code now instead of comments. If we think it's a behavior change the PR is incorrect, isn't it?
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218086462)
But we have prohibited one of the states here, we can document that with code now instead of comments. If we think it's a behavior change the PR is incorrect, isn't it?
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218086535)
I can make a follow-up to remove spent fresh and dirty entries from the test cases as well. I don't think that should happen in this PR though to keep it focused.
The previous PR also did this.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218086535)
I can make a follow-up to remove spent fresh and dirty entries from the test cases as well. I don't think that should happen in this PR though to keep it focused.
The previous PR also did this.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218092689)
Your suggestion is to assert that if fresh is false then the entry cannot already be fresh. This is not the case. An entry can be made DIRTY-and-FRESH, and then it can be made DIRTY again while already DIRTY-and-FRESH. The current behavior allows this to happen and keeps the entry DIRTY-and-FRESH when SetDirty is called. If we assert that we can't call SetDirty on a DIRTY-and-FRESH entry then it is a behavior change.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2218092689)
Your suggestion is to assert that if fresh is false then the entry cannot already be fresh. This is not the case. An entry can be made DIRTY-and-FRESH, and then it can be made DIRTY again while already DIRTY-and-FRESH. The current behavior allows this to happen and keeps the entry DIRTY-and-FRESH when SetDirty is called. If we assert that we can't call SetDirty on a DIRTY-and-FRESH entry then it is a behavior change.