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 function w3.eth.waitForTransactionReceipt(tx_hash) #574

Closed
carver opened this issue Jan 22, 2018 · 12 comments · Fixed by #669
Closed

Add function w3.eth.waitForTransactionReceipt(tx_hash) #574

carver opened this issue Jan 22, 2018 · 12 comments · Fixed by #669

Comments

@carver
Copy link
Collaborator

carver commented Jan 22, 2018

  • Version: 4

What was wrong?

It's a bit cumbersome to wait for a transaction to be mined. For example, after deploying a transaction, example code typically involves a small while loop, like in #573

This is a very common need, and should have an easy one-liner.

How can it be fixed?

Something like:

receipt = w3.eth.waitForTransactionReceipt(tx_hash)
contract = MyContract(receipt['contractAddress'])

# or

with w3.waitFor(lambda: w3.eth.getTransactionReceipt(tx_hash)) as receipt:
    contract = MyContract(receipt['contractAddress'])

Very open to alternative API suggestions.

This could potentially let us do cool things eventually, like use pubsub under the hood. Note that pubsub integration would be significantly easier in the first API than the second. But the first API implies adding other new functions that start with waitFor at some point in the future.

@jstoxrocky
Copy link
Contributor

Hey @carver I've been using this function. Is this different from different from what you were thinking?
https://github.com/ethereum/web3.py/blob/master/web3/utils/transactions.py#L139

@carver
Copy link
Collaborator Author

carver commented Jan 23, 2018

@jstoxrocky Yeah, it makes sense to reuse that implementation. But utils is presumed to be a private rather than a public API, and has no guarantees about API stability, etc.

So all that's really left for this issue is:

  1. Decide on the public API
  2. Wire in that public API to that utils implementation
  3. Document it
  4. Test it

@scottydelta
Copy link

@carver, I would like to work on this if you can provide some direction. thanks

@carver
Copy link
Collaborator Author

carver commented Feb 19, 2018

@scottydelta sure! Let's use that first API. So add waitForTransactionReceipt to eth.py which calls through to the wait_for_transaction_receipt method using the web3 attribute in the eth module.

Docs go in the docs folder, and a test to make sure that the method doesn't return until the transaction is mined.

Posting the pull request as a work in progress early on is a good way to make sure that you're in the right track or provide context when asking for help. Please do reach out if you get stuck.

@pipermerriam
Copy link
Member

@carver this is somewhat orthogonal but...

What do you think about introducing a web3.eth.async module with a coroutine version of this as well? It'd be nice to start supporting native asyncio. We might need to go as far as implementing new providers as well with coroutine versions of Provider.make_request and the corresponding Manager methods. This would also open the door for a Filter API with callbacks for logs.

@carver
Copy link
Collaborator Author

carver commented Feb 19, 2018

Or maybe web3.async.* that replicates the whole web3 namespace? Seems reasonable, but probably after getting v4 stable.

@pipermerriam
Copy link
Member

cc @NIC619 as I think this would be beneficial for the VMC at some point. Just want to put this in your awareness.

@carver
Copy link
Collaborator Author

carver commented Feb 20, 2018

@NIC619 moving async discussion to a new #657 so @scottydelta and I can focus on waitForTransactionReceipt here.

@mhchia
Copy link
Contributor

mhchia commented Feb 20, 2018

This is pretty helpful! Thanks for the work

@scottydelta
Copy link

@scottydelta sure! Let's use that first API. So add waitForTransactionReceipt to eth.py which calls through to the wait_for_transaction_receipt method using the web3 attribute in the eth module.

@carver, if we follow this approach then we will be making unnecessary loop.

  1. User calls web3.eth.waitForTransactionReceipt
  2. web3.eth.waitForTransactionReceipt calls web3.utils.transactions.wait_for_transaction_receipt
  3. web3.utils.transactions.wait_for_transaction_receipt calls web3.eth.getTransactionReceipt

so wont it be easier if web3.eth.waitForTransactionReceipt just calls web3.eth.getTransactionReceipt? Am I missing something?

@carver
Copy link
Collaborator Author

carver commented Feb 23, 2018

Only that we try to avoid keeping the logic at the "edges" where the public API is, so putting the logic in private utility modules and referencing it in the public API is more consistent with our style. I don't see this as being a significant performance hit, relative to the other things happening in the flow.

@scottydelta
Copy link

ok cool, I will add it in the way you suggested, thanks.

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 a pull request may close this issue.

5 participants