added waitForTransactionReceipt to web3.eth which returns the transac… #669

scottydelta · 2018-02-23T05:27:17Z

…tion after its mined

What was wrong?

Fixes #574

How was it fixed?

Added waitForTransactionReceipt to web3.eth which waits for transaction to mine and returns the transaction once it is mined.

Cute Animal Picture

scottydelta · 2018-02-23T05:28:29Z

Test and docs yet to be added. please add Work in Progress tag to it. thanks

scottydelta · 2018-02-23T06:55:09Z

how can I go about adding a test since it needs to have a pending transaction before it returns receipt? i did test it manually on Ropsten Network by creating a transaction manually and then running the function to test if it actually waits and returns the receipt after transaction is mined. thanks

carver · 2018-02-23T18:38:32Z

There is some new functionality in eth-tester to enable this. One approach would be something like:

eth_tester_provider = list(web3.providers).pop()
assert hasattr(eth_tester_provider, 'ethereum_tester')
eth_tester = eth_tester_provider.ethereum_tester

def disable_in_1s():
    time.sleep(1)
    eth_tester.enable_auto_mine_transactions()

eth_tester.disable_auto_mine_transactions()
txn_hash = w3.eth.sendTransaction(...)

with pytest.raises(TimeoutException):
    w3.eth.waitForTransactionReceipt(txn_hash, timeout=0.5)

Thread(disable_in_1s, daemon=True).start()

txn_receipt = eth.waitForTransactionReceipt(txn_hash, timeout=2)
assert txn_receipt is not None and txn_receipt['block_hash']

But yeah, it's a little hairy with the thread blocking. There's probably a better approach, but that would get it done.

pipermerriam · 2018-02-23T19:40:22Z

@carver what do you think about testing via something like this using middleware to simulate a transaction not yet being mined.

counter = itertools.counter()
def unmined_receipt_simulator_middleware(make_request, web3):
    receipt_counters = collections.defaultdict(itertools.counter)

    def middleware(method, params):
        if method == 'eth_getTransactionReceipt':
            txn_hash = params[0]
            if next(receipt_counters[txn_hash]) < 2:
                return {'result': None}
        return make_request(method, params)
    return middleware

carver · 2018-02-23T21:46:51Z

Yes, I like that much better 👍 It's simpler, and takes less time to run.

For @scottydelta: itertools.count instead of counter

It would be nice to confirm that the middleware is working with a manual check of the empty result first, then the wait (but with a short timeout so that the test completes quickly even if it fails):

assert w3.eth.getTransactionReceipt(txn_hash) == None

receipt = w3.eth.waitForTransactionReceipt(txn_hash, timeout=1)
assert receipt['transaction_hash'] == txn_hash
assert receipt['block_hash']

carver · 2018-03-01T23:42:56Z

You can run make lint-roll locally to cleanup and check for linting issues.

Based on the circle lint run, the only issue left is the ordering of the imports:

ERROR: /home/circleci/repo/web3/eth.py Imports are incorrectly sorted.
--- /home/circleci/repo/web3/eth.py:before	2018-02-26 07:03:47.343238
+++ /home/circleci/repo/web3/eth.py:after	2018-02-26 07:04:02.690568
@@ -41,9 +41,9 @@
     assert_valid_transaction_params,
     extract_valid_transaction_params,
     get_buffered_gas_estimate,
-    wait_for_transaction_receipt,
     get_required_transaction,
     replace_transaction,
+    wait_for_transaction_receipt,

scottydelta · 2018-03-02T00:22:37Z

@carver I have fixed it on my end, just trying to make some time to add a test and complete this pull request. It will be done soon, thanks.

carver · 2018-03-02T00:51:44Z

web3/eth.py

@@ -187,6 +188,10 @@ def getTransactionFromBlock(self, block_identifier, transaction_index):
            [block_identifier, transaction_index],
        )

+    def waitForTransactionReceipt(self, transaction_hash):


I would expect this to have a timeout, like the underlying method.

I had that in mind, missed it somehow. I will add that, thanks.

scottydelta · 2018-03-06T10:15:54Z

@carver while I understand your initial suggested method of testing, I am finding @pipermerriam middleware method to stimulate un-mined transaction hard to understand. I haven't worked with tests alot.

is the actual test executed like this?

make_request = Mock()
txn_receipt = unmined_receipt_simulator_middleware(make_request, web3)
txn_hash = web3.eth.sendTransaction({
    'from': web3.eth.coinbase,
    'to': '0xd3CdA913deB6f67967B99D67aCDFa1712C293601',
})
assert txn_receipt('eth_getTransactionReceipt', [txn_hash]) is None
..

pipermerriam · 2018-03-08T21:07:31Z

@scottydelta

Not exactly. You would add the unmined_receipt_simulator_middleware to the web3 middlewares:

w3.middleware_stack.add(unmined_receipt_simulator_middleware)

Then you can call web3.eth.getTransactionReceipt and the first two calls will always return None, after which it will return the receipt.

scottydelta · 2018-03-11T02:33:42Z

@pipermerriam thanks for the guidance. Wouldn't this test getTransactionReceipt instead of newly added method waitForTransactionReceipt? I know in the end getTransactionReceipt get called when waitForTransactionReceipt is invoked.

