Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

CREATE2 implementation in aleth-interpreter #5125

Merged
merged 6 commits into from
Jul 26, 2018
Merged

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Jul 24, 2018

#5119

Required change in EVMC ethereum/evmc#45

@gumb0 gumb0 added this to the Constantinople milestone Jul 24, 2018
@codecov-io
Copy link

codecov-io commented Jul 24, 2018

Codecov Report

Merging #5125 into develop will increase coverage by 0.77%.
The diff coverage is 65.51%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5125      +/-   ##
===========================================
+ Coverage    58.68%   59.46%   +0.77%     
===========================================
  Files          337      337              
  Lines        27126    27054      -72     
  Branches      3165     3164       -1     
===========================================
+ Hits         15920    16088     +168     
+ Misses       10141     9884     -257     
- Partials      1065     1082      +17

@gumb0 gumb0 removed the in progress label Jul 24, 2018
@gumb0 gumb0 requested a review from chfast July 24, 2018 16:55
{
assert(m_OP == Instruction::CREATE2);
// Pass salt data through EVMC as 32-byte prefix in evmc_message::input_data
saltAndInputData = toBigEndian(salt) + bytesConstRef(&m_mem[off], size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go with this, it must be documented in evmc properly.

I think it is a stretch, however uncontrollably growing evmc_message isn't a better option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because using the message input to pass the salt requires a copy of the init code, I'm more into adding the salt param to the message.

More generic approach would be to ask the VM to come up with the address and put it in the message.destination.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change it to salt as a separate member of evmc_message then.

@gumb0
Copy link
Member Author

gumb0 commented Jul 25, 2018

Still need to support CREATE2 calls through EVM::execute wrapper

msg.sender = m_message->destination;
msg.depth = m_message->depth + 1;
msg.kind = m_OP == Instruction::CREATE ? EVMC_CREATE : EVMC_CREATE2; // FIXME: In EVM-C move the kind to the top.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"EVM-C" -> "EMVC" :)

@gumb0 gumb0 force-pushed the create2-interpreter branch 2 times, most recently from 44eb2e2 to 16a573e Compare July 25, 2018 15:45
@gumb0
Copy link
Member Author

gumb0 commented Jul 25, 2018

I realized that no more changes are needed in EVM and ExtVMFace, because if it's the execution of CREATE2 itself (executing contract init code), then it's just the same call to EVM::execute with ExtVMFace::isCreate == true as for CREATE, just with another address in ExtVMFace::myAddress.

So this is ready for review.

@gumb0 gumb0 removed the in progress label Jul 25, 2018
@gumb0 gumb0 merged commit 3ac9424 into develop Jul 26, 2018
@gumb0 gumb0 deleted the create2-interpreter branch July 26, 2018 08:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants