diff --git a/exp/orderbook/edges.go b/exp/orderbook/edges.go index b88c117d13..6e6de66f7c 100644 --- a/exp/orderbook/edges.go +++ b/exp/orderbook/edges.go @@ -22,14 +22,7 @@ func (e edgeSet) add(key string, offer xdr.OfferEntry) { // find the smallest i such that Price of offers[i] > Price of offer insertIndex := sort.Search(len(offers), func(i int) bool { - // Price of offers[i] > Price of offer - // <==> - // (offers[i].Price.N / offers[i].Price.D) > (offer.Price.N / offer.Price.D) - // <==> - // (offers[i].Price.N / offers[i].Price.D) * (offers[i].Price.D * offer.Price.D) > (offer.Price.N / offer.Price.D) * (offers[i].Price.D * offer.Price.D) - // <==> - // offers[i].Price.N * offer.Price.D > offer.Price.N * offers[i].Price.D - return uint64(offers[i].Price.N)*uint64(offer.Price.D) > uint64(offer.Price.N)*uint64(offers[i].Price.D) + return offer.Price.Cheaper(offers[i].Price) }) offers = append(offers, offer) diff --git a/exp/orderbook/graph.go b/exp/orderbook/graph.go index e9503f4bf0..7e3afe7480 100644 --- a/exp/orderbook/graph.go +++ b/exp/orderbook/graph.go @@ -177,7 +177,8 @@ func (graph *OrderBookGraph) findOffers( } for _, offer := range offers { - if len(results) == 0 || results[len(results)-1].Price != offer.Price { + // Offers are sorted by price, so, equal prices will always be contiguous. + if len(results) == 0 || !results[len(results)-1].Price.Equal(offer.Price) { maxPriceLevels-- } if maxPriceLevels < 0 { diff --git a/exp/orderbook/graph_test.go b/exp/orderbook/graph_test.go index 396891f04f..09b975f819 100644 --- a/exp/orderbook/graph_test.go +++ b/exp/orderbook/graph_test.go @@ -402,7 +402,7 @@ func TestAddOfferOrderBook(t *testing.T) { expectedGraph := &OrderBookGraph{ edgesForSellingAsset: map[string]edgeSet{ - nativeAsset.String(): edgeSet{ + nativeAsset.String(): { usdAsset.String(): []xdr.OfferEntry{ quarterOffer, fiftyCentsOffer, @@ -414,52 +414,52 @@ func TestAddOfferOrderBook(t *testing.T) { threeEurOffer, }, }, - usdAsset.String(): edgeSet{ + usdAsset.String(): { eurAsset.String(): []xdr.OfferEntry{ eurUsdOffer, otherEurUsdOffer, }, }, - eurAsset.String(): edgeSet{ + eurAsset.String(): { usdAsset.String(): []xdr.OfferEntry{ usdEurOffer, }, }, }, tradingPairForOffer: map[xdr.Int64]tradingPair{ - quarterOffer.OfferId: tradingPair{ + quarterOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - fiftyCentsOffer.OfferId: tradingPair{ + fiftyCentsOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - dollarOffer.OfferId: tradingPair{ + dollarOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - eurOffer.OfferId: tradingPair{ + eurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - twoEurOffer.OfferId: tradingPair{ + twoEurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - threeEurOffer.OfferId: tradingPair{ + threeEurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - eurUsdOffer.OfferId: tradingPair{ + eurUsdOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: usdAsset.String(), }, - otherEurUsdOffer.OfferId: tradingPair{ + otherEurUsdOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: usdAsset.String(), }, - usdEurOffer.OfferId: tradingPair{ + usdEurOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: eurAsset.String(), }, @@ -574,7 +574,7 @@ func TestUpdateOfferOrderBook(t *testing.T) { expectedGraph := &OrderBookGraph{ edgesForSellingAsset: map[string]edgeSet{ - nativeAsset.String(): edgeSet{ + nativeAsset.String(): { usdAsset.String(): []xdr.OfferEntry{ quarterOffer, fiftyCentsOffer, @@ -586,52 +586,52 @@ func TestUpdateOfferOrderBook(t *testing.T) { threeEurOffer, }, }, - usdAsset.String(): edgeSet{ + usdAsset.String(): { eurAsset.String(): []xdr.OfferEntry{ otherEurUsdOffer, eurUsdOffer, }, }, - eurAsset.String(): edgeSet{ + eurAsset.String(): { usdAsset.String(): []xdr.OfferEntry{ usdEurOffer, }, }, }, tradingPairForOffer: map[xdr.Int64]tradingPair{ - quarterOffer.OfferId: tradingPair{ + quarterOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - fiftyCentsOffer.OfferId: tradingPair{ + fiftyCentsOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - dollarOffer.OfferId: tradingPair{ + dollarOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - eurOffer.OfferId: tradingPair{ + eurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - twoEurOffer.OfferId: tradingPair{ + twoEurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - threeEurOffer.OfferId: tradingPair{ + threeEurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - eurUsdOffer.OfferId: tradingPair{ + eurUsdOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: usdAsset.String(), }, - otherEurUsdOffer.OfferId: tradingPair{ + otherEurUsdOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: usdAsset.String(), }, - usdEurOffer.OfferId: tradingPair{ + usdEurOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: eurAsset.String(), }, @@ -753,7 +753,7 @@ func TestRemoveOfferOrderBook(t *testing.T) { expectedGraph := &OrderBookGraph{ edgesForSellingAsset: map[string]edgeSet{ - nativeAsset.String(): edgeSet{ + nativeAsset.String(): { usdAsset.String(): []xdr.OfferEntry{ quarterOffer, fiftyCentsOffer, @@ -764,34 +764,34 @@ func TestRemoveOfferOrderBook(t *testing.T) { threeEurOffer, }, }, - usdAsset.String(): edgeSet{ + usdAsset.String(): { eurAsset.String(): []xdr.OfferEntry{ eurUsdOffer, }, }, }, tradingPairForOffer: map[xdr.Int64]tradingPair{ - quarterOffer.OfferId: tradingPair{ + quarterOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - fiftyCentsOffer.OfferId: tradingPair{ + fiftyCentsOffer.OfferId: { buyingAsset: usdAsset.String(), sellingAsset: nativeAsset.String(), }, - eurOffer.OfferId: tradingPair{ + eurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - twoEurOffer.OfferId: tradingPair{ + twoEurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - threeEurOffer.OfferId: tradingPair{ + threeEurOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: nativeAsset.String(), }, - eurUsdOffer.OfferId: tradingPair{ + eurUsdOffer.OfferId: { buyingAsset: eurAsset.String(), sellingAsset: usdAsset.String(), }, @@ -940,6 +940,9 @@ func TestFindAsksAndBids(t *testing.T) { extraTwoEurOffers := []xdr.OfferEntry{} for i := 0; i < 4; i++ { otherTwoEurOffer := twoEurOffer + // make sure de-normalized prices are dealt with properly + otherTwoEurOffer.Price.N *= xdr.Int32(i + 1) + otherTwoEurOffer.Price.D *= xdr.Int32(i + 1) otherTwoEurOffer.OfferId += xdr.Int64(i + 17) graph.AddOffer(otherTwoEurOffer) extraTwoEurOffers = append(extraTwoEurOffers, otherTwoEurOffer) @@ -1204,7 +1207,7 @@ func TestConsumeOffersForBuyingAsset(t *testing.T) { func TestSortAndFilterPathsBySourceAsset(t *testing.T) { allPaths := []Path{ - Path{ + { SourceAmount: 3, SourceAsset: eurAsset, sourceAssetString: eurAsset.String(), @@ -1212,7 +1215,7 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 4, SourceAsset: eurAsset, sourceAssetString: eurAsset.String(), @@ -1220,7 +1223,7 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 1, SourceAsset: usdAsset, sourceAssetString: usdAsset.String(), @@ -1228,7 +1231,7 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 2, SourceAsset: eurAsset, sourceAssetString: eurAsset.String(), @@ -1236,7 +1239,7 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 2, SourceAsset: eurAsset, sourceAssetString: eurAsset.String(), @@ -1246,7 +1249,7 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 10, SourceAsset: nativeAsset, sourceAssetString: nativeAsset.String(), @@ -1264,14 +1267,14 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { t.Fatalf("unexpected error %v", err) } expectedPaths := []Path{ - Path{ + { SourceAmount: 2, SourceAsset: eurAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 2, SourceAsset: eurAsset, InteriorNodes: []xdr.Asset{ @@ -1280,21 +1283,21 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 3, SourceAsset: eurAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 1, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: yenAsset, DestinationAmount: 1000, }, - Path{ + { SourceAmount: 10, SourceAsset: nativeAsset, InteriorNodes: []xdr.Asset{}, @@ -1308,7 +1311,7 @@ func TestSortAndFilterPathsBySourceAsset(t *testing.T) { func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { allPaths := []Path{ - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, @@ -1316,7 +1319,7 @@ func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { destinationAssetString: eurAsset.String(), DestinationAmount: 3, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, @@ -1324,7 +1327,7 @@ func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { destinationAssetString: eurAsset.String(), DestinationAmount: 4, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, @@ -1332,7 +1335,7 @@ func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { destinationAssetString: usdAsset.String(), DestinationAmount: 1, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, sourceAssetString: eurAsset.String(), @@ -1341,7 +1344,7 @@ func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { destinationAssetString: eurAsset.String(), DestinationAmount: 2, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1351,7 +1354,7 @@ func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { destinationAssetString: eurAsset.String(), DestinationAmount: 2, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, @@ -1369,35 +1372,35 @@ func TestSortAndFilterPathsByDestinationAsset(t *testing.T) { t.Fatalf("unexpected error %v", err) } expectedPaths := []Path{ - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: eurAsset, DestinationAmount: 4, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: eurAsset, DestinationAmount: 3, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: eurAsset, DestinationAmount: 2, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: usdAsset, DestinationAmount: 1, }, - Path{ + { SourceAmount: 1000, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{}, @@ -1546,14 +1549,14 @@ func TestFindPaths(t *testing.T) { } expectedPaths := []Path{ - Path{ + { SourceAmount: 5, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 7, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{ @@ -1562,7 +1565,7 @@ func TestFindPaths(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1621,14 +1624,14 @@ func TestFindPaths(t *testing.T) { } expectedPaths = []Path{ - Path{ + { SourceAmount: 5, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 7, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{ @@ -1637,7 +1640,7 @@ func TestFindPaths(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 2, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1648,7 +1651,7 @@ func TestFindPaths(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1687,14 +1690,14 @@ func TestFindPaths(t *testing.T) { } expectedPaths = []Path{ - Path{ + { SourceAmount: 5, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 7, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{ @@ -1703,7 +1706,7 @@ func TestFindPaths(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 2, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1714,7 +1717,7 @@ func TestFindPaths(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1832,14 +1835,14 @@ func TestFindPathsStartingAt(t *testing.T) { } expectedPaths := []Path{ - Path{ + { SourceAmount: 5, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{}, DestinationAsset: nativeAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 5, SourceAsset: usdAsset, InteriorNodes: []xdr.Asset{ @@ -1885,7 +1888,7 @@ func TestFindPathsStartingAt(t *testing.T) { } expectedPaths = []Path{ - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1914,7 +1917,7 @@ func TestFindPathsStartingAt(t *testing.T) { } expectedPaths = []Path{ - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1925,7 +1928,7 @@ func TestFindPathsStartingAt(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 80, }, - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1954,7 +1957,7 @@ func TestFindPathsStartingAt(t *testing.T) { } expectedPaths = []Path{ - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1964,7 +1967,7 @@ func TestFindPathsStartingAt(t *testing.T) { DestinationAsset: usdAsset, DestinationAmount: 20, }, - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ @@ -1975,7 +1978,7 @@ func TestFindPathsStartingAt(t *testing.T) { DestinationAsset: nativeAsset, DestinationAmount: 80, }, - Path{ + { SourceAmount: 5, SourceAsset: yenAsset, InteriorNodes: []xdr.Asset{ diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md index e5478f1937..5dc87a2c70 100644 --- a/services/horizon/CHANGELOG.md +++ b/services/horizon/CHANGELOG.md @@ -6,6 +6,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased * Validate transaction hash IDs as 64 lowercase hex chars. As such, wrongly-formatted parameters which used to cause 404 (`Not found`) errors will now cause 400 (`Bad request`) HTTP errors. +* Fix ask and bid price levels of GET /order_book when encountering non-canonical price values. The `limit` parameter is now respected and levels are coallesced properly. Also, `price_r` is now in canonical form. ## v1.0.1 diff --git a/services/horizon/internal/actions/orderbook.go b/services/horizon/internal/actions/orderbook.go index 224fa07978..3d286808c7 100644 --- a/services/horizon/internal/actions/orderbook.go +++ b/services/horizon/internal/actions/orderbook.go @@ -85,17 +85,24 @@ type GetOrderbookHandler struct { func offersToPriceLevels(offers []xdr.OfferEntry, invert bool) ([]protocol.PriceLevel, error) { result := []protocol.PriceLevel{} + offersWithNormalizedPrices := []xdr.OfferEntry{} amountForPrice := map[xdr.Price]*big.Int{} + + // normalize offer prices and accumulate per-price amounts for _, offer := range offers { + offer.Price.Normalize() offerAmount := big.NewInt(int64(offer.Amount)) if amount, ok := amountForPrice[offer.Price]; ok { amount.Add(amount, offerAmount) } else { amountForPrice[offer.Price] = offerAmount } + offersWithNormalizedPrices = append(offersWithNormalizedPrices, offer) } - for _, offer := range offers { + + for _, offer := range offersWithNormalizedPrices { total, ok := amountForPrice[offer.Price] + // make we don't duplicate price-levels if !ok { continue } diff --git a/services/horizon/internal/actions/orderbook_test.go b/services/horizon/internal/actions/orderbook_test.go index 470529c3e6..d9e8083995 100644 --- a/services/horizon/internal/actions/orderbook_test.go +++ b/services/horizon/internal/actions/orderbook_test.go @@ -96,12 +96,12 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Bids: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "123", @@ -119,7 +119,7 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Bids: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", @@ -141,12 +141,12 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Asks: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "123", @@ -164,7 +164,7 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Asks: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", @@ -186,12 +186,12 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Bids: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "123", @@ -209,12 +209,12 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Bids: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 2, D: 1}, Price: "2.0", Amount: "123", @@ -236,12 +236,12 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Asks: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "123", @@ -259,12 +259,12 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Asks: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "12", @@ -286,19 +286,19 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Asks: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "123", }, }, Bids: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 3}, Price: "0.33", Amount: "13", @@ -316,19 +316,19 @@ func TestOrderBookResponseEquals(t *testing.T) { Type: "native", }, Bids: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 3}, Price: "0.33", Amount: "13", }, }, Asks: []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 2}, Price: "0.5", Amount: "123", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: 1, D: 1}, Price: "1.0", Amount: "123", @@ -492,7 +492,7 @@ func TestOrderbookGetResource(t *testing.T) { } asksButNoBidsResponse := empty asksButNoBidsResponse.Asks = []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(twoEurOffer.Price.N), D: int32(twoEurOffer.Price.D)}, Price: "2.0000000", Amount: "0.0000500", @@ -509,7 +509,7 @@ func TestOrderbookGetResource(t *testing.T) { } bidsButNoAsksResponse := empty bidsButNoAsksResponse.Bids = []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(sellEurOffer.Price.D), D: int32(sellEurOffer.Price.N)}, Price: "0.5000000", Amount: "0.0000500", @@ -528,48 +528,61 @@ func TestOrderbookGetResource(t *testing.T) { if err := fullGraph.Apply(4); err != nil { t.Fatalf("unexpected error %v", err) } + nonCanonicalPriceTwoEurOffer := twoEurOffer + nonCanonicalPriceTwoEurOffer.OfferId = 30 + // Add a separate offer with the same price value, but + // using a non-canonical representation, to make sure + // they are coalesced into the same price level + nonCanonicalPriceTwoEurOffer.Price.N *= 15 + nonCanonicalPriceTwoEurOffer.Price.D *= 15 + fullGraph.AddOffer(nonCanonicalPriceTwoEurOffer) + if err := fullGraph.Apply(5); err != nil { + t.Fatalf("unexpected error %v", err) + } threeEurOffer := twoEurOffer threeEurOffer.Price.N = 3 threeEurOffer.OfferId = 20 fullGraph.AddOffer(threeEurOffer) - if err := fullGraph.Apply(5); err != nil { + if err := fullGraph.Apply(6); err != nil { t.Fatalf("unexpected error %v", err) } sellEurOffer.Price.N = 9 sellEurOffer.Price.D = 10 fullGraph.AddOffer(sellEurOffer) - if err := fullGraph.Apply(6); err != nil { + if err := fullGraph.Apply(7); err != nil { t.Fatalf("unexpected error %v", err) } otherSellEurOffer := sellEurOffer otherSellEurOffer.OfferId = 17 - otherSellEurOffer.Price.N *= 2 + // sellEurOffer.Price * 2 + otherSellEurOffer.Price.N = 9 + otherSellEurOffer.Price.D = 5 fullGraph.AddOffer(otherSellEurOffer) - if err := fullGraph.Apply(7); err != nil { + if err := fullGraph.Apply(8); err != nil { t.Fatalf("unexpected error %v", err) } fullResponse := empty fullResponse.Asks = []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(twoEurOffer.Price.N), D: int32(twoEurOffer.Price.D)}, Price: "2.0000000", - Amount: "922337203685.4776307", + Amount: "922337203685.4776807", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(threeEurOffer.Price.N), D: int32(threeEurOffer.Price.D)}, Price: "3.0000000", Amount: "0.0000500", }, } fullResponse.Bids = []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(sellEurOffer.Price.D), D: int32(sellEurOffer.Price.N)}, Price: "1.1111111", Amount: "0.0000500", }, - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(otherSellEurOffer.Price.D), D: int32(otherSellEurOffer.Price.N)}, Price: "0.5555556", Amount: "0.0000500", @@ -578,14 +591,14 @@ func TestOrderbookGetResource(t *testing.T) { limitResponse := empty limitResponse.Asks = []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(twoEurOffer.Price.N), D: int32(twoEurOffer.Price.D)}, Price: "2.0000000", - Amount: "922337203685.4776307", + Amount: "922337203685.4776807", }, } limitResponse.Bids = []protocol.PriceLevel{ - protocol.PriceLevel{ + { PriceR: protocol.Price{N: int32(sellEurOffer.Price.D), D: int32(sellEurOffer.Price.N)}, Price: "1.1111111", Amount: "0.0000500", @@ -625,14 +638,14 @@ func TestOrderbookGetResource(t *testing.T) { fullGraph, 10, fullResponse, - "7", + "8", }, { "limit request", fullGraph, 1, limitResponse, - "7", + "8", }, } { t.Run(testCase.name, func(t *testing.T) { diff --git a/xdr/price.go b/xdr/price.go index c044fc279b..bfafb8bfc0 100644 --- a/xdr/price.go +++ b/xdr/price.go @@ -5,10 +5,42 @@ import ( ) // String returns a string represenation of `p` -func (p *Price) String() string { +func (p Price) String() string { return big.NewRat(int64(p.N), int64(p.D)).FloatString(7) } +// Equal returns whether the price's value is the same, +// taking into account denormalized representation +// (e.g. Price{1, 2}.EqualValue(Price{2,4}) == true ) +func (p Price) Equal(q Price) bool { + // See the Cheaper() method for the reasoning behind this: + return uint64(p.N)*uint64(q.D) == uint64(q.N)*uint64(p.D) +} + +// Cheaper indicates if the Price's value is lower, +// taking into account denormalized representation +// (e.g. Price{1, 2}.Cheaper(Price{2,4}) == false ) +func (p Price) Cheaper(q Price) bool { + // To avoid float precision issues when naively comparing Price.N/Price.D, + // we use the cross product instead: + // + // Price of p < Price of q + // <==> + // (p.N / p.D) < (q.N / q.D) + // <==> + // (p.N / p.D) * (p.D * q.D) < (q.N / q.D) * (p.D * q.D) + // <==> + // p.N * q.D < q.N * p.D + return uint64(p.N)*uint64(q.D) < uint64(q.N)*uint64(p.D) +} + +// Normalize sets Price to its rational canonical form +func (p *Price) Normalize() { + r := big.NewRat(int64(p.N), int64(p.D)) + p.N = Int32(r.Num().Int64()) + p.D = Int32(r.Denom().Int64()) +} + // Invert inverts Price. func (p *Price) Invert() { p.N, p.D = p.D, p.N diff --git a/xdr/price_test.go b/xdr/price_test.go index 5cbc9cbb86..c9c069ee73 100644 --- a/xdr/price_test.go +++ b/xdr/price_test.go @@ -1,21 +1,53 @@ package xdr_test import ( - . "github.com/stellar/go/xdr" + "testing" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" + + "github.com/stellar/go/xdr" ) -var _ = Describe("xdr.Price", func() { - Context("Price.Invert", func() { - price := Price{N: 1, D: 2} +func TestPriceInvert(t *testing.T) { + p := xdr.Price{N: 1, D: 2} + p.Invert() + assert.Equal(t, xdr.Price{N: 2, D: 1}, p) +} + +func TestPriceEqual(t *testing.T) { + // canonical + assert.True(t, xdr.Price{N: 1, D: 2}.Equal(xdr.Price{N: 1, D: 2})) + assert.False(t, xdr.Price{N: 1, D: 2}.Equal(xdr.Price{N: 2, D: 3})) + + // not canonical + assert.True(t, xdr.Price{N: 1, D: 2}.Equal(xdr.Price{N: 5, D: 10})) + assert.True(t, xdr.Price{N: 5, D: 10}.Equal(xdr.Price{N: 1, D: 2})) + assert.True(t, xdr.Price{N: 5, D: 10}.Equal(xdr.Price{N: 50, D: 100})) + assert.False(t, xdr.Price{N: 1, D: 3}.Equal(xdr.Price{N: 5, D: 10})) + assert.False(t, xdr.Price{N: 5, D: 10}.Equal(xdr.Price{N: 1, D: 3})) + assert.False(t, xdr.Price{N: 5, D: 15}.Equal(xdr.Price{N: 50, D: 100})) +} + +func TestPriceCheaper(t *testing.T) { + // canonical + assert.True(t, xdr.Price{N: 1, D: 4}.Cheaper(xdr.Price{N: 1, D: 3})) + assert.False(t, xdr.Price{N: 1, D: 3}.Cheaper(xdr.Price{N: 1, D: 4})) + assert.False(t, xdr.Price{N: 1, D: 4}.Cheaper(xdr.Price{N: 1, D: 4})) + + // not canonical + assert.True(t, xdr.Price{N: 10, D: 40}.Cheaper(xdr.Price{N: 3, D: 9})) + assert.False(t, xdr.Price{N: 3, D: 9}.Cheaper(xdr.Price{N: 10, D: 40})) + assert.False(t, xdr.Price{N: 10, D: 40}.Cheaper(xdr.Price{N: 10, D: 40})) +} - It("succeeds", func() { - price.Invert() +func TestNormalize(t *testing.T) { + // canonical + p := xdr.Price{N: 1, D: 4} + p.Normalize() + assert.Equal(t, xdr.Price{N: 1, D: 4}, p) - Expect(price.N).To(Equal(Int32(2))) - Expect(price.D).To(Equal(Int32(1))) - }) - }) -}) + // not canonical + p = xdr.Price{N: 500, D: 2000} + p.Normalize() + assert.Equal(t, xdr.Price{N: 1, D: 4}, p) +}