pipermerriam · 2018-03-12T17:36:03Z

@scottydelta the middlewares don't know the difference between web3.eth.getTransactionReceipt and web3.eth.waitForTransactionReceipt. The only thing that middlewares are aware of are the underlying JSON-RPC methods.

So, by constructing this middleware which responds to eth_getTransactionReceipt JSON-RPC requests, you're emulating the behavior of an unmined transaction by intercepting the request and returning None.

So, the idea is that you are testing web3.eth.waitForTransactionReceipt by artificially creating the JSON-RPC behavior for an unmined transaction.

Make sense?

scottydelta · 2018-03-14T03:38:20Z

@pipermerriam It makes sense but the method waitForTransaction which I just added never gets tested, instead the JSON-RPC method eth_getTransactionReceipt gets tested which must already be getting tested in some other test. is that a normal behavior?

pipermerriam

See my comments on how to change the tests.

pipermerriam · 2018-03-14T16:11:53Z

tests/core/eth-module/test_transactions.py

+        'to': '0xd3CdA913deB6f67967B99D67aCDFa1712C293601',
+        'value': 123457
+    })
+    assert web3.eth.getTransactionReceipt(txn_hash) is None


I would leave this assertion in place. This verifies that the transaction is showing as unmined.

pipermerriam · 2018-03-14T16:12:33Z

tests/core/eth-module/test_transactions.py

+    })
+    assert web3.eth.getTransactionReceipt(txn_hash) is None
+    assert web3.eth.getTransactionReceipt(txn_hash) is None
+    txn_receipt = web3.eth.getTransactionReceipt(txn_hash)


This should be changed to use the web3.eth.waitForTransactionReceipt API, and the line above this should be removed.

pipermerriam · 2018-03-14T16:13:06Z

web3/middleware/stimulate_unmined_transaction.py

+    def middleware(method, params):
+        if method == 'eth_getTransactionReceipt':
+            txn_hash = params[0]
+            if next(receipt_counters[txn_hash]) < 2:


I would go ahead and bump this number up to something like 5 to ensure that the waitForTransactionReceipt code hits this a few times prior to getting the actual receipt.

carver · 2018-04-03T20:28:24Z

@scottydelta it looks like this is almost at the finish line. Are you able to work on it more?

- stimulate -> simulate - poll latency reduced to 100ms - plenty of invocations for test to try out - doc fixups

carver added the Work in Progress label Feb 23, 2018

scottydelta closed this Feb 23, 2018

scottydelta reopened this Feb 23, 2018

carver reviewed Mar 2, 2018

View reviewed changes

scottydelta closed this Mar 14, 2018

scottydelta reopened this Mar 14, 2018

pipermerriam reviewed Mar 14, 2018

View reviewed changes

carver force-pushed the master branch from c741b05 to 23b2001 Compare April 6, 2018 00:54

Vikash Bajaj added 2 commits April 6, 2018 09:14

added docs entry for new waitForTransactionReceipt method in web3.eth

bb13367

added test for new waitForTransaction method in web3.eth

9538777

carver force-pushed the master branch from 23b2001 to 7aa26a1 Compare April 6, 2018 16:15

carver removed the Work in Progress label Apr 6, 2018

Clean up waitForTransactionReceipt for merging

8703399

- stimulate -> simulate - poll latency reduced to 100ms - plenty of invocations for test to try out - doc fixups

carver force-pushed the master branch from 7aa26a1 to 8703399 Compare April 6, 2018 16:26

carver merged commit c25be34 into ethereum:master Apr 6, 2018

added waitForTransactionReceipt to web3.eth which returns the transac… #669

added waitForTransactionReceipt to web3.eth which returns the transac… #669

Uh oh!

Conversation

scottydelta commented Feb 23, 2018

What was wrong?

How was it fixed?

Cute Animal Picture

Uh oh!

scottydelta commented Feb 23, 2018

Uh oh!

scottydelta commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carver commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pipermerriam commented Feb 23, 2018

Uh oh!

carver commented Feb 23, 2018

Uh oh!

carver commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottydelta commented Mar 2, 2018

Uh oh!

carver Mar 2, 2018

Choose a reason for hiding this comment

Uh oh!

scottydelta Mar 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottydelta commented Mar 6, 2018

Uh oh!

pipermerriam commented Mar 8, 2018

Uh oh!

scottydelta commented Mar 11, 2018

Uh oh!

pipermerriam commented Mar 12, 2018

Uh oh!

scottydelta commented Mar 14, 2018

Uh oh!

pipermerriam left a comment

Choose a reason for hiding this comment

Uh oh!

pipermerriam Mar 14, 2018

Choose a reason for hiding this comment

Uh oh!

pipermerriam Mar 14, 2018

Choose a reason for hiding this comment

Uh oh!

pipermerriam Mar 14, 2018

Choose a reason for hiding this comment

Uh oh!

carver commented Apr 3, 2018

Uh oh!

Uh oh!

scottydelta commented Feb 23, 2018 •

edited

Loading

carver commented Feb 23, 2018 •

edited

Loading

carver commented Mar 1, 2018 •

edited

Loading

scottydelta Mar 2, 2018 •

edited

Loading