Remove CreateAccount from State implementations#417
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 65 files with indirect coverage changes 🚀 New features to boost your workflow:
|
a80ecb8 to
b32ccef
Compare
CreateAccount from State implementations
There was a problem hiding this comment.
Pull request overview
This PR removes the CreateAccount method from common.UpdateTarget and deletes corresponding CreateAccount implementations/tests, shifting account creation semantics toward “implicit creation” via state-changing operations (balance/nonce/code/storage). It also adds a regression test to ensure implicitly created accounts get the expected empty-code hash.
Changes:
- Removed
CreateAccountfromcommon.UpdateTargetand stopped applyingUpdate.CreatedAccountsviaUpdate.ApplyTo. - Deleted
CreateAccountimplementations in state backends and adjusted MPT tests/benchmarks to create accounts via nonce/balance updates. - Added a StateDB test asserting implicitly created accounts have the empty-code Keccak hash.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go/common/update.go | Removes CreateAccount from UpdateTarget and drops CreatedAccounts handling in ApplyTo. |
| go/state/state_test.go | Removes tests tied to explicit account recreation/creation clearing storage. |
| go/state/state_db_and_state_test.go | Adds test ensuring implicitly created accounts have the empty-code hash; adds sha3 import. |
| go/state/gostate/go_state_test.go | Removes account recreation test that depended on explicit creation semantics. |
| go/state/externalstate/external_state.go | Removes ExternalState.CreateAccount. |
| go/database/mpt/state.go | Removes MptState.CreateAccount. |
| go/database/mpt/state_test.go | Updates benchmark setup to create accounts via SetNonce. |
| go/database/mpt/io/parallel_visit_test.go | Removes CreateAccount usage and relies on SetNonce for setup. |
| go/database/mpt/archive_trie_test.go | Removes CreateAccount usage and relies on SetBalance for setup. |
Comments suppressed due to low confidence (2)
go/database/mpt/io/parallel_visit_test.go:181
errors.Joinis now called with a single argument, which is redundant and makes the surrounding code noisier. Consider replacing it with a directerr = live.SetNonce(...)call, and update the following fatal message (currently "failed to create account") to match what’s actually happening.
addr := common.Address{}
err = errors.Join(
live.SetNonce(addr, common.Nonce{1}),
)
if err != nil {
t.Fatalf("failed to create account: %v", err)
}
go/database/mpt/archive_trie_test.go:723
errors.Joinis now invoked with a single error (live.SetBalance(...)), which is redundant. Simplifying this to a direct assignment would improve readability without changing behavior.
err = errors.Join(
live.SetBalance(addr, blc1),
)
if err != nil {
t.Fatalf("failed to update live db: %v", err)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b32ccef to
b9c2620
Compare
b9c2620 to
2a829c6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 34 changed files in this pull request and generated 8 comments.
Files not reviewed (2)
- go/common/update_mocks.go: Language not supported
- go/database/mpt/state_mocks.go: Language not supported
Comments suppressed due to low confidence (2)
go/common/update.go:48
- PR description notes that the
Updatefield for created accounts still exists, but in this PRCreatedAccountsis removed from the Gocommon.Updatestruct. If this field is intentionally being removed from Go as well, the PR description should be updated; otherwise the field removal here is a discrepancy.
// Valid instances can then be forwarded to the State as a block update.
type Update struct {
DeletedAccounts []Address
Balances []BalanceUpdate
Nonces []NonceUpdate
Codes []CodeUpdate
Slots []SlotUpdate
}
rust/src/types/update.rs:90
- The encoded update format changed (removed the created-accounts length/section) but the decoder still accepts
VERSION_0and the version number is unchanged. Since this encoding is used for cross-language/FI updates (see comment refs togo/common/update.go:ToBytes), consider bumping the version and/or supporting decoding of the previous version to avoid hard-to-diagnose mismatches between components built from different commits.
ad925fb to
c3a6f4b
Compare
c9f9a5f
c9f9a5f to
622472e
Compare
622472e to
26d07a5
Compare
This PR removes the
CreateAccountfunction from theUpdateTargetinterface, and therefore from eachStateimplementation.On his own,
CreateAccountcould never be called, and was always followed by a state-changing operation (balance, nonce, code).This also removes the empty code hash set by the state implementations when creating an account: this was complicating the implementation, adding an unexpected side effect to the update functions.
A note: the
geth-leveldbVK implementation uses the leveldb geth implementation, which sets all the fields in the account update, including the code hash, in each update. Therefore, it is not possible not to set the empty code hash in this implementation. For such reason, tests changing only one value and then checking the resulting hash are skipped for this implementation.Note that the CreateAccount function is still available in the
VMStateDBinterface, where it's called from the EVM.