chore: clean up unused EOF code #10715
Conversation
…n - no change in handling
zerosnacks
requested review from
DaniPopes,
klkvr,
mattsse,
grandizzy and
yash-atreya
as code owners
| // common create_end functionality for both legacy and EOF. | ||
| fn create_end_common( | ||
| &mut self, | ||
| ecx: Ecx, | ||
| call: Option<&CreateInputs>, | ||
| outcome: &mut CreateOutcome, | ||
| ) { |
There was a problem hiding this comment.
this is the main change, haven't reviewed this extractions line by line but this looks okay
There was a problem hiding this comment.
The diff appears large but it is basically just a direct inline of create_common / create_end_common as they were previously a shared implementation with EOF's create_* and it would now be an unnecessary abstraction to keep around
There was a problem hiding this comment.
I think this looks good and confident all OK but the debugger changes which we don't have tests for (left some comments, happy to work on a basic test suite to perform).
Also, just a note that we're going to have (at least?) one more iteration to cleanup EOF, see bluealloy/revm#2644
| @@ -14,32 +11,8 @@ impl OpcodeParam { | |||
| /// Returns the list of named parameters for the given opcode, accounts for special opcodes | |||
| /// requiring immediate bytes to determine stack items. | |||
| #[inline] | |||
| pub(crate) fn of(op: u8, immediate: Option<&Bytes>) -> Option<Vec<Self>> { | |||
There was a problem hiding this comment.
in is_jump we keep the way we identify using immediate_bytes but here we don't rely on it anymore, wonder if this is right?
There was a problem hiding this comment.
this is correct, I prefer keeping this abstraction as it is more readable than reverting to what was defined previously inline, ref: paradigmxyz/revm-inspectors#301 (record_immediate_bytes). This is an abstraction introduced as part of the EOF PR but not strictly an EOF feature.
There was a problem hiding this comment.
lgtm, ideally we should be able to combine this with bump to bluealloy/revm#2644
|
I'll follow up in the Revm 25+ bump with the final removal of any EOF related fields to be deprecated in the next Revm release |
Motivation
Related PRs:
Updates:
Solution
Removes EOF leftovers, simplifies where possible + retaining abstractions that help readability
PR Checklist