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: implement EOF methods in Inspector trait #8123

Merged
merged 19 commits into from
Jun 28, 2024

Conversation

fgimenez
Copy link
Collaborator

Motivation

The Inspector trait in revm includes now the eofcreate and eofcreate_end methods with default implementations, we need to add custom implementations for each type implementing the trait

Solution

The implementations aim to have the same behaviour as the existing create and create_end methods.

crates/cheatcodes/src/inspector.rs Outdated Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

okay, I think I understood this.

with EOF we have an additional create hook.
with the new enums we can unify our handlers for both input/output pairs

I think this makes sense and is the best solution

crates/cheatcodes/src/inspector.rs Outdated Show resolved Hide resolved
crates/cheatcodes/src/inspector.rs Outdated Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this approach lgtm, only have a suggestion re mod layout

wdyt @DaniPopes

crates/cheatcodes/src/inspector.rs Outdated Show resolved Hide resolved
crates/cheatcodes/src/inspector.rs Outdated Show resolved Hide resolved
crates/cheatcodes/src/inspector.rs Outdated Show resolved Hide resolved
@DaniPopes
Copy link
Member

Since there hasn't been a revm release yet, I would like to fix this upstream.

@fgimenez
Copy link
Collaborator Author

ok then what is missing in this PR, how can we move it forward? or should we close it? @DaniPopes @mattsse @rakita

@DaniPopes
Copy link
Member

DaniPopes commented Jun 19, 2024

We're waiting on a revm release that includes working EOF

@fgimenez
Copy link
Collaborator Author

ok, updated for latest changes in revm, ptal @DaniPopes @mattsse @rakita

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

lgtm

@DaniPopes DaniPopes merged commit 48b95f9 into foundry-rs:master Jun 28, 2024
19 checks passed
@fgimenez fgimenez deleted the fgimenez/eof-inspectors branch June 28, 2024 15:44
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.

4 participants