Implement MCOPY (EIP-5656) #629
Conversation
| @@ -162,6 +162,7 @@ enum Opcode : uint8_t | |||
|
|
|||
| OP_DUPN = 0xb5, | |||
| OP_SWAPN = 0xb6, | |||
| OP_MCOPY = 0xb7, | |||
There was a problem hiding this comment.
The EIP has 0x5c, but that is taken by RJUMP.
lib/evmone/instructions.hpp
Outdated
| @@ -900,6 +900,29 @@ inline code_iterator swapn(StackTop stack, ExecutionState& state, code_iterator | |||
| return pos + 2; | |||
| } | |||
|
|
|||
| inline code_iterator mcopy(StackTop stack, ExecutionState& state, code_iterator pos) noexcept | |||
There was a problem hiding this comment.
This code is based on calldatacopy, given the structure of this feature mirrors that.
The main difference is std::max(dst_index, src_index) ensures memory is expanded to the highest point, so copying can be sure it will copy 0s on padding. This matches the specification in the EIP.
3cf1aa0
to
e6f81b7
Compare
|
Need to add some unit tests. |
| auto copy_size = static_cast<size_t>(size); | ||
|
|
||
| const auto copy_cost = num_words(copy_size) * 3; | ||
| if ((gas_left -= copy_cost) < 0) |
There was a problem hiding this comment.
Could merge these two branches.
There was a problem hiding this comment.
@gumb0 here will be one more use of your helper 😅
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #629 +/- ##
==========================================
+ Coverage 97.31% 97.34% +0.03%
==========================================
Files 80 80
Lines 7677 7729 +52
==========================================
+ Hits 7471 7524 +53
+ Misses 206 205 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@gumb0 are you interested taking this over for adding unit tests? Then imho we could merge it, and move on to state tests. The only question mark is the opcode. |
|
@gumb0 it seems the overlapping one is detected by asan. |
082d461
to
de3e232
Compare
0be2974
to
fa37f7c
Compare
|
I've added an overlapping test with src > dst. |
lib/evmone/instructions.hpp
Outdated
| @@ -900,6 +900,29 @@ inline code_iterator swapn(StackTop stack, ExecutionState& state, code_iterator | |||
| return pos + 2; | |||
| } | |||
|
|
|||
| inline Result mcopy(StackTop stack, int64_t gas_left, ExecutionState& state) noexcept | |||
| { | |||
| const auto& dst_index = stack.pop(); | |||
There was a problem hiding this comment.
| const auto& dst_index = stack.pop(); | |
| const auto dst_index = stack.pop(); |
The .pop() returns a copy of a value so name it like that (reference is technically valid C++ because of some special rule but unnecessary confusion IMHO).
There was a problem hiding this comment.
I changed it, but it's a reference in many other similar places.
There was a problem hiding this comment.
Sorry, I was wrong about it. I forgot the StackTop is just a view type and .pop() does not do much besides helps with counting arguments.
823aec8
to
ec658bb
Compare
Implements EIP-5656: Memory copying instruction.