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

Add reset function to Counters library #2677

Closed
simontianx opened this issue May 17, 2021 · 4 comments · Fixed by #2678
Closed

Add reset function to Counters library #2677

simontianx opened this issue May 17, 2021 · 4 comments · Fixed by #2678

Comments

@simontianx
Copy link

simontianx commented May 17, 2021

🧐 Motivation

In a scenario like moving a point in one direction in a bounded segment, a counter can reach the upper limit and then needs to get reset to start all over again. It would be great if a reset function can be added to the Counters library.

📝 Details

function reset(Counter storage counter) internal {
    counter._value = 0;
}
@Amxx
Copy link
Collaborator

Amxx commented May 17, 2021

Hello issue @xu-tian , and tank for your issue.
This sounds like a reasonable request that is easy to implement!

@Amxx Amxx mentioned this issue May 17, 2021
3 tasks
@simontianx
Copy link
Author

simontianx commented May 18, 2021

You are welcome.

I noticed unchecked is added in the new function, and I also noticed increment and decrement also have unchecked as well. However, by default, the values are checked and will throw when overflow or underflow. Isn't that the right feature to have?

function reset(Counter storage counter) internal {
    unchecked {
        counter._value = 0;
    }
}

@Amxx
Copy link
Collaborator

Amxx commented May 18, 2021

uncheck removed solidity 0.8.0 built-in safe to save some gas. It is in the counter library, because the checks are not needed:

  • for increment to overflow, you would need to call it 2**256 times, which is not realistic in this universe
  • decrement is already protected for overflow thanks to the require() that will raise an error string.

In reset, assigning 0 doesn't include safemath protection, so unchecked is not needed (and won't do anything good or bad)

@simontianx
Copy link
Author

Got it. I did a test and it is indeed using less gas. Thanks for the clarification.

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

Successfully merging a pull request may close this issue.

2 participants