EIP 1014 CREATE2 #329

jwasinger · 2018-08-04T06:39:31Z

Depends on ethereumjs/ethereumjs-util#146.

Closes #356.

holgerd77 · 2018-08-30T09:11:30Z

Hi Jared, I've now updated the ethereumjs-testing repo with the latest tests, this also includes your new CREATE2 tests (see also #348 for reference).

holgerd77 · 2018-09-20T09:43:50Z

Tests are now updated to ethereumjs-testing v1.2.2 (#350), which includes the latest tests from the ethereum/tests repository with many new CREATE2 test cases.

@jwasinger Are you planning to continue to work on this? Or is this also open for someone else to be continued (e.g. from Kyokan)?

jwasinger · 2018-09-22T09:38:16Z

Feel free to take this over. I won't have time to complete it before devcon.

rmeissner · 2018-09-27T17:06:44Z

@holgerd77 @jwasinger as I am really interested in playing around with this feature, is there some ETA? Also is there some way I could contribute to get thisndone faster 😄

holgerd77 · 2018-09-28T08:40:18Z

Hi @rmeissner, as a first step I have now updated the according PR on the util library along the latest discussions from the EIP update PR #1375 page, can you do a review of this?

I think one thing you can do is to follow the discussion alongside the PR linked above and come up with some examples for the hash creation - covering different cases - that would also help other implementations a lot.

You can further directly take on this PR if your time permits, that would be great! 😄 I have added you as collaborator to this library, so feel free to directly take on the PR here, rebase and re-submit.

holgerd77 · 2018-09-29T14:23:23Z

Have now updated the generateAddress2 hash/address creation PR over on ethereumjs-util with the added examples from the EIP and along with this fixed a last bug preventing this from working properly.

Tests are now passing and it should now be possible to continue to work on this here on the VM side.

rmeissner · 2018-09-29T19:22:53Z

EIP-1014 makes address collisions possible. As I see it right now in that case the ethereumjs-vm just clears the address and creates the new contract. According to the update to the EIP this should fail. Therefore I would add this check.

Also the gascosts will be dynamic according to the latest comment, so that should be changed too.

I would try to get this done monday/tuesday if it is not done before 😉

holgerd77 · 2018-09-29T20:31:03Z

Cool, pretty sure it won't be done before 😄

rmeissner · 2018-10-02T13:55:30Z

The test cases don't included the correct gas costs yet. Also there are still quite some issues with the constantinople tests (the ci failures can be resolved if the new version if the utils package is released)

holgerd77 · 2018-10-10T10:06:20Z

Just released ethereumjs-util v6.0.0 with the generateAddress2() helper function now included, sorry for the short delay.

rmeissner · 2018-10-10T12:06:55Z

@holgerd77 It seems that just upgrading ethereumjs-utils from 5.2.0 to 6.0.0 is breaking quite some tests, if possible I would appreciate some help with that :)

holgerd77 · 2018-10-10T12:37:49Z

Just looking into it.

holgerd77 · 2018-10-10T12:55:31Z

lib/runCall.js

@@ -5,6 +5,7 @@ const BN = ethUtil.BN
 const exceptions = require('./exceptions.js')

 const ERROR = exceptions.ERROR
+const EMPTY_CODE_HASH = ethUtil.keccak256()


You are introducing an ethUtil call here, eventually that can also be a cause?

holgerd77 · 2018-10-10T12:55:43Z

I've done a pure ethereumjs-util update PR #369 to see if there are side/other effects or if it is really the library update.

holgerd77 · 2018-10-10T12:56:39Z

Update: ah no, tests are also failing on the test PR, seems to be really the util update.

holgerd77 · 2018-10-10T13:24:59Z

Ok, here is my theory, quite crazy stuff:

Tests like the API tests (npm run testAPI) now fail with errors like:

not ok 21 it has default trie
  ---
    operator: equal
    expected: |-
      <Buffer 56 e8 1f 17 1b cc 55 a6 ff 83 45 e6 92 c0 f8 6e 5b 48 e0 1b 99 6c ad c0 01 62 2f b5 e3 63 b4 21>
    actual: |-
      <Buffer 56 e8 1f 17 1b cc 55 a6 ff 83 45 e6 92 c0 f8 6e 5b 48 e0 1b 99 6c ad c0 01 62 2f b5 e3 63 b4 21>
    at: Test.t.test (/home/circleci/project/ethereumjs-vm/tests/api/index.js:14:8)
  ...
