Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: vm.etch() leaves storage uninitialized #363

Merged
merged 4 commits into from
Sep 17, 2024
Merged

Conversation

karmacoma-eth
Copy link
Collaborator

this means that running etch'ed bytecode that reads from storage can result in KeyErrors from ex.storage

fixes #362

this means that running etch'ed bytecode that reads from storage can result in KeyErrors from ex.storage

fixes #362
@karmacoma-eth
Copy link
Collaborator Author

TODO: add test, but it fixes the repro in #362

@@ -721,6 +723,9 @@ def handle(sevm, ex, arg: ByteVec, stack, step_id) -> ByteVec | None:
code_bytes = arg[code_loc : code_loc + code_length]
ex.set_code(who, code_bytes)

# vm.etch() initializes but does not clear storage
ex.storage.setdefault(who, sevm.mk_storagedata())

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!


sevm.sstore(ex, store_account_alias, store_slot, store_value)
sevm.sstore(ex, store_account, store_slot, store_value)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowing vm.store() on empty accounts needs further discussions. can we focus on the vm.etch() fix in this pr, without making the above changes to vm.store()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, reverted the vm.store change in 55fba55

@karmacoma-eth
Copy link
Collaborator Author

TODO: add test, but it fixes the repro in #362

test added in 7dc60fc

@karmacoma-eth karmacoma-eth merged commit 115e66d into main Sep 17, 2024
53 checks passed
@karmacoma-eth karmacoma-eth deleted the fix/etch-sload branch September 17, 2024 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage is not initialized for etched contracts
2 participants