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

General Improvements To Ethereum Reference Test Suite #474

Open
Matthalp-zz opened this issue Jul 1, 2018 · 6 comments
Open

General Improvements To Ethereum Reference Test Suite #474

Matthalp-zz opened this issue Jul 1, 2018 · 6 comments

Comments

@Matthalp-zz
Copy link

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 GeneralStateTests typically contain a single property that encompasses the tests and the post field differentiates milestones while the BlockchainTests have a key per milestone. 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 GeneralStateTests to BlockchainTests. 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 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.

@Matthalp-zz
Copy link
Author

If I was to summarize this, it would be to make the Ethereum reference tests more systematic in a way that lowers the barrier to implementing a client from scratch, which include more granular test suites as well as a road map for how implementers should go about bringing capabilities online and testing them. Additionally there are opportunities to add conventions/consistency amongst test suites specifications/formats and include information about what test suites and individual test cases are evaluating.

@winsvega
Copy link
Collaborator

winsvega commented Jul 3, 2018

Making Test Specification Formats as Consistent as Possible Across Test Suites: 

The idea on this one is to make a uniform test format which will make it easier to parse the tests.
Another idea is that client developers should not bother about different test formats. testing should be done through the test RPC interface.

Fill in the comment. + Keep the “Read the Docs” up to date: 

Need a free hands on that

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: 

A guide on ethereum client develop process would be nice. +1 free hands here.

@Matthalp-zz
Copy link
Author

@winsvega I'm on vacation the next few days, but will double back next week to reply about next steps, etc.

@winsvega
Copy link
Collaborator

winsvega commented May 7, 2021

@qbzzt thats basicaly what you did, right?
bullet 1 and 4 are all done

@qbzzt
Copy link
Collaborator

qbzzt commented May 7, 2021

@qbzzt thats basicaly what you did, right?
bullet 1 and 4 are all done

I did bullet 4. I think you did bullet 1 before I even got here.

Bullet 2 we usually do with comments, but as they are in # <whatever> lines, they aren't propagated into the filled tests. Is that a problem?

@mzabaluev
Copy link

Added #889 to expand on bullet 4.

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

4 participants