go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

core/state: auto-finalise state objects during copy

Open karalabe opened this issue 2 years ago • 3 comments

Fixes https://github.com/ethereum/go-ethereum/issues/20119.

When we copy a statedb, we auto-flatten the journal into the dirty objects map. The Copy method did not call finalize on the individual objects, rather deep copied the dirtyness too. That's nice and all, but the statedb.Finalize has an optimization to only finalize "touched" objects (i.e. in the journal), so that would omit finalizing just-flattened stuff from commit.

This is a bit of a pickle:

  • Most importantly, this does not affect us. We never copy a non-finalized statedb.
  • Originally copy was meant to be able to copy kind of all the dirtyness too, but some finality is leaking into it already if we flatten the journal (and not flattening the journal leaks the previous execution stack, which is again weird). It's unclear at this point whether we should just force-finalize before copy, should perhaps error if non-finalized is copied; should try to make it work by copying dirtyness and have statedb.Finalize somehow detect it, etc.

karalabe avatar Dec 07 '23 10:12 karalabe

I think we need to make a conscious decision here: either allow copy of only finalized states (then either implicitly finalize or error out); or allow copying dirty states too, but them IMO copying the entire journal might be needed to avoid forever-cornercases.

In Geth's production code, we would still always Copy finalized stuff, so should be mostly moot for us.

karalabe avatar Dec 07 '23 10:12 karalabe

either allow copy of only finalized states (then either implicitly finalize or error out); or allow copying dirty states too

I kind of would like to allow only copying already (explicitly) finalized state. But we currently don't have any error-return on Copy, which we would need to do that properly. And that might be somewhat breaking

holiman avatar Dec 07 '23 10:12 holiman

but them IMO copying the entire journal might be needed to avoid forever-cornercases.

I would prefer to go with this direction, I have a pull request to support journal copy.

rjl493456442 avatar Dec 07 '23 12:12 rjl493456442

This issue was fixed with a different pr.

karalabe avatar Jun 03 '24 12:06 karalabe