ethereum / ethereum/tests

General Improvements To Ethereum Reference Test Suite

Open
#474 6 comments 0 reactions 0 assignees View on GitHub
enhancement up-for-grabs
Dominant language
JavaScript
Stars
626
Forks
364
PR merge metrics
No merged PRs in 30d

Description

I am a core developer of Pantheon, the Java-based Ethereum client being developed at PegaSys/ConsenSys. My main area of focus has been on block processing/importing where I’ve spent a lot of time working with the various Ethereum reference tests.

There Ethereum reference tests have been invaluable getting our block processing capabilities online and I am grateful for all of the effort everyone has put into them— thank you! I could not imagine how hard it would be to implement block processing without having that level of test coverage — especially all of the edge cases that are not immediately clear just by reading the Yellow Paper.

While the `retesteth` project is underway, I think it would be a good time to revisit the different test suites and test specification formats. Having just entered the space ten months ago and implementing these functionalities these test from scratch, I think there are some good opportunities to make it easier for the next set of Ethereum clients to come online, which I am more than happy to help with/drive. Some of these improvements include:

1. *Making Test Specification Formats as Consistent as Possible Across Test Suites:* The conventions from different test suites sometimes deviate from each other. For example, the `GeneralStateTest`s typically contain a [single property](https://github.com/ethereum/tests/blob/develop/GeneralStateTests/stCodeSizeLimit/codesizeValid.json#L2) that encompasses the tests and the `post` field differentiates milestones while the `BlockchainTest`s have a [key per milestone](https://github.com/ethereum/tests/blob/develop/BlockchainTests/GeneralStateTests/stCodeSizeLimit/codesizeValid_d0g0v0.json#L2). There is also a slight difference in their naming as well (e.g. `_d0g0v0`). Also the `GeneralStateTests` can nest multiple test cases by the `indexes` mechanism, which was done to save storage space, but now means that each JSON file corresponds to more than one test per milestone.

2. *Fill in the `comment` field to provide insight into what the test case is doing:* Sometimes it’s not immediately clear what the test case is doing and testing for. For example, it’s not immediately clear that `tx_e1c174e2` is testing the transaction that caused a consensus bug due to a client implementation committing data for a message that exceptionally halted to the RipeMD precompile. Another option is to provide this information in a `README` but that probably will not scale well and be hard to remember to always enforce when reviewing PRs.

3. *Decrease the scope of what is tested in a single test suite and increase the number of test suites to allow client functionalities to be developed methodically:* An important aspect of the Ethereum reference test is that they make it easier for new clients to be developed. Many of the tests build on each other, such as the transition from `GeneralStateTest`s to `BlockchainTest`s. However we can make these test suites even more granular to create a “client implementor plan” that walks future client implementors through the steps of building a client. For example, having tests to thoroughly test RLP, creating and manipulating tries, deserialize/deserialize block constructs (e.g. blocks, transactions, etc), testing call/create messages, etc. Some of these tests exist in some aspect

4. *Keep the “Read the Docs” up to date:* Some documents, such as the [TransactionTests](https://ethereum-tests.readthedocs.io/en/latest/test_types/transaction_tests.html) are not up-to-date where the actual `transaction` fields are no longer there. This also goes back to point (3) as well because these fields would allow a client implementor to check more easily check/debug RLP serialization/deserialization for transactions.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.