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
Math.average #1170
Math.average #1170
Conversation
contracts/math/Math.sol
Outdated
function average(uint256 _a, uint256 _b) internal pure returns (uint256) { | ||
// (_a + _b) / 2 can overflow, so we distribute | ||
uint256 c = (_a / 2) + (_b / 2) + ((_a % 2 + _b % 2) / 2); | ||
assert((_a <= c && c <= _b) || (_a >= c && c >= _b)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leonardoalt how do you feel about these sort of assertions? Do you consider them necessary and correct, or do you think they may add too much unnecessary overhead (and gas costs)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the assertion unnecessary and somewhat arbitrary, as there are other conditions that should be checked in order to verify the correctness of the operation.
The implementation is good. Perhaps we should consider putting the operation in SafeMath
, given that it does have overflow protection. What do you think?
I've always understood |
I've removed the assertion, since we don't yet have a criteria for this kind of stuff. Maybe once we get a FV module? |
The need for an
average
function has come up a couple times.I also wanted to try out the new
assert
semantics as described in #1120.