Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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!: add a non-blocking call for deploy contract #2597
base: master
Are you sure you want to change the base?
feat!: add a non-blocking call for deploy contract #2597
Changes from 11 commits
1649707
7051478
80303b8
5f6ffdd
188474f
ea3fd88
f54f398
5ea4cec
1ff12dd
9fa3296
5d5a85d
7eabf88
c23e76e
db54c12
7aecce8
8ee880e
c727f37
647f906
6eb8dcb
7c800e1
5626f9a
84a9540
f8fa58a
972118c
ffd8aa8
c41de19
0d8cb33
26c5dbe
f528fac
15968e7
86508f9
17b9832
922ca4a
b313e3c
b9c0ed9
d2e0195
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the expected behaviour when calling a contract function before the transaction has been resolved via
waitForResult
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petertonysmith94 An error will be thrown because the contract was not deployed yet, therefore, it does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worthy of mentioning is that if the
awaitExecution = true
argument is passed, then there is no need to callwaitForResult
. Whoever uses the non-blockingawaitExecution = false
should accept the nature of blockchain and do awaitForResult
before running contract calls.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the rationale and that this is a delicate issue.
Still, I suspect this error case can be counterintuitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to still not fail a contract call and not have this error case, we could couple deployment to contract calls and wait for deployment to finish before running the actual contract call:
Then we can just explain in the documentation that, for
awaitExecution=false
, the first contract call might take a bit longer because the contract might not have been deployed yet.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Torres-ssf what would be the error type thrown if one calls the contract before it's deployed? I think a better path would be to document that error so that consumers can error handle appropriately and retry when necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider splitting the functionality.