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.

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

services/horizon: Exclude trades with >10% rounding slippage from trade aggregations #4178

Merged
merged 28 commits into from
Feb 21, 2022
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
083ee7f
Exclude trades with >10% rounding_slippage from trade aggregations
Jan 10, 2022
38a0283
Fixing tests
Jan 12, 2022
578e62b
Add db.NullRat for null bigrats in the db
Jan 13, 2022
f97371c
Add integration tests for rounding slippage filtering in trade aggs
Jan 13, 2022
ff0c409
Add --rounding-slippage-filter flag
Jan 13, 2022
074a4c0
Fix typo
Jan 13, 2022
32b9074
Review feedback
Jan 14, 2022
98fffa3
note div-by-zero
Jan 19, 2022
aefa014
Track the base and counter reserves on each trade so we can sanity-ch…
Jan 20, 2022
96da0d3
Review feedback
Jan 21, 2022
6860cac
re-use orderbook package to calculate rounding slippage
Jan 21, 2022
780642b
Clean up test a bit
Jan 21, 2022
151818e
Missed a few test updates
Jan 21, 2022
ec2eeca
Clarify terminology around slippage to fix calculations
Jan 24, 2022
2955409
Add an extra column we might need in order to calculate rounding slip…
Jan 25, 2022
bdb31e0
Disable roundingSlippage calc for /paths
Jan 25, 2022
5f749a9
OPTIMIZE ALLOCATIONS :robot-face:
Jan 25, 2022
59586d2
Workaround for https://github.com/stellar/go/issues/4203
Feb 9, 2022
12f1bd4
Merge remote-tracking branch 'origin/master' into 4160/exclude_dust_t…
Feb 9, 2022
714b391
Merge remote-tracking branch 'origin/master' into 4160/exclude_dust_t…
Feb 17, 2022
affa952
Add changelog entry
Feb 17, 2022
94a17cf
RoundingSlippage should be in bips not megabips
Feb 17, 2022
aeca2e7
remove redundant if clauses
Feb 17, 2022
8a6c7b1
Fixing test
Feb 17, 2022
93ada08
Fix flag description
Feb 17, 2022
d9086c8
remove trade reserves tracking for debugging
Feb 17, 2022
d081882
Add some comments to clarify slippage calculations
Feb 18, 2022
1dfa41b
Merge branch 'master' into 4160/exclude_dust_trades_from_aggregations
Feb 18, 2022
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
73 changes: 55 additions & 18 deletions exp/orderbook/pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ func makeTrade(
var result xdr.Int64
switch tradeType {
case tradeTypeDeposit:
result, ok = calculatePoolPayout(X, Y, amount, details.Params.Fee)
result, _, ok = CalculatePoolPayout(X, Y, amount, details.Params.Fee, false)

case tradeTypeExpectation:
result, ok = calculatePoolExpectation(X, Y, amount, details.Params.Fee)
result, _, ok = CalculatePoolExpectation(X, Y, amount, details.Params.Fee, false)

default:
return 0, errBadTradeType
Expand All @@ -84,65 +84,85 @@ func makeTrade(
return result, nil
}

// calculatePoolPayout calculates the amount of `reserveB` disbursed from the
// CalculatePoolPayout calculates the amount of `reserveB` disbursed from the
// pool for a `received` amount of `reserveA` . From CAP-38:
//
// y = floor[(1 - F) Yx / (X + x - Fx)]
//
// It returns false if the calculation overflows.
func calculatePoolPayout(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int32) (xdr.Int64, bool) {
func CalculatePoolPayout(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int32, calculateRoundingSlippage bool) (xdr.Int64, xdr.Int64, bool) {
paulbellamy marked this conversation as resolved.
Show resolved Hide resolved
X, Y := uint256.NewInt(uint64(reserveA)), uint256.NewInt(uint64(reserveB))
F, x := uint256.NewInt(uint64(feeBips)), uint256.NewInt(uint64(received))

// would this deposit overflow the reserve?
if received > math.MaxInt64-reserveA {
return 0, false
return 0, 0, false
}

// We do all of the math in bips, so it's all upscaled by this value.
// 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)
f := new(uint256.Int).Sub(maxBips, F) // upscaled 1 - F

// right half: X + (1 - F)x
denom := X.Mul(X, maxBips).Add(X, new(uint256.Int).Mul(x, f))
if denom.IsZero() { // avoid div-by-zero panic
return 0, false
return 0, 0, false
}

// left half, a: (1 - F) Yx
numer := Y.Mul(Y, x).Mul(Y, f)

// divide & check overflow
result := numer.Div(numer, denom)
result := new(uint256.Int)
result.Div(numer, denom)

var roundingSlippageBips xdr.Int64
ok := true
if calculateRoundingSlippage && !new(uint256.Int).Mod(numer, denom).IsZero() {
S := new(uint256.Int) // Rounding Slippage in bips
// Recalculate with more precision
unrounded, rounded := new(uint256.Int), new(uint256.Int)
unrounded.Mul(numer, maxBips).Div(unrounded, denom)
rounded.Mul(result, maxBips)
S.Sub(unrounded, rounded)
S.Abs(S).Mul(S, maxBips)
S.Div(S, unrounded)
S.Div(S, uint256.NewInt(100)) // Take off the excess 2 decimal places
roundingSlippageBips = xdr.Int64(S.Uint64())
ok = ok && S.IsUint64() && roundingSlippageBips >= 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: looks like ok is always true before this line.

Suggested change
ok = ok && S.IsUint64() && roundingSlippageBips >= 0
ok = S.IsUint64() && roundingSlippageBips >= 0

}
paulbellamy marked this conversation as resolved.
Show resolved Hide resolved

val := xdr.Int64(result.Uint64())
return val, result.IsUint64() && val >= 0
ok = ok && result.IsUint64() && val >= 0
2opremio marked this conversation as resolved.
Show resolved Hide resolved
return val, roundingSlippageBips, ok
}

// calculatePoolExpectation determines how much of `reserveA` you would need to
// CalculatePoolExpectation determines how much of `reserveA` you would need to
// put into a pool to get the `disbursed` amount of `reserveB`.
//
// x = ceil[Xy / ((Y - y)(1 - F))]
//
// It returns false if the calculation overflows.
func calculatePoolExpectation(
reserveA, reserveB, disbursed xdr.Int64, feeBips xdr.Int32,
) (xdr.Int64, bool) {
func CalculatePoolExpectation(
reserveA, reserveB, disbursed xdr.Int64, feeBips xdr.Int32, calculateRoundingSlippage bool,
2opremio marked this conversation as resolved.
Show resolved Hide resolved
) (xdr.Int64, xdr.Int64, bool) {
X, Y := uint256.NewInt(uint64(reserveA)), uint256.NewInt(uint64(reserveB))
F, y := uint256.NewInt(uint64(feeBips)), uint256.NewInt(uint64(disbursed))

// sanity check: disbursing shouldn't underflow the reserve
if disbursed >= reserveB {
return 0, false
return 0, 0, false
}

// We do all of the math in bips, so it's all upscaled by this value.
maxBips := uint256.NewInt(10000)
// 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)
f := new(uint256.Int).Sub(maxBips, F) // upscaled 1 - F

denom := Y.Sub(Y, y).Mul(Y, f) // right half: (Y - y)(1 - F)
if denom.IsZero() { // avoid div-by-zero panic
return 0, false
return 0, 0, false
}

numer := X.Mul(X, y).Mul(X, maxBips) // left half: Xy
Expand All @@ -152,12 +172,29 @@ func calculatePoolExpectation(
rem.Mod(numer, denom)

// hacky way to ceil(): if there's a remainder, add 1
var roundingSlippageBips xdr.Int64
ok := true
if !rem.IsZero() {
result.AddUint64(result, 1)

if calculateRoundingSlippage {
S := new(uint256.Int) // Rounding Slippage in bips
// Recalculate with more precision
unrounded, rounded := new(uint256.Int), new(uint256.Int)
unrounded.Mul(numer, maxBips).Div(unrounded, denom)
rounded.Mul(result, maxBips)
S.Sub(unrounded, rounded)
S.Abs(S).Mul(S, maxBips)
S.Div(S, unrounded)
S.Div(S, uint256.NewInt(100)) // Take off the excess 2 decimal places
roundingSlippageBips = xdr.Int64(S.Uint64())
ok = ok && S.IsUint64() && roundingSlippageBips >= 0
paulbellamy marked this conversation as resolved.
Show resolved Hide resolved
}
}

val := xdr.Int64(result.Uint64())
return val, result.IsUint64() && val >= 0
ok = ok && result.IsUint64() && val >= 0
return val, roundingSlippageBips, ok
}

// getOtherAsset returns the other asset in the liquidity pool. Note that
Expand Down
151 changes: 132 additions & 19 deletions exp/orderbook/pools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/stellar/go/xdr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestLiquidityPoolExchanges(t *testing.T) {
Expand Down Expand Up @@ -171,7 +172,7 @@ func TestLiquidityPoolMath(t *testing.T) {

t.Run("Potential Internal Overflow", func(t *testing.T) {

// Test for internal uint128 underflow/overflow in calculatePoolPayout() and calculatePoolExpectation() by providing
// Test for internal uint128 underflow/overflow in CalculatePoolPayout() and CalculatePoolExpectation() by providing
// input values which cause the maximum internal calculations

assertPoolExchange(t, send, math.MaxInt64, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, false, 0, 0)
Expand Down Expand Up @@ -202,14 +203,14 @@ func assertPoolExchange(t *testing.T,

switch exchangeType {
case tradeTypeDeposit:
fromPool, ok = calculatePoolPayout(
fromPool, _, ok = CalculatePoolPayout(
reservesBeingDeposited, reservesBeingDisbursed,
deposited, poolFeeBips)
deposited, poolFeeBips, false)

case tradeTypeExpectation:
toPool, ok = calculatePoolExpectation(
toPool, _, ok = CalculatePoolExpectation(
reservesBeingDeposited, reservesBeingDisbursed,
disbursed, poolFeeBips)
disbursed, poolFeeBips, false)

default:
t.FailNow()
Expand All @@ -227,41 +228,125 @@ func TestCalculatePoolExpectations(t *testing.T) {
reserveB := xdr.Int64(rand.Int63())
disbursed := xdr.Int64(rand.Int63())

result, ok := calculatePoolExpectationBig(reserveA, reserveB, disbursed, 30)
result1, ok1 := calculatePoolExpectation(reserveA, reserveB, disbursed, 30)
result, roundingSlippage, ok := calculatePoolExpectationBig(reserveA, reserveB, disbursed, 30)
result1, roundingSlippage1, ok1 := CalculatePoolExpectation(reserveA, reserveB, disbursed, 30, true)
if assert.Equal(t, ok, ok1) {
assert.Equal(t, result, result1)
assert.Equal(t, roundingSlippage, roundingSlippage1)
}
}
}

func TestCalculatePoolExpectationsRoundingSlippage(t *testing.T) {
t.Run("big", func(t *testing.T) {
reserveA := xdr.Int64(3740000000)
reserveB := xdr.Int64(162020000000)
disbursed := xdr.Int64(1)

result, roundingSlippage, ok := CalculatePoolExpectation(reserveA, reserveB, disbursed, 30, true)
require.True(t, ok)
assert.Equal(t, xdr.Int64(1), result)
assert.Equal(t, xdr.Int64(4229), roundingSlippage)
})

t.Run("small", func(t *testing.T) {
reserveA := xdr.Int64(200)
reserveB := xdr.Int64(400)
disbursed := xdr.Int64(20)

result, roundingSlippage, ok := CalculatePoolExpectation(reserveA, reserveB, disbursed, 30, true)
require.True(t, ok)
assert.Equal(t, xdr.Int64(11), result)
assert.Equal(t, xdr.Int64(4), roundingSlippage)
})

t.Run("very small", func(t *testing.T) {
reserveA := xdr.Int64(200)
reserveB := xdr.Int64(400)
disbursed := xdr.Int64(50)

result, roundingSlippage, ok := CalculatePoolExpectation(reserveA, reserveB, disbursed, 30, true)
require.True(t, ok)
assert.Equal(t, xdr.Int64(29), result)
assert.Equal(t, xdr.Int64(1), roundingSlippage)
})
}

func TestCalculatePoolPayout(t *testing.T) {
for i := 0; i < 1000000; i++ {
reserveA := xdr.Int64(rand.Int63())
reserveB := xdr.Int64(rand.Int63())
received := xdr.Int64(rand.Int63())

result, ok := calculatePoolPayoutBig(reserveA, reserveB, received, 30)
result1, ok1 := calculatePoolPayout(reserveA, reserveB, received, 30)
result, roundingSlippage, ok := calculatePoolPayoutBig(reserveA, reserveB, received, 30)
result1, roundingSlippage1, ok1 := CalculatePoolPayout(reserveA, reserveB, received, 30, true)
if assert.Equal(t, ok, ok1) {
assert.Equal(t, result, result1)
assert.Equal(t, roundingSlippage, roundingSlippage1)
}
}
}

// calculatePoolPayout calculates the amount of `reserveB` disbursed from the
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)

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("small", func(t *testing.T) {
reserveA := xdr.Int64(200)
reserveB := xdr.Int64(300)
received := xdr.Int64(1)

result, roundingSlippage, ok := CalculatePoolPayout(reserveA, reserveB, received, 30, true)
require.True(t, ok)
assert.Equal(t, xdr.Int64(1), result)
assert.Equal(t, xdr.Int64(32), roundingSlippage)
})

t.Run("very small", func(t *testing.T) {
reserveA := xdr.Int64(200)
reserveB := xdr.Int64(210)
received := xdr.Int64(1)

result, roundingSlippage, ok := CalculatePoolPayout(reserveA, reserveB, received, 30, true)
require.True(t, ok)
assert.Equal(t, xdr.Int64(1), result)
assert.Equal(t, xdr.Int64(3), roundingSlippage)
})
}

// CalculatePoolPayout calculates the amount of `reserveB` disbursed from the
// pool for a `received` amount of `reserveA` . From CAP-38:
//
// y = floor[(1 - F) Yx / (X + x - Fx)]
//
// It returns false if the calculation overflows.
func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int32) (xdr.Int64, bool) {
func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr.Int32) (xdr.Int64, xdr.Int64, bool) {
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

// would this deposit overflow the reserve?
if received > math.MaxInt64-reserveA {
return 0, false
return 0, 0, false
}

// We do all of the math in bips, so it's all upscaled by this value.
Expand All @@ -271,17 +356,32 @@ func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr.
// right half: X + (1 - F)x
denom := X.Mul(X, maxBips).Add(X, new(big.Int).Mul(x, f))
if denom.Cmp(big.NewInt(0)) == 0 { // avoid div-by-zero panic
return 0, false
return 0, 0, false
}

// left half, a: (1 - F) Yx
numer := Y.Mul(Y, x).Mul(Y, f)

// divide & check overflow
result := numer.Div(numer, denom)
result, rem := new(big.Int), new(big.Int)
result.Div(numer, denom)
rem.Mod(numer, denom)

if rem.Cmp(big.NewInt(0)) > 0 {
// Recalculate with more precision
unrounded, rounded := new(big.Int), new(big.Int)
unrounded.Mul(numer, maxBips).Div(unrounded, denom)
rounded.Mul(result, maxBips)
S.Sub(unrounded, rounded)
S.Abs(S).Mul(S, maxBips)
S.Div(S, unrounded)
S.Div(S, big.NewInt(100))
}
Shaptic marked this conversation as resolved.
Show resolved Hide resolved

i := xdr.Int64(result.Int64())
return i, result.IsInt64() && i >= 0
s := xdr.Int64(S.Int64())
ok := result.IsInt64() && i >= 0 && S.IsInt64() && s >= 0
return i, s, ok
}

// calculatePoolExpectation determines how much of `reserveA` you would need to
Expand All @@ -292,13 +392,14 @@ func calculatePoolPayoutBig(reserveA, reserveB, received xdr.Int64, feeBips xdr.
// It returns false if the calculation overflows.
func calculatePoolExpectationBig(
reserveA, reserveB, disbursed xdr.Int64, feeBips xdr.Int32,
) (xdr.Int64, bool) {
) (xdr.Int64, xdr.Int64, bool) {
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

// sanity check: disbursing shouldn't underflow the reserve
if disbursed >= reserveB {
return 0, false
return 0, 0, false
}

// We do all of the math in bips, so it's all upscaled by this value.
Expand All @@ -307,7 +408,7 @@ func calculatePoolExpectationBig(

denom := Y.Sub(Y, y).Mul(Y, f) // right half: (Y - y)(1 - F)
if denom.Cmp(big.NewInt(0)) == 0 { // avoid div-by-zero panic
return 0, false
return 0, 0, false
}

numer := X.Mul(X, y).Mul(X, maxBips) // left half: Xy
Expand All @@ -318,7 +419,19 @@ func calculatePoolExpectationBig(
// hacky way to ceil(): if there's a remainder, add 1
if rem.Cmp(big.NewInt(0)) > 0 {
result.Add(result, big.NewInt(1))

// Recalculate with more precision
paulbellamy marked this conversation as resolved.
Show resolved Hide resolved
unrounded, rounded := new(big.Int), new(big.Int)
unrounded.Mul(numer, maxBips).Div(unrounded, denom)
rounded.Mul(result, maxBips)
S.Sub(unrounded, rounded)
S.Abs(S).Mul(S, maxBips)
S.Div(S, unrounded)
S.Div(S, big.NewInt(100))
}

return xdr.Int64(result.Int64()), result.IsInt64()
i := xdr.Int64(result.Int64())
s := xdr.Int64(S.Int64())
ok := result.IsInt64() && i >= 0 && S.IsInt64() && s >= 0
return i, s, ok
}
Loading