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

Rich validation errors #157

Open
chfast opened this issue Sep 3, 2024 · 5 comments
Open

Rich validation errors #157

chfast opened this issue Sep 3, 2024 · 5 comments

Comments

@chfast
Copy link
Member

chfast commented Sep 3, 2024

So far EOF validation implementations return their own error code it is rather difficult to match them one-to-one.

Here is an idea to extend the return type to a tuple:

  • error code (8 bits) (as it is now)
  • error category (8 bits)
  • element index (32 bits?)

The error category is "more standardized" way of informing at what phase of validation the error happened. E.g. header, body, type section, code section, subcontainer.

The index informs at which element the error happened. We need to find a global or category-based recursive indexing.
First of all, we need to find a convenient method of numbering all subcontainers.

@chfast
Copy link
Member Author

chfast commented Sep 3, 2024

This is the categorization currently used in the EOF validation fuzzer.

EOFErrCat get_cat(evmone::EOFValidationError err) noexcept {
  using enum evmone::EOFValidationError;
  switch (err) {
  case invalid_prefix:
  case eof_version_unknown:
  case header_terminator_missing:
  case type_section_missing:
  case code_section_missing:
  case data_section_missing:
  case section_headers_not_terminated:
  case zero_section_size:
  case incomplete_section_size:
  case incomplete_section_number:
  case too_many_code_sections:
  case too_many_container_sections:
  case invalid_type_section_size:
    return EOFErrCat::header;
  case invalid_section_bodies_size:
    return EOFErrCat::body;
  case invalid_first_section_type:
  case inputs_outputs_num_above_limit:
  case max_stack_height_above_limit:
  case toplevel_container_truncated: // ?
    return EOFErrCat::type;
  case undefined_instruction:
  case truncated_instruction:
  case invalid_container_section_index:
  case stack_underflow: // stack?
  case unreachable_instructions:
  case invalid_code_section_index:
  case invalid_rjump_destination:
  case callf_to_non_returning_function:
  case invalid_dataloadn_index:
  case no_terminating_instruction:
  case invalid_non_returning_flag:         // ?
  case invalid_max_stack_height:           // stack?
  case stack_height_mismatch:              // stack?
  case stack_overflow:                     // stack?
  case stack_higher_than_outputs_required: // stack?
  case incompatible_container_kind:
  case jumpf_destination_incompatible_outputs:
    return EOFErrCat::code;
  case unreferenced_subcontainer:
  case eofcreate_with_truncated_container:
  case ambiguous_container_kind:
    return EOFErrCat::subcont;
  case unreachable_code_sections:
    return EOFErrCat::ref;
  case container_size_above_limit:
    return EOFErrCat::other;
  case success:
    assert(!"success");
    __builtin_trap();
  case impossible:
    assert(!"impossible");
    __builtin_trap();
  }
}

@shemnon
Copy link
Contributor

shemnon commented Sep 3, 2024

This starts to move into specifying an implementation instead of a data structure. Besu layout parsing integrates the header parsing and header content parsing, so it always gets order different than evmone. EEST accepted multiple error codes in ethereum/execution-spec-tests#759, which besu has adapted our error codes to include.

@chfast
Copy link
Member Author

chfast commented Sep 3, 2024

This can be a single category then. I don't need this to be very specific, but a way of distinguishing header syntax error from invalid instructions would be nice for fuzzing.

@shemnon
Copy link
Contributor

shemnon commented Sep 3, 2024

So if they fail for different categories it's a fuzz break?

@chfast
Copy link
Member Author

chfast commented Sep 3, 2024

That's the intention. If we cannot make it work there is no point in standardizing it.

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

No branches or pull requests

2 participants