ok 22 it has fake blockchain by default

On a first and second look, buffers look actually quite the same, so one is puzzled. On a third thought I experimented a bit with deepEqual and equal, because I never understood this in the first place and furthermore one of the line failing in tests/api/index.js is:

st.equal(vm.stateManager.trie.root, util.KECCAK256_RLP, 'it has default trie')

util.KECCAK256_RLP absolutely didn't change in the update, so this has to be something else. My theorie is that this is caused by not using the less strict deepEqual comparison. After the VM update to ethereumjs-util v6.0.0 the merkle tree library providing vm.stateManager.trie.root is still using the util library v5.2.0. I would assume that before the equal comparison accidentally worked, cause both references were pointing to the same object using the same npm ethereumjs-util dependency v5.2.0. Now both point to different objects and things fail.

This would mean that there is no mistake with the util library but that this can be fixed by replacing the equal comparisons in tests (probably on various places) with deepEqual comparisons.

I will test this by doing the ethereumjs-util v6.0.0 update as a separate PR and then merge if tests are passing. I would ask you to then rebase on top and along remove your version update commit within this PR.

holgerd77 · 2018-10-10T13:29:08Z

Ok, first update on #369, if theory is correct the API tests should pass now.

holgerd77 · 2018-10-10T13:43:05Z

API tests are passing, seems to me that I was on the correct track. However other tests are still failing. It just occurred to me that similar comparisons in the production code will likely now also fail.

The easier way to go would be eventually to update the merkle tree library to also use the v6.0.0 util version.

Not sure about the way to go, directly fixing this would probably be the more correct way.

holgerd77 · 2018-10-10T14:08:48Z

Ok, I found one left-over occurence of a sha3-named constant in StateManager, which have been removed in the v6.0.0 util version, this might very well be it, waiting for the tests to finish.

holgerd77 · 2018-10-23T13:17:14Z

The failures on collision are very likely not CREATE2 specific (e.g. create2collisionCode test) but these cases are also not working under CREATE, see the corresponding issue here: #232

Will also try to dig a bit into that.

holgerd77 · 2018-10-23T13:43:41Z

Rebased this to run with the latest tests.

holgerd77 · 2018-10-23T13:51:51Z

Results with latest Constantinople state tests (ethereumjs-testing v1.2.4):

1..4776
# tests 4776
# pass  4749
# fail  27

Update: just saw, same as above, there is also a more detailed summary listing the failing tests.

holgerd77 · 2018-10-31T23:08:00Z

Hi @rmeissner, tried to rebase this, this is currently conflicting with the latest StateManager refactoring changes from @mattdean-digicatapult introduced with this PR https://github.com/ethereumjs/ethereumjs-vm/pull/366/files#diff-466ee465f36ed5789ade4601e1b53f5b, so probably better if you have a look yourself and do a careful rebase once time permits.

rmeissner · 2018-10-31T23:38:33Z

@holgerd77 i will rebase it and fix it after devcon and try to figure out why all the create and create2 collision tests are failing

rmeissner · 2018-11-03T22:36:20Z

@holgerd77 I rebased and squashed the commits (to make future rebasing easier). I also fixed the linting tests. Will start looking into the create2/create tests

rmeissner · 2018-11-04T00:32:23Z

@holgerd77 I got it down to 4 failing tests. Tried to figure out what goes wrong but I think these are unrelated to create2.

E.g. RevertInCreateInInitCreate2

ethereumjs-vm

...
# {"pc":21,"op":96,"gas":"0x9fffea821","gasCost":"0x3","stack":["0x20"],"depth":0,"opName":"PUSH1"}
# {"pc":23,"op":85,"gas":"0x9fffea81e","gasCost":"0x0","stack":["0x20","0x0"],"depth":0,"opName":"SSTORE"}
# {"pc":24,"op":96,"gas":"0x9fffea756","gasCost":"0x3","stack":[],"depth":0,"opName":"PUSH1"}
...

