diff --git a/bins/revme/src/cmd.rs b/bins/revme/src/cmd.rs index 47769c2aca..281fcf3019 100644 --- a/bins/revme/src/cmd.rs +++ b/bins/revme/src/cmd.rs @@ -34,6 +34,11 @@ pub enum Error { KzgErrors(#[from] format_kzg_setup::KzgErrors), #[error(transparent)] EvmRunnerErrors(#[from] evmrunner::Errors), + #[error("Eof validation failed: {:?}/{total_tests}", total_tests-failed_test)] + EofValidation { + failed_test: usize, + total_tests: usize, + }, #[error("Custom error: {0}")] Custom(&'static str), } diff --git a/bins/revme/src/cmd/eofvalidation.rs b/bins/revme/src/cmd/eofvalidation.rs index 9fd0d20201..403a8e16a2 100644 --- a/bins/revme/src/cmd/eofvalidation.rs +++ b/bins/revme/src/cmd/eofvalidation.rs @@ -23,12 +23,11 @@ impl Cmd { if !self.path.exists() { return Err(Error::Custom("The specified path does not exist")); } - run_test(&self.path); - Ok(()) + run_test(&self.path) } } -pub fn run_test(path: &Path) { +pub fn run_test(path: &Path) -> Result<(), Error> { let test_files = find_all_json_tests(path); let mut test_sum = 0; let mut passed_tests = 0; @@ -73,6 +72,14 @@ pub fn run_test(path: &Path) { } } } - println!("Types of error: {:#?}", types_of_error); println!("Passed tests: {}/{}", passed_tests, test_sum); + if passed_tests != test_sum { + println!("Types of error: {:#?}", types_of_error); + Err(Error::EofValidation { + failed_test: test_sum - passed_tests, + total_tests: test_sum, + }) + } else { + Ok(()) + } } diff --git a/crates/interpreter/src/interpreter/analysis.rs b/crates/interpreter/src/interpreter/analysis.rs index b214f34a72..dc5bd44dfb 100644 --- a/crates/interpreter/src/interpreter/analysis.rs +++ b/crates/interpreter/src/interpreter/analysis.rs @@ -293,11 +293,9 @@ pub enum EofValidationError { SubContainerNotAccessed, /// Data size needs to be filled for ReturnContract type. DataNotFilled, - /// RETF opcode found in non returning section. - RETFInNonReturningSection, - /// JUMPF opcode found in non returning section and it does - /// not jumps to non returning section - JUMPFInNonReturningSection, + /// Section is marked as non-returning but has either RETF or + /// JUMPF to returning section opcodes. + NonReturningSectionIsReturning, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -449,8 +447,7 @@ impl fmt::Display for EofValidationError { Self::SubContainerCalledInTwoModes => "Sub container called in two modes", Self::SubContainerNotAccessed => "Sub container not accessed", Self::DataNotFilled => "Data not filled", - Self::RETFInNonReturningSection => "RETF in non returning code section", - Self::JUMPFInNonReturningSection => "JUMPF in non returning code section", + Self::NonReturningSectionIsReturning => "Non returning section is returning", } ) } @@ -518,6 +515,8 @@ pub fn validate_eof_code( let mut next_smallest = this_types.inputs as i32; let mut next_biggest = this_types.inputs as i32; + let mut is_returning = false; + let mut i = 0; // We can check validity and jump destinations in one pass. while i < code.len() { @@ -652,16 +651,11 @@ pub fn validate_eof_code( } tracker.access_code(target_index); - // TODO check if this is correct. - if target_types.is_non_returning() != this_types.is_non_returning() { - // JUMPF in non returning code section and it does not jumps to non returning section - return Err(EofValidationError::JUMPFInNonReturningSection); - } - if target_types.is_non_returning() { // if it is not returning stack_requirement = target_types.inputs as i32; } else { + is_returning = true; // check if target code produces enough outputs. if this_types.outputs < target_types.outputs { return Err(EofValidationError::JUMPFEnoughOutputs); @@ -724,10 +718,8 @@ pub fn validate_eof_code( } opcode::RETF => { stack_requirement = this_types.outputs as i32; - - if this_types.is_non_returning() { - return Err(EofValidationError::RETFInNonReturningSection); - } + // mark section as returning. + is_returning = true; if this_instruction.biggest > stack_requirement { return Err(EofValidationError::RETFBiggestStackNumMoreThenOutputs); @@ -800,6 +792,12 @@ pub fn validate_eof_code( i += 1 + opcode.immediate_size() as usize + rjumpv_additional_immediates; } + // error if section is returning but marked as non-returning. + if is_returning == this_types.is_non_returning() { + // wrong termination. + return Err(EofValidationError::NonReturningSectionIsReturning); + } + // last opcode should be terminating if !is_after_termination { // wrong termination. @@ -925,7 +923,7 @@ mod test { assert_eq!( eof, Err(EofError::Validation( - EofValidationError::JUMPFInNonReturningSection + EofValidationError::NonReturningSectionIsReturning )) ); }