EIP 1052 EXTCODEHASH #326

jwasinger · 2018-07-30T22:11:51Z

Closes #357.

lib/opFns.js

axic · 2018-07-30T22:26:15Z

lib/opFns.js

-    stateManager.getContractCode(address, function (err, code) {
+
+    if (runState._precompiled[address.toString('hex')]) {
+      cb(null, new BN(0))


If non-existent account has a result 0, while empty code has the empty keccak256, why is the hash of the precompile 0?

I would think that this is a way to signal that the contract shouldn't be calling EXTCODEHASH on a precompile address.

I can see how this would be confusing behavior. maybe the result of calling EXTCODEHASH on a precompile should be KECCAK256_NULL

I think it should be the empty code hash for precompiles, since the account does exists.

Okay. It seems Geth is the only client that has implemented this EIP so I will look over the behavior there.

lib/opFns.js

coveralls · 2018-08-03T18:03:59Z

Coverage decreased (-0.6%) to 90.405% when pulling 45f1ef8 on eip-1052 into fca78fc on master.

lib/opFns.js

holgerd77 · 2018-09-20T09:45:53Z

Rebased this to latest master.

holgerd77 · 2018-09-28T09:09:10Z

What is the status of this?

holgerd77 · 2018-10-12T09:10:16Z

Current state here: tests from ethereum/tests#484 are not yet merged, so test result stays the same.

5chdn · 2018-11-14T09:38:33Z

What's the status of this?

holgerd77 · 2018-11-14T09:41:35Z

@5chdn I will today do a new release on the ethereum/tests library including the first EXTCODEHASH consensus tests and then directly update our proxy testing library ethereumjs-testing to have tests for this.

We should then be able to complete here within a week or so and hopefully do a release at the end of next week of we are lucky (this also depends on 1-2 other outstanding changes).

Out of interest: what is your connection on (need for) this?

5chdn · 2018-11-14T09:44:39Z

Out of interest: what is your connection on (need for) this?

I just checked all clients in the tracker against Constantinople readiness and this is the last open PR. :)

No pressure :)

holgerd77 · 2018-11-14T12:05:04Z

Rebased this, this will now run some initial EXTCODEHASH tests from #387.

holgerd77 · 2018-11-14T13:46:52Z

Results in:

ok 770 the state roots should match
# file: extCodeHashCALL test: extCodeHashCALL
/home/circleci/project/ethereumjs-vm/dist/runCode.js:243
          throw e;
          ^

TypeError: stateManager.exists is not a function
    at EXTCODEHASH (/home/circleci/project/ethereumjs-vm/dist/opFns.js:304:18)
    at runOp (/home/circleci/project/ethereumjs-vm/dist/runCode.js:237:27)
    at /home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:3880:24
    at replenish (/home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:1011:17)
    at /home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:1016:9
    at eachOfLimit (/home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:1041:24)
    at /home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:1046:16
    at _parallel (/home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:3879:5)
    at Object.series (/home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:4735:5)
    at iterateVm (/home/circleci/project/ethereumjs-vm/dist/runCode.js:138:11)
    at next (/home/circleci/project/ethereumjs-vm/node_modules/async/dist/async.js:5223:28)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)

holgerd77 · 2018-11-15T13:09:39Z

Rebased this.

rmeissner · 2018-11-15T14:10:37Z

I would love to take a closer look on this if nobody else is working on it :)

Also on another note I found some weird behaviour with --jsontrace. I found a test where it fails if I use --jsontrace, but passes if I don't use it.

node ./tests/tester -s --fork='Constantinople' --test='extCodeHashDynamicArgument' --data=0 --gas=0 --value=0

TAP version 13
# GeneralStateTests
# file: extCodeHashDynamicArgument test: extCodeHashDynamicArgument
ok 1 the state roots should match

1..1
# tests 1
# pass  1

# ok

vs

node ./tests/tester -s --fork='Constantinople' --test='extCodeHashDynamicArgument' --data=0 --gas=0 --value=0 --jsontrace

TAP version 13
# GeneralStateTests
# file: extCodeHashDynamicArgument test: extCodeHashDynamicArgument
# {"pc":0,"op":96,"gas":"0x5c7b8","gasCost":"0x3","stack":[],"depth":0,"opName":"PUSH1"}
# {"pc":2,"op":53,"gas":"0x5c7b5","gasCost":"0x3","stack":["0x0"],"depth":0,"opName":"CALLDATALOAD"}
# {"pc":3,"op":63,"gas":"0x5c7b2","gasCost":"0x190","stack":["0x1"],"depth":0,"opName":"EXTCODEHASH"}
# {"pc":4,"op":96,"gas":"0x5c622","gasCost":"0x3","stack":["0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":6,"op":85,"gas":"0x5c61f","gasCost":"0x0","stack":["0x0","0x0"],"depth":0,"opName":"SSTORE"}
# {"pc":7,"op":96,"gas":"0x5b297","gasCost":"0x3","stack":[],"depth":0,"opName":"PUSH1"}
# {"pc":9,"op":53,"gas":"0x5b294","gasCost":"0x3","stack":["0x0"],"depth":0,"opName":"CALLDATALOAD"}
# {"pc":10,"op":59,"gas":"0x5b291","gasCost":"0x2bc","stack":["0x1"],"depth":0,"opName":"EXTCODESIZE"}
# {"pc":11,"op":96,"gas":"0x5afd5","gasCost":"0x3","stack":["0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":13,"op":85,"gas":"0x5afd2","gasCost":"0x0","stack":["0x0","0x1"],"depth":0,"opName":"SSTORE"}
not ok 1 the state roots should match
  ---
    operator: equal
    expected: |-
      '2ceab025ea1bcf156d3b82dcd37082bbd7550c50933057550a55a7dce4c06df0'
    actual: |-
      'dfbd02fee291d961905b27a5b42838a7c946fb89514534217e6bb3344d2dad71'
    at: replenish (/Users/rimeissner/gnosis/ethereumjs-vm/node_modules/async/dist/async.js:1011:17)
  ...

1..1
# tests 1
# pass  0
# fail  1

Is this known or should I open an issue?

holgerd77 · 2018-11-15T14:19:44Z

@rmeissner Feel free, that would be great! 😄

jsontrace: This is not known and you should open an issue.

holgerd77 · 2018-11-15T14:20:20Z

Side-note: I am currently targeting a VM release mid or end of next week.

holgerd77 · 2018-11-15T15:08:10Z

lib/opFns.js

+      if (err) return cb(err)
+      if (!exists) {
+        return cb(null, new BN(0))
+      }


Hi @mattdean-digicatapult, short note: what occurred to mean when having a short look on how to update here regarding StateManager: we should make it clear in the docs especially for the the get* methods (so getAccount, getContractStorage,...) what the behavior is when called on a non-existing account (will it throw or not, what is returned).

Think it would also not be overblown to have additional test cases for that. If you don't want to do yourself you can also alternatively open an issue on that.

No problem, I've started working a bit on the documentation so I'll make that clear. If I find too many inconsistencies for the missing values I might make some changes as well.

Thanks. 😀

rmeissner · 2018-11-15T23:30:09Z

I removed the check for precompiles, because this is what I saw when I looked at go-ethereum, also this seems to be satisfying the tests :D

Also right now I use the Account.isEmpty() to check if an account exists. The tests pass, but I am not sure if that is the most correct way.

axic · 2018-11-16T10:41:21Z

Can some of these commits be squashed?

holgerd77 · 2018-11-16T10:41:48Z

Whew, just 1 (!) failing test on the Constantinople test run, how amazing is that?? 🥂 😄

holgerd77 · 2018-11-16T10:42:43Z

@axic I think actually all. @rmeissner can you do that?

lib/opFns.js

add EXTCODEHASH rules for nonexistent accounts and precompiles . Replace removed state manager method Fix lint

holgerd77

Looks good now, thanks!

jwasinger force-pushed the eip-1052 branch from 9cb48e0 to 23d21b4 Compare July 30, 2018 22:12

This was referenced Jul 30, 2018

Add EXTCODEHASH opcode implementation #324

Closed

Constantinople Implementation Progress #322

Closed

axic reviewed Jul 30, 2018

View reviewed changes

lib/opFns.js Outdated Show resolved Hide resolved

axic reviewed Jul 30, 2018

View reviewed changes

jwasinger force-pushed the eip-1052 branch from 9664d77 to b92530a Compare July 30, 2018 22:50

axic reviewed Jul 31, 2018

View reviewed changes

lib/opFns.js Outdated Show resolved Hide resolved

axic reviewed Jul 31, 2018

View reviewed changes

lib/opFns.js Show resolved Hide resolved

Souptacular mentioned this pull request Aug 4, 2018

Constantinople Progress Tracker ethereum/pm#53

Closed

axic reviewed Aug 6, 2018

View reviewed changes

lib/opFns.js Outdated Show resolved Hide resolved

axic self-assigned this Aug 7, 2018

holgerd77 force-pushed the eip-1052 branch from 74eeb67 to 8a97f31 Compare September 20, 2018 09:45

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

holgerd77 mentioned this pull request Sep 24, 2018

[Constantinople] EIP 1052 EXTCODEHASH Opcode #357

Closed

holgerd77 force-pushed the eip-1052 branch from 8a97f31 to bf96587 Compare October 12, 2018 08:40

holgerd77 mentioned this pull request Nov 6, 2018

