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

A function which returns the absolute value of a signed value #2984

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

barakman
Copy link
Contributor

Following feature request #2981, adding a function which returns the absolute (and obviously unsigned) value of a signed value.

@barakman barakman mentioned this pull request Nov 23, 2021
@Amxx
Copy link
Collaborator

Amxx commented Nov 23, 2021

Thank you @barakman for starting a PR.

This will need some documentation and a Changelog entry.

@barakman
Copy link
Contributor Author

Thank you @barakman for starting a PR.

This will need some documentation and a Changelog entry.

I can update the CHANGE log, but what other documentation do you want (and where)?

@Amxx
Copy link
Collaborator

Amxx commented Nov 23, 2021

Documentation is in docs/modules/ROOT/pages/utilities.adoc ... but I am now realizing that the Math part was not upgraded when we moved to 0.8.0 and safemath became native. Lets not touch that, we'll have to rework it later.

Just a changelog entry, fixing the lint test, and its good to merge !

@barakman
Copy link
Contributor Author

barakman commented Nov 23, 2021

Documentation is in docs/modules/ROOT/pages/utilities.adoc ... but I am now realizing that the Math part was not upgraded when we moved to 0.8.0 and safemath became native. Lets not touch that, we'll have to rework it later.

Just a changelog entry, fixing the lint test, and its good to merge !

BTW, I'm 100% sure that Math is the most adequate library for this function, but it seemed more adequate than SafeMath and SignedSafeMath.

I guess that SignedMath would be the most adequate (if you had such library).

@Amxx
Copy link
Collaborator

Amxx commented Nov 23, 2021

The utilities.adoc page is about everything under contract/utils.

I agree .abs() should be in Math.sol. SafeMath and SignedSafeMath are about overflow protection, while Math is about math primitives, which I believe abs() to be one.

@Amxx Amxx merged commit f6db5c1 into OpenZeppelin:master Nov 24, 2021
@frangio
Copy link
Contributor

frangio commented Jan 14, 2022

@Amxx When we merged this we didn't have SignedMath, but now we do. Should we move this function over there? (It hasn't been released yet so this is not a breaking change.)

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 this pull request may close these issues.

3 participants