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

feat: scroll account #39

Merged
merged 21 commits into from
Nov 28, 2024
Merged

Conversation

greged93
Copy link
Collaborator

@greged93 greged93 commented Nov 20, 2024

Propagates all changes made on accounts from #33. This PR should only be reviewed once #33 is merged.

Adds test_create in the execution stage in order to test the runtime computation of the Scroll fields. The caching characteristic is already covered by test_selfdestruct.

Builds towards #7

@greged93 greged93 mentioned this pull request Nov 22, 2024
@greged93 greged93 force-pushed the feat/scroll-account branch 2 times, most recently from 8583350 to d17f014 Compare November 25, 2024 11:09
crates/revm/src/database.rs Outdated Show resolved Hide resolved
Signed-off-by: Gregory Edison <[email protected]>
Signed-off-by: Gregory Edison <[email protected]>
Signed-off-by: Gregory Edison <[email protected]>
Signed-off-by: Gregory Edison <[email protected]>
Signed-off-by: Gregory Edison <[email protected]>
Signed-off-by: Gregory Edison <[email protected]>
@greged93 greged93 force-pushed the feat/scroll-account branch from dc00745 to 1a6bb9d Compare November 25, 2024 15:59
Signed-off-by: Gregory Edison <[email protected]>
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

Looks good! I've left some comments in line. Regarding indentation of dependencies lets try and stay consistent with the formatting currently used in reth. Looks like there's a few Cargo.toml files in which we diverge. I also think we should not change the HashedPostState type. I would like to understand your reasoning for doing so. Also added comment around the pattern for finalization of the State<DB> I suspect we can simplify this but wanted to see from your perspective if there's a blocker to the proposed solution.

crates/blockchain-tree/Cargo.toml Outdated Show resolved Hide resolved
crates/blockchain-tree/Cargo.toml Outdated Show resolved Hide resolved
crates/engine/util/src/reorg.rs Show resolved Hide resolved
crates/revm/Cargo.toml Outdated Show resolved Hide resolved
crates/revm/Cargo.toml Outdated Show resolved Hide resolved
crates/trie/common/Cargo.toml Show resolved Hide resolved
crates/trie/common/Cargo.toml Outdated Show resolved Hide resolved
crates/trie/trie/src/state.rs Outdated Show resolved Hide resolved
crates/trie/trie/src/state.rs Outdated Show resolved Hide resolved
crates/trie/trie/src/state.rs Outdated Show resolved Hide resolved
@greged93 greged93 force-pushed the feat/scroll-account branch from 19efb8d to d59038b Compare November 27, 2024 14:38
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

Looks good, left one comment inline in regards to doc comment change which I think we should revert unless there is good reason not to.

examples/custom-engine-types/src/main.rs Show resolved Hide resolved
@frisitano
Copy link
Collaborator

LGTM

@frisitano frisitano merged commit 9730a0f into scroll-tech:scroll Nov 28, 2024
40 checks passed
@frisitano frisitano mentioned this pull request Dec 6, 2024
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.

2 participants