-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Async sign and send raw middleware #3025
Async sign and send raw middleware #3025
Conversation
5a98a83
to
83b3a8a
Compare
- Copy the tests for sync over to async
- Don't assume some conversion is going to happen later down the line. The JSON-RPC asks for hexstr value, give it the hexstr value if we can guarantee that it is a hexstr. We already have the type as HexBytes so call ``.hex()`` on the value and send that as the param. - closes ethereum#2936
83b3a8a
to
de2f61a
Compare
"value": 1, | ||
"nonce": 0, | ||
} | ||
if isinstance(expected, type) and issubclass(expected, Exception): |
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.
Exceptions could be covered in a separate parameterized test, but really your preference here.
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.
Agreed, this is not ideal for new tests. I wanted to just copy the older sync cases so they can share the same test params but revamping both would be a good idea at some point.
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.
This LGTM!
What was wrong?
closes #2773
closes #2936
closes #2937
How was it fixed?
Todo:
Cute Animal Picture