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

Feat tx required confs #593

Merged
merged 22 commits into from
Jun 3, 2020

Conversation

matnad
Copy link
Collaborator

@matnad matnad commented Jun 2, 2020

What I did

Add required_confs as an argument to transactions with a default value of 1 (same as current behaviour).

Setting required_confs = n > 1 will wait for n blocks to be mined before the transaction receipt is processed.

If during the confirmation time the transaction is dropped (uncle block), the transaction will wait until it is picked up again and has the required amount of confirmations.

if required_confs == 0 the transaction will immediately return a pending tx receipt and not wait for a confirmation.

Add tx.wait(n) to retroactively add required_confs and wait until they are confirmed.

TODO: Still needs more documentation

Closes #587

How I did it

Added the argument to all deply, transfer and transact functions.

Refactored the confirmation logic

How to verify it

I added tests for this.
or try accounts[0].transfer(accounts[1], "1 ether", {'required_confs': 3})

Checklist

  • I have confirmed that my PR passes all linting checks
  • I have included test cases
  • I have updated the documentation
  • I have added an entry to the changelog

@matnad matnad marked this pull request as ready for review June 3, 2020 09:22
Copy link
Member

@iamdefinitelyahuman iamdefinitelyahuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good overall, couple minor things and those commented fixtures need removing.

brownie/network/transaction.py Outdated Show resolved Hide resolved
brownie/network/transaction.py Outdated Show resolved Hide resolved
tests/network/transaction/test_required_confirmations.py Outdated Show resolved Hide resolved
matnad added 2 commits June 3, 2020 13:37
remove required_confs as state, add it as function argument (remove api doc)
add whitespace to "Transaction Lost....   "
Copy link
Member

@iamdefinitelyahuman iamdefinitelyahuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@iamdefinitelyahuman iamdefinitelyahuman merged commit 0abe1cf into eth-brownie:master Jun 3, 2020
@matnad matnad deleted the feat-tx-required-confs branch December 28, 2020 12:16
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.

User-defined wait time for broadcasted transactions
2 participants