-
Notifications
You must be signed in to change notification settings - Fork 302
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
EOF code validation (EIP-3670) #366
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,11 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
#include "eof.hpp" | ||
#include "instructions_traits.hpp" | ||
|
||
#include <array> | ||
#include <cassert> | ||
#include <limits> | ||
|
||
namespace evmone | ||
{ | ||
|
@@ -95,15 +97,46 @@ std::pair<EOFSectionHeaders, EOFValidationError> validate_eof_headers(bytes_view | |
return {section_headers, EOFValidationError::success}; | ||
} | ||
|
||
std::pair<EOF1Header, EOFValidationError> validate_eof1(bytes_view container) noexcept | ||
EOFValidationError validate_instructions(evmc_revision rev, bytes_view code) noexcept | ||
{ | ||
const auto [section_headers, error] = validate_eof_headers(container); | ||
if (error != EOFValidationError::success) | ||
return {{}, error}; | ||
assert(code.size() > 0); // guaranteed by EOF headers validation | ||
|
||
size_t i = 0; | ||
uint8_t op = code[0]; | ||
while (i < code.size()) | ||
chfast marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
op = code[i]; | ||
const auto& since = instr::traits[op].since; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is fine for now, but |
||
if (!since.has_value() || *since > rev) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this doesn't cover the case when we want to remove an opcode (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, when that happens I guess we will need to add final revision to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This looks like a good solution if needed. |
||
return EOFValidationError::undefined_instruction; | ||
|
||
i += instr::traits[op].immediate_size; | ||
++i; | ||
} | ||
|
||
if (!instr::traits[op].is_terminating) | ||
chfast marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return EOFValidationError::missing_terminating_instruction; | ||
|
||
return EOFValidationError::success; | ||
} | ||
|
||
std::pair<EOF1Header, EOFValidationError> validate_eof1( | ||
evmc_revision rev, bytes_view container) noexcept | ||
{ | ||
const auto [section_headers, error_header] = validate_eof_headers(container); | ||
if (error_header != EOFValidationError::success) | ||
return {{}, error_header}; | ||
|
||
EOF1Header header{section_headers[CODE_SECTION], section_headers[DATA_SECTION]}; | ||
|
||
const auto error_instr = | ||
validate_instructions(rev, {&container[header.code_begin()], header.code_size}); | ||
if (error_instr != EOFValidationError::success) | ||
return {{}, error_instr}; | ||
|
||
const EOF1Header header{section_headers[CODE_SECTION], section_headers[DATA_SECTION]}; | ||
return {header, EOFValidationError::success}; | ||
} | ||
|
||
} // namespace | ||
|
||
size_t EOF1Header::code_begin() const noexcept | ||
|
@@ -154,7 +187,7 @@ EOFValidationError validate_eof(evmc_revision rev, bytes_view container) noexcep | |
{ | ||
if (rev < EVMC_SHANGHAI) | ||
return EOFValidationError::eof_version_unknown; | ||
return validate_eof1(container).second; | ||
return validate_eof1(rev, container).second; | ||
} | ||
else | ||
return EOFValidationError::eof_version_unknown; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this an iterator instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because with an iterator loop end condition can be only
it != code.end()
, but loop can overrun end in case of truncated immediate.