From 267cd15b0717b1a65fa3299f406aacaa9a479943 Mon Sep 17 00:00:00 2001 From: Roman Date: Mon, 18 Sep 2023 09:55:51 -0400 Subject: [PATCH] fix(math): revert to correct version of ApproxRoot from a former state breaking change (#17725) --- math/CHANGELOG.md | 6 ++++++ math/dec.go | 19 ++++--------------- math/dec_test.go | 1 + 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/math/CHANGELOG.md b/math/CHANGELOG.md index 4dadc5f0d7..cdf45d6d76 100644 --- a/math/CHANGELOG.md +++ b/math/CHANGELOG.md @@ -34,6 +34,12 @@ Ref: https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.j # Changelog +## [Unreleased] + +### Bug Fixes + +* [#17725](https://github.com/cosmos/cosmos-sdk/pull/17725) Fix state break in ApproxRoot. This has been present since math/v1.0.1. It changed the rounding behavior at precision end in an intermediary division from banker's to truncation. The truncation occurs from binary right shift in the case of square roots. The change is now reverted back to banker's rounding universally for any root. + ## [math/v1.1.2](https://github.com/cosmos/cosmos-sdk/releases/tag/math/v1.1.2) - 2023-08-21 ### Bug Fixes diff --git a/math/dec.go b/math/dec.go index 8833a14a03..794a93b4e3 100644 --- a/math/dec.go +++ b/math/dec.go @@ -42,6 +42,7 @@ var ( zeroInt = big.NewInt(0) oneInt = big.NewInt(1) tenInt = big.NewInt(10) + smallestDec = LegacySmallestDec() ) // Decimal errors @@ -470,27 +471,15 @@ func (d LegacyDec) ApproxRoot(root uint64) (guess LegacyDec, err error) { } guess, delta := scratchOneDec, LegacyOneDec() - smallestDec := LegacySmallestDec() - for iter := 0; delta.AbsMut().GT(smallestDec) && iter < maxApproxRootIterations; iter++ { - // Set prev = guess^{root - 1}, with an optimization for sqrt - // where root=2 => prev = guess. (And thus no extra heap allocations) - prev := guess - if root != 2 { - prev = guess.Power(root - 1) - } + for iter := 0; iter < maxApproxRootIterations && delta.Abs().GT(smallestDec); iter++ { + prev := guess.Power(root - 1) if prev.IsZero() { prev = smallestDec } delta.Set(d).QuoMut(prev) delta.SubMut(guess) - // delta = delta / root. - // We optimize for sqrt, where root=2 => delta = delta >> 1 - if root == 2 { - delta.i.Rsh(delta.i, 1) - } else { - delta.QuoInt64Mut(int64(root)) - } + delta.QuoInt64Mut(int64(root)) guess.AddMut(delta) } diff --git a/math/dec_test.go b/math/dec_test.go index 7bbe0b9ada..85ff732c52 100644 --- a/math/dec_test.go +++ b/math/dec_test.go @@ -480,6 +480,7 @@ func (s *decimalTestSuite) TestApproxSqrt() { math.LegacyNewDec(2).Power(127).Sub(math.LegacyOneDec()), math.LegacyMustNewDecFromStr("13043817825332782212.349571806252508369"), }, + {math.LegacyMustNewDecFromStr("1.000000011823380862"), math.LegacyMustNewDecFromStr("1.000000005911690414")}, } for i, tc := range testCases {