fix(eof): dont run precompile on ext delegate call (bluealloy#1964)

* feat: bump eof validation tests

* fix(eof): dont run precompile on ext delegate call

* updates eof validation paths

* some cleanup

.github/workflows/ethereum-tests.yml

@@ -55,6 +55,5 @@ jobs:
       - name: Run EOF validation tests
         run: |
           cross run --target ${{matrix.target}} --profile ${{ matrix.profile }} -p revme -- eof-validation \
-          ethtests/EOFTests \
-          tests/eof_suite/eest/eof_tests/prague
+          tests/eof_suite/eest/eof_tests/osaka
 

bins/revme/src/cmd/eofvalidation.rs

@@ -81,16 +81,21 @@ pub fn run_test(path: &Path) -> Result<(), Error> {
                 } else {
                     Some(CodeType::ReturnOrStop)
                 };
-                let test_result = test_vector.results.get("Osaka");
+                // In future this can be generalized to cover multiple forks, Not just Osaka.
+                let Some(test_result) = test_vector.results.get("Osaka") else {
+                    // if test does not have a result that we can compare to, we skip it
+                    println!("Test without result: {} - {}", name, vector_name);
+                    continue;
+                };
                 let res = validate_raw_eof_inner(test_vector.code.clone(), kind);
-                if test_result.map(|r| r.result).unwrap_or(res.is_ok()) != res.is_ok() {
+                if test_result.result != res.is_ok() {
                     println!(
                         "\nTest failed: {} - {}\nresult:{:?}\nrevm err_result:{:#?}\nExpected exception:{:?}\nbytes:{:?}\n",
                         name,
                         vector_name,
-                        test_result.unwrap().result,
+                        test_result.result,
                         res.as_ref().err(),
-                        test_result.unwrap().exception,
+                        test_result.exception,
                         test_vector.code
                     );
                     *types_of_error

bins/revme/src/cmd/statetest/runner.rs

@@ -342,12 +342,7 @@ pub fn execute_test_suite(
                 continue;
             }
 
-            // Enable EOF in Prague tests.
-            cfg.spec = if spec_name == SpecName::Prague {
-                SpecId::OSAKA
-            } else {
-                spec_name.to_spec_id()
-            };
+            cfg.spec = spec_name.to_spec_id();
 
             if cfg.spec.is_enabled_in(SpecId::MERGE) && block.prevrandao.is_none() {
                 // If spec is merge and prevrandao is not set, set it to default

crates/handler/src/frame.rs

@@ -126,79 +126,79 @@ where
                 return return_result(i.into());
             }
         }
-
-        if let Some(result) = precompile.run(
-            context,
-            &inputs.bytecode_address,
-            &inputs.input,
-            inputs.gas_limit,
-        )? {
-            if result.result.is_ok() {
-                context.journal().checkpoint_commit();
-            } else {
-                context.journal().checkpoint_revert(checkpoint);
+        let is_ext_delegate_call = inputs.scheme.is_ext_delegate_call();
+        if !is_ext_delegate_call {
+            if let Some(result) = precompile.run(
+                context,
+                &inputs.bytecode_address,
+                &inputs.input,
+                inputs.gas_limit,
+            )? {
+                if result.result.is_ok() {
+                    context.journal().checkpoint_commit();
+                } else {
+                    context.journal().checkpoint_revert(checkpoint);
+                }
+                return Ok(FrameOrResultGen::Result(FrameResult::Call(CallOutcome {
+                    result,
+                    memory_offset: inputs.return_memory_offset.clone(),
+                })));
             }
-            Ok(FrameOrResultGen::Result(FrameResult::Call(CallOutcome {
-                result,
-                memory_offset: inputs.return_memory_offset.clone(),
-            })))
-        } else {
-            let account = context
-                .journal()
-                .load_account_code(inputs.bytecode_address)?;
+        }
 
-            let mut code_hash = account.info.code_hash();
-            let mut bytecode = account.info.code.clone().unwrap_or_default();
+        let account = context
+            .journal()
+            .load_account_code(inputs.bytecode_address)?;
 
-            // ExtDelegateCall is not allowed to call non-EOF contracts.
-            if inputs.scheme.is_ext_delegate_call()
-                && !bytecode.bytes_slice().starts_with(&EOF_MAGIC_BYTES)
-            {
-                return return_result(InstructionResult::InvalidExtDelegateCallTarget);
-            }
+        let mut code_hash = account.info.code_hash();
+        let mut bytecode = account.info.code.clone().unwrap_or_default();
 
-            if bytecode.is_empty() {
-                context.journal().checkpoint_commit();
-                return return_result(InstructionResult::Stop);
-            }
+        // ExtDelegateCall is not allowed to call non-EOF contracts.
+        if is_ext_delegate_call && !bytecode.bytes_slice().starts_with(&EOF_MAGIC_BYTES) {
+            return return_result(InstructionResult::InvalidExtDelegateCallTarget);
+        }
 
-            if let Bytecode::Eip7702(eip7702_bytecode) = bytecode {
-                let account = &context
-                    .journal()
-                    .load_account_code(eip7702_bytecode.delegated_address)?
-                    .info;
-                bytecode = account.code.clone().unwrap_or_default();
-                code_hash = account.code_hash();
-            }
+        if bytecode.is_empty() {
+            context.journal().checkpoint_commit();
+            return return_result(InstructionResult::Stop);
+        }
 
-            // Create interpreter and executes call and push new CallStackFrame.
-            let interpreter_input = InputsImpl {
-                target_address: inputs.target_address,
-                caller_address: inputs.caller,
-                input: inputs.input.clone(),
-                call_value: inputs.value.get(),
-            };
-
-            Ok(FrameOrResultGen::Frame(Self::new(
-                FrameData::Call(CallFrame {
-                    return_memory_range: inputs.return_memory_offset.clone(),
-                }),
-                depth,
-                Interpreter::new(
-                    memory.clone(),
-                    ExtBytecode::new_with_hash(bytecode, code_hash),
-                    interpreter_input,
-                    inputs.is_static,
-                    false,
-                    context.cfg().spec().into(),
-                    inputs.gas_limit,
-                ),
-                checkpoint,
-                precompile,
-                instructions,
-                memory,
-            )))
+        if let Bytecode::Eip7702(eip7702_bytecode) = bytecode {
+            let account = &context
+                .journal()
+                .load_account_code(eip7702_bytecode.delegated_address)?
+                .info;
+            bytecode = account.code.clone().unwrap_or_default();
+            code_hash = account.code_hash();
         }
+
+        // Create interpreter and executes call and push new CallStackFrame.
+        let interpreter_input = InputsImpl {
+            target_address: inputs.target_address,
+            caller_address: inputs.caller,
+            input: inputs.input.clone(),
+            call_value: inputs.value.get(),
+        };
+
+        Ok(FrameOrResultGen::Frame(Self::new(
+            FrameData::Call(CallFrame {
+                return_memory_range: inputs.return_memory_offset.clone(),
+            }),
+            depth,
+            Interpreter::new(
+                memory.clone(),
+                ExtBytecode::new_with_hash(bytecode, code_hash),
+                interpreter_input,
+                inputs.is_static,
+                false,
+                context.cfg().spec().into(),
+                inputs.gas_limit,
+            ),
+            checkpoint,
+            precompile,
+            instructions,
+            memory,
+        )))
     }
 
     /// Make create frame.

crates/statetest-types/src/spec.rs

@@ -50,10 +50,10 @@ impl SpecName {
             Self::Shanghai => SpecId::SHANGHAI,
             Self::Cancun => SpecId::CANCUN,
             Self::Prague => SpecId::PRAGUE,
+            Self::Osaka => SpecId::OSAKA,
             Self::ByzantiumToConstantinopleAt5 | Self::Constantinople => {
                 panic!("Overridden with PETERSBURG")
             }
-            Self::Osaka => panic!("Osaka is not implemented"),
             Self::Unknown => panic!("Unknown spec"),
         }
     }