Skip to content
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

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

e3243eric
Copy link
Contributor

@e3243eric e3243eric commented Feb 24, 2023

What was wrong?

Related to Issue #2824. The replace_transaction method is unavailable on the AsyncEth class.

How was it fixed?

Add the replace_transaction method to the AsyncEth class, and add corresponding tests.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@e3243eric e3243eric force-pushed the async-replace_transaction branch 4 times, most recently from 3da7bde to 12099fd Compare February 24, 2023 14:07
e3243eric added a commit to e3243eric/web3.py that referenced this pull request Feb 24, 2023
e3243eric added a commit to e3243eric/web3.py that referenced this pull request Feb 24, 2023
}
txn_hash = await async_w3.eth.send_transaction(txn_params)
try:
async_w3.geth.miner.start() # type: ignore
Copy link
Contributor Author

@e3243eric e3243eric Feb 24, 2023

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.

Copy link
Contributor Author

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"],
Copy link
Contributor Author

@e3243eric e3243eric Feb 24, 2023

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.

Copy link
Collaborator

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!

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 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"],
Copy link
Collaborator

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!

@kclowes kclowes merged commit a561cf0 into ethereum:master Apr 6, 2023
@e3243eric e3243eric deleted the async-replace_transaction branch April 7, 2023 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants