-
Notifications
You must be signed in to change notification settings - Fork 293
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
host: Make EOF opaque for EXTCODE* instructions #587
Conversation
cf08fe5
to
8891cbc
Compare
} | ||
|
||
bytes32 Host::get_code_hash(const address& addr) const noexcept | ||
{ | ||
// TODO: Cache code hash. It will be needed also to compute the MPT hash. | ||
const auto* const acc = m_state.find(addr); | ||
return (acc != nullptr && !acc->is_empty()) ? keccak256(acc->code) : bytes32{}; | ||
return (acc != nullptr && !acc->is_empty()) ? keccak256(extcode(acc->code)) : bytes32{}; |
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.
Patching EXTCODEHASH
will be a headache for real implementations. The code hash is available directly in the account node. Now you need to additionally load the code to obtain a possibly different hash.
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.
Now you need to additionally load the code to obtain a possibly different hash.
This was always the case with this approach. Need to figure out if it is an EOF account or not (by loading the account).
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.
keccak256("ef00")
should better be memoized or hard-coded
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.
I think this is generally fine for now although it conflicts with the TODO comment above.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #587 +/- ##
=======================================
Coverage 98.05% 98.05%
=======================================
Files 116 117 +1
Lines 11455 11487 +32
=======================================
+ Hits 11232 11264 +32
Misses 223 223
Flags with carried forward coverage won't be shown. Click here to find out more.
|
0f1e606
to
1012ec1
Compare
Uses tests from ethereum/tests#1353, which contains some changes on top of the currently used tag for testing EOF in CI state-tests. |
No description provided.