Skip to content

Commit

Permalink
refactor(osmomath): limit pow iterations in osmomath (#6627)
Browse files Browse the repository at this point in the history 
* refactor(osmomath): limit pow iterations in osmomath

* refactor(osmomath): limit pow iterations in osmomath

* Update x/gamm/pool-models/balancer/pool_suite_test.go

---------

Co-authored-by: Matt, Park <[email protected]>
  • Loading branch information
@p0mvn @mattverse
p0mvn and mattverse authored Oct 6, 2023
1 parent 5caafd3 commit a1d76ab
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 16 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#6309](https://github.com/osmosis-labs/osmosis/pull/6309) Add Cosmwasm Pool Queries to Stargate Query
* [#6493](https://github.com/osmosis-labs/osmosis/pull/6493) Add PoolManager Params query to Stargate Whitelist
* [#6421](https://github.com/osmosis-labs/osmosis/pull/6421) Moves ValidatePermissionlessPoolCreationEnabled out of poolmanager module
* [#6627](https://github.com/osmosis-labs/osmosis/pull/6627) Limit pow iterations in osmomath.
* [#6586](https://github.com/osmosis-labs/osmosis/pull/6586) add auth.moduleaccounts to the stargate whitelist


## v19.2.0

### Misc Improvements
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
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.20230924192433-36cf2950dca4
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20231003135651-7418f98f1a04
github.com/osmosis-labs/osmosis/osmoutils v0.0.7-0.20230929193736-aae32321cac7
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
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3 h1:Ylmch
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3/go.mod h1:lV6KnqXYD/ayTe7310MHtM3I2q8Z6bBfMAi+bhwPYtI=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230924192433-36cf2950dca4 h1:venI3u6DjxKcQbjiCO3V8s0ww/jx+cAZRkpwLHadms8=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20230924192433-36cf2950dca4/go.mod h1:IlCTpM2uoi8SUAigc9r9kAaoz7K5H9O84u7CwaTLDdY=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20231003135651-7418f98f1a04 h1:C8LtPGkhJxfJNj7Xtok1If5yAV53O6XG3USaOJSgeg4=
github.com/osmosis-labs/osmosis/osmomath v0.0.7-0.20231003135651-7418f98f1a04/go.mod h1:jtOM+8RJMOn5e8YIaodzvO0b8kvBcHDgtCVCmWrx6wU=
github.com/osmosis-labs/osmosis/osmoutils v0.0.7-0.20230929193736-aae32321cac7 h1:E5rwhxKEt6XOIfLkoLNiqCCFNCymJjiwMYIP+0ABMKM=
github.com/osmosis-labs/osmosis/osmoutils v0.0.7-0.20230929193736-aae32321cac7/go.mod h1:YT53hlXr54D4MVKp3eoBxigiiYvy3F+h+xTZuGPW5R8=
github.com/osmosis-labs/osmosis/x/epochs v0.0.3-0.20230911120014-b14342e08daf h1:8lkIsAj3L7zxvOZbqVLNJRpSdDxaYhYfAIG7XjPaJiU=
Expand Down
14 changes: 10 additions & 4 deletions osmomath/math.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
// TODO: Analyze choice here.
var powPrecision, _ = NewDecFromStr("0.00000001")

const powIterationLimit = 150_000

var (
one_half Dec = MustNewDecFromStr("0.5")
one Dec = OneDec()
Expand Down Expand Up @@ -81,8 +83,8 @@ func Pow(base Dec, exp Dec) Dec {

// Contract: 0 < base <= 2
// 0 <= exp < 1.
func PowApprox(base Dec, exp Dec, precision Dec) Dec {
if !base.IsPositive() {
func PowApprox(originalBase Dec, exp Dec, precision Dec) Dec {
if !originalBase.IsPositive() {
panic(fmt.Errorf("base must be greater than 0"))
}

Expand All @@ -93,7 +95,7 @@ func PowApprox(base Dec, exp Dec, precision Dec) Dec {
// Common case optimization
// Optimize for it being equal to one-half
if exp.Equal(one_half) {
output, err := base.ApproxSqrt()
output, err := originalBase.ApproxSqrt()
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -129,7 +131,7 @@ func PowApprox(base Dec, exp Dec, precision Dec) Dec {
// TODO: Check with our parameterization
// TODO: If theres a bug, balancer is also wrong here :thonk:

base = base.Clone()
base := originalBase.Clone()
x, xneg := AbsDifferenceWithSign(base, one)
term := OneDec()
sum := OneDec()
Expand Down Expand Up @@ -166,6 +168,10 @@ func PowApprox(base Dec, exp Dec, precision Dec) Dec {
} else {
sum.AddMut(term)
}

if i == powIterationLimit {
panic(fmt.Errorf("failed to reach precision within %d iterations, best guess: %s for %s^%s", powIterationLimit, sum, originalBase, exp))
}
}
return sum
}
56 changes: 50 additions & 6 deletions osmomath/math_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func TestAbsDifferenceWithSign(t *testing.T) {

func TestPowApprox(t *testing.T) {
testCases := []struct {
expectPanic bool
base Dec
exp Dec
powPrecision Dec
Expand All @@ -44,10 +45,10 @@ func TestPowApprox(t *testing.T) {
},
{
// zero base, this should panic
base: ZeroDec(),
exp: OneDec(),
powPrecision: MustNewDecFromStr("0.00001"),
expectedResult: ZeroDec(),
base: ZeroDec(),
exp: OneDec(),
powPrecision: MustNewDecFromStr("0.00001"),
expectPanic: true,
},
{
// large base, small exp
Expand Down Expand Up @@ -84,17 +85,60 @@ func TestPowApprox(t *testing.T) {
powPrecision: MustNewDecFromStr("0.00000001"),
expectedResult: OneDec(),
},
{
// base close to 2

base: MustNewDecFromStr("1.999999999999999999"),
exp: SmallestDec(),
powPrecision: powPrecision,
// In Python: 1.000000000000000000693147181
expectedResult: OneDec(),
},
{
// base close to 2 and hitting iteration bound

base: MustNewDecFromStr("1.999999999999999999"),
exp: MustNewDecFromStr("0.1"),
powPrecision: powPrecision,

// In Python: 1.071773462536293164

expectPanic: true,
},
{
// base close to 2 under iteration limit

base: MustNewDecFromStr("1.99999"),
exp: MustNewDecFromStr("0.1"),
powPrecision: powPrecision,

// expectedResult: MustNewDecFromStr("1.071772926648356147"),

// In Python: 1.071772926648356147102864087

expectPanic: true,
},
{
// base close to 2 under iteration limit

base: MustNewDecFromStr("1.9999"),
exp: MustNewDecFromStr("0.1"),
powPrecision: powPrecision,

// In Python: 1.071768103548402149880477100
expectedResult: MustNewDecFromStr("1.071768103548402149"),
},
}

for i, tc := range testCases {
var actualResult Dec
ConditionalPanic(t, tc.base.IsZero(), func() {
ConditionalPanic(t, tc.expectPanic, func() {
fmt.Println(tc.base)
actualResult = PowApprox(tc.base, tc.exp, tc.powPrecision)
require.True(
t,
tc.expectedResult.Sub(actualResult).Abs().LTE(tc.powPrecision),
fmt.Sprintf("test %d failed: expected value & actual value's difference should be less than precision", i),
fmt.Sprintf("test %d failed: expected value & actual value's difference should be less than precision, expected: %s, actual: %s, precision: %s", i, tc.expectedResult, actualResult, tc.powPrecision),
)
})
}
Expand Down
8 changes: 7 additions & 1 deletion osmomath/pow_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,13 @@ func BenchmarkSlowPowCases(b *testing.B) {
exp Dec
}{
{
base: MustNewDecFromStr("1.99999999999999"),
// Original worst case:
// 1.99999999999999
// 0.1
//
// The values below are hand-picked to be close to the worst case
// while under the iteration bound.
base: MustNewDecFromStr("1.9999"),
exp: MustNewDecFromStr("0.1"),
},
}
Expand Down
39 changes: 36 additions & 3 deletions x/gamm/pool-models/balancer/pool_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,36 @@ var calcSingleAssetJoinTestCases = []calcJoinSharesTestCase{
tokensIn: sdk.NewCoins(sdk.NewInt64Coin("uosmo", 156_736/4*3)),
expectShares: osmomath.NewIntFromUint64(9_644_655_900_000_000_000),
},
{
// Expected output from Balancer paper (https://balancer.fi/whitepaper.pdf) using equation (25) on page 10:
// P_issued = P_supply * ((1 + (A_t / B_t))^W_t - 1)
//
// 6_504_012_121_638_943_579 = 1e20 * (( 1 + (499_990 / 500_000))^0.09 - 1)
//
// where:
// P_supply = initial pool supply = 1e20
// A_t = amount of deposited asset = 195920
// B_t = existing balance of deposited asset in the pool prior to deposit = 156_736
// W_t = normalized weight of deposited asset in pool = 100 / (100 + 1000) approx = 0.09
// Plugging all of this in, we get:
// Full solution: https://www.wolframalpha.com/input?i=100+*10%5E18*%28%281+%2B++%28499990*%281+-+%281-%28100+%2F+%28100++%2B+1000%29%29%29+*+0%29%2F500000%29%29%5E%28100+%2F+%28100+%2B++1000%29%29+-+1%29
// Simplified: P_issued = 6_504_012_121_638_943_579
name: "single asset - (almost 1 == tokenIn / liquidity ratio), token in weight is smaller than the other token, with zero spread factor",
spreadFactor: osmomath.MustNewDecFromStr("0"),
poolAssets: []balancer.PoolAsset{
{
Token: sdk.NewInt64Coin("uosmo", 500_000),
Weight: osmomath.NewInt(100),
},
{
Token: sdk.NewInt64Coin("uatom", 1e12),
Weight: osmomath.NewInt(1000),
},
},
// Increasing this by 1 would cause a panic due to pow iteration limit reached
tokensIn: sdk.NewCoins(sdk.NewInt64Coin("uosmo", 499_990)),
expectShares: osmomath.NewIntFromUint64(6_504_012_121_638_943_579),
},
{
// Expected output from Balancer paper (https://balancer.fi/whitepaper.pdf) using equation (25) on page 10:
// P_issued = P_supply * ((1 + (A_t / B_t))^W_t - 1)
Expand All @@ -339,7 +369,8 @@ var calcSingleAssetJoinTestCases = []calcJoinSharesTestCase{
// Plugging all of this in, we get:
// Full solution: https://www.wolframalpha.com/input?i=100+*10%5E18*%28%281+%2B+%28499999*%281+-+%281-%28100+%2F+%28100+%2B+1000%29%29%29+*+0%29%2F500000%29%29%5E%28100+%2F+%28100+%2B+1000%29%29+-+1%29
// Simplified: P_issued = 6_504_099_261_800_144_638
name: "single asset - (almost 1 == tokenIn / liquidity ratio), token in weight is smaller than the other token, with zero spread factor",
// Note: this test was converted from a panic to being valid after adding iteration limit in Pow()
name: "panic (pow iteration limit) single asset - (almost 1 == tokenIn / liquidity ratio), token in weight is smaller than the other token, with zero spread factor",
spreadFactor: osmomath.MustNewDecFromStr("0"),
poolAssets: []balancer.PoolAsset{
{
Expand All @@ -351,8 +382,10 @@ var calcSingleAssetJoinTestCases = []calcJoinSharesTestCase{
Weight: osmomath.NewInt(1000),
},
},
tokensIn: sdk.NewCoins(sdk.NewInt64Coin("uosmo", 499_999)),
expectShares: osmomath.NewIntFromUint64(6_504_099_261_800_144_638),
tokensIn: sdk.NewCoins(sdk.NewInt64Coin("uosmo", 499_999)),

// pow iteration limit reached
expectPanic: true,
},
{
// Currently, our Pow approximation function does not work correctly when one tries
Expand Down

0 comments on commit a1d76ab

Please sign in to comment.