-
Notifications
You must be signed in to change notification settings - Fork 87
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
Remove any unchecked arithmetic #170
Comments
Just to elaborate on this: there are For optimization, there are also unstable and unsafe |
Looks like clippy has a lint for unchecked math, Once we've addressed this issue, we can add |
After spending a couple of days on this, it's not clear the best approach. In terms of capturing the error effectively, it makes sense to return It was when those It seems more valuable to have thorough fuzzing/prop testing than be able to catch the error telling us the chain is bricked. With all that in mind, I'm going to put this story back and work on higher-priority issues. |
In order to prevent overflows and unpredictable behaviors, all arithmetic (esp gas or free balances) should use
checked_*
operations rather than std ops likeAddAssign
. In cases when we know that it is not possible to fail or the context around is valid, then we can ignore orexpect
the operation.The main idea is to make the code safe and keep API as simple as possible. If the input from the user may invalidate the state/behavior, then we need to return an
Option
orError
.It is better to do this change crate by crate to avoid conflicts with other PRs:
fuel-merkle
Deny clippy::arithmetic_side_effects for fuel-merkle #729fuel-crypto
: Deny clippy::arithmetic_side_effects #725fuel-tx
: Deny clippy::arithmetic_side_effects #725fuel-types
: Deny clippy::arithmetic_side_effects #725fuel-vm
: Deny clippy::arithmetic_side_effects #725The text was updated successfully, but these errors were encountered: