-
Notifications
You must be signed in to change notification settings - Fork 112
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
fix(eof): eofwrap.py to ignore invalid blocks #1028
Conversation
It seems the request hash problem affects filling |
This fixes filling of EOF tests for me. |
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.
One comment that is a breaking issue, there must be some other approach to this.
0861df4
to
10e5ad6
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.
Thanks! We can revisit the requests issue in a follow up PR if that still is necessary, I think it should be solved at a different level to be honest, but we can discuss about it further.
yes, in case you've missed my comment, the solution was to have So no need to follow up PR, and thank you. |
🗒️ 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 therequests_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 problemEDIT: request handling remains unchanged, see thread below
🔗 Related Issues
NA
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.