Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants