Skip to content

py-evm-issue-1466: Added test cases for EIP170 #2002

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

junning-tong
Copy link
Contributor

What was wrong?

Check issue-1466 for reference

Double checking this is the intended functionality.

EIP170 states that the contract size limit was changed to 2**14 + 2**13 which is 24,576 bytes. The following line implementations the constant for EIP170, but it is off by one.

EIP170_CODE_SIZE_LIMIT = 24577

How was it fixed?

  • Change EIP170_CODE_SIZE_LIMIT from 24577 to 24576
  • Add test cases

Cute Animal Picture

🐖🐖🐖🐖🐖🐖🐖🐖

@junning-tong junning-tong force-pushed the py-evm-issue-1466 branch 3 times, most recently from ad5f40e to 94c781b Compare April 18, 2021 22:34
@pipermerriam
Copy link
Member

Before we merge this, I'd like to understand why this wasn't caught using consensus tests because this seems like the type of thing that 1) I would be surprised if it wasn't in the tests and 2) if it isn't in the tests it should be added upstream

@junning-tong
Copy link
Contributor Author

junning-tong commented Apr 19, 2021

@pipermerriam the newly added test file is in directory tests/core/vm/.
Should I move it to tests/core/consensus/?

@pipermerriam
Copy link
Member

No, I'm referring to these tests: https://github.com/ethereum/tests/

They exist as a submodule in the repository.

@junning-tong
Copy link
Contributor Author

Hi @pipermerriam I see a commit which added test cases in fixtures submodules. https://github.com/ethereum/py-evm/pull/1998/files

Should I move newly added .json to the submodule as well?
tests/core/vm/fixtures/spurious_dragon_computation_test_code_size_limitation.json

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants