Skip to content
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

Collision tests are failing (consensus bug) #232

Closed
holgerd77 opened this issue Nov 10, 2017 · 10 comments
Closed

Collision tests are failing (consensus bug) #232

holgerd77 opened this issue Nov 10, 2017 · 10 comments

Comments

@holgerd77
Copy link
Member

holgerd77 commented Nov 10, 2017

Description

The testing team recently added collision tests, testing the (unlikely but possible case) that a CREATE call is trying to create a contract on an existing address (hence: (address) collision).

These tests are still failing in the vm, failing tests cases:

  • CreateCollisionToEmpty (testfiller)
  • TransactionCollisionToEmptyButCode
  • TransactionCollisionToEmptyButNonce

Work Status

What I've got so far:

There has to be some additional check for account existence in runCall.js in the loadToAccount function in the form of:

stateManager.exists(toAddress, function (err, exists) {
  if (exists) {
    // This is new, stop/revert the execution
  } else {         
    stateManager.getAccount(createdAddress, function (err, account) {
      // Go on with the normal stuff...
    })
  }
})

I'm just not sure, what the semantics are here. Should the CREATE call just return and do nothing? Or throwing an OOG error? Some (detailed) hint on that would be helpful.

Reproduce

The following command produces a stack trace for a failing CreateCollisionToEmpty test:

node tests/tester -s --customStateTest='./CreateCollisionToEmpty.json' --data=2 --gas=0 --value=0 --jsontrace

Here is a gist with the succeeding stack trace from pyethereum.

@pirapira
Copy link

pirapira commented Nov 10, 2017

CREATE should overwrite the existing code.

That overwriting behavior will change when https://github.com/ethereum/EIPs/pull/208/files#diff-95b306e6e6d2d6dbb0c5fd3ac4ea45c4R27 is activated, but this EIP was postponed to Constantinople or later.

@holgerd77
Copy link
Member Author

@pirapira Ah, thanks so much!

@holgerd77
Copy link
Member Author

