Joystream / Joystream/joystream

Investigate places where `saturating_*` arithmetic should be replaced with `checked_*`

Open
#4,335 0 comments 0 reactions 1 assignee Claimed by @Lezek123 View on GitHub
runtime tech-debt
Dominant language
Rust
Stars
1.4k
Forks
116
PR merge metrics
No merged PRs in 30d

Description

Currently we're using `saturating_*` arithmetic in places where, if the max/min unsigned integer value was ever exceeded, that would actually result in a bad state. We sometimes do it under the assumption that it's just never going to happen, for example:

```rust
>::mutate(category_id, thread_id, |thread| {
// non editable post should leave the counter untouched
thread.number_of_editable_posts = thread.number_of_editable_posts.saturating_add(1);
});
```

Here `thread.number_of_editable_posts` is `u64` so we expect the `u64::MAX` to never be reached. However if we were to change the type to `u16` for example, this could become an issue, because the `thread.number_of_editable_posts` would no longer represent the valid number of editable posts if it ever reached `> 65535`. This in turn would make it possible to remove a thread that still has some outstanding posts, therefore breaking further assumptions about the runtime state in the code.

We should investigate the usages of `saturating_*` arithmetic in the code and replace them with safer `checked_*` equivalents where it makes sense.

┆Issue is synchronized with this [Asana task](https://app.asana.com/0/1201958687417145/1203092173559950) by [Unito](https://www.unito.io)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.