-
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
Asyncify eth.get_transaction_receipt and eth.wait_for_transaction_receipt #2265
Asyncify eth.get_transaction_receipt and eth.wait_for_transaction_receipt #2265
Conversation
dcc26d0
to
54abdde
Compare
54abdde
to
4a311ec
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.
This looks good to me! Looks like there is a left over comment that can be removed. Did you manually test it to make sure it works like you think it should?
mungers=[default_root_munger] | ||
) | ||
|
||
# _get_transaction_receipt_awaitable: Method[Callable[[_Hash32], |
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 can go :)
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.
Yup, thanks for catching
tx_receipt = None | ||
if tx_receipt is not None: | ||
break | ||
_timeout.sleep(poll_latency) |
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.
Sorry to pop up again after approving this, but I think we want to be able to await this so it's not blocking. I think the most straightforward route is to get rid of the timeout context and use await asyncio.sleep
here. If you want to see if it makes a difference at all, you could add it to our benchmarking suite and see what happens :)
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.
Makes sense. _timeout.sleep replaced with await asyncio.sleep
be2c88f
to
8eebc82
Compare
8eebc82
to
5c7863e
Compare
- cleaned up async wait_for_transaction_receipt() - added tests for sync wait_for_transaction_receipt() - updated tests for async wait_for_transaction_receipt()
What was wrong?
Added async
eth.get_transaction_receipt
andeth.wait_for_transaction_receipt
Related to Issue #1413
Closes Issue #2129
How was it fixed?
Todo:
Cute Animal Picture