test(math): add fuzzer for LegacyDec.ApproxRoot to catch stark deviations #17770
Conversation
odeke-em
force-pushed
the
math-add-fuzzer-for-ApproxRoot
branch
from
93a8619 to
e13ea18
Compare
github-prbot
requested review from
a team,
kocubinski and
atheeshp
and removed request for
a team
julienrbrt
changed the title
…ions This fuzzer performs a roundtrip comparison for square roots comparing the value from: * squaring LegacyDec.ApproxRoot(2) against itself then checking if the value is very close to the original whole value * comparing against the Go standard library's math/big.Float.Sqrt and panicking if we find a deviation of greater than 5% It's great to note that the results from this library so far from fuzzing show a higher precision/accuracy than math/big.Float.Sqrt for example with: LegacyDec.ApproxRoot(100000000000000000.000000000000000000, 2) Stdlib sqrt: 316227766.000000000000000000 cosmossdk.io/math.*Dec.ApproxSqrt: 316227766.016837933199889354 Python3.7.6: math.sqrt(100000000000000000): 316227766.01683795 LegacyDec.ApproxRoot(10000700000000000.000000000000000000, 2) Stdlib sqrt: 100003499.900000000000000000 cosmossdk.io/math.*Dec.ApproxSqrt: 100003499.938752143656215533 Python3.7.6: math.sqrt(10000700000000000): 100003499.93875214 Updates PR #16141 Updates PR #17725
odeke-em
force-pushed
the
math-add-fuzzer-for-ApproxRoot
branch
from
e13ea18 to
4a57301
Compare
| // Firstly ensure that gotApproxSqrt * gotApproxSqrt is | ||
| // super duper close to the value of dec. | ||
| squared := gotApproxSqrt.Mul(gotApproxSqrt) | ||
| if !squared.Equal(dec) { |
There was a problem hiding this comment.
Nit: This if condition is just an optimization, right? If so, I'd drop it for clarity.
| f.Fuzz(func(t *testing.T, input string) { | ||
| splits := strings.Split(input, ",") | ||
| if len(splits) < 2 { | ||
| // Invalid input, just skip over it. | ||
| return | ||
| } |
There was a problem hiding this comment.
Many input strings are going to be immediate rejected. I suggest tightening the fuzzing function to something like
func(t *testing.T, dec string, nthRoot uint64)Further, I think you can avoid invalid strings by going through big.Rat:
func(t *testing.T, nomBytes, denomBytes []byte, nthRoot uint64)
nom := big.Int{}.SetBytes(nomBytes)
denom := big.Int{}.SetBytes(denomBytes)
decRat := big.Rat{}.SetFrac(nom, denom)
dec := big.Float{}.SetRat(decRat)If you want negative numbers, add a bool argument to the fuzzing function.
WalkthroughThe introduced changes enhance the Changes
Poem
Validation with GitHub issue(Beta)Assessment of the PR changes against GitHub Issue
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- math/fuzz_test.go (2 hunks)
Additional comments: 7
math/fuzz_test.go (7)
1-8: The new imports are used appropriately in the fuzzing function. Ensure that these packages are included in your project dependencies.
29-32: The global variables
max5PercentanddecDiv100are used for precision comparison in the fuzzing function. Ensure that these values are appropriate for your use case.34-138: The
FuzzLegacyDecApproxRootfunction is well-structured and handles errors appropriately. It also includes checks to ensure the input is valid and the results are within the expected range. However, consider adding more comments to explain the logic and calculations, especially for the precision comparison and the use ofmax5PercentanddecDiv100.+ // max5Percent is the maximum allowed deviation (5%) between the square of the approximated root and the original value. + // decDiv100 is used to convert the deviation to a percentage.
50-64: The function handles invalid inputs by simply returning without throwing an error. This is a good practice for fuzzing as it allows the function to continue testing with other inputs.
78-84: The function handles errors from
dec.ApproxRoot(nthRoot)appropriately. It skips over out-of-bounds errors and fails the test for other errors.92-100: The function checks if the square of the approximated root is close to the original value. If the difference exceeds 5%, it fails the test. This is a good practice for fuzzing as it helps identify significant deviations.
102-136: The function compares the result of
LegacyDec.ApproxRootwith the Go standard library'smath/big.Float.Sqrtfunction. If the difference exceeds 5%, it fails the test. This is a good practice for fuzzing as it helps identify significant deviations.
| @@ -22,3 +25,114 @@ func FuzzLegacyNewDecFromStr(f *testing.F) { | |||
| } | |||
| }) | |||
| } | |||
|
|
|||
| var ( | |||
| max5Percent, _ = LegacyNewDecFromStr("5") // 5% max tolerance of difference in values | |||
There was a problem hiding this comment.
| max5Percent, _ = LegacyNewDecFromStr("5") // 5% max tolerance of difference in values | |
| maxTolerance, _ = LegacyNewDecFromStr("5") // 5% max tolerance of difference in values |
|
closing due to staleness |
This fuzzer performs a roundtrip comparison for square roots comparing the value from:
It's great to note that the results from this library so far from fuzzing show a higher precision/accuracy than math/big.Float.Sqrt for example with:
Updates PR #16141
Updates PR #17725
Summary by CodeRabbit
Tests:
FuzzLegacyDecApproxRootin themathpackage for fuzz testing. This function tests the square root approximation with various input values, enhancing the robustness of the math package.