@pirapira ah, maybe as an addition: what are the differences then? Are there any differences in gas costs compared to a normal CREATE execution (I'm getting different gas costs then in the pyethereum stack)? Other?

@pirapira
Copy link

There should be no differences in gas consumption (overwriting = newly deploying).

I'm just trying up to conjure up the relevant YP page in my head though.

@pirapira
Copy link

And the existing nonce (as well as the code) should be just forgotten.

@axic
Copy link
Member

axic commented Jul 21, 2018

@holgerd77 I think these tests are passing now?

@holgerd77
Copy link
Member Author

@axic Hmm, these tests are currently commented out in the test runner, what makes you assume that they are passing now?

I once had a somewhat deeper look into this, I assumed that this would need some significant changes in the contract creation code in runCall.js, can't really imagine how this could have been fixed.

@axic
Copy link
Member

axic commented Jul 23, 2018

I didn't know they were commented out, but since we pass the "almost latest" tests repo I assumed they would be working.

@holgerd77
Copy link
Member Author

holgerd77 commented Oct 23, 2018

For an updated test setup: tested the single test case from above by creating a custom CreateCollisionToEmpty.json file just leaving the selected test value combination in the post section (data: 2 is the version "contract with code", taken from test comment field):

  { "Byzantium" : [
                {
                    "hash" : "0x5bfb8ec11eebda3c851a62b9923bac77e93819fa4f09898300c61ed4a7871dee",
                    "indexes" : {
                        "data" : 2,
                        "gas" : 0,
                        "value" : 0
                    },
                    "logs" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347"
                }
            ]
}

This can be run on ethereumjs-vm with:

node tests/tester -s --customStateTest='./CreateCollisionToEmpty.json' --data=2 --gas=0 --value=0 --jsontrace

producing the following stack trace:

TAP version 13
# GeneralStateTests
# file: ./CreateCollisionToEmpty.json test: CreateCollisionToEmpty
# {"pc":0,"op":96,"gas":"0x8d4f8","gasCost":"0x3","stack":[],"depth":0,"opName":"PUSH1"}
# {"pc":2,"op":96,"gas":"0x8d4f5","gasCost":"0x3","stack":["0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":4,"op":96,"gas":"0x8d4f2","gasCost":"0x3","stack":["0x0","0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":6,"op":96,"gas":"0x8d4ef","gasCost":"0x3","stack":["0x0","0x0","0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":8,"op":96,"gas":"0x8d4ec","gasCost":"0x3","stack":["0x0","0x0","0x0","0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":10,"op":96,"gas":"0x8d4e9","gasCost":"0x3","stack":["0x0","0x0","0x0","0x0","0x0"],"depth":0,"opName":"PUSH1"}
# {"pc":12,"op":53,"gas":"0x8d4e6","gasCost":"0x3","stack":["0x0","0x0","0x0","0x0","0x0","0x0"],"depth":0,"opName":"CALLDATALOAD"}
# {"pc":13,"op":98,"gas":"0x8d4e3","gasCost":"0x3","stack":["0x0","0x0","0x0","0x0","0x0","0x3000000000000000000000000000000000000000"],"depth":0,"opName":"PUSH3"}
# {"pc":17,"op":241,"gas":"0x8d4e0","gasCost":"0x2bc","stack":["0x0","0x0","0x0","0x0","0x0","0x3000000000000000000000000000000000000000","0x13880"],"depth":0,"opName":"CALL"}
# {"pc":0,"op":100,"gas":"0x13880","gasCost":"0x3","stack":[],"depth":1,"opName":"PUSH5"}
# {"pc":6,"op":96,"gas":"0x1387d","gasCost":"0x3","stack":["0x6001600155"],"depth":1,"opName":"PUSH1"}
# {"pc":8,"op":82,"gas":"0x1387a","gasCost":"0x3","stack":["0x6001600155","0x0"],"depth":1,"opName":"MSTORE"}
# {"pc":9,"op":96,"gas":"0x13874","gasCost":"0x3","stack":[],"depth":1,"opName":"PUSH1"}
# {"pc":11,"op":96,"gas":"0x13871","gasCost":"0x3","stack":["0x5"],"depth":1,"opName":"PUSH1"}
# {"pc":13,"op":96,"gas":"0x1386e","gasCost":"0x3","stack":["0x5","0x1b"],"depth":1,"opName":"PUSH1"}
# {"pc":15,"op":240,"gas":"0x1386b","gasCost":"0x7d00","stack":["0x5","0x1b","0x0"],"depth":1,"opName":"CREATE"}
# {"pc":0,"op":96,"gas":"0xb87e","gasCost":"0x3","stack":[],"depth":2,"opName":"PUSH1"}
# {"pc":2,"op":96,"gas":"0xb87b","gasCost":"0x3","stack":["0x1"],"depth":2,"opName":"PUSH1"}
# {"pc":4,"op":85,"gas":"0xb878","gasCost":"0x0","stack":["0x1","0x1"],"depth":2,"opName":"SSTORE"}
# {"pc":16,"op":96,"gas":"0x6d45","gasCost":"0x3","stack":["0x4b86c4ed99b87f0f396bc0c76885453c343916ed"],"depth":1,"opName":"PUSH1"}
# {"pc":18,"op":85,"gas":"0x6d42","gasCost":"0x0","stack":["0x4b86c4ed99b87f0f396bc0c76885453c343916ed","0x1"],"depth":1,"opName":"SSTORE"}
# {"stateRoot":"9120d5b9b8bb94243065d84501db1ecde7e48bb410ecd53bc45f0986b59fbb77"}
not ok 1 the state roots should match
  ---
    operator: equal
    expected: |-
      '5bfb8ec11eebda3c851a62b9923bac77e93819fa4f09898300c61ed4a7871dee'
    actual: |-
      '9120d5b9b8bb94243065d84501db1ecde7e48bb410ecd53bc45f0986b59fbb77'
    at: replenish (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/node_modules/async/dist/async.js:998:17)
  ...

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

Result from geth by running:

evm --json --nomemory statetest CreateCollisionToEmpty.json
{"pc":0,"op":96,"gas":"0x8d4f8","gasCost":"0x3","memory":"0x","memSize":0,"stack":[],"depth":1,"opName":"PUSH1","error":""}
{"pc":2,"op":96,"gas":"0x8d4f5","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0"],"depth":1,"opName":"PUSH1","error":""}
{"pc":4,"op":96,"gas":"0x8d4f2","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0"],"depth":1,"opName":"PUSH1","error":""}
{"pc":6,"op":96,"gas":"0x8d4ef","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0"],"depth":1,"opName":"PUSH1","error":""}
{"pc":8,"op":96,"gas":"0x8d4ec","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0"],"depth":1,"opName":"PUSH1","error":""}
{"pc":10,"op":96,"gas":"0x8d4e9","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0"],"depth":1,"opName":"PUSH1","error":""}
{"pc":12,"op":53,"gas":"0x8d4e6","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x0"],"depth":1,"opName":"CALLDATALOAD","error":""}
{"pc":13,"op":98,"gas":"0x8d4e3","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x3000000000000000000000000000000000000000"],"depth":1,"opName":"PUSH3","error":""}
{"pc":17,"op":241,"gas":"0x8d4e0","gasCost":"0x13b3c","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x3000000000000000000000000000000000000000","0x13880"],"depth":1,"opName":"CALL","error":""}
{"pc":0,"op":100,"gas":"0x13880","gasCost":"0x3","memory":"0x","memSize":0,"stack":[],"depth":2,"opName":"PUSH5","error":""}
{"pc":6,"op":96,"gas":"0x1387d","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x6001600155"],"depth":2,"opName":"PUSH1","error":""}
{"pc":8,"op":82,"gas":"0x1387a","gasCost":"0x6","memory":"0x","memSize":32,"stack":["0x6001600155","0x0"],"depth":2,"opName":"MSTORE","error":""}
{"pc":9,"op":96,"gas":"0x13874","gasCost":"0x3","memory":"0x","memSize":32,"stack":[],"depth":2,"opName":"PUSH1","error":""}
{"pc":11,"op":96,"gas":"0x13871","gasCost":"0x3","memory":"0x","memSize":32,"stack":["0x5"],"depth":2,"opName":"PUSH1","error":""}
{"pc":13,"op":96,"gas":"0x1386e","gasCost":"0x3","memory":"0x","memSize":32,"stack":["0x5","0x1b"],"depth":2,"opName":"PUSH1","error":""}
{"pc":15,"op":240,"gas":"0x1386b","gasCost":"0x7d00","memory":"0x","memSize":32,"stack":["0x5","0x1b","0x0"],"depth":2,"opName":"CREATE","error":""}
{"pc":16,"op":96,"gas":"0x2ed","gasCost":"0x3","memory":"0x","memSize":32,"stack":["0x0"],"depth":2,"opName":"PUSH1","error":""}
{"pc":18,"op":85,"gas":"0x2ea","gasCost":"0x1388","memory":"0x","memSize":32,"stack":["0x0","0x1"],"depth":2,"opName":"SSTORE","error":"out of gas"}
{"pc":18,"op":0,"gas":"0x799a4","gasCost":"0x0","memory":"0x","memSize":0,"stack":["0x0"],"depth":1,"opName":"STOP","error":""}
{"output":"","gasUsed":"0x13b54","time":1625727}
{"stateRoot": "5bfb8ec11eebda3c851a62b9923bac77e93819fa4f09898300c61ed4a7871dee"}
[
  {
    "name": "CreateCollisionToEmpty",
    "pass": true,
    "fork": "Byzantium"
  }
]

So gas values after the CREATE call significantly differ.

@holgerd77
Copy link
Member Author

Fixed (on the sideline 😄) by #329, will close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants