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
[move] add standard math library #3172
Conversation
|
||
/// Return the average of two. | ||
public fun average(a: u64, b: u64): u64 { | ||
(a & b) + (a ^ b) / 2 |
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 wonder if we really need the gas optimization here vs just doing (a + b) / 2
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.
Especially as a ^ b can overflow
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.
Got your point
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 think a + b
has the overflow risk other than the original solution.
let's do (a & b) + (a ^ b) >> 1
using bit ops for the whole expression.
module aptos_std::math { | ||
|
||
/// Return the largest of two numbers. | ||
public fun max(a: u64, b: u64): u64 { |
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.
What about u128
? And even if you don't introduce now, perhaps call these here max_u64
et. al?
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.
Alternatively, we can also put this into the module name, as in math64::max
et. al
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 love the way to split the file into the two
p = p * p; | ||
if (e % 2 == 1) { | ||
p = p * n; | ||
p | ||
} else { | ||
p | ||
} |
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.
p = p * p; | |
if (e % 2 == 1) { | |
p = p * n; | |
p | |
} else { | |
p | |
} | |
if (e % 2 == 1) { | |
p * p * n | |
} else { | |
p * p | |
} |
I wonder which saves more gas?
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.
Synced with @movekevin please fix the average function. Sorry for the inconvenience.
gonna land it for now. I'll fix it later. |
120975c
to
c5fb4f8
Compare
Forge is running with
|
Description
Adds a math standard library. There are currently four functions; max, min, average, and pow. These will be helpful, especially for DeFi builders on Aptos Network to calculate something.
Test Plan
In the file
math.move
.This change is