-
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
Add async eth.replace_transaction #2847
Conversation
3da7bde
to
12099fd
Compare
e75399c
to
68cfdca
Compare
68cfdca
to
2425352
Compare
} | ||
txn_hash = await async_w3.eth.send_transaction(txn_params) | ||
try: | ||
async_w3.geth.miner.start() # type: ignore |
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.
The AsyncGethMiner
is not implemented, so I mark this test as an expected failure.
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.
The go-ethereum miner
namespace is now deprecated, but I can help implement it if needed, just like the other async Geth namespaces.
@@ -219,7 +223,7 @@ def assert_valid_transaction_params(transaction_params: TxParams) -> None: | |||
|
|||
|
|||
def prepare_replacement_transaction( | |||
w3: "Web3", | |||
w3: Union["Web3", "AsyncWeb3"], |
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.
The prepare_replacement_transaction
doesn't use any async method, so I just reuse the sync version of prepare_replacement_transaction
. I'm unsure if it is better to reuse it this way or copy it to async_transactions.py.
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.
I think leaving it here is the better option. 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.
This looks good to me! Thanks @e3243eric!
@@ -219,7 +223,7 @@ def assert_valid_transaction_params(transaction_params: TxParams) -> None: | |||
|
|||
|
|||
def prepare_replacement_transaction( | |||
w3: "Web3", | |||
w3: Union["Web3", "AsyncWeb3"], |
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.
I think leaving it here is the better option. Thanks!
What was wrong?
Related to Issue #2824. The
replace_transaction
method is unavailable on theAsyncEth
class.How was it fixed?
Add the
replace_transaction
method to theAsyncEth
class, and add corresponding tests.Todo:
Cute Animal Picture