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

Enable tests that are _still_ disabled #2

Conversation

veox
Copy link

@veox veox commented Dec 7, 2018

What was wrong?

CREATE2 was not acting CREATE-like.

Also, some Constantinople tests are still disabled.

How was it fixed?

See explanation in ethereum#1181 (comment) or commit 00377ed.

Essential commit also submitted to ethereum/py-evm separately, as ethereum#1560.

Also, the other commits relevant to ethereum#1181 are included.

Cute Animal Picture

put a cute animal picture link inside the parentheses

Source: Noah's Ark companion animal hospital in Franklin, NC

@veox veox changed the title kkInvestigate create2 failures Fix CREATE2 failures in badOpcode fixture Dec 7, 2018
@veox
Copy link
Author

veox commented Dec 7, 2018

Tests:

https://circleci.com/gh/veox/py-evm/tree/investigate-create2-failures

(Does this link work for anyone but me?..)

@lithp
Copy link

lithp commented Dec 7, 2018

The link works for me!

@veox
Copy link
Author

veox commented Dec 7, 2018

I see at least one CI test job fails there - Create2Recursive fails with very long output. :/


There are many more failing cases, not just CREATE2-related.

@veox veox changed the title Fix CREATE2 failures in badOpcode fixture Fix CREATE2 failures in badOpcode fixture + enable tests that are _still_ disabled Dec 7, 2018
@veox
Copy link
Author

veox commented Dec 7, 2018

Applying this PR increases the failure count from 10 (as in ethereum#1181 ATM) to 179 (in json-fixtures only), thanks to the "missing" tests enabled in commit ed3e92e.

2 COMMITS SQUASHED:

fixtures: include missing Constantinople in helpers.

Debugging CREATE2 using the full "Blockchain" tests in `fixtures`
(they are disabled in `py-evm`, because they're slow and a duplication
of "State" tests).

A few definitions are missing - so add them.

tests/conftest: fix VM-tracing log-to-file helper.

Also move `import`s out of the helper, so they're not encountered
by the interpreter on every invocation of the helper.
@veox veox force-pushed the investigate-create2-failures branch from 00377ed to d30382f Compare December 11, 2018 23:54
@veox
Copy link
Author

veox commented Dec 11, 2018

Rebased on underlying PR - which has itself been rebased on upstream master, - and added more tests that will likely never work to the "incorrect upstream" list. (Unless py-evm starts contributing to ethereum/tests...)

One is existing `RevertInCreateInInit`, but now for Constantinople,
not just Byzantium.

The other is `RevertInCreateInInitCreate2`, which contains the same
"synthhetic" state which can't be arrived at by regular means in the
EVM. It's likely a copy-paste atavism.
@veox veox force-pushed the investigate-create2-failures branch from d30382f to d2d119d Compare December 12, 2018 00:07
@veox veox changed the title Fix CREATE2 failures in badOpcode fixture + enable tests that are _still_ disabled Enable tests that are _still_ disabled Dec 12, 2018
@cburgdorf cburgdorf merged commit 84b2ca7 into cburgdorf:christoph/tests/constantinople Dec 12, 2018
@veox veox deleted the investigate-create2-failures branch October 4, 2019 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants