From 6a46ec24ca28fb1e587ffa8f4814aacff34f4893 Mon Sep 17 00:00:00 2001 From: tamirms Date: Wed, 4 Dec 2024 10:05:34 +0100 Subject: [PATCH] exp/orderbook: Fix bug in CalculatePoolExpectation() (#5541) --- exp/orderbook/pools.go | 20 +++++++++++--- exp/orderbook/pools_test.go | 51 +++++++++++++++++++++-------------- services/horizon/CHANGELOG.md | 5 ++++ 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/exp/orderbook/pools.go b/exp/orderbook/pools.go index 6acc967588..3e78189c4e 100644 --- a/exp/orderbook/pools.go +++ b/exp/orderbook/pools.go @@ -4,6 +4,7 @@ import ( "math" "github.com/holiman/uint256" + "github.com/stellar/go/support/errors" "github.com/stellar/go/xdr" ) @@ -20,6 +21,7 @@ import ( const ( tradeTypeDeposit = iota // deposit into pool, what's the payout? tradeTypeExpectation = iota // expect payout, what to deposit? + maxBasisPoints = 10_000 ) var ( @@ -91,6 +93,9 @@ func makeTrade( // // It returns false if the calculation overflows. func CalculatePoolPayout(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int32, calculateRoundingSlippage bool) (xdr.Int64, xdr.Int64, bool) { + if feeBips < 0 || feeBips >= maxBasisPoints { + return 0, 0, false + } X, Y := uint256.NewInt(uint64(reserveA)), uint256.NewInt(uint64(reserveB)) F, x := uint256.NewInt(uint64(feeBips)), uint256.NewInt(uint64(received)) @@ -101,7 +106,7 @@ func CalculatePoolPayout(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int // We do all of the math with 4 extra decimal places of precision, so it's // all upscaled by this value. - maxBips := uint256.NewInt(10000) + maxBips := uint256.NewInt(maxBasisPoints) f := new(uint256.Int).Sub(maxBips, F) // upscaled 1 - F // right half: X + (1 - F)x @@ -153,7 +158,7 @@ func CalculatePoolPayout(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int } val := xdr.Int64(result.Uint64()) - ok = ok && result.IsUint64() && val >= 0 + ok = ok && result.IsUint64() && val > 0 return val, roundingSlippageBips, ok } @@ -166,6 +171,9 @@ func CalculatePoolPayout(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int func CalculatePoolExpectation( reserveA, reserveB, disbursed xdr.Int64, feeBips xdr.Int32, calculateRoundingSlippage bool, ) (xdr.Int64, xdr.Int64, bool) { + if feeBips < 0 || feeBips >= maxBasisPoints { + return 0, 0, false + } X, Y := uint256.NewInt(uint64(reserveA)), uint256.NewInt(uint64(reserveB)) F, y := uint256.NewInt(uint64(feeBips)), uint256.NewInt(uint64(disbursed)) @@ -176,7 +184,7 @@ func CalculatePoolExpectation( // We do all of the math with 4 extra decimal places of precision, so it's // all upscaled by this value. - maxBips := uint256.NewInt(10_000) + maxBips := uint256.NewInt(maxBasisPoints) f := new(uint256.Int).Sub(maxBips, F) // upscaled 1 - F denom := Y.Sub(Y, y).Mul(Y, f) // right half: (Y - y)(1 - F) @@ -231,7 +239,11 @@ func CalculatePoolExpectation( } val := xdr.Int64(result.Uint64()) - ok = ok && result.IsUint64() && val >= 0 + ok = ok && + result.IsUint64() && + val >= 0 && + // check that the calculated deposit would not overflow the reserve + val <= math.MaxInt64-reserveA return val, roundingSlippageBips, ok } diff --git a/exp/orderbook/pools_test.go b/exp/orderbook/pools_test.go index c75848521f..9f8b4a50f7 100644 --- a/exp/orderbook/pools_test.go +++ b/exp/orderbook/pools_test.go @@ -6,9 +6,10 @@ import ( "math/rand" "testing" - "github.com/stellar/go/xdr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/stellar/go/xdr" ) func TestLiquidityPoolExchanges(t *testing.T) { @@ -177,16 +178,19 @@ func TestLiquidityPoolMath(t *testing.T) { assertPoolExchange(t, send, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, false, 0, 0) assertPoolExchange(t, send, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, false, 0, 0) - assertPoolExchange(t, recv, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, 0, false, 0, 0) + assertPoolExchange(t, recv, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, 0, true, 0, -1) // Check with reserveB < disbursed assertPoolExchange(t, recv, math.MaxInt64, math.MaxInt64, 0, 1, 0, false, 0, 0) + // Check with calculated deposit overflows reserveA + assertPoolExchange(t, recv, 9223372036654845862, 0, 2694994506, 4515739, 30, false, 0, 0) + // Check with poolFeeBips > 10000 assertPoolExchange(t, send, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, 10001, false, 0, 0) assertPoolExchange(t, recv, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, 10010, false, 0, 0) - assertPoolExchange(t, send, 92017260901926686, 9157376027422527, 4000000000000000000, 30, 1, false, 0, 0) + assertPoolExchange(t, send, 92017260901926686, 9157376027422527, 4000000000000000000, 30, 1, true, -1, 362009430194478152) }) } @@ -206,17 +210,29 @@ func assertPoolExchange(t *testing.T, fromPool, _, ok = CalculatePoolPayout( reservesBeingDeposited, reservesBeingDisbursed, deposited, poolFeeBips, false) + fromPoolBig, _, okBig := calculatePoolPayoutBig( + reservesBeingDeposited, reservesBeingDisbursed, + deposited, poolFeeBips) + assert.Equal(t, okBig, ok) + assert.Equal(t, fromPoolBig, fromPool) case tradeTypeExpectation: toPool, _, ok = CalculatePoolExpectation( reservesBeingDeposited, reservesBeingDisbursed, disbursed, poolFeeBips, false) + toPoolBig, _, okBig := calculatePoolExpectationBig( + reservesBeingDeposited, reservesBeingDisbursed, + disbursed, poolFeeBips, + ) + assert.Equal(t, okBig, ok) + assert.Equal(t, toPoolBig, toPool) default: t.FailNow() } - if expectedReturn && assert.Equal(t, expectedReturn, ok, "wrong exchange success state") { + assert.Equal(t, expectedReturn, ok, "wrong exchange success state") + if expectedReturn { assert.EqualValues(t, expectedDisbursed, fromPool, "wrong payout") assert.EqualValues(t, expectedDeposited, toPool, "wrong expectation") } @@ -288,26 +304,15 @@ func TestCalculatePoolPayout(t *testing.T) { } func TestCalculatePoolPayoutRoundingSlippage(t *testing.T) { - t.Run("max", func(t *testing.T) { - reserveA := xdr.Int64(162020000000) - reserveB := xdr.Int64(3740000000) - received := xdr.Int64(1) - - result, roundingSlippage, ok := CalculatePoolPayout(reserveA, reserveB, received, 30, true) - require.True(t, ok) - assert.Equal(t, xdr.Int64(0), result) - assert.Equal(t, xdr.Int64(100), roundingSlippage) - }) - t.Run("big", func(t *testing.T) { reserveA := xdr.Int64(162020000000) reserveB := xdr.Int64(3740000000) - received := xdr.Int64(2) + received := xdr.Int64(50) result, roundingSlippage, ok := CalculatePoolPayout(reserveA, reserveB, received, 30, true) require.True(t, ok) - assert.Equal(t, xdr.Int64(0), result) - assert.Equal(t, xdr.Int64(100), roundingSlippage) + assert.Equal(t, xdr.Int64(1), result) + assert.Equal(t, xdr.Int64(13), roundingSlippage) }) t.Run("small", func(t *testing.T) { @@ -340,6 +345,9 @@ func TestCalculatePoolPayoutRoundingSlippage(t *testing.T) { // // It returns false if the calculation overflows. func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int32) (xdr.Int64, xdr.Int64, bool) { + if feeBips < 0 || feeBips >= maxBasisPoints { + return 0, 0, false + } X, Y := big.NewInt(int64(reserveA)), big.NewInt(int64(reserveB)) F, x := big.NewInt(int64(feeBips)), big.NewInt(int64(received)) S := new(big.Int) // Rounding Slippage @@ -380,7 +388,7 @@ func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr. i := xdr.Int64(result.Int64()) s := xdr.Int64(S.Int64()) - ok := result.IsInt64() && i >= 0 && S.IsInt64() && s >= 0 + ok := result.IsInt64() && i > 0 && S.IsInt64() && s >= 0 return i, s, ok } @@ -393,6 +401,9 @@ func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr. func calculatePoolExpectationBig( reserveA, reserveB, disbursed xdr.Int64, feeBips xdr.Int32, ) (xdr.Int64, xdr.Int64, bool) { + if feeBips < 0 || feeBips >= maxBasisPoints { + return 0, 0, false + } X, Y := big.NewInt(int64(reserveA)), big.NewInt(int64(reserveB)) F, y := big.NewInt(int64(feeBips)), big.NewInt(int64(disbursed)) S := new(big.Int) // Rounding Slippage @@ -432,6 +443,6 @@ func calculatePoolExpectationBig( i := xdr.Int64(result.Int64()) s := xdr.Int64(S.Int64()) - ok := result.IsInt64() && i >= 0 && S.IsInt64() && s >= 0 + ok := result.IsInt64() && i >= 0 && i <= math.MaxInt64-reserveA && S.IsInt64() && s >= 0 return i, s, ok } diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md index 729068b64a..0f71cbe46d 100644 --- a/services/horizon/CHANGELOG.md +++ b/services/horizon/CHANGELOG.md @@ -3,6 +3,11 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## Pending + +### Fixed +- Fix liquidity pool bug which resulted in invalid paths being included in the `/paths/strict-receive` response ([5541](https://github.com/stellar/go/pull/5541)). + ## 22.0.1 ### Fixed