[Constantinople] EIP 1014 CREATE2 Opcode #356

Closed

5chdn mentioned this pull request Nov 14, 2018

Ethereum Core Devs Meeting 50 Agenda ethereum/pm#62

Closed

holgerd77 force-pushed the eip-1052 branch from bf96587 to b1482a5 Compare November 14, 2018 12:04

holgerd77 force-pushed the eip-1052 branch from b1482a5 to d661bde Compare November 15, 2018 13:09

holgerd77 reviewed Nov 15, 2018

View reviewed changes

axic removed their assignment Nov 16, 2018

rmeissner force-pushed the eip-1052 branch from 4590927 to 5f996c6 Compare November 16, 2018 10:47

axic reviewed Nov 16, 2018

View reviewed changes

lib/opFns.js Outdated Show resolved Hide resolved

add EXTCODEHASH opcode implementation

45f1ef8

add EXTCODEHASH rules for nonexistent accounts and precompiles . Replace removed state manager method Fix lint

rmeissner force-pushed the eip-1052 branch from 5f996c6 to 45f1ef8 Compare November 16, 2018 10:52

mattdean-digicatapult mentioned this pull request Nov 16, 2018

Reset selfdestruct on revert #392

Merged

holgerd77 approved these changes Nov 16, 2018

View reviewed changes

holgerd77 merged commit 839417d into master Nov 16, 2018

holgerd77 deleted the eip-1052 branch November 16, 2018 16:17

evertonfraga added the package: vm label Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EIP 1052 EXTCODEHASH #326

EIP 1052 EXTCODEHASH #326

jwasinger commented Jul 30, 2018 •

edited by axic

Loading

axic Jul 30, 2018

jwasinger Jul 30, 2018

jwasinger Jul 30, 2018 •

edited

Loading

axic Jul 30, 2018

jwasinger Jul 31, 2018

coveralls commented Aug 3, 2018 •

edited

Loading

holgerd77 commented Sep 20, 2018

holgerd77 commented Sep 28, 2018

holgerd77 commented Oct 12, 2018

5chdn commented Nov 14, 2018

holgerd77 commented Nov 14, 2018

5chdn commented Nov 14, 2018 •

edited

Loading

holgerd77 commented Nov 14, 2018

holgerd77 commented Nov 14, 2018

holgerd77 commented Nov 15, 2018

rmeissner commented Nov 15, 2018

holgerd77 commented Nov 15, 2018

holgerd77 commented Nov 15, 2018

holgerd77 Nov 15, 2018

mattdean-digicatapult Nov 15, 2018

holgerd77 Nov 15, 2018

rmeissner commented Nov 15, 2018 •

edited

Loading

axic commented Nov 16, 2018

holgerd77 commented Nov 16, 2018

holgerd77 commented Nov 16, 2018

holgerd77 left a comment

EIP 1052 EXTCODEHASH #326

EIP 1052 EXTCODEHASH #326

Conversation

jwasinger commented Jul 30, 2018 • edited by axic Loading

axic Jul 30, 2018

Choose a reason for hiding this comment

jwasinger Jul 30, 2018

Choose a reason for hiding this comment

jwasinger Jul 30, 2018 • edited Loading

Choose a reason for hiding this comment

axic Jul 30, 2018

Choose a reason for hiding this comment

jwasinger Jul 31, 2018

Choose a reason for hiding this comment

coveralls commented Aug 3, 2018 • edited Loading

holgerd77 commented Sep 20, 2018

holgerd77 commented Sep 28, 2018

holgerd77 commented Oct 12, 2018

5chdn commented Nov 14, 2018

holgerd77 commented Nov 14, 2018

5chdn commented Nov 14, 2018 • edited Loading

holgerd77 commented Nov 14, 2018

holgerd77 commented Nov 14, 2018

holgerd77 commented Nov 15, 2018

rmeissner commented Nov 15, 2018

holgerd77 commented Nov 15, 2018

holgerd77 commented Nov 15, 2018

holgerd77 Nov 15, 2018

Choose a reason for hiding this comment

mattdean-digicatapult Nov 15, 2018

Choose a reason for hiding this comment

holgerd77 Nov 15, 2018

Choose a reason for hiding this comment

rmeissner commented Nov 15, 2018 • edited Loading

axic commented Nov 16, 2018

holgerd77 commented Nov 16, 2018

holgerd77 commented Nov 16, 2018

holgerd77 left a comment

Choose a reason for hiding this comment

jwasinger commented Jul 30, 2018 •

edited by axic

Loading

jwasinger Jul 30, 2018 •

edited

Loading

coveralls commented Aug 3, 2018 •

edited

Loading

5chdn commented Nov 14, 2018 •

edited

Loading

rmeissner commented Nov 15, 2018 •

edited

Loading