Skip to content
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.

Sign up for GitHub

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

Jump to bottom

osmomath: Mutative version for QuoRoundUp #6437

Merged
merged 9 commits into from
Sep 20, 2023
Merged

osmomath: Mutative version for QuoRoundUp #6437

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ce6da1b
mutative version for QuoRoundUp
hieuvubk Sep 19, 2023
1dc7237
change log & go mod
hieuvubk Sep 19, 2023
ba81890
actual mutative input
hieuvubk Sep 19, 2023
5f7fbd6
testing
hieuvubk Sep 19, 2023
9b4f216
no reuse global var zeroInt
hieuvubk Sep 19, 2023
792639e
update go.mod
hieuvubk Sep 19, 2023
0622c30
reduce duplication and cover non-mutative with a test
p0mvn Sep 20, 2023
28712c6
Merge branch 'main' into hieu/quoroundup_mut
p0mvn Sep 20, 2023
69097fe
update go.mod
hieuvubk Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Features

* [#6427](https://github.com/osmosis-labs/osmosis/pull/6427) sdk.Coins Mul and Quo helpers in osmoutils
* [#6437](https://github.com/osmosis-labs/osmosis/pull/6437) mutative version for QuoRoundUp

### Misc Improvements

Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
github.com/mattn/go-sqlite3 v1.14.17
github.com/ory/dockertest/v3 v3.10.0
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230918184012-da92c9cdf6bd
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230919185310-9b4f21671e55
github.com/osmosis-labs/osmosis/osmoutils v0.0.7-0.20230911120014-b14342e08daf
github.com/osmosis-labs/osmosis/x/epochs v0.0.3-0.20230911120014-b14342e08daf
github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.9-0.20230911120014-b14342e08daf
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,10 @@ github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230911120014-b14342e08daf h1
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230911120014-b14342e08daf/go.mod h1:pIItelRYvonB+H8A6KqucOi7xFnJxzDHaWJqOdFNLI4=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230918184012-da92c9cdf6bd h1:U7r0uBLTWeLrgGOu1re0aTl10yreX1j3dNDu12KqBpE=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230918184012-da92c9cdf6bd/go.mod h1:pIItelRYvonB+H8A6KqucOi7xFnJxzDHaWJqOdFNLI4=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230919084539-ce6da1bda6d1 h1:2I8Lp4pGa9lfjKWAnglNf5Oz9qRKnjXC1Kjxcd+60jw=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230919084539-ce6da1bda6d1/go.mod h1:pIItelRYvonB+H8A6KqucOi7xFnJxzDHaWJqOdFNLI4=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230919185310-9b4f21671e55 h1:Q5ietH67MLVuFF7bAnCnxb1uVBmcIxPJeiPGv2Jvzok=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230919185310-9b4f21671e55/go.mod h1:pIItelRYvonB+H8A6KqucOi7xFnJxzDHaWJqOdFNLI4=
github.com/osmosis-labs/osmosis/osmoutils v0.0.7-0.20230911120014-b14342e08daf h1:r5R/L3tzH+vGPahAdvnVB2Vo0KPhZR0oMNyX4+G2FEo=
github.com/osmosis-labs/osmosis/osmoutils v0.0.7-0.20230911120014-b14342e08daf/go.mod h1:7VoXHwrSSx8Sii0UFc9YIixF6C/9XfV1pdU2Dliu4WA=
github.com/osmosis-labs/osmosis/x/epochs v0.0.3-0.20230911120014-b14342e08daf h1:8lkIsAj3L7zxvOZbqVLNJRpSdDxaYhYfAIG7XjPaJiU=
Expand Down
44 changes: 44 additions & 0 deletions osmomath/decimal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const (

var (
precisionReuse = new(big.Int).Exp(big.NewInt(10), big.NewInt(PrecisionBigDec), nil)
squaredPrecisionReuse = new(big.Int).Mul(precisionReuse, precisionReuse)
precisionReuseSDK = new(big.Int).Exp(big.NewInt(10), big.NewInt(PrecisionDec), nil)
fivePrecision = new(big.Int).Quo(precisionReuse, big.NewInt(2))
precisionMultipliers []*big.Int
Expand Down Expand Up @@ -401,6 +402,20 @@ func (d BigDec) QuoRoundUp(d2 BigDec) BigDec {
return BigDec{chopped}
}

// quotient, round up (mutative)
func (d BigDec) QuoRoundUpMut(d2 BigDec) BigDec {
hieuvubk marked this conversation as resolved.
Show resolved Hide resolved
// multiply precision twice
d.i.Mul(d.i, squaredPrecisionReuse)
d.i.Quo(d.i, d2.i)

chopPrecisionAndRoundUpMut(d.i, precisionReuse)

if d.i.BitLen() > maxDecBitLen {
panic("Int overflow")
}
return BigDec{d.i}
}

// quotient
func (d BigDec) QuoInt(i BigInt) BigDec {
mul := new(big.Int).Quo(d.i, i.i)
Expand Down Expand Up @@ -680,6 +695,29 @@ func chopPrecisionAndRoundUp(d *big.Int, precisionReuse *big.Int) *big.Int {
return quo.Add(quo, oneInt)
}

// chopPrecisionAndRoundUp removes a Precision amount of rightmost digits and rounds up.
// mutative input
func chopPrecisionAndRoundUpMut(d *big.Int, precisionReuse *big.Int) *big.Int {
// remove the negative and add it back when returning
if d.Sign() == -1 {
// make d positive, compute chopped value, and then un-mutate d
d = d.Neg(d)
// truncate since d is negative...
d = chopPrecisionAndTruncateMut(d)
d = d.Neg(d)
return d
}

// get the truncated quotient and remainder
_, rem := d.QuoRem(d, precisionReuse, big.NewInt(0))

if rem.Sign() == 0 { // remainder is zero
return d
}

return d.Add(d, oneInt)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this differ from chopPrecisionAndRoundUp? I'm not seeing any difference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using chopPrecisionAndTruncateMut that uses d as result. Also use d as QuoRem res instead of declaring new quo variable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the context. I've made commits directly to the branch with changes that were missing from approval.

What was added:

  • remove the non-mutative chop version since we can now use the mutative one + copy the input
  • covered non-mutative QuoRoundUp with a test

Commit: 0622c30

func chopPrecisionAndRoundNonMutative(d *big.Int) *big.Int {
tmp := new(big.Int).Set(d)
return chopPrecisionAndRound(tmp)
Expand All @@ -705,6 +743,12 @@ func chopPrecisionAndTruncate(d *big.Int) *big.Int {
return new(big.Int).Quo(d, precisionReuse)
}

// chopPrecisionAndTruncate is similar to chopPrecisionAndRound,
// but always rounds down. It mutates the input.
func chopPrecisionAndTruncateMut(d *big.Int) *big.Int {
return d.Quo(d, precisionReuse)
}

// TruncateInt64 truncates the decimals from the number and returns an int64
func (d BigDec) TruncateInt64() int64 {
chopped := chopPrecisionAndTruncate(d.i)
Expand Down
47 changes: 47 additions & 0 deletions osmomath/decimal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,3 +1481,50 @@ func (s *decimalTestSuite) TestPower() {
})
}
}

func (s *decimalTestSuite) TestQuoRoundUpMut() {
tests := []struct {
d1, d2, expQuoRoundUpMut osmomath.BigDec
}{
{osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0)},
{osmomath.NewBigDec(1), osmomath.NewBigDec(0), osmomath.NewBigDec(0)},
{osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(0)},
{osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(0)},
{osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(0)},

{osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1)},
{osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(1)},
{osmomath.NewBigDec(1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1)},
{osmomath.NewBigDec(-1), osmomath.NewBigDec(1), osmomath.NewBigDec(-1)},

{
osmomath.NewBigDec(3), osmomath.NewBigDec(7), osmomath.MustNewBigDecFromStr("0.428571428571428571428571428571428572"),
},
{
osmomath.NewBigDec(2), osmomath.NewBigDec(4), osmomath.NewBigDecWithPrec(5, 1),
},

{osmomath.NewBigDec(100), osmomath.NewBigDec(100), osmomath.NewBigDec(1)},

{
osmomath.NewBigDecWithPrec(15, 1), osmomath.NewBigDecWithPrec(15, 1), osmomath.NewBigDec(1),
},
{
osmomath.NewBigDecWithPrec(3333, 4), osmomath.NewBigDecWithPrec(333, 4), osmomath.MustNewBigDecFromStr("10.009009009009009009009009009009009010"),
},
}

for tcIndex, tc := range tests {
tc := tc

if tc.d2.IsZero() { // panic for divide by zero
s.Require().Panics(func() { tc.d1.QuoRoundUpMut(tc.d2) })
} else {
tc.d1.QuoRoundUpMut(tc.d2)

// Make sure d1 equals to expected
s.Require().True(tc.expQuoRoundUpMut.Equal(tc.d1), "exp %v, res %v, tc %d", tc.expQuoRoundUpMut.String(), tc.d1.String(), tcIndex)

}
}
}
2 changes: 1 addition & 1 deletion x/concentrated-liquidity/math/math.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func CalcAmount0Delta(liq, sqrtPriceA, sqrtPriceB osmomath.BigDec, roundUp bool)
// Note that the order of divisions is important here. First, we divide by a larger number (sqrtPriceB) and then by a smaller number (sqrtPriceA).
// This leads to a smaller error amplification. This only matters in cases where at least one of the sqrt prices is below 1.
// TODO (perf): QuoRoundUpMut with no reallocation.
return liq.MulRoundUp(diff).QuoRoundUp(sqrtPriceB).QuoRoundUp(sqrtPriceA).Ceil()
return liq.MulRoundUp(diff).QuoRoundUpMut(sqrtPriceB).QuoRoundUpMut(sqrtPriceA).Ceil()
}
// These are truncated at precision end to round in favor of the pool when:
// - calculating amount out during swap
Expand Down