-
Notifications
You must be signed in to change notification settings - Fork 9
Fixed point sqrt
#286
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
Open
immrsd
wants to merge
16
commits into
main
Choose a base branch
from
fixed-point-sqrt
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Fixed point sqrt
#286
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
7c5064c
Add fixed-point helper for sqrt calculation
immrsd c0631d6
Implement sqrt for SD29x9 type
immrsd 45d9674
Implement sqrt for UD30x9
immrsd 59e4a9e
Add tests for fixed-point sqrt impl
immrsd f100373
Add sqrt and remaining functions to README
immrsd 25fd007
Merge main
immrsd 06f659d
Format files
immrsd cedb015
Use sqrt_floor implementation from math/core package
immrsd 699d6af
Address review comments
immrsd 63e7155
Merge main
immrsd 2aa9db4
Move sqrt_floor function from fp_helpers to common module
immrsd 7e9cb7f
Fix tests to use assert_eq instead of deprecated expect
immrsd b62c72c
Remove redundant zero check in ud30x9::sqrt
immrsd 53fd0bd
Format files
immrsd bcf64d4
Merge branch 'main' into fixed-point-sqrt
bidzyyys db5b40b
Address review comments
immrsd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| #[test_only] | ||
| module openzeppelin_fp_math::sd29x9_sqrt_tests; | ||
|
|
||
| use openzeppelin_fp_math::sd29x9; | ||
| use openzeppelin_fp_math::sd29x9_base; | ||
| use openzeppelin_fp_math::sd29x9_test_helpers::{pos, neg}; | ||
| use std::unit_test::assert_eq; | ||
|
|
||
| const SCALE: u128 = 1_000_000_000; | ||
| const MAX_POSITIVE_VALUE: u128 = 0x7FFF_FFFF_FFFF_FFFF_FFFF_FFFF_FFFF_FFFF; | ||
|
|
||
| // ==== Tests ==== | ||
|
|
||
| #[test] | ||
| fun sqrt_of_zero_is_zero() { | ||
| assert_eq!(sd29x9::zero().sqrt(), sd29x9::zero()); | ||
| } | ||
|
|
||
| #[test] | ||
| fun sqrt_of_positive_one() { | ||
| assert_eq!(sd29x9::one().sqrt(), sd29x9::one()); | ||
| } | ||
|
|
||
| #[test] | ||
| fun sqrt_of_positive_perfect_squares() { | ||
| // sqrt(+4.0) = +2.0 | ||
| assert_eq!(pos(4 * SCALE).sqrt(), pos(2 * SCALE)); | ||
| // sqrt(+9.0) = +3.0 | ||
| assert_eq!(pos(9 * SCALE).sqrt(), pos(3 * SCALE)); | ||
| // sqrt(+25.0) = +5.0 | ||
| assert_eq!(pos(25 * SCALE).sqrt(), pos(5 * SCALE)); | ||
| // sqrt(+100.0) = +10.0 | ||
| assert_eq!(pos(100 * SCALE).sqrt(), pos(10 * SCALE)); | ||
| // sqrt(+10000.0) = +100.0 | ||
| assert_eq!(pos(10_000 * SCALE).sqrt(), pos(100 * SCALE)); | ||
| } | ||
|
|
||
| #[test] | ||
| fun sqrt_of_positive_fractional_squares() { | ||
| // sqrt(+0.25) = +0.5 | ||
| assert_eq!(pos(250_000_000).sqrt(), pos(500_000_000)); | ||
| // sqrt(+0.01) = +0.1 | ||
| assert_eq!(pos(10_000_000).sqrt(), pos(100_000_000)); | ||
| // sqrt(+2.25) = +1.5 | ||
| assert_eq!(pos(2_250_000_000).sqrt(), pos(1_500_000_000)); | ||
| } | ||
|
|
||
| #[test] | ||
| fun sqrt_truncates_irrational_results() { | ||
| // sqrt(+2.0) = +1.414213562 (truncated) | ||
| assert_eq!(pos(2 * SCALE).sqrt(), pos(1_414_213_562)); | ||
| // sqrt(+3.0) = +1.732050807 (truncated) | ||
| assert_eq!(pos(3 * SCALE).sqrt(), pos(1_732_050_807)); | ||
| // sqrt(+5.0) = +2.236067977 (truncated) | ||
| assert_eq!(pos(5 * SCALE).sqrt(), pos(2_236_067_977)); | ||
| } | ||
|
|
||
| #[test] | ||
| fun sqrt_of_max_positive() { | ||
| // sqrt(sd29x9::max()) should not abort and satisfy the floor property | ||
| let result = sd29x9::max().sqrt(); | ||
| let r = result.unwrap() as u256; | ||
| let max_scaled = (sd29x9::max().unwrap() as u256) * (SCALE as u256); | ||
| assert!(r * r <= max_scaled); | ||
| assert!((r + 1) * (r + 1) > max_scaled); | ||
| } | ||
|
|
||
| #[random_test] | ||
| fun sqrt_result_is_always_non_negative(raw: u128) { | ||
| let raw = raw % (MAX_POSITIVE_VALUE + 1); | ||
| let result = sd29x9::wrap(raw, false).sqrt(); | ||
| // Result is non-negative: raw bits should not have sign bit set | ||
| assert!(result.unwrap() <= MAX_POSITIVE_VALUE); | ||
| } | ||
|
|
||
| #[random_test] | ||
| fun sqrt_floor_invariant(raw: u128) { | ||
| let raw = raw % (MAX_POSITIVE_VALUE + 1); | ||
| let result = sd29x9::wrap(raw, false).sqrt(); | ||
| // Floor property: r^2 <= x * SCALE < (r + 1)^2 | ||
| let r = result.unwrap() as u256; | ||
| let scaled = (raw as u256) * (SCALE as u256); | ||
| assert!(r * r <= scaled); | ||
| assert!((r + 1) * (r + 1) > scaled); | ||
|
immrsd marked this conversation as resolved.
|
||
| } | ||
|
|
||
| #[test] | ||
| fun sqrt_squared_roundtrip_for_perfect_squares() { | ||
| let values = vector[ | ||
| pos(4 * SCALE), | ||
| pos(9 * SCALE), | ||
| pos(25 * SCALE), | ||
| pos(250_000_000), // 0.25 | ||
| ]; | ||
| values.destroy!(|x| { | ||
| let root = x.sqrt(); | ||
| assert_eq!(root.mul(root), x); | ||
| }); | ||
| } | ||
|
|
||
| #[test, expected_failure(abort_code = sd29x9_base::ENegativeSqrt)] | ||
| fun sqrt_of_negative_aborts() { | ||
| neg(SCALE).sqrt(); | ||
| } | ||
|
|
||
| #[test, expected_failure(abort_code = sd29x9_base::ENegativeSqrt)] | ||
| fun sqrt_of_small_negative_aborts() { | ||
| neg(1).sqrt(); | ||
| } | ||
|
|
||
| #[test, expected_failure(abort_code = sd29x9_base::ENegativeSqrt)] | ||
| fun sqrt_of_min_value_aborts() { | ||
| sd29x9::min().sqrt(); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Shouldn't it be imported with 3 sections?
1.
mainnet2.
testnet3.
dev-dependencies?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.
This approach should copy/paste the function at deployment so there's no environment linking. What I had in mind was linking through the mvr not locally. But if we confirm with Sui (and we must before merging) that this indeed copies only the required function source code and not the whole math package we can continue with this approach.