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

Deprecate StateTest format #487

Open
chfast opened this issue Mar 27, 2024 · 4 comments
Open

Deprecate StateTest format #487

chfast opened this issue Mar 27, 2024 · 4 comments

Comments

@chfast
Copy link
Contributor

chfast commented Mar 27, 2024

The StateTest format has many disadvantages for users and is very close to the single-transaction BlockchainTest.

What is blocking us from using only BlockchainTests?

Disadvantages

  1. Weird multi-transaction definition. It does not make sense for generating tests. Potentially, it can save some disk space, but in practice both ethereum/tests and execution-spec-tests generate the same tests in BlockchainTest and StateTest formats. This is also bad for users because they need special transaction representation only for StateTests.
  2. Hashed post state. Users are not able to tell what's wrong with a failing test because they only can see that the state hash mismatch. Moreover, computing the state hash is not trivial (e.g. evmone implemented MPT just for that).
@marioevz
Copy link
Member

marioevz commented Apr 3, 2024

Yes, definitely agree on 1, it's not good and it makes the definition of our types way more convoluted than it has to be, and this is just the result of us trying to adapt the existing fixture format without breaking anything.

The use I see of the state test format is that you can test for rejected transactions more easily than in the blockchain test format:

In the blockchain test format, if the cause of invalidation is an invalid transaction, you get a passing test if the client rejects the block, but if you want to make sure that it was rejected because the transaction was deemed as invalid (and not some other block validation error) you need to check the error string.

In the case of the state test you check that the state hash did not change and then you can be sure the transaction was the cause of the rejection.

IMO I think we need to standardize the block invalidation errors before phasing out the state test format.

@chfast
Copy link
Contributor Author

chfast commented Apr 3, 2024

The t8n has the "rejected" array where it puts the invalid transactions with error messages.

@chfast
Copy link
Contributor Author

chfast commented Apr 3, 2024

cc @holiman

@winsvega
Copy link
Collaborator

winsvega commented Apr 4, 2024

The multy transaction types are fillers only. When you parse the generated test you don't need to support it. Use txbytes field

The state hash could be provided with full state no problem.

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

No branches or pull requests

3 participants