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

Handle the overflow in "balances" calculation. #2428

Open
rafal-ch opened this issue Nov 11, 2024 · 0 comments
Open

Handle the overflow in "balances" calculation. #2428

rafal-ch opened this issue Nov 11, 2024 · 0 comments

Comments

@rafal-ch
Copy link
Contributor

rafal-ch commented Nov 11, 2024

Originally posted by @rafal-ch in #2383 (comment)

This issue is for tracking the error handling in the indexation module:

For Balances:

At the moment all "amounts" for coins, messages and contracts is stored as u64. When implementing the "balances cache" I noticed that when we calculate the total balances, we use the accumulator which is u64 and we call saturating_add() on it. This may lead to the balances being calculated incorrectly.

In the original PR the accumulator has been changed to u128, which vastly reduces the problem with accumulation (one would require humongous amount of u64s to overflow u128). However, there is still a problem with balance deduction, as we might find ourselves in the situation where, for example, our balance is 100, but we try to reduce it by 200.

This is a fatal error and indicates a serious bug in the logic. Currently, we just log it and continue.

To be considered:

  1. Properly handle the underflow error
  • maybe change the return type of "balances()" from StorageResult to something else, since overflow is not a storage error
  • update integration tests - some of them are written with the assumption that balances calculation is infallible
  1. Consider changing all "amounts" to be u128, so that even a single coin can have a balance greater than u64

For Coins To Spend:

When "coins to spend" index is considered, the possible errors to be handled are:

  1. trying to index the same coin twice
  2. trying to remove the coin that is not in the index
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

No branches or pull requests

1 participant