-
Notifications
You must be signed in to change notification settings - Fork 111
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
feat(fw): add missing retesteth exceptions #572
Conversation
Hey @winsvega, do you have a corresponding branch in ethereum/tests that uses these exceptions? I think the end goal for this PR should be able to load every fixture from https://github.com/ethereum/tests/tree/2e37a9f41167534b07e0e8f247ea934a5fe3cac9/BlockchainTests into an execution-spec-tests blockchain pydantic model ( This can be tested by generating a test index file for ethereum/tests, i.e.,
Once we can load all these fixtures and generate an index file, we can then execute ethereum/tests fixtures against clients via |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. As mentioned in the comment above I think we should test this against ethereum/tests before merge.
This would mean preparing a branch of ethereum/tests with the new EEST exceptions changed, for example:
diff --git a/BlockchainTests/InvalidBlocks/bcInvalidHeaderTest/DifficultyIsZero.json b/BlockchainTests/InvalidBlocks/bcInvalidHeaderTest/DifficultyIsZero.json
index b90de9907d..f0f208e8c9 100644
--- a/BlockchainTests/InvalidBlocks/bcInvalidHeaderTest/DifficultyIsZero.json
+++ b/BlockchainTests/InvalidBlocks/bcInvalidHeaderTest/DifficultyIsZero.json
@@ -14,7 +14,7 @@
{
"blocknumber" : "1",
"chainname" : "default",
- "expectException" : "InvalidDifficulty",
+ "expectException" : "BlockException.INVALID_DIFFICULTY",
"rlp" : "0xf90261f901f6a0aa6c733b954fb03814267ff4e698d18b5dd210aa85e7457d856718c43fa4d5f9a01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347948888f1f195afa192cfee860698584c030f4c9db1a0edf42f78d05a438f6408aef357002a7380f4aca2587645d674498efc64705bd4a0e7e93016861d480e9fd60fc3dfd935eff097aa94c7380ed8df2ca7ec5b578214a021f4ebc5b0fb1ad80a00f78d04e67d1b30af68cecb3a8a2bf55247f11df1e3edb90100000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000400000000000000000000000000000000000000000000000000000008001832fefd882560b8454c9906942a00000000000000000000000000000000000000000000000000000000000000000880000000000000000f865f863808203e882c35094095e7baea6a6c7c4c2dfeb977efac326af552d87821388801ca0742e5cbd6ff6fedbc62821d019c5809bf620cba582f2dd33c23d3ca8956bd117a039c7a217fe45c3a6aa2e2138311af6eef76b571dc7523dd1745112bcc44851abc0",
"rlp_decoded" : {
"blockHeader" : {
@@ -251,7 +251,7 @@
{
"blocknumber" : "1",
"chainname" : "default",
- "expectException" : "InvalidDifficulty",
+ "expectException" : "BlockException.INVALID_DIFFICULTY",
"rlp" : "0xf90261f901f6a0aa6c733b954fb03814267ff4e698d18b5dd210aa85e7457d856718c43fa4d5f9a01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347948888f1f195afa192cfee860698584c030f4c9db1a0edf42f78d05a438f6408aef357002a7380f4aca2587645d674498efc64705bd4a0e7e93016861d480e9fd60fc3dfd935eff097aa94c7380ed8df2ca7ec5b578214a021f4ebc5b0fb1ad80a00f78d04e67d1b30af68cecb3a8a2bf55247f11df1e3edb90100000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000400000000000000000000000000000000000000000000000000000008001832fefd882560b8454c9906942a00000000000000000000000000000000000000000000000000000000000000000880000000000000000f865f863808203e882c35094095e7baea6a6c7c4c2dfeb977efac326af552d87821388801ca0742e5cbd6ff6fedbc62821d019c5809bf620cba582f2dd33c23d3ca8956bd117a039c7a217fe45c3a6aa2e2138311af6eef76b571dc7523dd1745112bcc44851abc0",
"rlp_decoded" : {
"blockHeader" : {
I also wondered if you wanted to preserve the exception <-> string mappings that retesteth has? Similar to what we added for EOF exceptions in #519 (src/ethereum_test_tools/exceptions/evmone_exceptions.py)?
eb995b0
to
813cce3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Ideally we should put some warning somewhere to remind us that we don't have any test that triggers some of these exceptions.
We have with consume rlp existing tests .jsons |
Thanks a lot for this! |
ποΈ Description
Convert retesteth exceptions into pyspec format
Adjust the .json fillers accrodingly for consume command
π Related Issues
β Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.