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

EVM backend overlay #2611

Merged
merged 24 commits into from
Oct 25, 2023
Merged

EVM backend overlay #2611

merged 24 commits into from
Oct 25, 2023

Conversation

Jouzo
Copy link
Collaborator

@Jouzo Jouzo commented Oct 23, 2023

Summary

  • Keeps track of block changes in in-memory overlay layer
  • Keeps EVMBackend in BlockTemplate
  • Commit backend a single time on construct_block
  • Remove cache by state root
  • Keep vector of changeset to be able to revert to an overlay state at a particular TX (used by remove_txs_above_hash)

})
}

pub fn increase_tx_count(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

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

Will renaming this method to something like cache_state make it more readable?

Comment on lines +77 to +81
if let Some(existing_storage) = self.storage(&address) {
for (k, v) in existing_storage {
storage.entry(*k).or_insert_with(|| *v);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if I am seeing this correctly, but it seems like by doing this, we could potentially overwrite the current storage state with an old value with the same key in the existing storage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, in the case where we would write multiple time to the same storage slot in a block (such as TD total_supply), we would overwrite the current storage with the updated value

Copy link
Member

Choose a reason for hiding this comment

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

Isn't existing_storage the old value? Meaning that we are overwriting the updated value with the old value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So it keeps the storage entry as is with .entry(*k) or if the key does not exist in storage, insert the existing key val from existing_storage. Feels correct but we can add some test if needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Logic could probably be reversed since we can assume that existing_storage will grow and be on average bigger than storage

sieniven
sieniven previously approved these changes Oct 25, 2023
@Jouzo Jouzo mentioned this pull request Oct 25, 2023
@prasannavl prasannavl merged commit 07ba855 into master Oct 25, 2023
14 of 15 checks passed
@prasannavl prasannavl deleted the evm_backend_overlay branch October 25, 2023 14:58
prasannavl added a commit that referenced this pull request Oct 26, 2023
Jouzo added a commit that referenced this pull request Oct 26, 2023
prasannavl added a commit that referenced this pull request Oct 26, 2023
* Revert "EVM backend overlay (#2611)"

This reverts commit 07ba855.

* Pass str copy to prevent incorrect lambda optimizations

* Add pre-cache with minimal locking

* fmt

* Revert "Revert "EVM backend overlay (#2611)""

This reverts commit f8106f2.

* Minor refactors, eager caching

* Add WaitForCompletion

---------

Co-authored-by: jouzo <[email protected]>
Co-authored-by: Jouzo <[email protected]>
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.

3 participants