Added wait_for_confirmation() to AlgodClient #214
Conversation
…with dedicated wait_for_confirmation()
|
|
algosdk/v2client/algod.py
Outdated
| # Wait until the block for the `current_round` is confirmed | ||
| self.status_after_block(current_round) | ||
|
|
||
| # Incremenent the `current_round` |
algosdk/future/transaction.py
Outdated
| @@ -3008,3 +3008,32 @@ def assign_group_id(txns, address=None): | |||
| tx.group = gid | |||
| result.append(tx) | |||
| return result | |||
|
|
|||
| def wait_for_confirmation(algod_client, transaction_id, timeout=None, **kwargs): | |||
There was a problem hiding this comment.
| def wait_for_confirmation(algod_client, transaction_id, timeout=None, **kwargs): | |
| def wait_for_confirmation(algod_client, txid, wait_rounds=0 **kwargs): |
Rationale:
txid is most used ident for txn ids across all the code
timeout usually assumes time, but the func accepts rounds
algosdk/future/transaction.py
Outdated
| while True: | ||
| # Check that the `timeout` has not passed | ||
| if timeout is not None and current_round > last_round + timeout: | ||
| raise error.AlgodResponseError(f"Wait for transaction id {transaction_id} timed out") |
There was a problem hiding this comment.
Please add a new error type ConfirmationTimeout or similar
algosdk/v2client/algod.py
Outdated
| @@ -373,16 +373,16 @@ def genesis(self, **kwargs): | |||
| """Returns the entire genesis file.""" | |||
| req = "/genesis" | |||
| return self.algod_request("GET", req, **kwargs) | |||
|
|
|||
| def proof(self, round_num, txid, **kwargs): | |||
|
|
|||
There was a problem hiding this comment.
all these changes are not needed
algosdk/future/transaction.py
Outdated
|
|
||
| # The transaction has been confirmed | ||
| if 'confirmed-round' in tx_info: | ||
| return |
There was a problem hiding this comment.
It would be useful to return the tx_info here. Often there is a need to check the some properties of this after waiting.
There was a problem hiding this comment.
LGTM. Formatting issues
$ black --check .
would reformat algosdk/v2client/algod.py
would reformat algosdk/future/transaction.py
would reformat test/steps/steps.py
Oh no! 💥 💔 💥
3 files would be reformatted, 61 files would be left unchanged.
The command "black --check ." exited with 1.
|
Please merge-in develop |
|
Github is telling me: "Only those with write access to this repository can merge pull requests." What should I do in this case? |
|
@algorandskiy meant to merge develop into this branch so it's up to date. However, the PR can still be merged automatically by github, so no need to do this. @DamianB-BitFlipper can you please add |
|
Ah, I see, I had "rebase and merge" option selected for some reason and it indicated conflicts. "squash and merge" is green - so please ignore my last comment. |
|
Should be pushed with the doc comment updated. I do not have write permissions to this repo, so I cannot be the one merging (I believe). |
|
Thank you for updating the docstring |
* Incorporated wait_for_confirmation into the AlgodClient * Replaced parts of code that manually wait for transaction to confirm with dedicated wait_for_confirmation() * Fixed small bug * Moved locatoin of wait_for_confirmation code to transaction.future * Moved locatoin of wait_for_confirmation code to transaction.future * Integrated comments from PR * Small change in how wait_rounds branches or not * Reformatted files * Added doc comment for algod_client
When using the Python SDK, I oftentimes find myself copying the same code to wait_for_confirmation() of a pending transaction.
The wait_for_confirmation() waits for a pending transaction to be accepted and confirmed by the network. Until then, it blocks. I have implemented this function with a timeout feature as well that will raise an Exception if the timeout is achieved.
I also went through the code and found spots where a rigged "wait_for_confirmation" like code was used to wait for the pending transaction to be accepted. Instead, I replaced those instances with the proper wait_for_confirmation()
I am unsure what to do about the CHANGELOG