-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
✨ Added async eth.modify_transaction #2921
Conversation
We've been busy with other things, but will take a look when we get a chance. Thanks for the PR! |
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.
It looks like you have some failing tests on CI. It wasn't running before for some reason. I also left a comment about re-using the existing extract_valid_transaction_params
. Let me know if you don't have time/bandwidth to update this PR and one of us can take over. Thanks!
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! Looks good!
What was wrong?
Related to Issue #2825
How was it fixed?
Added the
modify_transaction
method to theAsyncEth
class, and adds corresponding tests.Todo:
Cute Animal Picture