vs evm

...
{"pc":21,"op":96,"gas":"0x9fffea821","gasCost":"0x3","memory":"0x","memSize":32,"stack":["0x20"],"depth":1,"refund":0,"opName":"PUSH1","error":""}
{"pc":23,"op":85,"gas":"0x9fffea81e","gasCost":"0x4e20","memory":"0x","memSize":32,"stack":["0x20","0x0"],"depth":1,"refund":0,"opName":"SSTORE","error":""}
{"pc":24,"op":96,"gas":"0x9fffe59fe","gasCost":"0x3","memory":"0x","memSize":32,"stack":[],"depth":1,"refund":0,"opName":"PUSH1","error":""}
...

The costs for SSTORE are different in both cases.

One side note the gasCost output of ethereumjs-vm is misleading since it doesn't include dynamic gas (e.g. hashing costs for create2 or the costs for sstore)

rmeissner · 2018-11-04T00:50:25Z

this might actually also fix #232 didn't check everything but the example provided in the latest comment now passes

holgerd77 · 2018-11-06T11:27:50Z

Hi @rmeissner, already a bit late to congratulate you on this, great achievement!!! 😄

I would very much have a tendency to merge this for now, since there is always this rebasing back and forth with the work from @mattdean-digicatapult with both of you working on larger structural changes. It should also get easier then to tackle the last failing tests, being them related to CREATE2 or not.

I would then rebase this and - if you are ok with it - approve and merge.

trap if not constantinople add check if nonce or code at address Fix lint errors Refactoring Fix lint Use invalid init code on colission Fix lint

holgerd77 · 2018-11-06T11:33:59Z

Rebased this.

holgerd77 · 2018-11-06T13:52:37Z

If you are not actively intervening in the next half-an-hour or so, I would merge this 😄, would like to forward to @mattdean-digicatapult, hope this is ok.

rmeissner · 2018-11-06T14:18:07Z

sounds good 👍

holgerd77

Looks good, thanks @rmeissner for the great work and staying on with this for so long!

holgerd77 · 2018-11-06T15:26:32Z

Assumption was right, collision tests are now also fixed, see #383.

😄

jwasinger mentioned this pull request Aug 7, 2018

Constantinople Implementation Progress #322

Closed

5 tasks

axic self-assigned this Aug 7, 2018

holgerd77 added the PR state: WIP label Sep 20, 2018

holgerd77 changed the title ~~add support for CREATE2~~ EIP 1014: CREATE2 Sep 24, 2018

holgerd77 changed the title ~~EIP 1014: CREATE2~~ EIP 1014 CREATE2 Sep 24, 2018

holgerd77 added this to In Progress in Constantinople Implementation Progress Sep 24, 2018

holgerd77 mentioned this pull request Sep 24, 2018

[Constantinople] EIP 1014 CREATE2 Opcode #356

Closed

rmeissner force-pushed the eip-1014 branch from d2edc07 to dd707f0 Compare October 2, 2018 13:58

rmeissner mentioned this pull request Oct 4, 2018

add generateAddress2 for CREATE2 ethereumjs/ethereumjs-util#146

Merged

rmeissner force-pushed the eip-1014 branch from 32783e5 to d6ef147 Compare October 10, 2018 11:46

holgerd77 mentioned this pull request Oct 10, 2018

Update of ethereumjs-util to v6.0.0 #369

Merged

holgerd77 reviewed Oct 10, 2018

View reviewed changes

holgerd77 force-pushed the eip-1014 branch from b549fdb to ea23f87 Compare October 23, 2018 13:43

rmeissner force-pushed the eip-1014 branch from ea23f87 to 21cf775 Compare November 3, 2018 22:35

rmeissner force-pushed the eip-1014 branch 2 times, most recently from 0a04199 to 6a2c9e2 Compare November 4, 2018 00:17

rmeissner force-pushed the eip-1014 branch from 6a2c9e2 to bdc84a0 Compare November 4, 2018 00:34

jwasinger and others added 3 commits November 6, 2018 12:31

add support for CREATE2

6752073

trap if not constantinople add check if nonce or code at address Fix lint errors Refactoring Fix lint Use invalid init code on colission Fix lint

Fix creation of invalid code

a8abcfa

Handle revert in create2 with message

6efe3d1

holgerd77 force-pushed the eip-1014 branch from bdc84a0 to 6efe3d1 Compare November 6, 2018 11:33

holgerd77 mentioned this pull request Nov 6, 2018

Misleading gasCost output for dynamic gas calculations #382

Closed

holgerd77 approved these changes Nov 6, 2018

View reviewed changes

holgerd77 merged commit 5fdce83 into master Nov 6, 2018

Constantinople Implementation Progress automation moved this from In Progress to Done Nov 6, 2018

holgerd77 deleted the eip-1014 branch November 6, 2018 14:58

holgerd77 mentioned this pull request Nov 6, 2018

Fix failing blockchain tests #338

Closed

holgerd77 mentioned this pull request Nov 28, 2018

Collision tests are failing (consensus bug) #232

Closed

evertonfraga added the package: vm label Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EIP 1014 CREATE2 #329

EIP 1014 CREATE2 #329

jwasinger commented Aug 4, 2018 •

edited by axic

Loading

holgerd77 commented Aug 30, 2018

holgerd77 commented Sep 20, 2018

jwasinger commented Sep 22, 2018

rmeissner commented Sep 27, 2018

holgerd77 commented Sep 28, 2018

holgerd77 commented Sep 29, 2018

rmeissner commented Sep 29, 2018 •

edited

Loading

holgerd77 commented Sep 29, 2018

rmeissner commented Oct 2, 2018

holgerd77 commented Oct 10, 2018

rmeissner commented Oct 10, 2018 •

edited

Loading

holgerd77 commented Oct 10, 2018

holgerd77 Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 23, 2018

holgerd77 commented Oct 23, 2018

holgerd77 commented Oct 23, 2018 •

edited

Loading

holgerd77 commented Oct 31, 2018

rmeissner commented Oct 31, 2018

rmeissner commented Nov 3, 2018 •

edited

Loading

rmeissner commented Nov 4, 2018

rmeissner commented Nov 4, 2018

holgerd77 commented Nov 6, 2018

holgerd77 commented Nov 6, 2018

holgerd77 commented Nov 6, 2018

rmeissner commented Nov 6, 2018

holgerd77 left a comment

holgerd77 commented Nov 6, 2018

EIP 1014 CREATE2 #329

EIP 1014 CREATE2 #329

Conversation

jwasinger commented Aug 4, 2018 • edited by axic Loading

holgerd77 commented Aug 30, 2018

holgerd77 commented Sep 20, 2018

jwasinger commented Sep 22, 2018

rmeissner commented Sep 27, 2018

holgerd77 commented Sep 28, 2018

holgerd77 commented Sep 29, 2018

rmeissner commented Sep 29, 2018 • edited Loading

holgerd77 commented Sep 29, 2018

rmeissner commented Oct 2, 2018

holgerd77 commented Oct 10, 2018

rmeissner commented Oct 10, 2018 • edited Loading

holgerd77 commented Oct 10, 2018

holgerd77 Oct 10, 2018

Choose a reason for hiding this comment

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 10, 2018

holgerd77 commented Oct 23, 2018

holgerd77 commented Oct 23, 2018

holgerd77 commented Oct 23, 2018 • edited Loading

holgerd77 commented Oct 31, 2018

rmeissner commented Oct 31, 2018

rmeissner commented Nov 3, 2018 • edited Loading

rmeissner commented Nov 4, 2018

rmeissner commented Nov 4, 2018

holgerd77 commented Nov 6, 2018

holgerd77 commented Nov 6, 2018

holgerd77 commented Nov 6, 2018

rmeissner commented Nov 6, 2018

holgerd77 left a comment

Choose a reason for hiding this comment

holgerd77 commented Nov 6, 2018

jwasinger commented Aug 4, 2018 •

edited by axic

Loading

rmeissner commented Sep 29, 2018 •

edited

Loading

rmeissner commented Oct 10, 2018 •

edited

Loading

holgerd77 commented Oct 23, 2018 •

edited

Loading

rmeissner commented Nov 3, 2018 •

edited

Loading