-
Notifications
You must be signed in to change notification settings - Fork 757
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
Proposal for waitForTransaction
improvement
#1237
Comments
hello @Cussone can i hop on this issue? |
Not sure what you are referring to, but feel free to write anything you think about this. |
Has been discussed in Slack: will not be modified, as user can customize the value. |
Yup, that was the conclusion for the current version, but I opened this proposal for the next version, as we also discussed on Slack. |
Another improvements would be:
I've had issue with my tests where everything was super slow (even when using starknet-devnet) and actually the issue is that each call to |
The issue is with starknetjs implementation of waitForTransaction, that will always take at least 10s with default settings. We can improve it a lot by just decreasing intervals. starknet-io/starknet.js#1237 (comment)
The issue is with starknetjs implementation of waitForTransaction, that will always take at least 10s with default settings. We can improve it a lot by just decreasing intervals. starknet-io/starknet.js#1237 (comment)
Now that block times are much faster it would make sense to lower the
retryInterval
param from5000ms
(waited on twice, so it's10s
) to something more realistic.Also, I find it weird that
retryInterval
is an optional param. In my opinion, it would make more sense to require it explicitly, with a suggested value in the docs. I see it as a better DX.This would be a breaking change. So, it is something to consider for the next major release.
The text was updated successfully, but these errors were encountered: