Skip to content

Conversation

@fselmo
Copy link
Collaborator

@fselmo fselmo commented Jul 6, 2023

What was wrong?

closes #2773
closes #2936
closes #2937

How was it fixed?

  • Add async support for sign-and-send raw transaction middleware
  • Use the hex value directly, instead of relying on formatters, since the JSON-RPC specs expect the hex
  • Add tests

Todo:

Cute Animal Picture

20230705_082223

@fselmo fselmo force-pushed the async-sign-and-send-raw-middleware branch from 5a98a83 to 83b3a8a Compare July 6, 2023 15:33
fselmo added 3 commits July 6, 2023 09:34
- 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
@fselmo fselmo force-pushed the async-sign-and-send-raw-middleware branch from 83b3a8a to de2f61a Compare July 6, 2023 15:35
@fselmo fselmo marked this pull request as ready for review July 6, 2023 15:35
@fselmo fselmo requested review from kclowes, pacrob and reedsa July 6, 2023 15:42
"value": 1,
"nonce": 0,
}
if isinstance(expected, type) and issubclass(expected, Exception):
Copy link
Contributor

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.

Copy link
Collaborator Author

@fselmo fselmo Jul 6, 2023

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.

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM! :shipit:

@fselmo fselmo merged commit 896fd77 into ethereum:main Jul 6, 2023
@fselmo fselmo deleted the async-sign-and-send-raw-middleware branch April 3, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants