-
Notifications
You must be signed in to change notification settings - Fork 143
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
Better sanity checks when creating/encoding transactions #179
Comments
@fabrice102 and @ian-algorand is the team taking PRs for this? Happy to follow #117 and #107 and implement something along these lines. |
@jkschin Samuel, I'm sorry, @Niraj-Kamdar has requested to tackle this issue yesterday. If he can't finish the issue, I'm happy for you to tackle it and submit it to the bounty program. Thanks for understanding! |
Nice, I will add sanity checks for other data-structures as well then. This library looks more promising and I will check it out if it works better: https://typeguard.readthedocs.io/en/latest/userguide.html |
I have added sanity type check for all the transaction classes. Many tests also broke so it's working as expected 😉 |
Currently, if arguments of transactions are of the wrong type, most of the time the SDK will not throw any exception and will even allow to sign and send the transactions.
But then, the node will reject the transaction with a relatively cryptic error message.
For example, if you make the asset ID a string:
you get the following error:
I am thinking of two complementary solutions:
algosdk.future.*
and/or when encoding/decoding, similar to what was done in Raises an exception if the amount field is incorrect #117 and Add checks that the note field is properly formed #107 but in a systematic way.The text was updated successfully, but these errors were encountered: