fix(eof): eofwrap.py to ignore invalid blocks

[pdobacz]

🗒️ Description

There is no support for invalid FixtureTransactions to be read from JSON, so we cannot include invalid blocks easily. However as this is not central to the behavior eofwrap tests are intended to test, we can just skip these invalid blocks with the same end effect on the test.

Unfortunately, the fix isn't limited to that. It seems like the fix to requests serialization/hashing (#990) missed a detail when the requests_lists is used to construct requests (like from t8n output). Please double check carefully if this fix is fine.

Note also that for devnet-5 this would be partially hidden by the fact, that we don't include empty requests in the commitment anymore, meaning that all empty requests will not trigger the problem

EDIT: request handling remains unchanged, see thread below

🔗 Related Issues

NA

✅ Checklist

• All: Set appropriate labels for the changes.
• All: Considered squashing commits to improve commit history.
• All: Added an entry to CHANGELOG.md.
• All: Considered updating the online docs in the ./docs/ directory.
• Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
• Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
• Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
• Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

[pdobacz]

It seems the request hash problem affects filling state_tests with evmone too, presumably with other t8n's too?

[chfast]

This fixes filling of EOF tests for me.

[marioevz]

One comment that is a breaking issue, there must be some other approach to this.

[marioevz]

This still breaks our test generation because this is the code path we use to process the list returned by the t8n tool, and the list returned by t8n (correctly) already contains the type byte at the beginning of each request.

I think the interface could definitely be improved, and it could be that we need a different parameter for this use case in this PR, and a different one for the values returned by t8n.

It would also help me to better understand this use case with an example test that causes an issue when wrapped with an EOF container.

[pdobacz]

Okay, I'm back to this after holidays, sorry about the wait @marioevz .

evmone's t8n doesn't include the type in the result, and it does include empty requests. (mind that in the commitment it follows the spec, i.e. empty requests are excluded from it per the recent change). And from what I understand, the eofwrap use case also works on the t8n output.

Where is the t8n output format formally specified / agreed upon?

(I'll try to produce an example)

[pdobacz]

Meanwhile, I think I've found the answer to my own question - t8n follows the execution-apis as spec https://github.com/ethereum/execution-apis/pull/599/files ?

As such I guess I should fix on evmone's end, let me try that.

[pdobacz]

OK, that worked, I force-pushed those commits away from this PR and fixed in evmone. Thank you for the thorough review and patience, I really appreciate this.